Hi Tomi,
Thank you for the patch.
On Friday 19 February 2016 11:48:07 Tomi Valkeinen wrote:
> Before universal planes we had to have plane specific properties for the
> crtc too, as on the hardware level a crtc uses a plane. In other words,
> e.g. 'zorder' property was added to both planes and crtcs, and
> omap_crtc.c would delegate the property set/get to the primary plane.
>
> However, the delegation was a bit too generic, delegating all property
> set/get calls to planes. Thus it's possible to set, say, FB_ID, on a
> crtc, which gets redirected to the primary plane.
>
> This is not standard, and shouldn't be allowed. To keep backward
> compatibility, we still need to redirect the properties we supported
> earlier for crtcs, namely 'zorder' and 'rotation'.
>
> This patch redirects only the allowed properties from crtcs to planes.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
> ---
> drivers/gpu/drm/omapdrm/omap_crtc.c | 58 +++++++++++++++++++++++-----------
> 1 file changed, 40 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 86b77c4f299a..280d80781635
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -419,24 +419,40 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> *crtc, }
> }
>
> +static bool omap_crtc_is_plane_prop(struct drm_device *dev,
> + struct drm_property *property)
> +{
> + struct omap_drm_private *priv = dev->dev_private;
> +
> + return property == priv->zorder_prop ||
s/return\t/return / ?
Apart from that,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Good catch.
> + property == dev->mode_config.rotation_property;
> +}
> +
> static int omap_crtc_atomic_set_property(struct drm_crtc *crtc,
> struct drm_crtc_state *state,
> struct drm_property *property,
> uint64_t val)
> {
> - struct drm_plane_state *plane_state;
> - struct drm_plane *plane = crtc->primary;
> + struct drm_device *dev = crtc->dev;
>
> - /*
> - * Delegate property set to the primary plane. Get the plane state and
> - * set the property directly.
> - */
> + if (omap_crtc_is_plane_prop(dev, property)) {
> + struct drm_plane_state *plane_state;
> + struct drm_plane *plane = crtc->primary;
> +
> + /*
> + * Delegate property set to the primary plane. Get the plane
> + * state and set the property directly.
> + */
>
> - plane_state = drm_atomic_get_plane_state(state->state, plane);
> - if (IS_ERR(plane_state))
> - return PTR_ERR(plane_state);
> + plane_state = drm_atomic_get_plane_state(state->state, plane);
> + if (IS_ERR(plane_state))
> + return PTR_ERR(plane_state);
>
> - return drm_atomic_plane_set_property(plane, plane_state, property, val);
> + return drm_atomic_plane_set_property(plane, plane_state,
> + property, val);
> + }
> +
> + return -EINVAL;
> }
>
> static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
> @@ -444,14 +460,20 @@ static int omap_crtc_atomic_get_property(struct
> drm_crtc *crtc, struct drm_property *property,
> uint64_t *val)
> {
> - /*
> - * Delegate property get to the primary plane. The
> - * drm_atomic_plane_get_property() function isn't exported, but can be
> - * called through drm_object_property_get_value() as that will call
> - * drm_atomic_get_property() for atomic drivers.
> - */
> - return drm_object_property_get_value(&crtc->primary->base, property,
> - val);
> + struct drm_device *dev = crtc->dev;
> +
> + if (omap_crtc_is_plane_prop(dev, property)) {
> + /*
> + * Delegate property get to the primary plane. The
> + * drm_atomic_plane_get_property() function isn't exported, but
> + * can be called through drm_object_property_get_value() as that
> + * will call drm_atomic_get_property() for atomic drivers.
> + */
> + return drm_object_property_get_value(&crtc->primary->base,
> + property, val);
> + }
> +
> + return -EINVAL;
> }
>
> static const struct drm_crtc_funcs omap_crtc_funcs = {
--
Regards,
Laurent Pinchart
