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