Hello,
We have a patch in Weston to replace drmGetConnector() calls with drmGetConnectorCurrent():
https://gitlab.freedesktop.org/wayland/weston/-/issues/437 https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/518
Unfortunately this is not working when we use VKMS (upstream version tested). Please see
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/518#note_7003...
for more information, and feel free to jump into the discussion. If there's more helpful information that I can share, please let me know.
Thank you, Leandro
On Fri, Nov 20, 2020 at 01:19:04PM -0300, Leandro Ribeiro wrote:
I guess this is without fbdev configured on vkms? That's what usually papers over this problem for most drivers.
for more information, and feel free to jump into the discussion. If there's more helpful information that I can share, please let me know.
Like Simon suggested, please submit that patch you have for discussion. -Daniel
In userspace we can use drmGetConnector() or drmGetConnectorCurrent() in order to retrieve connector information. The difference between both is that the former retrieves the complete set of modes and encoders associated with the connector, while the latter only retrieves the currently known set of modes and encoders - but is much faster.
This performance improvement is the reason why userspace applications may prefer to use drmGetConnectorCurrent() when they need to retrieve information from a device. The problem is that until now VKMS used to init its output with an encoder, but without any valid mode, so these userspace applications would not be able to use VKMS.
Call drm_helper_probe_single_connector_modes() during VKMS output initialization in order to start with the set of all valid modes.
Signed-off-by: Leandro Ribeiro leandro.ribeiro@collabora.com --- drivers/gpu/drm/vkms/vkms_output.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index 4a1848b0318f..20343592d38a 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -80,6 +80,12 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) goto err_attach; }
+ ret = drm_helper_probe_single_connector_modes(connector, XRES_MAX, YRES_MAX); + if (ret == 0) { + DRM_ERROR("Failed to get modes for connector\n"); + goto err_attach; + } + ret = vkms_enable_writeback_connector(vkmsdev); if (ret) DRM_ERROR("Failed to init writeback connector\n");
On Monday, November 30, 2020 2:03 AM, Leandro Ribeiro leandro.ribeiro@collabora.com wrote:
We discussed with Daniel Vetter and it turns out user-space should always use drmGetConnector(). See [1].
[1]: https://lists.freedesktop.org/archives/dri-devel/2020-November/289506.html
On Mon, 30 Nov 2020 10:23:04 +0000 Simon Ser contact@emersion.fr wrote:
Hi,
where is the discussion?
Please record the justitication for that patch in its commit message. "Can't" does not explain anything.
Thanks, pq
On Monday, November 30, 2020 12:13 PM, Pekka Paalanen ppaalanen@gmail.com wrote:
where is the discussion?
https://dri.freedesktop.org/~cbrill/dri-log/?channel=intel-gfx&date=2020...
Please record the justitication for that patch in its commit message. "Can't" does not explain anything.
Yeah, sorry about that. I'm just annoyed by all of this get_connector uAPI, so I don't really want to spend a lot of time documenting why it's so gross.
On Mon, 30 Nov 2020 11:16:56 +0000 Simon Ser contact@emersion.fr wrote:
Thanks, I read that.
But I still don't understand why the kernel cannot be fixed to do the right thing that most of us assumed it should be doing: probe automatically so userspace never needs to.
Thanks, pq
CC Daniel and Ville
On Monday, November 30, 2020 12:24 PM, Pekka Paalanen ppaalanen@gmail.com wrote:
My understanding is that it could maybe be implemented this way, but that it's not the way it works right now. So someone would need to go through all DRM drivers and implement the better behavior, then could restore this doc section.
On Mon, 30 Nov 2020 12:20:08 +0000 Simon Ser contact@emersion.fr wrote:
Right, so that would be really good to explain in the commit message.
Thanks, pq
On Tuesday, December 1, 2020 9:46 AM, Pekka Paalanen ppaalanen@gmail.com wrote:
v2 should take care of it. Let me know if you think the new commit message can be improved.
Greeting,
FYI, we noticed the following commit (built with gcc-9):
commit: b6c9ddb8c427706c12a28944d95bbade47f17554 ("[PATCH] drm/vkms: detect modes during output initialization") url: https://github.com/0day-ci/linux/commits/Leandro-Ribeiro/drm-vkms-detect-mod... base: git://anongit.freedesktop.org/drm-intel for-linux-next
in testcase: trinity version: trinity-x86_64-af355e9-1_2019-12-03 with following parameters:
runtime: 300s
test-description: Trinity is a linux system call fuzz tester. test-url: http://codemonkey.org.uk/projects/trinity/
on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
+----------------------------------------------------------------------------------------+------------+------------+ | | 5cc2b61fb2 | b6c9ddb8c4 | +----------------------------------------------------------------------------------------+------------+------------+ | WARNING:at_drivers/gpu/drm/drm_probe_helper.c:#drm_helper_probe_single_connector_modes | 0 | 2 | | RIP:drm_helper_probe_single_connector_modes | 0 | 2 | | WARNING:at_drivers/gpu/drm/drm_modes.c:#drm_mode_probed_add | 0 | 2 | | RIP:drm_mode_probed_add | 0 | 2 | | WARNING:at_drivers/gpu/drm/drm_modes.c:#drm_connector_list_update | 0 | 2 | | RIP:drm_connector_list_update | 0 | 2 | +----------------------------------------------------------------------------------------+------------+------------+
If you fix the issue, kindly add following tag Reported-by: kernel test robot oliver.sang@intel.com
[ 26.025573] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/drm_probe_helper.c:429 drm_helper_probe_single_connector_modes+0xd0/0x11fc [ 26.026322] Modules linked in: [ 26.027751] CPU: 0 PID: 1 Comm: swapper Not tainted 5.10.0-rc2-00482-gb6c9ddb8c427 #1 [ 26.028271] RIP: 0010:drm_helper_probe_single_connector_modes+0xd0/0x11fc [ 26.028722] Code: ea 03 80 3c 02 00 74 05 e8 40 78 f7 fe 48 8b 83 88 03 00 00 49 8d bc 24 80 04 00 00 48 89 44 24 28 e8 3c 4e d4 fe 84 c0 75 02 <0f> 0b 31 f6 48 8d bc 24 a0 00 00 00 e8 c0 3c 05 00 48 8d 43 48 48 [ 26.029931] RSP: 0018:ffff88810020fb70 EFLAGS: 00010246 [ 26.030286] RAX: 0000000000000000 RBX: ffff88815f8295a8 RCX: ffffffffa1faee29 [ 26.030780] RDX: 1ffff1102bf05090 RSI: 0000000000000008 RDI: ffff88815f828480 [ 26.031255] RBP: ffff88815f828000 R08: ffffed102bf05091 R09: 0000000000000002 [ 26.031727] R10: ffffed102bf05091 R11: ffff88815f828487 R12: ffff88815f828000 [ 26.032207] R13: ffff88815f6d7000 R14: ffff88815f6d6800 R15: ffff88815f8295a8 [ 26.032681] FS: 0000000000000000(0000) GS:ffffffffa5ca1000(0000) knlGS:0000000000000000 [ 26.033214] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 26.033601] CR2: 00007f3a14e77860 CR3: 0000000158c34000 CR4: 00000000000406f0 [ 26.034089] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 26.034577] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 26.035050] Call Trace: [ 26.035228] ? kasprintf+0x8c/0xa8 [ 26.035466] ? drm_connector_mode_valid+0x12a/0x12a [ 26.035796] ? drm_encoder_init+0x231/0x434 [ 26.036082] ? drm_atomic_normalize_zpos+0x76f/0x76f [ 26.036418] ? drm_connector_init+0xcc2/0xd5b [ 26.036714] ? drm_connector_list_iter_next+0x16c/0x16c [ 26.037067] vkms_output_init+0x1a9/0x24f [ 26.037362] vkms_init+0x3d2/0x43e [ 26.037599] ? drm_core_init+0xc5/0xc5 [ 26.037858] ? drm_core_init+0xc5/0xc5 [ 26.038115] ? drm_core_init+0xc5/0xc5 [ 26.038373] do_one_initcall+0x111/0x28f [ 26.038649] ? rcu_read_unlock+0x1e/0x1e [ 26.038921] ? parameq+0x28/0x28 [ 26.039147] do_basic_setup+0x1b6/0x1e5 [ 26.039411] kernel_init_freeable+0x126/0x155 [ 26.039709] ? rest_init+0x10c/0x10c [ 26.039956] kernel_init+0x8/0x107 [ 26.040191] ret_from_fork+0x1f/0x30 [ 26.040455] CPU: 0 PID: 1 Comm: swapper Not tainted 5.10.0-rc2-00482-gb6c9ddb8c427 #1 [ 26.040973] Call Trace: [ 26.041150] __warn+0x133/0x16d [ 26.041371] ? drm_helper_probe_single_connector_modes+0xd0/0x11fc [ 26.041785] report_bug+0x151/0x192 [ 26.042028] handle_bug+0x3c/0x5d [ 26.042258] exc_invalid_op+0x14/0x3d [ 26.042545] asm_exc_invalid_op+0x12/0x20 [ 26.042825] RIP: 0010:drm_helper_probe_single_connector_modes+0xd0/0x11fc [ 26.043279] Code: ea 03 80 3c 02 00 74 05 e8 40 78 f7 fe 48 8b 83 88 03 00 00 49 8d bc 24 80 04 00 00 48 89 44 24 28 e8 3c 4e d4 fe 84 c0 75 02 <0f> 0b 31 f6 48 8d bc 24 a0 00 00 00 e8 c0 3c 05 00 48 8d 43 48 48 [ 26.044511] RSP: 0018:ffff88810020fb70 EFLAGS: 00010246 [ 26.044871] RAX: 0000000000000000 RBX: ffff88815f8295a8 RCX: ffffffffa1faee29 [ 26.045346] RDX: 1ffff1102bf05090 RSI: 0000000000000008 RDI: ffff88815f828480 [ 26.045821] RBP: ffff88815f828000 R08: ffffed102bf05091 R09: 0000000000000002 [ 26.046295] R10: ffffed102bf05091 R11: ffff88815f828487 R12: ffff88815f828000 [ 26.046790] R13: ffff88815f6d7000 R14: ffff88815f6d6800 R15: ffff88815f8295a8 [ 26.047283] ? mutex_is_locked+0xe/0x3a [ 26.047551] ? kasprintf+0x8c/0xa8 [ 26.047789] ? drm_connector_mode_valid+0x12a/0x12a [ 26.048121] ? drm_encoder_init+0x231/0x434 [ 26.048408] ? drm_atomic_normalize_zpos+0x76f/0x76f [ 26.048744] ? drm_connector_init+0xcc2/0xd5b [ 26.049042] ? drm_connector_list_iter_next+0x16c/0x16c [ 26.049396] vkms_output_init+0x1a9/0x24f [ 26.049672] vkms_init+0x3d2/0x43e [ 26.049909] ? drm_core_init+0xc5/0xc5 [ 26.050169] ? drm_core_init+0xc5/0xc5 [ 26.050450] ? drm_core_init+0xc5/0xc5 [ 26.050707] do_one_initcall+0x111/0x28f [ 26.050975] ? rcu_read_unlock+0x1e/0x1e [ 26.051244] ? parameq+0x28/0x28 [ 26.051469] do_basic_setup+0x1b6/0x1e5 [ 26.051732] kernel_init_freeable+0x126/0x155 [ 26.052027] ? rest_init+0x10c/0x10c [ 26.052273] kernel_init+0x8/0x107 [ 26.052507] ret_from_fork+0x1f/0x30 [ 26.052753] irq event stamp: 4080453 [ 26.053000] hardirqs last enabled at (4080461): [<ffffffffa1fcecfa>] console_unlock+0x651/0x69a [ 26.053577] hardirqs last disabled at (4080468): [<ffffffffa1fce784>] console_unlock+0xdb/0x69a [ 26.054161] softirqs last enabled at (4068892): [<ffffffffa4600516>] __do_softirq+0x516/0x5b1 [ 26.054732] softirqs last disabled at (4068887): [<ffffffffa4400e1f>] asm_call_irq_on_stack+0xf/0x20 [ 26.055333] ---[ end trace f71b1d00dc6fce64 ]---
To reproduce:
# build kernel cd linux cp config-5.10.0-rc2-00482-gb6c9ddb8c427 .config make HOSTCC=gcc-9 CC=gcc-9 ARCH=x86_64 olddefconfig prepare modules_prepare bzImage
git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email
Thanks, Oliver Sang
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
kernel test robot -
Leandro Ribeiro -
Pekka Paalanen -
Simon Ser