[PATCH 0/2] drm/vkms: Fix plane blending z-order

[PATCH 0/2] drm/vkms: Fix plane blending z-order

[Louis Chauvet]

As reported by Marius [1], the current blending algorithm for vkms planes 
is not future-proof. Currently the z-ordering is only garanteed by the 
creation order. As the future ConfigFS interface will allows to create 
planes, this order may vary.

To avoid this, add the zpos property and blend the planes according to 
this zpos order.

[1]:https://lore.kernel.org/all/aHpGGxZyimpJ8Ehz@xpredator/

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
Louis Chauvet (2):
      drm/vkms: Add zpos property to planes
      drm/vkms: Properly order plane for blending

 drivers/gpu/drm/vkms/vkms_crtc.c  | 26 ++++++++++++++++++++------
 drivers/gpu/drm/vkms/vkms_drv.c   |  1 +
 drivers/gpu/drm/vkms/vkms_plane.c | 12 ++++++++++++
 3 files changed, 33 insertions(+), 6 deletions(-)
---
base-commit: 82928cc1c2b2be16ea6ee9e23799ca182e1cd37c
change-id: 20250801-vkms-fix-zpos-1dc6a3f51a50

Best regards,
-- 
Louis Chauvet <louis.chauvet@bootlin.com>

[PATCH 1/2] drm/vkms: Add zpos property to planes

[Louis Chauvet]

Currently no zpos are defined for vkms planes. This is not yet an
issue, but with the future introduction of configfs, we need to have a way
to properly order plane composition.

In order to make it predictable, add zpos=0 for primary, zpos=1 for
overlays and zpos=2 for cursor. This will be configurable later.

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/gpu/drm/vkms/vkms_plane.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
index e3fdd161d0f0a1d20c14a79dbe51c08c8486d12f..44bf9eedf5a7641f2eb171c8995079ca65dfa04e 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -232,5 +232,17 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
 					  DRM_COLOR_YCBCR_BT601,
 					  DRM_COLOR_YCBCR_FULL_RANGE);
 
+	switch (type) {
+	case DRM_PLANE_TYPE_PRIMARY:
+		drm_plane_create_zpos_immutable_property(&plane->base, 0);
+		break;
+	case DRM_PLANE_TYPE_OVERLAY:
+		drm_plane_create_zpos_immutable_property(&plane->base, 1);
+		break;
+	case DRM_PLANE_TYPE_CURSOR:
+		drm_plane_create_zpos_immutable_property(&plane->base, 2);
+		break;
+	}
+
 	return plane;
 }

-- 
2.50.1

[PATCH 2/2] drm/vkms: Properly order plane for blending

[Louis Chauvet]

The current blending algorithm requires that vkms_state->active_planes be
ordered by z-order. This has not been an issue so far because the planes
are registered in the correct order in DRM (primary-overlay-cursor).
This new algorithm now uses the configured z-order to populate
active_planes.

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 26 ++++++++++++++++++++------
 drivers/gpu/drm/vkms/vkms_drv.c  |  1 +
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index e60573e0f3e9510252e1f198b00e28bcc7987620..f3f3ad8ede29987b385d53834970bcd0c6adefa7 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -200,14 +200,28 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
 	vkms_state->num_active_planes = i;
 
 	i = 0;
-	drm_for_each_plane_mask(plane, crtc->dev, crtc_state->plane_mask) {
-		plane_state = drm_atomic_get_existing_plane_state(crtc_state->state, plane);
 
-		if (!plane_state->visible)
-			continue;
+	unsigned int current_zpos = 0;
+
+	while (i < vkms_state->num_active_planes) {
+		unsigned int next_zpos = UINT_MAX;
+
+		drm_for_each_plane_mask(plane, crtc->dev, crtc_state->plane_mask) {
+			plane_state = drm_atomic_get_existing_plane_state(crtc_state->state, plane);
+
+			if (!plane_state->visible)
+				continue;
+
+			if (plane_state->normalized_zpos == current_zpos)
+				vkms_state->active_planes[i++] = to_vkms_plane_state(plane_state);
+
+			if (plane_state->normalized_zpos > current_zpos &&
+			    plane_state->normalized_zpos < next_zpos)
+				next_zpos = plane_state->normalized_zpos;
+		}
+
+		current_zpos = next_zpos;
 
-		vkms_state->active_planes[i++] =
-			to_vkms_plane_state(plane_state);
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index e8472d9b6e3b2b5d6d497763288bf3dc6fde5987..e492791a7a970b768184292bf82318e3fca6b36a 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -135,6 +135,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev)
 	dev->mode_config.max_height = YRES_MAX;
 	dev->mode_config.cursor_width = 512;
 	dev->mode_config.cursor_height = 512;
+	dev->mode_config.normalize_zpos = true;
 	/*
 	 * FIXME: There's a confusion between bpp and depth between this and
 	 * fbdev helpers. We have to go with 0, meaning "pick the default",

-- 
2.50.1

[PATCH 0/2] drm/vkms: Fix plane blending z-order

[José Expósito]

Hi Louis,

I already made some comments about zpos here:
https://lore.kernel.org/dri-devel/aJDDr_9soeNRAmm0@fedora/
But let's start the conversation here as well!

> As reported by Marius [1], the current blending algorithm for vkms planes 
> is not future-proof. Currently the z-ordering is only garanteed by the 
> creation order. As the future ConfigFS interface will allows to create 
> planes, this order may vary.
>
> To avoid this, add the zpos property and blend the planes according to 
> this zpos order.
>
> [1]:https://lore.kernel.org/all/aHpGGxZyimpJ8Ehz@xpredator/

In case you want to have a look, 3 years ago I sent a patch adding the
property and blending following the zpos order, but it wasn't merged:
https://github.com/JoseExposito/linux/commit/befc79a1341b27eb328b582c3841097d17ccce71
The way "vkms_state->active_planes" is set is a bit simpler, but it might
not be valid anymore due to code changes.

About this series, I didn't have a chance to run IGT test to validate it,
but in general your code looks good.

My only question is, how do we avoid breaking changes in the configfs side?

For the mutable/immutable configuration it'd be easy: We set it to
immutable by default, i.e, when the user creates a new plane via configfs:

  $ sudo mkdir /sys/kernel/config/vkms/<device name>/planes/<plane name>

We set "planes/<plane name>/zpos_mutability" to immutable.

However, we don't know the plane type (required to set the zpos value) when
the user creates a new plane on configfs.
Therefore, we can not set the correct value in "planes/<plane name>/zpos".
Have you already figured out a solution for this?

Jose

PS - In case you missed it, I created:
https://github.com/JoseExposito/vkmsctl
I'll add zpos there once we support it in configfs :)

Re: [PATCH 0/2] drm/vkms: Fix plane blending z-order

[Louis Chauvet]

Le 01/09/2025 à 16:52, José Expósito a écrit :
> Hi Louis,
> 
> I already made some comments about zpos here:
> https://lore.kernel.org/dri-devel/aJDDr_9soeNRAmm0@fedora/
> But let's start the conversation here as well!
> 
>>   As reported by Marius [1], the current blending algorithm for vkms planes
>> is not future-proof. Currently the z-ordering is only garanteed by the
>> creation order. As the future ConfigFS interface will allows to create
>> planes, this order may vary.
>>
>> To avoid this, add the zpos property and blend the planes according to
>> this zpos order.
>>
>> [1]:https://lore.kernel.org/all/aHpGGxZyimpJ8Ehz@xpredator/
> 
> In case you want to have a look, 3 years ago I sent a patch adding the
> property and blending following the zpos order, but it wasn't merged:
> https://github.com/JoseExposito/linux/commit/befc79a1341b27eb328b582c3841097d17ccce71
> The way "vkms_state->active_planes" is set is a bit simpler, but it might
> not be valid anymore due to code changes.

I looked quickly at your series, your solution is better (no complex 
loops), I need to check if I can use your solution, it is on my 
todo-list (finish to review igt patches too).

> About this series, I didn't have a chance to run IGT test to validate it,
> but in general your code looks good.
> 
> My only question is, how do we avoid breaking changes in the configfs side?

The "easy" solution is to have zpos in the first iteration of ConfigFS, 
but that's not cool and will slow down again the merge, I would like to 
avoid.

> For the mutable/immutable configuration it'd be easy: We set it to
> immutable by default, i.e, when the user creates a new plane via configfs:
> 
>    $ sudo mkdir /sys/kernel/config/vkms/<device name>/planes/<plane name>
> 
> We set "planes/<plane name>/zpos_mutability" to immutable.
> 
> However, we don't know the plane type (required to set the zpos value) when
> the user creates a new plane on configfs.

Two ideas:

if (plane_cfg.zpos was not updated by the user)
	change the plane_cfg.zpos every time plane_cfg.type is changed
else
	don't change plane_cfg.zpos

IDK if this is a good idea or not, but it should not break UAPI (older 
software can't change zpos because it did not exists, new softwares can 
manually override it).

This is not good because it will change the behavior depending on the 
previous action of the user + we need to track in vkms if the value was 
changed or not, clearly not ideal.

Maybe better idea:

plane_cfg.zpos = -1 by default (or whatever special value, I need to 
check which values are invalid for zpos)

On DRM plane creation:
if(plane_cfg.zpos == -1)
	plane.zpos = 1/2/3 depending on plane type
else
	plane.zpos = plane_cfg.zpos

> Therefore, we can not set the correct value in "planes/<plane name>/zpos".
> Have you already figured out a solution for this?
>
> Jose
> 
> PS - In case you missed it, I created:
> https://github.com/JoseExposito/vkmsctl
> I'll add zpos there once we support it in configfs :)

I just received it, amazing tool (in fact, I already had mine (in rust 
too), but very very dirty and some stuff hardcoded)
Once I have the time, I will create a PR with some properties I have on 
my branches, so they will be ready.

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com