Re: [PATCH] drm/omap: Rework the rotation-on-crtc hack

[Maarten Lankhorst]

Op 04-08-17 om 12:02 schreef Daniel Vetter:
> On Fri, Aug 4, 2017 at 11:57 AM, Tomi Valkeinen  wrote:
>>> Here you go, compile tested version. :)
>>> 8<
>>> I want/need to rework the core property handling, and this hack is
>>> getting in the way. But since it's a non-standard propety only used by
>>> legacy userspace we know that this will only every be called from
>>> ioctl code. And never on some other free-standing state struct, where
>>> this old hack wouldn't work either.
>>>
>>> v2: don't forget zorder and get_property!
>>>
>>> v3: Shadow the legacy state to avoid locking issues in get_property
>>> (Maarten).
>>>
>>> v4: Review from Laurent
>>> - Move struct omap_crtc_state into omap_crtc.c
>>> - Clean up comments.
>>> - Don't forget to copy the shadowed state over on duplicate.
>>>
>>> v5: Don't forget to update the reset handler (Maarten).
>>> v6: Update omap_crtc_state shadow values in omap_crtc_atomic_check 
>>> (Maarten).
>>>
>>> Reviewed-by: Laurent Pinchart  (v4)
>>> Cc: Maarten Lankhorst 
>>> Cc: Tomi Valkeinen >> Cc: Laurent Pinchart 
>>> Signed-off-by: Daniel Vetter 
>>> Signed-off-by: Maarten Lankhorst 
>>> ---
>>>  drivers/gpu/drm/omapdrm/omap_crtc.c | 118 
>>> +---
>>>  1 file changed, 81 insertions(+), 37 deletions(-)
>> This makes all the CRTC properties disappear...
> Strange, we should still register them, that's really surprising.
>
>> This doesn't depend on anything in drm-next, does it? I'm currently on
>> v4.13-rc3. I'll look a bit more on what's going on later today.
> Not that I know of. Later patches will, but this one should be stand-alone.
> -Daniel

Oh derp, I also see what's going wrong..

omap_crtc_atomic_get_property returns the value, instead of setting *val and 
returning 0.

Next version coming up!

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/omap: Rework the rotation-on-crtc hack

[Maarten Lankhorst]

Op 04-08-17 om 12:02 schreef Daniel Vetter:
> On Fri, Aug 4, 2017 at 11:57 AM, Tomi Valkeinen  wrote:
>>> Here you go, compile tested version. :)
>>> 8<
>>> I want/need to rework the core property handling, and this hack is
>>> getting in the way. But since it's a non-standard propety only used by
>>> legacy userspace we know that this will only every be called from
>>> ioctl code. And never on some other free-standing state struct, where
>>> this old hack wouldn't work either.
>>>
>>> v2: don't forget zorder and get_property!
>>>
>>> v3: Shadow the legacy state to avoid locking issues in get_property
>>> (Maarten).
>>>
>>> v4: Review from Laurent
>>> - Move struct omap_crtc_state into omap_crtc.c
>>> - Clean up comments.
>>> - Don't forget to copy the shadowed state over on duplicate.
>>>
>>> v5: Don't forget to update the reset handler (Maarten).
>>> v6: Update omap_crtc_state shadow values in omap_crtc_atomic_check 
>>> (Maarten).
>>>
>>> Reviewed-by: Laurent Pinchart  (v4)
>>> Cc: Maarten Lankhorst 
>>> Cc: Tomi Valkeinen >> Cc: Laurent Pinchart 
>>> Signed-off-by: Daniel Vetter 
>>> Signed-off-by: Maarten Lankhorst 
>>> ---
>>>  drivers/gpu/drm/omapdrm/omap_crtc.c | 118 
>>> +---
>>>  1 file changed, 81 insertions(+), 37 deletions(-)
>> This makes all the CRTC properties disappear...
> Strange, we should still register them, that's really surprising.
>
>> This doesn't depend on anything in drm-next, does it? I'm currently on
>> v4.13-rc3. I'll look a bit more on what's going on later today.
> Not that I know of. Later patches will, but this one should be stand-alone.
> -Daniel

The properties should not disappear, does kms_properties pass?

~Maarten

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/omap: Rework the rotation-on-crtc hack

[Daniel Vetter]

On Fri, Aug 4, 2017 at 11:57 AM, Tomi Valkeinen  wrote:
>> Here you go, compile tested version. :)
>> 8<
>> I want/need to rework the core property handling, and this hack is
>> getting in the way. But since it's a non-standard propety only used by
>> legacy userspace we know that this will only every be called from
>> ioctl code. And never on some other free-standing state struct, where
>> this old hack wouldn't work either.
>>
>> v2: don't forget zorder and get_property!
>>
>> v3: Shadow the legacy state to avoid locking issues in get_property
>> (Maarten).
>>
>> v4: Review from Laurent
>> - Move struct omap_crtc_state into omap_crtc.c
>> - Clean up comments.
>> - Don't forget to copy the shadowed state over on duplicate.
>>
>> v5: Don't forget to update the reset handler (Maarten).
>> v6: Update omap_crtc_state shadow values in omap_crtc_atomic_check (Maarten).
>>
>> Reviewed-by: Laurent Pinchart  (v4)
>> Cc: Maarten Lankhorst 
>> Cc: Tomi Valkeinen > Cc: Laurent Pinchart 
>> Signed-off-by: Daniel Vetter 
>> Signed-off-by: Maarten Lankhorst 
>> ---
>>  drivers/gpu/drm/omapdrm/omap_crtc.c | 118 
>> +---
>>  1 file changed, 81 insertions(+), 37 deletions(-)
>
> This makes all the CRTC properties disappear...

Strange, we should still register them, that's really surprising.

> This doesn't depend on anything in drm-next, does it? I'm currently on
> v4.13-rc3. I'll look a bit more on what's going on later today.

Not that I know of. Later patches will, but this one should be stand-alone.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/omap: Rework the rotation-on-crtc hack

[Tomi Valkeinen]

Hi,

> Here you go, compile tested version. :)
> 8<
> I want/need to rework the core property handling, and this hack is
> getting in the way. But since it's a non-standard propety only used by
> legacy userspace we know that this will only every be called from
> ioctl code. And never on some other free-standing state struct, where
> this old hack wouldn't work either.
> 
> v2: don't forget zorder and get_property!
> 
> v3: Shadow the legacy state to avoid locking issues in get_property
> (Maarten).
> 
> v4: Review from Laurent
> - Move struct omap_crtc_state into omap_crtc.c
> - Clean up comments.
> - Don't forget to copy the shadowed state over on duplicate.
> 
> v5: Don't forget to update the reset handler (Maarten).
> v6: Update omap_crtc_state shadow values in omap_crtc_atomic_check (Maarten).
> 
> Reviewed-by: Laurent Pinchart  (v4)
> Cc: Maarten Lankhorst 
> Cc: Tomi Valkeinen  Cc: Laurent Pinchart 
> Signed-off-by: Daniel Vetter 
> Signed-off-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 118 
> +---
>  1 file changed, 81 insertions(+), 37 deletions(-)

This makes all the CRTC properties disappear...

This doesn't depend on anything in drm-next, does it? I'm currently on
v4.13-rc3. I'll look a bit more on what's going on later today.

 Tomi



signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/omap: Rework the rotation-on-crtc hack

[Daniel Vetter]

On Tue, Aug 1, 2017 at 12:20 PM, Laurent Pinchart
 wrote:
> On Tuesday 01 Aug 2017 07:59:13 Maarten Lankhorst wrote:
>> +   pri_state = drm_atomic_get_new_plane_state(crtc->primary,
>> state->state);
>> +   if (pri_state) {
>> +   struct omap_crtc_state *omap_crtc_state =
>> +   to_omap_crtc_state(state);
>> +
>> +   omap_crtc_state->zpos = pri_state->zpos;
>> +   omap_crtc_state->rotation = pri_state->rotation;
>> +   }
>>
>> That way even when updating the property through the primary plane, it gets
>> reflected correctly. For example when vt switching with fbdev.
>
> Let's not make it over-complicated. This hack is only needed fo the legacy X
> OMAP modesetting driver. The CRTC zpos and rotation properties should not be
> used through the atomic API.

Marten is right, the atomic properties are all such that you can
unconditionally restore them (e.g. fences report a no-op value), to
make compositor switching easier. Not sure anyone implements that, but
I think it's a useful idea to keep. I'll respin (or maybe Maarten
simply submits his patch with a proper sob ...).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/omap: Rework the rotation-on-crtc hack

[Laurent Pinchart]

Hi Maarten,

On Tuesday 01 Aug 2017 07:59:13 Maarten Lankhorst wrote:
> Op 31-07-17 om 17:42 schreef Daniel Vetter:
> > I want/need to rework the core property handling, and this hack is
> > getting in the way. But since it's a non-standard propety only used by
> > legacy userspace we know that this will only every be called from
> > ioctl code. And never on some other free-standing state struct, where
> > this old hack wouldn't work either.
> > 
> > v2: don't forget zorder and get_property!
> > 
> > v3: Shadow the legacy state to avoid locking issues in get_property
> > (Maarten).
> > 
> > v4: Review from Laurent
> > - Move struct omap_crtc_state into omap_crtc.c
> > - Clean up comments.
> > - Don't forget to copy the shadowed state over on duplicate.
> > 
> > v5: Don't forget to update the reset handler (Maarten).
> > 
> > Reviewed-by: Laurent Pinchart  (v4)
> > Cc: Maarten Lankhorst 
> > Cc: Tomi Valkeinen  > Cc: Laurent Pinchart 
> > Signed-off-by: Daniel Vetter 
> > ---
> > 
> >  drivers/gpu/drm/omapdrm/omap_crtc.c | 109 ---
> >  1 file changed, 72 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> > b/drivers/gpu/drm/omapdrm/omap_crtc.c index 14e8a7738b06..9014085c33df
> > 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c

[snip]

> >  static int omap_crtc_atomic_set_property(struct drm_crtc *crtc,
> >  struct drm_crtc_state *state,
> >  struct drm_property *property,
> >  uint64_t val)
> >  {
> > -   if (omap_crtc_is_plane_prop(crtc, 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);
> > +   struct omap_drm_private *priv = crtc->dev->dev_private;
> > +   struct omap_crtc_state *omap_state = to_omap_crtc_state(state);
> > +   struct drm_plane_state *plane_state;
> > 
> > -   return drm_atomic_plane_set_property(plane, plane_state,
> > -   property, val);
> > +   /*
> > +* Delegate property set to the primary plane. Get the plane state and
> > +* set the property directly, but keep a shadow copy for the
> > +* atomic_get_property callback.
> > +*/
> > +   plane_state = drm_atomic_get_plane_state(state->state, crtc->primary);
> > +   if (IS_ERR(plane_state))
> > +   return PTR_ERR(plane_state);
> > +
> > +   if (property == crtc->primary->rotation_property) {
> > +   plane_state->rotation = val;
> > +   omap_state->rotation = val;
> > +   } else if (property == priv->zorder_prop) {
> > +   plane_state->zpos = val;
> > +   omap_state->zpos = val;
> 
> With atomic we should try to always make the getprop values accurate, or
> compositors might have troubles restoring.
> 
> I would update the shadow values in omap_crtc_atomic_check through
> omap_crtc_atomic_check instead, with something like this:
> 
> +   pri_state = drm_atomic_get_new_plane_state(crtc->primary,
> state->state);
> +   if (pri_state) {
> +   struct omap_crtc_state *omap_crtc_state =
> +   to_omap_crtc_state(state);
> +
> +   omap_crtc_state->zpos = pri_state->zpos;
> +   omap_crtc_state->rotation = pri_state->rotation;
> +   }
> 
> That way even when updating the property through the primary plane, it gets
> reflected correctly. For example when vt switching with fbdev.

Let's not make it over-complicated. This hack is only needed fo the legacy X 
OMAP modesetting driver. The CRTC zpos and rotation properties should not be 
used through the atomic API.

> > +   } else {
> > +   return -EINVAL;
> > }
> > 
> > -   return -EINVAL;
> > +   return 0;
> >  }

-- 
Regards,

Laurent Pinchart

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/omap: Rework the rotation-on-crtc hack

[Maarten Lankhorst]

Op 31-07-17 om 17:42 schreef Daniel Vetter:
> I want/need to rework the core property handling, and this hack is
> getting in the way. But since it's a non-standard propety only used by
> legacy userspace we know that this will only every be called from
> ioctl code. And never on some other free-standing state struct, where
> this old hack wouldn't work either.
>
> v2: don't forget zorder and get_property!
>
> v3: Shadow the legacy state to avoid locking issues in get_property
> (Maarten).
>
> v4: Review from Laurent
> - Move struct omap_crtc_state into omap_crtc.c
> - Clean up comments.
> - Don't forget to copy the shadowed state over on duplicate.
>
> v5: Don't forget to update the reset handler (Maarten).
>
> Reviewed-by: Laurent Pinchart  (v4)
> Cc: Maarten Lankhorst 
> Cc: Tomi Valkeinen  Cc: Laurent Pinchart 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 109 
> 
>  1 file changed, 72 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c 
> b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index 14e8a7738b06..9014085c33df 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -26,6 +26,16 @@
>  
>  #include "omap_drv.h"
>  
> +#define to_omap_crtc_state(x) container_of(x, struct omap_crtc_state, base)
> +
> +struct omap_crtc_state {
> + /* Must be first. */
> + struct drm_crtc_state base;
> + /* Shadow values for legacy userspace support. */
> + unsigned int rotation;
> + unsigned int zpos;
> +};
> +
>  #define to_omap_crtc(x) container_of(x, struct omap_crtc, base)
>  
>  struct omap_crtc {
> @@ -498,39 +508,35 @@ static void omap_crtc_atomic_flush(struct drm_crtc 
> *crtc,
>   spin_unlock_irq(>dev->event_lock);
>  }
>  
> -static bool omap_crtc_is_plane_prop(struct drm_crtc *crtc,
> - struct drm_property *property)
> -{
> - struct drm_device *dev = crtc->dev;
> - struct omap_drm_private *priv = dev->dev_private;
> -
> - return property == priv->zorder_prop ||
> - property == crtc->primary->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)
>  {
> - if (omap_crtc_is_plane_prop(crtc, 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);
> + struct omap_drm_private *priv = crtc->dev->dev_private;
> + struct omap_crtc_state *omap_state = to_omap_crtc_state(state);
> + struct drm_plane_state *plane_state;
>  
> - return drm_atomic_plane_set_property(plane, plane_state,
> - property, val);
> + /*
> +  * Delegate property set to the primary plane. Get the plane state and
> +  * set the property directly, but keep a shadow copy for the
> +  * atomic_get_property callback.
> +  */
> + plane_state = drm_atomic_get_plane_state(state->state, crtc->primary);
> + if (IS_ERR(plane_state))
> + return PTR_ERR(plane_state);
> +
> + if (property == crtc->primary->rotation_property) {
> + plane_state->rotation = val;
> + omap_state->rotation = val;
> + } else if (property == priv->zorder_prop) {
> + plane_state->zpos = val;
> + omap_state->zpos = val;
With atomic we should try to always make the getprop values accurate, or 
compositors might have
troubles restoring.

I would update the shadow values in omap_crtc_atomic_check through 
omap_crtc_atomic_check instead,
with something like this:

+   pri_state = drm_atomic_get_new_plane_state(crtc->primary, state->state);
+   if (pri_state) {
+   struct omap_crtc_state *omap_crtc_state =
+   to_omap_crtc_state(state);
+
+   omap_crtc_state->zpos = pri_state->zpos;
+   omap_crtc_state->rotation = pri_state->rotation;
+   }

That way even when updating the property through the primary plane, it gets 
reflected correctly. For example when vt switching
with fbdev.

> + } else {
> + return -EINVAL;
>   }
>  
> - return -EINVAL;
> + return 0;
>  }
>  
>  static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
> @@ -538,28 +544,57 @@ static int 

Re: [PATCH] drm/omap: Rework the rotation-on-crtc hack

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Monday 31 Jul 2017 14:45:16 Daniel Vetter wrote:
> I want/need to rework the core property handling, and this hack is
> getting in the way. But since it's a non-standard propety only used by
> legacy userspace we know that this will only every be called from
> ioctl code. And never on some other free-standing state struct, where
> this old hack wouldn't work either.
> 
> v2: don't forget zorder and get_property!
> 
> v3: Shadow the legacy state to avoid locking issues in get_property
> (Maarten).
> 
> v4: Review from Laurent
> - Move struct omap_crtc_state into omap_crtc.c
> - Clean up comments.
> - Don't forget to copy the shadowed state over on duplicate.
> 
> Cc: Maarten Lankhorst 
> Cc: Tomi Valkeinen  Cc: Laurent Pinchart 
> Signed-off-by: Daniel Vetter 

I wish we could do better, but I think that's the best we can achieve without 
removing the hack.

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 95 --
>  1 file changed, 59 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 14e8a7738b06..16d8b291921b
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -26,6 +26,16 @@
> 
>  #include "omap_drv.h"
> 
> +#define to_omap_crtc_state(x) container_of(x, struct omap_crtc_state, base)
> +
> +struct omap_crtc_state {
> + /* Must be first. */
> + struct drm_crtc_state base;
> + /* Shadow values for legacy userspace support. */
> + unsigned int rotation;
> + unsigned int zpos;
> +};
> +
>  #define to_omap_crtc(x) container_of(x, struct omap_crtc, base)
> 
>  struct omap_crtc {
> @@ -498,39 +508,35 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> *crtc, spin_unlock_irq(>dev->event_lock);
>  }
> 
> -static bool omap_crtc_is_plane_prop(struct drm_crtc *crtc,
> - struct drm_property *property)
> -{
> - struct drm_device *dev = crtc->dev;
> - struct omap_drm_private *priv = dev->dev_private;
> -
> - return property == priv->zorder_prop ||
> - property == crtc->primary->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)
>  {
> - if (omap_crtc_is_plane_prop(crtc, 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);
> + struct omap_drm_private *priv = crtc->dev->dev_private;
> + struct omap_crtc_state *omap_state = to_omap_crtc_state(state);
> + struct drm_plane_state *plane_state;
> 
> - return drm_atomic_plane_set_property(plane, plane_state,
> - property, val);
> + /*
> +  * Delegate property set to the primary plane. Get the plane state and
> +  * set the property directly, but keep a shadow copy for the
> +  * atomic_get_property callback.
> +  */
> + plane_state = drm_atomic_get_plane_state(state->state, crtc->primary);
> + if (IS_ERR(plane_state))
> + return PTR_ERR(plane_state);
> +
> + if (property == crtc->primary->rotation_property) {
> + plane_state->rotation = val;
> + omap_state->rotation = val;
> + } else if (property == priv->zorder_prop) {
> + plane_state->zpos = val;
> + omap_state->zpos = val;
> + } else {
> + return -EINVAL;
>   }
> 
> - return -EINVAL;
> + return 0;
>  }
> 
>  static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
> @@ -538,20 +544,37 @@ static int omap_crtc_atomic_get_property(struct
> drm_crtc *crtc, struct drm_property *property,
>uint64_t *val)
>  {
> - if (omap_crtc_is_plane_prop(crtc, 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(>primary->base,
> - property, val);
> - }

Re: [PATCH] drm/omap: Rework the rotation-on-crtc hack

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Monday 31 Jul 2017 12:54:19 Daniel Vetter wrote:
> I want/need to rework the core property handling, and this hack is
> getting in the way. But since it's a non-standard propety only used by

s/propety/property/

> legacy userspace we know that this will only every be called from
> ioctl code. And never on some other free-standing state struct, where
> this old hack wouldn't work either.
> 
> v2: don't forget zorder and get_property!
> 
> v3: Shadow the legacy state to avoid locking issues in get_property
> (Maarten).
> 
> Cc: Maarten Lankhorst 
> Cc: Tomi Valkeinen  Cc: Laurent Pinchart 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 84 ++
>  drivers/gpu/drm/omapdrm/omap_drv.h  |  9 
>  2 files changed, 57 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 14e8a7738b06..fc8b25748f10
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -498,39 +498,34 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> *crtc, spin_unlock_irq(>dev->event_lock);
>  }
> 
> -static bool omap_crtc_is_plane_prop(struct drm_crtc *crtc,
> - struct drm_property *property)
> -{
> - struct drm_device *dev = crtc->dev;
> - struct omap_drm_private *priv = dev->dev_private;
> -
> - return property == priv->zorder_prop ||
> - property == crtc->primary->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)
>  {
> - if (omap_crtc_is_plane_prop(crtc, 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);
> + struct omap_drm_private *priv = crtc->dev->dev_private;
> + struct omap_crtc_state *omap_state = to_omap_crtc_state(state);
> + struct drm_plane_state *plane_state;
> 
> - return drm_atomic_plane_set_property(plane, plane_state,
> - property, val);
> + /*
> +  * Delegate property set to the primary plane. Get the plane
> +  * state and set the property directly.

Nitpicking, you can reformat the comment to go towards the 80 columns limit.

> +  */
> + plane_state = drm_atomic_get_plane_state(state->state, crtc->primary);
> + if (IS_ERR(plane_state))
> + return PTR_ERR(plane_state);
> +
> + if (property == crtc->primary->rotation_property) {
> + plane_state->rotation = val;
> + omap_state->rotation = val;
> + } else if (property == priv->zorder_prop) {
> + plane_state->zpos = val;
> + omap_state->zpos = val;
> + } else {
> + return -EINVAL;
>   }
> 
> - return -EINVAL;
> + return 0;
>  }
> 
>  static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
> @@ -538,20 +533,37 @@ static int omap_crtc_atomic_get_property(struct
> drm_crtc *crtc, struct drm_property *property,
>uint64_t *val)
>  {
> - if (omap_crtc_is_plane_prop(crtc, 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(>primary->base,
> - property, val);
> - }
> + struct omap_drm_private *priv = crtc->dev->dev_private;
> + struct omap_crtc_state *omap_state = to_omap_crtc_state(state);
> +
> + /*
> +  * Remap to the plane rotation/zorder property. We can peek at the 
plane
> +  * state directly since holding the crtc locks gives you a read-lock 
on
> +  * the plane state.

Isn't this comment outdated ?

> +  */
> + if (property == crtc->primary->rotation_property)
> + return omap_state->rotation;
> + else if (property == priv->zorder_prop)
> + return omap_state->zpos;
> 
>   return -EINVAL;
>  }
> 
> +static struct drm_crtc_state *
> +omap_crtc_duplicate_state(struct drm_crtc *crtc)