Message-ID: <20250219-versatile-smilodon-of-felicity-cbcc4d@houat>
Date: Wed, 19 Feb 2025 10:28:56 +0100
From: Maxime Ripard <mripard@...nel.org>
To: 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>, Simona Vetter <simona.vetter@...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

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
> > 
> > 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).

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)

Powered by blists - more mailing lists