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.
Signed-off-by: Simon Ser contact@emersion.fr ---
Err, I'm sorry about the double-post. I sent this to intel-gfx by mistake.
drivers/gpu/drm/drm_blend.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..51bd5454e50a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@ * planes. Without this property the primary plane is always below the cursor * plane, and ordering between all other planes is undefined. The positive * Z axis points towards the user, i.e. planes with lower Z position values - * are underneath planes with higher Z position values. Note that the Z - * position value can also be immutable, to inform userspace about the - * hard-coded stacking of overlay planes, see - * drm_plane_create_zpos_immutable_property(). + * are underneath planes with higher Z position values. Two planes with the + * same Z position value have undefined ordering. Note that the Z position + * value can also be immutable, to inform userspace about the hard-coded + * stacking of overlay planes, see drm_plane_create_zpos_immutable_property(). * * pixel blend mode: * Pixel blend mode is set up with drm_plane_create_blend_mode_property(). -- 2.23.0
On Tue, 10 Sep 2019 10:09:55 +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.
Signed-off-by: Simon Ser contact@emersion.fr
Err, I'm sorry about the double-post. I sent this to intel-gfx by mistake.
drivers/gpu/drm/drm_blend.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..51bd5454e50a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@
- planes. Without this property the primary plane is always below the cursor
- plane, and ordering between all other planes is undefined. The positive
- Z axis points towards the user, i.e. planes with lower Z position values
- are underneath planes with higher Z position values. Note that the Z
- position value can also be immutable, to inform userspace about the
- hard-coded stacking of overlay planes, see
- drm_plane_create_zpos_immutable_property().
- are underneath planes with higher Z position values. Two planes with the
- same Z position value have undefined ordering. Note that the Z position
- value can also be immutable, to inform userspace about the hard-coded
- stacking of overlay planes, see drm_plane_create_zpos_immutable_property().
- pixel blend mode:
- Pixel blend mode is set up with drm_plane_create_blend_mode_property().
Hi,
this seems to contradict what the docs say in another place:
zpos
Priority of the given plane on crtc (optional).
Note that multiple active planes on the same crtc can have an identical zpos value. The rule to solving the conflict is to compare the plane object IDs; the plane with a higher ID must be stacked on top of a plane with a lower ID.
See drm_plane_create_zpos_property() and drm_plane_create_zpos_immutable_property() for more details.
from https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms.html#plane-functions-r...
Thanks, pq
On Tuesday, September 10, 2019 1:38 PM, Pekka Paalanen ppaalanen@gmail.com wrote:
On Tue, 10 Sep 2019 10:09:55 +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.
Signed-off-by: Simon Ser contact@emersion.fr
Err, I'm sorry about the double-post. I sent this to intel-gfx by mistake. drivers/gpu/drm/drm_blend.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..51bd5454e50a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@
planes. Without this property the primary plane is always below the cursor
plane, and ordering between all other planes is undefined. The positive
Z axis points towards the user, i.e. planes with lower Z position values
- are underneath planes with higher Z position values. Note that the Z
- position value can also be immutable, to inform userspace about the
- hard-coded stacking of overlay planes, see
- drm_plane_create_zpos_immutable_property().
- are underneath planes with higher Z position values. Two planes with the
- same Z position value have undefined ordering. Note that the Z position
- value can also be immutable, to inform userspace about the hard-coded
- stacking of overlay planes, see drm_plane_create_zpos_immutable_property().
- pixel blend mode:
- Pixel blend mode is set up with drm_plane_create_blend_mode_property().
Hi,
this seems to contradict what the docs say in another place:
Except this comment is about drm_plane_state.zpos, an internal DRM property. This is not about the zpos property itself.
The comment was introduced in v2 of [1], although the motivation for the change isn't documented.
[1]: https://patchwork.freedesktop.org/series/13528/#rev2
zpos
Priority of the given plane on crtc (optional).
Note that multiple active planes on the same crtc can have an identical zpos value. The rule to solving the conflict is to compare the plane object IDs; the plane with a higher ID must be stacked on top of a plane with a lower ID.
See drm_plane_create_zpos_property() and drm_plane_create_zpos_immutable_property() for more details.
from https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms.html#plane-functions-r...
Thanks, pq
On Tue, 10 Sep 2019 11:20:16 +0000 Simon Ser contact@emersion.fr wrote:
On Tuesday, September 10, 2019 1:38 PM, Pekka Paalanen ppaalanen@gmail.com wrote:
On Tue, 10 Sep 2019 10:09:55 +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.
Signed-off-by: Simon Ser contact@emersion.fr
Err, I'm sorry about the double-post. I sent this to intel-gfx by mistake. drivers/gpu/drm/drm_blend.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..51bd5454e50a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@
planes. Without this property the primary plane is always below the cursor
plane, and ordering between all other planes is undefined. The positive
Z axis points towards the user, i.e. planes with lower Z position values
- are underneath planes with higher Z position values. Note that the Z
- position value can also be immutable, to inform userspace about the
- hard-coded stacking of overlay planes, see
- drm_plane_create_zpos_immutable_property().
- are underneath planes with higher Z position values. Two planes with the
- same Z position value have undefined ordering. Note that the Z position
- value can also be immutable, to inform userspace about the hard-coded
- stacking of overlay planes, see drm_plane_create_zpos_immutable_property().
- pixel blend mode:
- Pixel blend mode is set up with drm_plane_create_blend_mode_property().
Hi,
this seems to contradict what the docs say in another place:
Except this comment is about drm_plane_state.zpos, an internal DRM property. This is not about the zpos property itself.
Hi,
then I'm very confused. What's the difference?
The text you are changing was specifically added to explain uAPI behaviour, that is, what the userspace sees and does with the zpos property in uAPI.
Having two conflicting pieces of documentation is confusing, especially when it is not absolutely clear which one is for driver implementors and which one for uAPI consumers, or that one must ignore the other doc depending on who you are.
The comment was introduced in v2 of [1], although the motivation for the change isn't documented.
That does not look like the comment you are changing.
Wait, which one is "this comment"?
zpos
Priority of the given plane on crtc (optional).
Note that multiple active planes on the same crtc can have an identical zpos value. The rule to solving the conflict is to compare the plane object IDs; the plane with a higher ID must be stacked on top of a plane with a lower ID.
See drm_plane_create_zpos_property() and drm_plane_create_zpos_immutable_property() for more details.
from https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms.html#plane-functions-r...
Me and others have taken the above to document uAPI. How could that not be the case?
Is it wrong for userspace to assume that plane object ID order reflects the plane stacking order? Weston does not do this yet, but since we just recently found the above comment, we planned to make use of it.
Actually, if the ID ordering is true, then the DRM core could always expose the zpos property as immutable to userspace, just manufacture it from object IDs if nothing is set by a driver explicitly.
Or is the comment about object IDs wrong and should be changed to say the opposite?
Thanks, pq
On Tue, 10 Sep 2019 11:20:16 +0000 Simon Ser contact@emersion.fr wrote:
On Tuesday, September 10, 2019 1:38 PM, Pekka Paalanen ppaalanen@gmail.com wrote:
On Tue, 10 Sep 2019 10:09:55 +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.
Signed-off-by: Simon Ser contact@emersion.fr
Err, I'm sorry about the double-post. I sent this to intel-gfx by mistake. drivers/gpu/drm/drm_blend.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..51bd5454e50a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@
planes. Without this property the primary plane is always below the cursor
plane, and ordering between all other planes is undefined. The positive
Z axis points towards the user, i.e. planes with lower Z position values
- are underneath planes with higher Z position values. Note that the Z
- position value can also be immutable, to inform userspace about the
- hard-coded stacking of overlay planes, see
- drm_plane_create_zpos_immutable_property().
- are underneath planes with higher Z position values. Two planes with the
- same Z position value have undefined ordering. Note that the Z position
- value can also be immutable, to inform userspace about the hard-coded
- stacking of overlay planes, see drm_plane_create_zpos_immutable_property().
- pixel blend mode:
- Pixel blend mode is set up with drm_plane_create_blend_mode_property().
Hi,
this seems to contradict what the docs say in another place:
Except this comment is about drm_plane_state.zpos, an internal DRM property. This is not about the zpos property itself.
Hi,
then I'm very confused. What's the difference?
The text you are changing was specifically added to explain uAPI behaviour, that is, what the userspace sees and does with the zpos property in uAPI.
Having two conflicting pieces of documentation is confusing, especially when it is not absolutely clear which one is for driver implementors and which one for uAPI consumers, or that one must ignore the other doc depending on who you are.
Yes, I agree that this is confusing.
To be completely honest, I don't really care what the outcome of this discussion is, as long as there are no conflicting documentation comments.
So, my interpretation of the docs is that there are:
1. Some documentation for KMS properties, that is, what is exposed to user-space via DRM ioctls. This is the KMS uAPI. 2. Some documentation for kernel drivers, that is, internal DRM state that can be set by kernel developers. This includes helper functions and common structs.
Obviously as user-space developers we only care about (1).
Now, back to zpos' case: there are two doc comments about zpos.
The first one is [1], the one this patch changes:
zpos: Z position is set up with drm_plane_create_zpos_immutable_property() and drm_plane_create_zpos_property(). It controls the visibility of overlapping planes. Without this property the primary plane is always below the cursor plane, and ordering between all other planes is undefined. The positive Z axis points towards the user, i.e. planes with lower Z position values are underneath planes with higher Z position values. Note that the Z position value can also be immutable, to inform userspace about the hard-coded stacking of overlay planes, see drm_plane_create_zpos_immutable_property().
This is in the "Plane Composition Properties". I believe this paragraph documents the standard plane properties (standard as in not driver-specific). So I'd say this documents the KMS uAPI.
The second one is [2], the one you've quoted:
zpos
Priority of the given plane on crtc (optional).
Note that multiple active planes on the same crtc can have an identical zpos value. The rule to solving the conflict is to compare the plane object IDs; the plane with a higher ID must be stacked on top of a plane with a lower ID.
See drm_plane_create_zpos_property() and drm_plane_create_zpos_immutable_property() for more details.
This is in the "Plane Functions Reference" section, more precisely in the "struct drm_plane_state" docs. I believe this is really just about the common DRM struct that can be used by all drivers. This struct isn't exposed to user-space. It's just an implementation detail of DRM.
(We can see that even without this patch, these two comments already kind of conflict. The first one says that without zpos ordering is undefined. The second one says that two identical zpos values means the plane ID should be used. However drm_plane_state is zero-filled, so a driver not supporting zpos would have all drm_plane_state.zpos fields set to zero? Since they are all equal, is the object ID ordering rule relevant?)
[1]: https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms.html#plane-composition... [2]: https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms.html#plane-functions-r...
The comment was introduced in v2 of [1], although the motivation for the change isn't documented.
That does not look like the comment you are changing.
Wait, which one is "this comment"?
zpos
Priority of the given plane on crtc (optional).
Note that multiple active planes on the same crtc can have an identical zpos value. The rule to solving the conflict is to compare the plane object IDs; the plane with a higher ID must be stacked on top of a plane with a lower ID.
See drm_plane_create_zpos_property() and drm_plane_create_zpos_immutable_property() for more details.
from https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms.html#plane-functions-r...
Me and others have taken the above to document uAPI. How could that not be the case?
(See above)
Is it wrong for userspace to assume that plane object ID order reflects the plane stacking order? Weston does not do this yet, but since we just recently found the above comment, we planned to make use of it.
Actually, if the ID ordering is true, then the DRM core could always expose the zpos property as immutable to userspace, just manufacture it from object IDs if nothing is set by a driver explicitly.
Or is the comment about object IDs wrong and should be changed to say the opposite?
I believe so. If zpos could always be obtained from plane object IDs, then what would be the point of exposing the immutable zpos property at all?
I've heard that some drivers break this object ID assumption, which is the reason why this zpos property exists. Can someone more familiar with the kernel provide some feedback?
On Mon, 16 Sep 2019 09:19:09 +0000 Simon Ser contact@emersion.fr wrote:
On Tue, 10 Sep 2019 11:20:16 +0000 Simon Ser contact@emersion.fr wrote:
On Tuesday, September 10, 2019 1:38 PM, Pekka Paalanen ppaalanen@gmail.com wrote:
On Tue, 10 Sep 2019 10:09:55 +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.
Signed-off-by: Simon Ser contact@emersion.fr
Err, I'm sorry about the double-post. I sent this to intel-gfx by mistake. drivers/gpu/drm/drm_blend.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..51bd5454e50a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@
planes. Without this property the primary plane is always below the cursor
plane, and ordering between all other planes is undefined. The positive
Z axis points towards the user, i.e. planes with lower Z position values
- are underneath planes with higher Z position values. Note that the Z
- position value can also be immutable, to inform userspace about the
- hard-coded stacking of overlay planes, see
- drm_plane_create_zpos_immutable_property().
- are underneath planes with higher Z position values. Two planes with the
- same Z position value have undefined ordering. Note that the Z position
- value can also be immutable, to inform userspace about the hard-coded
- stacking of overlay planes, see drm_plane_create_zpos_immutable_property().
- pixel blend mode:
- Pixel blend mode is set up with drm_plane_create_blend_mode_property().
Hi,
this seems to contradict what the docs say in another place:
Except this comment is about drm_plane_state.zpos, an internal DRM property. This is not about the zpos property itself.
Hi,
then I'm very confused. What's the difference?
The text you are changing was specifically added to explain uAPI behaviour, that is, what the userspace sees and does with the zpos property in uAPI.
Having two conflicting pieces of documentation is confusing, especially when it is not absolutely clear which one is for driver implementors and which one for uAPI consumers, or that one must ignore the other doc depending on who you are.
Yes, I agree that this is confusing.
To be completely honest, I don't really care what the outcome of this discussion is, as long as there are no conflicting documentation comments.
Hi,
that is exactly what I want too.
So, my interpretation of the docs is that there are:
- Some documentation for KMS properties, that is, what is exposed to user-space via DRM ioctls. This is the KMS uAPI.
- Some documentation for kernel drivers, that is, internal DRM state that can be set by kernel developers. This includes helper functions and common structs.
Obviously as user-space developers we only care about (1).
Theoretically yes, but the problem is that one cannot make that distinction. I'm pretty sure both categories of comments are not complete, and answers to some questions in one must be dug up from the other, if documented at all.
So you cannot use wording that looks conflicting between the two. If the wording is correct nevertheless, it needs more explaining on how it doesn't actually conflict, so that people randomly reading just one side or the other don't get the wrong idea.
Now, back to zpos' case: there are two doc comments about zpos.
The first one is [1], the one this patch changes:
zpos: Z position is set up with drm_plane_create_zpos_immutable_property() and drm_plane_create_zpos_property(). It controls the visibility of overlapping planes. Without this property the primary plane is always below the cursor plane, and ordering between all other planes is undefined. The positive Z axis points towards the user, i.e. planes with lower Z position values are underneath planes with higher Z position values. Note that the Z position value can also be immutable, to inform userspace about the hard-coded stacking of overlay planes, see drm_plane_create_zpos_immutable_property().
This is in the "Plane Composition Properties". I believe this paragraph documents the standard plane properties (standard as in not driver-specific). So I'd say this documents the KMS uAPI.
The second one is [2], the one you've quoted:
zpos
Priority of the given plane on crtc (optional).
Note that multiple active planes on the same crtc can have an identical zpos value. The rule to solving the conflict is to compare the plane object IDs; the plane with a higher ID must be stacked on top of a plane with a lower ID.
See drm_plane_create_zpos_property() and drm_plane_create_zpos_immutable_property() for more details.
This is in the "Plane Functions Reference" section, more precisely in the "struct drm_plane_state" docs. I believe this is really just about the common DRM struct that can be used by all drivers. This struct isn't exposed to user-space. It's just an implementation detail of DRM.
(We can see that even without this patch, these two comments already kind of conflict. The first one says that without zpos ordering is undefined. The second one says that two identical zpos values means the plane ID should be used. However drm_plane_state is zero-filled, so a driver not supporting zpos would have all drm_plane_state.zpos fields set to zero? Since they are all equal, is the object ID ordering rule relevant?)
Right, and we are suffering from that confusion already. Should userspace use ID order if zpos property is not there or not? I have no idea.
Thanks, pq
On Thu, Sep 19, 2019 at 9:18 AM Pekka Paalanen ppaalanen@gmail.com wrote:
On Mon, 16 Sep 2019 09:19:09 +0000 Simon Ser contact@emersion.fr wrote:
On Tue, 10 Sep 2019 11:20:16 +0000 Simon Ser contact@emersion.fr wrote:
On Tuesday, September 10, 2019 1:38 PM, Pekka Paalanen ppaalanen@gmail.com wrote:
On Tue, 10 Sep 2019 10:09:55 +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.
Signed-off-by: Simon Ser contact@emersion.fr
Err, I'm sorry about the double-post. I sent this to intel-gfx by mistake. drivers/gpu/drm/drm_blend.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..51bd5454e50a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@
planes. Without this property the primary plane is always below the cursor
plane, and ordering between all other planes is undefined. The positive
Z axis points towards the user, i.e. planes with lower Z position values
- are underneath planes with higher Z position values. Note that the Z
- position value can also be immutable, to inform userspace about the
- hard-coded stacking of overlay planes, see
- drm_plane_create_zpos_immutable_property().
- are underneath planes with higher Z position values. Two planes with the
- same Z position value have undefined ordering. Note that the Z position
- value can also be immutable, to inform userspace about the hard-coded
- stacking of overlay planes, see drm_plane_create_zpos_immutable_property().
- pixel blend mode:
- Pixel blend mode is set up with drm_plane_create_blend_mode_property().
Hi,
this seems to contradict what the docs say in another place:
Except this comment is about drm_plane_state.zpos, an internal DRM property. This is not about the zpos property itself.
Hi,
then I'm very confused. What's the difference?
The text you are changing was specifically added to explain uAPI behaviour, that is, what the userspace sees and does with the zpos property in uAPI.
Having two conflicting pieces of documentation is confusing, especially when it is not absolutely clear which one is for driver implementors and which one for uAPI consumers, or that one must ignore the other doc depending on who you are.
Yes, I agree that this is confusing.
To be completely honest, I don't really care what the outcome of this discussion is, as long as there are no conflicting documentation comments.
Hi,
that is exactly what I want too.
So, my interpretation of the docs is that there are:
- Some documentation for KMS properties, that is, what is exposed to user-space via DRM ioctls. This is the KMS uAPI.
- Some documentation for kernel drivers, that is, internal DRM state that can be set by kernel developers. This includes helper functions and common structs.
Obviously as user-space developers we only care about (1).
Theoretically yes, but the problem is that one cannot make that distinction. I'm pretty sure both categories of comments are not complete, and answers to some questions in one must be dug up from the other, if documented at all.
So you cannot use wording that looks conflicting between the two. If the wording is correct nevertheless, it needs more explaining on how it doesn't actually conflict, so that people randomly reading just one side or the other don't get the wrong idea.
Now, back to zpos' case: there are two doc comments about zpos.
The first one is [1], the one this patch changes:
zpos: Z position is set up with drm_plane_create_zpos_immutable_property() and drm_plane_create_zpos_property(). It controls the visibility of overlapping planes. Without this property the primary plane is always below the cursor plane, and ordering between all other planes is undefined. The positive Z axis points towards the user, i.e. planes with lower Z position values are underneath planes with higher Z position values. Note that the Z position value can also be immutable, to inform userspace about the hard-coded stacking of overlay planes, see drm_plane_create_zpos_immutable_property().
This is in the "Plane Composition Properties". I believe this paragraph documents the standard plane properties (standard as in not driver-specific). So I'd say this documents the KMS uAPI.
The second one is [2], the one you've quoted:
zpos
Priority of the given plane on crtc (optional).
Note that multiple active planes on the same crtc can have an identical zpos value. The rule to solving the conflict is to compare the plane object IDs; the plane with a higher ID must be stacked on top of a plane with a lower ID.
See drm_plane_create_zpos_property() and drm_plane_create_zpos_immutable_property() for more details.
This is in the "Plane Functions Reference" section, more precisely in the "struct drm_plane_state" docs. I believe this is really just about the common DRM struct that can be used by all drivers. This struct isn't exposed to user-space. It's just an implementation detail of DRM.
(We can see that even without this patch, these two comments already kind of conflict. The first one says that without zpos ordering is undefined. The second one says that two identical zpos values means the plane ID should be used. However drm_plane_state is zero-filled, so a driver not supporting zpos would have all drm_plane_state.zpos fields set to zero? Since they are all equal, is the object ID ordering rule relevant?)
Right, and we are suffering from that confusion already. Should userspace use ID order if zpos property is not there or not? I have no idea.
Nope. I think the only options for this case are: - file bug against upstream driver so they add zpos - you magically know how planes work on that hw - you don't overlap planes at all - cursor is above primary, that much we can guarantee
Yes it's kinda uapi fail we didn't add zpos from the start :-/ -Daniel
Thanks, pq
On Thu, 19 Sep 2019 10:18:04 +0200 Daniel Vetter daniel.vetter@ffwll.ch wrote:
On Thu, Sep 19, 2019 at 9:18 AM Pekka Paalanen ppaalanen@gmail.com wrote:
...
Right, and we are suffering from that confusion already. Should userspace use ID order if zpos property is not there or not? I have no idea.
Nope. I think the only options for this case are:
- file bug against upstream driver so they add zpos
- you magically know how planes work on that hw
- you don't overlap planes at all
- cursor is above primary, that much we can guarantee
Yes it's kinda uapi fail we didn't add zpos from the start :-/
Good. Weston does the last two. The confusion did not last long enough to let us add code using the object ID to infer stacking order.
Although, Weston does have the assumption that overlays are in unknown order between primary and cursor, which now seems false.
Thanks, pq
On Thu, Sep 19, 2019 at 11:02 AM Pekka Paalanen ppaalanen@gmail.com wrote:
On Thu, 19 Sep 2019 10:18:04 +0200 Daniel Vetter daniel.vetter@ffwll.ch wrote:
On Thu, Sep 19, 2019 at 9:18 AM Pekka Paalanen ppaalanen@gmail.com wrote:
...
Right, and we are suffering from that confusion already. Should userspace use ID order if zpos property is not there or not? I have no idea.
Nope. I think the only options for this case are:
- file bug against upstream driver so they add zpos
- you magically know how planes work on that hw
- you don't overlap planes at all
- cursor is above primary, that much we can guarantee
Yes it's kinda uapi fail we didn't add zpos from the start :-/
Good. Weston does the last two. The confusion did not last long enough to let us add code using the object ID to infer stacking order.
Although, Weston does have the assumption that overlays are in unknown order between primary and cursor, which now seems false.
I think cursor is always on top, but some of the overlays can be underlays. -Daniel
On Tue, Sep 10, 2019 at 12:38 PM Pekka Paalanen ppaalanen@gmail.com wrote:
On Tue, 10 Sep 2019 10:09:55 +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.
Signed-off-by: Simon Ser contact@emersion.fr
Err, I'm sorry about the double-post. I sent this to intel-gfx by mistake.
drivers/gpu/drm/drm_blend.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..51bd5454e50a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@
- planes. Without this property the primary plane is always below the cursor
- plane, and ordering between all other planes is undefined. The positive
- Z axis points towards the user, i.e. planes with lower Z position values
- are underneath planes with higher Z position values. Note that the Z
- position value can also be immutable, to inform userspace about the
- hard-coded stacking of overlay planes, see
- drm_plane_create_zpos_immutable_property().
- are underneath planes with higher Z position values. Two planes with the
- same Z position value have undefined ordering. Note that the Z position
- value can also be immutable, to inform userspace about the hard-coded
- stacking of overlay planes, see drm_plane_create_zpos_immutable_property().
- pixel blend mode:
- Pixel blend mode is set up with drm_plane_create_blend_mode_property().
Hi,
this seems to contradict what the docs say in another place:
zpos
Priority of the given plane on crtc (optional). Note that multiple active planes on the same crtc can have an identical zpos value. The rule to solving the conflict is to
I think we should do an s/rule/recommendation to drivers to avoid surprises in hw programming/ or something like that. Plus we allow that for legacy ioctl, otherways it's impossible to change zpos with legacy ioctls (not that I'm sure that's a reasonable use-case, but whatever). With that also clarified I think Simon's patch is good to go, same zpos value is really not something we want to encourage. Plus "implicit by plane id" is already something we use in other places (like figuring out which primary plan belongs to which crtc), and it's just terrible uapi. -Daniel
compare the plane object IDs; the plane with a higher ID must be stacked on top of a plane with a lower ID. See drm_plane_create_zpos_property() and drm_plane_create_zpos_immutable_property() for more details.from https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms.html#plane-functions-r...
Thanks, pq
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.
Additionally, update the drm_plane_state.zpos docs to clarify that zpos disambiguation via plane object IDs is a recommendation for drivers, not something user-space can rely on.
v2: clarify drm_plane_state.zpos docs (Daniel)
Signed-off-by: Simon Ser contact@emersion.fr Cc: Pekka Paalanen ppaalanen@gmail.com Cc: Marius Vlad marius.vlad@collabora.com Cc: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_blend.c | 8 ++++---- include/drm/drm_plane.h | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..51bd5454e50a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@ * planes. Without this property the primary plane is always below the cursor * plane, and ordering between all other planes is undefined. The positive * Z axis points towards the user, i.e. planes with lower Z position values - * are underneath planes with higher Z position values. Note that the Z - * position value can also be immutable, to inform userspace about the - * hard-coded stacking of overlay planes, see - * drm_plane_create_zpos_immutable_property(). + * are underneath planes with higher Z position values. Two planes with the + * same Z position value have undefined ordering. Note that the Z position + * value can also be immutable, to inform userspace about the hard-coded + * stacking of overlay planes, see drm_plane_create_zpos_immutable_property(). * * pixel blend mode: * Pixel blend mode is set up with drm_plane_create_blend_mode_property(). diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index cd5903ad33f7..7ac68057b19d 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -141,9 +141,9 @@ struct drm_plane_state { * Priority of the given plane on crtc (optional). * * Note that multiple active planes on the same crtc can have an - * identical zpos value. The rule to solving the conflict is to compare - * the plane object IDs; the plane with a higher ID must be stacked on - * top of a plane with a lower ID. + * identical zpos value. To solve the conflict, the recommendation to + * drivers to avoid surprises is to compare the plane object IDs; the + * plane with a higher ID is stacked on top of a plane with a lower ID. * * See drm_plane_create_zpos_property() and * drm_plane_create_zpos_immutable_property() for more details. -- 2.23.0
On 9/17/19 8:24 PM, Simon Ser 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.
Additionally, update the drm_plane_state.zpos docs to clarify that zpos disambiguation via plane object IDs is a recommendation for drivers, not something user-space can rely on.
v2: clarify drm_plane_state.zpos docs (Daniel)
Signed-off-by: Simon Ser contact@emersion.fr Cc: Pekka Paalanen ppaalanen@gmail.com Cc: Marius Vlad marius.vlad@collabora.com Cc: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_blend.c | 8 ++++---- include/drm/drm_plane.h | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..51bd5454e50a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@
- planes. Without this property the primary plane is always below the cursor
- plane, and ordering between all other planes is undefined. The positive
- Z axis points towards the user, i.e. planes with lower Z position values
- are underneath planes with higher Z position values. Note that the Z
- position value can also be immutable, to inform userspace about the
- hard-coded stacking of overlay planes, see
- drm_plane_create_zpos_immutable_property().
- are underneath planes with higher Z position values. Two planes with the
- same Z position value have undefined ordering. Note that the Z position
- value can also be immutable, to inform userspace about the hard-coded
- stacking of overlay planes, see drm_plane_create_zpos_immutable_property().
stacking of overlay (and or) scanout planes?
- pixel blend mode:
- Pixel blend mode is set up with drm_plane_create_blend_mode_property().
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index cd5903ad33f7..7ac68057b19d 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -141,9 +141,9 @@ struct drm_plane_state { * Priority of the given plane on crtc (optional). * * Note that multiple active planes on the same crtc can have an
* identical zpos value. The rule to solving the conflict is to compare* the plane object IDs; the plane with a higher ID must be stacked on* top of a plane with a lower ID.
* identical zpos value. To solve the conflict, the recommendation to* drivers to avoid surprises is to compare the plane object IDs; the* plane with a higher ID is stacked on top of a plane with a lower ID.
- See drm_plane_create_zpos_property() and
- drm_plane_create_zpos_immutable_property() for more details.
-- 2.23.0
On Wed, Sep 18, 2019 at 1:14 PM Marius Vlad marius.vlad@collabora.com wrote:
On 9/17/19 8:24 PM, Simon Ser 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.
Additionally, update the drm_plane_state.zpos docs to clarify that zpos disambiguation via plane object IDs is a recommendation for drivers, not something user-space can rely on.
v2: clarify drm_plane_state.zpos docs (Daniel)
Signed-off-by: Simon Ser contact@emersion.fr Cc: Pekka Paalanen ppaalanen@gmail.com Cc: Marius Vlad marius.vlad@collabora.com Cc: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_blend.c | 8 ++++---- include/drm/drm_plane.h | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..51bd5454e50a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@
- planes. Without this property the primary plane is always below the cursor
- plane, and ordering between all other planes is undefined. The positive
- Z axis points towards the user, i.e. planes with lower Z position values
- are underneath planes with higher Z position values. Note that the Z
- position value can also be immutable, to inform userspace about the
- hard-coded stacking of overlay planes, see
- drm_plane_create_zpos_immutable_property().
- are underneath planes with higher Z position values. Two planes with the
- same Z position value have undefined ordering. Note that the Z position
- value can also be immutable, to inform userspace about the hard-coded
- stacking of overlay planes, see drm_plane_create_zpos_immutable_property().
stacking of overlay (and or) scanout planes?
Yeah might be better to drop the "overlay" here, this is for planes in general. Overlay vs. other planes is really only relevant for the legacy ioctls, to make sure they know which plane to pick. Otherwise there's nothing special with them. -Daniel
- pixel blend mode:
- Pixel blend mode is set up with drm_plane_create_blend_mode_property().
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index cd5903ad33f7..7ac68057b19d 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -141,9 +141,9 @@ struct drm_plane_state { * Priority of the given plane on crtc (optional). * * Note that multiple active planes on the same crtc can have an
* identical zpos value. The rule to solving the conflict is to compare* the plane object IDs; the plane with a higher ID must be stacked on* top of a plane with a lower ID.
* identical zpos value. To solve the conflict, the recommendation to* drivers to avoid surprises is to compare the plane object IDs; the* plane with a higher ID is stacked on top of a plane with a lower ID. * * See drm_plane_create_zpos_property() and * drm_plane_create_zpos_immutable_property() for more details.-- 2.23.0
-- Marius Vlad
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.
While at it, remove the assumption that zpos is only for overlay planes.
Additionally, update the drm_plane_state.zpos docs to clarify that zpos disambiguation via plane object IDs is a recommendation for drivers, not something user-space can rely on.
v2: clarify drm_plane_state.zpos docs (Daniel)
v3: zpos is for all planes (Marius, Daniel)
Signed-off-by: Simon Ser contact@emersion.fr Cc: Pekka Paalanen ppaalanen@gmail.com Cc: Marius Vlad marius.vlad@collabora.com Cc: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_blend.c | 8 ++++---- include/drm/drm_plane.h | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..121481f6aa71 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@ * planes. Without this property the primary plane is always below the cursor * plane, and ordering between all other planes is undefined. The positive * Z axis points towards the user, i.e. planes with lower Z position values - * are underneath planes with higher Z position values. Note that the Z - * position value can also be immutable, to inform userspace about the - * hard-coded stacking of overlay planes, see - * drm_plane_create_zpos_immutable_property(). + * are underneath planes with higher Z position values. Two planes with the + * same Z position value have undefined ordering. Note that the Z position + * value can also be immutable, to inform userspace about the hard-coded + * stacking of planes, see drm_plane_create_zpos_immutable_property(). * * pixel blend mode: * Pixel blend mode is set up with drm_plane_create_blend_mode_property(). diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index cd5903ad33f7..7ac68057b19d 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -141,9 +141,9 @@ struct drm_plane_state { * Priority of the given plane on crtc (optional). * * Note that multiple active planes on the same crtc can have an - * identical zpos value. The rule to solving the conflict is to compare - * the plane object IDs; the plane with a higher ID must be stacked on - * top of a plane with a lower ID. + * identical zpos value. To solve the conflict, the recommendation to + * drivers to avoid surprises is to compare the plane object IDs; the + * plane with a higher ID is stacked on top of a plane with a lower ID. * * See drm_plane_create_zpos_property() and * drm_plane_create_zpos_immutable_property() for more details. -- 2.23.0
On Wed, 18 Sep 2019 16:33:47 +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.
While at it, remove the assumption that zpos is only for overlay planes.
Additionally, update the drm_plane_state.zpos docs to clarify that zpos disambiguation via plane object IDs is a recommendation for drivers, not something user-space can rely on.
v2: clarify drm_plane_state.zpos docs (Daniel)
v3: zpos is for all planes (Marius, Daniel)
Signed-off-by: Simon Ser contact@emersion.fr Cc: Pekka Paalanen ppaalanen@gmail.com Cc: Marius Vlad marius.vlad@collabora.com Cc: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_blend.c | 8 ++++---- include/drm/drm_plane.h | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..121481f6aa71 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@
- planes. Without this property the primary plane is always below the cursor
- plane, and ordering between all other planes is undefined. The positive
- Z axis points towards the user, i.e. planes with lower Z position values
- are underneath planes with higher Z position values. Note that the Z
- position value can also be immutable, to inform userspace about the
- hard-coded stacking of overlay planes, see
- drm_plane_create_zpos_immutable_property().
- are underneath planes with higher Z position values. Two planes with the
- same Z position value have undefined ordering. Note that the Z position
- value can also be immutable, to inform userspace about the hard-coded
- stacking of planes, see drm_plane_create_zpos_immutable_property().
Hi,
the above looks good to me.
- pixel blend mode:
- Pixel blend mode is set up with drm_plane_create_blend_mode_property().
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index cd5903ad33f7..7ac68057b19d 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -141,9 +141,9 @@ struct drm_plane_state { * Priority of the given plane on crtc (optional). * * Note that multiple active planes on the same crtc can have an
* identical zpos value. The rule to solving the conflict is to compare* the plane object IDs; the plane with a higher ID must be stacked on* top of a plane with a lower ID.
* identical zpos value. To solve the conflict, the recommendation to* drivers to avoid surprises is to compare the plane object IDs; the* plane with a higher ID is stacked on top of a plane with a lower ID.
While better, I think this would need a less subtle clarification:
+ "Userspace should never rely on stacking order derived from IDs."
Oh, I think I realised it only now. This comment is for the drivers to handle a case where userspace is being stupid and assigns mutable zpos properties with duplicate values, right?
It does *not* allow drivers themselves to assign duplicate zpos values. So I've been looking at this from the wrong angle.
Maybe it should just say that then?
Instead of:
"Note that multiple active planes on the same crtc can have an identical zpos value. The rule to solving the conflict is to compare the plane object IDs;"
The paragraph could start with:
"Userspace may set mutable zpos properties so that multiple active planes on the same crtc have identical zpos value. This is a userspace bug, but drivers can solve the conflict deterministically by comparing the plane object IDs;"
Thanks, pq
* * See drm_plane_create_zpos_property() and * drm_plane_create_zpos_immutable_property() for more details.-- 2.23.0
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.
While at it, remove the assumption that zpos is only for overlay planes.
Additionally, update the drm_plane_state.zpos docs to clarify that zpos disambiguation via plane object IDs is a recommendation for drivers, not something user-space can rely on.
v2: clarify drm_plane_state.zpos docs (Daniel)
v3: zpos is for all planes (Marius, Daniel)
v4: completely reword the drm_plane_state.zpos docs to make it clear the recommendation to use plane IDs is for drivers in case user-space uses duplicate zpos values (Pekka)
Signed-off-by: Simon Ser contact@emersion.fr Cc: Pekka Paalanen ppaalanen@gmail.com Cc: Marius Vlad marius.vlad@collabora.com Cc: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_blend.c | 8 ++++---- include/drm/drm_plane.h | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..121481f6aa71 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@ * planes. Without this property the primary plane is always below the cursor * plane, and ordering between all other planes is undefined. The positive * Z axis points towards the user, i.e. planes with lower Z position values - * are underneath planes with higher Z position values. Note that the Z - * position value can also be immutable, to inform userspace about the - * hard-coded stacking of overlay planes, see - * drm_plane_create_zpos_immutable_property(). + * are underneath planes with higher Z position values. Two planes with the + * same Z position value have undefined ordering. Note that the Z position + * value can also be immutable, to inform userspace about the hard-coded + * stacking of planes, see drm_plane_create_zpos_immutable_property(). * * pixel blend mode: * Pixel blend mode is set up with drm_plane_create_blend_mode_property(). diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index cd5903ad33f7..328773690851 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -140,10 +140,11 @@ struct drm_plane_state { * @zpos: * Priority of the given plane on crtc (optional). * - * Note that multiple active planes on the same crtc can have an - * identical zpos value. The rule to solving the conflict is to compare - * the plane object IDs; the plane with a higher ID must be stacked on - * top of a plane with a lower ID. + * User-space may set mutable zpos properties so that multiple active + * planes on the same CRTC have identical zpos values. This is a + * user-space bug, but drivers can solve the conflict by comparing the + * plane object IDs; the plane with a higher ID is stacked on top of a + * plane with a lower ID. * * See drm_plane_create_zpos_property() and * drm_plane_create_zpos_immutable_property() for more details. -- 2.23.0
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.
Anyway, it's fine. The actual doc below is what matters here.
While at it, remove the assumption that zpos is only for overlay planes.
Additionally, update the drm_plane_state.zpos docs to clarify that zpos disambiguation via plane object IDs is a recommendation for drivers, not something user-space can rely on.
v2: clarify drm_plane_state.zpos docs (Daniel)
v3: zpos is for all planes (Marius, Daniel)
v4: completely reword the drm_plane_state.zpos docs to make it clear the recommendation to use plane IDs is for drivers in case user-space uses duplicate zpos values (Pekka)
Signed-off-by: Simon Ser contact@emersion.fr Cc: Pekka Paalanen ppaalanen@gmail.com Cc: Marius Vlad marius.vlad@collabora.com Cc: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_blend.c | 8 ++++---- include/drm/drm_plane.h | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..121481f6aa71 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@
- planes. Without this property the primary plane is always below the cursor
- plane, and ordering between all other planes is undefined. The positive
- Z axis points towards the user, i.e. planes with lower Z position values
- are underneath planes with higher Z position values. Note that the Z
- position value can also be immutable, to inform userspace about the
- hard-coded stacking of overlay planes, see
- drm_plane_create_zpos_immutable_property().
- are underneath planes with higher Z position values. Two planes with the
- same Z position value have undefined ordering. Note that the Z position
- value can also be immutable, to inform userspace about the hard-coded
- stacking of planes, see drm_plane_create_zpos_immutable_property().
- pixel blend mode:
- Pixel blend mode is set up with drm_plane_create_blend_mode_property().
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index cd5903ad33f7..328773690851 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -140,10 +140,11 @@ struct drm_plane_state { * @zpos: * Priority of the given plane on crtc (optional). *
* Note that multiple active planes on the same crtc can have an* identical zpos value. The rule to solving the conflict is to compare* the plane object IDs; the plane with a higher ID must be stacked on* top of a plane with a lower ID.
* User-space may set mutable zpos properties so that multiple active* planes on the same CRTC have identical zpos values. This is a* user-space bug, but drivers can solve the conflict by comparing the* plane object IDs; the plane with a higher ID is stacked on top of a* plane with a lower ID.
- See drm_plane_create_zpos_property() and
- drm_plane_create_zpos_immutable_property() for more details.
-- 2.23.0
Reviewed-by: Pekka Paalanen pekka.paalanen@collabora.com
Thanks, pq
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.
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.
Note that user-space needs to handle undefined order between planes anyway in case zpos isn't advertised.
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
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 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.
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.
Note that user-space needs to handle undefined order between planes anyway in case zpos isn't advertised.
Of course.
Thanks, pq
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
On Mon, Sep 23, 2019 at 12:39:20PM +0000, Simon Ser 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.
Some Arm's usage cases about two planes with same zpos.
- "Smart layer" which can accepts 4 different fbs with different src/display rect, but this smart layer has one restriction:
4 display rects must have no overlaps in V direction (A optimization for android window like Toolbar/Navigation bar)
So when map this Smart-layer to drm world, it might be 4 different drm-planes, but with same zorder to indicate that these 4 planes are smart-laye planes.
- "VR-Layer" One VR-layer comprises two different viewports which can be configured with totoally different fbs, src/display rect. we use two differernt drm-planes to drive on HW "VR-Layer", and these two drm-planes must be configured with same zpos.
Thanks James
While at it, remove the assumption that zpos is only for overlay planes.
Additionally, update the drm_plane_state.zpos docs to clarify that zpos disambiguation via plane object IDs is a recommendation for drivers, not something user-space can rely on.
v2: clarify drm_plane_state.zpos docs (Daniel)
v3: zpos is for all planes (Marius, Daniel)
v4: completely reword the drm_plane_state.zpos docs to make it clear the recommendation to use plane IDs is for drivers in case user-space uses duplicate zpos values (Pekka)
Signed-off-by: Simon Ser contact@emersion.fr Cc: Pekka Paalanen ppaalanen@gmail.com Cc: Marius Vlad marius.vlad@collabora.com Cc: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_blend.c | 8 ++++---- include/drm/drm_plane.h | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-)
-- 2.23.0
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index d02709dd2d4a..121481f6aa71 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -132,10 +132,10 @@
- planes. Without this property the primary plane is always below the cursor
- plane, and ordering between all other planes is undefined. The positive
- Z axis points towards the user, i.e. planes with lower Z position values
- are underneath planes with higher Z position values. Note that the Z
- position value can also be immutable, to inform userspace about the
- hard-coded stacking of overlay planes, see
- drm_plane_create_zpos_immutable_property().
- are underneath planes with higher Z position values. Two planes with the
- same Z position value have undefined ordering. Note that the Z position
- value can also be immutable, to inform userspace about the hard-coded
- stacking of planes, see drm_plane_create_zpos_immutable_property().
- pixel blend mode:
- Pixel blend mode is set up with drm_plane_create_blend_mode_property().
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index cd5903ad33f7..328773690851 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -140,10 +140,11 @@ struct drm_plane_state { * @zpos: * Priority of the given plane on crtc (optional). *
* Note that multiple active planes on the same crtc can have an* identical zpos value. The rule to solving the conflict is to compare* the plane object IDs; the plane with a higher ID must be stacked on* top of a plane with a lower ID.
* User-space may set mutable zpos properties so that multiple active* planes on the same CRTC have identical zpos values. This is a* user-space bug, but drivers can solve the conflict by comparing the* plane object IDs; the plane with a higher ID is stacked on top of a* plane with a lower ID.
- See drm_plane_create_zpos_property() and
- drm_plane_create_zpos_immutable_property() for more details.
Hi,
On Mon, Sep 23, 2019 at 12:39:20PM +0000, Simon Ser 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.
Some Arm's usage cases about two planes with same zpos.
- "Smart layer"
which can accepts 4 different fbs with different src/display rect, but this smart layer has one restriction:
4 display rects must have no overlaps in V direction (A optimization for android window like Toolbar/Navigation bar)
So when map this Smart-layer to drm world, it might be 4 different drm-planes, but with same zorder to indicate that these 4 planes are smart-laye planes.
- "VR-Layer"
One VR-layer comprises two different viewports which can be configured with totoally different fbs, src/display rect. we use two differernt drm-planes to drive on HW "VR-Layer", and these two drm-planes must be configured with same zpos.
Thanks a lot for your feedback James, that's exactly what I was looking for. Both of these use-cases make sense to me.
I think user-space should be prepared to handle identical immutable zpos values. Pekka and Daniel, thoughts?
In any case, this doesn't change the patch itself. Probably something worth mentionning in the commit message.
On Sun, 29 Sep 2019 20:30:44 +0000 Simon Ser contact@emersion.fr wrote:
Hi,
On Mon, Sep 23, 2019 at 12:39:20PM +0000, Simon Ser 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.
Some Arm's usage cases about two planes with same zpos.
- "Smart layer"
which can accepts 4 different fbs with different src/display rect, but this smart layer has one restriction:
4 display rects must have no overlaps in V direction (A optimization for android window like Toolbar/Navigation bar)
So when map this Smart-layer to drm world, it might be 4 different drm-planes, but with same zorder to indicate that these 4 planes are smart-laye planes.
- "VR-Layer"
One VR-layer comprises two different viewports which can be configured with totoally different fbs, src/display rect. we use two differernt drm-planes to drive on HW "VR-Layer", and these two drm-planes must be configured with same zpos.
Thanks a lot for your feedback James, that's exactly what I was looking for. Both of these use-cases make sense to me.
I think user-space should be prepared to handle identical immutable zpos values. Pekka and Daniel, thoughts?
Hi,
given those explained use cases, sure, I agree.
In any case, this doesn't change the patch itself. Probably something worth mentionning in the commit message.
Yes, recording these use cases would be very nice.
Thanks, pq
On Mon, Sep 30, 2019 at 10:07:07AM +0300, Pekka Paalanen wrote:
On Sun, 29 Sep 2019 20:30:44 +0000 Simon Ser contact@emersion.fr wrote:
Hi,
On Mon, Sep 23, 2019 at 12:39:20PM +0000, Simon Ser 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.
Some Arm's usage cases about two planes with same zpos.
- "Smart layer"
which can accepts 4 different fbs with different src/display rect, but this smart layer has one restriction:
4 display rects must have no overlaps in V direction (A optimization for android window like Toolbar/Navigation bar)
So when map this Smart-layer to drm world, it might be 4 different drm-planes, but with same zorder to indicate that these 4 planes are smart-laye planes.
- "VR-Layer"
One VR-layer comprises two different viewports which can be configured with totoally different fbs, src/display rect. we use two differernt drm-planes to drive on HW "VR-Layer", and these two drm-planes must be configured with same zpos.
Thanks a lot for your feedback James, that's exactly what I was looking for. Both of these use-cases make sense to me.
I think user-space should be prepared to handle identical immutable zpos values. Pekka and Daniel, thoughts?
Hi,
given those explained use cases, sure, I agree.
In any case, this doesn't change the patch itself. Probably something worth mentionning in the commit message.
Yes, recording these use cases would be very nice.
There's a lot more hw that does the same tricks (qualcom is one for sure). Imo before we encode this we should make sure that: a) everyone is happy with this new uapi b) drivers are consistent c) maybe even testcases in igt d) definitely an open source user e) no breaking of existing userspace
I.e. definitely a separate patch. -Daniel
On Tue, 8 Oct 2019 11:59:04 +0200 Daniel Vetter daniel@ffwll.ch wrote:
On Mon, Sep 30, 2019 at 10:07:07AM +0300, Pekka Paalanen wrote:
On Sun, 29 Sep 2019 20:30:44 +0000 Simon Ser contact@emersion.fr wrote:
Hi,
On Mon, Sep 23, 2019 at 12:39:20PM +0000, Simon Ser 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.
Some Arm's usage cases about two planes with same zpos.
- "Smart layer"
which can accepts 4 different fbs with different src/display rect, but this smart layer has one restriction:
4 display rects must have no overlaps in V direction (A optimization for android window like Toolbar/Navigation bar)
So when map this Smart-layer to drm world, it might be 4 different drm-planes, but with same zorder to indicate that these 4 planes are smart-laye planes.
- "VR-Layer"
One VR-layer comprises two different viewports which can be configured with totoally different fbs, src/display rect. we use two differernt drm-planes to drive on HW "VR-Layer", and these two drm-planes must be configured with same zpos.
Thanks a lot for your feedback James, that's exactly what I was looking for. Both of these use-cases make sense to me.
I think user-space should be prepared to handle identical immutable zpos values. Pekka and Daniel, thoughts?
Hi,
given those explained use cases, sure, I agree.
In any case, this doesn't change the patch itself. Probably something worth mentionning in the commit message.
Yes, recording these use cases would be very nice.
There's a lot more hw that does the same tricks (qualcom is one for sure). Imo before we encode this we should make sure that: a) everyone is happy with this new uapi
Sorry, what new UAPI?
We're just trying to make the documentation match what currently happens, right?
Thanks, pq
b) drivers are consistent c) maybe even testcases in igt d) definitely an open source user e) no breaking of existing userspace
I.e. definitely a separate patch. -Daniel
On Tue, Oct 8, 2019 at 1:39 PM Pekka Paalanen ppaalanen@gmail.com wrote:
On Tue, 8 Oct 2019 11:59:04 +0200 Daniel Vetter daniel@ffwll.ch wrote:
On Mon, Sep 30, 2019 at 10:07:07AM +0300, Pekka Paalanen wrote:
On Sun, 29 Sep 2019 20:30:44 +0000 Simon Ser contact@emersion.fr wrote:
Hi,
On Mon, Sep 23, 2019 at 12:39:20PM +0000, Simon Ser 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.
Some Arm's usage cases about two planes with same zpos.
- "Smart layer"
which can accepts 4 different fbs with different src/display rect, but this smart layer has one restriction:
4 display rects must have no overlaps in V direction (A optimization for android window like Toolbar/Navigation bar)
So when map this Smart-layer to drm world, it might be 4 different drm-planes, but with same zorder to indicate that these 4 planes are smart-laye planes.
- "VR-Layer"
One VR-layer comprises two different viewports which can be configured with totoally different fbs, src/display rect. we use two differernt drm-planes to drive on HW "VR-Layer", and these two drm-planes must be configured with same zpos.
Thanks a lot for your feedback James, that's exactly what I was looking for. Both of these use-cases make sense to me.
I think user-space should be prepared to handle identical immutable zpos values. Pekka and Daniel, thoughts?
Hi,
given those explained use cases, sure, I agree.
In any case, this doesn't change the patch itself. Probably something worth mentionning in the commit message.
Yes, recording these use cases would be very nice.
There's a lot more hw that does the same tricks (qualcom is one for sure). Imo before we encode this we should make sure that: a) everyone is happy with this new uapi
Sorry, what new UAPI?
We're just trying to make the documentation match what currently happens, right?
It's so much new uapi that I've sent out a few reverts for 5.4 (it landed in that merge window) to undo the stuff the arm driver team has done (it didn't come with userspace, proper discussion on dri-devel, docs or testcases in igt). I also just spotted that a leftover is that arm/komeda still computes its own version of normalized_zpos, which probably should be ditched too (it's not actually different from the one in helpers without the reverted uapi).
So yeah, separate patch :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Tuesday, October 8, 2019 6:03 PM, Daniel Vetter daniel@ffwll.ch wrote:
In any case, this doesn't change the patch itself. Probably something worth mentionning in the commit message.
Yes, recording these use cases would be very nice.
There's a lot more hw that does the same tricks (qualcom is one for sure). Imo before we encode this we should make sure that: a) everyone is happy with this new uapi
Sorry, what new UAPI? We're just trying to make the documentation match what currently happens, right?
It's so much new uapi that I've sent out a few reverts for 5.4 (it landed in that merge window) to undo the stuff the arm driver team has done (it didn't come with userspace, proper discussion on dri-devel, docs or testcases in igt). I also just spotted that a leftover is that arm/komeda still computes its own version of normalized_zpos, which probably should be ditched too (it's not actually different from the one in helpers without the reverted uapi).
So yeah, separate patch :-)
I don't get it. Do you want to split the docs changes for user-space, only keeping the doc changes for drivers in this patch?
User-space could already see duplicate zpos because of the non-atomic API. I don't think this introduces a new uAPI.
On Tue, Oct 8, 2019 at 5:11 PM Simon Ser contact@emersion.fr wrote:
On Tuesday, October 8, 2019 6:03 PM, Daniel Vetter daniel@ffwll.ch wrote:
In any case, this doesn't change the patch itself. Probably something worth mentionning in the commit message.
Yes, recording these use cases would be very nice.
There's a lot more hw that does the same tricks (qualcom is one for sure). Imo before we encode this we should make sure that: a) everyone is happy with this new uapi
Sorry, what new UAPI? We're just trying to make the documentation match what currently happens, right?
It's so much new uapi that I've sent out a few reverts for 5.4 (it landed in that merge window) to undo the stuff the arm driver team has done (it didn't come with userspace, proper discussion on dri-devel, docs or testcases in igt). I also just spotted that a leftover is that arm/komeda still computes its own version of normalized_zpos, which probably should be ditched too (it's not actually different from the one in helpers without the reverted uapi).
So yeah, separate patch :-)
I don't get it. Do you want to split the docs changes for user-space, only keeping the doc changes for drivers in this patch?
User-space could already see duplicate zpos because of the non-atomic API. I don't think this introduces a new uAPI.
I'm talking specifically about the "duplicated zpos values indicate special cloned planes like in the arm example" clarification. Not about splitting the zpos documentation any more, that's indeed just documenting existing uapi. But assigning the special meaning to duplicated zpos values (instead of just "can happen because non-atomic legacy userspace"), that is new uapi. Especially if we allow duplicated zpos for immutable properties (afaik that doesn't exist yet). -Daniel
On Tuesday, October 8, 2019 6:16 PM, Daniel Vetter daniel@ffwll.ch wrote:
On Tue, Oct 8, 2019 at 5:11 PM Simon Ser contact@emersion.fr wrote:
On Tuesday, October 8, 2019 6:03 PM, Daniel Vetter daniel@ffwll.ch wrote:
> In any case, this doesn't change the patch itself. Probably something worth > mentionning in the commit message.
Yes, recording these use cases would be very nice.
There's a lot more hw that does the same tricks (qualcom is one for sure). Imo before we encode this we should make sure that: a) everyone is happy with this new uapi
Sorry, what new UAPI? We're just trying to make the documentation match what currently happens, right?
It's so much new uapi that I've sent out a few reverts for 5.4 (it landed in that merge window) to undo the stuff the arm driver team has done (it didn't come with userspace, proper discussion on dri-devel, docs or testcases in igt). I also just spotted that a leftover is that arm/komeda still computes its own version of normalized_zpos, which probably should be ditched too (it's not actually different from the one in helpers without the reverted uapi). So yeah, separate patch :-)
I don't get it. Do you want to split the docs changes for user-space, only keeping the doc changes for drivers in this patch? User-space could already see duplicate zpos because of the non-atomic API. I don't think this introduces a new uAPI.
I'm talking specifically about the "duplicated zpos values indicate special cloned planes like in the arm example" clarification. Not about splitting the zpos documentation any more, that's indeed just documenting existing uapi. But assigning the special meaning to duplicated zpos values (instead of just "can happen because non-atomic legacy userspace"), that is new uapi. Especially if we allow duplicated zpos for immutable properties (afaik that doesn't exist yet).
Oh, I see. That makes sense, will send a new version.
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Daniel Vetter -
james qian wang (Arm Technology China) -
Marius Vlad -
Pekka Paalanen -
Simon Ser