Re: [PATCH v2 2/3] vkms: Add support for multiple pipes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/25/23 04:44, Marius Vlad wrote:
Hello,

On 4/21/23 15:53, Maíra Canal wrote:
On 4/21/23 04:05, Marius Vlad wrote:
Hi Maíra,

Thanks a lot for taking a look!

On Thu, Apr 20, 2023 at 01:47:59PM -0300, Maíra Canal wrote:
Hi Marius,

Thanks for the changing the commit message! Just a few nits:

On 4/20/23 05:41, Marius Vlad wrote:
This adds support for creating multiple virtual pipes, in case one would
need to display multiple independent things on different outputs.

We use a module parameter to specify how many to create,  defaulting to
just one with the possibility of specifying a maximum of 4 possible pipes.

This is of particular importance when testing out the DRM back-end in
compositors, but also to be able to independently set different DPMS states.

When I run the VKMS CI, I'm getting the following warning in most tests
with max_pipes=3:

(kms_atomic:2085) igt_kms-WARNING: Output Virtual-2 could not be assigned to a pipe
(kms_atomic:2085) igt_kms-WARNING: Output Virtual-3 could not be assigned to a pipe
Hmm, first time I'm seeing these. By VKMS CI you mean piglit with some a
list of igt tests? Noticed some warnings about the Writeback connector but
those warnings are with or without these patches.

I'm sorry about this one. I believe it was a misconfigure. Please ignore
it. About the other warnings, I still can reproduce them.

About the VKMS CI, I mean:

./scripts/run-tests.sh -T tests/vkms_ci/vkms.testlist
Thanks for the heads-up, been doing a few runs of these and I'm not able to trigger any crashes, using the whole range (1-4) for max_pipes. I'm running the tests in a virtualized environment and instead I'm seeing hangs for all the suspend tests in that testlist. But that's with or without this series, and probably due to running these in a VM.

The crash (for max_pipes=4) seems to be a consquence of loading the driver (I'm seeing modprobe) and unloading it.

Actually, I'm checking here that you can only reproduce this error when
you enable the overlay planes. So,

$ modprobe vkms max_pipes=4

works just fine, but

$ modprobe vkms enable_overlay=1 max_pipes=4

doesn't work.

By taking a quick look at the error, I can see that the plane initialization
is crashing in:

[  260.748860] WARNING: CPU: 2 PID: 1604 at
drivers/gpu/drm/drm_plane.c:252 __drm_universal_plane_init+0x4fe/0x550

which is:

/* plane index is used with 32bit bitmasks */
if (WARN_ON(config->num_total_plane >= 32))
	return -EINVAL;

I believe that using 8 overlay planes for each pipe, you are probably going
to end up activating this WARN_ON.

BTW You'll probably need to rebase the series again, as I have pushed
some new changes to vkms.

Best Regards,
- Maíra Canal


Been doing that as well, and haven't seen any crashes so far.

I know that the tests would  actually try to modprobe the driver but it seems it can't find it in my case when I run it virtualized, so I need to that before running the tests or just build-it in. Do you load the driver by yourself or you just let the test do that? Or maybe there's something else going (a particular test not included in that list?) that causes that.


Best Regards,
- Maíra Canal



Signed-off-by: Marius Vlad <marius.vlad@xxxxxxxxxxxxx>
---
   drivers/gpu/drm/vkms/vkms_crtc.c      |  3 +--
   drivers/gpu/drm/vkms/vkms_drv.c       | 27 ++++++++++++++++++++++-----
   drivers/gpu/drm/vkms/vkms_drv.h       | 11 ++++++++---
   drivers/gpu/drm/vkms/vkms_output.c    |  5 ++---
   drivers/gpu/drm/vkms/vkms_writeback.c | 24 ++++++++++++------------
   5 files changed, 45 insertions(+), 25 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 6d3a2d57d992..63091ce6735a 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -51,14 +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_pipes = 1;

I believe this should be unsigned int, otherwise if you configure
max_pipes=-3, you will get an error like:
Yeah, probably need to add a also a minimum lower range range. Even if with an
uint8_t, it would wrap to unnecessary large values. I'd just ignore
everything under 1.

[   31.015005] ------------[ cut here ]------------
[   31.015011] WARNING: CPU: 0 PID: 1062 at mm/page_alloc.c:5568 __alloc_pages+0x65/0x300
[   31.015021] Modules linked in: vkms(+) [last unloaded: vkms]
[   31.015030] CPU: 0 PID: 1062 Comm: modprobe Not tainted 6.3.0-rc4-01502-g91c249b2b9f6-dirty #92
[   31.015035] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc37 04/01/2014
[   31.015038] RIP: 0010:__alloc_pages+0x65/0x300
[   31.015043] Code: 00 f7 c7 00 20 00 00 75 0e 83 fe 0b 72 09 f6 05 16 02 c3 01 01 74 0c 83 fe 0a 76 15 31 ed e9 51 01 00 00 c6 05 01 02 c3 01 01 <0f> 0b 83 fe 0a 77 eb 23 1d f6 05 c5 01 65 48 8b 3d 26 77 fd 71 8b
[   31.015047] RSP: 0018:ffffa95442247880 EFLAGS: 00010246
[   31.015052] RAX: 0bb151d28a828b00 RBX: 0000000000040dc0 RCX: 0000000000000000
[   31.015055] RDX: 0000000000000000 RSI: 000000000000001d RDI: 0000000000040dc0
[   31.015059] RBP: 0000000000000000 R08: ffffffff8f6cbeac R09: 0000000000000010
[   31.015061] R10: ffff9cb282c6a500 R11: 0000000400000001 R12: 0000000000000dc0
[   31.015065] R13: ffffffffc01e3258 R14: 0000000000000000 R15: 000000000000001d
[   31.015067] FS:  00007f95f41b6740(0000) GS:ffff9cb2bbc00000(0000) knlGS:0000000000000000
[   31.015071] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   31.015075] CR2: 00007ffc3933cec8 CR3: 000000011c110000 CR4: 00000000000006f0
[   31.015082] Call Trace:
[   31.015102]  <TASK>
[   31.015106]  __kmalloc_large_node+0x82/0x120
[   31.015113]  ? drm_vblank_init+0x30/0x100
[   31.015120]  __kmalloc_node_track_caller+0xc4/0x180
[   31.015125]  ? __kmalloc_node_track_caller+0x8f/0x180
[   31.015128]  ? preempt_count_add+0x5e/0xa0
[   31.015134]  drmm_kmalloc+0x31/0x110
[   31.015140]  drm_vblank_init+0x30/0x100
[   31.015145]  init_module+0x179/0xff0 [vkms]
[   31.015160]  ? __pfx_init_module+0x10/0x10 [vkms]
[   31.015171]  do_one_initcall+0x10c/0x2d0
[   31.015180]  ? do_init_module+0x1d/0x1c0
[   31.015187]  do_init_module+0x45/0x1c0
[   31.015192]  load_module+0x16eb/0x1970
[   31.015198]  ? kernel_read_file_from_fd+0x5a/0x90
[   31.015205]  __se_sys_finit_module+0xb3/0xf0
[   31.015212]  do_syscall_64+0x43/0x90
[   31.015218]  entry_SYSCALL_64_after_hwframe+0x72/0xdc
[   31.015224] RIP: 0033:0x7f95f42bf6bd
[   31.015228] Code: 5d c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 43 d7 0c 00 f7 d8 64 89 01 48
[   31.015232] RSP: 002b:00007ffc3933ef08 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[   31.015236] RAX: ffffffffffffffda RBX: 000055ec9b730a10 RCX: 00007f95f42bf6bd
[   31.015239] RDX: 0000000000000000 RSI: 000055ec9b730470 RDI: 0000000000000003
[   31.015241] RBP: 000055ec9b730470 R08: 0000000000000000 R09: 00007ffc3933f030
[   31.015243] R10: 0000000000000003 R11: 0000000000000246 R12: 0000000000040000
[   31.015245] R13: 000055ec9b730ab0 R14: 0000000000000000 R15: 000055ec9b730c70
[   31.015248]  </TASK>
[   31.015250] ---[ end trace 0000000000000000 ]---
[   31.015253] [drm:init_module [vkms]] *ERROR* Failed to vblank

Also, when I setted max_pipes=4, I'm getting the following error:
I'll try to replicate these with igt tests, as those seems to stress
things a bit. Seems a bit odd to happen only at four.

[   74.804396] ------------[ cut here ]------------
[   74.804402] WARNING: CPU: 2 PID: 1587 at drivers/gpu/drm/drm_plane.c:252 __drm_universal_plane_init+0x4fe/0x550
[   74.804415] Modules linked in: vkms(+)
[   74.804421] CPU: 2 PID: 1587 Comm: modprobe Tainted: G W          6.3.0-rc4-01502-g91c249b2b9f6-dirty #92
[   74.804426] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc37 04/01/2014
[   74.804429] RIP: 0010:__drm_universal_plane_init+0x4fe/0x550
[   74.804434] Code: c4 20 5b 41 5c 41 5d 41 5e 41 5f 5d c3 cc cc cc cc 49 8b b6 90 05 00 00 48 8b 44 24 08 8b 10 48 8b 3c 24 e8 84 b8 ff ff eb d1 <0f> 0b 41 bc ea ff ff ff eb c7 0f 0b 41 bc ea ff ff ff eb bd 49 8b
[   74.804438] RSP: 0018:ffffb2cc8217f858 EFLAGS: 00010246
[   74.804453] RAX: 0000000000000000 RBX: ffffffffc0223318 RCX: ffffffffc0223318
[   74.804456] RDX: 0000000000000008 RSI: ffffa24080913028 RDI: ffffa24098d20000
[   74.804458] RBP: ffffb2cc8217f970 R08: ffffffffc0223300 R09: 0000000000000005
[   74.804461] R10: ffffb2cc8217f910 R11: 0000000000000000 R12: 0000000000000008
[   74.804463] R13: ffffa24080913028 R14: ffffa24080913028 R15: ffffa24098d20000
[   74.804465] FS:  00007fbd9fc9b740(0000) GS:ffffa240bbd00000(0000) knlGS:0000000000000000
[   74.804469] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   74.804471] CR2: 00007f74a59d1f30 CR3: 0000000103304000 CR4: 00000000000006e0
[   74.804479] Call Trace:
[   74.804483]  <TASK>
[   74.804485]  ? preempt_count_add+0x5e/0xa0
[   74.804492]  __drmm_universal_plane_alloc+0xce/0x140
[   74.804497]  ? __drm_connector_init+0x3d6/0x4c0
[   74.804502]  vkms_plane_init+0x4d/0x70 [vkms]
[   74.804516]  vkms_output_init+0xa7/0x380 [vkms]
[   74.804527]  init_module+0x25d/0xff0 [vkms]
[   74.804538]  ? __pfx_init_module+0x10/0x10 [vkms]
[   74.804548]  do_one_initcall+0x10c/0x2d0
[   74.804557]  ? do_init_module+0x1d/0x1c0
[   74.804564]  do_init_module+0x45/0x1c0
[   74.804569]  load_module+0x16eb/0x1970
[   74.804575]  ? kernel_read_file_from_fd+0x5a/0x90
[   74.804615]  __se_sys_finit_module+0xb3/0xf0
[   74.804624]  do_syscall_64+0x43/0x90
[   74.804632]  entry_SYSCALL_64_after_hwframe+0x72/0xdc
[   74.804640] RIP: 0033:0x7fbd9fda46bd
[   74.804645] Code: 5d c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 43 d7 0c 00 f7 d8 64 89 01 48
[   74.804649] RSP: 002b:00007ffd8cdc6d58 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[   74.804655] RAX: ffffffffffffffda RBX: 00005614c1f3ea10 RCX: 00007fbd9fda46bd
[   74.804659] RDX: 0000000000000000 RSI: 00005614c1f3e470 RDI: 0000000000000003
[   74.804662] RBP: 00005614c1f3e470 R08: 0000000000000000 R09: 00007ffd8cdc6e80
[   74.804665] R10: 0000000000000003 R11: 0000000000000246 R12: 0000000000040000
[   74.804668] R13: 00005614c1f3eab0 R14: 0000000000000000 R15: 00005614c1f3ec70
[   74.804673]  </TASK>
[   74.804675] ---[ end trace 0000000000000000 ]---
[   74.804736] BUG: kernel NULL pointer dereference, address: 00000000000000a8
[   74.806126] #PF: supervisor read access in kernel mode
[   74.806590] #PF: error_code(0x0000) - not-present page
[   74.807160] PGD 0 P4D 0
[   74.807463] Oops: 0000 [#1] PREEMPT SMP PTI
[   74.807874] CPU: 2 PID: 1587 Comm: modprobe Tainted: G W          6.3.0-rc4-01502-g91c249b2b9f6-dirty #92
[   74.808597] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc37 04/01/2014
[   74.809097] RIP: 0010:destroy_workqueue+0xf/0x2f0
[   74.809397] Code: 41 5e 41 5f 5d e9 b1 24 ff 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 41 57 41 56 41 54 53 48 83 ec 10 49 89 ff <48> 8b bf a8 00 00 00 48 85 ff 74 14 49 c7 87 a8 00 00 00 00 00 00
[   74.810308] RSP: 0018:ffffb2cc8217f928 EFLAGS: 00010286
[   74.810637] RAX: 0000000000000004 RBX: 0000000000000003 RCX: 0000000000000001
[   74.811041] RDX: 0000000000000002 RSI: 0000000000000082 RDI: 0000000000000000
[   74.811436] RBP: ffffa23f9df001c0 R08: 0000000000b90826 R09: 000003fffffffc00
[   74.811857] R10: 000000000000015d R11: 0000000000000000 R12: 0000000000000282
[   74.812256] R13: 0000000000000001 R14: ffffa24098d20000 R15: 0000000000000000
[   74.812673] FS:  00007fbd9fc9b740(0000) GS:ffffa240bbd00000(0000) knlGS:0000000000000000
[   74.813294] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   74.814296] CR2: 00000000000000a8 CR3: 0000000103304000 CR4: 00000000000006e0
[   74.814824] Call Trace:
[   74.815114]  <TASK>
[   74.815388]  ? _raw_spin_unlock_irq+0xe/0x30
[   74.815806]  vkms_release+0x26/0x50 [vkms]
[   74.816187]  devm_drm_dev_init_release+0x2d/0x60
[   74.816590]  devres_release_group+0xeb/0x140
[   74.817005]  init_module+0x196/0xff0 [vkms]
[   74.817394]  ? __pfx_init_module+0x10/0x10 [vkms]
[   74.817833]  do_one_initcall+0x10c/0x2d0
[   74.818208]  ? do_init_module+0x1d/0x1c0
[   74.818587]  do_init_module+0x45/0x1c0
[   74.818869]  load_module+0x16eb/0x1970
[   74.819147]  ? kernel_read_file_from_fd+0x5a/0x90
[   74.819458]  __se_sys_finit_module+0xb3/0xf0
[   74.819786]  do_syscall_64+0x43/0x90
[   74.820041]  entry_SYSCALL_64_after_hwframe+0x72/0xdc
[   74.821852] RIP: 0033:0x7fbd9fda46bd
[   74.822118] Code: 5d c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 43 d7 0c 00 f7 d8 64 89 01 48
[   74.822902] RSP: 002b:00007ffd8cdc6d58 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[   74.823311] RAX: ffffffffffffffda RBX: 00005614c1f3ea10 RCX: 00007fbd9fda46bd
[   74.823648] RDX: 0000000000000000 RSI: 00005614c1f3e470 RDI: 0000000000000003
[   74.823977] RBP: 00005614c1f3e470 R08: 0000000000000000 R09: 00007ffd8cdc6e80
[   74.824297] R10: 0000000000000003 R11: 0000000000000246 R12: 0000000000040000
[   74.824612] R13: 00005614c1f3eab0 R14: 0000000000000000 R15: 00005614c1f3ec70
[   74.824931]  </TASK>
[   74.825080] Modules linked in: vkms(+)
[   74.825269] CR2: 00000000000000a8
[   74.825505] ---[ end trace 0000000000000000 ]---
[   74.825735] RIP: 0010:destroy_workqueue+0xf/0x2f0
[   74.825948] Code: 41 5e 41 5f 5d e9 b1 24 ff 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 41 57 41 56 41 54 53 48 83 ec 10 49 89 ff <48> 8b bf a8 00 00 00 48 85 ff 74 14 49 c7 87 a8 00 00 00 00 00 00
[   74.826609] RSP: 0018:ffffb2cc8217f928 EFLAGS: 00010286
[   74.826843] RAX: 0000000000000004 RBX: 0000000000000003 RCX: 0000000000000001
[   74.827114] RDX: 0000000000000002 RSI: 0000000000000082 RDI: 0000000000000000
[   74.827382] RBP: ffffa23f9df001c0 R08: 0000000000b90826 R09: 000003fffffffc00
[   74.827622] R10: 000000000000015d R11: 0000000000000000 R12: 0000000000000282
[   74.827818] R13: 0000000000000001 R14: ffffa24098d20000 R15: 0000000000000000
[   74.828008] FS:  00007fbd9fc9b740(0000) GS:ffffa240bbd00000(0000) knlGS:0000000000000000
[   74.828245] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   74.828415] CR2: 00000000000000a8 CR3: 0000000103304000 CR4: 00000000000006e0

+module_param_named(max_pipes, max_pipes, int, 0444);
+MODULE_PARM_DESC(max_pipes, "Specify how many pipes 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);
-    if (vkms->output.composer_workq)
-        destroy_workqueue(vkms->output.composer_workq);
+    for (i = 0; i < vkmsdev->config->max_pipes; i++)
+        destroy_workqueue(vkmsdev->output[i].composer_workq);
   }
   static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
@@ -99,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, "pipes=%d\n", vkmsdev->config->max_pipes);
       return 0;
   }
@@ -133,6 +139,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);

I commited a patch a while ago that changed this bit. Therefore, your patch
isn't applicable in the drm-misc-next. It would be nice if you could rebase
the patches on top of the drm-misc-next.
Okay, will do.

Best Regards,
- Maíra Canal



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux