Message-ID: <4005bcd6-968b-4f49-a836-691f522af90e@bootlin.com>
Date: Mon, 1 Sep 2025 17:33:00 +0200
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: José Expósito <jose.exposito89@...il.com>
Cc: airlied@...il.com, dri-devel@...ts.freedesktop.org,
 hamohammed.sa@...il.com, linux-kernel@...r.kernel.org,
 maarten.lankhorst@...ux.intel.com, melissa.srw@...il.com,
 mripard@...nel.org, simona@...ll.ch, thomas.petazzoni@...tlin.com,
 tzimmermann@...e.de
Subject: Re: [PATCH 0/2] drm/vkms: Fix plane blending z-order



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

Powered by blists - more mailing lists