Hi Peter, On Monday, 8 January 2018 10:20:24 EET Peter Ujfalusi wrote: > On 2018-01-05 16:04, Laurent Pinchart wrote: > > On Friday, 5 January 2018 13:30:37 EET Peter Ujfalusi wrote: > >> Use the plane index as default zpos for all planes. Even if the > >> application is not setting zpos/zorder explicitly we will have unique > >> zpos for each plane. > >> > >> Enforce that all planes must have unique zpos on the given crtc. > > > > Could you explain the rationale for that in the commit message, what's > > wrong with duplicate zpos values ? > > Planes with identical zpos is only 'valid' _if_ they are not > overlapping, if they do overlap then it is - imho - not a valid > configuration anyway (which one should be on top?). > Based on my tests the plane with lower planeID is going to disappear > from the screen if we have duplicated zpos. Please see my reply to Tomi on this topic. I'm not against the change, but I think the rationale should be captured in the commit message. > > Isn't there a risk of breaking the non-atomic userspace with this ? > > Without atomic commits userspace can't change the zpos of multiple planes > > in one go, so it might be impossible to reorder planes without going > > through a state that has duplicated zpos values. > > Two planes occupying the same position on the screen is not valid > (again, imho). At the hardware level for the DSS, sure. According to the KMS API, however, it is valid, even if the conflict resolution is driver-dependent. > If application wants to swap two planes, then it must disable one, move the > other to the new position, then enable and move the first plane. Applications don't do that at the moment, so there's a risk of breakage. As the current behaviour is undefined we might not considered that as a problem, but there's a risk of returning an error for an operation that currently succeeds. Personally I think all applications should move to the atomic API and handle zpos explicitly so I don't mind too much :) > >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx> > >> --- > >> Hi, > >> > >> Changes since v3: > >> - Use drm_plane_index() instead of storing the same index wothin > >> omap_plane struct > >> - Optimize the zpos validation loop so we avoid extra checks. > >> > >> Changes since v2: > >> - The check for duplicate zpos is moved to omap_crtc > >> > >> Changes since v1: > >> - Dropped the zpos normalization related patches > >> - New patch based on the discussion, see commit message. > >> > >> Regards, > >> Peter > >> > >> drivers/gpu/drm/omapdrm/omap_crtc.c | 36 +++++++++++++++++++++++++++++- > >> drivers/gpu/drm/omapdrm/omap_plane.c | 15 ++++----------- > >> 2 files changed, 39 insertions(+), 12 deletions(-) -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
