+ 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 <[email protected]>
>>>>
>>>> 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 <[email protected]>
>>>> Reviewed-by: Sascha Hauer <[email protected]>
>>>
>>> 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/[email protected]/
>>>> ---
>>>>
>>>> (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