* [Intel-xe] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path @ 2023-04-18 16:43 José Roberto de Souza 2023-04-18 16:46 ` [Intel-xe] ✗ CI.Patch_applied: failure for drm/i915: Initialize dkl_phy spin lock from display code path (rev3) Patchwork 2023-04-19 6:30 ` [Intel-xe] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path Lucas De Marchi 0 siblings, 2 replies; 11+ messages in thread From: José Roberto de Souza @ 2023-04-18 16:43 UTC (permalink / raw) To: intel-gfx; +Cc: Jani Nikula, Rodrigo Vivi, intel-xe, Ville Syrjälä Start to move the initialization of display locks from i915_driver_early_probe(), this way it is also executed when running Xe kmd. This will fix a warning in Xe kmd: [ 201.894839] xe 0000:00:02.0: [drm] [ENCODER:235:DDI A/PHY A] failed to retrieve link info, disabling eDP [ 202.136336] xe 0000:00:02.0: [drm] *ERROR* Failed to write source OUI [ 202.175346] INFO: trying to register non-static key. [ 202.175347] irq event stamp: 754060 [ 202.175359] hardirqs last enabled at (754059): [<ffffffff8122cf79>] tick_nohz_idle_enter+0x59/0x80 [ 202.180294] The code is fine but needs lockdep annotation, or maybe [ 202.183774] hardirqs last disabled at (754060): [<ffffffff811a5539>] do_idle+0x99/0x230 [ 202.192734] you didn't initialize this object before use? [ 202.198951] softirqs last enabled at (753948): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 [ 202.206882] turning off the locking correctness validator. [ 202.212236] softirqs last disabled at (753943): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 [ 202.220592] CPU: 2 PID: 1415 Comm: modprobe Tainted: G W 6.3.0-rc4+zeh-xe+ #909 [ 202.243002] Hardware name: Intel Corporation Raptor Lake Client Platform/RaptorLake-P LP5 RVP, BIOS RPLPFWI1.R00.3361.A14.2211151548 11/15/2022 [ 202.255737] Call Trace: [ 202.258179] <TASK> [ 202.260275] dump_stack_lvl+0x58/0xc0 [ 202.263922] register_lock_class+0x756/0x7d0 [ 202.268165] ? find_held_lock+0x2b/0x80 [ 202.271975] __lock_acquire+0x72/0x28b0 [ 202.275786] ? debug_object_free+0xb4/0x160 [ 202.279946] lock_acquire+0xd1/0x2d0 [ 202.283503] ? intel_dkl_phy_read+0x18/0x60 [xe] [ 202.288181] _raw_spin_lock+0x2a/0x40 [ 202.291825] ? intel_dkl_phy_read+0x18/0x60 [xe] [ 202.296475] intel_dkl_phy_read+0x18/0x60 [xe] [ 202.300949] icl_aux_power_well_enable+0x2bd/0x400 [xe] [ 202.306202] ? intel_display_power_grab_async_put_ref+0x75/0x120 [xe] [ 202.312649] intel_power_well_enable+0x1c/0x70 [xe] [ 202.317543] __intel_display_power_get_domain.part.0+0x4d/0x70 [xe] [ 202.323812] intel_display_power_get+0x43/0x70 [xe] [ 202.328708] intel_tc_port_init+0x199/0x2a0 [xe] [ 202.333363] intel_ddi_init+0x6ad/0xb00 [xe] [ 202.337678] intel_modeset_init_nogem+0x536/0x6d0 [xe] [ 202.342838] xe_display_init_noaccel+0x19/0x40 [xe] [ 202.347743] xe_device_probe+0x1f5/0x2a0 [xe] [ 202.352127] xe_pci_probe+0x28c/0x480 [xe] [ 202.356260] pci_device_probe+0x9d/0x150 [ 202.360164] really_probe+0x19a/0x400 [ 202.363809] ? __pfx___driver_attach+0x10/0x10 [ 202.368226] __driver_probe_device+0x73/0x170 [ 202.372558] driver_probe_device+0x1a/0x90 [ 202.376632] __driver_attach+0xcd/0x1c0 [ 202.380442] bus_for_each_dev+0x72/0xc0 [ 202.384253] bus_add_driver+0x110/0x210 [ 202.388063] driver_register+0x50/0x100 [ 202.391873] ? __pfx_init_module+0x10/0x10 [xe] [ 202.396431] do_one_initcall+0x55/0x260 [ 202.400245] ? rcu_is_watching+0xd/0x40 [ 202.404058] ? kmalloc_trace+0xa0/0xb0 [ 202.407786] do_init_module+0x45/0x1e0 [ 202.411512] __do_sys_finit_module+0xac/0x120 [ 202.415838] do_syscall_64+0x37/0x90 [ 202.419397] entry_SYSCALL_64_after_hwframe+0x72/0xdc [ 202.424409] RIP: 0033:0x7fd11291ea3d [ 202.427967] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 a3 0f 00 f7 d8 64 89 01 48 [ 202.446530] RSP: 002b:00007ffffde11368 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 [ 202.454031] RAX: ffffffffffffffda RBX: 00005616a617f210 RCX: 00007fd11291ea3d [ 202.461106] RDX: 0000000000000000 RSI: 00005616a617fe60 RDI: 000000000000000e [ 202.468182] RBP: 0000000000040000 R08: 0000000000000000 R09: 0000000000000002 [ 202.475250] R10: 000000000000000e R11: 0000000000000246 R12: 00005616a617fe60 [ 202.482319] R13: 00005616a617f340 R14: 0000000000000000 R15: 00005616a6180650 [ 202.489396] </TASK> v2: - added intel_display_locks_init() v3: - rebased v4: - drop intel_display_locks_init() Cc: intel-gfx@lists.freedesktop.org Cc: intel-xe@lists.freedesktop.org Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/display/intel_display_driver.c | 2 ++ drivers/gpu/drm/i915/display/intel_dkl_phy.c | 5 +++++ drivers/gpu/drm/i915/display/intel_dkl_phy.h | 1 + drivers/gpu/drm/i915/i915_driver.c | 1 - 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c index b3dbfe2a892e6..60ce10fc72058 100644 --- a/drivers/gpu/drm/i915/display/intel_display_driver.c +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c @@ -30,6 +30,7 @@ #include "intel_display_driver.h" #include "intel_display_power.h" #include "intel_display_types.h" +#include "intel_dkl_phy.h" #include "intel_dmc.h" #include "intel_dp.h" #include "intel_dpll.h" @@ -175,6 +176,7 @@ void intel_display_driver_early_probe(struct drm_i915_private *i915) if (!HAS_DISPLAY(i915)) return; + intel_dkl_phy_init(i915); intel_color_init_hooks(i915); intel_init_cdclk_hooks(i915); intel_audio_hooks_init(i915); diff --git a/drivers/gpu/drm/i915/display/intel_dkl_phy.c b/drivers/gpu/drm/i915/display/intel_dkl_phy.c index 57cc3edba0163..69d863dfb3a03 100644 --- a/drivers/gpu/drm/i915/display/intel_dkl_phy.c +++ b/drivers/gpu/drm/i915/display/intel_dkl_phy.c @@ -104,3 +104,8 @@ intel_dkl_phy_posting_read(struct drm_i915_private *i915, struct intel_dkl_phy_r spin_unlock(&i915->display.dkl.phy_lock); } + +void intel_dkl_phy_init(struct drm_i915_private *i915) +{ + spin_lock_init(&i915->display.dkl.phy_lock); +} diff --git a/drivers/gpu/drm/i915/display/intel_dkl_phy.h b/drivers/gpu/drm/i915/display/intel_dkl_phy.h index 570ee36f9386f..a0183d322e058 100644 --- a/drivers/gpu/drm/i915/display/intel_dkl_phy.h +++ b/drivers/gpu/drm/i915/display/intel_dkl_phy.h @@ -20,5 +20,6 @@ void intel_dkl_phy_rmw(struct drm_i915_private *i915, struct intel_dkl_phy_reg reg, u32 clear, u32 set); void intel_dkl_phy_posting_read(struct drm_i915_private *i915, struct intel_dkl_phy_reg reg); +void intel_dkl_phy_init(struct drm_i915_private *i915); #endif /* __INTEL_DKL_PHY_H__ */ diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index a52db8a809006..fd198700272b1 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -223,7 +223,6 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv) mutex_init(&dev_priv->display.wm.wm_mutex); mutex_init(&dev_priv->display.pps.mutex); mutex_init(&dev_priv->display.hdcp.comp_mutex); - spin_lock_init(&dev_priv->display.dkl.phy_lock); i915_memcpy_init_early(dev_priv); intel_runtime_pm_init_early(&dev_priv->runtime_pm); -- 2.40.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Intel-xe] ✗ CI.Patch_applied: failure for drm/i915: Initialize dkl_phy spin lock from display code path (rev3) 2023-04-18 16:43 [Intel-xe] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path José Roberto de Souza @ 2023-04-18 16:46 ` Patchwork 2023-04-19 6:30 ` [Intel-xe] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path Lucas De Marchi 1 sibling, 0 replies; 11+ messages in thread From: Patchwork @ 2023-04-18 16:46 UTC (permalink / raw) To: José Roberto de Souza; +Cc: intel-xe == Series Details == Series: drm/i915: Initialize dkl_phy spin lock from display code path (rev3) URL : https://patchwork.freedesktop.org/series/116326/ State : failure == Summary == === Applying kernel patches on branch 'drm-xe-next' with base: === Base commit: 5c309c81e fixup! drm/i915/display: Remaining changes to make xe compile === git am output follows === error: drivers/gpu/drm/i915/display/intel_display_driver.c: does not exist in index hint: Use 'git am --show-current-patch' to see the failed patch Applying: drm/i915: Initialize dkl_phy spin lock from display code path Patch failed at 0001 drm/i915: Initialize dkl_phy spin lock from display code path When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-xe] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path 2023-04-18 16:43 [Intel-xe] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path José Roberto de Souza 2023-04-18 16:46 ` [Intel-xe] ✗ CI.Patch_applied: failure for drm/i915: Initialize dkl_phy spin lock from display code path (rev3) Patchwork @ 2023-04-19 6:30 ` Lucas De Marchi 2023-04-19 6:41 ` Lucas De Marchi 1 sibling, 1 reply; 11+ messages in thread From: Lucas De Marchi @ 2023-04-19 6:30 UTC (permalink / raw) To: José Roberto de Souza Cc: Jani Nikula, intel-gfx, intel-xe, Ville Syrjälä, Rodrigo Vivi On Tue, Apr 18, 2023 at 09:43:37AM -0700, Jose Souza wrote: >Start to move the initialization of display locks from >i915_driver_early_probe(), this way it is also executed when running >Xe kmd. > >This will fix a warning in Xe kmd: > >[ 201.894839] xe 0000:00:02.0: [drm] [ENCODER:235:DDI A/PHY A] failed to retrieve link info, disabling eDP >[ 202.136336] xe 0000:00:02.0: [drm] *ERROR* Failed to write source OUI >[ 202.175346] INFO: trying to register non-static key. >[ 202.175347] irq event stamp: 754060 >[ 202.175359] hardirqs last enabled at (754059): [<ffffffff8122cf79>] tick_nohz_idle_enter+0x59/0x80 >[ 202.180294] The code is fine but needs lockdep annotation, or maybe >[ 202.183774] hardirqs last disabled at (754060): [<ffffffff811a5539>] do_idle+0x99/0x230 >[ 202.192734] you didn't initialize this object before use? >[ 202.198951] softirqs last enabled at (753948): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 >[ 202.206882] turning off the locking correctness validator. >[ 202.212236] softirqs last disabled at (753943): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 >[ 202.220592] CPU: 2 PID: 1415 Comm: modprobe Tainted: G W 6.3.0-rc4+zeh-xe+ #909 >[ 202.243002] Hardware name: Intel Corporation Raptor Lake Client Platform/RaptorLake-P LP5 RVP, BIOS RPLPFWI1.R00.3361.A14.2211151548 11/15/2022 >[ 202.255737] Call Trace: >[ 202.258179] <TASK> >[ 202.260275] dump_stack_lvl+0x58/0xc0 >[ 202.263922] register_lock_class+0x756/0x7d0 >[ 202.268165] ? find_held_lock+0x2b/0x80 >[ 202.271975] __lock_acquire+0x72/0x28b0 >[ 202.275786] ? debug_object_free+0xb4/0x160 >[ 202.279946] lock_acquire+0xd1/0x2d0 >[ 202.283503] ? intel_dkl_phy_read+0x18/0x60 [xe] >[ 202.288181] _raw_spin_lock+0x2a/0x40 >[ 202.291825] ? intel_dkl_phy_read+0x18/0x60 [xe] >[ 202.296475] intel_dkl_phy_read+0x18/0x60 [xe] >[ 202.300949] icl_aux_power_well_enable+0x2bd/0x400 [xe] >[ 202.306202] ? intel_display_power_grab_async_put_ref+0x75/0x120 [xe] >[ 202.312649] intel_power_well_enable+0x1c/0x70 [xe] >[ 202.317543] __intel_display_power_get_domain.part.0+0x4d/0x70 [xe] >[ 202.323812] intel_display_power_get+0x43/0x70 [xe] >[ 202.328708] intel_tc_port_init+0x199/0x2a0 [xe] >[ 202.333363] intel_ddi_init+0x6ad/0xb00 [xe] >[ 202.337678] intel_modeset_init_nogem+0x536/0x6d0 [xe] >[ 202.342838] xe_display_init_noaccel+0x19/0x40 [xe] >[ 202.347743] xe_device_probe+0x1f5/0x2a0 [xe] >[ 202.352127] xe_pci_probe+0x28c/0x480 [xe] >[ 202.356260] pci_device_probe+0x9d/0x150 >[ 202.360164] really_probe+0x19a/0x400 >[ 202.363809] ? __pfx___driver_attach+0x10/0x10 >[ 202.368226] __driver_probe_device+0x73/0x170 >[ 202.372558] driver_probe_device+0x1a/0x90 >[ 202.376632] __driver_attach+0xcd/0x1c0 >[ 202.380442] bus_for_each_dev+0x72/0xc0 >[ 202.384253] bus_add_driver+0x110/0x210 >[ 202.388063] driver_register+0x50/0x100 >[ 202.391873] ? __pfx_init_module+0x10/0x10 [xe] >[ 202.396431] do_one_initcall+0x55/0x260 >[ 202.400245] ? rcu_is_watching+0xd/0x40 >[ 202.404058] ? kmalloc_trace+0xa0/0xb0 >[ 202.407786] do_init_module+0x45/0x1e0 >[ 202.411512] __do_sys_finit_module+0xac/0x120 >[ 202.415838] do_syscall_64+0x37/0x90 >[ 202.419397] entry_SYSCALL_64_after_hwframe+0x72/0xdc >[ 202.424409] RIP: 0033:0x7fd11291ea3d >[ 202.427967] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 a3 0f 00 f7 d8 64 89 01 48 >[ 202.446530] RSP: 002b:00007ffffde11368 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 >[ 202.454031] RAX: ffffffffffffffda RBX: 00005616a617f210 RCX: 00007fd11291ea3d >[ 202.461106] RDX: 0000000000000000 RSI: 00005616a617fe60 RDI: 000000000000000e >[ 202.468182] RBP: 0000000000040000 R08: 0000000000000000 R09: 0000000000000002 >[ 202.475250] R10: 000000000000000e R11: 0000000000000246 R12: 00005616a617fe60 >[ 202.482319] R13: 00005616a617f340 R14: 0000000000000000 R15: 00005616a6180650 >[ 202.489396] </TASK> > >v2: >- added intel_display_locks_init() > >v3: >- rebased > >v4: >- drop intel_display_locks_init() why? Lucas De Marchi ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-xe] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path 2023-04-19 6:30 ` [Intel-xe] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path Lucas De Marchi @ 2023-04-19 6:41 ` Lucas De Marchi 2023-04-19 7:16 ` Jani Nikula 0 siblings, 1 reply; 11+ messages in thread From: Lucas De Marchi @ 2023-04-19 6:41 UTC (permalink / raw) To: José Roberto de Souza Cc: Jani Nikula, intel-gfx, intel-xe, Ville Syrjälä, Rodrigo Vivi On Tue, Apr 18, 2023 at 11:30:18PM -0700, Lucas De Marchi wrote: >On Tue, Apr 18, 2023 at 09:43:37AM -0700, Jose Souza wrote: >>Start to move the initialization of display locks from >>i915_driver_early_probe(), this way it is also executed when running >>Xe kmd. >> >>This will fix a warning in Xe kmd: >> >>[ 201.894839] xe 0000:00:02.0: [drm] [ENCODER:235:DDI A/PHY A] failed to retrieve link info, disabling eDP >>[ 202.136336] xe 0000:00:02.0: [drm] *ERROR* Failed to write source OUI >>[ 202.175346] INFO: trying to register non-static key. >>[ 202.175347] irq event stamp: 754060 >>[ 202.175359] hardirqs last enabled at (754059): [<ffffffff8122cf79>] tick_nohz_idle_enter+0x59/0x80 >>[ 202.180294] The code is fine but needs lockdep annotation, or maybe >>[ 202.183774] hardirqs last disabled at (754060): [<ffffffff811a5539>] do_idle+0x99/0x230 >>[ 202.192734] you didn't initialize this object before use? >>[ 202.198951] softirqs last enabled at (753948): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 >>[ 202.206882] turning off the locking correctness validator. >>[ 202.212236] softirqs last disabled at (753943): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 >>[ 202.220592] CPU: 2 PID: 1415 Comm: modprobe Tainted: G W 6.3.0-rc4+zeh-xe+ #909 >>[ 202.243002] Hardware name: Intel Corporation Raptor Lake Client Platform/RaptorLake-P LP5 RVP, BIOS RPLPFWI1.R00.3361.A14.2211151548 11/15/2022 >>[ 202.255737] Call Trace: >>[ 202.258179] <TASK> >>[ 202.260275] dump_stack_lvl+0x58/0xc0 >>[ 202.263922] register_lock_class+0x756/0x7d0 >>[ 202.268165] ? find_held_lock+0x2b/0x80 >>[ 202.271975] __lock_acquire+0x72/0x28b0 >>[ 202.275786] ? debug_object_free+0xb4/0x160 >>[ 202.279946] lock_acquire+0xd1/0x2d0 >>[ 202.283503] ? intel_dkl_phy_read+0x18/0x60 [xe] >>[ 202.288181] _raw_spin_lock+0x2a/0x40 >>[ 202.291825] ? intel_dkl_phy_read+0x18/0x60 [xe] >>[ 202.296475] intel_dkl_phy_read+0x18/0x60 [xe] >>[ 202.300949] icl_aux_power_well_enable+0x2bd/0x400 [xe] >>[ 202.306202] ? intel_display_power_grab_async_put_ref+0x75/0x120 [xe] >>[ 202.312649] intel_power_well_enable+0x1c/0x70 [xe] >>[ 202.317543] __intel_display_power_get_domain.part.0+0x4d/0x70 [xe] >>[ 202.323812] intel_display_power_get+0x43/0x70 [xe] >>[ 202.328708] intel_tc_port_init+0x199/0x2a0 [xe] >>[ 202.333363] intel_ddi_init+0x6ad/0xb00 [xe] >>[ 202.337678] intel_modeset_init_nogem+0x536/0x6d0 [xe] >>[ 202.342838] xe_display_init_noaccel+0x19/0x40 [xe] >>[ 202.347743] xe_device_probe+0x1f5/0x2a0 [xe] >>[ 202.352127] xe_pci_probe+0x28c/0x480 [xe] >>[ 202.356260] pci_device_probe+0x9d/0x150 >>[ 202.360164] really_probe+0x19a/0x400 >>[ 202.363809] ? __pfx___driver_attach+0x10/0x10 >>[ 202.368226] __driver_probe_device+0x73/0x170 >>[ 202.372558] driver_probe_device+0x1a/0x90 >>[ 202.376632] __driver_attach+0xcd/0x1c0 >>[ 202.380442] bus_for_each_dev+0x72/0xc0 >>[ 202.384253] bus_add_driver+0x110/0x210 >>[ 202.388063] driver_register+0x50/0x100 >>[ 202.391873] ? __pfx_init_module+0x10/0x10 [xe] >>[ 202.396431] do_one_initcall+0x55/0x260 >>[ 202.400245] ? rcu_is_watching+0xd/0x40 >>[ 202.404058] ? kmalloc_trace+0xa0/0xb0 >>[ 202.407786] do_init_module+0x45/0x1e0 >>[ 202.411512] __do_sys_finit_module+0xac/0x120 >>[ 202.415838] do_syscall_64+0x37/0x90 >>[ 202.419397] entry_SYSCALL_64_after_hwframe+0x72/0xdc >>[ 202.424409] RIP: 0033:0x7fd11291ea3d >>[ 202.427967] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 a3 0f 00 f7 d8 64 89 01 48 >>[ 202.446530] RSP: 002b:00007ffffde11368 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 >>[ 202.454031] RAX: ffffffffffffffda RBX: 00005616a617f210 RCX: 00007fd11291ea3d >>[ 202.461106] RDX: 0000000000000000 RSI: 00005616a617fe60 RDI: 000000000000000e >>[ 202.468182] RBP: 0000000000040000 R08: 0000000000000000 R09: 0000000000000002 >>[ 202.475250] R10: 000000000000000e R11: 0000000000000246 R12: 00005616a617fe60 >>[ 202.482319] R13: 00005616a617f340 R14: 0000000000000000 R15: 00005616a6180650 >>[ 202.489396] </TASK> >> >>v2: >>- added intel_display_locks_init() >> >>v3: >>- rebased >> >>v4: >>- drop intel_display_locks_init() > >why? ah... ok, now we have intel_display_driver_early_probe(). I thought you were dropping the call from i915_driver.c Remaining question for display: do we want to spread the lock initialization to each compilation unit? Or should we just keep a static locks_init() { <all the locks here> } the lock init seems a very cheap init that maybe doesn't deserve to be spread. Then this patch could just move all the mutexes initialization that were left behind. > >Lucas De Marchi ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-xe] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path 2023-04-19 6:41 ` Lucas De Marchi @ 2023-04-19 7:16 ` Jani Nikula 2023-04-19 7:29 ` [Intel-xe] [Intel-gfx] " Lucas De Marchi 0 siblings, 1 reply; 11+ messages in thread From: Jani Nikula @ 2023-04-19 7:16 UTC (permalink / raw) To: Lucas De Marchi, José Roberto de Souza Cc: intel-gfx, intel-xe, Ville Syrjälä, Rodrigo Vivi On Tue, 18 Apr 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote: > On Tue, Apr 18, 2023 at 11:30:18PM -0700, Lucas De Marchi wrote: >>On Tue, Apr 18, 2023 at 09:43:37AM -0700, Jose Souza wrote: >>>Start to move the initialization of display locks from >>>i915_driver_early_probe(), this way it is also executed when running >>>Xe kmd. >>> >>>This will fix a warning in Xe kmd: >>> >>>[ 201.894839] xe 0000:00:02.0: [drm] [ENCODER:235:DDI A/PHY A] failed to retrieve link info, disabling eDP >>>[ 202.136336] xe 0000:00:02.0: [drm] *ERROR* Failed to write source OUI >>>[ 202.175346] INFO: trying to register non-static key. >>>[ 202.175347] irq event stamp: 754060 >>>[ 202.175359] hardirqs last enabled at (754059): [<ffffffff8122cf79>] tick_nohz_idle_enter+0x59/0x80 >>>[ 202.180294] The code is fine but needs lockdep annotation, or maybe >>>[ 202.183774] hardirqs last disabled at (754060): [<ffffffff811a5539>] do_idle+0x99/0x230 >>>[ 202.192734] you didn't initialize this object before use? >>>[ 202.198951] softirqs last enabled at (753948): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 >>>[ 202.206882] turning off the locking correctness validator. >>>[ 202.212236] softirqs last disabled at (753943): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 >>>[ 202.220592] CPU: 2 PID: 1415 Comm: modprobe Tainted: G W 6.3.0-rc4+zeh-xe+ #909 >>>[ 202.243002] Hardware name: Intel Corporation Raptor Lake Client Platform/RaptorLake-P LP5 RVP, BIOS RPLPFWI1.R00.3361.A14.2211151548 11/15/2022 >>>[ 202.255737] Call Trace: >>>[ 202.258179] <TASK> >>>[ 202.260275] dump_stack_lvl+0x58/0xc0 >>>[ 202.263922] register_lock_class+0x756/0x7d0 >>>[ 202.268165] ? find_held_lock+0x2b/0x80 >>>[ 202.271975] __lock_acquire+0x72/0x28b0 >>>[ 202.275786] ? debug_object_free+0xb4/0x160 >>>[ 202.279946] lock_acquire+0xd1/0x2d0 >>>[ 202.283503] ? intel_dkl_phy_read+0x18/0x60 [xe] >>>[ 202.288181] _raw_spin_lock+0x2a/0x40 >>>[ 202.291825] ? intel_dkl_phy_read+0x18/0x60 [xe] >>>[ 202.296475] intel_dkl_phy_read+0x18/0x60 [xe] >>>[ 202.300949] icl_aux_power_well_enable+0x2bd/0x400 [xe] >>>[ 202.306202] ? intel_display_power_grab_async_put_ref+0x75/0x120 [xe] >>>[ 202.312649] intel_power_well_enable+0x1c/0x70 [xe] >>>[ 202.317543] __intel_display_power_get_domain.part.0+0x4d/0x70 [xe] >>>[ 202.323812] intel_display_power_get+0x43/0x70 [xe] >>>[ 202.328708] intel_tc_port_init+0x199/0x2a0 [xe] >>>[ 202.333363] intel_ddi_init+0x6ad/0xb00 [xe] >>>[ 202.337678] intel_modeset_init_nogem+0x536/0x6d0 [xe] >>>[ 202.342838] xe_display_init_noaccel+0x19/0x40 [xe] >>>[ 202.347743] xe_device_probe+0x1f5/0x2a0 [xe] >>>[ 202.352127] xe_pci_probe+0x28c/0x480 [xe] >>>[ 202.356260] pci_device_probe+0x9d/0x150 >>>[ 202.360164] really_probe+0x19a/0x400 >>>[ 202.363809] ? __pfx___driver_attach+0x10/0x10 >>>[ 202.368226] __driver_probe_device+0x73/0x170 >>>[ 202.372558] driver_probe_device+0x1a/0x90 >>>[ 202.376632] __driver_attach+0xcd/0x1c0 >>>[ 202.380442] bus_for_each_dev+0x72/0xc0 >>>[ 202.384253] bus_add_driver+0x110/0x210 >>>[ 202.388063] driver_register+0x50/0x100 >>>[ 202.391873] ? __pfx_init_module+0x10/0x10 [xe] >>>[ 202.396431] do_one_initcall+0x55/0x260 >>>[ 202.400245] ? rcu_is_watching+0xd/0x40 >>>[ 202.404058] ? kmalloc_trace+0xa0/0xb0 >>>[ 202.407786] do_init_module+0x45/0x1e0 >>>[ 202.411512] __do_sys_finit_module+0xac/0x120 >>>[ 202.415838] do_syscall_64+0x37/0x90 >>>[ 202.419397] entry_SYSCALL_64_after_hwframe+0x72/0xdc >>>[ 202.424409] RIP: 0033:0x7fd11291ea3d >>>[ 202.427967] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 a3 0f 00 f7 d8 64 89 01 48 >>>[ 202.446530] RSP: 002b:00007ffffde11368 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 >>>[ 202.454031] RAX: ffffffffffffffda RBX: 00005616a617f210 RCX: 00007fd11291ea3d >>>[ 202.461106] RDX: 0000000000000000 RSI: 00005616a617fe60 RDI: 000000000000000e >>>[ 202.468182] RBP: 0000000000040000 R08: 0000000000000000 R09: 0000000000000002 >>>[ 202.475250] R10: 000000000000000e R11: 0000000000000246 R12: 00005616a617fe60 >>>[ 202.482319] R13: 00005616a617f340 R14: 0000000000000000 R15: 00005616a6180650 >>>[ 202.489396] </TASK> >>> >>>v2: >>>- added intel_display_locks_init() >>> >>>v3: >>>- rebased >>> >>>v4: >>>- drop intel_display_locks_init() >> >>why? > > ah... ok, now we have intel_display_driver_early_probe(). I thought you > were dropping the call from i915_driver.c > > Remaining question for display: do we want to spread the lock > initialization to each compilation unit? Or should we just keep a > > static locks_init() { <all the locks here> } > > the lock init seems a very cheap init that maybe doesn't deserve to be > spread. Then this patch could just move all the mutexes initialization > that were left behind. I still think if only one file uses something, then that file should include the init for it too, and nobody else should touch it. Including locks. Ideally, they would be stowed away in allocated opaque structs that can't even be accessed (or initialized) by anyone else. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-xe] [Intel-gfx] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path 2023-04-19 7:16 ` Jani Nikula @ 2023-04-19 7:29 ` Lucas De Marchi 2023-04-20 13:19 ` Souza, Jose 0 siblings, 1 reply; 11+ messages in thread From: Lucas De Marchi @ 2023-04-19 7:29 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx, intel-xe, Rodrigo Vivi On Wed, Apr 19, 2023 at 10:16:22AM +0300, Jani Nikula wrote: >On Tue, 18 Apr 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote: >> On Tue, Apr 18, 2023 at 11:30:18PM -0700, Lucas De Marchi wrote: >>>On Tue, Apr 18, 2023 at 09:43:37AM -0700, Jose Souza wrote: >>>>Start to move the initialization of display locks from >>>>i915_driver_early_probe(), this way it is also executed when running >>>>Xe kmd. >>>> >>>>This will fix a warning in Xe kmd: >>>> >>>>[ 201.894839] xe 0000:00:02.0: [drm] [ENCODER:235:DDI A/PHY A] failed to retrieve link info, disabling eDP >>>>[ 202.136336] xe 0000:00:02.0: [drm] *ERROR* Failed to write source OUI >>>>[ 202.175346] INFO: trying to register non-static key. >>>>[ 202.175347] irq event stamp: 754060 >>>>[ 202.175359] hardirqs last enabled at (754059): [<ffffffff8122cf79>] tick_nohz_idle_enter+0x59/0x80 >>>>[ 202.180294] The code is fine but needs lockdep annotation, or maybe >>>>[ 202.183774] hardirqs last disabled at (754060): [<ffffffff811a5539>] do_idle+0x99/0x230 >>>>[ 202.192734] you didn't initialize this object before use? >>>>[ 202.198951] softirqs last enabled at (753948): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 >>>>[ 202.206882] turning off the locking correctness validator. >>>>[ 202.212236] softirqs last disabled at (753943): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 >>>>[ 202.220592] CPU: 2 PID: 1415 Comm: modprobe Tainted: G W 6.3.0-rc4+zeh-xe+ #909 >>>>[ 202.243002] Hardware name: Intel Corporation Raptor Lake Client Platform/RaptorLake-P LP5 RVP, BIOS RPLPFWI1.R00.3361.A14.2211151548 11/15/2022 >>>>[ 202.255737] Call Trace: >>>>[ 202.258179] <TASK> >>>>[ 202.260275] dump_stack_lvl+0x58/0xc0 >>>>[ 202.263922] register_lock_class+0x756/0x7d0 >>>>[ 202.268165] ? find_held_lock+0x2b/0x80 >>>>[ 202.271975] __lock_acquire+0x72/0x28b0 >>>>[ 202.275786] ? debug_object_free+0xb4/0x160 >>>>[ 202.279946] lock_acquire+0xd1/0x2d0 >>>>[ 202.283503] ? intel_dkl_phy_read+0x18/0x60 [xe] >>>>[ 202.288181] _raw_spin_lock+0x2a/0x40 >>>>[ 202.291825] ? intel_dkl_phy_read+0x18/0x60 [xe] >>>>[ 202.296475] intel_dkl_phy_read+0x18/0x60 [xe] >>>>[ 202.300949] icl_aux_power_well_enable+0x2bd/0x400 [xe] >>>>[ 202.306202] ? intel_display_power_grab_async_put_ref+0x75/0x120 [xe] >>>>[ 202.312649] intel_power_well_enable+0x1c/0x70 [xe] >>>>[ 202.317543] __intel_display_power_get_domain.part.0+0x4d/0x70 [xe] >>>>[ 202.323812] intel_display_power_get+0x43/0x70 [xe] >>>>[ 202.328708] intel_tc_port_init+0x199/0x2a0 [xe] >>>>[ 202.333363] intel_ddi_init+0x6ad/0xb00 [xe] >>>>[ 202.337678] intel_modeset_init_nogem+0x536/0x6d0 [xe] >>>>[ 202.342838] xe_display_init_noaccel+0x19/0x40 [xe] >>>>[ 202.347743] xe_device_probe+0x1f5/0x2a0 [xe] >>>>[ 202.352127] xe_pci_probe+0x28c/0x480 [xe] >>>>[ 202.356260] pci_device_probe+0x9d/0x150 >>>>[ 202.360164] really_probe+0x19a/0x400 >>>>[ 202.363809] ? __pfx___driver_attach+0x10/0x10 >>>>[ 202.368226] __driver_probe_device+0x73/0x170 >>>>[ 202.372558] driver_probe_device+0x1a/0x90 >>>>[ 202.376632] __driver_attach+0xcd/0x1c0 >>>>[ 202.380442] bus_for_each_dev+0x72/0xc0 >>>>[ 202.384253] bus_add_driver+0x110/0x210 >>>>[ 202.388063] driver_register+0x50/0x100 >>>>[ 202.391873] ? __pfx_init_module+0x10/0x10 [xe] >>>>[ 202.396431] do_one_initcall+0x55/0x260 >>>>[ 202.400245] ? rcu_is_watching+0xd/0x40 >>>>[ 202.404058] ? kmalloc_trace+0xa0/0xb0 >>>>[ 202.407786] do_init_module+0x45/0x1e0 >>>>[ 202.411512] __do_sys_finit_module+0xac/0x120 >>>>[ 202.415838] do_syscall_64+0x37/0x90 >>>>[ 202.419397] entry_SYSCALL_64_after_hwframe+0x72/0xdc >>>>[ 202.424409] RIP: 0033:0x7fd11291ea3d >>>>[ 202.427967] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 a3 0f 00 f7 d8 64 89 01 48 >>>>[ 202.446530] RSP: 002b:00007ffffde11368 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 >>>>[ 202.454031] RAX: ffffffffffffffda RBX: 00005616a617f210 RCX: 00007fd11291ea3d >>>>[ 202.461106] RDX: 0000000000000000 RSI: 00005616a617fe60 RDI: 000000000000000e >>>>[ 202.468182] RBP: 0000000000040000 R08: 0000000000000000 R09: 0000000000000002 >>>>[ 202.475250] R10: 000000000000000e R11: 0000000000000246 R12: 00005616a617fe60 >>>>[ 202.482319] R13: 00005616a617f340 R14: 0000000000000000 R15: 00005616a6180650 >>>>[ 202.489396] </TASK> >>>> >>>>v2: >>>>- added intel_display_locks_init() >>>> >>>>v3: >>>>- rebased >>>> >>>>v4: >>>>- drop intel_display_locks_init() >>> >>>why? >> >> ah... ok, now we have intel_display_driver_early_probe(). I thought you >> were dropping the call from i915_driver.c >> >> Remaining question for display: do we want to spread the lock >> initialization to each compilation unit? Or should we just keep a >> >> static locks_init() { <all the locks here> } >> >> the lock init seems a very cheap init that maybe doesn't deserve to be >> spread. Then this patch could just move all the mutexes initialization >> that were left behind. > >I still think if only one file uses something, then that file should >include the init for it too, and nobody else should touch it. Including >locks. Ideally, they would be stowed away in allocated opaque structs >that can't even be accessed (or initialized) by anyone else. so... this version of the patch? We will need to spread the mutexes around then. Lucas De Marchi > >BR, >Jani. > > >-- >Jani Nikula, Intel Open Source Graphics Center ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-xe] [Intel-gfx] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path 2023-04-19 7:29 ` [Intel-xe] [Intel-gfx] " Lucas De Marchi @ 2023-04-20 13:19 ` Souza, Jose 2023-04-20 15:27 ` Rodrigo Vivi 0 siblings, 1 reply; 11+ messages in thread From: Souza, Jose @ 2023-04-20 13:19 UTC (permalink / raw) To: Nikula, Jani, De Marchi, Lucas Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org, Vivi, Rodrigo On Wed, 2023-04-19 at 00:29 -0700, Lucas De Marchi wrote: > On Wed, Apr 19, 2023 at 10:16:22AM +0300, Jani Nikula wrote: > > On Tue, 18 Apr 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote: > > > On Tue, Apr 18, 2023 at 11:30:18PM -0700, Lucas De Marchi wrote: > > > > On Tue, Apr 18, 2023 at 09:43:37AM -0700, Jose Souza wrote: > > > > > Start to move the initialization of display locks from > > > > > i915_driver_early_probe(), this way it is also executed when running > > > > > Xe kmd. > > > > > > > > > > This will fix a warning in Xe kmd: > > > > > > > > > > [ 201.894839] xe 0000:00:02.0: [drm] [ENCODER:235:DDI A/PHY A] failed to retrieve link info, disabling eDP > > > > > [ 202.136336] xe 0000:00:02.0: [drm] *ERROR* Failed to write source OUI > > > > > [ 202.175346] INFO: trying to register non-static key. > > > > > [ 202.175347] irq event stamp: 754060 > > > > > [ 202.175359] hardirqs last enabled at (754059): [<ffffffff8122cf79>] tick_nohz_idle_enter+0x59/0x80 > > > > > [ 202.180294] The code is fine but needs lockdep annotation, or maybe > > > > > [ 202.183774] hardirqs last disabled at (754060): [<ffffffff811a5539>] do_idle+0x99/0x230 > > > > > [ 202.192734] you didn't initialize this object before use? > > > > > [ 202.198951] softirqs last enabled at (753948): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 > > > > > [ 202.206882] turning off the locking correctness validator. > > > > > [ 202.212236] softirqs last disabled at (753943): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 > > > > > [ 202.220592] CPU: 2 PID: 1415 Comm: modprobe Tainted: G W 6.3.0-rc4+zeh-xe+ #909 > > > > > [ 202.243002] Hardware name: Intel Corporation Raptor Lake Client Platform/RaptorLake-P LP5 RVP, BIOS RPLPFWI1.R00.3361.A14.2211151548 11/15/2022 > > > > > [ 202.255737] Call Trace: > > > > > [ 202.258179] <TASK> > > > > > [ 202.260275] dump_stack_lvl+0x58/0xc0 > > > > > [ 202.263922] register_lock_class+0x756/0x7d0 > > > > > [ 202.268165] ? find_held_lock+0x2b/0x80 > > > > > [ 202.271975] __lock_acquire+0x72/0x28b0 > > > > > [ 202.275786] ? debug_object_free+0xb4/0x160 > > > > > [ 202.279946] lock_acquire+0xd1/0x2d0 > > > > > [ 202.283503] ? intel_dkl_phy_read+0x18/0x60 [xe] > > > > > [ 202.288181] _raw_spin_lock+0x2a/0x40 > > > > > [ 202.291825] ? intel_dkl_phy_read+0x18/0x60 [xe] > > > > > [ 202.296475] intel_dkl_phy_read+0x18/0x60 [xe] > > > > > [ 202.300949] icl_aux_power_well_enable+0x2bd/0x400 [xe] > > > > > [ 202.306202] ? intel_display_power_grab_async_put_ref+0x75/0x120 [xe] > > > > > [ 202.312649] intel_power_well_enable+0x1c/0x70 [xe] > > > > > [ 202.317543] __intel_display_power_get_domain.part.0+0x4d/0x70 [xe] > > > > > [ 202.323812] intel_display_power_get+0x43/0x70 [xe] > > > > > [ 202.328708] intel_tc_port_init+0x199/0x2a0 [xe] > > > > > [ 202.333363] intel_ddi_init+0x6ad/0xb00 [xe] > > > > > [ 202.337678] intel_modeset_init_nogem+0x536/0x6d0 [xe] > > > > > [ 202.342838] xe_display_init_noaccel+0x19/0x40 [xe] > > > > > [ 202.347743] xe_device_probe+0x1f5/0x2a0 [xe] > > > > > [ 202.352127] xe_pci_probe+0x28c/0x480 [xe] > > > > > [ 202.356260] pci_device_probe+0x9d/0x150 > > > > > [ 202.360164] really_probe+0x19a/0x400 > > > > > [ 202.363809] ? __pfx___driver_attach+0x10/0x10 > > > > > [ 202.368226] __driver_probe_device+0x73/0x170 > > > > > [ 202.372558] driver_probe_device+0x1a/0x90 > > > > > [ 202.376632] __driver_attach+0xcd/0x1c0 > > > > > [ 202.380442] bus_for_each_dev+0x72/0xc0 > > > > > [ 202.384253] bus_add_driver+0x110/0x210 > > > > > [ 202.388063] driver_register+0x50/0x100 > > > > > [ 202.391873] ? __pfx_init_module+0x10/0x10 [xe] > > > > > [ 202.396431] do_one_initcall+0x55/0x260 > > > > > [ 202.400245] ? rcu_is_watching+0xd/0x40 > > > > > [ 202.404058] ? kmalloc_trace+0xa0/0xb0 > > > > > [ 202.407786] do_init_module+0x45/0x1e0 > > > > > [ 202.411512] __do_sys_finit_module+0xac/0x120 > > > > > [ 202.415838] do_syscall_64+0x37/0x90 > > > > > [ 202.419397] entry_SYSCALL_64_after_hwframe+0x72/0xdc > > > > > [ 202.424409] RIP: 0033:0x7fd11291ea3d > > > > > [ 202.427967] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 a3 0f 00 f7 d8 64 89 01 48 > > > > > [ 202.446530] RSP: 002b:00007ffffde11368 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 > > > > > [ 202.454031] RAX: ffffffffffffffda RBX: 00005616a617f210 RCX: 00007fd11291ea3d > > > > > [ 202.461106] RDX: 0000000000000000 RSI: 00005616a617fe60 RDI: 000000000000000e > > > > > [ 202.468182] RBP: 0000000000040000 R08: 0000000000000000 R09: 0000000000000002 > > > > > [ 202.475250] R10: 000000000000000e R11: 0000000000000246 R12: 00005616a617fe60 > > > > > [ 202.482319] R13: 00005616a617f340 R14: 0000000000000000 R15: 00005616a6180650 > > > > > [ 202.489396] </TASK> > > > > > > > > > > v2: > > > > > - added intel_display_locks_init() > > > > > > > > > > v3: > > > > > - rebased > > > > > > > > > > v4: > > > > > - drop intel_display_locks_init() > > > > > > > > why? > > > > > > ah... ok, now we have intel_display_driver_early_probe(). I thought you > > > were dropping the call from i915_driver.c > > > > > > Remaining question for display: do we want to spread the lock > > > initialization to each compilation unit? Or should we just keep a > > > > > > static locks_init() { <all the locks here> } > > > > > > the lock init seems a very cheap init that maybe doesn't deserve to be > > > spread. Then this patch could just move all the mutexes initialization > > > that were left behind. > > > > I still think if only one file uses something, then that file should > > include the init for it too, and nobody else should touch it. Including > > locks. Ideally, they would be stowed away in allocated opaque structs > > that can't even be accessed (or initialized) by anyone else. > > so... this version of the patch? We will need to spread the mutexes > around then. Do we have an agreement here? I'm also in favor of init all variables in the only file that touches it. The other mutexes can be moved gradually. > > Lucas De Marchi > > > > > BR, > > Jani. > > > > > > -- > > Jani Nikula, Intel Open Source Graphics Center ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-xe] [Intel-gfx] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path 2023-04-20 13:19 ` Souza, Jose @ 2023-04-20 15:27 ` Rodrigo Vivi 2023-04-20 15:36 ` Souza, Jose 0 siblings, 1 reply; 11+ messages in thread From: Rodrigo Vivi @ 2023-04-20 15:27 UTC (permalink / raw) To: Souza, Jose Cc: Nikula, Jani, intel-gfx@lists.freedesktop.org, De Marchi, Lucas, intel-xe@lists.freedesktop.org On Thu, Apr 20, 2023 at 09:19:09AM -0400, Souza, Jose wrote: > On Wed, 2023-04-19 at 00:29 -0700, Lucas De Marchi wrote: > > On Wed, Apr 19, 2023 at 10:16:22AM +0300, Jani Nikula wrote: > > > On Tue, 18 Apr 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote: > > > > On Tue, Apr 18, 2023 at 11:30:18PM -0700, Lucas De Marchi wrote: > > > > > On Tue, Apr 18, 2023 at 09:43:37AM -0700, Jose Souza wrote: > > > > > > Start to move the initialization of display locks from > > > > > > i915_driver_early_probe(), this way it is also executed when running > > > > > > Xe kmd. > > > > > > > > > > > > This will fix a warning in Xe kmd: > > > > > > > > > > > > [ 201.894839] xe 0000:00:02.0: [drm] [ENCODER:235:DDI A/PHY A] failed to retrieve link info, disabling eDP > > > > > > [ 202.136336] xe 0000:00:02.0: [drm] *ERROR* Failed to write source OUI > > > > > > [ 202.175346] INFO: trying to register non-static key. > > > > > > [ 202.175347] irq event stamp: 754060 > > > > > > [ 202.175359] hardirqs last enabled at (754059): [<ffffffff8122cf79>] tick_nohz_idle_enter+0x59/0x80 > > > > > > [ 202.180294] The code is fine but needs lockdep annotation, or maybe > > > > > > [ 202.183774] hardirqs last disabled at (754060): [<ffffffff811a5539>] do_idle+0x99/0x230 > > > > > > [ 202.192734] you didn't initialize this object before use? > > > > > > [ 202.198951] softirqs last enabled at (753948): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 > > > > > > [ 202.206882] turning off the locking correctness validator. > > > > > > [ 202.212236] softirqs last disabled at (753943): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 > > > > > > [ 202.220592] CPU: 2 PID: 1415 Comm: modprobe Tainted: G W 6.3.0-rc4+zeh-xe+ #909 > > > > > > [ 202.243002] Hardware name: Intel Corporation Raptor Lake Client Platform/RaptorLake-P LP5 RVP, BIOS RPLPFWI1.R00.3361.A14.2211151548 11/15/2022 > > > > > > [ 202.255737] Call Trace: > > > > > > [ 202.258179] <TASK> > > > > > > [ 202.260275] dump_stack_lvl+0x58/0xc0 > > > > > > [ 202.263922] register_lock_class+0x756/0x7d0 > > > > > > [ 202.268165] ? find_held_lock+0x2b/0x80 > > > > > > [ 202.271975] __lock_acquire+0x72/0x28b0 > > > > > > [ 202.275786] ? debug_object_free+0xb4/0x160 > > > > > > [ 202.279946] lock_acquire+0xd1/0x2d0 > > > > > > [ 202.283503] ? intel_dkl_phy_read+0x18/0x60 [xe] > > > > > > [ 202.288181] _raw_spin_lock+0x2a/0x40 > > > > > > [ 202.291825] ? intel_dkl_phy_read+0x18/0x60 [xe] > > > > > > [ 202.296475] intel_dkl_phy_read+0x18/0x60 [xe] > > > > > > [ 202.300949] icl_aux_power_well_enable+0x2bd/0x400 [xe] > > > > > > [ 202.306202] ? intel_display_power_grab_async_put_ref+0x75/0x120 [xe] > > > > > > [ 202.312649] intel_power_well_enable+0x1c/0x70 [xe] > > > > > > [ 202.317543] __intel_display_power_get_domain.part.0+0x4d/0x70 [xe] > > > > > > [ 202.323812] intel_display_power_get+0x43/0x70 [xe] > > > > > > [ 202.328708] intel_tc_port_init+0x199/0x2a0 [xe] > > > > > > [ 202.333363] intel_ddi_init+0x6ad/0xb00 [xe] > > > > > > [ 202.337678] intel_modeset_init_nogem+0x536/0x6d0 [xe] > > > > > > [ 202.342838] xe_display_init_noaccel+0x19/0x40 [xe] > > > > > > [ 202.347743] xe_device_probe+0x1f5/0x2a0 [xe] > > > > > > [ 202.352127] xe_pci_probe+0x28c/0x480 [xe] > > > > > > [ 202.356260] pci_device_probe+0x9d/0x150 > > > > > > [ 202.360164] really_probe+0x19a/0x400 > > > > > > [ 202.363809] ? __pfx___driver_attach+0x10/0x10 > > > > > > [ 202.368226] __driver_probe_device+0x73/0x170 > > > > > > [ 202.372558] driver_probe_device+0x1a/0x90 > > > > > > [ 202.376632] __driver_attach+0xcd/0x1c0 > > > > > > [ 202.380442] bus_for_each_dev+0x72/0xc0 > > > > > > [ 202.384253] bus_add_driver+0x110/0x210 > > > > > > [ 202.388063] driver_register+0x50/0x100 > > > > > > [ 202.391873] ? __pfx_init_module+0x10/0x10 [xe] > > > > > > [ 202.396431] do_one_initcall+0x55/0x260 > > > > > > [ 202.400245] ? rcu_is_watching+0xd/0x40 > > > > > > [ 202.404058] ? kmalloc_trace+0xa0/0xb0 > > > > > > [ 202.407786] do_init_module+0x45/0x1e0 > > > > > > [ 202.411512] __do_sys_finit_module+0xac/0x120 > > > > > > [ 202.415838] do_syscall_64+0x37/0x90 > > > > > > [ 202.419397] entry_SYSCALL_64_after_hwframe+0x72/0xdc > > > > > > [ 202.424409] RIP: 0033:0x7fd11291ea3d > > > > > > [ 202.427967] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 a3 0f 00 f7 d8 64 89 01 48 > > > > > > [ 202.446530] RSP: 002b:00007ffffde11368 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 > > > > > > [ 202.454031] RAX: ffffffffffffffda RBX: 00005616a617f210 RCX: 00007fd11291ea3d > > > > > > [ 202.461106] RDX: 0000000000000000 RSI: 00005616a617fe60 RDI: 000000000000000e > > > > > > [ 202.468182] RBP: 0000000000040000 R08: 0000000000000000 R09: 0000000000000002 > > > > > > [ 202.475250] R10: 000000000000000e R11: 0000000000000246 R12: 00005616a617fe60 > > > > > > [ 202.482319] R13: 00005616a617f340 R14: 0000000000000000 R15: 00005616a6180650 > > > > > > [ 202.489396] </TASK> > > > > > > > > > > > > v2: > > > > > > - added intel_display_locks_init() > > > > > > > > > > > > v3: > > > > > > - rebased > > > > > > > > > > > > v4: > > > > > > - drop intel_display_locks_init() > > > > > > > > > > why? > > > > > > > > ah... ok, now we have intel_display_driver_early_probe(). I thought you > > > > were dropping the call from i915_driver.c > > > > > > > > Remaining question for display: do we want to spread the lock > > > > initialization to each compilation unit? Or should we just keep a > > > > > > > > static locks_init() { <all the locks here> } > > > > > > > > the lock init seems a very cheap init that maybe doesn't deserve to be > > > > spread. Then this patch could just move all the mutexes initialization > > > > that were left behind. > > > > > > I still think if only one file uses something, then that file should > > > include the init for it too, and nobody else should touch it. Including > > > locks. Ideally, they would be stowed away in allocated opaque structs > > > that can't even be accessed (or initialized) by anyone else. > > > > so... this version of the patch? We will need to spread the mutexes > > around then. > > Do we have an agreement here? > I'm also in favor of init all variables in the only file that touches it. > > The other mutexes can be moved gradually. I think we all agree here. Also I believe it can start with this and later do the rest of the clean up. Probably change the commit message to remove the Xe, which is still out of the tree? > > > > > Lucas De Marchi > > > > > > > > BR, > > > Jani. > > > > > > > > > -- > > > Jani Nikula, Intel Open Source Graphics Center > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-xe] [Intel-gfx] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path 2023-04-20 15:27 ` Rodrigo Vivi @ 2023-04-20 15:36 ` Souza, Jose 2023-04-20 16:35 ` Lucas De Marchi 0 siblings, 1 reply; 11+ messages in thread From: Souza, Jose @ 2023-04-20 15:36 UTC (permalink / raw) To: Vivi, Rodrigo Cc: Nikula, Jani, intel-gfx@lists.freedesktop.org, De Marchi, Lucas, intel-xe@lists.freedesktop.org On Thu, 2023-04-20 at 11:27 -0400, Rodrigo Vivi wrote: > On Thu, Apr 20, 2023 at 09:19:09AM -0400, Souza, Jose wrote: > > On Wed, 2023-04-19 at 00:29 -0700, Lucas De Marchi wrote: > > > On Wed, Apr 19, 2023 at 10:16:22AM +0300, Jani Nikula wrote: > > > > On Tue, 18 Apr 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote: > > > > > On Tue, Apr 18, 2023 at 11:30:18PM -0700, Lucas De Marchi wrote: > > > > > > On Tue, Apr 18, 2023 at 09:43:37AM -0700, Jose Souza wrote: > > > > > > > Start to move the initialization of display locks from > > > > > > > i915_driver_early_probe(), this way it is also executed when running > > > > > > > Xe kmd. > > > > > > > > > > > > > > This will fix a warning in Xe kmd: > > > > > > > > > > > > > > [ 201.894839] xe 0000:00:02.0: [drm] [ENCODER:235:DDI A/PHY A] failed to retrieve link info, disabling eDP > > > > > > > [ 202.136336] xe 0000:00:02.0: [drm] *ERROR* Failed to write source OUI > > > > > > > [ 202.175346] INFO: trying to register non-static key. > > > > > > > [ 202.175347] irq event stamp: 754060 > > > > > > > [ 202.175359] hardirqs last enabled at (754059): [<ffffffff8122cf79>] tick_nohz_idle_enter+0x59/0x80 > > > > > > > [ 202.180294] The code is fine but needs lockdep annotation, or maybe > > > > > > > [ 202.183774] hardirqs last disabled at (754060): [<ffffffff811a5539>] do_idle+0x99/0x230 > > > > > > > [ 202.192734] you didn't initialize this object before use? > > > > > > > [ 202.198951] softirqs last enabled at (753948): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 > > > > > > > [ 202.206882] turning off the locking correctness validator. > > > > > > > [ 202.212236] softirqs last disabled at (753943): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 > > > > > > > [ 202.220592] CPU: 2 PID: 1415 Comm: modprobe Tainted: G W 6.3.0-rc4+zeh-xe+ #909 > > > > > > > [ 202.243002] Hardware name: Intel Corporation Raptor Lake Client Platform/RaptorLake-P LP5 RVP, BIOS RPLPFWI1.R00.3361.A14.2211151548 11/15/2022 > > > > > > > [ 202.255737] Call Trace: > > > > > > > [ 202.258179] <TASK> > > > > > > > [ 202.260275] dump_stack_lvl+0x58/0xc0 > > > > > > > [ 202.263922] register_lock_class+0x756/0x7d0 > > > > > > > [ 202.268165] ? find_held_lock+0x2b/0x80 > > > > > > > [ 202.271975] __lock_acquire+0x72/0x28b0 > > > > > > > [ 202.275786] ? debug_object_free+0xb4/0x160 > > > > > > > [ 202.279946] lock_acquire+0xd1/0x2d0 > > > > > > > [ 202.283503] ? intel_dkl_phy_read+0x18/0x60 [xe] > > > > > > > [ 202.288181] _raw_spin_lock+0x2a/0x40 > > > > > > > [ 202.291825] ? intel_dkl_phy_read+0x18/0x60 [xe] > > > > > > > [ 202.296475] intel_dkl_phy_read+0x18/0x60 [xe] > > > > > > > [ 202.300949] icl_aux_power_well_enable+0x2bd/0x400 [xe] > > > > > > > [ 202.306202] ? intel_display_power_grab_async_put_ref+0x75/0x120 [xe] > > > > > > > [ 202.312649] intel_power_well_enable+0x1c/0x70 [xe] > > > > > > > [ 202.317543] __intel_display_power_get_domain.part.0+0x4d/0x70 [xe] > > > > > > > [ 202.323812] intel_display_power_get+0x43/0x70 [xe] > > > > > > > [ 202.328708] intel_tc_port_init+0x199/0x2a0 [xe] > > > > > > > [ 202.333363] intel_ddi_init+0x6ad/0xb00 [xe] > > > > > > > [ 202.337678] intel_modeset_init_nogem+0x536/0x6d0 [xe] > > > > > > > [ 202.342838] xe_display_init_noaccel+0x19/0x40 [xe] > > > > > > > [ 202.347743] xe_device_probe+0x1f5/0x2a0 [xe] > > > > > > > [ 202.352127] xe_pci_probe+0x28c/0x480 [xe] > > > > > > > [ 202.356260] pci_device_probe+0x9d/0x150 > > > > > > > [ 202.360164] really_probe+0x19a/0x400 > > > > > > > [ 202.363809] ? __pfx___driver_attach+0x10/0x10 > > > > > > > [ 202.368226] __driver_probe_device+0x73/0x170 > > > > > > > [ 202.372558] driver_probe_device+0x1a/0x90 > > > > > > > [ 202.376632] __driver_attach+0xcd/0x1c0 > > > > > > > [ 202.380442] bus_for_each_dev+0x72/0xc0 > > > > > > > [ 202.384253] bus_add_driver+0x110/0x210 > > > > > > > [ 202.388063] driver_register+0x50/0x100 > > > > > > > [ 202.391873] ? __pfx_init_module+0x10/0x10 [xe] > > > > > > > [ 202.396431] do_one_initcall+0x55/0x260 > > > > > > > [ 202.400245] ? rcu_is_watching+0xd/0x40 > > > > > > > [ 202.404058] ? kmalloc_trace+0xa0/0xb0 > > > > > > > [ 202.407786] do_init_module+0x45/0x1e0 > > > > > > > [ 202.411512] __do_sys_finit_module+0xac/0x120 > > > > > > > [ 202.415838] do_syscall_64+0x37/0x90 > > > > > > > [ 202.419397] entry_SYSCALL_64_after_hwframe+0x72/0xdc > > > > > > > [ 202.424409] RIP: 0033:0x7fd11291ea3d > > > > > > > [ 202.427967] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 a3 0f 00 f7 d8 64 89 01 48 > > > > > > > [ 202.446530] RSP: 002b:00007ffffde11368 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 > > > > > > > [ 202.454031] RAX: ffffffffffffffda RBX: 00005616a617f210 RCX: 00007fd11291ea3d > > > > > > > [ 202.461106] RDX: 0000000000000000 RSI: 00005616a617fe60 RDI: 000000000000000e > > > > > > > [ 202.468182] RBP: 0000000000040000 R08: 0000000000000000 R09: 0000000000000002 > > > > > > > [ 202.475250] R10: 000000000000000e R11: 0000000000000246 R12: 00005616a617fe60 > > > > > > > [ 202.482319] R13: 00005616a617f340 R14: 0000000000000000 R15: 00005616a6180650 > > > > > > > [ 202.489396] </TASK> > > > > > > > > > > > > > > v2: > > > > > > > - added intel_display_locks_init() > > > > > > > > > > > > > > v3: > > > > > > > - rebased > > > > > > > > > > > > > > v4: > > > > > > > - drop intel_display_locks_init() > > > > > > > > > > > > why? > > > > > > > > > > ah... ok, now we have intel_display_driver_early_probe(). I thought you > > > > > were dropping the call from i915_driver.c > > > > > > > > > > Remaining question for display: do we want to spread the lock > > > > > initialization to each compilation unit? Or should we just keep a > > > > > > > > > > static locks_init() { <all the locks here> } > > > > > > > > > > the lock init seems a very cheap init that maybe doesn't deserve to be > > > > > spread. Then this patch could just move all the mutexes initialization > > > > > that were left behind. > > > > > > > > I still think if only one file uses something, then that file should > > > > include the init for it too, and nobody else should touch it. Including > > > > locks. Ideally, they would be stowed away in allocated opaque structs > > > > that can't even be accessed (or initialized) by anyone else. > > > > > > so... this version of the patch? We will need to spread the mutexes > > > around then. > > > > Do we have an agreement here? > > I'm also in favor of init all variables in the only file that touches it. > > > > The other mutexes can be moved gradually. > > I think we all agree here. Also I believe it can start with this and > later do the rest of the clean up. > > Probably change the commit message to remove the Xe, which is still out > of the tree? If there is no other changes, I can do that when applying. > > > > > > > > > Lucas De Marchi > > > > > > > > > > > BR, > > > > Jani. > > > > > > > > > > > > -- > > > > Jani Nikula, Intel Open Source Graphics Center > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-xe] [Intel-gfx] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path 2023-04-20 15:36 ` Souza, Jose @ 2023-04-20 16:35 ` Lucas De Marchi 2023-04-20 17:54 ` Souza, Jose 0 siblings, 1 reply; 11+ messages in thread From: Lucas De Marchi @ 2023-04-20 16:35 UTC (permalink / raw) To: Souza, Jose Cc: Nikula, Jani, intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org, Vivi, Rodrigo On Thu, Apr 20, 2023 at 08:36:37AM -0700, Jose Souza wrote: >On Thu, 2023-04-20 at 11:27 -0400, Rodrigo Vivi wrote: >> On Thu, Apr 20, 2023 at 09:19:09AM -0400, Souza, Jose wrote: >> > On Wed, 2023-04-19 at 00:29 -0700, Lucas De Marchi wrote: >> > > On Wed, Apr 19, 2023 at 10:16:22AM +0300, Jani Nikula wrote: >> > > > On Tue, 18 Apr 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote: >> > > > > On Tue, Apr 18, 2023 at 11:30:18PM -0700, Lucas De Marchi wrote: >> > > > > > On Tue, Apr 18, 2023 at 09:43:37AM -0700, Jose Souza wrote: >> > > > > > > Start to move the initialization of display locks from >> > > > > > > i915_driver_early_probe(), this way it is also executed when running >> > > > > > > Xe kmd. >> > > > > > > >> > > > > > > This will fix a warning in Xe kmd: >> > > > > > > >> > > > > > > [ 201.894839] xe 0000:00:02.0: [drm] [ENCODER:235:DDI A/PHY A] failed to retrieve link info, disabling eDP >> > > > > > > [ 202.136336] xe 0000:00:02.0: [drm] *ERROR* Failed to write source OUI >> > > > > > > [ 202.175346] INFO: trying to register non-static key. >> > > > > > > [ 202.175347] irq event stamp: 754060 >> > > > > > > [ 202.175359] hardirqs last enabled at (754059): [<ffffffff8122cf79>] tick_nohz_idle_enter+0x59/0x80 >> > > > > > > [ 202.180294] The code is fine but needs lockdep annotation, or maybe >> > > > > > > [ 202.183774] hardirqs last disabled at (754060): [<ffffffff811a5539>] do_idle+0x99/0x230 >> > > > > > > [ 202.192734] you didn't initialize this object before use? >> > > > > > > [ 202.198951] softirqs last enabled at (753948): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 >> > > > > > > [ 202.206882] turning off the locking correctness validator. >> > > > > > > [ 202.212236] softirqs last disabled at (753943): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 >> > > > > > > [ 202.220592] CPU: 2 PID: 1415 Comm: modprobe Tainted: G W 6.3.0-rc4+zeh-xe+ #909 >> > > > > > > [ 202.243002] Hardware name: Intel Corporation Raptor Lake Client Platform/RaptorLake-P LP5 RVP, BIOS RPLPFWI1.R00.3361.A14.2211151548 11/15/2022 >> > > > > > > [ 202.255737] Call Trace: >> > > > > > > [ 202.258179] <TASK> >> > > > > > > [ 202.260275] dump_stack_lvl+0x58/0xc0 >> > > > > > > [ 202.263922] register_lock_class+0x756/0x7d0 >> > > > > > > [ 202.268165] ? find_held_lock+0x2b/0x80 >> > > > > > > [ 202.271975] __lock_acquire+0x72/0x28b0 >> > > > > > > [ 202.275786] ? debug_object_free+0xb4/0x160 >> > > > > > > [ 202.279946] lock_acquire+0xd1/0x2d0 >> > > > > > > [ 202.283503] ? intel_dkl_phy_read+0x18/0x60 [xe] >> > > > > > > [ 202.288181] _raw_spin_lock+0x2a/0x40 >> > > > > > > [ 202.291825] ? intel_dkl_phy_read+0x18/0x60 [xe] >> > > > > > > [ 202.296475] intel_dkl_phy_read+0x18/0x60 [xe] >> > > > > > > [ 202.300949] icl_aux_power_well_enable+0x2bd/0x400 [xe] >> > > > > > > [ 202.306202] ? intel_display_power_grab_async_put_ref+0x75/0x120 [xe] >> > > > > > > [ 202.312649] intel_power_well_enable+0x1c/0x70 [xe] >> > > > > > > [ 202.317543] __intel_display_power_get_domain.part.0+0x4d/0x70 [xe] >> > > > > > > [ 202.323812] intel_display_power_get+0x43/0x70 [xe] >> > > > > > > [ 202.328708] intel_tc_port_init+0x199/0x2a0 [xe] >> > > > > > > [ 202.333363] intel_ddi_init+0x6ad/0xb00 [xe] >> > > > > > > [ 202.337678] intel_modeset_init_nogem+0x536/0x6d0 [xe] >> > > > > > > [ 202.342838] xe_display_init_noaccel+0x19/0x40 [xe] >> > > > > > > [ 202.347743] xe_device_probe+0x1f5/0x2a0 [xe] >> > > > > > > [ 202.352127] xe_pci_probe+0x28c/0x480 [xe] >> > > > > > > [ 202.356260] pci_device_probe+0x9d/0x150 >> > > > > > > [ 202.360164] really_probe+0x19a/0x400 >> > > > > > > [ 202.363809] ? __pfx___driver_attach+0x10/0x10 >> > > > > > > [ 202.368226] __driver_probe_device+0x73/0x170 >> > > > > > > [ 202.372558] driver_probe_device+0x1a/0x90 >> > > > > > > [ 202.376632] __driver_attach+0xcd/0x1c0 >> > > > > > > [ 202.380442] bus_for_each_dev+0x72/0xc0 >> > > > > > > [ 202.384253] bus_add_driver+0x110/0x210 >> > > > > > > [ 202.388063] driver_register+0x50/0x100 >> > > > > > > [ 202.391873] ? __pfx_init_module+0x10/0x10 [xe] >> > > > > > > [ 202.396431] do_one_initcall+0x55/0x260 >> > > > > > > [ 202.400245] ? rcu_is_watching+0xd/0x40 >> > > > > > > [ 202.404058] ? kmalloc_trace+0xa0/0xb0 >> > > > > > > [ 202.407786] do_init_module+0x45/0x1e0 >> > > > > > > [ 202.411512] __do_sys_finit_module+0xac/0x120 >> > > > > > > [ 202.415838] do_syscall_64+0x37/0x90 >> > > > > > > [ 202.419397] entry_SYSCALL_64_after_hwframe+0x72/0xdc >> > > > > > > [ 202.424409] RIP: 0033:0x7fd11291ea3d >> > > > > > > [ 202.427967] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 a3 0f 00 f7 d8 64 89 01 48 >> > > > > > > [ 202.446530] RSP: 002b:00007ffffde11368 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 >> > > > > > > [ 202.454031] RAX: ffffffffffffffda RBX: 00005616a617f210 RCX: 00007fd11291ea3d >> > > > > > > [ 202.461106] RDX: 0000000000000000 RSI: 00005616a617fe60 RDI: 000000000000000e >> > > > > > > [ 202.468182] RBP: 0000000000040000 R08: 0000000000000000 R09: 0000000000000002 >> > > > > > > [ 202.475250] R10: 000000000000000e R11: 0000000000000246 R12: 00005616a617fe60 >> > > > > > > [ 202.482319] R13: 00005616a617f340 R14: 0000000000000000 R15: 00005616a6180650 >> > > > > > > [ 202.489396] </TASK> >> > > > > > > >> > > > > > > v2: >> > > > > > > - added intel_display_locks_init() >> > > > > > > >> > > > > > > v3: >> > > > > > > - rebased >> > > > > > > >> > > > > > > v4: >> > > > > > > - drop intel_display_locks_init() >> > > > > > >> > > > > > why? >> > > > > >> > > > > ah... ok, now we have intel_display_driver_early_probe(). I thought you >> > > > > were dropping the call from i915_driver.c >> > > > > >> > > > > Remaining question for display: do we want to spread the lock >> > > > > initialization to each compilation unit? Or should we just keep a >> > > > > >> > > > > static locks_init() { <all the locks here> } >> > > > > >> > > > > the lock init seems a very cheap init that maybe doesn't deserve to be >> > > > > spread. Then this patch could just move all the mutexes initialization >> > > > > that were left behind. >> > > > >> > > > I still think if only one file uses something, then that file should >> > > > include the init for it too, and nobody else should touch it. Including >> > > > locks. Ideally, they would be stowed away in allocated opaque structs >> > > > that can't even be accessed (or initialized) by anyone else. >> > > >> > > so... this version of the patch? We will need to spread the mutexes >> > > around then. >> > >> > Do we have an agreement here? >> > I'm also in favor of init all variables in the only file that touches it. >> > >> > The other mutexes can be moved gradually. >> >> I think we all agree here. Also I believe it can start with this and >> later do the rest of the clean up. >> >> Probably change the commit message to remove the Xe, which is still out >> of the tree? > >If there is no other changes, I can do that when applying. - no mention of xe in the commit message - please move the init() in intel_dkl_phy.h as the first function. - send it again, no change while applying With that, Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> Lucas De Marchi > >> >> > >> > > >> > > Lucas De Marchi >> > > >> > > > >> > > > BR, >> > > > Jani. >> > > > >> > > > >> > > > -- >> > > > Jani Nikula, Intel Open Source Graphics Center >> > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-xe] [Intel-gfx] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path 2023-04-20 16:35 ` Lucas De Marchi @ 2023-04-20 17:54 ` Souza, Jose 0 siblings, 0 replies; 11+ messages in thread From: Souza, Jose @ 2023-04-20 17:54 UTC (permalink / raw) To: De Marchi, Lucas Cc: Nikula, Jani, intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org, Vivi, Rodrigo On Thu, 2023-04-20 at 09:35 -0700, Lucas De Marchi wrote: > On Thu, Apr 20, 2023 at 08:36:37AM -0700, Jose Souza wrote: > > On Thu, 2023-04-20 at 11:27 -0400, Rodrigo Vivi wrote: > > > On Thu, Apr 20, 2023 at 09:19:09AM -0400, Souza, Jose wrote: > > > > On Wed, 2023-04-19 at 00:29 -0700, Lucas De Marchi wrote: > > > > > On Wed, Apr 19, 2023 at 10:16:22AM +0300, Jani Nikula wrote: > > > > > > On Tue, 18 Apr 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote: > > > > > > > On Tue, Apr 18, 2023 at 11:30:18PM -0700, Lucas De Marchi wrote: > > > > > > > > On Tue, Apr 18, 2023 at 09:43:37AM -0700, Jose Souza wrote: > > > > > > > > > Start to move the initialization of display locks from > > > > > > > > > i915_driver_early_probe(), this way it is also executed when running > > > > > > > > > Xe kmd. > > > > > > > > > > > > > > > > > > This will fix a warning in Xe kmd: > > > > > > > > > > > > > > > > > > [ 201.894839] xe 0000:00:02.0: [drm] [ENCODER:235:DDI A/PHY A] failed to retrieve link info, disabling eDP > > > > > > > > > [ 202.136336] xe 0000:00:02.0: [drm] *ERROR* Failed to write source OUI > > > > > > > > > [ 202.175346] INFO: trying to register non-static key. > > > > > > > > > [ 202.175347] irq event stamp: 754060 > > > > > > > > > [ 202.175359] hardirqs last enabled at (754059): [<ffffffff8122cf79>] tick_nohz_idle_enter+0x59/0x80 > > > > > > > > > [ 202.180294] The code is fine but needs lockdep annotation, or maybe > > > > > > > > > [ 202.183774] hardirqs last disabled at (754060): [<ffffffff811a5539>] do_idle+0x99/0x230 > > > > > > > > > [ 202.192734] you didn't initialize this object before use? > > > > > > > > > [ 202.198951] softirqs last enabled at (753948): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 > > > > > > > > > [ 202.206882] turning off the locking correctness validator. > > > > > > > > > [ 202.212236] softirqs last disabled at (753943): [<ffffffff8114abae>] irq_exit_rcu+0xbe/0x130 > > > > > > > > > [ 202.220592] CPU: 2 PID: 1415 Comm: modprobe Tainted: G W 6.3.0-rc4+zeh-xe+ #909 > > > > > > > > > [ 202.243002] Hardware name: Intel Corporation Raptor Lake Client Platform/RaptorLake-P LP5 RVP, BIOS RPLPFWI1.R00.3361.A14.2211151548 11/15/2022 > > > > > > > > > [ 202.255737] Call Trace: > > > > > > > > > [ 202.258179] <TASK> > > > > > > > > > [ 202.260275] dump_stack_lvl+0x58/0xc0 > > > > > > > > > [ 202.263922] register_lock_class+0x756/0x7d0 > > > > > > > > > [ 202.268165] ? find_held_lock+0x2b/0x80 > > > > > > > > > [ 202.271975] __lock_acquire+0x72/0x28b0 > > > > > > > > > [ 202.275786] ? debug_object_free+0xb4/0x160 > > > > > > > > > [ 202.279946] lock_acquire+0xd1/0x2d0 > > > > > > > > > [ 202.283503] ? intel_dkl_phy_read+0x18/0x60 [xe] > > > > > > > > > [ 202.288181] _raw_spin_lock+0x2a/0x40 > > > > > > > > > [ 202.291825] ? intel_dkl_phy_read+0x18/0x60 [xe] > > > > > > > > > [ 202.296475] intel_dkl_phy_read+0x18/0x60 [xe] > > > > > > > > > [ 202.300949] icl_aux_power_well_enable+0x2bd/0x400 [xe] > > > > > > > > > [ 202.306202] ? intel_display_power_grab_async_put_ref+0x75/0x120 [xe] > > > > > > > > > [ 202.312649] intel_power_well_enable+0x1c/0x70 [xe] > > > > > > > > > [ 202.317543] __intel_display_power_get_domain.part.0+0x4d/0x70 [xe] > > > > > > > > > [ 202.323812] intel_display_power_get+0x43/0x70 [xe] > > > > > > > > > [ 202.328708] intel_tc_port_init+0x199/0x2a0 [xe] > > > > > > > > > [ 202.333363] intel_ddi_init+0x6ad/0xb00 [xe] > > > > > > > > > [ 202.337678] intel_modeset_init_nogem+0x536/0x6d0 [xe] > > > > > > > > > [ 202.342838] xe_display_init_noaccel+0x19/0x40 [xe] > > > > > > > > > [ 202.347743] xe_device_probe+0x1f5/0x2a0 [xe] > > > > > > > > > [ 202.352127] xe_pci_probe+0x28c/0x480 [xe] > > > > > > > > > [ 202.356260] pci_device_probe+0x9d/0x150 > > > > > > > > > [ 202.360164] really_probe+0x19a/0x400 > > > > > > > > > [ 202.363809] ? __pfx___driver_attach+0x10/0x10 > > > > > > > > > [ 202.368226] __driver_probe_device+0x73/0x170 > > > > > > > > > [ 202.372558] driver_probe_device+0x1a/0x90 > > > > > > > > > [ 202.376632] __driver_attach+0xcd/0x1c0 > > > > > > > > > [ 202.380442] bus_for_each_dev+0x72/0xc0 > > > > > > > > > [ 202.384253] bus_add_driver+0x110/0x210 > > > > > > > > > [ 202.388063] driver_register+0x50/0x100 > > > > > > > > > [ 202.391873] ? __pfx_init_module+0x10/0x10 [xe] > > > > > > > > > [ 202.396431] do_one_initcall+0x55/0x260 > > > > > > > > > [ 202.400245] ? rcu_is_watching+0xd/0x40 > > > > > > > > > [ 202.404058] ? kmalloc_trace+0xa0/0xb0 > > > > > > > > > [ 202.407786] do_init_module+0x45/0x1e0 > > > > > > > > > [ 202.411512] __do_sys_finit_module+0xac/0x120 > > > > > > > > > [ 202.415838] do_syscall_64+0x37/0x90 > > > > > > > > > [ 202.419397] entry_SYSCALL_64_after_hwframe+0x72/0xdc > > > > > > > > > [ 202.424409] RIP: 0033:0x7fd11291ea3d > > > > > > > > > [ 202.427967] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 a3 0f 00 f7 d8 64 89 01 48 > > > > > > > > > [ 202.446530] RSP: 002b:00007ffffde11368 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 > > > > > > > > > [ 202.454031] RAX: ffffffffffffffda RBX: 00005616a617f210 RCX: 00007fd11291ea3d > > > > > > > > > [ 202.461106] RDX: 0000000000000000 RSI: 00005616a617fe60 RDI: 000000000000000e > > > > > > > > > [ 202.468182] RBP: 0000000000040000 R08: 0000000000000000 R09: 0000000000000002 > > > > > > > > > [ 202.475250] R10: 000000000000000e R11: 0000000000000246 R12: 00005616a617fe60 > > > > > > > > > [ 202.482319] R13: 00005616a617f340 R14: 0000000000000000 R15: 00005616a6180650 > > > > > > > > > [ 202.489396] </TASK> > > > > > > > > > > > > > > > > > > v2: > > > > > > > > > - added intel_display_locks_init() > > > > > > > > > > > > > > > > > > v3: > > > > > > > > > - rebased > > > > > > > > > > > > > > > > > > v4: > > > > > > > > > - drop intel_display_locks_init() > > > > > > > > > > > > > > > > why? > > > > > > > > > > > > > > ah... ok, now we have intel_display_driver_early_probe(). I thought you > > > > > > > were dropping the call from i915_driver.c > > > > > > > > > > > > > > Remaining question for display: do we want to spread the lock > > > > > > > initialization to each compilation unit? Or should we just keep a > > > > > > > > > > > > > > static locks_init() { <all the locks here> } > > > > > > > > > > > > > > the lock init seems a very cheap init that maybe doesn't deserve to be > > > > > > > spread. Then this patch could just move all the mutexes initialization > > > > > > > that were left behind. > > > > > > > > > > > > I still think if only one file uses something, then that file should > > > > > > include the init for it too, and nobody else should touch it. Including > > > > > > locks. Ideally, they would be stowed away in allocated opaque structs > > > > > > that can't even be accessed (or initialized) by anyone else. > > > > > > > > > > so... this version of the patch? We will need to spread the mutexes > > > > > around then. > > > > > > > > Do we have an agreement here? > > > > I'm also in favor of init all variables in the only file that touches it. > > > > > > > > The other mutexes can be moved gradually. > > > > > > I think we all agree here. Also I believe it can start with this and > > > later do the rest of the clean up. > > > > > > Probably change the commit message to remove the Xe, which is still out > > > of the tree? > > > > If there is no other changes, I can do that when applying. > > - no mention of xe in the commit message > - please move the init() in intel_dkl_phy.h as the first function. > - send it again, no change while applying > > With that, > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> Thank you. Done: https://patchwork.freedesktop.org/patch/533128/?series=116325&rev=4 Waiting CI to merge it. > > Lucas De Marchi > > > > > > > > > > > > > > > > > > > > Lucas De Marchi > > > > > > > > > > > > > > > > > BR, > > > > > > Jani. > > > > > > > > > > > > > > > > > > -- > > > > > > Jani Nikula, Intel Open Source Graphics Center > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-04-20 18:09 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-04-18 16:43 [Intel-xe] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path José Roberto de Souza 2023-04-18 16:46 ` [Intel-xe] ✗ CI.Patch_applied: failure for drm/i915: Initialize dkl_phy spin lock from display code path (rev3) Patchwork 2023-04-19 6:30 ` [Intel-xe] [PATCH v4] drm/i915: Initialize dkl_phy spin lock from display code path Lucas De Marchi 2023-04-19 6:41 ` Lucas De Marchi 2023-04-19 7:16 ` Jani Nikula 2023-04-19 7:29 ` [Intel-xe] [Intel-gfx] " Lucas De Marchi 2023-04-20 13:19 ` Souza, Jose 2023-04-20 15:27 ` Rodrigo Vivi 2023-04-20 15:36 ` Souza, Jose 2023-04-20 16:35 ` Lucas De Marchi 2023-04-20 17:54 ` Souza, Jose
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox