[PATCH 0/2] vkms: Add support for multiple connectors

[PATCH 0/2] vkms: Add support for multiple connectors

[Marius Vlad]

With multiple outputs available we can perform management of outputs at
a more granular level, such that we're able to turn off or on, several
outputs at a time, or combinations that arise from doing that. 

The Weston project use VKMS when running its test suite in CI, and we
have now uses cases which would need to ability to set-up the outputs
DPMS/state individually, rather than globally -- which would affect all
outputs. This an attempt on fixing that by giving the possibility to
create more than one output, and thus allowing to run tests that could
exercise code paths in the compositor related to management of outputs.

Marius Vlad (2):
  vkms: Pass the correct bitmask for possible crtcs
  vkms: Add support for multiple connectors

 drivers/gpu/drm/vkms/vkms_crtc.c      |  3 +--
 drivers/gpu/drm/vkms/vkms_drv.c       | 26 ++++++++++++++++++++++----
 drivers/gpu/drm/vkms/vkms_drv.h       |  8 +++++---
 drivers/gpu/drm/vkms/vkms_output.c    |  7 +++----
 drivers/gpu/drm/vkms/vkms_writeback.c | 18 ++++++++----------
 5 files changed, 39 insertions(+), 23 deletions(-)

-- 
2.35.1

[PATCH 1/2] vkms: Pass the correct bitmask for possible crtcs

[Marius Vlad]

In preparation of having multiple outputs/virtual connectors we need to
able to chose the correct encoder/connectors/crtc combination so pass also
the index as a bitmask as possible crtcs for the encoder.

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
---
 drivers/gpu/drm/vkms/vkms_output.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index 991857125bb4..ebb75ede65ab 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -96,7 +96,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
 		DRM_ERROR("Failed to init encoder\n");
 		goto err_encoder;
 	}
-	encoder->possible_crtcs = 1;
+	encoder->possible_crtcs = (1 << index);
 
 	ret = drm_connector_attach_encoder(connector, encoder);
 	if (ret) {
-- 
2.35.1

[PATCH 2/2] vkms: Add support for multiple connectors

[Marius Vlad]

This patch adds support for creating multiple virtual connectors, in
case one would need it. Use module parameters to specify how many,
defaulting to just one, allocating from the start, a maximum of 4
possible outputs.

This is of particular importance when testing out the DRM backend in
compositors, but also to be able to independently handle multiple
outputs/connectors, like setting one to off/sleep on while the others
are on, and combinations that arise from that.

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
---
 drivers/gpu/drm/vkms/vkms_crtc.c      |  3 +--
 drivers/gpu/drm/vkms/vkms_drv.c       | 26 ++++++++++++++++++++++----
 drivers/gpu/drm/vkms/vkms_drv.h       |  8 +++++---
 drivers/gpu/drm/vkms/vkms_output.c    |  5 ++---
 drivers/gpu/drm/vkms/vkms_writeback.c | 18 ++++++++----------
 5 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 57bbd32e9beb..0b6c40ac80b6 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -89,8 +89,7 @@ static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
 {
 	struct drm_device *dev = crtc->dev;
 	unsigned int pipe = crtc->index;
-	struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
-	struct vkms_output *output = &vkmsdev->output;
+	struct vkms_output *output = drm_crtc_to_vkms_output(crtc);
 	struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
 
 	if (!READ_ONCE(vblank->enabled)) {
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 0ffe5f0e33f7..205546dafc70 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -51,13 +51,19 @@ static bool enable_overlay;
 module_param_named(enable_overlay, enable_overlay, bool, 0444);
 MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
 
+static int max_connectors = 1;
+module_param_named(max_connectors, max_connectors, int, 0444);
+MODULE_PARM_DESC(max_connectors, "Specify how many virtual connectors to create");
+
 DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
 
 static void vkms_release(struct drm_device *dev)
 {
-	struct vkms_device *vkms = drm_device_to_vkms_device(dev);
+	int i;
+	struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
 
-	destroy_workqueue(vkms->output.composer_workq);
+	for (i = 0; i < vkmsdev->config->max_connectors; i++)
+		destroy_workqueue(vkmsdev->output[i].composer_workq);
 }
 
 static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
@@ -98,6 +104,7 @@ static int vkms_config_show(struct seq_file *m, void *data)
 	seq_printf(m, "writeback=%d\n", vkmsdev->config->writeback);
 	seq_printf(m, "cursor=%d\n", vkmsdev->config->cursor);
 	seq_printf(m, "overlay=%d\n", vkmsdev->config->overlay);
+	seq_printf(m, "connectors=%d\n", vkmsdev->config->max_connectors);
 
 	return 0;
 }
@@ -140,6 +147,7 @@ static const struct drm_mode_config_helper_funcs vkms_mode_config_helpers = {
 static int vkms_modeset_init(struct vkms_device *vkmsdev)
 {
 	struct drm_device *dev = &vkmsdev->drm;
+	int i, ret = 0;
 
 	drm_mode_config_init(dev);
 	dev->mode_config.funcs = &vkms_mode_funcs;
@@ -155,7 +163,14 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev)
 	dev->mode_config.preferred_depth = 0;
 	dev->mode_config.helper_private = &vkms_mode_config_helpers;
 
-	return vkms_output_init(vkmsdev, 0);
+	for (i = 0; i < vkmsdev->config->max_connectors; i++) {
+		ret = vkms_output_init(vkmsdev, i);
+		if (ret)
+			return ret;
+	}
+
+	drm_mode_config_reset(dev);
+	return ret;
 }
 
 static int vkms_create(struct vkms_config *config)
@@ -191,7 +206,7 @@ static int vkms_create(struct vkms_config *config)
 		goto out_devres;
 	}
 
-	ret = drm_vblank_init(&vkms_device->drm, 1);
+	ret = drm_vblank_init(&vkms_device->drm, config->max_connectors);
 	if (ret) {
 		DRM_ERROR("Failed to vblank\n");
 		goto out_devres;
@@ -229,6 +244,9 @@ static int __init vkms_init(void)
 	config->cursor = enable_cursor;
 	config->writeback = enable_writeback;
 	config->overlay = enable_overlay;
+	config->max_connectors = max_connectors;
+	if (config->max_connectors > NUM_MAX_CONNECTORS)
+		config->max_connectors = NUM_MAX_CONNECTORS;
 
 	return vkms_create(config);
 }
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 0a67b8073f7e..fdea37db3b1d 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -21,7 +21,8 @@
 #define XRES_MAX  8192
 #define YRES_MAX  8192
 
-#define NUM_OVERLAY_PLANES 8
+#define NUM_OVERLAY_PLANES	8
+#define NUM_MAX_CONNECTORS	4
 
 struct vkms_frame_info {
 	struct drm_framebuffer *fb;
@@ -113,6 +114,7 @@ struct vkms_config {
 	bool writeback;
 	bool cursor;
 	bool overlay;
+	int max_connectors;
 	/* only set when instantiated */
 	struct vkms_device *dev;
 };
@@ -120,7 +122,7 @@ struct vkms_config {
 struct vkms_device {
 	struct drm_device drm;
 	struct platform_device *platform;
-	struct vkms_output output;
+	struct vkms_output output[NUM_MAX_CONNECTORS];
 	const struct vkms_config *config;
 };
 
@@ -157,6 +159,6 @@ void vkms_composer_worker(struct work_struct *work);
 void vkms_set_composer(struct vkms_output *out, bool enabled);
 
 /* Writeback */
-int vkms_enable_writeback_connector(struct vkms_device *vkmsdev);
+int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, int index);
 
 #endif /* _VKMS_DRV_H_ */
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index ebb75ede65ab..c7b11d0a9753 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -50,7 +50,7 @@ static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int index,
 
 int vkms_output_init(struct vkms_device *vkmsdev, int index)
 {
-	struct vkms_output *output = &vkmsdev->output;
+	struct vkms_output *output = &vkmsdev->output[index];
 	struct drm_device *dev = &vkmsdev->drm;
 	struct drm_connector *connector = &output->connector;
 	struct drm_encoder *encoder = &output->encoder;
@@ -105,12 +105,11 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
 	}
 
 	if (vkmsdev->config->writeback) {
-		writeback = vkms_enable_writeback_connector(vkmsdev);
+		writeback = vkms_enable_writeback_connector(vkmsdev, index);
 		if (writeback)
 			DRM_ERROR("Failed to init writeback connector\n");
 	}
 
-	drm_mode_config_reset(dev);
 
 	return 0;
 
diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
index 84a51cd281b9..002c374f634a 100644
--- a/drivers/gpu/drm/vkms/vkms_writeback.c
+++ b/drivers/gpu/drm/vkms/vkms_writeback.c
@@ -101,17 +101,15 @@ static void vkms_wb_cleanup_job(struct drm_writeback_connector *connector,
 				struct drm_writeback_job *job)
 {
 	struct vkms_writeback_job *vkmsjob = job->priv;
-	struct vkms_device *vkmsdev;
-
+	struct vkms_output *output = container_of(connector, struct vkms_output,
+						  wb_connector);
 	if (!job->fb)
 		return;
 
 	drm_gem_fb_vunmap(job->fb, vkmsjob->wb_frame_info.map);
 
 	drm_framebuffer_put(vkmsjob->wb_frame_info.fb);
-
-	vkmsdev = drm_device_to_vkms_device(job->fb->dev);
-	vkms_set_composer(&vkmsdev->output, false);
+	vkms_set_composer(output, false);
 	kfree(vkmsjob);
 }
 
@@ -120,8 +118,7 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
 {
 	struct drm_connector_state *connector_state = drm_atomic_get_new_connector_state(state,
 											 conn);
-	struct vkms_device *vkmsdev = drm_device_to_vkms_device(conn->dev);
-	struct vkms_output *output = &vkmsdev->output;
+	struct vkms_output *output = container_of(conn, struct vkms_output, connector);
 	struct drm_writeback_connector *wb_conn = &output->wb_connector;
 	struct drm_connector_state *conn_state = wb_conn->base.state;
 	struct vkms_crtc_state *crtc_state = output->composer_state;
@@ -135,7 +132,7 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
 	if (!conn_state)
 		return;
 
-	vkms_set_composer(&vkmsdev->output, true);
+	vkms_set_composer(output, true);
 
 	active_wb = conn_state->writeback_job->priv;
 	wb_frame_info = &active_wb->wb_frame_info;
@@ -147,6 +144,7 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
 	wb_frame_info->cpp = fb->format->cpp[0];
 	crtc_state->wb_pending = true;
 	spin_unlock_irq(&output->composer_lock);
+
 	drm_writeback_queue_job(wb_conn, connector_state);
 	active_wb->wb_write = get_line_to_frame_function(wb_format);
 	drm_rect_init(&wb_frame_info->src, 0, 0, crtc_width, crtc_height);
@@ -160,9 +158,9 @@ static const struct drm_connector_helper_funcs vkms_wb_conn_helper_funcs = {
 	.atomic_commit = vkms_wb_atomic_commit,
 };
 
-int vkms_enable_writeback_connector(struct vkms_device *vkmsdev)
+int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, int index)
 {
-	struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector;
+	struct drm_writeback_connector *wb = &vkmsdev->output[index].wb_connector;
 
 	drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs);
 
-- 
2.35.1

Re: [PATCH 2/2] vkms: Add support for multiple connectors

[Maíra Canal]

Hi Marius,

> This patch adds support for creating multiple virtual connectors, in
> case one would need it. Use module parameters to specify how many,
> defaulting to just one, allocating from the start, a maximum of 4
> possible outputs.

I got a bit confused by this description. The commit message says that you
are creating multiple connectors, but it seems like you are creating multiple
pipes. From what I could see, for each new connector created, you are also
creating a new CRTC and new planes.

Moreover, if your real intention is to create multiple pipes, I believe that
you don't really need to duplicate the planes as well.

I ran the VKMS CI [1] with your patches applied and, although most of the 
tests are now passing with multiple pipes, which is really nice, the test
kms_writeback crashes with the following error:

[ 1997.244347] [IGT] kms_writeback: starting subtest writeback-check-output
[ 1997.250673] BUG: kernel NULL pointer dereference, address: 000000000000016c
[ 1997.250691] #PF: supervisor read access in kernel mode
[ 1997.250693] #PF: error_code(0x0000) - not-present page
[ 1997.250694] PGD 800000001a877067 P4D 800000001a877067 PUD 1a872067 PMD 0
[ 1997.250697] Oops: 0000 [#1] PREEMPT SMP PTI
[ 1997.250699] CPU: 2 PID: 3223 Comm: kms_writeback Not tainted 6.3.0-rc4-g8c2c29ba129f-dirty #257
[ 1997.250701] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc37 04/01/2014
[ 1997.250703] RIP: 0010:drm_vblank_get+0xa/0xe0
[ 1997.250710] Code: a9 66 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 41 57 41 56 41 55 41 54 53 <8b> 87 6c 01 00 00 41 bc ea ff ff ff 85 c0 74 74 89 f5 48 89 fb 39
[ 1997.250712] RSP: 0018:ffffb84d407a3b08 EFLAGS: 00010202
[ 1997.250714] RAX: 0000000000000000 RBX: ffffa2eb02bf8b70 RCX: 0000000000000000
[ 1997.250718] RDX: ffffa2eb180d2420 RSI: 0000000000000000 RDI: 0000000000000000
[ 1997.250719] RBP: ffffa2eb02bf99e8 R08: 0000000000000036 R09: ffffa2eb01d620c0
[ 1997.250720] R10: ffffe82b84027e40 R11: ffffffffc0042520 R12: ffffa2eb01c01900
[ 1997.250721] R13: ffffa2eb02bf9b60 R14: 0000000000000001 R15: ffffa2ea1ecd6b80
[ 1997.250722] FS:  00007f7d44e89a80(0000) GS:ffffa2eb3bd00000(0000) knlGS:0000000000000000
[ 1997.250723] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1997.250725] CR2: 000000000000016c CR3: 000000001ec02000 CR4: 00000000000006e0
[ 1997.250728] Call Trace:
[ 1997.250735]  <TASK>
[ 1997.250736]  vkms_set_composer+0x18/0x60 [vkms]
[ 1997.250745]  vkms_wb_atomic_commit+0x93/0x150 [vkms]
[ 1997.250749]  drm_atomic_helper_commit_modeset_enables+0x1d9/0x250
[ 1997.250754]  vkms_atomic_commit_tail+0x33/0xb0 [vkms]
[ 1997.250758]  commit_tail+0x8d/0x170
[ 1997.250761]  drm_atomic_helper_commit+0x26b/0x280
[ 1997.250763]  drm_atomic_commit+0x9f/0xc0
[ 1997.250766]  ? __pfx___drm_printfn_info+0x10/0x10
[ 1997.250769]  drm_mode_atomic_ioctl+0x34c/0x480
[ 1997.250771]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
[ 1997.250773]  drm_ioctl_kernel+0xd7/0x150
[ 1997.250780]  drm_ioctl+0x31f/0x490
[ 1997.250781]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
[ 1997.250783]  __se_sys_ioctl+0x6d/0xb0
[ 1997.250789]  do_syscall_64+0x43/0x90
[ 1997.250795]  entry_SYSCALL_64_after_hwframe+0x72/0xdc

[1] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/tree/master/tests/vkms_ci

Best Regards,
- Maíra Canal

> 
> This is of particular importance when testing out the DRM backend in
> compositors, but also to be able to independently handle multiple
> outputs/connectors, like setting one to off/sleep on while the others
> are on, and combinations that arise from that.
> 
> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> ---
>  drivers/gpu/drm/vkms/vkms_crtc.c      |  3 +--
>  drivers/gpu/drm/vkms/vkms_drv.c       | 26 ++++++++++++++++++++++----
>  drivers/gpu/drm/vkms/vkms_drv.h       |  8 +++++---
>  drivers/gpu/drm/vkms/vkms_output.c    |  5 ++---
>  drivers/gpu/drm/vkms/vkms_writeback.c | 18 ++++++++----------
>  5 files changed, 38 insertions(+), 22 deletions(-)
> 

Re: [PATCH 2/2] vkms: Add support for multiple connectors

[Marius Vlad]

Hi Maira,

Thanks a lot for taking a look. Yeah, indeed, this creates a new 
connector, a CRTC and planes for it. Terminology wise, multiple pipes. 
The idea is to have multiple outputs, each with its own connector, as to 
be able to simulate (a few) more outputs. At least that's what I'm 
looking for.

I'll adjust the commits description to clarify that.

Regarding the planes, it seemed a bit easier to just create a new
tuple of planes for each output, as in to reuse vkms_output_init(). So I 
guess that you're suggestion would be to make use the existing planes.

Seems a bit more work, not that keen on changing that, but I do have 
some follow-up questions for my own curiosity in case I do this:

-  Don't I need an entire pipe (a primary plane, crtc, encoder, 
connector) to power up the end side of the sink (display)?
- Can the primary one be sufficient for multiple outputs?
- Can the overlay planes be shared or I need to
   discard the ones that are already in-use by other outputs?

I'll have a look at that writeback test as well see what's causing that
NULL pointer deref.


On 4/5/23 16:51, Maíra Canal wrote:
> Hi Marius,
> 
>> This patch adds support for creating multiple virtual connectors, in
>> case one would need it. Use module parameters to specify how many,
>> defaulting to just one, allocating from the start, a maximum of 4
>> possible outputs.
> 
> I got a bit confused by this description. The commit message says that you
> are creating multiple connectors, but it seems like you are creating multiple
> pipes. From what I could see, for each new connector created, you are also
> creating a new CRTC and new planes.
> 
> Moreover, if your real intention is to create multiple pipes, I believe that
> you don't really need to duplicate the planes as well.
> 
> I ran the VKMS CI [1] with your patches applied and, although most of the
> tests are now passing with multiple pipes, which is really nice, the test
> kms_writeback crashes with the following error:
> 
> [ 1997.244347] [IGT] kms_writeback: starting subtest writeback-check-output
> [ 1997.250673] BUG: kernel NULL pointer dereference, address: 000000000000016c
> [ 1997.250691] #PF: supervisor read access in kernel mode
> [ 1997.250693] #PF: error_code(0x0000) - not-present page
> [ 1997.250694] PGD 800000001a877067 P4D 800000001a877067 PUD 1a872067 PMD 0
> [ 1997.250697] Oops: 0000 [#1] PREEMPT SMP PTI
> [ 1997.250699] CPU: 2 PID: 3223 Comm: kms_writeback Not tainted 6.3.0-rc4-g8c2c29ba129f-dirty #257
> [ 1997.250701] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc37 04/01/2014
> [ 1997.250703] RIP: 0010:drm_vblank_get+0xa/0xe0
> [ 1997.250710] Code: a9 66 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 41 57 41 56 41 55 41 54 53 <8b> 87 6c 01 00 00 41 bc ea ff ff ff 85 c0 74 74 89 f5 48 89 fb 39
> [ 1997.250712] RSP: 0018:ffffb84d407a3b08 EFLAGS: 00010202
> [ 1997.250714] RAX: 0000000000000000 RBX: ffffa2eb02bf8b70 RCX: 0000000000000000
> [ 1997.250718] RDX: ffffa2eb180d2420 RSI: 0000000000000000 RDI: 0000000000000000
> [ 1997.250719] RBP: ffffa2eb02bf99e8 R08: 0000000000000036 R09: ffffa2eb01d620c0
> [ 1997.250720] R10: ffffe82b84027e40 R11: ffffffffc0042520 R12: ffffa2eb01c01900
> [ 1997.250721] R13: ffffa2eb02bf9b60 R14: 0000000000000001 R15: ffffa2ea1ecd6b80
> [ 1997.250722] FS:  00007f7d44e89a80(0000) GS:ffffa2eb3bd00000(0000) knlGS:0000000000000000
> [ 1997.250723] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1997.250725] CR2: 000000000000016c CR3: 000000001ec02000 CR4: 00000000000006e0
> [ 1997.250728] Call Trace:
> [ 1997.250735]  <TASK>
> [ 1997.250736]  vkms_set_composer+0x18/0x60 [vkms]
> [ 1997.250745]  vkms_wb_atomic_commit+0x93/0x150 [vkms]
> [ 1997.250749]  drm_atomic_helper_commit_modeset_enables+0x1d9/0x250
> [ 1997.250754]  vkms_atomic_commit_tail+0x33/0xb0 [vkms]
> [ 1997.250758]  commit_tail+0x8d/0x170
> [ 1997.250761]  drm_atomic_helper_commit+0x26b/0x280
> [ 1997.250763]  drm_atomic_commit+0x9f/0xc0
> [ 1997.250766]  ? __pfx___drm_printfn_info+0x10/0x10
> [ 1997.250769]  drm_mode_atomic_ioctl+0x34c/0x480
> [ 1997.250771]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
> [ 1997.250773]  drm_ioctl_kernel+0xd7/0x150
> [ 1997.250780]  drm_ioctl+0x31f/0x490
> [ 1997.250781]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
> [ 1997.250783]  __se_sys_ioctl+0x6d/0xb0
> [ 1997.250789]  do_syscall_64+0x43/0x90
> [ 1997.250795]  entry_SYSCALL_64_after_hwframe+0x72/0xdc
> 
> [1] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/tree/master/tests/vkms_ci
> 
> Best Regards,
> - Maíra Canal
> 
>>
>> This is of particular importance when testing out the DRM backend in
>> compositors, but also to be able to independently handle multiple
>> outputs/connectors, like setting one to off/sleep on while the others
>> are on, and combinations that arise from that.
>>
>> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
>> ---
>>   drivers/gpu/drm/vkms/vkms_crtc.c      |  3 +--
>>   drivers/gpu/drm/vkms/vkms_drv.c       | 26 ++++++++++++++++++++++----
>>   drivers/gpu/drm/vkms/vkms_drv.h       |  8 +++++---
>>   drivers/gpu/drm/vkms/vkms_output.c    |  5 ++---
>>   drivers/gpu/drm/vkms/vkms_writeback.c | 18 ++++++++----------
>>   5 files changed, 38 insertions(+), 22 deletions(-)
>>

Re: [PATCH 2/2] vkms: Add support for multiple connectors

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 4725 bytes --]

Hi

Am 05.04.23 um 15:51 schrieb Maíra Canal:
> Hi Marius,
> 
>> This patch adds support for creating multiple virtual connectors, in
>> case one would need it. Use module parameters to specify how many,
>> defaulting to just one, allocating from the start, a maximum of 4
>> possible outputs.
> 
> I got a bit confused by this description. The commit message says that you
> are creating multiple connectors, but it seems like you are creating multiple
> pipes. From what I could see, for each new connector created, you are also
> creating a new CRTC and new planes.
> 
> Moreover, if your real intention is to create multiple pipes, I believe that
> you don't really need to duplicate the planes as well.

Terminology can be fuzzy, but a pipe is typically considered everything 
from plane to connector. Depending on the hardware, some components can 
be part of multiple pipes or change pipes, though.

Best regards
Thomas

> 
> I ran the VKMS CI [1] with your patches applied and, although most of the
> tests are now passing with multiple pipes, which is really nice, the test
> kms_writeback crashes with the following error:
> 
> [ 1997.244347] [IGT] kms_writeback: starting subtest writeback-check-output
> [ 1997.250673] BUG: kernel NULL pointer dereference, address: 000000000000016c
> [ 1997.250691] #PF: supervisor read access in kernel mode
> [ 1997.250693] #PF: error_code(0x0000) - not-present page
> [ 1997.250694] PGD 800000001a877067 P4D 800000001a877067 PUD 1a872067 PMD 0
> [ 1997.250697] Oops: 0000 [#1] PREEMPT SMP PTI
> [ 1997.250699] CPU: 2 PID: 3223 Comm: kms_writeback Not tainted 6.3.0-rc4-g8c2c29ba129f-dirty #257
> [ 1997.250701] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc37 04/01/2014
> [ 1997.250703] RIP: 0010:drm_vblank_get+0xa/0xe0
> [ 1997.250710] Code: a9 66 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 41 57 41 56 41 55 41 54 53 <8b> 87 6c 01 00 00 41 bc ea ff ff ff 85 c0 74 74 89 f5 48 89 fb 39
> [ 1997.250712] RSP: 0018:ffffb84d407a3b08 EFLAGS: 00010202
> [ 1997.250714] RAX: 0000000000000000 RBX: ffffa2eb02bf8b70 RCX: 0000000000000000
> [ 1997.250718] RDX: ffffa2eb180d2420 RSI: 0000000000000000 RDI: 0000000000000000
> [ 1997.250719] RBP: ffffa2eb02bf99e8 R08: 0000000000000036 R09: ffffa2eb01d620c0
> [ 1997.250720] R10: ffffe82b84027e40 R11: ffffffffc0042520 R12: ffffa2eb01c01900
> [ 1997.250721] R13: ffffa2eb02bf9b60 R14: 0000000000000001 R15: ffffa2ea1ecd6b80
> [ 1997.250722] FS:  00007f7d44e89a80(0000) GS:ffffa2eb3bd00000(0000) knlGS:0000000000000000
> [ 1997.250723] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1997.250725] CR2: 000000000000016c CR3: 000000001ec02000 CR4: 00000000000006e0
> [ 1997.250728] Call Trace:
> [ 1997.250735]  <TASK>
> [ 1997.250736]  vkms_set_composer+0x18/0x60 [vkms]
> [ 1997.250745]  vkms_wb_atomic_commit+0x93/0x150 [vkms]
> [ 1997.250749]  drm_atomic_helper_commit_modeset_enables+0x1d9/0x250
> [ 1997.250754]  vkms_atomic_commit_tail+0x33/0xb0 [vkms]
> [ 1997.250758]  commit_tail+0x8d/0x170
> [ 1997.250761]  drm_atomic_helper_commit+0x26b/0x280
> [ 1997.250763]  drm_atomic_commit+0x9f/0xc0
> [ 1997.250766]  ? __pfx___drm_printfn_info+0x10/0x10
> [ 1997.250769]  drm_mode_atomic_ioctl+0x34c/0x480
> [ 1997.250771]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
> [ 1997.250773]  drm_ioctl_kernel+0xd7/0x150
> [ 1997.250780]  drm_ioctl+0x31f/0x490
> [ 1997.250781]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
> [ 1997.250783]  __se_sys_ioctl+0x6d/0xb0
> [ 1997.250789]  do_syscall_64+0x43/0x90
> [ 1997.250795]  entry_SYSCALL_64_after_hwframe+0x72/0xdc
> 
> [1] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/tree/master/tests/vkms_ci
> 
> Best Regards,
> - Maíra Canal
> 
>>
>> This is of particular importance when testing out the DRM backend in
>> compositors, but also to be able to independently handle multiple
>> outputs/connectors, like setting one to off/sleep on while the others
>> are on, and combinations that arise from that.
>>
>> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
>> ---
>>   drivers/gpu/drm/vkms/vkms_crtc.c      |  3 +--
>>   drivers/gpu/drm/vkms/vkms_drv.c       | 26 ++++++++++++++++++++++----
>>   drivers/gpu/drm/vkms/vkms_drv.h       |  8 +++++---
>>   drivers/gpu/drm/vkms/vkms_output.c    |  5 ++---
>>   drivers/gpu/drm/vkms/vkms_writeback.c | 18 ++++++++----------
>>   5 files changed, 38 insertions(+), 22 deletions(-)
>>

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 2/2] vkms: Add support for multiple connectors

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 7121 bytes --]

Hi

Am 06.04.23 um 11:17 schrieb Marius Vlad:
> Hi Maira,
> 
> Thanks a lot for taking a look. Yeah, indeed, this creates a new 
> connector, a CRTC and planes for it. Terminology wise, multiple pipes. 
> The idea is to have multiple outputs, each with its own connector, as to 
> be able to simulate (a few) more outputs. At least that's what I'm 
> looking for.
> 
> I'll adjust the commits description to clarify that.
> 
> Regarding the planes, it seemed a bit easier to just create a new
> tuple of planes for each output, as in to reuse vkms_output_init(). So I 
> guess that you're suggestion would be to make use the existing planes.
> 
> Seems a bit more work, not that keen on changing that, but I do have 
> some follow-up questions for my own curiosity in case I do this:
> 
> -  Don't I need an entire pipe (a primary plane, crtc, encoder, 
> connector) to power up the end side of the sink (display)?

Yes, you need at least one full pipeline. I don't see how you'd get 
something displayed otherwise.

> - Can the primary one be sufficient for multiple outputs?

We have no concept of "primary pipelines." The individual components 
have index numbers. There's a primary plane attached to each CRTC, but 
even that concept comes more from HW limitations and historical designs, 
than fundamental requirements.

For multiple outputs, you can attach multiple encoders/connectors to the 
same CRTC. They will then all display the same screen at the same 
display resolution.

> - Can the overlay planes be shared or I need to
>    discard the ones that are already in-use by other outputs?

Even if your hardware planes support it, you cannot share planes among 
CRTCs with DRM. At least I'm not aware how to. Each plane struct has a 
designated CRTC struct. For most flexibility I'd have to match HW planes 
and DRM planes dynamically. For example if you have 2 CRTCs that can 
share 10 HW planes, you can allocate 10 DRM planes for each CRTC. The 
atomic_check functions have to implement the mapping from hardware to 
software plane and fail if no more hardware planes are available.

If you want to display the same screen on multiple CRTCs, it's possible 
to share the DRM framebuffers among similar the planes.

Best regards
Thomas

> 
> I'll have a look at that writeback test as well see what's causing that
> NULL pointer deref.
> 
> 
> On 4/5/23 16:51, Maíra Canal wrote:
>> Hi Marius,
>>
>>> This patch adds support for creating multiple virtual connectors, in
>>> case one would need it. Use module parameters to specify how many,
>>> defaulting to just one, allocating from the start, a maximum of 4
>>> possible outputs.
>>
>> I got a bit confused by this description. The commit message says that 
>> you
>> are creating multiple connectors, but it seems like you are creating 
>> multiple
>> pipes. From what I could see, for each new connector created, you are 
>> also
>> creating a new CRTC and new planes.
>>
>> Moreover, if your real intention is to create multiple pipes, I 
>> believe that
>> you don't really need to duplicate the planes as well.
>>
>> I ran the VKMS CI [1] with your patches applied and, although most of the
>> tests are now passing with multiple pipes, which is really nice, the test
>> kms_writeback crashes with the following error:
>>
>> [ 1997.244347] [IGT] kms_writeback: starting subtest 
>> writeback-check-output
>> [ 1997.250673] BUG: kernel NULL pointer dereference, address: 
>> 000000000000016c
>> [ 1997.250691] #PF: supervisor read access in kernel mode
>> [ 1997.250693] #PF: error_code(0x0000) - not-present page
>> [ 1997.250694] PGD 800000001a877067 P4D 800000001a877067 PUD 1a872067 
>> PMD 0
>> [ 1997.250697] Oops: 0000 [#1] PREEMPT SMP PTI
>> [ 1997.250699] CPU: 2 PID: 3223 Comm: kms_writeback Not tainted 
>> 6.3.0-rc4-g8c2c29ba129f-dirty #257
>> [ 1997.250701] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
>> BIOS 1.16.2-1.fc37 04/01/2014
>> [ 1997.250703] RIP: 0010:drm_vblank_get+0xa/0xe0
>> [ 1997.250710] Code: a9 66 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 
>> 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 41 57 41 56 41 55 
>> 41 54 53 <8b> 87 6c 01 00 00 41 bc ea ff ff ff 85 c0 74 74 89 f5 48 89 
>> fb 39
>> [ 1997.250712] RSP: 0018:ffffb84d407a3b08 EFLAGS: 00010202
>> [ 1997.250714] RAX: 0000000000000000 RBX: ffffa2eb02bf8b70 RCX: 
>> 0000000000000000
>> [ 1997.250718] RDX: ffffa2eb180d2420 RSI: 0000000000000000 RDI: 
>> 0000000000000000
>> [ 1997.250719] RBP: ffffa2eb02bf99e8 R08: 0000000000000036 R09: 
>> ffffa2eb01d620c0
>> [ 1997.250720] R10: ffffe82b84027e40 R11: ffffffffc0042520 R12: 
>> ffffa2eb01c01900
>> [ 1997.250721] R13: ffffa2eb02bf9b60 R14: 0000000000000001 R15: 
>> ffffa2ea1ecd6b80
>> [ 1997.250722] FS:  00007f7d44e89a80(0000) GS:ffffa2eb3bd00000(0000) 
>> knlGS:0000000000000000
>> [ 1997.250723] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [ 1997.250725] CR2: 000000000000016c CR3: 000000001ec02000 CR4: 
>> 00000000000006e0
>> [ 1997.250728] Call Trace:
>> [ 1997.250735]  <TASK>
>> [ 1997.250736]  vkms_set_composer+0x18/0x60 [vkms]
>> [ 1997.250745]  vkms_wb_atomic_commit+0x93/0x150 [vkms]
>> [ 1997.250749]  drm_atomic_helper_commit_modeset_enables+0x1d9/0x250
>> [ 1997.250754]  vkms_atomic_commit_tail+0x33/0xb0 [vkms]
>> [ 1997.250758]  commit_tail+0x8d/0x170
>> [ 1997.250761]  drm_atomic_helper_commit+0x26b/0x280
>> [ 1997.250763]  drm_atomic_commit+0x9f/0xc0
>> [ 1997.250766]  ? __pfx___drm_printfn_info+0x10/0x10
>> [ 1997.250769]  drm_mode_atomic_ioctl+0x34c/0x480
>> [ 1997.250771]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
>> [ 1997.250773]  drm_ioctl_kernel+0xd7/0x150
>> [ 1997.250780]  drm_ioctl+0x31f/0x490
>> [ 1997.250781]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
>> [ 1997.250783]  __se_sys_ioctl+0x6d/0xb0
>> [ 1997.250789]  do_syscall_64+0x43/0x90
>> [ 1997.250795]  entry_SYSCALL_64_after_hwframe+0x72/0xdc
>>
>> [1] 
>> https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/tree/master/tests/vkms_ci
>>
>> Best Regards,
>> - Maíra Canal
>>
>>>
>>> This is of particular importance when testing out the DRM backend in
>>> compositors, but also to be able to independently handle multiple
>>> outputs/connectors, like setting one to off/sleep on while the others
>>> are on, and combinations that arise from that.
>>>
>>> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
>>> ---
>>>   drivers/gpu/drm/vkms/vkms_crtc.c      |  3 +--
>>>   drivers/gpu/drm/vkms/vkms_drv.c       | 26 ++++++++++++++++++++++----
>>>   drivers/gpu/drm/vkms/vkms_drv.h       |  8 +++++---
>>>   drivers/gpu/drm/vkms/vkms_output.c    |  5 ++---
>>>   drivers/gpu/drm/vkms/vkms_writeback.c | 18 ++++++++----------
>>>   5 files changed, 38 insertions(+), 22 deletions(-)
>>>

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 2/2] vkms: Add support for multiple connectors

[Marius Vlad]

Hi Thomas,

Thanks for the clarifications! Made a couple of remarks in-line.

On 4/6/23 14:29, Thomas Zimmermann wrote:
> Hi
> 
> Am 06.04.23 um 11:17 schrieb Marius Vlad:
>> Hi Maira,
>>
>> Thanks a lot for taking a look. Yeah, indeed, this creates a new 
>> connector, a CRTC and planes for it. Terminology wise, multiple pipes. 
>> The idea is to have multiple outputs, each with its own connector, as 
>> to be able to simulate (a few) more outputs. At least that's what I'm 
>> looking for.
>>
>> I'll adjust the commits description to clarify that.
>>
>> Regarding the planes, it seemed a bit easier to just create a new
>> tuple of planes for each output, as in to reuse vkms_output_init(). So 
>> I guess that you're suggestion would be to make use the existing planes.
>>
>> Seems a bit more work, not that keen on changing that, but I do have 
>> some follow-up questions for my own curiosity in case I do this:
>>
>> -  Don't I need an entire pipe (a primary plane, crtc, encoder, 
>> connector) to power up the end side of the sink (display)?
> 
> Yes, you need at least one full pipeline. I don't see how you'd get 
> something displayed otherwise.
> 
>> - Can the primary one be sufficient for multiple outputs?
> 
> We have no concept of "primary pipelines." The individual components 
> have index numbers. There's a primary plane attached to each CRTC, but 
> even that concept comes more from HW limitations and historical designs, 
> than fundamental requirements.

Right, meant the primary plane, rather than pipeline.

> 
> For multiple outputs, you can attach multiple encoders/connectors to the 
> same CRTC. They will then all display the same screen at the same 
> display resolution
Yeah, this kind of sounds like cloning to me, and would like to display 
different things at the same time, on different outputs, to me it sounds 
I need primary plane and a CRTC for each connector. Right?

> 
>> - Can the overlay planes be shared or I need to
>>    discard the ones that are already in-use by other outputs?
> 
> Even if your hardware planes support it, you cannot share planes among 
> CRTCs with DRM. At least I'm not aware how to. Each plane struct has a 
> designated CRTC struct. For most flexibility I'd have to match HW planes 
> and DRM planes dynamically. For example if you have 2 CRTCs that can 
> share 10 HW planes, you can allocate 10 DRM planes for each CRTC. The 
> atomic_check functions have to implement the mapping from hardware to 
> software plane and fail if no more hardware planes are available.
> 
> If you want to display the same screen on multiple CRTCs, it's possible 
> to share the DRM framebuffers among similar the planes.

Aham, understood, thanks again!

> 
> Best regards
> Thomas
> 
>>
>> I'll have a look at that writeback test as well see what's causing that
>> NULL pointer deref.
>>
>>
>> On 4/5/23 16:51, Maíra Canal wrote:
>>> Hi Marius,
>>>
>>>> This patch adds support for creating multiple virtual connectors, in
>>>> case one would need it. Use module parameters to specify how many,
>>>> defaulting to just one, allocating from the start, a maximum of 4
>>>> possible outputs.
>>>
>>> I got a bit confused by this description. The commit message says 
>>> that you
>>> are creating multiple connectors, but it seems like you are creating 
>>> multiple
>>> pipes. From what I could see, for each new connector created, you are 
>>> also
>>> creating a new CRTC and new planes.
>>>
>>> Moreover, if your real intention is to create multiple pipes, I 
>>> believe that
>>> you don't really need to duplicate the planes as well.
>>>
>>> I ran the VKMS CI [1] with your patches applied and, although most of 
>>> the
>>> tests are now passing with multiple pipes, which is really nice, the 
>>> test
>>> kms_writeback crashes with the following error:
>>>
>>> [ 1997.244347] [IGT] kms_writeback: starting subtest 
>>> writeback-check-output
>>> [ 1997.250673] BUG: kernel NULL pointer dereference, address: 
>>> 000000000000016c
>>> [ 1997.250691] #PF: supervisor read access in kernel mode
>>> [ 1997.250693] #PF: error_code(0x0000) - not-present page
>>> [ 1997.250694] PGD 800000001a877067 P4D 800000001a877067 PUD 1a872067 
>>> PMD 0
>>> [ 1997.250697] Oops: 0000 [#1] PREEMPT SMP PTI
>>> [ 1997.250699] CPU: 2 PID: 3223 Comm: kms_writeback Not tainted 
>>> 6.3.0-rc4-g8c2c29ba129f-dirty #257
>>> [ 1997.250701] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
>>> BIOS 1.16.2-1.fc37 04/01/2014
>>> [ 1997.250703] RIP: 0010:drm_vblank_get+0xa/0xe0
>>> [ 1997.250710] Code: a9 66 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 
>>> 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 41 57 41 56 41 55 
>>> 41 54 53 <8b> 87 6c 01 00 00 41 bc ea ff ff ff 85 c0 74 74 89 f5 48 
>>> 89 fb 39
>>> [ 1997.250712] RSP: 0018:ffffb84d407a3b08 EFLAGS: 00010202
>>> [ 1997.250714] RAX: 0000000000000000 RBX: ffffa2eb02bf8b70 RCX: 
>>> 0000000000000000
>>> [ 1997.250718] RDX: ffffa2eb180d2420 RSI: 0000000000000000 RDI: 
>>> 0000000000000000
>>> [ 1997.250719] RBP: ffffa2eb02bf99e8 R08: 0000000000000036 R09: 
>>> ffffa2eb01d620c0
>>> [ 1997.250720] R10: ffffe82b84027e40 R11: ffffffffc0042520 R12: 
>>> ffffa2eb01c01900
>>> [ 1997.250721] R13: ffffa2eb02bf9b60 R14: 0000000000000001 R15: 
>>> ffffa2ea1ecd6b80
>>> [ 1997.250722] FS:  00007f7d44e89a80(0000) GS:ffffa2eb3bd00000(0000) 
>>> knlGS:0000000000000000
>>> [ 1997.250723] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [ 1997.250725] CR2: 000000000000016c CR3: 000000001ec02000 CR4: 
>>> 00000000000006e0
>>> [ 1997.250728] Call Trace:
>>> [ 1997.250735]  <TASK>
>>> [ 1997.250736]  vkms_set_composer+0x18/0x60 [vkms]
>>> [ 1997.250745]  vkms_wb_atomic_commit+0x93/0x150 [vkms]
>>> [ 1997.250749]  drm_atomic_helper_commit_modeset_enables+0x1d9/0x250
>>> [ 1997.250754]  vkms_atomic_commit_tail+0x33/0xb0 [vkms]
>>> [ 1997.250758]  commit_tail+0x8d/0x170
>>> [ 1997.250761]  drm_atomic_helper_commit+0x26b/0x280
>>> [ 1997.250763]  drm_atomic_commit+0x9f/0xc0
>>> [ 1997.250766]  ? __pfx___drm_printfn_info+0x10/0x10
>>> [ 1997.250769]  drm_mode_atomic_ioctl+0x34c/0x480
>>> [ 1997.250771]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
>>> [ 1997.250773]  drm_ioctl_kernel+0xd7/0x150
>>> [ 1997.250780]  drm_ioctl+0x31f/0x490
>>> [ 1997.250781]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
>>> [ 1997.250783]  __se_sys_ioctl+0x6d/0xb0
>>> [ 1997.250789]  do_syscall_64+0x43/0x90
>>> [ 1997.250795]  entry_SYSCALL_64_after_hwframe+0x72/0xdc
>>>
>>> [1] 
>>> https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/tree/master/tests/vkms_ci
>>>
>>> Best Regards,
>>> - Maíra Canal
>>>
>>>>
>>>> This is of particular importance when testing out the DRM backend in
>>>> compositors, but also to be able to independently handle multiple
>>>> outputs/connectors, like setting one to off/sleep on while the others
>>>> are on, and combinations that arise from that.
>>>>
>>>> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
>>>> ---
>>>>   drivers/gpu/drm/vkms/vkms_crtc.c      |  3 +--
>>>>   drivers/gpu/drm/vkms/vkms_drv.c       | 26 ++++++++++++++++++++++----
>>>>   drivers/gpu/drm/vkms/vkms_drv.h       |  8 +++++---
>>>>   drivers/gpu/drm/vkms/vkms_output.c    |  5 ++---
>>>>   drivers/gpu/drm/vkms/vkms_writeback.c | 18 ++++++++----------
>>>>   5 files changed, 38 insertions(+), 22 deletions(-)
>>>>
> 

Re: [PATCH 2/2] vkms: Add support for multiple connectors

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 6415 bytes --]

Hi

Am 06.04.23 um 16:04 schrieb Marius Vlad:
[...]
> 
>>
>> For multiple outputs, you can attach multiple encoders/connectors to 
>> the same CRTC. They will then all display the same screen at the same 
>> display resolution
> Yeah, this kind of sounds like cloning to me, and would like to display 
> different things at the same time, on different outputs, to me it sounds 
> I need primary plane and a CRTC for each connector. Right?

Exactly.

Best regards
Thomas

> 
>>
>>> - Can the overlay planes be shared or I need to
>>>    discard the ones that are already in-use by other outputs?
>>
>> Even if your hardware planes support it, you cannot share planes among 
>> CRTCs with DRM. At least I'm not aware how to. Each plane struct has a 
>> designated CRTC struct. For most flexibility I'd have to match HW 
>> planes and DRM planes dynamically. For example if you have 2 CRTCs 
>> that can share 10 HW planes, you can allocate 10 DRM planes for each 
>> CRTC. The atomic_check functions have to implement the mapping from 
>> hardware to software plane and fail if no more hardware planes are 
>> available.
>>
>> If you want to display the same screen on multiple CRTCs, it's 
>> possible to share the DRM framebuffers among similar the planes.
> 
> Aham, understood, thanks again!
> 
>>
>> Best regards
>> Thomas
>>
>>>
>>> I'll have a look at that writeback test as well see what's causing that
>>> NULL pointer deref.
>>>
>>>
>>> On 4/5/23 16:51, Maíra Canal wrote:
>>>> Hi Marius,
>>>>
>>>>> This patch adds support for creating multiple virtual connectors, in
>>>>> case one would need it. Use module parameters to specify how many,
>>>>> defaulting to just one, allocating from the start, a maximum of 4
>>>>> possible outputs.
>>>>
>>>> I got a bit confused by this description. The commit message says 
>>>> that you
>>>> are creating multiple connectors, but it seems like you are creating 
>>>> multiple
>>>> pipes. From what I could see, for each new connector created, you 
>>>> are also
>>>> creating a new CRTC and new planes.
>>>>
>>>> Moreover, if your real intention is to create multiple pipes, I 
>>>> believe that
>>>> you don't really need to duplicate the planes as well.
>>>>
>>>> I ran the VKMS CI [1] with your patches applied and, although most 
>>>> of the
>>>> tests are now passing with multiple pipes, which is really nice, the 
>>>> test
>>>> kms_writeback crashes with the following error:
>>>>
>>>> [ 1997.244347] [IGT] kms_writeback: starting subtest 
>>>> writeback-check-output
>>>> [ 1997.250673] BUG: kernel NULL pointer dereference, address: 
>>>> 000000000000016c
>>>> [ 1997.250691] #PF: supervisor read access in kernel mode
>>>> [ 1997.250693] #PF: error_code(0x0000) - not-present page
>>>> [ 1997.250694] PGD 800000001a877067 P4D 800000001a877067 PUD 
>>>> 1a872067 PMD 0
>>>> [ 1997.250697] Oops: 0000 [#1] PREEMPT SMP PTI
>>>> [ 1997.250699] CPU: 2 PID: 3223 Comm: kms_writeback Not tainted 
>>>> 6.3.0-rc4-g8c2c29ba129f-dirty #257
>>>> [ 1997.250701] Hardware name: QEMU Standard PC (i440FX + PIIX, 
>>>> 1996), BIOS 1.16.2-1.fc37 04/01/2014
>>>> [ 1997.250703] RIP: 0010:drm_vblank_get+0xa/0xe0
>>>> [ 1997.250710] Code: a9 66 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 
>>>> 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 41 57 41 56 41 55 
>>>> 41 54 53 <8b> 87 6c 01 00 00 41 bc ea ff ff ff 85 c0 74 74 89 f5 48 
>>>> 89 fb 39
>>>> [ 1997.250712] RSP: 0018:ffffb84d407a3b08 EFLAGS: 00010202
>>>> [ 1997.250714] RAX: 0000000000000000 RBX: ffffa2eb02bf8b70 RCX: 
>>>> 0000000000000000
>>>> [ 1997.250718] RDX: ffffa2eb180d2420 RSI: 0000000000000000 RDI: 
>>>> 0000000000000000
>>>> [ 1997.250719] RBP: ffffa2eb02bf99e8 R08: 0000000000000036 R09: 
>>>> ffffa2eb01d620c0
>>>> [ 1997.250720] R10: ffffe82b84027e40 R11: ffffffffc0042520 R12: 
>>>> ffffa2eb01c01900
>>>> [ 1997.250721] R13: ffffa2eb02bf9b60 R14: 0000000000000001 R15: 
>>>> ffffa2ea1ecd6b80
>>>> [ 1997.250722] FS:  00007f7d44e89a80(0000) GS:ffffa2eb3bd00000(0000) 
>>>> knlGS:0000000000000000
>>>> [ 1997.250723] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [ 1997.250725] CR2: 000000000000016c CR3: 000000001ec02000 CR4: 
>>>> 00000000000006e0
>>>> [ 1997.250728] Call Trace:
>>>> [ 1997.250735]  <TASK>
>>>> [ 1997.250736]  vkms_set_composer+0x18/0x60 [vkms]
>>>> [ 1997.250745]  vkms_wb_atomic_commit+0x93/0x150 [vkms]
>>>> [ 1997.250749]  drm_atomic_helper_commit_modeset_enables+0x1d9/0x250
>>>> [ 1997.250754]  vkms_atomic_commit_tail+0x33/0xb0 [vkms]
>>>> [ 1997.250758]  commit_tail+0x8d/0x170
>>>> [ 1997.250761]  drm_atomic_helper_commit+0x26b/0x280
>>>> [ 1997.250763]  drm_atomic_commit+0x9f/0xc0
>>>> [ 1997.250766]  ? __pfx___drm_printfn_info+0x10/0x10
>>>> [ 1997.250769]  drm_mode_atomic_ioctl+0x34c/0x480
>>>> [ 1997.250771]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
>>>> [ 1997.250773]  drm_ioctl_kernel+0xd7/0x150
>>>> [ 1997.250780]  drm_ioctl+0x31f/0x490
>>>> [ 1997.250781]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
>>>> [ 1997.250783]  __se_sys_ioctl+0x6d/0xb0
>>>> [ 1997.250789]  do_syscall_64+0x43/0x90
>>>> [ 1997.250795]  entry_SYSCALL_64_after_hwframe+0x72/0xdc
>>>>
>>>> [1] 
>>>> https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/tree/master/tests/vkms_ci
>>>>
>>>> Best Regards,
>>>> - Maíra Canal
>>>>
>>>>>
>>>>> This is of particular importance when testing out the DRM backend in
>>>>> compositors, but also to be able to independently handle multiple
>>>>> outputs/connectors, like setting one to off/sleep on while the others
>>>>> are on, and combinations that arise from that.
>>>>>
>>>>> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
>>>>> ---
>>>>>   drivers/gpu/drm/vkms/vkms_crtc.c      |  3 +--
>>>>>   drivers/gpu/drm/vkms/vkms_drv.c       | 26 
>>>>> ++++++++++++++++++++++----
>>>>>   drivers/gpu/drm/vkms/vkms_drv.h       |  8 +++++---
>>>>>   drivers/gpu/drm/vkms/vkms_output.c    |  5 ++---
>>>>>   drivers/gpu/drm/vkms/vkms_writeback.c | 18 ++++++++----------
>>>>>   5 files changed, 38 insertions(+), 22 deletions(-)
>>>>>
>>

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 2/2] vkms: Add support for multiple connectors

[Melissa Wen]

[-- Attachment #1: Type: text/plain, Size: 8526 bytes --]

On 04/06, Marius Vlad wrote:
> Hi Thomas,
> 
> Thanks for the clarifications! Made a couple of remarks in-line.
> 
> On 4/6/23 14:29, Thomas Zimmermann wrote:
> > Hi
> > 
> > Am 06.04.23 um 11:17 schrieb Marius Vlad:
> > > Hi Maira,
> > > 
> > > Thanks a lot for taking a look. Yeah, indeed, this creates a new
> > > connector, a CRTC and planes for it. Terminology wise, multiple
> > > pipes. The idea is to have multiple outputs, each with its own
> > > connector, as to be able to simulate (a few) more outputs. At least
> > > that's what I'm looking for.
> > > 
> > > I'll adjust the commits description to clarify that.
> > > 
> > > Regarding the planes, it seemed a bit easier to just create a new
> > > tuple of planes for each output, as in to reuse vkms_output_init().
> > > So I guess that you're suggestion would be to make use the existing
> > > planes.
> > > 
> > > Seems a bit more work, not that keen on changing that, but I do have
> > > some follow-up questions for my own curiosity in case I do this:
> > > 
> > > -  Don't I need an entire pipe (a primary plane, crtc, encoder,
> > > connector) to power up the end side of the sink (display)?
> > 
> > Yes, you need at least one full pipeline. I don't see how you'd get
> > something displayed otherwise.
> > 
> > > - Can the primary one be sufficient for multiple outputs?
> > 
> > We have no concept of "primary pipelines." The individual components
> > have index numbers. There's a primary plane attached to each CRTC, but
> > even that concept comes more from HW limitations and historical designs,
> > than fundamental requirements.
> 
> Right, meant the primary plane, rather than pipeline.
> 
> > 
> > For multiple outputs, you can attach multiple encoders/connectors to the
> > same CRTC. They will then all display the same screen at the same
> > display resolution
> Yeah, this kind of sounds like cloning to me, and would like to display
> different things at the same time, on different outputs, to me it sounds I
> need primary plane and a CRTC for each connector. Right?
> 
> > 
> > > - Can the overlay planes be shared or I need to
> > >    discard the ones that are already in-use by other outputs?
> > 
> > Even if your hardware planes support it, you cannot share planes among
> > CRTCs with DRM. At least I'm not aware how to. Each plane struct has a
> > designated CRTC struct. For most flexibility I'd have to match HW planes
> > and DRM planes dynamically. For example if you have 2 CRTCs that can
> > share 10 HW planes, you can allocate 10 DRM planes for each CRTC. The
> > atomic_check functions have to implement the mapping from hardware to
> > software plane and fail if no more hardware planes are available.
> > 
> > If you want to display the same screen on multiple CRTCs, it's possible
> > to share the DRM framebuffers among similar the planes.
> 
> Aham, understood, thanks again!

Some drivers allow overlay planes to move between CRTCs. We have briefly
discussed on IRC that it would be interesting to have this (even for all
plane types) in vkms for testing and validation, but in a next step. As
it's not included in this proposal here, I'd suggest you to include this
feature/improvement in the vkms TODO in your next version, to keep it in
our minds for future works.

BR,

Melissa

> 
> > 
> > Best regards
> > Thomas
> > 
> > > 
> > > I'll have a look at that writeback test as well see what's causing that
> > > NULL pointer deref.
> > > 
> > > 
> > > On 4/5/23 16:51, Maíra Canal wrote:
> > > > Hi Marius,
> > > > 
> > > > > This patch adds support for creating multiple virtual connectors, in
> > > > > case one would need it. Use module parameters to specify how many,
> > > > > defaulting to just one, allocating from the start, a maximum of 4
> > > > > possible outputs.
> > > > 
> > > > I got a bit confused by this description. The commit message
> > > > says that you
> > > > are creating multiple connectors, but it seems like you are
> > > > creating multiple
> > > > pipes. From what I could see, for each new connector created,
> > > > you are also
> > > > creating a new CRTC and new planes.
> > > > 
> > > > Moreover, if your real intention is to create multiple pipes, I
> > > > believe that
> > > > you don't really need to duplicate the planes as well.
> > > > 
> > > > I ran the VKMS CI [1] with your patches applied and, although
> > > > most of the
> > > > tests are now passing with multiple pipes, which is really nice,
> > > > the test
> > > > kms_writeback crashes with the following error:
> > > > 
> > > > [ 1997.244347] [IGT] kms_writeback: starting subtest
> > > > writeback-check-output
> > > > [ 1997.250673] BUG: kernel NULL pointer dereference, address:
> > > > 000000000000016c
> > > > [ 1997.250691] #PF: supervisor read access in kernel mode
> > > > [ 1997.250693] #PF: error_code(0x0000) - not-present page
> > > > [ 1997.250694] PGD 800000001a877067 P4D 800000001a877067 PUD
> > > > 1a872067 PMD 0
> > > > [ 1997.250697] Oops: 0000 [#1] PREEMPT SMP PTI
> > > > [ 1997.250699] CPU: 2 PID: 3223 Comm: kms_writeback Not tainted
> > > > 6.3.0-rc4-g8c2c29ba129f-dirty #257
> > > > [ 1997.250701] Hardware name: QEMU Standard PC (i440FX + PIIX,
> > > > 1996), BIOS 1.16.2-1.fc37 04/01/2014
> > > > [ 1997.250703] RIP: 0010:drm_vblank_get+0xa/0xe0
> > > > [ 1997.250710] Code: a9 66 66 66 66 66 66 2e 0f 1f 84 00 00 00
> > > > 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 41 57
> > > > 41 56 41 55 41 54 53 <8b> 87 6c 01 00 00 41 bc ea ff ff ff 85 c0
> > > > 74 74 89 f5 48 89 fb 39
> > > > [ 1997.250712] RSP: 0018:ffffb84d407a3b08 EFLAGS: 00010202
> > > > [ 1997.250714] RAX: 0000000000000000 RBX: ffffa2eb02bf8b70 RCX:
> > > > 0000000000000000
> > > > [ 1997.250718] RDX: ffffa2eb180d2420 RSI: 0000000000000000 RDI:
> > > > 0000000000000000
> > > > [ 1997.250719] RBP: ffffa2eb02bf99e8 R08: 0000000000000036 R09:
> > > > ffffa2eb01d620c0
> > > > [ 1997.250720] R10: ffffe82b84027e40 R11: ffffffffc0042520 R12:
> > > > ffffa2eb01c01900
> > > > [ 1997.250721] R13: ffffa2eb02bf9b60 R14: 0000000000000001 R15:
> > > > ffffa2ea1ecd6b80
> > > > [ 1997.250722] FS:  00007f7d44e89a80(0000)
> > > > GS:ffffa2eb3bd00000(0000) knlGS:0000000000000000
> > > > [ 1997.250723] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > [ 1997.250725] CR2: 000000000000016c CR3: 000000001ec02000 CR4:
> > > > 00000000000006e0
> > > > [ 1997.250728] Call Trace:
> > > > [ 1997.250735]  <TASK>
> > > > [ 1997.250736]  vkms_set_composer+0x18/0x60 [vkms]
> > > > [ 1997.250745]  vkms_wb_atomic_commit+0x93/0x150 [vkms]
> > > > [ 1997.250749]  drm_atomic_helper_commit_modeset_enables+0x1d9/0x250
> > > > [ 1997.250754]  vkms_atomic_commit_tail+0x33/0xb0 [vkms]
> > > > [ 1997.250758]  commit_tail+0x8d/0x170
> > > > [ 1997.250761]  drm_atomic_helper_commit+0x26b/0x280
> > > > [ 1997.250763]  drm_atomic_commit+0x9f/0xc0
> > > > [ 1997.250766]  ? __pfx___drm_printfn_info+0x10/0x10
> > > > [ 1997.250769]  drm_mode_atomic_ioctl+0x34c/0x480
> > > > [ 1997.250771]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
> > > > [ 1997.250773]  drm_ioctl_kernel+0xd7/0x150
> > > > [ 1997.250780]  drm_ioctl+0x31f/0x490
> > > > [ 1997.250781]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
> > > > [ 1997.250783]  __se_sys_ioctl+0x6d/0xb0
> > > > [ 1997.250789]  do_syscall_64+0x43/0x90
> > > > [ 1997.250795]  entry_SYSCALL_64_after_hwframe+0x72/0xdc
> > > > 
> > > > [1] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/tree/master/tests/vkms_ci
> > > > 
> > > > Best Regards,
> > > > - Maíra Canal
> > > > 
> > > > > 
> > > > > This is of particular importance when testing out the DRM backend in
> > > > > compositors, but also to be able to independently handle multiple
> > > > > outputs/connectors, like setting one to off/sleep on while the others
> > > > > are on, and combinations that arise from that.
> > > > > 
> > > > > Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> > > > > ---
> > > > >   drivers/gpu/drm/vkms/vkms_crtc.c      |  3 +--
> > > > >   drivers/gpu/drm/vkms/vkms_drv.c       | 26 ++++++++++++++++++++++----
> > > > >   drivers/gpu/drm/vkms/vkms_drv.h       |  8 +++++---
> > > > >   drivers/gpu/drm/vkms/vkms_output.c    |  5 ++---
> > > > >   drivers/gpu/drm/vkms/vkms_writeback.c | 18 ++++++++----------
> > > > >   5 files changed, 38 insertions(+), 22 deletions(-)
> > > > > 
> > 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] vkms: Add support for multiple connectors

[Marius Vlad]

[-- Attachment #1: Type: text/plain, Size: 9044 bytes --]

Hi Melissa,

Added a note about that in the latest version.

Thanks!

On Mon, Apr 10, 2023 at 05:17:56PM -0100, Melissa Wen wrote:
> On 04/06, Marius Vlad wrote:
> > Hi Thomas,
> > 
> > Thanks for the clarifications! Made a couple of remarks in-line.
> > 
> > On 4/6/23 14:29, Thomas Zimmermann wrote:
> > > Hi
> > > 
> > > Am 06.04.23 um 11:17 schrieb Marius Vlad:
> > > > Hi Maira,
> > > > 
> > > > Thanks a lot for taking a look. Yeah, indeed, this creates a new
> > > > connector, a CRTC and planes for it. Terminology wise, multiple
> > > > pipes. The idea is to have multiple outputs, each with its own
> > > > connector, as to be able to simulate (a few) more outputs. At least
> > > > that's what I'm looking for.
> > > > 
> > > > I'll adjust the commits description to clarify that.
> > > > 
> > > > Regarding the planes, it seemed a bit easier to just create a new
> > > > tuple of planes for each output, as in to reuse vkms_output_init().
> > > > So I guess that you're suggestion would be to make use the existing
> > > > planes.
> > > > 
> > > > Seems a bit more work, not that keen on changing that, but I do have
> > > > some follow-up questions for my own curiosity in case I do this:
> > > > 
> > > > -  Don't I need an entire pipe (a primary plane, crtc, encoder,
> > > > connector) to power up the end side of the sink (display)?
> > > 
> > > Yes, you need at least one full pipeline. I don't see how you'd get
> > > something displayed otherwise.
> > > 
> > > > - Can the primary one be sufficient for multiple outputs?
> > > 
> > > We have no concept of "primary pipelines." The individual components
> > > have index numbers. There's a primary plane attached to each CRTC, but
> > > even that concept comes more from HW limitations and historical designs,
> > > than fundamental requirements.
> > 
> > Right, meant the primary plane, rather than pipeline.
> > 
> > > 
> > > For multiple outputs, you can attach multiple encoders/connectors to the
> > > same CRTC. They will then all display the same screen at the same
> > > display resolution
> > Yeah, this kind of sounds like cloning to me, and would like to display
> > different things at the same time, on different outputs, to me it sounds I
> > need primary plane and a CRTC for each connector. Right?
> > 
> > > 
> > > > - Can the overlay planes be shared or I need to
> > > >    discard the ones that are already in-use by other outputs?
> > > 
> > > Even if your hardware planes support it, you cannot share planes among
> > > CRTCs with DRM. At least I'm not aware how to. Each plane struct has a
> > > designated CRTC struct. For most flexibility I'd have to match HW planes
> > > and DRM planes dynamically. For example if you have 2 CRTCs that can
> > > share 10 HW planes, you can allocate 10 DRM planes for each CRTC. The
> > > atomic_check functions have to implement the mapping from hardware to
> > > software plane and fail if no more hardware planes are available.
> > > 
> > > If you want to display the same screen on multiple CRTCs, it's possible
> > > to share the DRM framebuffers among similar the planes.
> > 
> > Aham, understood, thanks again!
> 
> Some drivers allow overlay planes to move between CRTCs. We have briefly
> discussed on IRC that it would be interesting to have this (even for all
> plane types) in vkms for testing and validation, but in a next step. As
> it's not included in this proposal here, I'd suggest you to include this
> feature/improvement in the vkms TODO in your next version, to keep it in
> our minds for future works.
> 
> BR,
> 
> Melissa
> 
> > 
> > > 
> > > Best regards
> > > Thomas
> > > 
> > > > 
> > > > I'll have a look at that writeback test as well see what's causing that
> > > > NULL pointer deref.
> > > > 
> > > > 
> > > > On 4/5/23 16:51, Maíra Canal wrote:
> > > > > Hi Marius,
> > > > > 
> > > > > > This patch adds support for creating multiple virtual connectors, in
> > > > > > case one would need it. Use module parameters to specify how many,
> > > > > > defaulting to just one, allocating from the start, a maximum of 4
> > > > > > possible outputs.
> > > > > 
> > > > > I got a bit confused by this description. The commit message
> > > > > says that you
> > > > > are creating multiple connectors, but it seems like you are
> > > > > creating multiple
> > > > > pipes. From what I could see, for each new connector created,
> > > > > you are also
> > > > > creating a new CRTC and new planes.
> > > > > 
> > > > > Moreover, if your real intention is to create multiple pipes, I
> > > > > believe that
> > > > > you don't really need to duplicate the planes as well.
> > > > > 
> > > > > I ran the VKMS CI [1] with your patches applied and, although
> > > > > most of the
> > > > > tests are now passing with multiple pipes, which is really nice,
> > > > > the test
> > > > > kms_writeback crashes with the following error:
> > > > > 
> > > > > [ 1997.244347] [IGT] kms_writeback: starting subtest
> > > > > writeback-check-output
> > > > > [ 1997.250673] BUG: kernel NULL pointer dereference, address:
> > > > > 000000000000016c
> > > > > [ 1997.250691] #PF: supervisor read access in kernel mode
> > > > > [ 1997.250693] #PF: error_code(0x0000) - not-present page
> > > > > [ 1997.250694] PGD 800000001a877067 P4D 800000001a877067 PUD
> > > > > 1a872067 PMD 0
> > > > > [ 1997.250697] Oops: 0000 [#1] PREEMPT SMP PTI
> > > > > [ 1997.250699] CPU: 2 PID: 3223 Comm: kms_writeback Not tainted
> > > > > 6.3.0-rc4-g8c2c29ba129f-dirty #257
> > > > > [ 1997.250701] Hardware name: QEMU Standard PC (i440FX + PIIX,
> > > > > 1996), BIOS 1.16.2-1.fc37 04/01/2014
> > > > > [ 1997.250703] RIP: 0010:drm_vblank_get+0xa/0xe0
> > > > > [ 1997.250710] Code: a9 66 66 66 66 66 66 2e 0f 1f 84 00 00 00
> > > > > 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 41 57
> > > > > 41 56 41 55 41 54 53 <8b> 87 6c 01 00 00 41 bc ea ff ff ff 85 c0
> > > > > 74 74 89 f5 48 89 fb 39
> > > > > [ 1997.250712] RSP: 0018:ffffb84d407a3b08 EFLAGS: 00010202
> > > > > [ 1997.250714] RAX: 0000000000000000 RBX: ffffa2eb02bf8b70 RCX:
> > > > > 0000000000000000
> > > > > [ 1997.250718] RDX: ffffa2eb180d2420 RSI: 0000000000000000 RDI:
> > > > > 0000000000000000
> > > > > [ 1997.250719] RBP: ffffa2eb02bf99e8 R08: 0000000000000036 R09:
> > > > > ffffa2eb01d620c0
> > > > > [ 1997.250720] R10: ffffe82b84027e40 R11: ffffffffc0042520 R12:
> > > > > ffffa2eb01c01900
> > > > > [ 1997.250721] R13: ffffa2eb02bf9b60 R14: 0000000000000001 R15:
> > > > > ffffa2ea1ecd6b80
> > > > > [ 1997.250722] FS:  00007f7d44e89a80(0000)
> > > > > GS:ffffa2eb3bd00000(0000) knlGS:0000000000000000
> > > > > [ 1997.250723] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > > [ 1997.250725] CR2: 000000000000016c CR3: 000000001ec02000 CR4:
> > > > > 00000000000006e0
> > > > > [ 1997.250728] Call Trace:
> > > > > [ 1997.250735]  <TASK>
> > > > > [ 1997.250736]  vkms_set_composer+0x18/0x60 [vkms]
> > > > > [ 1997.250745]  vkms_wb_atomic_commit+0x93/0x150 [vkms]
> > > > > [ 1997.250749]  drm_atomic_helper_commit_modeset_enables+0x1d9/0x250
> > > > > [ 1997.250754]  vkms_atomic_commit_tail+0x33/0xb0 [vkms]
> > > > > [ 1997.250758]  commit_tail+0x8d/0x170
> > > > > [ 1997.250761]  drm_atomic_helper_commit+0x26b/0x280
> > > > > [ 1997.250763]  drm_atomic_commit+0x9f/0xc0
> > > > > [ 1997.250766]  ? __pfx___drm_printfn_info+0x10/0x10
> > > > > [ 1997.250769]  drm_mode_atomic_ioctl+0x34c/0x480
> > > > > [ 1997.250771]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
> > > > > [ 1997.250773]  drm_ioctl_kernel+0xd7/0x150
> > > > > [ 1997.250780]  drm_ioctl+0x31f/0x490
> > > > > [ 1997.250781]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
> > > > > [ 1997.250783]  __se_sys_ioctl+0x6d/0xb0
> > > > > [ 1997.250789]  do_syscall_64+0x43/0x90
> > > > > [ 1997.250795]  entry_SYSCALL_64_after_hwframe+0x72/0xdc
> > > > > 
> > > > > [1] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/tree/master/tests/vkms_ci
> > > > > 
> > > > > Best Regards,
> > > > > - Maíra Canal
> > > > > 
> > > > > > 
> > > > > > This is of particular importance when testing out the DRM backend in
> > > > > > compositors, but also to be able to independently handle multiple
> > > > > > outputs/connectors, like setting one to off/sleep on while the others
> > > > > > are on, and combinations that arise from that.
> > > > > > 
> > > > > > Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> > > > > > ---
> > > > > >   drivers/gpu/drm/vkms/vkms_crtc.c      |  3 +--
> > > > > >   drivers/gpu/drm/vkms/vkms_drv.c       | 26 ++++++++++++++++++++++----
> > > > > >   drivers/gpu/drm/vkms/vkms_drv.h       |  8 +++++---
> > > > > >   drivers/gpu/drm/vkms/vkms_output.c    |  5 ++---
> > > > > >   drivers/gpu/drm/vkms/vkms_writeback.c | 18 ++++++++----------
> > > > > >   5 files changed, 38 insertions(+), 22 deletions(-)
> > > > > > 
> > > 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]