Re: [v5,02/16] Revert "drm/rockchip: vop2: Use regcache_sync() to fix suspend/resume"
From: Andy Yan
Date: Fri Dec 15 2023 - 22:26:45 EST
+ 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@xxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxx>
Reviewed-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
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@xxxxxxxxxx/
---
(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