On Tuesday, September 24, 2019 11:48 AM, Pekka Paalanen ppaalanen@gmail.com wrote:
On Tue, 24 Sep 2019 07:34:30 +0000 Simon Ser contact@emersion.fr wrote:
On Tuesday, September 24, 2019 10:26 AM, Pekka Paalanen ppaalanen@gmail.com wrote:
On Mon, 23 Sep 2019 12:39:20 +0000 Simon Ser contact@emersion.fr wrote:
Currently the property docs don't specify whether it's okay for two planes to have the same zpos value and what user-space should expect in this case. The rule mentionned in the past was to disambiguate with object IDs. However some drivers break this rule (that's why the ordering is documented as unspecified in case the zpos property is missing). Additionally it doesn't really make sense for a driver to user identical zpos values if it knows their relative position: the driver can just pick different values instead. So two solutions would make sense: either disallow completely identical zpos values for two different planes, either make the ordering unspecified. To allow drivers that don't know the relative ordering between two planes to still expose the zpos property, choose the latter solution.
Hi Simon, the commit message still talks about the drivers only, and I think that is what lead me astray in the first place until I realized the duplicate zpos value issue concerns only stupid userspace, not that drivers were allowed to use duplicate zpos values which userspace then needs to untangle with ID ordering.
Drivers never have undefined plane ordering themselves - if they do, that's a broken driver that doesn't know what hardware it is driving. If the driver doesn't know, then userspace definitely cannot know any better - if some userspace does, that's a gfx driver stack misdesign. So there is no reason for a driver to use ambiguous zpos values.
This patch in fact explains why duplicate plane IDs are allowed. The two alternatives are mentioned: either disallow duplicate plane zpos values, or either make ordering undefined. I chose to allow duplicate values because some HW might not know the zpos ordering between e.g. some overlay planes, but might know primary is under overlays.
Ok, seems like we are still crossing some streams here. There are two different and independent cases:
- zpos set by drivers
- zpos set by userspace
I think we sorted out the "zpos set by userspace" properly. Let's just focus on "zpos set by drivers".
Zpos set by drivers into nonsensical (duplicate) values is a driver bug to me. Drivers do have the opportunity to not advertise zpos at all if they don't know, even if that makes no sense to me.
I don't know about that. Let's say a driver knows it has underlay planes, but doesn't know their relative ordering for each revision of the HW. Maybe it could just set the same zpos for all underlays to signal to user-space that these planes are under the primary plane?
I'm no driver developer so I don't know whether this could happen. The goal of this patch is to allow drivers to provide partial zpos info to user-space (better than no info at all, especially when overlay planes have undefined ordering wrt. the primary plane when zpos is not advertised).
I assumed that zpos set by userspace must be allowed to have duplicate zpos values, because such broken userspace already exists. But do they exist? If probably not, then atomic check failing on duplicate zpos would be nice. But there must be some reason why the doc already mentions that ID ordering rule for making sense of ambiguous userspace.
I don't know, I haven't found anything explaining where these dup zpos values set by user-space are coming from.
If that doesn't make sense at all, I'd be happy to change the spec to say that duplicate zpos values are a kernel bug. We'll need to make sure this doesn't happen, e.g. with a check in the function creating the property.
I thought that was already the case.
No, this patch specifically says duplicate zpos (coming from the driver or set by user-space) will have undefined ordering.
Note that user-space needs to handle undefined order between planes anyway in case zpos isn't advertised.
Of course.
Thanks, pq