On Fri, Aug 07, 2020 at 09:33:16AM +0000, Simon Ser wrote: > On Friday, August 7, 2020 11:07 AM, Daniel Vetter <daniel@xxxxxxxx> 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@xxxxxxxxxxx> > > > Cc: Daniel Vetter <daniel@xxxxxxxx> > > > --- > > > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel