Re: [PATCH] drm: drivers may provide multiple primary planes per CRTC

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Aug 07, 2020 at 09:33:16AM +0000, Simon Ser wrote:
> On Friday, August 7, 2020 11:07 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, Aug 06, 2020 at 10:33:31AM +0000, Simon Ser wrote:
> > > Some drivers may expose primary planes compatible with multiple CRTCs.
> > > Make this clear in the docs: the current wording may be misunderstood as
> > > "exactly one primary plane per CRTC".
> > >
> > > Signed-off-by: Simon Ser <contact@emersion.fr>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > ---
> > >  drivers/gpu/drm/drm_plane.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > > index b7b90b3a2e38..108a922e8c23 100644
> > > --- a/drivers/gpu/drm/drm_plane.c
> > > +++ b/drivers/gpu/drm/drm_plane.c
> > > @@ -49,8 +49,8 @@
> > >   * &struct drm_plane (possibly as part of a larger structure) and registers it
> > >   * with a call to drm_universal_plane_init().
> > >   *
> > > - * Cursor and overlay planes are optional. All drivers should provide one
> > > - * primary plane per CRTC to avoid surprising userspace too much. See enum
> > > + * Cursor and overlay planes are optional. All drivers should provide at least
> > > + * one primary plane per CRTC to avoid surprising userspace too much. See enum
> >
> > I think that's even more confusing, since this reads like there could be
> > multiple primary planes for a specific CRTC. That's not the case, there'
> > only one pointer going from drm_crtc->primary to a drm_plane in the
> > kernel.
> >
> > The problem is that userspace doesn't have a drm_property to read this
> > pointer, and needs to guess.
> >
> > I thought the rule is:
> >
> > Nth primary plane (or cursor) is the primary plane for the Nth crtc.
> > Enumaration with increasing drm kms object ids.
> >
> > And I guess we should explain that on some hw any plane (including primary
> > ones, since that's only a sw construct) can be freely assinged to crtc.
> 
> Eh, wow. Okay, I didn't expect that. :P
> 
> What does drm_crtc->primary mean in case a GPU exposes more than one CRTC in
> the primary planes' possible_crtcs bitfield? Does this pointer change if I
> assign primary plane 2 to CRTC 1? Is the primary n → CRTC n assignment just a
> default?

Nope this pointer is entirely static. It's simply the plane that legacy
ioctls (SETCRTC, PAGE_FLIP, ...) will pick. If you've done something funny
and assigned that plane to a different crtc, then the page_flip or setcrtc
might actually fail, because we can't steal it.

Wrt the rule, it's just a guess. I think we should actually verify it at
drm_dev_register time, if we document it as uapi. So example, 2 crtc, 2
plane:

CRTC ids: 0, 1
PLANE ids: 2, 3

possible_crtc for both planes: 0x3

So here userspace should assume that plane id 2 is the primary plane for
crtc 0, and plane id 3 is the primary plane for crtc 1.

But if you only have one output, you can use both planes on a single crtc.

Now if you have more planes, e.g. 4 planes, then maybe the example is:

CRTC ids: 0, 1,
PLANE ids with primary tag: 2, 4
PLANE ids with overlay tag: 3, 5

Then plane id 2 is primary plane for crtc 0, and plane id 4 is primary
plane for crtc 1. It could also be that the crtc ids are interleaved in
the planes, depending how planes/crtc are initialized in the driver.

> Maybe we can just say that at most a single primary plane may be assigned to a
> CRTC, and that a primary plane may be compatible with multiple CRTCs?

Nope, see above example, if you have true universal plane hw you can
assign _all_ planes, including primary ones, to a single crtc. If the
memory bw and all these other constraints allow that ofc.

Primary plane is purely a tag for legacy uapi users, it tells you nothing
about what a plane can do.

> > Yes it's probably the most gloriously bonkers uapi we've come up with.
> > Might be so bad that a libdrm helper to look up the primary plane for a
> > crtc (or it's cursor plane if it exists) would be in order :-)
> 
> Well, if user-space can change drm_crtc->primary, I'm not sure this is a good
> idea.

Nope you cannot. It's just a tag for the kernel ioctls.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel