Re: [Intel-gfx] [PATCH 1/8] drm/omap: Simplify the rotation-on-crtc hack

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Tomi Valkeinen <tomi.valkeinen@ti.com>,
	DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH 1/8] drm/omap: Simplify the rotation-on-crtc hack
Date: Mon, 31 Jul 2017 14:48:32 +0300	[thread overview]
Message-ID: <5875908.nkarGct6Ju@avalon> (raw)
In-Reply-To: <CAKMK7uH61Q2Pve5s+hq9R=tmz46zwDE+F9m8aN2qce4WC0eVVg@mail.gmail.com>

Hi Daniel,

On Tuesday 25 Jul 2017 11:24:28 Daniel Vetter wrote:
> On Tue, Jul 25, 2017 at 10:47 AM, Maarten Lankhorst wrote:
> > Op 25-07-17 om 10:01 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!
> >> 
> >> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com
> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >> ---
> >> 
> >>  drivers/gpu/drm/omapdrm/omap_crtc.c | 64 ++++++++++---------------------
> >>  1 file changed, 28 insertions(+), 36 deletions(-)

[snip]

> > The comment about read lock is only valid when the plane is bound to the
> > crtc. In general this is not always the case. You can only peak at
> > plane->state when crtc->state->plane_mask & BIT(drm_plane_index(plane))
> > is true.
> > 
> > I think we might need -EDEADLK handling for getprop then, even though it's
> > not optimal. :(
>
> Well both the old an new way only worked because we grab all the locks
> unconditionally. And I'd really want to avoid get_prop being anything
> but a simple lookup. Unfortuantely that breaks omapdrm, so no idea
> what exactly to do here.
> 
> In a way adding properties without standardizing them across drivers
> first was a really bad idea, because then we have disjoint sets of
> uapi expectations, and there's just no way to make that work.
> 
> I guess one radical approach might be to make this the "standard", and
> just redirect rotation from the CRTC to the primary plane.
> 
> Or omapdrm needs to duplicate the property properly, and update one if
> the other is set. I think that's probably the most workable approach.

Maybe the first question we should answer is whether this hack is still needed 
in the omapdrm driver. Tomi, do you know whether userspace still needs this ?

-- 
Regards,

Laurent Pinchart

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