All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: drivers may provide multiple primary planes per CRTC
@ 2020-08-06 10:33 Simon Ser
  2020-08-07  9:07 ` Daniel Vetter
  0 siblings, 1 reply; 12+ messages in thread
From: Simon Ser @ 2020-08-06 10:33 UTC (permalink / raw)
  To: dri-devel

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
  * drm_plane_type for a more in-depth discussion of these special uapi-relevant
  * plane types. Special planes are associated with their CRTC by calling
  * drm_crtc_init_with_planes().
-- 
2.28.0


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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm: drivers may provide multiple primary planes per CRTC
  2020-08-06 10:33 [PATCH] drm: drivers may provide multiple primary planes per CRTC Simon Ser
@ 2020-08-07  9:07 ` Daniel Vetter
  2020-08-07  9:33   ` Simon Ser
  2020-08-07  9:38   ` Pekka Paalanen
  0 siblings, 2 replies; 12+ messages in thread
From: Daniel Vetter @ 2020-08-07  9:07 UTC (permalink / raw)
  To: Simon Ser; +Cc: dri-devel

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.

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 :-)

Cheers, Daniel


>   * drm_plane_type for a more in-depth discussion of these special uapi-relevant
>   * plane types. Special planes are associated with their CRTC by calling
>   * drm_crtc_init_with_planes().
> -- 
> 2.28.0
> 
> 

-- 
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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm: drivers may provide multiple primary planes per CRTC
  2020-08-07  9:07 ` Daniel Vetter
@ 2020-08-07  9:33   ` Simon Ser
  2020-08-07 13:03     ` Daniel Vetter
  2020-08-07  9:38   ` Pekka Paalanen
  1 sibling, 1 reply; 12+ messages in thread
From: Simon Ser @ 2020-08-07  9:33 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

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?

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?

> 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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm: drivers may provide multiple primary planes per CRTC
  2020-08-07  9:07 ` Daniel Vetter
  2020-08-07  9:33   ` Simon Ser
@ 2020-08-07  9:38   ` Pekka Paalanen
  2020-08-07 13:06     ` Daniel Vetter
  1 sibling, 1 reply; 12+ messages in thread
From: Pekka Paalanen @ 2020-08-07  9:38 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2793 bytes --]

On Fri, 7 Aug 2020 11:07:06 +0200
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.

There could be multiple primary planes *usable* for a specific CRTC but
just one used at a time, right?

> 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.

Why is that needed? With universal planes, I thought
drmModePlane::possible_crtcs bitmask is trustworthy?

In the legacy KMS UAPI you can't even pick your primary plane, because
it's implied in drmModeSetCrtc(), right?

> 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.
> 
> 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 :-)

I'm not sure I see the bonkers there.


Thanks,
pq

> 
> Cheers, Daniel
> 
> 
> >   * drm_plane_type for a more in-depth discussion of these special uapi-relevant
> >   * plane types. Special planes are associated with their CRTC by calling
> >   * drm_crtc_init_with_planes().
> > -- 
> > 2.28.0
> > 
> >   
> 


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm: drivers may provide multiple primary planes per CRTC
  2020-08-07  9:33   ` Simon Ser
@ 2020-08-07 13:03     ` Daniel Vetter
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2020-08-07 13:03 UTC (permalink / raw)
  To: Simon Ser; +Cc: dri-devel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm: drivers may provide multiple primary planes per CRTC
  2020-08-07  9:38   ` Pekka Paalanen
@ 2020-08-07 13:06     ` Daniel Vetter
  2020-12-06 15:24       ` Simon Ser
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2020-08-07 13:06 UTC (permalink / raw)
  To: Pekka Paalanen; +Cc: dri-devel

On Fri, Aug 07, 2020 at 12:38:02PM +0300, Pekka Paalanen wrote:
> On Fri, 7 Aug 2020 11:07:06 +0200
> 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.
> 
> There could be multiple primary planes *usable* for a specific CRTC but
> just one used at a time, right?

I'm not sure what you mean here, the crtc->primary link is invariant over
the lifetime of a driver load. You can't pick a different one, that's set
at driver init before drm_dev_register (and hence before userspace ever
sees anything).

> > 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.
> 
> Why is that needed? With universal planes, I thought
> drmModePlane::possible_crtcs bitmask is trustworthy?

Yes it should be.

> In the legacy KMS UAPI you can't even pick your primary plane, because
> it's implied in drmModeSetCrtc(), right?

Yup, I thought this all was so userspace knows which plane is the implied
one for legacy ioctls. Which does somewhat matter, since page_flip ioctl
has more features than atomic (target frame and async mode).

> > 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.
> > 
> > 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 :-)
> 
> I'm not sure I see the bonkers there.

Userspace has to guess which primary plane is for which crtc, at least if
the possible_crtc mask has more than one bit set. Which can happen.
-Daniel
> 
> 
> Thanks,
> pq
> 
> > 
> > Cheers, Daniel
> > 
> > 
> > >   * drm_plane_type for a more in-depth discussion of these special uapi-relevant
> > >   * plane types. Special planes are associated with their CRTC by calling
> > >   * drm_crtc_init_with_planes().
> > > -- 
> > > 2.28.0
> > > 
> > >   
> > 
> 



-- 
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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm: drivers may provide multiple primary planes per CRTC
  2020-08-07 13:06     ` Daniel Vetter
@ 2020-12-06 15:24       ` Simon Ser
  2020-12-07  8:45         ` Pekka Paalanen
  0 siblings, 1 reply; 12+ messages in thread
From: Simon Ser @ 2020-12-06 15:24 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

Sorry, I think I lost track of this thread at some point and forgot
about it. That said…

On Friday, August 7th, 2020 at 3:06 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Fri, Aug 07, 2020 at 12:38:02PM +0300, Pekka Paalanen wrote:
> > On Fri, 7 Aug 2020 11:07:06 +0200
> > 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.
> >
> > There could be multiple primary planes *usable* for a specific CRTC but
> > just one used at a time, right?
>
> I'm not sure what you mean here, the crtc->primary link is invariant over
> the lifetime of a driver load. You can't pick a different one, that's set
> at driver init before drm_dev_register (and hence before userspace ever
> sees anything).

OK. I'm personally not very interested in documenting legacy bits, so I'll skip
that. I'm mainly interested here in making it clear possible_crtcs for a
primary plane can have more than one bit set. Because of the paragraph in the
current docs, some user-space developers have understood "more than one bit set
in possible_crtcs for a primary plane is a kernel bug".

I'll send a v2 that makes it clear these pointers are for legacy uAPI.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm: drivers may provide multiple primary planes per CRTC
  2020-12-06 15:24       ` Simon Ser
@ 2020-12-07  8:45         ` Pekka Paalanen
  2020-12-07  9:10           ` Simon Ser
  0 siblings, 1 reply; 12+ messages in thread
From: Pekka Paalanen @ 2020-12-07  8:45 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 4174 bytes --]

On Sun, 06 Dec 2020 15:24:29 +0000
Simon Ser <contact@emersion.fr> wrote:

> Sorry, I think I lost track of this thread at some point and forgot
> about it. That said…
> 
> On Friday, August 7th, 2020 at 3:06 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Fri, Aug 07, 2020 at 12:38:02PM +0300, Pekka Paalanen wrote:  
> > > On Fri, 7 Aug 2020 11:07:06 +0200
> > > 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.  
> > >
> > > There could be multiple primary planes *usable* for a specific CRTC but
> > > just one used at a time, right?  
> >
> > I'm not sure what you mean here, the crtc->primary link is invariant over
> > the lifetime of a driver load. You can't pick a different one, that's set
> > at driver init before drm_dev_register (and hence before userspace ever
> > sees anything).  
> 
> OK. I'm personally not very interested in documenting legacy bits, so I'll skip
> that. I'm mainly interested here in making it clear possible_crtcs for a
> primary plane can have more than one bit set. Because of the paragraph in the
> current docs, some user-space developers have understood "more than one bit set
> in possible_crtcs for a primary plane is a kernel bug".
> 
> I'll send a v2 that makes it clear these pointers are for legacy uAPI.

Right, so this and what danvet said seem to be in direct conflict in
atomic uAPI, repeating above:

> > I'm not sure what you mean here, the crtc->primary link is invariant over
> > the lifetime of a driver load. You can't pick a different one, that's set
> > at driver init before drm_dev_register (and hence before userspace ever
> > sees anything).  

But still, it is considered not a kernel bug that a primary plane has
more than one bit set in its possible_crtcs.

If a primary plane has more than one bit set in possible_crtcs, and it
is not a kernel bug, then userspace expects to be able to choose any
of the multiple indicated possible CRTCs for this primary plane.

Which way is it?

Or, is there a different limitation that for each CRTC, there must be
exactly one primary plane with that CRTCs bit set in its possible_crtcs?

IOW, you can have more CRTCs than primary planes in total, and you can
activate each CRTC alone, but you cannot activate all CRTCs
simultaneously because there are not enough primary planes for them?

Representing it mathematically, the possible assignments according to
possible_crtcs while ignoring all other limitations are:
N CRTCs <-> M primary planes

- Is N one or greater than one?
- Is M one or greater than one?


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm: drivers may provide multiple primary planes per CRTC
  2020-12-07  8:45         ` Pekka Paalanen
@ 2020-12-07  9:10           ` Simon Ser
  2020-12-09  0:36             ` Daniel Vetter
  0 siblings, 1 reply; 12+ messages in thread
From: Simon Ser @ 2020-12-07  9:10 UTC (permalink / raw)
  To: Pekka Paalanen; +Cc: dri-devel

On Monday, December 7th, 2020 at 9:45 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> > > > > > - * 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.
> > > >
> > > > There could be multiple primary planes *usable* for a specific CRTC but
> > > > just one used at a time, right?
> > >
> > > I'm not sure what you mean here, the crtc->primary link is invariant over
> > > the lifetime of a driver load. You can't pick a different one, that's set
> > > at driver init before drm_dev_register (and hence before userspace ever
> > > sees anything).
> >
> > OK. I'm personally not very interested in documenting legacy bits, so I'll skip
> > that. I'm mainly interested here in making it clear possible_crtcs for a
> > primary plane can have more than one bit set. Because of the paragraph in the
> > current docs, some user-space developers have understood "more than one bit set
> > in possible_crtcs for a primary plane is a kernel bug".
> >
> > I'll send a v2 that makes it clear these pointers are for legacy uAPI.
>
> Right, so this and what danvet said seem to be in direct conflict in
> atomic uAPI, repeating above:
>
> > > I'm not sure what you mean here, the crtc->primary link is invariant over
> > > the lifetime of a driver load. You can't pick a different one, that's set
> > > at driver init before drm_dev_register (and hence before userspace ever
> > > sees anything).
>
> But still, it is considered not a kernel bug that a primary plane has
> more than one bit set in its possible_crtcs.
>
> If a primary plane has more than one bit set in possible_crtcs, and it
> is not a kernel bug, then userspace expects to be able to choose any
> of the multiple indicated possible CRTCs for this primary plane.
>
> Which way is it?
>
> Or, is there a different limitation that for each CRTC, there must be
> exactly one primary plane with that CRTCs bit set in its possible_crtcs?
>
> IOW, you can have more CRTCs than primary planes in total, and you can
> activate each CRTC alone, but you cannot activate all CRTCs
> simultaneously because there are not enough primary planes for them?
>
> Representing it mathematically, the possible assignments according to
> possible_crtcs while ignoring all other limitations are:
> N CRTCs <-> M primary planes
>
> - Is N one or greater than one?
> - Is M one or greater than one?

I think the current situation is that:

- It's perfectly fine for a driver to expose multiple bits in possible_crtcs.
  User-space can attach the primary plane to any of these CRTCs (of course, a
  primary plane still can only be attached to a single CRTC at a time). Drivers
  should provide as many primary planes as there are CRTCs.
- The legacy API doesn't expose primary planes. Some legacy IOCTLs like
  drmModeSetCrtc allow user-space to attach a FB directly to a CRTC. The driver
  needs to implicitly select a primary plane for this operation. That's the
  only case where the internal CRTC → primary plane link is used in the kernel.

Is this correct, Daniel?

So I believe M > 1 and N > 1 is possible and isn't a kernel bug. For instance
some drivers hardcode possible_crtcs to 0xFF (although it might be nicer to
user-space to set the bitmask depending on the number of CRTCs, to avoid
setting bits for non-existing CRTCs).

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm: drivers may provide multiple primary planes per CRTC
  2020-12-07  9:10           ` Simon Ser
@ 2020-12-09  0:36             ` Daniel Vetter
  2020-12-09  8:23               ` Pekka Paalanen
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2020-12-09  0:36 UTC (permalink / raw)
  To: Simon Ser; +Cc: dri-devel

On Mon, Dec 07, 2020 at 09:10:00AM +0000, Simon Ser wrote:
> On Monday, December 7th, 2020 at 9:45 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> 
> > > > > > > - * 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.
> > > > >
> > > > > There could be multiple primary planes *usable* for a specific CRTC but
> > > > > just one used at a time, right?
> > > >
> > > > I'm not sure what you mean here, the crtc->primary link is invariant over
> > > > the lifetime of a driver load. You can't pick a different one, that's set
> > > > at driver init before drm_dev_register (and hence before userspace ever
> > > > sees anything).
> > >
> > > OK. I'm personally not very interested in documenting legacy bits, so I'll skip
> > > that. I'm mainly interested here in making it clear possible_crtcs for a
> > > primary plane can have more than one bit set. Because of the paragraph in the
> > > current docs, some user-space developers have understood "more than one bit set
> > > in possible_crtcs for a primary plane is a kernel bug".
> > >
> > > I'll send a v2 that makes it clear these pointers are for legacy uAPI.
> >
> > Right, so this and what danvet said seem to be in direct conflict in
> > atomic uAPI, repeating above:
> >
> > > > I'm not sure what you mean here, the crtc->primary link is invariant over
> > > > the lifetime of a driver load. You can't pick a different one, that's set
> > > > at driver init before drm_dev_register (and hence before userspace ever
> > > > sees anything).
> >
> > But still, it is considered not a kernel bug that a primary plane has
> > more than one bit set in its possible_crtcs.
> >
> > If a primary plane has more than one bit set in possible_crtcs, and it
> > is not a kernel bug, then userspace expects to be able to choose any
> > of the multiple indicated possible CRTCs for this primary plane.
> >
> > Which way is it?
> >
> > Or, is there a different limitation that for each CRTC, there must be
> > exactly one primary plane with that CRTCs bit set in its possible_crtcs?
> >
> > IOW, you can have more CRTCs than primary planes in total, and you can
> > activate each CRTC alone, but you cannot activate all CRTCs
> > simultaneously because there are not enough primary planes for them?
> >
> > Representing it mathematically, the possible assignments according to
> > possible_crtcs while ignoring all other limitations are:
> > N CRTCs <-> M primary planes
> >
> > - Is N one or greater than one?
> > - Is M one or greater than one?
> 
> I think the current situation is that:
> 
> - It's perfectly fine for a driver to expose multiple bits in possible_crtcs.
>   User-space can attach the primary plane to any of these CRTCs (of course, a
>   primary plane still can only be attached to a single CRTC at a time). Drivers
>   should provide as many primary planes as there are CRTCs.
> - The legacy API doesn't expose primary planes. Some legacy IOCTLs like
>   drmModeSetCrtc allow user-space to attach a FB directly to a CRTC. The driver
>   needs to implicitly select a primary plane for this operation. That's the
>   only case where the internal CRTC → primary plane link is used in the kernel.
> 
> Is this correct, Daniel?

Yup. atomic doesn't use crtc->primary link at all.

Pekka, where did you see an indication that this crtc->primary link is
used for atomic? My statement was only about legacy ioctl impact of
->primary. Atomic userspace can pick any plane it wants and consider that
the "primary" one (the hw/driver might reject that, but that's a different
issue).

> So I believe M > 1 and N > 1 is possible and isn't a kernel bug. For instance
> some drivers hardcode possible_crtcs to 0xFF (although it might be nicer to
> user-space to set the bitmask depending on the number of CRTCs, to avoid
> setting bits for non-existing CRTCs).

possible_crtcs for a primary plane has exactly the same constraints as
possible_crtcs for any other plane. The only additional constraint there
is that:
- first primary plane you iterate must have the first bit set in
  possible_crtcs, and it is the primary plane for that crtc
- 2nd primary plane has the 2nd bit set in possible_crtcs, and it is the
  primary plane for that crtc

and so on. That's all. I'm not sure all drivers get this right, so I think
it'd be good to check that at drm_dev_register time (we check a few other
things about these possible_crtcs masks already, so it's a good fit).
-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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm: drivers may provide multiple primary planes per CRTC
  2020-12-09  0:36             ` Daniel Vetter
@ 2020-12-09  8:23               ` Pekka Paalanen
  2020-12-09 10:40                 ` Daniel Vetter
  0 siblings, 1 reply; 12+ messages in thread
From: Pekka Paalanen @ 2020-12-09  8:23 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 6887 bytes --]

On Wed, 9 Dec 2020 01:36:37 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Mon, Dec 07, 2020 at 09:10:00AM +0000, Simon Ser wrote:
> > On Monday, December 7th, 2020 at 9:45 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> >   
> > > > > > > > - * 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.  

So this comment from Daniel is actually wrong, because possible_crtcs
mask for a primary plane can have more than one bit set, and Simon's
wording is accurate.

Except "should" should be "must", see below.

> > > > > >
> > > > > > There could be multiple primary planes *usable* for a specific CRTC but
> > > > > > just one used at a time, right?  
> > > > >
> > > > > I'm not sure what you mean here, the crtc->primary link is invariant over
> > > > > the lifetime of a driver load. You can't pick a different one, that's set
> > > > > at driver init before drm_dev_register (and hence before userspace ever
> > > > > sees anything).  
> > > >
> > > > OK. I'm personally not very interested in documenting legacy bits, so I'll skip
> > > > that. I'm mainly interested here in making it clear possible_crtcs for a
> > > > primary plane can have more than one bit set. Because of the paragraph in the
> > > > current docs, some user-space developers have understood "more than one bit set
> > > > in possible_crtcs for a primary plane is a kernel bug".
> > > >
> > > > I'll send a v2 that makes it clear these pointers are for legacy uAPI.  
> > >
> > > Right, so this and what danvet said seem to be in direct conflict in
> > > atomic uAPI, repeating above:
> > >  
> > > > > I'm not sure what you mean here, the crtc->primary link is invariant over
> > > > > the lifetime of a driver load. You can't pick a different one, that's set
> > > > > at driver init before drm_dev_register (and hence before userspace ever
> > > > > sees anything).  
> > >
> > > But still, it is considered not a kernel bug that a primary plane has
> > > more than one bit set in its possible_crtcs.
> > >
> > > If a primary plane has more than one bit set in possible_crtcs, and it
> > > is not a kernel bug, then userspace expects to be able to choose any
> > > of the multiple indicated possible CRTCs for this primary plane.
> > >
> > > Which way is it?
> > >
> > > Or, is there a different limitation that for each CRTC, there must be
> > > exactly one primary plane with that CRTCs bit set in its possible_crtcs?
> > >
> > > IOW, you can have more CRTCs than primary planes in total, and you can
> > > activate each CRTC alone, but you cannot activate all CRTCs
> > > simultaneously because there are not enough primary planes for them?
> > >
> > > Representing it mathematically, the possible assignments according to
> > > possible_crtcs while ignoring all other limitations are:
> > > N CRTCs <-> M primary planes
> > >
> > > - Is N one or greater than one?
> > > - Is M one or greater than one?  
> > 
> > I think the current situation is that:
> > 
> > - It's perfectly fine for a driver to expose multiple bits in possible_crtcs.
> >   User-space can attach the primary plane to any of these CRTCs (of course, a
> >   primary plane still can only be attached to a single CRTC at a time). Drivers
> >   should provide as many primary planes as there are CRTCs.
> > - The legacy API doesn't expose primary planes. Some legacy IOCTLs like
> >   drmModeSetCrtc allow user-space to attach a FB directly to a CRTC. The driver
> >   needs to implicitly select a primary plane for this operation. That's the
> >   only case where the internal CRTC → primary plane link is used in the kernel.
> > 
> > Is this correct, Daniel?  
> 
> Yup. atomic doesn't use crtc->primary link at all.
> 
> Pekka, where did you see an indication that this crtc->primary link is
> used for atomic?

Hi Daniel,

I was asking about KMS in general, atomic included, and you were
talking about that variable, so I got really confused. I do not care
about kernel internals, only uAPI.

> My statement was only about legacy ioctl impact of
> ->primary. Atomic userspace can pick any plane it wants and consider that  
> the "primary" one (the hw/driver might reject that, but that's a different
> issue).

Even from non-primary planes?

Ok.

But to keep things simpler in userspace, userspace will probably settle
to always use exactly one primary type KMS plane for any CRTC it is
using.

> > So I believe M > 1 and N > 1 is possible and isn't a kernel bug. For instance
> > some drivers hardcode possible_crtcs to 0xFF (although it might be nicer to
> > user-space to set the bitmask depending on the number of CRTCs, to avoid
> > setting bits for non-existing CRTCs).  
> 
> possible_crtcs for a primary plane has exactly the same constraints as
> possible_crtcs for any other plane. The only additional constraint there
> is that:
> - first primary plane you iterate must have the first bit set in
>   possible_crtcs, and it is the primary plane for that crtc
> - 2nd primary plane has the 2nd bit set in possible_crtcs, and it is the
>   primary plane for that crtc
> 
> and so on. That's all. I'm not sure all drivers get this right, so I think
> it'd be good to check that at drm_dev_register time (we check a few other
> things about these possible_crtcs masks already, so it's a good fit).

That seems to create a 1:1 relationship between CRTCs and primary
planes, but additionally allowing other associations as well.

This means that there cannot be two primary planes that are *only*
possible with the same CRTC.

Also, there cannot be two CRTCs that could *only* be possible with the
same primary plane.

In other words, enabling a CRTC never fails because you run out of
primary planes, assuming you use one plane in total for each CRTC.

I understand that the answer to my question "There could be multiple
primary planes *usable* for a specific CRTC but just one used at a
time, right?" is largely "Yes." because possible_crtcs mask for any
plane can have more than one bit set. Whether it is actually possible
to use two KMS planes of type primary simultaneously on the same CRTC
is up to a driver, I guess, but not forbidden, yet unlikely to work.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm: drivers may provide multiple primary planes per CRTC
  2020-12-09  8:23               ` Pekka Paalanen
@ 2020-12-09 10:40                 ` Daniel Vetter
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2020-12-09 10:40 UTC (permalink / raw)
  To: Pekka Paalanen; +Cc: dri-devel

On Wed, Dec 9, 2020 at 9:23 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>
> On Wed, 9 Dec 2020 01:36:37 +0100
> Daniel Vetter <daniel@ffwll.ch> wrote:
>
> > On Mon, Dec 07, 2020 at 09:10:00AM +0000, Simon Ser wrote:
> > > On Monday, December 7th, 2020 at 9:45 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> > >
> > > > > > > > > - * 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.
>
> So this comment from Daniel is actually wrong, because possible_crtcs
> mask for a primary plane can have more than one bit set, and Simon's
> wording is accurate.

My comment is right: There's two directions where you can multiple:

- A given CRTC only has one primary plane. It can have a lot more
other planes, and some of these planes can also be of type primary,
but they're primary planes for a different CRTC.

- A given primary plane must have it's corresponding CRTC in its
possible_crtcs mask. But it can have a lot more.

I think the confusion we're having here is the difference between "the
primary plane for a specific CRTC" and "a primary plane for any CRTC".
A crtc has only one of the former, but can have a bunch more of the
latter (if you have freely reassignable planes that all have the same
feature, as an example).

> Except "should" should be "must", see below.

Not in what I wrote.

> > > > > > >
> > > > > > > There could be multiple primary planes *usable* for a specific CRTC but
> > > > > > > just one used at a time, right?
> > > > > >
> > > > > > I'm not sure what you mean here, the crtc->primary link is invariant over
> > > > > > the lifetime of a driver load. You can't pick a different one, that's set
> > > > > > at driver init before drm_dev_register (and hence before userspace ever
> > > > > > sees anything).
> > > > >
> > > > > OK. I'm personally not very interested in documenting legacy bits, so I'll skip
> > > > > that. I'm mainly interested here in making it clear possible_crtcs for a
> > > > > primary plane can have more than one bit set. Because of the paragraph in the
> > > > > current docs, some user-space developers have understood "more than one bit set
> > > > > in possible_crtcs for a primary plane is a kernel bug".
> > > > >
> > > > > I'll send a v2 that makes it clear these pointers are for legacy uAPI.
> > > >
> > > > Right, so this and what danvet said seem to be in direct conflict in
> > > > atomic uAPI, repeating above:
> > > >
> > > > > > I'm not sure what you mean here, the crtc->primary link is invariant over
> > > > > > the lifetime of a driver load. You can't pick a different one, that's set
> > > > > > at driver init before drm_dev_register (and hence before userspace ever
> > > > > > sees anything).
> > > >
> > > > But still, it is considered not a kernel bug that a primary plane has
> > > > more than one bit set in its possible_crtcs.
> > > >
> > > > If a primary plane has more than one bit set in possible_crtcs, and it
> > > > is not a kernel bug, then userspace expects to be able to choose any
> > > > of the multiple indicated possible CRTCs for this primary plane.
> > > >
> > > > Which way is it?
> > > >
> > > > Or, is there a different limitation that for each CRTC, there must be
> > > > exactly one primary plane with that CRTCs bit set in its possible_crtcs?
> > > >
> > > > IOW, you can have more CRTCs than primary planes in total, and you can
> > > > activate each CRTC alone, but you cannot activate all CRTCs
> > > > simultaneously because there are not enough primary planes for them?
> > > >
> > > > Representing it mathematically, the possible assignments according to
> > > > possible_crtcs while ignoring all other limitations are:
> > > > N CRTCs <-> M primary planes
> > > >
> > > > - Is N one or greater than one?
> > > > - Is M one or greater than one?
> > >
> > > I think the current situation is that:
> > >
> > > - It's perfectly fine for a driver to expose multiple bits in possible_crtcs.
> > >   User-space can attach the primary plane to any of these CRTCs (of course, a
> > >   primary plane still can only be attached to a single CRTC at a time). Drivers
> > >   should provide as many primary planes as there are CRTCs.
> > > - The legacy API doesn't expose primary planes. Some legacy IOCTLs like
> > >   drmModeSetCrtc allow user-space to attach a FB directly to a CRTC. The driver
> > >   needs to implicitly select a primary plane for this operation. That's the
> > >   only case where the internal CRTC → primary plane link is used in the kernel.
> > >
> > > Is this correct, Daniel?
> >
> > Yup. atomic doesn't use crtc->primary link at all.
> >
> > Pekka, where did you see an indication that this crtc->primary link is
> > used for atomic?
>
> Hi Daniel,
>
> I was asking about KMS in general, atomic included, and you were
> talking about that variable, so I got really confused. I do not care
> about kernel internals, only uAPI.

There is an uapi guarantee (which we're trying to document) about how
userspace can figure out the crtc->primary pointer. I thought that's
what this entire discussion is all about here.

> > My statement was only about legacy ioctl impact of
> > ->primary. Atomic userspace can pick any plane it wants and consider that
> > the "primary" one (the hw/driver might reject that, but that's a different
> > issue).
>
> Even from non-primary planes?

If the hw has totally interchangable and reassignable planes, then
yes.  You could also try to use the cursor as your primary/single
plane.

> Ok.
>
> But to keep things simpler in userspace, userspace will probably settle
> to always use exactly one primary type KMS plane for any CRTC it is
> using.
>
> > > So I believe M > 1 and N > 1 is possible and isn't a kernel bug. For instance
> > > some drivers hardcode possible_crtcs to 0xFF (although it might be nicer to
> > > user-space to set the bitmask depending on the number of CRTCs, to avoid
> > > setting bits for non-existing CRTCs).
> >
> > possible_crtcs for a primary plane has exactly the same constraints as
> > possible_crtcs for any other plane. The only additional constraint there
> > is that:
> > - first primary plane you iterate must have the first bit set in
> >   possible_crtcs, and it is the primary plane for that crtc
> > - 2nd primary plane has the 2nd bit set in possible_crtcs, and it is the
> >   primary plane for that crtc
> >
> > and so on. That's all. I'm not sure all drivers get this right, so I think
> > it'd be good to check that at drm_dev_register time (we check a few other
> > things about these possible_crtcs masks already, so it's a good fit).
>
> That seems to create a 1:1 relationship between CRTCs and primary
> planes, but additionally allowing other associations as well.
>
> This means that there cannot be two primary planes that are *only*
> possible with the same CRTC.

Yup.

> Also, there cannot be two CRTCs that could *only* be possible with the
> same primary plane.

Yup.

> In other words, enabling a CRTC never fails because you run out of
> primary planes, assuming you use one plane in total for each CRTC.

Yeah that's what should be the case. Of course you might hit random
other restrictions that make these planes unusable. But as uapi
objects they exist.

> I understand that the answer to my question "There could be multiple
> primary planes *usable* for a specific CRTC but just one used at a
> time, right?" is largely "Yes." because possible_crtcs mask for any
> plane can have more than one bit set. Whether it is actually possible
> to use two KMS planes of type primary simultaneously on the same CRTC
> is up to a driver, I guess, but not forbidden, yet unlikely to work.

This reads correctly to me.
-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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-12-09 10:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-06 10:33 [PATCH] drm: drivers may provide multiple primary planes per CRTC Simon Ser
2020-08-07  9:07 ` Daniel Vetter
2020-08-07  9:33   ` Simon Ser
2020-08-07 13:03     ` Daniel Vetter
2020-08-07  9:38   ` Pekka Paalanen
2020-08-07 13:06     ` Daniel Vetter
2020-12-06 15:24       ` Simon Ser
2020-12-07  8:45         ` Pekka Paalanen
2020-12-07  9:10           ` Simon Ser
2020-12-09  0:36             ` Daniel Vetter
2020-12-09  8:23               ` Pekka Paalanen
2020-12-09 10:40                 ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.