From 06c3c406850e5495bb56ccf624d0c9477e1ba901 Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Tue, 13 Aug 2024 11:25:05 +0100 Subject: [PATCH 01/16] drm/v3d: Appease lockdep while updating GPU stats MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Lockdep thinks our seqcount_t usage is unsafe because the update path can be both from irq and worker context: [ ] ================================ [ ] WARNING: inconsistent lock state [ ] 6.10.3-v8-16k-numa #159 Tainted: G WC [ ] -------------------------------- [ ] inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage. [ ] swapper/0/0 [HC1[1]:SC0[0]:HE0:SE1] takes: [ ] ffff80003d7c08d0 (&v3d_priv->stats[i].lock){?.+.}-{0:0}, at: v3d_irq+0xc8/0x660 [v3d] [ ] {HARDIRQ-ON-W} state was registered at: [ ] lock_acquire+0x1f8/0x328 [ ] v3d_job_start_stats.isra.0+0xd8/0x218 [v3d] [ ] v3d_bin_job_run+0x23c/0x388 [v3d] [ ] drm_sched_run_job_work+0x520/0x6d0 [gpu_sched] [ ] process_one_work+0x62c/0xb48 [ ] worker_thread+0x468/0x5b0 [ ] kthread+0x1c4/0x1e0 [ ] ret_from_fork+0x10/0x20 [ ] irq event stamp: 337094 [ ] hardirqs last enabled at (337093): [] default_idle_call+0x11c/0x140 [ ] hardirqs last disabled at (337094): [] el1_interrupt+0x24/0x58 [ ] softirqs last enabled at (337082): [] handle_softirqs+0x4e0/0x538 [ ] softirqs last disabled at (337073): [] __do_softirq+0x1c/0x28 [ ] other info that might help us debug this: [ ] Possible unsafe locking scenario: [ ] CPU0 [ ] ---- [ ] lock(&v3d_priv->stats[i].lock); [ ] [ ] lock(&v3d_priv->stats[i].lock); [ ] *** DEADLOCK *** [ ] no locks held by swapper/0/0. [ ] stack backtrace: [ ] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G WC 6.10.3-v8-16k-numa #159 [ ] Hardware name: Raspberry Pi 5 Model B Rev 1.0 (DT) [ ] Call trace: [ ] dump_backtrace+0x170/0x1b8 [ ] show_stack+0x20/0x38 [ ] dump_stack_lvl+0xb4/0xd0 [ ] dump_stack+0x18/0x28 [ ] print_usage_bug+0x3cc/0x3f0 [ ] mark_lock+0x4d0/0x968 [ ] __lock_acquire+0x784/0x18c8 [ ] lock_acquire+0x1f8/0x328 [ ] v3d_job_update_stats+0xec/0x2e0 [v3d] [ ] v3d_irq+0xc8/0x660 [v3d] [ ] __handle_irq_event_percpu+0x1f8/0x488 [ ] handle_irq_event+0x88/0x128 [ ] handle_fasteoi_irq+0x298/0x408 [ ] generic_handle_domain_irq+0x50/0x78 But it is a false positive because all the queue-stats pairs have their own lock and jobs are also one at a time. Nevertheless we can appease lockdep by disabling local interrupts to make it see lock usage is consistent. Cc: Maíra Canal Fixes: 6abe93b621ab ("drm/v3d: Fix race-condition between sysfs/fdinfo and interrupt handler") Signed-off-by: Tvrtko Ursulin Signed-off-by: Maíra Canal Link: https://patchwork.freedesktop.org/patch/msgid/20240813102505.80512-2-tursulin@igalia.com --- drivers/gpu/drm/v3d/v3d_sched.c | 46 +++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c index f58c38d7de29..99ac4995b5a1 100644 --- a/drivers/gpu/drm/v3d/v3d_sched.c +++ b/drivers/gpu/drm/v3d/v3d_sched.c @@ -135,8 +135,31 @@ v3d_job_start_stats(struct v3d_job *job, enum v3d_queue queue) struct v3d_stats *global_stats = &v3d->queue[queue].stats; struct v3d_stats *local_stats = &file->stats[queue]; u64 now = local_clock(); - - preempt_disable(); + unsigned long flags; + + /* + * We only need to disable local interrupts to appease lockdep who + * otherwise would think v3d_job_start_stats vs v3d_stats_update has an + * unsafe in-irq vs no-irq-off usage problem. This is a false positive + * because all the locks are per queue and stats type, and all jobs are + * completely one at a time serialised. More specifically: + * + * 1. Locks for GPU queues are updated from interrupt handlers under a + * spin lock and started here with preemption disabled. + * + * 2. Locks for CPU queues are updated from the worker with preemption + * disabled and equally started here with preemption disabled. + * + * Therefore both are consistent. + * + * 3. Because next job can only be queued after the previous one has + * been signaled, and locks are per queue, there is also no scope for + * the start part to race with the update part. + */ + if (IS_ENABLED(CONFIG_LOCKDEP)) + local_irq_save(flags); + else + preempt_disable(); write_seqcount_begin(&local_stats->lock); local_stats->start_ns = now; @@ -146,7 +169,10 @@ v3d_job_start_stats(struct v3d_job *job, enum v3d_queue queue) global_stats->start_ns = now; write_seqcount_end(&global_stats->lock); - preempt_enable(); + if (IS_ENABLED(CONFIG_LOCKDEP)) + local_irq_restore(flags); + else + preempt_enable(); } static void @@ -167,11 +193,21 @@ v3d_job_update_stats(struct v3d_job *job, enum v3d_queue queue) struct v3d_stats *global_stats = &v3d->queue[queue].stats; struct v3d_stats *local_stats = &file->stats[queue]; u64 now = local_clock(); + unsigned long flags; + + /* See comment in v3d_job_start_stats() */ + if (IS_ENABLED(CONFIG_LOCKDEP)) + local_irq_save(flags); + else + preempt_disable(); - preempt_disable(); v3d_stats_update(local_stats, now); v3d_stats_update(global_stats, now); - preempt_enable(); + + if (IS_ENABLED(CONFIG_LOCKDEP)) + local_irq_restore(flags); + else + preempt_enable(); } static struct dma_fence *v3d_bin_job_run(struct drm_sched_job *sched_job) -- 2.51.0 From 319e53f155907cf2c6dabc16ec9dce0179bc04d1 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Mon, 16 Sep 2024 19:00:08 -0400 Subject: [PATCH 02/16] drm/panic: Fix uninitialized spinlock acquisition with CONFIG_DRM_PANIC=n It turns out that if you happen to have a kernel config where CONFIG_DRM_PANIC is disabled and spinlock debugging is enabled, along with KMS being enabled - we'll end up trying to acquire an uninitialized spin_lock with drm_panic_lock() when we try to do a commit: rvkms rvkms.0: [drm:drm_atomic_commit] committing 0000000068d2ade1 INFO: trying to register non-static key. The code is fine but needs lockdep annotation, or maybe you didn't initialize this object before use? turning off the locking correctness validator. CPU: 4 PID: 1347 Comm: modprobe Not tainted 6.10.0-rc1Lyude-Test+ #272 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS edk2-20240524-3.fc40 05/24/2024 Call Trace: dump_stack_lvl+0x77/0xa0 assign_lock_key+0x114/0x120 register_lock_class+0xa8/0x2c0 __lock_acquire+0x7d/0x2bd0 ? __vmap_pages_range_noflush+0x3a8/0x550 ? drm_atomic_helper_swap_state+0x2ad/0x3a0 lock_acquire+0xec/0x290 ? drm_atomic_helper_swap_state+0x2ad/0x3a0 ? lock_release+0xee/0x310 _raw_spin_lock_irqsave+0x4e/0x70 ? drm_atomic_helper_swap_state+0x2ad/0x3a0 drm_atomic_helper_swap_state+0x2ad/0x3a0 drm_atomic_helper_commit+0xb1/0x270 drm_atomic_commit+0xaf/0xe0 ? __pfx___drm_printfn_info+0x10/0x10 drm_client_modeset_commit_atomic+0x1a1/0x250 drm_client_modeset_commit_locked+0x4b/0x180 drm_client_modeset_commit+0x27/0x50 __drm_fb_helper_restore_fbdev_mode_unlocked+0x76/0x90 drm_fb_helper_set_par+0x38/0x40 fbcon_init+0x3c4/0x690 visual_init+0xc0/0x120 do_bind_con_driver+0x409/0x4c0 do_take_over_console+0x233/0x280 do_fb_registered+0x11f/0x210 fbcon_fb_registered+0x2c/0x60 register_framebuffer+0x248/0x2a0 __drm_fb_helper_initial_config_and_unlock+0x58a/0x720 drm_fbdev_generic_client_hotplug+0x6e/0xb0 drm_client_register+0x76/0xc0 _RNvXs_CsHeezP08sTT_5rvkmsNtB4_5RvkmsNtNtCs1cdwasc6FUb_6kernel8platform6Driver5probe+0xed2/0x1060 [rvkms] ? _RNvMs_NtCs1cdwasc6FUb_6kernel8platformINtB4_7AdapterNtCsHeezP08sTT_5rvkms5RvkmsE14probe_callbackBQ_+0x2b/0x70 [rvkms] ? acpi_dev_pm_attach+0x25/0x110 ? platform_probe+0x6a/0xa0 ? really_probe+0x10b/0x400 ? __driver_probe_device+0x7c/0x140 ? driver_probe_device+0x22/0x1b0 ? __device_attach_driver+0x13a/0x1c0 ? __pfx___device_attach_driver+0x10/0x10 ? bus_for_each_drv+0x114/0x170 ? __device_attach+0xd6/0x1b0 ? bus_probe_device+0x9e/0x120 ? device_add+0x288/0x4b0 ? platform_device_add+0x75/0x230 ? platform_device_register_full+0x141/0x180 ? rust_helper_platform_device_register_simple+0x85/0xb0 ? _RNvMs2_NtCs1cdwasc6FUb_6kernel8platformNtB5_6Device13create_simple+0x1d/0x60 ? _RNvXs0_CsHeezP08sTT_5rvkmsNtB5_5RvkmsNtCs1cdwasc6FUb_6kernel6Module4init+0x11e/0x160 [rvkms] ? 0xffffffffc083f000 ? init_module+0x20/0x1000 [rvkms] ? kernfs_xattr_get+0x3e/0x80 ? do_one_initcall+0x148/0x3f0 ? __lock_acquire+0x5ef/0x2bd0 ? __lock_acquire+0x5ef/0x2bd0 ? __lock_acquire+0x5ef/0x2bd0 ? put_cpu_partial+0x51/0x1d0 ? lock_acquire+0xec/0x290 ? put_cpu_partial+0x51/0x1d0 ? lock_release+0xee/0x310 ? put_cpu_partial+0x51/0x1d0 ? fs_reclaim_acquire+0x69/0xf0 ? lock_acquire+0xec/0x290 ? fs_reclaim_acquire+0x69/0xf0 ? kfree+0x22f/0x340 ? lock_release+0xee/0x310 ? kmalloc_trace_noprof+0x48/0x340 ? do_init_module+0x22/0x240 ? kmalloc_trace_noprof+0x155/0x340 ? do_init_module+0x60/0x240 ? __se_sys_finit_module+0x2e0/0x3f0 ? do_syscall_64+0xa4/0x180 ? syscall_exit_to_user_mode+0x108/0x140 ? do_syscall_64+0xb0/0x180 ? vma_end_read+0xd0/0xe0 ? do_user_addr_fault+0x309/0x640 ? clear_bhb_loop+0x45/0xa0 ? clear_bhb_loop+0x45/0xa0 ? clear_bhb_loop+0x45/0xa0 ? entry_SYSCALL_64_after_hwframe+0x76/0x7e Fix this by stubbing these macros out when this config option isn't enabled, along with fixing the unused variable warning that introduces. Signed-off-by: Lyude Paul Reviewed-by: Daniel Vetter Fixes: e2a1cda3e0c7 ("drm/panic: Add drm panic locking") Cc: # v6.10+ Link: https://patchwork.freedesktop.org/patch/msgid/20240916230103.611490-1-lyude@redhat.com --- drivers/gpu/drm/drm_atomic_helper.c | 2 +- include/drm/drm_panic.h | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 43cdf39019a4..5186d2114a50 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3015,7 +3015,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, bool stall) { int i, ret; - unsigned long flags; + unsigned long flags = 0; struct drm_connector *connector; struct drm_connector_state *old_conn_state, *new_conn_state; struct drm_crtc *crtc; diff --git a/include/drm/drm_panic.h b/include/drm/drm_panic.h index 54085d5d05c3..f4e1fa9ae607 100644 --- a/include/drm/drm_panic.h +++ b/include/drm/drm_panic.h @@ -64,6 +64,8 @@ struct drm_scanout_buffer { }; +#ifdef CONFIG_DRM_PANIC + /** * drm_panic_trylock - try to enter the panic printing critical section * @dev: struct drm_device @@ -149,4 +151,16 @@ struct drm_scanout_buffer { #define drm_panic_unlock(dev, flags) \ raw_spin_unlock_irqrestore(&(dev)->mode_config.panic_lock, flags) +#else + +static inline bool drm_panic_trylock(struct drm_device *dev, unsigned long flags) +{ + return true; +} + +static inline void drm_panic_lock(struct drm_device *dev, unsigned long flags) {} +static inline void drm_panic_unlock(struct drm_device *dev, unsigned long flags) {} + +#endif + #endif /* __DRM_PANIC_H__ */ -- 2.51.0 From 6e4f0d39fd52648a1ce580fc040fb2f008ec2ad9 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 16 Sep 2024 10:25:13 +0200 Subject: [PATCH 03/16] drm/ast: Rename register constants for TX-chip types The type of the TX chip is provided in VGACRD1. Rename the constants accordingly. Signed-off-by: Thomas Zimmermann Reviewed-by: Jocelyn Falempe Link: https://patchwork.freedesktop.org/patch/msgid/20240916082920.56234-2-tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_main.c | 4 ++-- drivers/gpu/drm/ast/ast_reg.h | 26 ++++++++++++-------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 3d92d9e5208f..d0e4f0dc9234 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -120,8 +120,8 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool need_post) ast->tx_chip = AST_TX_DP501; } } else if (IS_AST_GEN7(ast)) { - if (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xD1, TX_TYPE_MASK) == - ASTDP_DPMCU_TX) { + if (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd1, AST_IO_VGACRD1_TX_TYPE_MASK) == + AST_IO_VGACRD1_TX_ASTDP) { int ret = ast_dp_launch(ast); if (!ret) diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h index 6a1f756650ab..daa5d3a9e6a1 100644 --- a/drivers/gpu/drm/ast/ast_reg.h +++ b/drivers/gpu/drm/ast/ast_reg.h @@ -37,7 +37,18 @@ #define AST_IO_VGACRCB_HWC_16BPP BIT(0) /* set: ARGB4444, cleared: 2bpp palette */ #define AST_IO_VGACRCB_HWC_ENABLED BIT(1) -#define AST_IO_VGACRD1_MCU_FW_EXECUTING BIT(5) +#define AST_IO_VGACRD1_MCU_FW_EXECUTING BIT(5) +/* Display Transmitter Type */ +#define AST_IO_VGACRD1_TX_TYPE_MASK GENMASK(3, 1) +#define AST_IO_VGACRD1_NO_TX 0x00 +#define AST_IO_VGACRD1_TX_ITE66121_VBIOS 0x02 +#define AST_IO_VGACRD1_TX_SIL164_VBIOS 0x04 +#define AST_IO_VGACRD1_TX_CH7003_VBIOS 0x06 +#define AST_IO_VGACRD1_TX_DP501_VBIOS 0x08 +#define AST_IO_VGACRD1_TX_ANX9807_VBIOS 0x0a +#define AST_IO_VGACRD1_TX_FW_EMBEDDED_FW 0x0c +#define AST_IO_VGACRD1_TX_ASTDP 0x0e + #define AST_IO_VGACRD7_EDID_VALID_FLAG BIT(0) #define AST_IO_VGACRDC_LINK_SUCCESS BIT(0) #define AST_IO_VGACRDF_HPD BIT(0) @@ -49,19 +60,6 @@ #define AST_IO_VGAIR1_R (0x5A) #define AST_IO_VGAIR1_VREFRESH BIT(3) -/* - * Display Transmitter Type - */ - -#define TX_TYPE_MASK GENMASK(3, 1) -#define NO_TX (0 << 1) -#define ITE66121_VBIOS_TX (1 << 1) -#define SI164_VBIOS_TX (2 << 1) -#define CH7003_VBIOS_TX (3 << 1) -#define DP501_VBIOS_TX (4 << 1) -#define ANX9807_VBIOS_TX (5 << 1) -#define TX_FW_EMBEDDED_FW_TX (6 << 1) -#define ASTDP_DPMCU_TX (7 << 1) #define AST_VRAM_INIT_STATUS_MASK GENMASK(7, 6) //#define AST_VRAM_INIT_BY_BMC BIT(7) -- 2.51.0 From f93d66635fb3d4e3995dcc20acfa0498a2fa609d Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 16 Sep 2024 10:25:14 +0200 Subject: [PATCH 04/16] drm/ast: Use TX-chip register constants Replace magic values with named constants when reading the TX chip from VGACRD1. Signed-off-by: Thomas Zimmermann Reviewed-by: Jocelyn Falempe Link: https://patchwork.freedesktop.org/patch/msgid/20240916082920.56234-3-tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_dp501.c | 13 +++++++------ drivers/gpu/drm/ast/ast_main.c | 9 +++++---- drivers/gpu/drm/ast/ast_reg.h | 2 +- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c index e5553334bfde..9e19d8c17730 100644 --- a/drivers/gpu/drm/ast/ast_dp501.c +++ b/drivers/gpu/drm/ast/ast_dp501.c @@ -444,18 +444,19 @@ static void ast_init_analog(struct ast_device *ast) void ast_init_3rdtx(struct ast_device *ast) { - u8 jreg; + u8 vgacrd1; if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast)) { - jreg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd1, 0xff); - switch (jreg & 0x0e) { - case 0x04: + vgacrd1 = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd1, + AST_IO_VGACRD1_TX_TYPE_MASK); + switch (vgacrd1) { + case AST_IO_VGACRD1_TX_SIL164_VBIOS: ast_init_dvo(ast); break; - case 0x08: + case AST_IO_VGACRD1_TX_DP501_VBIOS: ast_launch_m68k(ast); break; - case 0x0c: + case AST_IO_VGACRD1_TX_FW_EMBEDDED_FW: ast_init_dvo(ast); break; default: diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index d0e4f0dc9234..7289bdc6066e 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -101,12 +101,13 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool need_post) * the SOC scratch register #1 bits 11:8 (interestingly marked * as "reserved" in the spec) */ - jreg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd1, 0xff); + jreg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd1, + AST_IO_VGACRD1_TX_TYPE_MASK); switch (jreg) { - case 0x04: + case AST_IO_VGACRD1_TX_SIL164_VBIOS: ast->tx_chip = AST_TX_SIL164; break; - case 0x08: + case AST_IO_VGACRD1_TX_DP501_VBIOS: ast->dp501_fw_addr = drmm_kzalloc(dev, 32*1024, GFP_KERNEL); if (ast->dp501_fw_addr) { /* backup firmware */ @@ -116,7 +117,7 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool need_post) } } fallthrough; - case 0x0c: + case AST_IO_VGACRD1_TX_FW_EMBEDDED_FW: ast->tx_chip = AST_TX_DP501; } } else if (IS_AST_GEN7(ast)) { diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h index daa5d3a9e6a1..2aadf07d135a 100644 --- a/drivers/gpu/drm/ast/ast_reg.h +++ b/drivers/gpu/drm/ast/ast_reg.h @@ -46,7 +46,7 @@ #define AST_IO_VGACRD1_TX_CH7003_VBIOS 0x06 #define AST_IO_VGACRD1_TX_DP501_VBIOS 0x08 #define AST_IO_VGACRD1_TX_ANX9807_VBIOS 0x0a -#define AST_IO_VGACRD1_TX_FW_EMBEDDED_FW 0x0c +#define AST_IO_VGACRD1_TX_FW_EMBEDDED_FW 0x0c /* special case of DP501 */ #define AST_IO_VGACRD1_TX_ASTDP 0x0e #define AST_IO_VGACRD7_EDID_VALID_FLAG BIT(0) -- 2.51.0 From a5c2320151ff7cdf9ec50630d638a417ff927e31 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 16 Sep 2024 10:25:15 +0200 Subject: [PATCH 05/16] drm/ast: Warn about unsupported TX chips A number of TX chips are listed in VGACRD1, but not supported by the ast driver. Whether any existing product uses such a chip is unknown. Warn if the driver encounters any. We can then add support as necessary. Signed-off-by: Thomas Zimmermann Suggested-by: Jocelyn Falempe Reviewed-by: Jocelyn Falempe Link: https://patchwork.freedesktop.org/patch/msgid/20240916082920.56234-4-tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_main.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 7289bdc6066e..bc37c65305d4 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -76,7 +76,22 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool need_post) }; struct drm_device *dev = &ast->base; - u8 jreg; + u8 jreg, vgacrd1; + + /* + * Several of the listed TX chips are not explicitly supported + * by the ast driver. If these exist in real-world devices, they + * are most likely reported as VGA or SIL164 outputs. We warn here + * to get bug reports for these devices. If none come in for some + * time, we can begin to fail device probing on these values. + */ + vgacrd1 = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd1, AST_IO_VGACRD1_TX_TYPE_MASK); + drm_WARN(dev, vgacrd1 == AST_IO_VGACRD1_TX_ITE66121_VBIOS, + "ITE IT66121 detected, 0x%x, Gen%lu\n", vgacrd1, AST_GEN(ast)); + drm_WARN(dev, vgacrd1 == AST_IO_VGACRD1_TX_CH7003_VBIOS, + "Chrontel CH7003 detected, 0x%x, Gen%lu\n", vgacrd1, AST_GEN(ast)); + drm_WARN(dev, vgacrd1 == AST_IO_VGACRD1_TX_ANX9807_VBIOS, + "Analogix ANX9807 detected, 0x%x, Gen%lu\n", vgacrd1, AST_GEN(ast)); /* Check 3rd Tx option (digital output afaik) */ ast->tx_chip = AST_TX_NONE; -- 2.51.0 From 87d45979140e49611696e97e2b33df572bf4fa24 Mon Sep 17 00:00:00 2001 From: Andrew Kreimer Date: Sun, 15 Sep 2024 15:39:43 +0300 Subject: [PATCH 06/16] drm/rockchip: Fix a typo Fix a typo in comments. Reported-by: Matthew Wilcox Signed-off-by: Andrew Kreimer Acked-by: Andy Yan Signed-off-by: Heiko Stuebner Link: https://patchwork.freedesktop.org/patch/msgid/20240915123943.105118-1-algonell@gmail.com --- drivers/gpu/drm/rockchip/cdn-dp-reg.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.h b/drivers/gpu/drm/rockchip/cdn-dp-reg.h index 441248b7a79e..c7780ae3272a 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.h +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.h @@ -77,7 +77,7 @@ #define SOURCE_PIF_PKT_ALLOC_WR_EN 0x30830 #define SOURCE_PIF_SW_RESET 0x30834 -/* bellow registers need access by mailbox */ +/* below registers need access by mailbox */ /* source car addr */ #define SOURCE_HDTX_CAR 0x0900 #define SOURCE_DPTX_CAR 0x0904 -- 2.51.0 From 3303a206ae7474b2f8a5d17d8df9de08bac16ca5 Mon Sep 17 00:00:00 2001 From: Jonas Karlman Date: Sun, 8 Sep 2024 14:54:58 +0000 Subject: [PATCH 07/16] drm/rockchip: dw_hdmi: Filter modes based on hdmiphy_clk RK3228 and RK3328 clock rate is being validated against a mpll config table intended for a Synopsys phy, and not the used inno-hdmi-phy. Instead get a reference to the hdmiphy clk and validate rates against it to enable use of HDMI2.0 modes, e.g. 4K@60Hz, on RK3228 and RK3328. For Synopsis phy the max_tmds_clock validation is sufficient. Signed-off-by: Jonas Karlman Tested-by: Diederik de Haas # Rock64 Signed-off-by: Heiko Stuebner Link: https://patchwork.freedesktop.org/patch/msgid/20240908145511.3331451-2-jonas@kwiboo.se --- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 35 ++++++++++----------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 240552eb517f..36cc700766fd 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -76,6 +76,7 @@ struct rockchip_hdmi { struct rockchip_encoder encoder; const struct rockchip_hdmi_chip_data *chip_data; const struct dw_hdmi_plat_data *plat_data; + struct clk *hdmiphy_clk; struct clk *ref_clk; struct clk *grf_clk; struct dw_hdmi *hdmi; @@ -251,10 +252,7 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data, const struct drm_display_mode *mode) { struct rockchip_hdmi *hdmi = data; - const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg; int pclk = mode->clock * 1000; - bool exact_match = hdmi->plat_data->phy_force_vendor; - int i; if (hdmi->chip_data->max_tmds_clock && mode->clock > hdmi->chip_data->max_tmds_clock) @@ -263,26 +261,18 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data, if (hdmi->ref_clk) { int rpclk = clk_round_rate(hdmi->ref_clk, pclk); - if (abs(rpclk - pclk) > pclk / 1000) + if (rpclk < 0 || abs(rpclk - pclk) > pclk / 1000) return MODE_NOCLOCK; } - for (i = 0; mpll_cfg[i].mpixelclock != (~0UL); i++) { - /* - * For vendor specific phys force an exact match of the pixelclock - * to preserve the original behaviour of the driver. - */ - if (exact_match && pclk == mpll_cfg[i].mpixelclock) - return MODE_OK; - /* - * The Synopsys phy can work with pixelclocks up to the value given - * in the corresponding mpll_cfg entry. - */ - if (!exact_match && pclk <= mpll_cfg[i].mpixelclock) - return MODE_OK; + if (hdmi->hdmiphy_clk) { + int rpclk = clk_round_rate(hdmi->hdmiphy_clk, pclk); + + if (rpclk < 0 || abs(rpclk - pclk) > pclk / 1000) + return MODE_NOCLOCK; } - return MODE_BAD; + return MODE_OK; } static void dw_hdmi_rockchip_encoder_disable(struct drm_encoder *encoder) @@ -607,6 +597,15 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master, return ret; } + if (hdmi->phy) { + struct of_phandle_args clkspec; + + clkspec.np = hdmi->phy->dev.of_node; + hdmi->hdmiphy_clk = of_clk_get_from_provider(&clkspec); + if (IS_ERR(hdmi->hdmiphy_clk)) + hdmi->hdmiphy_clk = NULL; + } + if (hdmi->chip_data == &rk3568_chip_data) { regmap_write(hdmi->regmap, RK3568_GRF_VO_CON1, HIWORD_UPDATE(RK3568_HDMI_SDAIN_MSK | -- 2.51.0 From 6e94e2871eb706a17692acf7ef85ecf2789f6433 Mon Sep 17 00:00:00 2001 From: Yakir Yang Date: Sun, 8 Sep 2024 14:54:59 +0000 Subject: [PATCH 08/16] drm/rockchip: dw_hdmi: Adjust cklvl & txlvl for RF/EMI Dut to the high HDMI signal voltage driver, Mickey have meet a serious RF/EMI problem, so we decided to reduce HDMI signal voltage to a proper value. The default params for phy is cklvl = 20 & txlvl = 13 (RF/EMI failed) ck: lvl = 13, term=100, vlo = 2.71, vhi=3.14, vswing = 0.43 tx: lvl = 20, term=100, vlo = 2.81, vhi=3.16, vswing = 0.35 1. We decided to reduce voltage value to lower, but VSwing still keep high, RF/EMI have been improved but still failed. ck: lvl = 6, term=100, vlo = 2.61, vhi=3.11, vswing = 0.50 tx: lvl = 6, term=100, vlo = 2.61, vhi=3.11, vswing = 0.50 2. We try to keep voltage value and vswing both lower, then RF/EMI test all passed ;) ck: lvl = 11, term= 66, vlo = 2.68, vhi=3.09, vswing = 0.40 tx: lvl = 11, term= 66, vlo = 2.68, vhi=3.09, vswing = 0.40 When we back to run HDMI different test and single-end test, we see different test passed, but signle-end test failed. The oscilloscope show that simgle-end clock's VL value is 1.78v (which remind LowLimit should not lower then 2.6v). 3. That's to say there are some different between PHY document and measure value. And according to experiment 2 results, we need to higher clock voltage and lower data voltage, then we can keep RF/EMI satisfied and single-end & differen test passed. ck: lvl = 9, term=100, vlo = 2.65, vhi=3.12, vswing = 0.47 tx: lvl = 16, term=100, vlo = 2.75, vhi=3.15, vswing = 0.39 Signed-off-by: Yakir Yang Signed-off-by: Jonas Karlman Signed-off-by: Heiko Stuebner Link: https://patchwork.freedesktop.org/patch/msgid/20240908145511.3331451-3-jonas@kwiboo.se --- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 36cc700766fd..fc5e87285a91 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -202,7 +202,7 @@ static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = { static const struct dw_hdmi_phy_config rockchip_phy_config[] = { /*pixelclk symbol term vlev*/ { 74250000, 0x8009, 0x0004, 0x0272}, - { 148500000, 0x802b, 0x0004, 0x028d}, + { 165000000, 0x802b, 0x0004, 0x0209}, { 297000000, 0x8039, 0x0005, 0x028d}, { ~0UL, 0x0000, 0x0000, 0x0000} }; -- 2.51.0 From b60c86d305f46483d3ed0743e9ec97a76addcabc Mon Sep 17 00:00:00 2001 From: Nickey Yang Date: Sun, 8 Sep 2024 14:55:00 +0000 Subject: [PATCH 09/16] drm/rockchip: dw_hdmi: Add phy_config for 594Mhz pixel clock Add phy_config for 594Mhz pixel clock used for HDMI2.0 display modes. Signed-off-by: Nickey Yang Signed-off-by: Jonas Karlman Signed-off-by: Heiko Stuebner Link: https://patchwork.freedesktop.org/patch/msgid/20240908145511.3331451-4-jonas@kwiboo.se --- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index fc5e87285a91..dffbae005a96 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -204,6 +204,7 @@ static const struct dw_hdmi_phy_config rockchip_phy_config[] = { { 74250000, 0x8009, 0x0004, 0x0272}, { 165000000, 0x802b, 0x0004, 0x0209}, { 297000000, 0x8039, 0x0005, 0x028d}, + { 594000000, 0x8039, 0x0000, 0x019d}, { ~0UL, 0x0000, 0x0000, 0x0000} }; -- 2.51.0 From 7d324630f3515bd6e11cadeb1d748bd74ecc9664 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Sun, 8 Sep 2024 14:55:01 +0000 Subject: [PATCH 10/16] drm/rockchip: dw_hdmi: Set cur_ctr to 0 always Jitter was improved by lowering the MPLL bandwidth to account for high frequency noise in the rk3288 PLL. In each case MPLL bandwidth was lowered only enough to get us a comfortable margin. We believe that lowering the bandwidth like this is safe given sufficient testing. Signed-off-by: Douglas Anderson Signed-off-by: Yakir Yang Signed-off-by: Jonas Karlman Signed-off-by: Heiko Stuebner Link: https://patchwork.freedesktop.org/patch/msgid/20240908145511.3331451-5-jonas@kwiboo.se --- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index dffbae005a96..a050a65af8f2 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -179,23 +179,9 @@ static const struct dw_hdmi_mpll_config rockchip_mpll_cfg[] = { static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = { /* pixelclk bpp8 bpp10 bpp12 */ { - 40000000, { 0x0018, 0x0018, 0x0018 }, - }, { - 65000000, { 0x0028, 0x0028, 0x0028 }, - }, { - 66000000, { 0x0038, 0x0038, 0x0038 }, - }, { - 74250000, { 0x0028, 0x0038, 0x0038 }, - }, { - 83500000, { 0x0028, 0x0038, 0x0038 }, - }, { - 146250000, { 0x0038, 0x0038, 0x0038 }, - }, { - 148500000, { 0x0000, 0x0038, 0x0038 }, - }, { 600000000, { 0x0000, 0x0000, 0x0000 }, }, { - ~0UL, { 0x0000, 0x0000, 0x0000}, + ~0UL, { 0x0000, 0x0000, 0x0000 }, } }; -- 2.51.0 From 7595c7ef17ffe70d0f4fdda01f87f105a12de66b Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Sun, 8 Sep 2024 14:55:02 +0000 Subject: [PATCH 11/16] drm/rockchip: dw_hdmi: Use auto-generated tables The previous tables for mpll_cfg and curr_ctrl were created using the 20-pages of example settings provided by the PHY vendor. Those example settings weren't particularly dense, so there were places where we were guessing what the settings would be for 10-bit and 12-bit (not that we use those anyway). It was also always a lot of extra work every time we wanted to add a new clock rate since we had to cross-reference several tables. In I've gone through the work to figure out how to generate this table automatically. Let's now use the automatically generated table and then we'll never need to look at it again. We only support 8-bit mode right now and only support a small number of clock rates and I've verified that the only 8-bit rate that was affected was 148.5. That mode appears to have been wrong in the old table. Signed-off-by: Douglas Anderson Signed-off-by: Yakir Yang Signed-off-by: Jonas Karlman Signed-off-by: Heiko Stuebner Link: https://patchwork.freedesktop.org/patch/msgid/20240908145511.3331451-6-jonas@kwiboo.se --- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 104 ++++++++++---------- 1 file changed, 53 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index a050a65af8f2..090d8c0f306f 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -92,74 +92,70 @@ static struct rockchip_hdmi *to_rockchip_hdmi(struct drm_encoder *encoder) static const struct dw_hdmi_mpll_config rockchip_mpll_cfg[] = { { - 27000000, { - { 0x00b3, 0x0000}, - { 0x2153, 0x0000}, - { 0x40f3, 0x0000} + 30666000, { + { 0x00b3, 0x0000 }, + { 0x2153, 0x0000 }, + { 0x40f3, 0x0000 }, }, }, { - 36000000, { - { 0x00b3, 0x0000}, - { 0x2153, 0x0000}, - { 0x40f3, 0x0000} + 36800000, { + { 0x00b3, 0x0000 }, + { 0x2153, 0x0000 }, + { 0x40a2, 0x0001 }, }, }, { - 40000000, { - { 0x00b3, 0x0000}, - { 0x2153, 0x0000}, - { 0x40f3, 0x0000} + 46000000, { + { 0x00b3, 0x0000 }, + { 0x2142, 0x0001 }, + { 0x40a2, 0x0001 }, }, }, { - 54000000, { - { 0x0072, 0x0001}, - { 0x2142, 0x0001}, - { 0x40a2, 0x0001}, + 61333000, { + { 0x0072, 0x0001 }, + { 0x2142, 0x0001 }, + { 0x40a2, 0x0001 }, }, }, { - 65000000, { - { 0x0072, 0x0001}, - { 0x2142, 0x0001}, - { 0x40a2, 0x0001}, + 73600000, { + { 0x0072, 0x0001 }, + { 0x2142, 0x0001 }, + { 0x4061, 0x0002 }, }, }, { - 66000000, { - { 0x013e, 0x0003}, - { 0x217e, 0x0002}, - { 0x4061, 0x0002} + 92000000, { + { 0x0072, 0x0001 }, + { 0x2145, 0x0002 }, + { 0x4061, 0x0002 }, }, }, { - 74250000, { - { 0x0072, 0x0001}, - { 0x2145, 0x0002}, - { 0x4061, 0x0002} + 122666000, { + { 0x0051, 0x0002 }, + { 0x2145, 0x0002 }, + { 0x4061, 0x0002 }, }, }, { - 83500000, { - { 0x0072, 0x0001}, + 147200000, { + { 0x0051, 0x0002 }, + { 0x2145, 0x0002 }, + { 0x4064, 0x0003 }, }, }, { - 108000000, { - { 0x0051, 0x0002}, - { 0x2145, 0x0002}, - { 0x4061, 0x0002} + 184000000, { + { 0x0051, 0x0002 }, + { 0x214c, 0x0003 }, + { 0x4064, 0x0003 }, }, }, { - 106500000, { - { 0x0051, 0x0002}, - { 0x2145, 0x0002}, - { 0x4061, 0x0002} - }, - }, { - 146250000, { - { 0x0051, 0x0002}, - { 0x2145, 0x0002}, - { 0x4061, 0x0002} + 226666000, { + { 0x0040, 0x0003 }, + { 0x214c, 0x0003 }, + { 0x4064, 0x0003 }, }, }, { - 148500000, { - { 0x0051, 0x0003}, - { 0x214c, 0x0003}, - { 0x4064, 0x0003} + 272000000, { + { 0x0040, 0x0003 }, + { 0x214c, 0x0003 }, + { 0x5a64, 0x0003 }, }, }, { 340000000, { @@ -167,11 +163,17 @@ static const struct dw_hdmi_mpll_config rockchip_mpll_cfg[] = { { 0x3b4c, 0x0003 }, { 0x5a64, 0x0003 }, }, + }, { + 600000000, { + { 0x1a40, 0x0003 }, + { 0x3b4c, 0x0003 }, + { 0x5a64, 0x0003 }, + }, }, { ~0UL, { - { 0x00a0, 0x000a }, - { 0x2001, 0x000f }, - { 0x4002, 0x000f }, + { 0x0000, 0x0000 }, + { 0x0000, 0x0000 }, + { 0x0000, 0x0000 }, }, } }; -- 2.51.0 From 28f0ae48e7fdbd6cdcf3972c8d8686a529ae1ede Mon Sep 17 00:00:00 2001 From: Jonas Karlman Date: Sun, 8 Sep 2024 14:55:03 +0000 Subject: [PATCH 12/16] drm/rockchip: dw_hdmi: Enable 4K@60Hz mode on RK3399 and RK356x Use a maximum TMDS clock rate limit of 594MHz to enable use of HDMI2.0 modes, e.g. 4K@60Hz, on RK3399 and RK3568. Signed-off-by: Jonas Karlman Tested-by: Diederik de Haas # Quartz64 Model B Signed-off-by: Heiko Stuebner Link: https://patchwork.freedesktop.org/patch/msgid/20240908145511.3331451-7-jonas@kwiboo.se --- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 090d8c0f306f..96e1097f993d 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -481,7 +481,7 @@ static struct rockchip_hdmi_chip_data rk3399_chip_data = { .lcdsel_grf_reg = RK3399_GRF_SOC_CON20, .lcdsel_big = HIWORD_UPDATE(0, RK3399_HDMI_LCDC_SEL), .lcdsel_lit = HIWORD_UPDATE(RK3399_HDMI_LCDC_SEL, RK3399_HDMI_LCDC_SEL), - .max_tmds_clock = 340000, + .max_tmds_clock = 594000, }; static const struct dw_hdmi_plat_data rk3399_hdmi_drv_data = { @@ -495,7 +495,7 @@ static const struct dw_hdmi_plat_data rk3399_hdmi_drv_data = { static struct rockchip_hdmi_chip_data rk3568_chip_data = { .lcdsel_grf_reg = -1, - .max_tmds_clock = 340000, + .max_tmds_clock = 594000, }; static const struct dw_hdmi_plat_data rk3568_hdmi_drv_data = { -- 2.51.0 From 0c4558a1bc2df9b6e6fb311de9cab192b0943426 Mon Sep 17 00:00:00 2001 From: Jonas Karlman Date: Sun, 8 Sep 2024 14:55:04 +0000 Subject: [PATCH 13/16] drm/rockchip: Load crtc devices in preferred order On RK3399 the VOPL is loaded before VOPB and get registered as crtc-0. However, on RK3288 and PX30 VOPB is gets registered as crtc-0 instead of VOPL. With VOPL registered as crtc-0 the kernel kms client is not able to enable 4K display modes for console use on RK3399. Load VOPB before VOPL to help kernel kms client make use of 4K display modes for console use on RK3399. Signed-off-by: Jonas Karlman Signed-off-by: Heiko Stuebner Link: https://patchwork.freedesktop.org/patch/msgid/20240908145511.3331451-8-jonas@kwiboo.se --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 23 +++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index 44d769d9234d..b84451d59187 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -354,11 +354,34 @@ static void rockchip_drm_match_remove(struct device *dev) device_link_del(link); } +/* list of preferred vop devices */ +static const char *const rockchip_drm_match_preferred[] = { + "rockchip,rk3399-vop-big", + NULL, +}; + static struct component_match *rockchip_drm_match_add(struct device *dev) { struct component_match *match = NULL; + struct device_node *port; int i; + /* add preferred vop device match before adding driver device matches */ + for (i = 0; ; i++) { + port = of_parse_phandle(dev->of_node, "ports", i); + if (!port) + break; + + if (of_device_is_available(port->parent) && + of_device_compatible_match(port->parent, + rockchip_drm_match_preferred)) + drm_of_component_match_add(dev, &match, + component_compare_of, + port->parent); + + of_node_put(port); + } + for (i = 0; i < num_rockchip_sub_drivers; i++) { struct platform_driver *drv = rockchip_sub_drivers[i]; struct device *p = NULL, *d; -- 2.51.0 From 2facdd6002ad67357dd7f77a388ae602bc910ace Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20Hellstr=C3=B6m?= Date: Fri, 28 Apr 2023 14:52:32 +0200 Subject: [PATCH 14/16] dma-buf/dma-fence: Use a successful read_trylock() annotation for dma_fence_begin_signalling() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Condsider the following call sequence: /* Upper layer */ dma_fence_begin_signalling(); lock(tainted_shared_lock); /* Driver callback */ dma_fence_begin_signalling(); ... The driver might here use a utility that is annotated as intended for the dma-fence signalling critical path. Now if the upper layer isn't correctly annotated yet for whatever reason, resulting in /* Upper layer */ lock(tainted_shared_lock); /* Driver callback */ dma_fence_begin_signalling(); We will receive a false lockdep locking order violation notification from dma_fence_begin_signalling(). However entering a dma-fence signalling critical section itself doesn't block and could not cause a deadlock. So use a successful read_trylock() annotation instead for dma_fence_begin_signalling(). That will make sure that the locking order is correctly registered in the first case, and doesn't register any locking order in the second case. The alternative is of course to make sure that the "Upper layer" is always correctly annotated. But experience shows that's not easily achievable in all cases. Signed-off-by: Thomas Hellström Reviewed-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20230428125233.228353-1-thomas.hellstrom@linux.intel.com --- drivers/dma-buf/dma-fence.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 0393a9bba3a8..f8303ae99acf 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -309,8 +309,8 @@ bool dma_fence_begin_signalling(void) if (in_atomic()) return true; - /* ... and non-recursive readlock */ - lock_acquire(&dma_fence_lockdep_map, 0, 0, 1, 1, NULL, _RET_IP_); + /* ... and non-recursive successful read_trylock */ + lock_acquire(&dma_fence_lockdep_map, 0, 1, 1, 1, NULL, _RET_IP_); return false; } @@ -341,7 +341,7 @@ void __dma_fence_might_wait(void) lock_map_acquire(&dma_fence_lockdep_map); lock_map_release(&dma_fence_lockdep_map); if (tmp) - lock_acquire(&dma_fence_lockdep_map, 0, 0, 1, 1, NULL, _THIS_IP_); + lock_acquire(&dma_fence_lockdep_map, 0, 1, 1, 1, NULL, _THIS_IP_); } #endif -- 2.51.0 From 79cc4d2bf2c7b90d9f06e3f67e541978e96b9a64 Mon Sep 17 00:00:00 2001 From: Vivek Kasireddy Date: Wed, 21 Aug 2024 21:58:06 -0700 Subject: [PATCH 15/16] MAINTAINERS: udmabuf: Add myself as co-maintainer for udmabuf driver I would like to help maintain the udmabuf driver, in light of the recent changes that converted the driver to use folios instead of pages. Furthermore, I also contribute to Qemu's virtio-gpu module (and UI modules), that are primary users of udmabuf driver. Cc: Gerd Hoffmann Cc: Daniel Vetter Acked-by: Gerd Hoffmann Signed-off-by: Vivek Kasireddy Link: https://patchwork.freedesktop.org/patch/msgid/20240822045806.3563883-1-vivek.kasireddy@intel.com --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 333ed0718175..77efbded2bfe 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -23912,6 +23912,7 @@ F: lib/iov_iter.c USERSPACE DMA BUFFER DRIVER M: Gerd Hoffmann +M: Vivek Kasireddy L: dri-devel@lists.freedesktop.org S: Maintained T: git https://gitlab.freedesktop.org/drm/misc/kernel.git -- 2.51.0 From f0bbcc258e81288212c2092c587ae06428196598 Mon Sep 17 00:00:00 2001 From: Huan Yang Date: Wed, 18 Sep 2024 10:52:24 +0800 Subject: [PATCH 16/16] udmabuf: pre-fault when first page fault The current udmabuf mmap only fills the physical memory to the corresponding virtual address when the user actually accesses the virtual address. However, the current udmabuf has already obtained and pinned the folio upon completion of the creation.This means that the physical memory has already been acquired, rather than being accessed dynamically. As a result, the page fault has lost its purpose as a demanding page. Due to the fact that page fault requires trapping into kernel mode and filling in when accessing the corresponding virtual address in mmap, when creating a large size udmabuf, this represents a considerable overhead. This patch fill the pfn into page table, and then pre-fault each pfn into vma, when first access. Notice, if anything wrong , we do not return an error during this pre-fault step. However, an error will be returned if the failure occurs when the addr is truly accessed Suggested-by: Vivek Kasireddy Signed-off-by: Huan Yang Acked-by: Vivek Kasireddy Signed-off-by: Vivek Kasireddy Link: https://patchwork.freedesktop.org/patch/msgid/20240918025238.2957823-2-link@vivo.com --- drivers/dma-buf/udmabuf.c | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 047c3cd2ceff..2170d975cc76 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -43,7 +43,8 @@ static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) struct vm_area_struct *vma = vmf->vma; struct udmabuf *ubuf = vma->vm_private_data; pgoff_t pgoff = vmf->pgoff; - unsigned long pfn; + unsigned long addr, pfn; + vm_fault_t ret; if (pgoff >= ubuf->pagecount) return VM_FAULT_SIGBUS; @@ -51,7 +52,35 @@ static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) pfn = folio_pfn(ubuf->folios[pgoff]); pfn += ubuf->offsets[pgoff] >> PAGE_SHIFT; - return vmf_insert_pfn(vma, vmf->address, pfn); + ret = vmf_insert_pfn(vma, vmf->address, pfn); + if (ret & VM_FAULT_ERROR) + return ret; + + /* pre fault */ + pgoff = vma->vm_pgoff; + addr = vma->vm_start; + + for (; addr < vma->vm_end; pgoff++, addr += PAGE_SIZE) { + if (addr == vmf->address) + continue; + + if (WARN_ON(pgoff >= ubuf->pagecount)) + break; + + pfn = folio_pfn(ubuf->folios[pgoff]); + pfn += ubuf->offsets[pgoff] >> PAGE_SHIFT; + + /** + * If the below vmf_insert_pfn() fails, we do not return an + * error here during this pre-fault step. However, an error + * will be returned if the failure occurs when the addr is + * truly accessed. + */ + if (vmf_insert_pfn(vma, addr, pfn) & VM_FAULT_ERROR) + break; + } + + return ret; } static const struct vm_operations_struct udmabuf_vm_ops = { -- 2.51.0