[PATCH RFC v2 00/16] drm/vkms: ConfigFS interface

Louis Chauvet posted 16 patches 1 year, 2 months ago
There is a newer version of this series
drivers/gpu/drm/vkms/Kconfig         |    1 +
drivers/gpu/drm/vkms/Makefile        |    1 +
drivers/gpu/drm/vkms/vkms_config.c   |   36 +
drivers/gpu/drm/vkms/vkms_config.h   |    6 +-
drivers/gpu/drm/vkms/vkms_configfs.c | 1404 ++++++++++++++++++++++++++++++++++
drivers/gpu/drm/vkms/vkms_configfs.h |  128 ++++
drivers/gpu/drm/vkms/vkms_drv.c      |   21 +-
drivers/gpu/drm/vkms/vkms_drv.h      |    3 +
8 files changed, 1594 insertions(+), 6 deletions(-)
[PATCH RFC v2 00/16] drm/vkms: ConfigFS interface
Posted by Louis Chauvet 1 year, 2 months ago
VKMS is manly used to test userspace program and its behavior. The current 
implementation is not very configurable as you can only have one device, 
with few specific planes.

This series aims to introduce a new interface, using ConfigFS, to create 
and configure more devices. This will introduce:
- Device creation
- Plane creation
- Plane configuration (type, color encoding, color range, rotations)
- Encoder creation
- CRTC creation
- Linking between CRTC and planes/encoders

The proposition is:
/config/vkms
	DEVICE_1
	┣━ enable
	┣━ writeback
	┣━ planes
	┃  ┣━ PLANE_1
	┃  ┃  ┣━ type
	┃  ┃  ┣━ supported_rotations
	┃  ┃  ┣━ color_range
	┃  ┃  ┣━ color_encoding
	┃  ┃  ┣━ default_color_encoding
	┃  ┃  ┣━ default_rotations
	┃  ┃  ┣━ default_color_range
	┃  ┃  ┗━ possible_crtcs
	┃  ┃     ┗━ >> /config/vkms/DEVICE_1/crtc/CRTC_1 
	┃  ┣━ PLANE_2
	┃  ┃  ┗━ ditto
	┃  ┗━ PLANE_3
	┃     ┗━ ditto
	┃
	┣━ encoders
	┃  ┣━ ENCODER_1
	┃  ┃  ┗━ possible_crtcs
	┃  ┃     ┗━ >> /config/vkms/DEVICE_1/crtc/CRTC_1
	┃  ┗━ ENCODER_2
	┃     ┗━ ditto
	┃
	┗━ crtc
	   ┗━ CRTC_1
	
This interface aims to be extendable (new property can easly be added in 
objects) and easy to use (objects are created simply by creating folders, 
and configured by writing files).

This series depends on 
https://lore.kernel.org/all/20241122-google-remove-crtc-index-from-parameter-v2-0-81540742535a@bootlin.com
but as this is a bit complex to rebase, you can find a working branch 
here:
https://gitlab.freedesktop.org/louischauvet/kernel/-/tree/b4/vkms-configfs

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
Changes in v2:
- Added many new configuration (mainly connector)
- Link to v1: https://lore.kernel.org/r/20240814-google-config-fs-v1-0-8363181907a6@bootlin.com

---
Louis Chauvet (16):
      drm/vkms: Add vkms_delete/create_device helper
      drm/vkms: Cleanup configuration field on device destroy
      drm/vkms: Introduce ConfigFS interface
      drm/vkms: Introduce configfs for plane
      drm/vkms: Introduce configfs for plane rotation
      drm/vkms: Introduce configfs for plane color encoding
      drm/vkms: Introduce configfs for plane color range
      drm/vkms: Introduce configfs for crtc and encoder
      drm/vkms: Introduce configfs for connectors
      drm/vkms: Introduce configfs for connector type
      drm/vkms: Introduce configfs for plane format
      drm/vkms: Introduce configfs for device name
      drm/vkms: Introduce configfs for connector status
      drm/vkms: Introduce configfs for connector id
      drm/vkms: Introduce configfs for connector EDID
      drm/vkms: Introduce configfs for encoder type

 drivers/gpu/drm/vkms/Kconfig         |    1 +
 drivers/gpu/drm/vkms/Makefile        |    1 +
 drivers/gpu/drm/vkms/vkms_config.c   |   36 +
 drivers/gpu/drm/vkms/vkms_config.h   |    6 +-
 drivers/gpu/drm/vkms/vkms_configfs.c | 1404 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/vkms/vkms_configfs.h |  128 ++++
 drivers/gpu/drm/vkms/vkms_drv.c      |   21 +-
 drivers/gpu/drm/vkms/vkms_drv.h      |    3 +
 8 files changed, 1594 insertions(+), 6 deletions(-)
---
base-commit: 98efdd02e220fea84c1491012d7292749a71faeb
change-id: 20240521-google-config-fs-b935b66b8d7b
prerequisite-message-id: 20241122-google-vkms-managed-v5-0-1ab60403e960@bootlin.com
prerequisite-patch-id: b608594ad493a41000ee703792eac4b23f9e35dc
prerequisite-patch-id: 5697aa87c44bbf3eda8a1ba424465dc792545d4c
prerequisite-patch-id: 223d59c407ce28dacf3f563b5c0148d2398303f1
prerequisite-patch-id: 720b75b21d06ce3d3f060fb9238f7903834da0e1
prerequisite-patch-id: 30a1e033fa43241ca6a43006fd4f29f8e9217224
prerequisite-message-id: 20241122-b4-vkms-allocated-v2-0-ff7bddbf0bfb@bootlin.com
prerequisite-patch-id: 9741873a5f0a7a3cf117dec7837354c3ad38ac3a
prerequisite-patch-id: 1a383d1494e4f2142b62822f2ba482a3b813563a
prerequisite-patch-id: 7d3f49fee4d3553d52fc075b7868da9dea9209cd
prerequisite-patch-id: 57f5aeff2a9e8f2b6f47569e44dcd8fa587ed4bf
prerequisite-message-id: 20241122-b4-new-color-formats-v3-0-23f7776197c9@bootlin.com
prerequisite-patch-id: e6717b75d79ae5cfb0815bab88d722082107dc0e
prerequisite-patch-id: 4b3b1ea5ad2e3ba1922cd4b3d3d46214b27c8c2d
prerequisite-patch-id: 060874d5a7433cc8cc654bc63e0b411036727ebb
prerequisite-patch-id: 43115d21842e508d9d8b0468e15f67d442bffe3c
prerequisite-patch-id: 627d0970e76d4154c982d0d4172e7a0c4dfb9a4c
prerequisite-patch-id: 582445144ac0ab11175ef96262060b08a5e1467e
prerequisite-patch-id: a98fac5a2c60fe23fbc6a455e9a4ab8b0f187ee8
prerequisite-patch-id: 62c8d109a22b9978f755255b67f13fe74fb7008d
prerequisite-message-id: 20241122-writeback_line_by_line-v3-0-085d5810f6e3@bootlin.com
prerequisite-patch-id: 07868dd9c7bbb1ed96d675c689de86f0cf293248
prerequisite-patch-id: 736638b76050ef7a99cfad2c1560f7af114d5fbd
prerequisite-patch-id: 20d8823f9c1d372ab2b88f969f5110f77e49c7f9
prerequisite-message-id: 20241122-google-remove-crtc-index-from-parameter-v2-0-81540742535a@bootlin.com
prerequisite-patch-id: a71faf4da3bf2d2e3e8744d9ca688d7ad47d86a1
prerequisite-patch-id: aefc656e8bc9edfd5527971231b7db23b416a19a
prerequisite-patch-id: f17cbbe9923f78e4d5fdce56d0fb3c3af1fa3b82
prerequisite-patch-id: d7a7e404fcb1656d722b59ef13da771e6156916f
prerequisite-patch-id: f0d5640738b5947ab84272c458a2f729a611ab0f
prerequisite-patch-id: 26b9a61db21be516a2c84072a71f2c11d21a828d
prerequisite-patch-id: 93571d6f80d345fa5d705d8a677bca183852554f
prerequisite-patch-id: 9c8bee631c4a466b195adbfadb62eb15fa371cce
prerequisite-patch-id: b098550055ebeabd85bc5a5ea7c9aae2a52a4ac8
prerequisite-patch-id: 83a4752516471b5152d5e2cb6b56d7ecaae70f66
prerequisite-patch-id: f02ce6f3ffcb94f955f7b51f6d2120b93cc9f76e
prerequisite-patch-id: 75a8bb8ced4d23de2de8b5355b395171a7782981
prerequisite-patch-id: a6b7a6442de27a3815985a1857ae6eceecc3687a
prerequisite-patch-id: c47a619ac3b9b244375776aa03b6c19c32108cca
prerequisite-patch-id: 4a5efd82cc55ea5897e8bffad0d12f22e4ee3fd7
prerequisite-patch-id: c0de37e1ddbb9c040820a341284cb5814ceb4d71
prerequisite-patch-id: c87e20092e0a2f6d657837dbf1611b9dbf100b2c
prerequisite-patch-id: c12f46d01d578587d93d20e9d987394ab63a65ab

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

Re: [PATCH RFC v2 00/16] drm/vkms: ConfigFS interface
Posted by Louis Chauvet 1 year, 2 months ago
On 22/11/24 - 18:38, Louis Chauvet wrote:
> VKMS is manly used to test userspace program and its behavior. The current 
> implementation is not very configurable as you can only have one device, 
> with few specific planes.
> 
> This series aims to introduce a new interface, using ConfigFS, to create 
> and configure more devices. This will introduce:
> - Device creation
> - Plane creation
> - Plane configuration (type, color encoding, color range, rotations)
> - Encoder creation
> - CRTC creation
> - Linking between CRTC and planes/encoders
> 
> The proposition is:
> /config/vkms
> 	DEVICE_1
> 	┣━ enable
> 	┣━ writeback
> 	┣━ planes
> 	┃  ┣━ PLANE_1
> 	┃  ┃  ┣━ type
> 	┃  ┃  ┣━ supported_rotations
> 	┃  ┃  ┣━ color_range
> 	┃  ┃  ┣━ color_encoding
> 	┃  ┃  ┣━ default_color_encoding
> 	┃  ┃  ┣━ default_rotations
> 	┃  ┃  ┣━ default_color_range
> 	┃  ┃  ┗━ possible_crtcs
> 	┃  ┃     ┗━ >> /config/vkms/DEVICE_1/crtc/CRTC_1 
> 	┃  ┣━ PLANE_2
> 	┃  ┃  ┗━ ditto
> 	┃  ┗━ PLANE_3
> 	┃     ┗━ ditto
> 	┃
> 	┣━ encoders
> 	┃  ┣━ ENCODER_1
> 	┃  ┃  ┗━ possible_crtcs
> 	┃  ┃     ┗━ >> /config/vkms/DEVICE_1/crtc/CRTC_1
> 	┃  ┗━ ENCODER_2
> 	┃     ┗━ ditto
> 	┃
> 	┗━ crtc
> 	   ┗━ CRTC_1
> 	
> This interface aims to be extendable (new property can easly be added in 
> objects) and easy to use (objects are created simply by creating folders, 
> and configured by writing files).
> 
> This series depends on 
> https://lore.kernel.org/all/20241122-google-remove-crtc-index-from-parameter-v2-0-81540742535a@bootlin.com
> but as this is a bit complex to rebase, you can find a working branch 
> here:
> https://gitlab.freedesktop.org/louischauvet/kernel/-/tree/b4/vkms-configfs
> 
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>

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

Thanks a lot,
Louis Chauvet
Re: [PATCH RFC v2 00/16] drm/vkms: ConfigFS interface
Posted by Maxime Ripard 1 year, 2 months ago
Hi,

On Fri, Nov 22, 2024 at 06:44:18PM +0100, Louis Chauvet wrote:
> On 22/11/24 - 18:38, Louis Chauvet wrote:
> > VKMS is manly used to test userspace program and its behavior. The current 
> > implementation is not very configurable as you can only have one device, 
> > with few specific planes.
> > 
> > This series aims to introduce a new interface, using ConfigFS, to create 
> > and configure more devices. This will introduce:
> > - Device creation
> > - Plane creation
> > - Plane configuration (type, color encoding, color range, rotations)
> > - Encoder creation
> > - CRTC creation
> > - Linking between CRTC and planes/encoders
> > 
> > The proposition is:
> > /config/vkms
> > 	DEVICE_1
> > 	┣━ enable
> > 	┣━ writeback
> > 	┣━ planes
> > 	┃  ┣━ PLANE_1
> > 	┃  ┃  ┣━ type
> > 	┃  ┃  ┣━ supported_rotations
> > 	┃  ┃  ┣━ color_range
> > 	┃  ┃  ┣━ color_encoding
> > 	┃  ┃  ┣━ default_color_encoding
> > 	┃  ┃  ┣━ default_rotations
> > 	┃  ┃  ┣━ default_color_range
> > 	┃  ┃  ┗━ possible_crtcs
> > 	┃  ┃     ┗━ >> /config/vkms/DEVICE_1/crtc/CRTC_1 
> > 	┃  ┣━ PLANE_2
> > 	┃  ┃  ┗━ ditto
> > 	┃  ┗━ PLANE_3
> > 	┃     ┗━ ditto
> > 	┃
> > 	┣━ encoders
> > 	┃  ┣━ ENCODER_1
> > 	┃  ┃  ┗━ possible_crtcs
> > 	┃  ┃     ┗━ >> /config/vkms/DEVICE_1/crtc/CRTC_1
> > 	┃  ┗━ ENCODER_2
> > 	┃     ┗━ ditto
> > 	┃
> > 	┗━ crtc
> > 	   ┗━ CRTC_1
> > 	
> > This interface aims to be extendable (new property can easly be added in 
> > objects) and easy to use (objects are created simply by creating folders, 
> > and configured by writing files).
> > 
> > This series depends on 
> > https://lore.kernel.org/all/20241122-google-remove-crtc-index-from-parameter-v2-0-81540742535a@bootlin.com
> > but as this is a bit complex to rebase, you can find a working branch 
> > here:
> > https://gitlab.freedesktop.org/louischauvet/kernel/-/tree/b4/vkms-configfs
> > 
> > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> 
> 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.

Maxime
Re: [PATCH RFC v2 00/16] drm/vkms: ConfigFS interface
Posted by Louis Chauvet 1 year, 1 month ago
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"
}

How to use it (screen directly connected to connector, or complete
emulation by the eBPF program):

	gcc mst_ebpf.c
	bpftool register structops mst_ebpf
	# Create vkms device + connector
	mkdir -p /configfs/vkms/mydev/connectors/connector1
	#[skipped initialization of plane/crtc...]
	mkdir -p /configfs/vkms/mydev/mst_devices/device1
	echo "lg-screen" > /configfs/vkms/mydev/mst_devices/device1/name
	ln -s /configfs/vkms/mydev/connectors/connector1/device /configfs/vkms/mydev/mst_devices/device1

How to use it (hub + two screens, using vkms scaffolding to make the
emulation easier) (the idea is to do something like the tcp_congestion 
algorithm, where a few of them are implemented in the kernel, but 
userspace can inject custom implementations):

	bpftool register mst_ebpf_screen1 # struct_ops with name=lg-screen
	bpftool register mst_ebpf_screen2 # struct_ops with name=hp-screen
	#[skiped initialization of vkms dev]
	mkdir -p /configfs/vkms/mydev/mst_devices/screen1
	mkdir -p /configfs/vkms/mydev/mst_devices/screen2
	mkdir -p /configfs/vkms/mydev/mst_devices/hub
	echo "lg-screen" > /configfs/vkms/mydev/mst_devices/screen1/name
	echo "hp-screen" > /configfs/vkms/mydev/mst_devices/screen2/name
	# default-hub is the default hub emulation provided by VKMS
	echo "default-hub" > configfs/vkms/mydev/mst_devices/hub/name
	ln -s /configfs/vkms/mydev/connectors/connector1/device /configfs/vkms/mydev/mst_devices/hub
	ln -s /configfs/vkms/mydev/mst_devices/hub/child1 /configfs/vkms/mydev/mst_devices/screen1
	ln -s /configfs/vkms/mydev/mst_devices/hub/child2 /configfs/vkms/mydev/mst_devices/screen2

A few things that this approach can bring:
 - Full emulation by userspace (just add one device and provide an eBPF
   program that emulates the whole MST chain)
 - Partial emulation of devices (e.g., the default-screen implementation is
   sufficient, but you want to tweak something inside)
 - Full emulation by the kernel, using default implementations (I think
   only hub and screen, just to be able to emulate the "basic"
   configurations)
 - And cool new to reduce the "it should be perfect from the start", if we 
   use kfunc + struct_ops, both can change a little bit over time (kfunc 
   are not part of uAPI and struct_ops allows to add argument/functions). 
   Stabilisation can come later.

What do you think about this idea ?

My current plan is to continue on the "kernel-only" implementation, so I 
can have a working poc, and then switch to the eBPF work after.

Thanks,
Louis Chauvet

> Maxime
Re: [PATCH RFC v2 00/16] drm/vkms: ConfigFS interface
Posted by Maxime Ripard 11 months, 3 weeks ago
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
Re: [PATCH RFC v2 00/16] drm/vkms: ConfigFS interface
Posted by Simona Vetter 11 months, 3 weeks ago
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.

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

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

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

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

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.

Cheers, Sima
-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH RFC v2 00/16] drm/vkms: ConfigFS interface
Posted by Louis Chauvet 11 months, 3 weeks ago

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

Re: [PATCH RFC v2 00/16] drm/vkms: ConfigFS interface
Posted by Simona Vetter 11 months, 3 weeks ago
On Wed, Feb 19, 2025 at 05:28:37PM +0100, Louis Chauvet wrote:
> 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.

Yeah, I think that's the way to go. Well maybe with a change to always
include the PATH property, because currently that's true for all
hotpluggable connectors. And we probably want to keep that if we extend it
to hotpluggable bridges or similar.

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

Agreeing with you, connector hotplug is something we need to tackle as an
extension of the basic configfs support, since it's quite complex.

> > - 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?

Not sure this was discussed in-depth, but when you get into more complex
output configurations, there's all kinds of funny hw constraints that pop
up. Examples:
- Multiple mst outputs on the same physical port share the overall
  bandwidth. So individually you might be able to light up each connector
  at max resolution, but if you try to light up all of them, there's a
  limitation. This is the mst specific case.
- There's also lots of display hw constraints, like a limited amount of
  clocks (fewer than crtc), or memory bw constraints for scanout, and
  similar things.

The idea is to express these constraints with ebpf programs, so that you
can validate that a compositor correctly handles these cases and doesn't
try an invalide configuration and then just fails instead of trying to
fall back to something that works.

So it's a much bigger issue, but multi-connector mst is a fairly important
case here.

Cheers, Sima

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

-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH RFC v2 00/16] drm/vkms: ConfigFS interface
Posted by Louis Chauvet 11 months, 2 weeks ago

Le 20/02/2025 à 10:43, Simona Vetter a écrit :
> On Wed, Feb 19, 2025 at 05:28:37PM +0100, Louis Chauvet wrote:
>> 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:
>>

[1]:https://gitlab.freedesktop.org/louischauvet/kernel/-/tree/vkms-mst[1]: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 fixed it, so now [1] contains a working mst emulation with dp-aux 
emulation. It is far from perfect and probably have many flaws, but it 
works.

The last commit creates the following configuration:

host 0->0 hub1 1->0 display1
                2->0 hub2 1->0 display2
                          2-> Not Connected
                3-> NC
                4-> NC

>>
>>>>>> 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.
> 
> Yeah, I think that's the way to go. Well maybe with a change to always
> include the PATH property, because currently that's true for all
> hotpluggable connectors. And we probably want to keep that if we extend it
> to hotpluggable bridges or similar.
> 
>> [1]: Few discussions here
>> https://lore.kernel.org/all/Z5zJ1rEZyBEgd7DN@louis-chauvet-laptop/
> 
> Agreeing with you, connector hotplug is something we need to tackle as an
> extension of the basic configfs support, since it's quite complex.
> 
>>> - 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?
> 
> Not sure this was discussed in-depth, but when you get into more complex
> output configurations, there's all kinds of funny hw constraints that pop
> up. Examples:
> - Multiple mst outputs on the same physical port share the overall
>    bandwidth. So individually you might be able to light up each connector
>    at max resolution, but if you try to light up all of them, there's a
>    limitation. This is the mst specific case.
> - There's also lots of display hw constraints, like a limited amount of
>    clocks (fewer than crtc), or memory bw constraints for scanout, and
>    similar things.
> 
> The idea is to express these constraints with ebpf programs, so that you
> can validate that a compositor correctly handles these cases and doesn't
> try an invalide configuration and then just fails instead of trying to
> fall back to something that works.
> 
> So it's a much bigger issue, but multi-connector mst is a fairly important
> case here.

I think I understood. I don't think I will have the time to work on this 
issue, but it is clearly a good idea. I submitted [2] to add it on the 
TODO list.

[2]:https://lore.kernel.org/all/20250225-vkms-update-todo-v1-1-afb1e6f7d714@bootlin.com/ 
(not yet on lore)

Thanks,
Louis Chauvet


> Cheers, Sima
> 
>>> 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
>>
> 

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