Messages in this thread | ![/](/images/icornerl.gif) | | Date | Sat, 16 Dec 2023 11:17:28 +0800 | Subject | Re: [v5,02/16] Revert "drm/rockchip: vop2: Use regcache_sync() to fix suspend/resume" | From | Andy Yan <> |
| |
+ Mark Hi Marek andy Mark:
On 12/15/23 16:33, Andy Yan wrote: > Hi Marek: > > On 12/15/23 08:59, Andy Yan wrote: >> Hi Marek: >> >> Sorry for this issue. >> I also tested this series on RK3568/6 before I send them out. >> But I didn't find anyahing unusal, would you please share the >> linux kernel defconfig you used for this test and it would be >> greatly appreciated if you can share more test detial. > > > I can reproduce this warning when enable CONFIG_LOCKDEP, I will try > to fix it asap. > > Thanks. >> >> On 12/14/23 20:13, Marek Szyprowski wrote: >>> Dear All, >>> >>> On 11.12.2023 12:57, Andy Yan wrote: >>>> From: Andy Yan <andy.yan@rock-chips.com> >>>> >>>> This reverts commit b63a553e8f5aa6574eeb535a551817a93c426d8c. >>>> >>>> regcache_sync will try to reload the configuration in regcache to >>>> hardware, but the registers of 4 Cluster windows and Esmart1/2/3 on >>>> the upcoming rk3588 can not be set successfully before internal PD >>>> power on. >>>> >>>> Also it's better to keep the hardware register as it is before we really >>>> enable it. >>>> >>>> So let's revert this version, and keep the first version: >>>> commit afa965a45e01 ("drm/rockchip: vop2: fix suspend/resume") >>>> >>>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com> >>>> Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de> >>> >>> This patch landed in today's linux-next as commit 81a06f1d02e5 ("Revert >>> "drm/rockchip: vop2: Use regcache_sync() to fix suspend/resume""). >>> Unfortunately it triggers a following lock dep warning on my Odroid-M1 >>> test board: >>> >>> ======================================================== >>> WARNING: possible irq lock inversion dependency detected >>> 6.7.0-rc3+ #14328 Not tainted >>> -------------------------------------------------------- >>> swapper/0/0 just changed the state of lock: >>> ffff0001f3ae2c18 >>> (rockchip_drm_vop2:2723:(&vop2_regmap_config)->lock){-...}-{2:2}, at: >>> regmap_lock_spinlock+0x18/0x2c >>> but this lock took another, HARDIRQ-unsafe lock in the past: >>> (&mt->ma_lock){+.+.}-{2:2} >>> >>> >>> and interrupts could create inverse lock ordering between them. >>> >>> >>> other info that might help us debug this: >>> Possible interrupt unsafe locking scenario: >>> >>> CPU0 CPU1 >>> ---- ---- >>> lock(&mt->ma_lock); >>> local_irq_disable(); >>> lock(rockchip_drm_vop2:2723:(&vop2_regmap_config)->lock); >>> lock(&mt->ma_lock); >>> <Interrupt> >>> lock(rockchip_drm_vop2:2723:(&vop2_regmap_config)->lock); >>> >>> *** DEADLOCK *** >>> >>> no locks held by swapper/0/0. >>> >>> the shortest dependencies between 2nd lock and 1st lock: >>> -> (&mt->ma_lock){+.+.}-{2:2} { >>> HARDIRQ-ON-W at: >>> lock_acquire+0x1e8/0x318 >>> _raw_spin_lock+0x48/0x60 >>> regcache_maple_exit+0x5c/0xc0 >>> regcache_exit+0x48/0x74 >>> regmap_reinit_cache+0x1c/0xc4 >>> vop2_crtc_atomic_enable+0x86c/0xa0c [rockchipdrm] >>> drm_atomic_helper_commit_modeset_enables+0xb4/0x26c >>> drm_atomic_helper_commit_tail_rpm+0x50/0xa0 >>> commit_tail+0xa4/0x18c >>> drm_atomic_helper_commit+0x19c/0x1b0 >>> drm_atomic_commit+0xa8/0xe0 >>> drm_client_modeset_commit_atomic+0x22c/0x298 >>> drm_client_modeset_commit_locked+0x60/0x1c0 >>> drm_client_modeset_commit+0x30/0x58 >>> __drm_fb_helper_restore_fbdev_mode_unlocked+0xbc/0xfc >>> drm_fb_helper_set_par+0x30/0x4c >>> fbcon_init+0x234/0x4c0 >>> visual_init+0xb0/0x108 >>> do_bind_con_driver.isra.0+0x19c/0x394 >>> do_take_over_console+0x144/0x1f0 >>> do_fbcon_takeover+0x6c/0xe4 >>> fbcon_fb_registered+0x1e0/0x1e8 >>> register_framebuffer+0x19c/0x228 >>> __drm_fb_helper_initial_config_and_unlock+0x348/0x4fc >>> drm_fb_helper_hotplug_event.part.0+0xf0/0x110 >>> drm_fb_helper_hotplug_event+0x38/0x44 >>> drm_fbdev_generic_client_hotplug+0x28/0xd4 >>> drm_client_dev_hotplug+0xcc/0x130 >>> output_poll_execute+0x204/0x23c >>> process_one_work+0x1ec/0x53c >>> worker_thread+0x298/0x408 >>> kthread+0x124/0x128 >>> ret_from_fork+0x10/0x20 >>> SOFTIRQ-ON-W at: >>> lock_acquire+0x1e8/0x318 >>> _raw_spin_lock+0x48/0x60 >>> regcache_maple_exit+0x5c/0xc0 >>> regcache_exit+0x48/0x74 >>> regmap_reinit_cache+0x1c/0xc4 >>> vop2_crtc_atomic_enable+0x86c/0xa0c [rockchipdrm] >>> drm_atomic_helper_commit_modeset_enables+0xb4/0x26c >>> drm_atomic_helper_commit_tail_rpm+0x50/0xa0 >>> commit_tail+0xa4/0x18c >>> drm_atomic_helper_commit+0x19c/0x1b0 >>> drm_atomic_commit+0xa8/0xe0 >>> drm_client_modeset_commit_atomic+0x22c/0x298 >>> drm_client_modeset_commit_locked+0x60/0x1c0 >>> drm_client_modeset_commit+0x30/0x58 >>> __drm_fb_helper_restore_fbdev_mode_unlocked+0xbc/0xfc >>> drm_fb_helper_set_par+0x30/0x4c >>> fbcon_init+0x234/0x4c0 >>> visual_init+0xb0/0x108 >>> do_bind_con_driver.isra.0+0x19c/0x394 >>> do_take_over_console+0x144/0x1f0 >>> do_fbcon_takeover+0x6c/0xe4 >>> fbcon_fb_registered+0x1e0/0x1e8 >>> register_framebuffer+0x19c/0x228 >>> __drm_fb_helper_initial_config_and_unlock+0x348/0x4fc >>> drm_fb_helper_hotplug_event.part.0+0xf0/0x110 >>> drm_fb_helper_hotplug_event+0x38/0x44 >>> drm_fbdev_generic_client_hotplug+0x28/0xd4 >>> drm_client_dev_hotplug+0xcc/0x130 >>> output_poll_execute+0x204/0x23c >>> process_one_work+0x1ec/0x53c >>> worker_thread+0x298/0x408 >>> kthread+0x124/0x128 >>> ret_from_fork+0x10/0x20 >>> INITIAL USE at: >>> lock_acquire+0x1e8/0x318 >>> _raw_spin_lock+0x48/0x60 >>> regcache_maple_exit+0x5c/0xc0 >>> regcache_exit+0x48/0x74 >>> regmap_reinit_cache+0x1c/0xc4 >>> vop2_crtc_atomic_enable+0x86c/0xa0c [rockchipdrm] >>> drm_atomic_helper_commit_modeset_enables+0xb4/0x26c >>> drm_atomic_helper_commit_tail_rpm+0x50/0xa0 >>> commit_tail+0xa4/0x18c >>> drm_atomic_helper_commit+0x19c/0x1b0 >>> drm_atomic_commit+0xa8/0xe0 >>> drm_client_modeset_commit_atomic+0x22c/0x298 >>> drm_client_modeset_commit_locked+0x60/0x1c0 >>> drm_client_modeset_commit+0x30/0x58 >>> __drm_fb_helper_restore_fbdev_mode_unlocked+0xbc/0xfc >>> drm_fb_helper_set_par+0x30/0x4c >>> fbcon_init+0x234/0x4c0 >>> visual_init+0xb0/0x108 >>> do_bind_con_driver.isra.0+0x19c/0x394 >>> do_take_over_console+0x144/0x1f0 >>> do_fbcon_takeover+0x6c/0xe4 >>> fbcon_fb_registered+0x1e0/0x1e8 >>> register_framebuffer+0x19c/0x228 >>> __drm_fb_helper_initial_config_and_unlock+0x348/0x4fc >>> drm_fb_helper_hotplug_event.part.0+0xf0/0x110 >>> drm_fb_helper_hotplug_event+0x38/0x44 >>> drm_fbdev_generic_client_hotplug+0x28/0xd4 >>> drm_client_dev_hotplug+0xcc/0x130 >>> output_poll_execute+0x204/0x23c >>> process_one_work+0x1ec/0x53c >>> worker_thread+0x298/0x408 >>> kthread+0x124/0x128 >>> ret_from_fork+0x10/0x20 >>> } >>> ... key at: [<ffff800083de53b0>] __key.0+0x0/0x10 >>> ... acquired at: >>> _raw_spin_lock+0x48/0x60 >>> regcache_maple_write+0x1d8/0x31c >>> regcache_write+0x6c/0x84 >>> _regmap_read+0x1b4/0x1f4 >>> _regmap_update_bits+0xec/0x134 >>> regmap_field_update_bits_base+0x6c/0xa0 >>> vop2_plane_atomic_update+0x380/0x11d0 [rockchipdrm] >>> drm_atomic_helper_commit_planes+0xec/0x2a0 >>> drm_atomic_helper_commit_tail_rpm+0x60/0xa0 >>> commit_tail+0xa4/0x18c >>> drm_atomic_helper_commit+0x19c/0x1b0 >>> drm_atomic_commit+0xa8/0xe0 >>> drm_client_modeset_commit_atomic+0x22c/0x298 >>> drm_client_modeset_commit_locked+0x60/0x1c0 >>> drm_client_modeset_commit+0x30/0x58 >>> __drm_fb_helper_restore_fbdev_mode_unlocked+0xbc/0xfc >>> drm_fb_helper_set_par+0x30/0x4c >>> fbcon_init+0x234/0x4c0 >>> visual_init+0xb0/0x108 >>> do_bind_con_driver.isra.0+0x19c/0x394 >>> do_take_over_console+0x144/0x1f0 >>> do_fbcon_takeover+0x6c/0xe4 >>> fbcon_fb_registered+0x1e0/0x1e8 >>> register_framebuffer+0x19c/0x228 >>> __drm_fb_helper_initial_config_and_unlock+0x348/0x4fc >>> drm_fb_helper_hotplug_event.part.0+0xf0/0x110 >>> drm_fb_helper_hotplug_event+0x38/0x44 >>> drm_fbdev_generic_client_hotplug+0x28/0xd4 >>> drm_client_dev_hotplug+0xcc/0x130 >>> output_poll_execute+0x204/0x23c >>> process_one_work+0x1ec/0x53c >>> worker_thread+0x298/0x408 >>> kthread+0x124/0x128 >>> ret_from_fork+0x10/0x20 >>> >>> -> (rockchip_drm_vop2:2723:(&vop2_regmap_config)->lock){-...}-{2:2} { >>> IN-HARDIRQ-W at: >>> lock_acquire+0x1e8/0x318 >>> _raw_spin_lock_irqsave+0x60/0x88 >>> regmap_lock_spinlock+0x18/0x2c >>> regmap_read+0x3c/0x78 >>> vop2_isr+0x84/0x2a4 [rockchipdrm] >>> __handle_irq_event_percpu+0xb0/0x2d4 >>> handle_irq_event+0x4c/0xb8 >>> handle_fasteoi_irq+0xa4/0x1cc >>> generic_handle_domain_irq+0x2c/0x44 >>> gic_handle_irq+0x4c/0x110 >>> call_on_irq_stack+0x24/0x4c >>> do_interrupt_handler+0x80/0x84 >>> el1_interrupt+0x34/0x64 >>> el1h_64_irq_handler+0x18/0x24 >>> el1h_64_irq+0x64/0x68 >>> default_idle_call+0x9c/0x150 >>> do_idle+0x230/0x294 >>> cpu_startup_entry+0x34/0x3c >>> rest_init+0x100/0x190 >>> arch_post_acpi_subsys_init+0x0/0x8 >>> start_kernel+0x594/0x684 >>> __primary_switched+0xbc/0xc4 >>> INITIAL USE at: >>> lock_acquire+0x1e8/0x318 >>> _raw_spin_lock_irqsave+0x60/0x88 >>> regmap_lock_spinlock+0x18/0x2c >>> regmap_write+0x3c/0x78 >>> vop2_crtc_atomic_enable+0x894/0xa0c [rockchipdrm] >>> drm_atomic_helper_commit_modeset_enables+0xb4/0x26c >>> drm_atomic_helper_commit_tail_rpm+0x50/0xa0 >>> commit_tail+0xa4/0x18c >>> drm_atomic_helper_commit+0x19c/0x1b0 >>> drm_atomic_commit+0xa8/0xe0 >>> drm_client_modeset_commit_atomic+0x22c/0x298 >>> drm_client_modeset_commit_locked+0x60/0x1c0 >>> drm_client_modeset_commit+0x30/0x58 >>> __drm_fb_helper_restore_fbdev_mode_unlocked+0xbc/0xfc >>> drm_fb_helper_set_par+0x30/0x4c >>> fbcon_init+0x234/0x4c0 >>> visual_init+0xb0/0x108 >>> do_bind_con_driver.isra.0+0x19c/0x394 >>> do_take_over_console+0x144/0x1f0 >>> do_fbcon_takeover+0x6c/0xe4 >>> fbcon_fb_registered+0x1e0/0x1e8 >>> register_framebuffer+0x19c/0x228 >>> __drm_fb_helper_initial_config_and_unlock+0x348/0x4fc >>> drm_fb_helper_hotplug_event.part.0+0xf0/0x110 >>> drm_fb_helper_hotplug_event+0x38/0x44 >>> drm_fbdev_generic_client_hotplug+0x28/0xd4 >>> drm_client_dev_hotplug+0xcc/0x130 >>> output_poll_execute+0x204/0x23c >>> process_one_work+0x1ec/0x53c >>> worker_thread+0x298/0x408 >>> kthread+0x124/0x128 >>> ret_from_fork+0x10/0x20 >>> } >>> ... key at: [<ffff80007c090a18>] _key.6+0x0/0xffffffffffffd5e8 >>> [rockchipdrm] >>> ... acquired at: >>> __lock_acquire+0xad8/0x20c4 >>> lock_acquire+0x1e8/0x318 >>> _raw_spin_lock_irqsave+0x60/0x88 >>> regmap_lock_spinlock+0x18/0x2c >>> regmap_read+0x3c/0x78 >>> vop2_isr+0x84/0x2a4 [rockchipdrm] >>> __handle_irq_event_percpu+0xb0/0x2d4 >>> handle_irq_event+0x4c/0xb8 >>> handle_fasteoi_irq+0xa4/0x1cc >>> generic_handle_domain_irq+0x2c/0x44 >>> gic_handle_irq+0x4c/0x110 >>> call_on_irq_stack+0x24/0x4c >>> do_interrupt_handler+0x80/0x84 >>> el1_interrupt+0x34/0x64 >>> el1h_64_irq_handler+0x18/0x24 >>> el1h_64_irq+0x64/0x68 >>> default_idle_call+0x9c/0x150 >>> do_idle+0x230/0x294 >>> cpu_startup_entry+0x34/0x3c >>> rest_init+0x100/0x190 >>> arch_post_acpi_subsys_init+0x0/0x8 >>> start_kernel+0x594/0x684 >>> __primary_switched+0xbc/0xc4 >>> >>> >>> stack backtrace: >>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.7.0-rc3+ #14328 >>> Hardware name: Hardkernel ODROID-M1 (DT) >>> Call trace: >>> dump_backtrace+0x98/0xf0 >>> show_stack+0x18/0x24 >>> dump_stack_lvl+0x60/0xac >>> dump_stack+0x18/0x24 >>> print_irq_inversion_bug.part.0+0x1ec/0x27c >>> mark_lock+0x634/0x950 >>> __lock_acquire+0xad8/0x20c4 >>> lock_acquire+0x1e8/0x318 >>> _raw_spin_lock_irqsave+0x60/0x88 >>> regmap_lock_spinlock+0x18/0x2c >>> regmap_read+0x3c/0x78 >>> vop2_isr+0x84/0x2a4 [rockchipdrm] >>> __handle_irq_event_percpu+0xb0/0x2d4 >>> handle_irq_event+0x4c/0xb8 >>> handle_fasteoi_irq+0xa4/0x1cc >>> generic_handle_domain_irq+0x2c/0x44 >>> gic_handle_irq+0x4c/0x110 >>> call_on_irq_stack+0x24/0x4c >>> do_interrupt_handler+0x80/0x84 >>> el1_interrupt+0x34/0x64 >>> el1h_64_irq_handler+0x18/0x24 >>> el1h_64_irq+0x64/0x68 >>> default_idle_call+0x9c/0x150 >>> do_idle+0x230/0x294 >>> cpu_startup_entry+0x34/0x3c >>> rest_init+0x100/0x190 >>> arch_post_acpi_subsys_init+0x0/0x8 >>> start_kernel+0x594/0x684 >>> __primary_switched+0xbc/0xc4 >>> Console: switching to colour frame buffer device 240x67 >>> rockchip-drm display-subsystem: [drm] fb0: rockchipdrmfb frame buffer device >>> >>> >>> Reverting it on top of next-20231214 and resolving a conflict >>> fixes/hides the above lock dep issue. >>> >>>
I still dig what is going on here, but I also found that if I change the regmap cache_type back to REGCACHE_MAPLE(which means revert Mark's patch[0]), this warning is gone.
As I am not familiar with regmap and lockdep, I'm not sure if this wanring is caused by my improper use of regmap_reinit_cache.
I will continue do more dig, any light on this would be greatly appreciated.
[0]https://patchwork.kernel.org/project/linux-arm-kernel/patch/20231001-drm-rockchip-maple-v1-1-ca396ab75be7@kernel.org/
>>>> --- >>>> >>>> (no changes since v1) >>>> >>>> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 10 +++++++--- >>>> 1 file changed, 7 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >>>> index 312da5783362..57784d0a22a6 100644 >>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >>>> @@ -217,6 +217,8 @@ struct vop2 { >>>> struct vop2_win win[]; >>>> }; >>>> +static const struct regmap_config vop2_regmap_config; >>>> + >>>> static struct vop2_video_port *to_vop2_video_port(struct drm_crtc *crtc) >>>> { >>>> return container_of(crtc, struct vop2_video_port, crtc); >>>> @@ -883,7 +885,11 @@ static void vop2_enable(struct vop2 *vop2) >>>> return; >>>> } >>>> - regcache_sync(vop2->map); >>>> + ret = regmap_reinit_cache(vop2->map, &vop2_regmap_config); >>>> + if (ret) { >>>> + drm_err(vop2->drm, "failed to reinit cache: %d\n", ret); >>>> + return; >>>> + } >>>> if (vop2->data->soc_id == 3566) >>>> vop2_writel(vop2, RK3568_OTP_WIN_EN, 1); >>>> @@ -913,8 +919,6 @@ static void vop2_disable(struct vop2 *vop2) >>>> pm_runtime_put_sync(vop2->dev); >>>> - regcache_mark_dirty(vop2->map); >>>> - >>>> clk_disable_unprepare(vop2->aclk); >>>> clk_disable_unprepare(vop2->hclk); >>>> } >>> >>> Best regards
| ![\](/images/icornerr.gif) |