Message-ID: <4e0d66f7-6234-4265-bfbb-a919224ecb2a@bootlin.com>
Date: Wed, 19 Feb 2025 17:28:37 +0100
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: Maxime Ripard <mripard@...nel.org>,
 Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>,
 Melissa Wen <melissa.srw@...il.com>, Maíra Canal
 <mairacanal@...eup.net>, Haneen Mohammed <hamohammed.sa@...il.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, jose.exposito89@...il.com,
 dri-devel@...ts.freedesktop.org, arthurgrillo@...eup.net,
 linux-kernel@...r.kernel.org, jeremie.dautheribes@...tlin.com,
 miquel.raynal@...tlin.com, thomas.petazzoni@...tlin.com,
 seanpaul@...gle.com, nicolejadeyee@...gle.com
Subject: Re: [PATCH RFC v2 00/16] drm/vkms: ConfigFS interface



Le 19/02/2025 à 14:15, Simona Vetter a écrit :
> On Wed, Feb 19, 2025 at 10:28:56AM +0100, Maxime Ripard wrote:
>> On Tue, Dec 17, 2024 at 05:42:56PM +0100, Louis Chauvet wrote:
>>> Hi,
>>>
>>>>> Hi all,
>>>>>
>>>>> I am also currently working on MST emulation for VKMS. If someone can read
>>>>> what I already did and at tell me if my implementation seems on the right
>>>>> track it could be nice.
>>>>>
>>>>> The current status is not very advanced: I can emulate a mst HUB, but not
>>>>> a screen. I am currently working on properly emulating the HUB by using an
>>>>> other hub.
>>>>>
>>>>> You can find the branch for this work here:
>>>>> https://gitlab.freedesktop.org/louischauvet/kernel/-/tree/b4/vkms-mst
> 
> Can't look at this because it's private.

Hi Maxime, Sima,

Normally, those should work:

https://gitlab.freedesktop.org/louischauvet/kernel/-/tree/vkms-mst
https://github.com/Fomys/linux/tree/vkms-mst

I just re-tested, this is broken, probably because I never had the time 
to properly finish my last idea: simplifying vkms_connector by creating 
vkms_mst_emulator_root. With the rest of the code (i.e. 
vkms_mst_hub/display_emulator + vkms_connector), I was able to make this 
config working:

HUB1 -> HUB2 -> DISPLAY1
      |       -> DISPLAY2
      -> DISPLAY3

(working == it was listed properly by modetest + did not report any 
issue when using a connector with modetest -s)

Few things to note: no ConfigFS support, no eBPF support, poorly tested 
(there are probably multithread/recursion issues)

I had to stop working on it because I don't have anymore time, I plan to 
at least rebase + send an RFC once the VKMS+ConfigFS work is done.

>>>> I think this is exactly the kind of things where we'll want eBPF I
>>>> think. There's no way you'll be able to model each possible test
>>>> scenarios for MST through configfs, even more so with a stable
>>>> interface.
>>>
>>> I just spent some time to think about it. I agree that eBPF seems
>>> applicable: we want to allows userspace to emulate any MST device, and we
>>> don't want to create huge uAPI + whole emulation in the kernel.
>>>
>>> As most of the protocol is similar accros device kind, I currently built
>>> my code around the struct vkms_mst_sideband_helpers to specify only the
>>> changed behavior (this way the "write to registers" "parse command"... is
>>> only done in one place). The choice of function is not definitive at all
>>> (it was just practical at this moment).
>>>
>>> With eBPF, I know three different way to attach programs to the kernel:
>>>   - Using a kprobe/attaching to a function: I can change the behavior of
>>>     all the device, there is no way "attach prog1 to hub" and "attach prog2
>>>     to screen1", it will be "attach prog1 to the function
>>>     `vkms_mst_emulator_handle_transfer_write`, and all the device will be
>>>     affected. This should be very easy to implement (maybe it already
>>>     works without modification?), but very limited / make userspace stuff
>>>     very ugly => Not ideal at all
>>>   - Creating a whole architecture to attach eBPF programs in vkms: VKMS
>>>     manage the list of attached eBPF programs, call them when it needs...
>>>     This is probably the "most flexible" option (in the sense "VKMS can do
>>>     whatever we need to fit our usecase"). This seems not trivial to
>>>     implement (drm complexity + MST complexity + BPF complexity in the same
>>>     driver seems a bad idea, espicially because VKMS don't have a lot of
>>>     maintainance and I don't feel confident introducing more complexity)
>>>     => Can work, require some work, but may bring more complexity in VKMS
>>>   - Using BPF struct_ops: I can "simply" create/complete a struct ops for
>>>     diverse mst callbacks (see the proposition bellow). I looked at some
>>>     example, this seems to be "easy" to do, and the work in VKMS should be
>>>     limited.
>>>     => Can work, a bit less flexible than the previous solution, but avoid
>>>     a lot of complexity
>>>
>>> I don't know if I will be able to finish the implementation but I imagine
>>> something like that may be a nice interface (may be not possible "as is"):
>>>
>>> // vkms_mst.c struct_ops that can be filled by userspace with custom
>>> // functions
>>> // Known limitation: maximum 64 functions in this table
>>> struct vkms_mst_ops {
>>> 	// Completly overide the transfer function, so the userspace can
>>> 	// do whatever he wants (even emulating a complex MST tree
>>> 	// without using stuff in vkms)
>>> 	handle_transfer(drm_dp_aux_msg);
>>>
>>> 	// If default transfer function is used, those are the callback
>>> 	// you can use (again, they will come with default
>>> 	// implementation)
>>> 	clear_payload_id_table(..);
>>> 	link_address(..);
>>> 	enum_path_ressources(..);
>>> 	[...]
>>>
>>> 	// Used to identify this kind of device, in my example the
>>> 	// userspace could register "LG_screen", "dell dock", "HP screen",
>>> 	// and then configure each mst device with the correct kind
>>> 	name = "<unique name for this device kind>",
>>>
>>> 	// If you want to use the default "hub" implementation, but only
>>> 	// tweak one specific behavior, you can use this
>>> 	base = "<name of the other structops>"
>>> }
>>>
>>>
>>> // Needed to implement eBPF struct_ops, called when userspace registers a
>>> // struct_ops of type vkms_mst_ops
>>> void register_struct_ops(new_ops...) {
>>> 	vkms_registered_ops.append(new_ops).
>>> }
>>>
>>> // In vkms_connector.c
>>> // Callback called by drm core to do transfer on the connector
>>> void vkms_mst_transfer(aux, msg) {
>>> 	mst_emulator = get_mst_emulator(aux);
>>>
>>> 	ops = vkms_registered_ops.search_for(mst_emulator.name);
>>> 	ops->handle_transfer(msg);
>>> }
>>>
>>> // mst_ebpf.c In the BPF program, userspace side
>>> void handle_transfer(...) {
>>> 	[...]
>>> }
>>> struct vkms_mst_ops {
>>> 	handle_transfer = handle_transfer;
>>> 	name = "lg-screen";
>>> 	base = "default-screen"
>>> }
>>
>> I don't think MST is the right abstraction there. We should have MST
>> related helper functions available to eBPF programs, but we should load
>> them at the connector level, ie, implement a full blown connector
>> atomic_check for example. It's more flexible and will allow to implement
>> other use-cases people have been interested in (like panels).
> 
> So since I can't look at the code I'll just drop my thoughts here.
> 
> - I think validating the MST helpers implementation should be done with
>    kunit tests. So these are out of scope for vkms testing I think
>    entirely.

Yes, I agree with this, and it joins your last comment: the full dp-aux 
emulation does not belong only to VKMS. I had this idea only because my 
solution only use the normal core MST implementation (no special 
handling in VKMS, just pure dp-aux emulation), so technically you could 
also stress-test drm core with it.

> - Next up are the driver implementations. There we might want to be able
>    to inject fake mst devices to stress-test driver corner cases. But I
>    don't think that's a job for vkms either.

I agree, VKMS is not here to test other drivers.

> - Now for vkms itself, I think the interesting case here is being able to
>    test compositors against funny mst corner-cases, but for that we don't
>    really need an mst operation at all. So no dp-aux or anything like that,
>    we just hotplug-create connectors with names and PATH property and MST
>    type, without any of the kernel-internal presentations for hubs/branch
>    points and all that stuff. Because userspace doesn't ever see that.

I knew that user space don't really see the MST information (apart from 
PATH), but I did not think about this solution. This may work well to 
test user space, I agree!

I think we are on the good track with José, he is trying to implement 
connector hot-creation through ConfigFS [1]. To add "MST emulation", we 
can "simply" add the PATH property through ConfigFS.

[1]: Few discussions here 
https://lore.kernel.org/all/Z5zJ1rEZyBEgd7DN@louis-chauvet-laptop/

> - Next up is expressing all the funny constraints that can result in,
>    across different drivers. For that I think we want ebpf to implement the
>    various atomic_check hooks, so that you can implement all the funny
>    constraints of mst link bw limitations, but also host-side display
>    limitations. And I concur with Maxime that this ebpf support should be
>    entirely agnostic and just allow you to attach atomic_check
>    implementations to connectors, planes and crtcs. And then maybe one for
>    the overall commit, so that global constraints are easier to implement.

If I understand correctly, this has nothing to do with VKMS + MST?
I don't clearly understand the use case: why do we want the user space 
to express hardware limitations? If you have already discussed this on 
the mailing list, can you share the discussion?

> So summary: MST testing in vkms only needs to look like MST to userspace.
> Internally we'll just hand-roll the entire connector hotplug and leave out
> everything else. Because testing driver dp mst implementations and the
> helpers is better covered through different means.
 >
> Eventually we might want to implement fake i2c and dp-aux implementations
> for additional features like TV remote control and fun stuff like that (I
> forgot the CEA/HDMI name for these). But I think that's waaaayyyyyy down
> the road.

I think I am not that far from a full dp-aux emulation, it is precisely 
what I tried to do in VKMS. I don't have the time to transform it to 
kunit tests.

Thanks a lot for your feedback!
Louis Chauvet

> Cheers, Sima

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

Powered by blists - more mailing lists