| Message ID | 20230328023129.3596968-1-zhouzongmin@kylinos.cn (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | drm/probe_helper: fix the warning reported when calling drm_kms_helper_poll_disable during suspend | expand |
On 2023-03-28 10:31:29 +0800, Zongmin Zhou wrote: > When drivers call drm_kms_helper_poll_disable from > their device suspend implementation without enabled output polling before, > following warning will be reported,due to work->func not be initialized: we see the same warning with the wpork in progress kms driver for apple silicon SoCs. The connectors do not need to polled so the driver never calls drm_kms_helper_poll_init(). > [ 55.141361] WARNING: CPU: 3 PID: 372 at kernel/workqueue.c:3066 __flush_work+0x22f/0x240 > [ 55.141382] Modules linked in: nls_iso8859_1 snd_hda_codec_generic ledtrig_audio snd_hda_intel snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec snd_hda_core snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq intel_rapl_msr intel_rapl_common bochs drm_vram_helper drm_ttm_helper snd_seq_device nfit ttm crct10dif_pclmul snd_timer ghash_clmulni_intel binfmt_misc sha512_ssse3 aesni_intel drm_kms_helper joydev input_leds syscopyarea crypto_simd snd cryptd sysfillrect sysimgblt mac_hid serio_raw soundcore qemu_fw_cfg sch_fq_codel msr parport_pc ppdev lp parport drm ramoops reed_solomon pstore_blk pstore_zone efi_pstore virtio_rng ip_tables x_tables autofs4 hid_generic usbhid hid ahci virtio_net i2c_i801 crc32_pclmul psmouse virtio_scsi libahci i2c_smbus lpc_ich xhci_pci net_failover virtio_blk xhci_pci_renesas failover > [ 55.141430] CPU: 3 PID: 372 Comm: kworker/u16:9 Not tainted 6.2.0-rc6+ #16 > [ 55.141433] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 > [ 55.141435] Workqueue: events_unbound async_run_entry_fn > [ 55.141441] RIP: 0010:__flush_work+0x22f/0x240 > [ 55.141444] Code: 8b 43 28 48 8b 53 30 89 c1 e9 f9 fe ff ff 4c 89 f7 e8 b5 95 d9 00 e8 00 53 08 00 45 31 ff e9 11 ff ff ff 0f 0b e9 0a ff ff ff <0f> 0b 45 31 ff e9 00 ff ff ff e8 e2 54 d8 00 66 90 90 90 90 90 90 > [ 55.141446] RSP: 0018:ff59221940833c18 EFLAGS: 00010246 > [ 55.141449] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff9b72bcbe > [ 55.141450] RDX: 0000000000000001 RSI: 0000000000000001 RDI: ff3ea01e4265e330 > [ 55.141451] RBP: ff59221940833c90 R08: 0000000000000000 R09: 8080808080808080 > [ 55.141453] R10: ff3ea01e42b3caf4 R11: 000000000000000f R12: ff3ea01e4265e330 > [ 55.141454] R13: 0000000000000001 R14: ff3ea01e505e5e80 R15: 0000000000000001 > [ 55.141455] FS: 0000000000000000(0000) GS:ff3ea01fb7cc0000(0000) knlGS:0000000000000000 > [ 55.141456] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 55.141458] CR2: 0000563543ad1546 CR3: 000000010ee82005 CR4: 0000000000771ee0 > [ 55.141464] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > [ 55.141465] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > [ 55.141466] PKRU: 55555554 > [ 55.141467] Call Trace: > [ 55.141469] <TASK> > [ 55.141472] ? pcie_wait_cmd+0xdf/0x220 > [ 55.141478] ? mptcp_seq_show+0xe0/0x180 > [ 55.141484] __cancel_work_timer+0x124/0x1b0 > [ 55.141487] cancel_delayed_work_sync+0x17/0x20 > [ 55.141490] drm_kms_helper_poll_disable+0x26/0x40 [drm_kms_helper] > [ 55.141516] drm_mode_config_helper_suspend+0x25/0x90 [drm_kms_helper] > [ 55.141531] ? __pm_runtime_resume+0x64/0x90 > [ 55.141536] bochs_pm_suspend+0x16/0x20 [bochs] > [ 55.141540] pci_pm_suspend+0x8b/0x1b0 > [ 55.141545] ? __pfx_pci_pm_suspend+0x10/0x10 > [ 55.141547] dpm_run_callback+0x4c/0x160 > [ 55.141550] __device_suspend+0x14c/0x4c0 > [ 55.141553] async_suspend+0x24/0xa0 > [ 55.141555] async_run_entry_fn+0x34/0x120 > [ 55.141557] process_one_work+0x21a/0x3f0 > [ 55.141560] worker_thread+0x4e/0x3c0 > [ 55.141563] ? __pfx_worker_thread+0x10/0x10 > [ 55.141565] kthread+0xf2/0x120 > [ 55.141568] ? __pfx_kthread+0x10/0x10 > [ 55.141570] ret_from_fork+0x29/0x50 > [ 55.141575] </TASK> > [ 55.141575] ---[ end trace 0000000000000000 ]--- > > Fixes: a4e771729a51 ("drm/probe_helper: sort out poll_running vs poll_enabled") > Signed-off-by: Zongmin Zhou<zhouzongmin@kylinos.cn> > --- > drivers/gpu/drm/drm_probe_helper.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > index 8127be134c39..ac72b18e2257 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -855,7 +855,8 @@ void drm_kms_helper_poll_disable(struct drm_device *dev) > if (dev->mode_config.poll_running) > drm_kms_helper_disable_hpd(dev); > > - cancel_delayed_work_sync(&dev->mode_config.output_poll_work); > + if (dev->mode_config.poll_enabled) > + cancel_delayed_work_sync(&dev->mode_config.output_poll_work); Checking for dev->mode_config.poll_enabled at the start of the function and return early if it is not true looks more in style with the rest of drm_probe_helper.c. No difference functionally of course. Tested with the apple kms driver. Reviewed-by: Janne Grunau <j@jannau.net> ciao Janne
On Thu, 20 Apr 2023 at 23:01, Janne Grunau <j@jannau.net> wrote: > > On 2023-03-28 10:31:29 +0800, Zongmin Zhou wrote: > > When drivers call drm_kms_helper_poll_disable from > > their device suspend implementation without enabled output polling before, > > following warning will be reported,due to work->func not be initialized: > > we see the same warning with the wpork in progress kms driver for apple > silicon SoCs. The connectors do not need to polled so the driver never > calls drm_kms_helper_poll_init(). > > > [ 55.141361] WARNING: CPU: 3 PID: 372 at kernel/workqueue.c:3066 __flush_work+0x22f/0x240 > > [ 55.141382] Modules linked in: nls_iso8859_1 snd_hda_codec_generic ledtrig_audio snd_hda_intel snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec snd_hda_core snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq intel_rapl_msr intel_rapl_common bochs drm_vram_helper drm_ttm_helper snd_seq_device nfit ttm crct10dif_pclmul snd_timer ghash_clmulni_intel binfmt_misc sha512_ssse3 aesni_intel drm_kms_helper joydev input_leds syscopyarea crypto_simd snd cryptd sysfillrect sysimgblt mac_hid serio_raw soundcore qemu_fw_cfg sch_fq_codel msr parport_pc ppdev lp parport drm ramoops reed_solomon pstore_blk pstore_zone efi_pstore virtio_rng ip_tables x_tables autofs4 hid_generic usbhid hid ahci virtio_net i2c_i801 crc32_pclmul psmouse virtio_scsi libahci i2c_smbus lpc_ich xhci_pci net_failover virtio_blk xhci_pci_renesas failover > > [ 55.141430] CPU: 3 PID: 372 Comm: kworker/u16:9 Not tainted 6.2.0-rc6+ #16 > > [ 55.141433] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 > > [ 55.141435] Workqueue: events_unbound async_run_entry_fn > > [ 55.141441] RIP: 0010:__flush_work+0x22f/0x240 > > [ 55.141444] Code: 8b 43 28 48 8b 53 30 89 c1 e9 f9 fe ff ff 4c 89 f7 e8 b5 95 d9 00 e8 00 53 08 00 45 31 ff e9 11 ff ff ff 0f 0b e9 0a ff ff ff <0f> 0b 45 31 ff e9 00 ff ff ff e8 e2 54 d8 00 66 90 90 90 90 90 90 > > [ 55.141446] RSP: 0018:ff59221940833c18 EFLAGS: 00010246 > > [ 55.141449] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff9b72bcbe > > [ 55.141450] RDX: 0000000000000001 RSI: 0000000000000001 RDI: ff3ea01e4265e330 > > [ 55.141451] RBP: ff59221940833c90 R08: 0000000000000000 R09: 8080808080808080 > > [ 55.141453] R10: ff3ea01e42b3caf4 R11: 000000000000000f R12: ff3ea01e4265e330 > > [ 55.141454] R13: 0000000000000001 R14: ff3ea01e505e5e80 R15: 0000000000000001 > > [ 55.141455] FS: 0000000000000000(0000) GS:ff3ea01fb7cc0000(0000) knlGS:0000000000000000 > > [ 55.141456] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > [ 55.141458] CR2: 0000563543ad1546 CR3: 000000010ee82005 CR4: 0000000000771ee0 > > [ 55.141464] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > [ 55.141465] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > [ 55.141466] PKRU: 55555554 > > [ 55.141467] Call Trace: > > [ 55.141469] <TASK> > > [ 55.141472] ? pcie_wait_cmd+0xdf/0x220 > > [ 55.141478] ? mptcp_seq_show+0xe0/0x180 > > [ 55.141484] __cancel_work_timer+0x124/0x1b0 > > [ 55.141487] cancel_delayed_work_sync+0x17/0x20 > > [ 55.141490] drm_kms_helper_poll_disable+0x26/0x40 [drm_kms_helper] > > [ 55.141516] drm_mode_config_helper_suspend+0x25/0x90 [drm_kms_helper] > > [ 55.141531] ? __pm_runtime_resume+0x64/0x90 > > [ 55.141536] bochs_pm_suspend+0x16/0x20 [bochs] > > [ 55.141540] pci_pm_suspend+0x8b/0x1b0 > > [ 55.141545] ? __pfx_pci_pm_suspend+0x10/0x10 > > [ 55.141547] dpm_run_callback+0x4c/0x160 > > [ 55.141550] __device_suspend+0x14c/0x4c0 > > [ 55.141553] async_suspend+0x24/0xa0 > > [ 55.141555] async_run_entry_fn+0x34/0x120 > > [ 55.141557] process_one_work+0x21a/0x3f0 > > [ 55.141560] worker_thread+0x4e/0x3c0 > > [ 55.141563] ? __pfx_worker_thread+0x10/0x10 > > [ 55.141565] kthread+0xf2/0x120 > > [ 55.141568] ? __pfx_kthread+0x10/0x10 > > [ 55.141570] ret_from_fork+0x29/0x50 > > [ 55.141575] </TASK> > > [ 55.141575] ---[ end trace 0000000000000000 ]--- > > > > Fixes: a4e771729a51 ("drm/probe_helper: sort out poll_running vs poll_enabled") > > Signed-off-by: Zongmin Zhou<zhouzongmin@kylinos.cn> > > --- > > drivers/gpu/drm/drm_probe_helper.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > > index 8127be134c39..ac72b18e2257 100644 > > --- a/drivers/gpu/drm/drm_probe_helper.c > > +++ b/drivers/gpu/drm/drm_probe_helper.c > > @@ -855,7 +855,8 @@ void drm_kms_helper_poll_disable(struct drm_device *dev) > > if (dev->mode_config.poll_running) > > drm_kms_helper_disable_hpd(dev); > > > > - cancel_delayed_work_sync(&dev->mode_config.output_poll_work); > > + if (dev->mode_config.poll_enabled) > > + cancel_delayed_work_sync(&dev->mode_config.output_poll_work); > > Checking for dev->mode_config.poll_enabled at the start of the function > and return early if it is not true looks more in style with the rest of > drm_probe_helper.c. I think it is an error to call drm_kms_helper_poll_disable() if polling was not initialized. So, in my opinion the fix should go to the drm_mode_config_helper_suspend() / _resume() instead. Please add a guard there using dev->mode_config.poll_enabled. > No difference functionally of course. Tested with the apple kms driver. > > Reviewed-by: Janne Grunau <j@jannau.net> > > ciao > Janne
On Thu, 2023-04-20 at 23:07 +0300, Dmitry Baryshkov wrote: > On Thu, 20 Apr 2023 at 23:01, Janne Grunau <j@jannau.net> wrote: > > > > On 2023-03-28 10:31:29 +0800, Zongmin Zhou wrote: > > > When drivers call drm_kms_helper_poll_disable from > > > their device suspend implementation without enabled output > > > polling before, > > > following warning will be reported,due to work->func not be > > > initialized: > > > > we see the same warning with the wpork in progress kms driver for > > apple > > silicon SoCs. The connectors do not need to polled so the driver > > never > > calls drm_kms_helper_poll_init(). > > > > > [ 55.141361] WARNING: CPU: 3 PID: 372 at > > > kernel/workqueue.c:3066 __flush_work+0x22f/0x240 > > > [ 55.141382] Modules linked in: nls_iso8859_1 > > > snd_hda_codec_generic ledtrig_audio snd_hda_intel > > > snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec snd_hda_core > > > snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi > > > snd_seq intel_rapl_msr intel_rapl_common bochs drm_vram_helper > > > drm_ttm_helper snd_seq_device nfit ttm crct10dif_pclmul snd_timer > > > ghash_clmulni_intel binfmt_misc sha512_ssse3 aesni_intel > > > drm_kms_helper joydev input_leds syscopyarea crypto_simd snd > > > cryptd sysfillrect sysimgblt mac_hid serio_raw soundcore > > > qemu_fw_cfg sch_fq_codel msr parport_pc ppdev lp parport drm > > > ramoops reed_solomon pstore_blk pstore_zone efi_pstore virtio_rng > > > ip_tables x_tables autofs4 hid_generic usbhid hid ahci virtio_net > > > i2c_i801 crc32_pclmul psmouse virtio_scsi libahci i2c_smbus > > > lpc_ich xhci_pci net_failover virtio_blk xhci_pci_renesas > > > failover > > > [ 55.141430] CPU: 3 PID: 372 Comm: kworker/u16:9 Not tainted > > > 6.2.0-rc6+ #16 > > > [ 55.141433] Hardware name: QEMU Standard PC (Q35 + ICH9, > > > 2009), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org > > > 04/01/2014 > > > [ 55.141435] Workqueue: events_unbound async_run_entry_fn > > > [ 55.141441] RIP: 0010:__flush_work+0x22f/0x240 > > > [ 55.141444] Code: 8b 43 28 48 8b 53 30 89 c1 e9 f9 fe ff ff 4c > > > 89 f7 e8 b5 95 d9 00 e8 00 53 08 00 45 31 ff e9 11 ff ff ff 0f 0b > > > e9 0a ff ff ff <0f> 0b 45 31 ff e9 00 ff ff ff e8 e2 54 d8 00 66 > > > 90 90 90 90 90 90 > > > [ 55.141446] RSP: 0018:ff59221940833c18 EFLAGS: 00010246 > > > [ 55.141449] RAX: 0000000000000000 RBX: 0000000000000000 RCX: > > > ffffffff9b72bcbe > > > [ 55.141450] RDX: 0000000000000001 RSI: 0000000000000001 RDI: > > > ff3ea01e4265e330 > > > [ 55.141451] RBP: ff59221940833c90 R08: 0000000000000000 R09: > > > 8080808080808080 > > > [ 55.141453] R10: ff3ea01e42b3caf4 R11: 000000000000000f R12: > > > ff3ea01e4265e330 > > > [ 55.141454] R13: 0000000000000001 R14: ff3ea01e505e5e80 R15: > > > 0000000000000001 > > > [ 55.141455] FS: 0000000000000000(0000) > > > GS:ff3ea01fb7cc0000(0000) knlGS:0000000000000000 > > > [ 55.141456] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > [ 55.141458] CR2: 0000563543ad1546 CR3: 000000010ee82005 CR4: > > > 0000000000771ee0 > > > [ 55.141464] DR0: 0000000000000000 DR1: 0000000000000000 DR2: > > > 0000000000000000 > > > [ 55.141465] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: > > > 0000000000000400 > > > [ 55.141466] PKRU: 55555554 > > > [ 55.141467] Call Trace: > > > [ 55.141469] <TASK> > > > [ 55.141472] ? pcie_wait_cmd+0xdf/0x220 > > > [ 55.141478] ? mptcp_seq_show+0xe0/0x180 > > > [ 55.141484] __cancel_work_timer+0x124/0x1b0 > > > [ 55.141487] cancel_delayed_work_sync+0x17/0x20 > > > [ 55.141490] drm_kms_helper_poll_disable+0x26/0x40 > > > [drm_kms_helper] > > > [ 55.141516] drm_mode_config_helper_suspend+0x25/0x90 > > > [drm_kms_helper] > > > [ 55.141531] ? __pm_runtime_resume+0x64/0x90 > > > [ 55.141536] bochs_pm_suspend+0x16/0x20 [bochs] > > > [ 55.141540] pci_pm_suspend+0x8b/0x1b0 > > > [ 55.141545] ? __pfx_pci_pm_suspend+0x10/0x10 > > > [ 55.141547] dpm_run_callback+0x4c/0x160 > > > [ 55.141550] __device_suspend+0x14c/0x4c0 > > > [ 55.141553] async_suspend+0x24/0xa0 > > > [ 55.141555] async_run_entry_fn+0x34/0x120 > > > [ 55.141557] process_one_work+0x21a/0x3f0 > > > [ 55.141560] worker_thread+0x4e/0x3c0 > > > [ 55.141563] ? __pfx_worker_thread+0x10/0x10 > > > [ 55.141565] kthread+0xf2/0x120 > > > [ 55.141568] ? __pfx_kthread+0x10/0x10 > > > [ 55.141570] ret_from_fork+0x29/0x50 > > > [ 55.141575] </TASK> > > > [ 55.141575] ---[ end trace 0000000000000000 ]--- > > > > > > Fixes: a4e771729a51 ("drm/probe_helper: sort out poll_running vs > > > poll_enabled") > > > Signed-off-by: Zongmin Zhou<zhouzongmin@kylinos.cn> > > > --- > > > drivers/gpu/drm/drm_probe_helper.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/drm_probe_helper.c > > > b/drivers/gpu/drm/drm_probe_helper.c > > > index 8127be134c39..ac72b18e2257 100644 > > > --- a/drivers/gpu/drm/drm_probe_helper.c > > > +++ b/drivers/gpu/drm/drm_probe_helper.c > > > @@ -855,7 +855,8 @@ void drm_kms_helper_poll_disable(struct > > > drm_device *dev) > > > if (dev->mode_config.poll_running) > > > drm_kms_helper_disable_hpd(dev); > > > > > > - cancel_delayed_work_sync(&dev- > > > >mode_config.output_poll_work); > > > + if (dev->mode_config.poll_enabled) > > > + cancel_delayed_work_sync(&dev- > > > >mode_config.output_poll_work); > > > > Checking for dev->mode_config.poll_enabled at the start of the > > function > > and return early if it is not true looks more in style with the > > rest of > > drm_probe_helper.c. > > I think it is an error to call drm_kms_helper_poll_disable() if > polling was not initialized. So, in my opinion the fix should go to > the drm_mode_config_helper_suspend() / _resume() instead. Please add > a > guard there using dev->mode_config.poll_enabled. > In my test case,It's ok to check for dev->mode_config.poll_enabled in drm_mode_config_helper_suspend() / _resume() before calling drm_kms_helper_poll_disable().But I find that some device driver call drm_kms_helper_poll_disable()/_enable directly without call _suspend()/_resume() function. So I think the safest way is to add a guard in drm_kms_helper_poll_disable() using dev->mode_config.poll_enabled. I'm looking forward to your comments and Suggestions. Thanks. > > > No difference functionally of course. Tested with the apple kms > > driver. > > > > Reviewed-by: Janne Grunau <j@jannau.net> > > > > ciao > > Janne > > >
On 2023-04-20 23:07:01 +0300, Dmitry Baryshkov wrote: > On Thu, 20 Apr 2023 at 23:01, Janne Grunau <j@jannau.net> wrote: > > > > On 2023-03-28 10:31:29 +0800, Zongmin Zhou wrote: > > > When drivers call drm_kms_helper_poll_disable from > > > their device suspend implementation without enabled output polling before, > > > following warning will be reported,due to work->func not be initialized: > > > > we see the same warning with the wpork in progress kms driver for apple > > silicon SoCs. The connectors do not need to polled so the driver never > > calls drm_kms_helper_poll_init(). > > > > > [ 55.141361] WARNING: CPU: 3 PID: 372 at kernel/workqueue.c:3066 __flush_work+0x22f/0x240 > > > [ 55.141382] Modules linked in: nls_iso8859_1 snd_hda_codec_generic ledtrig_audio snd_hda_intel snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec snd_hda_core snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq intel_rapl_msr intel_rapl_common bochs drm_vram_helper drm_ttm_helper snd_seq_device nfit ttm crct10dif_pclmul snd_timer ghash_clmulni_intel binfmt_misc sha512_ssse3 aesni_intel drm_kms_helper joydev input_leds syscopyarea crypto_simd snd cryptd sysfillrect sysimgblt mac_hid serio_raw soundcore qemu_fw_cfg sch_fq_codel msr parport_pc ppdev lp parport drm ramoops reed_solomon pstore_blk pstore_zone efi_pstore virtio_rng ip_tables x_tables autofs4 hid_generic usbhid hid ahci virtio_net i2c_i801 crc32_pclmul psmouse virtio_scsi libahci i2c_smbus lpc_ich xhci_pci net_failover virtio_blk xhci_pci_renesas failover > > > [ 55.141430] CPU: 3 PID: 372 Comm: kworker/u16:9 Not tainted 6.2.0-rc6+ #16 > > > [ 55.141433] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 > > > [ 55.141435] Workqueue: events_unbound async_run_entry_fn > > > [ 55.141441] RIP: 0010:__flush_work+0x22f/0x240 > > > [ 55.141444] Code: 8b 43 28 48 8b 53 30 89 c1 e9 f9 fe ff ff 4c 89 f7 e8 b5 95 d9 00 e8 00 53 08 00 45 31 ff e9 11 ff ff ff 0f 0b e9 0a ff ff ff <0f> 0b 45 31 ff e9 00 ff ff ff e8 e2 54 d8 00 66 90 90 90 90 90 90 > > > [ 55.141446] RSP: 0018:ff59221940833c18 EFLAGS: 00010246 > > > [ 55.141449] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff9b72bcbe > > > [ 55.141450] RDX: 0000000000000001 RSI: 0000000000000001 RDI: ff3ea01e4265e330 > > > [ 55.141451] RBP: ff59221940833c90 R08: 0000000000000000 R09: 8080808080808080 > > > [ 55.141453] R10: ff3ea01e42b3caf4 R11: 000000000000000f R12: ff3ea01e4265e330 > > > [ 55.141454] R13: 0000000000000001 R14: ff3ea01e505e5e80 R15: 0000000000000001 > > > [ 55.141455] FS: 0000000000000000(0000) GS:ff3ea01fb7cc0000(0000) knlGS:0000000000000000 > > > [ 55.141456] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > [ 55.141458] CR2: 0000563543ad1546 CR3: 000000010ee82005 CR4: 0000000000771ee0 > > > [ 55.141464] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > > [ 55.141465] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > > [ 55.141466] PKRU: 55555554 > > > [ 55.141467] Call Trace: > > > [ 55.141469] <TASK> > > > [ 55.141472] ? pcie_wait_cmd+0xdf/0x220 > > > [ 55.141478] ? mptcp_seq_show+0xe0/0x180 > > > [ 55.141484] __cancel_work_timer+0x124/0x1b0 > > > [ 55.141487] cancel_delayed_work_sync+0x17/0x20 > > > [ 55.141490] drm_kms_helper_poll_disable+0x26/0x40 [drm_kms_helper] > > > [ 55.141516] drm_mode_config_helper_suspend+0x25/0x90 [drm_kms_helper] > > > [ 55.141531] ? __pm_runtime_resume+0x64/0x90 > > > [ 55.141536] bochs_pm_suspend+0x16/0x20 [bochs] > > > [ 55.141540] pci_pm_suspend+0x8b/0x1b0 > > > [ 55.141545] ? __pfx_pci_pm_suspend+0x10/0x10 > > > [ 55.141547] dpm_run_callback+0x4c/0x160 > > > [ 55.141550] __device_suspend+0x14c/0x4c0 > > > [ 55.141553] async_suspend+0x24/0xa0 > > > [ 55.141555] async_run_entry_fn+0x34/0x120 > > > [ 55.141557] process_one_work+0x21a/0x3f0 > > > [ 55.141560] worker_thread+0x4e/0x3c0 > > > [ 55.141563] ? __pfx_worker_thread+0x10/0x10 > > > [ 55.141565] kthread+0xf2/0x120 > > > [ 55.141568] ? __pfx_kthread+0x10/0x10 > > > [ 55.141570] ret_from_fork+0x29/0x50 > > > [ 55.141575] </TASK> > > > [ 55.141575] ---[ end trace 0000000000000000 ]--- > > > > > > Fixes: a4e771729a51 ("drm/probe_helper: sort out poll_running vs poll_enabled") > > > Signed-off-by: Zongmin Zhou<zhouzongmin@kylinos.cn> > > > --- > > > drivers/gpu/drm/drm_probe_helper.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > > > index 8127be134c39..ac72b18e2257 100644 > > > --- a/drivers/gpu/drm/drm_probe_helper.c > > > +++ b/drivers/gpu/drm/drm_probe_helper.c > > > @@ -855,7 +855,8 @@ void drm_kms_helper_poll_disable(struct drm_device *dev) > > > if (dev->mode_config.poll_running) > > > drm_kms_helper_disable_hpd(dev); > > > > > > - cancel_delayed_work_sync(&dev->mode_config.output_poll_work); > > > + if (dev->mode_config.poll_enabled) > > > + cancel_delayed_work_sync(&dev->mode_config.output_poll_work); > > > > Checking for dev->mode_config.poll_enabled at the start of the function > > and return early if it is not true looks more in style with the rest of > > drm_probe_helper.c. > > I think it is an error to call drm_kms_helper_poll_disable() if > polling was not initialized. So, in my opinion the fix should go to > the drm_mode_config_helper_suspend() / _resume() instead. Please add a > guard there using dev->mode_config.poll_enabled. While I tend to agree to the sentiment I do not think this is the correct fix in this situation. drm_kms_helper_poll_disable had the check since at least 2014. a4e771729a51 is a regression. If we want to change the behavior it should be done explicitly and after verifying all drm_kms_helper_poll_disable() calls. #regzbot ^introduced a4e771729a51 ciao Janne
On Sun, 2023-04-23 at 22:51 +0200, Janne Grunau wrote: > On 2023-04-20 23:07:01 +0300, Dmitry Baryshkov wrote: > > On Thu, 20 Apr 2023 at 23:01, Janne Grunau <j@jannau.net> wrote: > > > > > > On 2023-03-28 10:31:29 +0800, Zongmin Zhou wrote: > > > > When drivers call drm_kms_helper_poll_disable from > > > > their device suspend implementation without enabled output > > > > polling before, > > > > following warning will be reported,due to work->func not be > > > > initialized: > > > > > > we see the same warning with the wpork in progress kms driver for > > > apple > > > silicon SoCs. The connectors do not need to polled so the driver > > > never > > > calls drm_kms_helper_poll_init(). > > > > > > > [ 55.141361] WARNING: CPU: 3 PID: 372 at > > > > kernel/workqueue.c:3066 __flush_work+0x22f/0x240 > > > > [ 55.141382] Modules linked in: nls_iso8859_1 > > > > snd_hda_codec_generic ledtrig_audio snd_hda_intel > > > > snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec snd_hda_core > > > > snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi > > > > snd_seq intel_rapl_msr intel_rapl_common bochs drm_vram_helper > > > > drm_ttm_helper snd_seq_device nfit ttm crct10dif_pclmul > > > > snd_timer ghash_clmulni_intel binfmt_misc sha512_ssse3 > > > > aesni_intel drm_kms_helper joydev input_leds syscopyarea > > > > crypto_simd snd cryptd sysfillrect sysimgblt mac_hid serio_raw > > > > soundcore qemu_fw_cfg sch_fq_codel msr parport_pc ppdev lp > > > > parport drm ramoops reed_solomon pstore_blk pstore_zone > > > > efi_pstore virtio_rng ip_tables x_tables autofs4 hid_generic > > > > usbhid hid ahci virtio_net i2c_i801 crc32_pclmul psmouse > > > > virtio_scsi libahci i2c_smbus lpc_ich xhci_pci net_failover > > > > virtio_blk xhci_pci_renesas failover > > > > [ 55.141430] CPU: 3 PID: 372 Comm: kworker/u16:9 Not tainted > > > > 6.2.0-rc6+ #16 > > > > [ 55.141433] Hardware name: QEMU Standard PC (Q35 + ICH9, > > > > 2009), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org > > > > 04/01/2014 > > > > [ 55.141435] Workqueue: events_unbound async_run_entry_fn > > > > [ 55.141441] RIP: 0010:__flush_work+0x22f/0x240 > > > > [ 55.141444] Code: 8b 43 28 48 8b 53 30 89 c1 e9 f9 fe ff ff > > > > 4c 89 f7 e8 b5 95 d9 00 e8 00 53 08 00 45 31 ff e9 11 ff ff ff > > > > 0f 0b e9 0a ff ff ff <0f> 0b 45 31 ff e9 00 ff ff ff e8 e2 54 > > > > d8 00 66 90 90 90 90 90 90 > > > > [ 55.141446] RSP: 0018:ff59221940833c18 EFLAGS: 00010246 > > > > [ 55.141449] RAX: 0000000000000000 RBX: 0000000000000000 RCX: > > > > ffffffff9b72bcbe > > > > [ 55.141450] RDX: 0000000000000001 RSI: 0000000000000001 RDI: > > > > ff3ea01e4265e330 > > > > [ 55.141451] RBP: ff59221940833c90 R08: 0000000000000000 R09: > > > > 8080808080808080 > > > > [ 55.141453] R10: ff3ea01e42b3caf4 R11: 000000000000000f R12: > > > > ff3ea01e4265e330 > > > > [ 55.141454] R13: 0000000000000001 R14: ff3ea01e505e5e80 R15: > > > > 0000000000000001 > > > > [ 55.141455] FS: 0000000000000000(0000) > > > > GS:ff3ea01fb7cc0000(0000) knlGS:0000000000000000 > > > > [ 55.141456] CS: 0010 DS: 0000 ES: 0000 CR0: > > > > 0000000080050033 > > > > [ 55.141458] CR2: 0000563543ad1546 CR3: 000000010ee82005 CR4: > > > > 0000000000771ee0 > > > > [ 55.141464] DR0: 0000000000000000 DR1: 0000000000000000 DR2: > > > > 0000000000000000 > > > > [ 55.141465] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: > > > > 0000000000000400 > > > > [ 55.141466] PKRU: 55555554 > > > > [ 55.141467] Call Trace: > > > > [ 55.141469] <TASK> > > > > [ 55.141472] ? pcie_wait_cmd+0xdf/0x220 > > > > [ 55.141478] ? mptcp_seq_show+0xe0/0x180 > > > > [ 55.141484] __cancel_work_timer+0x124/0x1b0 > > > > [ 55.141487] cancel_delayed_work_sync+0x17/0x20 > > > > [ 55.141490] drm_kms_helper_poll_disable+0x26/0x40 > > > > [drm_kms_helper] > > > > [ 55.141516] drm_mode_config_helper_suspend+0x25/0x90 > > > > [drm_kms_helper] > > > > [ 55.141531] ? __pm_runtime_resume+0x64/0x90 > > > > [ 55.141536] bochs_pm_suspend+0x16/0x20 [bochs] > > > > [ 55.141540] pci_pm_suspend+0x8b/0x1b0 > > > > [ 55.141545] ? __pfx_pci_pm_suspend+0x10/0x10 > > > > [ 55.141547] dpm_run_callback+0x4c/0x160 > > > > [ 55.141550] __device_suspend+0x14c/0x4c0 > > > > [ 55.141553] async_suspend+0x24/0xa0 > > > > [ 55.141555] async_run_entry_fn+0x34/0x120 > > > > [ 55.141557] process_one_work+0x21a/0x3f0 > > > > [ 55.141560] worker_thread+0x4e/0x3c0 > > > > [ 55.141563] ? __pfx_worker_thread+0x10/0x10 > > > > [ 55.141565] kthread+0xf2/0x120 > > > > [ 55.141568] ? __pfx_kthread+0x10/0x10 > > > > [ 55.141570] ret_from_fork+0x29/0x50 > > > > [ 55.141575] </TASK> > > > > [ 55.141575] ---[ end trace 0000000000000000 ]--- > > > > > > > > Fixes: a4e771729a51 ("drm/probe_helper: sort out poll_running > > > > vs poll_enabled") > > > > Signed-off-by: Zongmin Zhou<zhouzongmin@kylinos.cn> > > > > --- > > > > drivers/gpu/drm/drm_probe_helper.c | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_probe_helper.c > > > > b/drivers/gpu/drm/drm_probe_helper.c > > > > index 8127be134c39..ac72b18e2257 100644 > > > > --- a/drivers/gpu/drm/drm_probe_helper.c > > > > +++ b/drivers/gpu/drm/drm_probe_helper.c > > > > @@ -855,7 +855,8 @@ void drm_kms_helper_poll_disable(struct > > > > drm_device *dev) > > > > if (dev->mode_config.poll_running) > > > > drm_kms_helper_disable_hpd(dev); > > > > > > > > - cancel_delayed_work_sync(&dev- > > > > >mode_config.output_poll_work); > > > > + if (dev->mode_config.poll_enabled) > > > > + cancel_delayed_work_sync(&dev- > > > > >mode_config.output_poll_work); > > > > > > Checking for dev->mode_config.poll_enabled at the start of the > > > function > > > and return early if it is not true looks more in style with the > > > rest of > > > drm_probe_helper.c. > > > > I think it is an error to call drm_kms_helper_poll_disable() if > > polling was not initialized. So, in my opinion the fix should go to > > the drm_mode_config_helper_suspend() / _resume() instead. Please > > add a > > guard there using dev->mode_config.poll_enabled. > > While I tend to agree to the sentiment I do not think this is the > correct fix in this situation. drm_kms_helper_poll_disable had the > check since at least 2014. a4e771729a51 is a regression. If we want > to > change the behavior it should be done explicitly and after verifying > all > drm_kms_helper_poll_disable() calls. > > #regzbot ^introduced a4e771729a51 > > ciao > Janne Dear Janne: I agree with you like I mentioned on last letter. Thanks for your time. Dear Dmitry: Is there anything else I can do? Looking forward to your reply. Thanks
On Wed, 26 Apr 2023 at 12:09, zongmin zhou <zhouzongmin@kylinos.cn> wrote: > > On Sun, 2023-04-23 at 22:51 +0200, Janne Grunau wrote: > > On 2023-04-20 23:07:01 +0300, Dmitry Baryshkov wrote: > > > On Thu, 20 Apr 2023 at 23:01, Janne Grunau <j@jannau.net> wrote: > > > > > > > > On 2023-03-28 10:31:29 +0800, Zongmin Zhou wrote: > > > > > When drivers call drm_kms_helper_poll_disable from > > > > > their device suspend implementation without enabled output > > > > > polling before, > > > > > following warning will be reported,due to work->func not be > > > > > initialized: > > > > > > > > we see the same warning with the wpork in progress kms driver for > > > > apple > > > > silicon SoCs. The connectors do not need to polled so the driver > > > > never > > > > calls drm_kms_helper_poll_init(). > > > > > > > > > [ 55.141361] WARNING: CPU: 3 PID: 372 at > > > > > kernel/workqueue.c:3066 __flush_work+0x22f/0x240 > > > > > [ 55.141382] Modules linked in: nls_iso8859_1 > > > > > snd_hda_codec_generic ledtrig_audio snd_hda_intel > > > > > snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec snd_hda_core > > > > > snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi > > > > > snd_seq intel_rapl_msr intel_rapl_common bochs drm_vram_helper > > > > > drm_ttm_helper snd_seq_device nfit ttm crct10dif_pclmul > > > > > snd_timer ghash_clmulni_intel binfmt_misc sha512_ssse3 > > > > > aesni_intel drm_kms_helper joydev input_leds syscopyarea > > > > > crypto_simd snd cryptd sysfillrect sysimgblt mac_hid serio_raw > > > > > soundcore qemu_fw_cfg sch_fq_codel msr parport_pc ppdev lp > > > > > parport drm ramoops reed_solomon pstore_blk pstore_zone > > > > > efi_pstore virtio_rng ip_tables x_tables autofs4 hid_generic > > > > > usbhid hid ahci virtio_net i2c_i801 crc32_pclmul psmouse > > > > > virtio_scsi libahci i2c_smbus lpc_ich xhci_pci net_failover > > > > > virtio_blk xhci_pci_renesas failover > > > > > [ 55.141430] CPU: 3 PID: 372 Comm: kworker/u16:9 Not tainted > > > > > 6.2.0-rc6+ #16 > > > > > [ 55.141433] Hardware name: QEMU Standard PC (Q35 + ICH9, > > > > > 2009), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org > > > > > 04/01/2014 > > > > > [ 55.141435] Workqueue: events_unbound async_run_entry_fn > > > > > [ 55.141441] RIP: 0010:__flush_work+0x22f/0x240 > > > > > [ 55.141444] Code: 8b 43 28 48 8b 53 30 89 c1 e9 f9 fe ff ff > > > > > 4c 89 f7 e8 b5 95 d9 00 e8 00 53 08 00 45 31 ff e9 11 ff ff ff > > > > > 0f 0b e9 0a ff ff ff <0f> 0b 45 31 ff e9 00 ff ff ff e8 e2 54 > > > > > d8 00 66 90 90 90 90 90 90 > > > > > [ 55.141446] RSP: 0018:ff59221940833c18 EFLAGS: 00010246 > > > > > [ 55.141449] RAX: 0000000000000000 RBX: 0000000000000000 RCX: > > > > > ffffffff9b72bcbe > > > > > [ 55.141450] RDX: 0000000000000001 RSI: 0000000000000001 RDI: > > > > > ff3ea01e4265e330 > > > > > [ 55.141451] RBP: ff59221940833c90 R08: 0000000000000000 R09: > > > > > 8080808080808080 > > > > > [ 55.141453] R10: ff3ea01e42b3caf4 R11: 000000000000000f R12: > > > > > ff3ea01e4265e330 > > > > > [ 55.141454] R13: 0000000000000001 R14: ff3ea01e505e5e80 R15: > > > > > 0000000000000001 > > > > > [ 55.141455] FS: 0000000000000000(0000) > > > > > GS:ff3ea01fb7cc0000(0000) knlGS:0000000000000000 > > > > > [ 55.141456] CS: 0010 DS: 0000 ES: 0000 CR0: > > > > > 0000000080050033 > > > > > [ 55.141458] CR2: 0000563543ad1546 CR3: 000000010ee82005 CR4: > > > > > 0000000000771ee0 > > > > > [ 55.141464] DR0: 0000000000000000 DR1: 0000000000000000 DR2: > > > > > 0000000000000000 > > > > > [ 55.141465] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: > > > > > 0000000000000400 > > > > > [ 55.141466] PKRU: 55555554 > > > > > [ 55.141467] Call Trace: > > > > > [ 55.141469] <TASK> > > > > > [ 55.141472] ? pcie_wait_cmd+0xdf/0x220 > > > > > [ 55.141478] ? mptcp_seq_show+0xe0/0x180 > > > > > [ 55.141484] __cancel_work_timer+0x124/0x1b0 > > > > > [ 55.141487] cancel_delayed_work_sync+0x17/0x20 > > > > > [ 55.141490] drm_kms_helper_poll_disable+0x26/0x40 > > > > > [drm_kms_helper] > > > > > [ 55.141516] drm_mode_config_helper_suspend+0x25/0x90 > > > > > [drm_kms_helper] > > > > > [ 55.141531] ? __pm_runtime_resume+0x64/0x90 > > > > > [ 55.141536] bochs_pm_suspend+0x16/0x20 [bochs] > > > > > [ 55.141540] pci_pm_suspend+0x8b/0x1b0 > > > > > [ 55.141545] ? __pfx_pci_pm_suspend+0x10/0x10 > > > > > [ 55.141547] dpm_run_callback+0x4c/0x160 > > > > > [ 55.141550] __device_suspend+0x14c/0x4c0 > > > > > [ 55.141553] async_suspend+0x24/0xa0 > > > > > [ 55.141555] async_run_entry_fn+0x34/0x120 > > > > > [ 55.141557] process_one_work+0x21a/0x3f0 > > > > > [ 55.141560] worker_thread+0x4e/0x3c0 > > > > > [ 55.141563] ? __pfx_worker_thread+0x10/0x10 > > > > > [ 55.141565] kthread+0xf2/0x120 > > > > > [ 55.141568] ? __pfx_kthread+0x10/0x10 > > > > > [ 55.141570] ret_from_fork+0x29/0x50 > > > > > [ 55.141575] </TASK> > > > > > [ 55.141575] ---[ end trace 0000000000000000 ]--- > > > > > > > > > > Fixes: a4e771729a51 ("drm/probe_helper: sort out poll_running > > > > > vs poll_enabled") > > > > > Signed-off-by: Zongmin Zhou<zhouzongmin@kylinos.cn> > > > > > --- > > > > > drivers/gpu/drm/drm_probe_helper.c | 3 ++- > > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_probe_helper.c > > > > > b/drivers/gpu/drm/drm_probe_helper.c > > > > > index 8127be134c39..ac72b18e2257 100644 > > > > > --- a/drivers/gpu/drm/drm_probe_helper.c > > > > > +++ b/drivers/gpu/drm/drm_probe_helper.c > > > > > @@ -855,7 +855,8 @@ void drm_kms_helper_poll_disable(struct > > > > > drm_device *dev) > > > > > if (dev->mode_config.poll_running) > > > > > drm_kms_helper_disable_hpd(dev); > > > > > > > > > > - cancel_delayed_work_sync(&dev- > > > > > >mode_config.output_poll_work); > > > > > + if (dev->mode_config.poll_enabled) > > > > > + cancel_delayed_work_sync(&dev- > > > > > >mode_config.output_poll_work); > > > > > > > > Checking for dev->mode_config.poll_enabled at the start of the > > > > function > > > > and return early if it is not true looks more in style with the > > > > rest of > > > > drm_probe_helper.c. > > > > > > I think it is an error to call drm_kms_helper_poll_disable() if > > > polling was not initialized. So, in my opinion the fix should go to > > > the drm_mode_config_helper_suspend() / _resume() instead. Please > > > add a > > > guard there using dev->mode_config.poll_enabled. > > > > While I tend to agree to the sentiment I do not think this is the > > correct fix in this situation. drm_kms_helper_poll_disable had the > > check since at least 2014. a4e771729a51 is a regression. If we want > > to > > change the behavior it should be done explicitly and after verifying > > all > > drm_kms_helper_poll_disable() calls. > > > > #regzbot ^introduced a4e771729a51 > > > > ciao > > Janne > > Dear Janne: > > I agree with you like I mentioned on last letter. > Thanks for your time. > > > Dear Dmitry: > > Is there anything else I can do? > Looking forward to your reply. If it is a common consensus, I'm fine with your approach.
On Wed, 2023-04-26 at 16:10 +0300, Dmitry Baryshkov wrote: > On Wed, 26 Apr 2023 at 12:09, zongmin zhou <zhouzongmin@kylinos.cn> > wrote: > > > > On Sun, 2023-04-23 at 22:51 +0200, Janne Grunau wrote: > > > On 2023-04-20 23:07:01 +0300, Dmitry Baryshkov wrote: > > > > On Thu, 20 Apr 2023 at 23:01, Janne Grunau <j@jannau.net> > > > > wrote: > > > > > > > > > > On 2023-03-28 10:31:29 +0800, Zongmin Zhou wrote: > > > > > > When drivers call drm_kms_helper_poll_disable from > > > > > > their device suspend implementation without enabled output > > > > > > polling before, > > > > > > following warning will be reported,due to work->func not be > > > > > > initialized: > > > > > > > > > > we see the same warning with the wpork in progress kms driver > > > > > for > > > > > apple > > > > > silicon SoCs. The connectors do not need to polled so the > > > > > driver > > > > > never > > > > > calls drm_kms_helper_poll_init(). > > > > > > > > > > > [ 55.141361] WARNING: CPU: 3 PID: 372 at > > > > > > kernel/workqueue.c:3066 __flush_work+0x22f/0x240 > > > > > > [ 55.141382] Modules linked in: nls_iso8859_1 > > > > > > snd_hda_codec_generic ledtrig_audio snd_hda_intel > > > > > > snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec > > > > > > snd_hda_core > > > > > > snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event > > > > > > snd_rawmidi > > > > > > snd_seq intel_rapl_msr intel_rapl_common bochs > > > > > > drm_vram_helper > > > > > > drm_ttm_helper snd_seq_device nfit ttm crct10dif_pclmul > > > > > > snd_timer ghash_clmulni_intel binfmt_misc sha512_ssse3 > > > > > > aesni_intel drm_kms_helper joydev input_leds syscopyarea > > > > > > crypto_simd snd cryptd sysfillrect sysimgblt mac_hid > > > > > > serio_raw > > > > > > soundcore qemu_fw_cfg sch_fq_codel msr parport_pc ppdev lp > > > > > > parport drm ramoops reed_solomon pstore_blk pstore_zone > > > > > > efi_pstore virtio_rng ip_tables x_tables autofs4 > > > > > > hid_generic > > > > > > usbhid hid ahci virtio_net i2c_i801 crc32_pclmul psmouse > > > > > > virtio_scsi libahci i2c_smbus lpc_ich xhci_pci net_failover > > > > > > virtio_blk xhci_pci_renesas failover > > > > > > [ 55.141430] CPU: 3 PID: 372 Comm: kworker/u16:9 Not > > > > > > tainted > > > > > > 6.2.0-rc6+ #16 > > > > > > [ 55.141433] Hardware name: QEMU Standard PC (Q35 + ICH9, > > > > > > 2009), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org > > > > > > 04/01/2014 > > > > > > [ 55.141435] Workqueue: events_unbound async_run_entry_fn > > > > > > [ 55.141441] RIP: 0010:__flush_work+0x22f/0x240 > > > > > > [ 55.141444] Code: 8b 43 28 48 8b 53 30 89 c1 e9 f9 fe ff > > > > > > ff > > > > > > 4c 89 f7 e8 b5 95 d9 00 e8 00 53 08 00 45 31 ff e9 11 ff ff > > > > > > ff > > > > > > 0f 0b e9 0a ff ff ff <0f> 0b 45 31 ff e9 00 ff ff ff e8 e2 > > > > > > 54 > > > > > > d8 00 66 90 90 90 90 90 90 > > > > > > [ 55.141446] RSP: 0018:ff59221940833c18 EFLAGS: 00010246 > > > > > > [ 55.141449] RAX: 0000000000000000 RBX: 0000000000000000 > > > > > > RCX: > > > > > > ffffffff9b72bcbe > > > > > > [ 55.141450] RDX: 0000000000000001 RSI: 0000000000000001 > > > > > > RDI: > > > > > > ff3ea01e4265e330 > > > > > > [ 55.141451] RBP: ff59221940833c90 R08: 0000000000000000 > > > > > > R09: > > > > > > 8080808080808080 > > > > > > [ 55.141453] R10: ff3ea01e42b3caf4 R11: 000000000000000f > > > > > > R12: > > > > > > ff3ea01e4265e330 > > > > > > [ 55.141454] R13: 0000000000000001 R14: ff3ea01e505e5e80 > > > > > > R15: > > > > > > 0000000000000001 > > > > > > [ 55.141455] FS: 0000000000000000(0000) > > > > > > GS:ff3ea01fb7cc0000(0000) knlGS:0000000000000000 > > > > > > [ 55.141456] CS: 0010 DS: 0000 ES: 0000 CR0: > > > > > > 0000000080050033 > > > > > > [ 55.141458] CR2: 0000563543ad1546 CR3: 000000010ee82005 > > > > > > CR4: > > > > > > 0000000000771ee0 > > > > > > [ 55.141464] DR0: 0000000000000000 DR1: 0000000000000000 > > > > > > DR2: > > > > > > 0000000000000000 > > > > > > [ 55.141465] DR3: 0000000000000000 DR6: 00000000fffe0ff0 > > > > > > DR7: > > > > > > 0000000000000400 > > > > > > [ 55.141466] PKRU: 55555554 > > > > > > [ 55.141467] Call Trace: > > > > > > [ 55.141469] <TASK> > > > > > > [ 55.141472] ? pcie_wait_cmd+0xdf/0x220 > > > > > > [ 55.141478] ? mptcp_seq_show+0xe0/0x180 > > > > > > [ 55.141484] __cancel_work_timer+0x124/0x1b0 > > > > > > [ 55.141487] cancel_delayed_work_sync+0x17/0x20 > > > > > > [ 55.141490] drm_kms_helper_poll_disable+0x26/0x40 > > > > > > [drm_kms_helper] > > > > > > [ 55.141516] drm_mode_config_helper_suspend+0x25/0x90 > > > > > > [drm_kms_helper] > > > > > > [ 55.141531] ? __pm_runtime_resume+0x64/0x90 > > > > > > [ 55.141536] bochs_pm_suspend+0x16/0x20 [bochs] > > > > > > [ 55.141540] pci_pm_suspend+0x8b/0x1b0 > > > > > > [ 55.141545] ? __pfx_pci_pm_suspend+0x10/0x10 > > > > > > [ 55.141547] dpm_run_callback+0x4c/0x160 > > > > > > [ 55.141550] __device_suspend+0x14c/0x4c0 > > > > > > [ 55.141553] async_suspend+0x24/0xa0 > > > > > > [ 55.141555] async_run_entry_fn+0x34/0x120 > > > > > > [ 55.141557] process_one_work+0x21a/0x3f0 > > > > > > [ 55.141560] worker_thread+0x4e/0x3c0 > > > > > > [ 55.141563] ? __pfx_worker_thread+0x10/0x10 > > > > > > [ 55.141565] kthread+0xf2/0x120 > > > > > > [ 55.141568] ? __pfx_kthread+0x10/0x10 > > > > > > [ 55.141570] ret_from_fork+0x29/0x50 > > > > > > [ 55.141575] </TASK> > > > > > > [ 55.141575] ---[ end trace 0000000000000000 ]--- > > > > > > > > > > > > Fixes: a4e771729a51 ("drm/probe_helper: sort out > > > > > > poll_running > > > > > > vs poll_enabled") > > > > > > Signed-off-by: Zongmin Zhou<zhouzongmin@kylinos.cn> > > > > > > --- > > > > > > drivers/gpu/drm/drm_probe_helper.c | 3 ++- > > > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_probe_helper.c > > > > > > b/drivers/gpu/drm/drm_probe_helper.c > > > > > > index 8127be134c39..ac72b18e2257 100644 > > > > > > --- a/drivers/gpu/drm/drm_probe_helper.c > > > > > > +++ b/drivers/gpu/drm/drm_probe_helper.c > > > > > > @@ -855,7 +855,8 @@ void drm_kms_helper_poll_disable(struct > > > > > > drm_device *dev) > > > > > > if (dev->mode_config.poll_running) > > > > > > drm_kms_helper_disable_hpd(dev); > > > > > > > > > > > > - cancel_delayed_work_sync(&dev- > > > > > > > mode_config.output_poll_work); > > > > > > + if (dev->mode_config.poll_enabled) > > > > > > + cancel_delayed_work_sync(&dev- > > > > > > > mode_config.output_poll_work); > > > > > > > > > > Checking for dev->mode_config.poll_enabled at the start of > > > > > the > > > > > function > > > > > and return early if it is not true looks more in style with > > > > > the > > > > > rest of > > > > > drm_probe_helper.c. > > > > > > > > I think it is an error to call drm_kms_helper_poll_disable() if > > > > polling was not initialized. So, in my opinion the fix should > > > > go to > > > > the drm_mode_config_helper_suspend() / _resume() instead. > > > > Please > > > > add a > > > > guard there using dev->mode_config.poll_enabled. > > > > > > While I tend to agree to the sentiment I do not think this is the > > > correct fix in this situation. drm_kms_helper_poll_disable had > > > the > > > check since at least 2014. a4e771729a51 is a regression. If we > > > want > > > to > > > change the behavior it should be done explicitly and after > > > verifying > > > all > > > drm_kms_helper_poll_disable() calls. > > > > > > #regzbot ^introduced a4e771729a51 > > > > > > ciao > > > Janne > > > > Dear Janne: > > > > I agree with you like I mentioned on last letter. > > Thanks for your time. > > > > > > Dear Dmitry: > > > > Is there anything else I can do? > > Looking forward to your reply. > > If it is a common consensus, I'm fine with your approach. > Dear Dmitry: Ok.Thanks for your reply. Best regards!
Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting for once, to make this easily accessible to everyone. Dmitry, was any progress made to address this regression? Doesn't look like it, but I strongly suspect I'm missing something, as I'm not really sure if I properly understood this thread. It sounded a bit like a4e771729a51 should be reverted for now until all drm_kms_helper_poll_disable() calls have been verified. Is that right? Or did somebody already verify and fix all of them with bugs? Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. #regzbot poke On 28.04.23 03:17, zongmin zhou wrote: > On Wed, 2023-04-26 at 16:10 +0300, Dmitry Baryshkov wrote: >> On Wed, 26 Apr 2023 at 12:09, zongmin zhou <zhouzongmin@kylinos.cn> >> wrote: >>> On Sun, 2023-04-23 at 22:51 +0200, Janne Grunau wrote: >>>> On 2023-04-20 23:07:01 +0300, Dmitry Baryshkov wrote: >>>>> On Thu, 20 Apr 2023 at 23:01, Janne Grunau <j@jannau.net> >>>>> wrote: >>>>>> >>>>>> On 2023-03-28 10:31:29 +0800, Zongmin Zhou wrote: >>>>>>> When drivers call drm_kms_helper_poll_disable from >>>>>>> their device suspend implementation without enabled output >>>>>>> polling before, >>>>>>> following warning will be reported,due to work->func not be >>>>>>> initialized: >>>>>> >>>>>> we see the same warning with the wpork in progress kms driver >>>>>> for >>>>>> apple >>>>>> silicon SoCs. The connectors do not need to polled so the >>>>>> driver >>>>>> never >>>>>> calls drm_kms_helper_poll_init(). >>>>>> >>>>>>> [ 55.141361] WARNING: CPU: 3 PID: 372 at >>>>>>> kernel/workqueue.c:3066 __flush_work+0x22f/0x240 >>>>>>> [ 55.141382] Modules linked in: nls_iso8859_1 >>>>>>> snd_hda_codec_generic ledtrig_audio snd_hda_intel >>>>>>> snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec >>>>>>> snd_hda_core >>>>>>> snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event >>>>>>> snd_rawmidi >>>>>>> snd_seq intel_rapl_msr intel_rapl_common bochs >>>>>>> drm_vram_helper >>>>>>> drm_ttm_helper snd_seq_device nfit ttm crct10dif_pclmul >>>>>>> snd_timer ghash_clmulni_intel binfmt_misc sha512_ssse3 >>>>>>> aesni_intel drm_kms_helper joydev input_leds syscopyarea >>>>>>> crypto_simd snd cryptd sysfillrect sysimgblt mac_hid >>>>>>> serio_raw >>>>>>> soundcore qemu_fw_cfg sch_fq_codel msr parport_pc ppdev lp >>>>>>> parport drm ramoops reed_solomon pstore_blk pstore_zone >>>>>>> efi_pstore virtio_rng ip_tables x_tables autofs4 >>>>>>> hid_generic >>>>>>> usbhid hid ahci virtio_net i2c_i801 crc32_pclmul psmouse >>>>>>> virtio_scsi libahci i2c_smbus lpc_ich xhci_pci net_failover >>>>>>> virtio_blk xhci_pci_renesas failover >>>>>>> [ 55.141430] CPU: 3 PID: 372 Comm: kworker/u16:9 Not >>>>>>> tainted >>>>>>> 6.2.0-rc6+ #16 >>>>>>> [ 55.141433] Hardware name: QEMU Standard PC (Q35 + ICH9, >>>>>>> 2009), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org >>>>>>> 04/01/2014 >>>>>>> [ 55.141435] Workqueue: events_unbound async_run_entry_fn >>>>>>> [ 55.141441] RIP: 0010:__flush_work+0x22f/0x240 >>>>>>> [ 55.141444] Code: 8b 43 28 48 8b 53 30 89 c1 e9 f9 fe ff >>>>>>> ff >>>>>>> 4c 89 f7 e8 b5 95 d9 00 e8 00 53 08 00 45 31 ff e9 11 ff ff >>>>>>> ff >>>>>>> 0f 0b e9 0a ff ff ff <0f> 0b 45 31 ff e9 00 ff ff ff e8 e2 >>>>>>> 54 >>>>>>> d8 00 66 90 90 90 90 90 90 >>>>>>> [ 55.141446] RSP: 0018:ff59221940833c18 EFLAGS: 00010246 >>>>>>> [ 55.141449] RAX: 0000000000000000 RBX: 0000000000000000 >>>>>>> RCX: >>>>>>> ffffffff9b72bcbe >>>>>>> [ 55.141450] RDX: 0000000000000001 RSI: 0000000000000001 >>>>>>> RDI: >>>>>>> ff3ea01e4265e330 >>>>>>> [ 55.141451] RBP: ff59221940833c90 R08: 0000000000000000 >>>>>>> R09: >>>>>>> 8080808080808080 >>>>>>> [ 55.141453] R10: ff3ea01e42b3caf4 R11: 000000000000000f >>>>>>> R12: >>>>>>> ff3ea01e4265e330 >>>>>>> [ 55.141454] R13: 0000000000000001 R14: ff3ea01e505e5e80 >>>>>>> R15: >>>>>>> 0000000000000001 >>>>>>> [ 55.141455] FS: 0000000000000000(0000) >>>>>>> GS:ff3ea01fb7cc0000(0000) knlGS:0000000000000000 >>>>>>> [ 55.141456] CS: 0010 DS: 0000 ES: 0000 CR0: >>>>>>> 0000000080050033 >>>>>>> [ 55.141458] CR2: 0000563543ad1546 CR3: 000000010ee82005 >>>>>>> CR4: >>>>>>> 0000000000771ee0 >>>>>>> [ 55.141464] DR0: 0000000000000000 DR1: 0000000000000000 >>>>>>> DR2: >>>>>>> 0000000000000000 >>>>>>> [ 55.141465] DR3: 0000000000000000 DR6: 00000000fffe0ff0 >>>>>>> DR7: >>>>>>> 0000000000000400 >>>>>>> [ 55.141466] PKRU: 55555554 >>>>>>> [ 55.141467] Call Trace: >>>>>>> [ 55.141469] <TASK> >>>>>>> [ 55.141472] ? pcie_wait_cmd+0xdf/0x220 >>>>>>> [ 55.141478] ? mptcp_seq_show+0xe0/0x180 >>>>>>> [ 55.141484] __cancel_work_timer+0x124/0x1b0 >>>>>>> [ 55.141487] cancel_delayed_work_sync+0x17/0x20 >>>>>>> [ 55.141490] drm_kms_helper_poll_disable+0x26/0x40 >>>>>>> [drm_kms_helper] >>>>>>> [ 55.141516] drm_mode_config_helper_suspend+0x25/0x90 >>>>>>> [drm_kms_helper] >>>>>>> [ 55.141531] ? __pm_runtime_resume+0x64/0x90 >>>>>>> [ 55.141536] bochs_pm_suspend+0x16/0x20 [bochs] >>>>>>> [ 55.141540] pci_pm_suspend+0x8b/0x1b0 >>>>>>> [ 55.141545] ? __pfx_pci_pm_suspend+0x10/0x10 >>>>>>> [ 55.141547] dpm_run_callback+0x4c/0x160 >>>>>>> [ 55.141550] __device_suspend+0x14c/0x4c0 >>>>>>> [ 55.141553] async_suspend+0x24/0xa0 >>>>>>> [ 55.141555] async_run_entry_fn+0x34/0x120 >>>>>>> [ 55.141557] process_one_work+0x21a/0x3f0 >>>>>>> [ 55.141560] worker_thread+0x4e/0x3c0 >>>>>>> [ 55.141563] ? __pfx_worker_thread+0x10/0x10 >>>>>>> [ 55.141565] kthread+0xf2/0x120 >>>>>>> [ 55.141568] ? __pfx_kthread+0x10/0x10 >>>>>>> [ 55.141570] ret_from_fork+0x29/0x50 >>>>>>> [ 55.141575] </TASK> >>>>>>> [ 55.141575] ---[ end trace 0000000000000000 ]--- >>>>>>> >>>>>>> Fixes: a4e771729a51 ("drm/probe_helper: sort out >>>>>>> poll_running >>>>>>> vs poll_enabled") >>>>>>> Signed-off-by: Zongmin Zhou<zhouzongmin@kylinos.cn> >>>>>>> --- >>>>>>> drivers/gpu/drm/drm_probe_helper.c | 3 ++- >>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/drm_probe_helper.c >>>>>>> b/drivers/gpu/drm/drm_probe_helper.c >>>>>>> index 8127be134c39..ac72b18e2257 100644 >>>>>>> --- a/drivers/gpu/drm/drm_probe_helper.c >>>>>>> +++ b/drivers/gpu/drm/drm_probe_helper.c >>>>>>> @@ -855,7 +855,8 @@ void drm_kms_helper_poll_disable(struct >>>>>>> drm_device *dev) >>>>>>> if (dev->mode_config.poll_running) >>>>>>> drm_kms_helper_disable_hpd(dev); >>>>>>> >>>>>>> - cancel_delayed_work_sync(&dev- >>>>>>>> mode_config.output_poll_work); >>>>>>> + if (dev->mode_config.poll_enabled) >>>>>>> + cancel_delayed_work_sync(&dev- >>>>>>>> mode_config.output_poll_work); >>>>>> >>>>>> Checking for dev->mode_config.poll_enabled at the start of >>>>>> the >>>>>> function >>>>>> and return early if it is not true looks more in style with >>>>>> the >>>>>> rest of >>>>>> drm_probe_helper.c. >>>>> >>>>> I think it is an error to call drm_kms_helper_poll_disable() if >>>>> polling was not initialized. So, in my opinion the fix should >>>>> go to >>>>> the drm_mode_config_helper_suspend() / _resume() instead. >>>>> Please >>>>> add a >>>>> guard there using dev->mode_config.poll_enabled. >>>> >>>> While I tend to agree to the sentiment I do not think this is the >>>> correct fix in this situation. drm_kms_helper_poll_disable had >>>> the >>>> check since at least 2014. a4e771729a51 is a regression. If we >>>> want >>>> to >>>> change the behavior it should be done explicitly and after >>>> verifying >>>> all >>>> drm_kms_helper_poll_disable() calls. >>>> >>>> #regzbot ^introduced a4e771729a51 >>>> >>>> ciao >>>> Janne >>> >>> Dear Janne: >>> >>> I agree with you like I mentioned on last letter. >>> Thanks for your time. >>> >>> >>> Dear Dmitry: >>> >>> Is there anything else I can do? >>> Looking forward to your reply. >> >> If it is a common consensus, I'm fine with your approach. >> > Dear Dmitry: > > Ok.Thanks for your reply. > > Best regards! > >
On 17.05.23 17:15, Linux regression tracking (Thorsten Leemhuis) wrote: > Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting > for once, to make this easily accessible to everyone. > > Dmitry, was any progress made to address this regression? Doesn't look > like it, but I strongly suspect I'm missing something, FWIW, I'm dropping this from the regression tracking. It might be still around, but it's a warning and nothing that really breaks things afaics. Please holler if I got this wrong and think it's something that needs to be fixed to ensure the "no regressions" rule is adhered. #regzbot inconclusive: a warn() without serious impact, see discussion #regzbot ignore-activity Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. > as I'm not really > sure if I properly understood this thread. It sounded a bit like > a4e771729a51 should be reverted for now until all > drm_kms_helper_poll_disable() calls have been verified. Is that right? > Or did somebody already verify and fix all of them with bugs? > > On 28.04.23 03:17, zongmin zhou wrote: >> On Wed, 2023-04-26 at 16:10 +0300, Dmitry Baryshkov wrote: >>> On Wed, 26 Apr 2023 at 12:09, zongmin zhou <zhouzongmin@kylinos.cn> >>> wrote: >>>> On Sun, 2023-04-23 at 22:51 +0200, Janne Grunau wrote: >>>>> On 2023-04-20 23:07:01 +0300, Dmitry Baryshkov wrote: >>>>>> On Thu, 20 Apr 2023 at 23:01, Janne Grunau <j@jannau.net> >>>>>> wrote: >>>>>>> >>>>>>> On 2023-03-28 10:31:29 +0800, Zongmin Zhou wrote: >>>>>>>> When drivers call drm_kms_helper_poll_disable from >>>>>>>> their device suspend implementation without enabled output >>>>>>>> polling before, >>>>>>>> following warning will be reported,due to work->func not be >>>>>>>> initialized: >>>>>>> >>>>>>> we see the same warning with the wpork in progress kms driver >>>>>>> for >>>>>>> apple >>>>>>> silicon SoCs. The connectors do not need to polled so the >>>>>>> driver >>>>>>> never >>>>>>> calls drm_kms_helper_poll_init(). >>>>>>> >>>>>>>> [ 55.141361] WARNING: CPU: 3 PID: 372 at >>>>>>>> kernel/workqueue.c:3066 __flush_work+0x22f/0x240 >>>>>>>> [ 55.141382] Modules linked in: nls_iso8859_1 >>>>>>>> snd_hda_codec_generic ledtrig_audio snd_hda_intel >>>>>>>> snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec >>>>>>>> snd_hda_core >>>>>>>> snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event >>>>>>>> snd_rawmidi >>>>>>>> snd_seq intel_rapl_msr intel_rapl_common bochs >>>>>>>> drm_vram_helper >>>>>>>> drm_ttm_helper snd_seq_device nfit ttm crct10dif_pclmul >>>>>>>> snd_timer ghash_clmulni_intel binfmt_misc sha512_ssse3 >>>>>>>> aesni_intel drm_kms_helper joydev input_leds syscopyarea >>>>>>>> crypto_simd snd cryptd sysfillrect sysimgblt mac_hid >>>>>>>> serio_raw >>>>>>>> soundcore qemu_fw_cfg sch_fq_codel msr parport_pc ppdev lp >>>>>>>> parport drm ramoops reed_solomon pstore_blk pstore_zone >>>>>>>> efi_pstore virtio_rng ip_tables x_tables autofs4 >>>>>>>> hid_generic >>>>>>>> usbhid hid ahci virtio_net i2c_i801 crc32_pclmul psmouse >>>>>>>> virtio_scsi libahci i2c_smbus lpc_ich xhci_pci net_failover >>>>>>>> virtio_blk xhci_pci_renesas failover >>>>>>>> [ 55.141430] CPU: 3 PID: 372 Comm: kworker/u16:9 Not >>>>>>>> tainted >>>>>>>> 6.2.0-rc6+ #16 >>>>>>>> [ 55.141433] Hardware name: QEMU Standard PC (Q35 + ICH9, >>>>>>>> 2009), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org >>>>>>>> 04/01/2014 >>>>>>>> [ 55.141435] Workqueue: events_unbound async_run_entry_fn >>>>>>>> [ 55.141441] RIP: 0010:__flush_work+0x22f/0x240 >>>>>>>> [ 55.141444] Code: 8b 43 28 48 8b 53 30 89 c1 e9 f9 fe ff >>>>>>>> ff >>>>>>>> 4c 89 f7 e8 b5 95 d9 00 e8 00 53 08 00 45 31 ff e9 11 ff ff >>>>>>>> ff >>>>>>>> 0f 0b e9 0a ff ff ff <0f> 0b 45 31 ff e9 00 ff ff ff e8 e2 >>>>>>>> 54 >>>>>>>> d8 00 66 90 90 90 90 90 90 >>>>>>>> [ 55.141446] RSP: 0018:ff59221940833c18 EFLAGS: 00010246 >>>>>>>> [ 55.141449] RAX: 0000000000000000 RBX: 0000000000000000 >>>>>>>> RCX: >>>>>>>> ffffffff9b72bcbe >>>>>>>> [ 55.141450] RDX: 0000000000000001 RSI: 0000000000000001 >>>>>>>> RDI: >>>>>>>> ff3ea01e4265e330 >>>>>>>> [ 55.141451] RBP: ff59221940833c90 R08: 0000000000000000 >>>>>>>> R09: >>>>>>>> 8080808080808080 >>>>>>>> [ 55.141453] R10: ff3ea01e42b3caf4 R11: 000000000000000f >>>>>>>> R12: >>>>>>>> ff3ea01e4265e330 >>>>>>>> [ 55.141454] R13: 0000000000000001 R14: ff3ea01e505e5e80 >>>>>>>> R15: >>>>>>>> 0000000000000001 >>>>>>>> [ 55.141455] FS: 0000000000000000(0000) >>>>>>>> GS:ff3ea01fb7cc0000(0000) knlGS:0000000000000000 >>>>>>>> [ 55.141456] CS: 0010 DS: 0000 ES: 0000 CR0: >>>>>>>> 0000000080050033 >>>>>>>> [ 55.141458] CR2: 0000563543ad1546 CR3: 000000010ee82005 >>>>>>>> CR4: >>>>>>>> 0000000000771ee0 >>>>>>>> [ 55.141464] DR0: 0000000000000000 DR1: 0000000000000000 >>>>>>>> DR2: >>>>>>>> 0000000000000000 >>>>>>>> [ 55.141465] DR3: 0000000000000000 DR6: 00000000fffe0ff0 >>>>>>>> DR7: >>>>>>>> 0000000000000400 >>>>>>>> [ 55.141466] PKRU: 55555554 >>>>>>>> [ 55.141467] Call Trace: >>>>>>>> [ 55.141469] <TASK> >>>>>>>> [ 55.141472] ? pcie_wait_cmd+0xdf/0x220 >>>>>>>> [ 55.141478] ? mptcp_seq_show+0xe0/0x180 >>>>>>>> [ 55.141484] __cancel_work_timer+0x124/0x1b0 >>>>>>>> [ 55.141487] cancel_delayed_work_sync+0x17/0x20 >>>>>>>> [ 55.141490] drm_kms_helper_poll_disable+0x26/0x40 >>>>>>>> [drm_kms_helper] >>>>>>>> [ 55.141516] drm_mode_config_helper_suspend+0x25/0x90 >>>>>>>> [drm_kms_helper] >>>>>>>> [ 55.141531] ? __pm_runtime_resume+0x64/0x90 >>>>>>>> [ 55.141536] bochs_pm_suspend+0x16/0x20 [bochs] >>>>>>>> [ 55.141540] pci_pm_suspend+0x8b/0x1b0 >>>>>>>> [ 55.141545] ? __pfx_pci_pm_suspend+0x10/0x10 >>>>>>>> [ 55.141547] dpm_run_callback+0x4c/0x160 >>>>>>>> [ 55.141550] __device_suspend+0x14c/0x4c0 >>>>>>>> [ 55.141553] async_suspend+0x24/0xa0 >>>>>>>> [ 55.141555] async_run_entry_fn+0x34/0x120 >>>>>>>> [ 55.141557] process_one_work+0x21a/0x3f0 >>>>>>>> [ 55.141560] worker_thread+0x4e/0x3c0 >>>>>>>> [ 55.141563] ? __pfx_worker_thread+0x10/0x10 >>>>>>>> [ 55.141565] kthread+0xf2/0x120 >>>>>>>> [ 55.141568] ? __pfx_kthread+0x10/0x10 >>>>>>>> [ 55.141570] ret_from_fork+0x29/0x50 >>>>>>>> [ 55.141575] </TASK> >>>>>>>> [ 55.141575] ---[ end trace 0000000000000000 ]--- >>>>>>>> >>>>>>>> Fixes: a4e771729a51 ("drm/probe_helper: sort out >>>>>>>> poll_running >>>>>>>> vs poll_enabled") >>>>>>>> Signed-off-by: Zongmin Zhou<zhouzongmin@kylinos.cn> >>>>>>>> --- >>>>>>>> drivers/gpu/drm/drm_probe_helper.c | 3 ++- >>>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/drivers/gpu/drm/drm_probe_helper.c >>>>>>>> b/drivers/gpu/drm/drm_probe_helper.c >>>>>>>> index 8127be134c39..ac72b18e2257 100644 >>>>>>>> --- a/drivers/gpu/drm/drm_probe_helper.c >>>>>>>> +++ b/drivers/gpu/drm/drm_probe_helper.c >>>>>>>> @@ -855,7 +855,8 @@ void drm_kms_helper_poll_disable(struct >>>>>>>> drm_device *dev) >>>>>>>> if (dev->mode_config.poll_running) >>>>>>>> drm_kms_helper_disable_hpd(dev); >>>>>>>> >>>>>>>> - cancel_delayed_work_sync(&dev- >>>>>>>>> mode_config.output_poll_work); >>>>>>>> + if (dev->mode_config.poll_enabled) >>>>>>>> + cancel_delayed_work_sync(&dev- >>>>>>>>> mode_config.output_poll_work); >>>>>>> >>>>>>> Checking for dev->mode_config.poll_enabled at the start of >>>>>>> the >>>>>>> function >>>>>>> and return early if it is not true looks more in style with >>>>>>> the >>>>>>> rest of >>>>>>> drm_probe_helper.c. >>>>>> >>>>>> I think it is an error to call drm_kms_helper_poll_disable() if >>>>>> polling was not initialized. So, in my opinion the fix should >>>>>> go to >>>>>> the drm_mode_config_helper_suspend() / _resume() instead. >>>>>> Please >>>>>> add a >>>>>> guard there using dev->mode_config.poll_enabled. >>>>> >>>>> While I tend to agree to the sentiment I do not think this is the >>>>> correct fix in this situation. drm_kms_helper_poll_disable had >>>>> the >>>>> check since at least 2014. a4e771729a51 is a regression. If we >>>>> want >>>>> to >>>>> change the behavior it should be done explicitly and after >>>>> verifying >>>>> all >>>>> drm_kms_helper_poll_disable() calls. >>>>> >>>>> #regzbot ^introduced a4e771729a51 >>>>> >>>>> ciao >>>>> Janne >>>> >>>> Dear Janne: >>>> >>>> I agree with you like I mentioned on last letter. >>>> Thanks for your time. >>>> >>>> >>>> Dear Dmitry: >>>> >>>> Is there anything else I can do? >>>> Looking forward to your reply. >>> >>> If it is a common consensus, I'm fine with your approach. >>> >> Dear Dmitry: >> >> Ok.Thanks for your reply. >> >> Best regards! >> >>
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 8127be134c39..ac72b18e2257 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -855,7 +855,8 @@ void drm_kms_helper_poll_disable(struct drm_device *dev) if (dev->mode_config.poll_running) drm_kms_helper_disable_hpd(dev); - cancel_delayed_work_sync(&dev->mode_config.output_poll_work); + if (dev->mode_config.poll_enabled) + cancel_delayed_work_sync(&dev->mode_config.output_poll_work); dev->mode_config.poll_running = false; }
When drivers call drm_kms_helper_poll_disable from their device suspend implementation without enabled output polling before, following warning will be reported,due to work->func not be initialized: [ 55.141361] WARNING: CPU: 3 PID: 372 at kernel/workqueue.c:3066 __flush_work+0x22f/0x240 [ 55.141382] Modules linked in: nls_iso8859_1 snd_hda_codec_generic ledtrig_audio snd_hda_intel snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec snd_hda_core snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq intel_rapl_msr intel_rapl_common bochs drm_vram_helper drm_ttm_helper snd_seq_device nfit ttm crct10dif_pclmul snd_timer ghash_clmulni_intel binfmt_misc sha512_ssse3 aesni_intel drm_kms_helper joydev input_leds syscopyarea crypto_simd snd cryptd sysfillrect sysimgblt mac_hid serio_raw soundcore qemu_fw_cfg sch_fq_codel msr parport_pc ppdev lp parport drm ramoops reed_solomon pstore_blk pstore_zone efi_pstore virtio_rng ip_tables x_tables autofs4 hid_generic usbhid hid ahci virtio_net i2c_i801 crc32_pclmul psmouse virtio_scsi libahci i2c_smbus lpc_ich xhci_pci net_failover virtio_blk xhci_pci_renesas failover [ 55.141430] CPU: 3 PID: 372 Comm: kworker/u16:9 Not tainted 6.2.0-rc6+ #16 [ 55.141433] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 [ 55.141435] Workqueue: events_unbound async_run_entry_fn [ 55.141441] RIP: 0010:__flush_work+0x22f/0x240 [ 55.141444] Code: 8b 43 28 48 8b 53 30 89 c1 e9 f9 fe ff ff 4c 89 f7 e8 b5 95 d9 00 e8 00 53 08 00 45 31 ff e9 11 ff ff ff 0f 0b e9 0a ff ff ff <0f> 0b 45 31 ff e9 00 ff ff ff e8 e2 54 d8 00 66 90 90 90 90 90 90 [ 55.141446] RSP: 0018:ff59221940833c18 EFLAGS: 00010246 [ 55.141449] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff9b72bcbe [ 55.141450] RDX: 0000000000000001 RSI: 0000000000000001 RDI: ff3ea01e4265e330 [ 55.141451] RBP: ff59221940833c90 R08: 0000000000000000 R09: 8080808080808080 [ 55.141453] R10: ff3ea01e42b3caf4 R11: 000000000000000f R12: ff3ea01e4265e330 [ 55.141454] R13: 0000000000000001 R14: ff3ea01e505e5e80 R15: 0000000000000001 [ 55.141455] FS: 0000000000000000(0000) GS:ff3ea01fb7cc0000(0000) knlGS:0000000000000000 [ 55.141456] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 55.141458] CR2: 0000563543ad1546 CR3: 000000010ee82005 CR4: 0000000000771ee0 [ 55.141464] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 55.141465] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 55.141466] PKRU: 55555554 [ 55.141467] Call Trace: [ 55.141469] <TASK> [ 55.141472] ? pcie_wait_cmd+0xdf/0x220 [ 55.141478] ? mptcp_seq_show+0xe0/0x180 [ 55.141484] __cancel_work_timer+0x124/0x1b0 [ 55.141487] cancel_delayed_work_sync+0x17/0x20 [ 55.141490] drm_kms_helper_poll_disable+0x26/0x40 [drm_kms_helper] [ 55.141516] drm_mode_config_helper_suspend+0x25/0x90 [drm_kms_helper] [ 55.141531] ? __pm_runtime_resume+0x64/0x90 [ 55.141536] bochs_pm_suspend+0x16/0x20 [bochs] [ 55.141540] pci_pm_suspend+0x8b/0x1b0 [ 55.141545] ? __pfx_pci_pm_suspend+0x10/0x10 [ 55.141547] dpm_run_callback+0x4c/0x160 [ 55.141550] __device_suspend+0x14c/0x4c0 [ 55.141553] async_suspend+0x24/0xa0 [ 55.141555] async_run_entry_fn+0x34/0x120 [ 55.141557] process_one_work+0x21a/0x3f0 [ 55.141560] worker_thread+0x4e/0x3c0 [ 55.141563] ? __pfx_worker_thread+0x10/0x10 [ 55.141565] kthread+0xf2/0x120 [ 55.141568] ? __pfx_kthread+0x10/0x10 [ 55.141570] ret_from_fork+0x29/0x50 [ 55.141575] </TASK> [ 55.141575] ---[ end trace 0000000000000000 ]--- Fixes: a4e771729a51 ("drm/probe_helper: sort out poll_running vs poll_enabled") Signed-off-by: Zongmin Zhou<zhouzongmin@kylinos.cn> --- drivers/gpu/drm/drm_probe_helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)