| Message ID | 20230410183219.191543-1-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/6] drm/i915: Initialize dkl_phy spin lock from display code path | expand |
On Mon, 10 Apr 2023, José Roberto de Souza <jose.souza@intel.com> wrote: > Start to move the initialization of some lock from > i915_driver_early_probe(). Please send this as standalone patch to i915. It won't get merged to i915 as part of series only parts of which apply to i915. BR, Jani. > No dkl function is called prior to intel_setup_outputs(), so this is > a good place to initialize it. > > This will also 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> > > Cc: intel-gfx@lists.freedesktop.org > Cc: intel-xe@lists.freedesktop.org > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 3 +++ > drivers/gpu/drm/i915/display/intel_dkl_phy.c | 6 ++++++ > drivers/gpu/drm/i915/display/intel_dkl_phy.h | 2 ++ > drivers/gpu/drm/i915/i915_driver.c | 1 - > 4 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 24c96f388d521..cf3a3b3afdd13 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -83,6 +83,7 @@ > #include "intel_display_debugfs.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_dp_link_training.h" > @@ -7883,6 +7884,8 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv) > if (!HAS_DISPLAY(dev_priv)) > return; > > + intel_dkl_phy_init(dev_priv); > + > if (IS_DG2(dev_priv)) { > intel_ddi_init(dev_priv, PORT_A); > intel_ddi_init(dev_priv, PORT_B); > diff --git a/drivers/gpu/drm/i915/display/intel_dkl_phy.c b/drivers/gpu/drm/i915/display/intel_dkl_phy.c > index 57cc3edba0163..5bce7b5b27bc7 100644 > --- a/drivers/gpu/drm/i915/display/intel_dkl_phy.c > +++ b/drivers/gpu/drm/i915/display/intel_dkl_phy.c > @@ -104,3 +104,9 @@ 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..615429b6392c5 100644 > --- a/drivers/gpu/drm/i915/display/intel_dkl_phy.h > +++ b/drivers/gpu/drm/i915/display/intel_dkl_phy.h > @@ -20,5 +20,7 @@ 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 066d79c2069c4..6fe1a97cc246d 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -224,7 +224,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);
On Tue, 11 Apr 2023, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Mon, 10 Apr 2023, José Roberto de Souza <jose.souza@intel.com> wrote: >> Start to move the initialization of some lock from >> i915_driver_early_probe(). > > Please send this as standalone patch to i915. It won't get merged to > i915 as part of series only parts of which apply to i915. Not to mention drm-xe-next is *not* the path to upstream i915 changes. Each i915 merged there is a potential problem in upstreaming xe. BR, Jani.
On Mon, Apr 10, 2023 at 11:32:14AM -0700, José Roberto de Souza wrote: > Start to move the initialization of some lock from > i915_driver_early_probe(). > No dkl function is called prior to intel_setup_outputs(), so this is > a good place to initialize it. I disagree. We don't want to sprinke these all over the place. > This will also 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> > > Cc: intel-gfx@lists.freedesktop.org > Cc: intel-xe@lists.freedesktop.org > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 3 +++ > drivers/gpu/drm/i915/display/intel_dkl_phy.c | 6 ++++++ > drivers/gpu/drm/i915/display/intel_dkl_phy.h | 2 ++ > drivers/gpu/drm/i915/i915_driver.c | 1 - > 4 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 24c96f388d521..cf3a3b3afdd13 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -83,6 +83,7 @@ > #include "intel_display_debugfs.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_dp_link_training.h" > @@ -7883,6 +7884,8 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv) > if (!HAS_DISPLAY(dev_priv)) > return; > > + intel_dkl_phy_init(dev_priv); > + > if (IS_DG2(dev_priv)) { > intel_ddi_init(dev_priv, PORT_A); > intel_ddi_init(dev_priv, PORT_B); > diff --git a/drivers/gpu/drm/i915/display/intel_dkl_phy.c b/drivers/gpu/drm/i915/display/intel_dkl_phy.c > index 57cc3edba0163..5bce7b5b27bc7 100644 > --- a/drivers/gpu/drm/i915/display/intel_dkl_phy.c > +++ b/drivers/gpu/drm/i915/display/intel_dkl_phy.c > @@ -104,3 +104,9 @@ 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..615429b6392c5 100644 > --- a/drivers/gpu/drm/i915/display/intel_dkl_phy.h > +++ b/drivers/gpu/drm/i915/display/intel_dkl_phy.h > @@ -20,5 +20,7 @@ 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 066d79c2069c4..6fe1a97cc246d 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -224,7 +224,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
On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Mon, Apr 10, 2023 at 11:32:14AM -0700, José Roberto de Souza wrote: >> Start to move the initialization of some lock from >> i915_driver_early_probe(). >> No dkl function is called prior to intel_setup_outputs(), so this is >> a good place to initialize it. > > I disagree. We don't want to sprinke these all over the place. I'm thinking if only foo.c uses a lock, foo.c should initialize it, not someone else. BR, Jani. > >> This will also 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> >> >> Cc: intel-gfx@lists.freedesktop.org >> Cc: intel-xe@lists.freedesktop.org >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_display.c | 3 +++ >> drivers/gpu/drm/i915/display/intel_dkl_phy.c | 6 ++++++ >> drivers/gpu/drm/i915/display/intel_dkl_phy.h | 2 ++ >> drivers/gpu/drm/i915/i915_driver.c | 1 - >> 4 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> index 24c96f388d521..cf3a3b3afdd13 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -83,6 +83,7 @@ >> #include "intel_display_debugfs.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_dp_link_training.h" >> @@ -7883,6 +7884,8 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv) >> if (!HAS_DISPLAY(dev_priv)) >> return; >> >> + intel_dkl_phy_init(dev_priv); >> + >> if (IS_DG2(dev_priv)) { >> intel_ddi_init(dev_priv, PORT_A); >> intel_ddi_init(dev_priv, PORT_B); >> diff --git a/drivers/gpu/drm/i915/display/intel_dkl_phy.c b/drivers/gpu/drm/i915/display/intel_dkl_phy.c >> index 57cc3edba0163..5bce7b5b27bc7 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dkl_phy.c >> +++ b/drivers/gpu/drm/i915/display/intel_dkl_phy.c >> @@ -104,3 +104,9 @@ 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..615429b6392c5 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dkl_phy.h >> +++ b/drivers/gpu/drm/i915/display/intel_dkl_phy.h >> @@ -20,5 +20,7 @@ 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 066d79c2069c4..6fe1a97cc246d 100644 >> --- a/drivers/gpu/drm/i915/i915_driver.c >> +++ b/drivers/gpu/drm/i915/i915_driver.c >> @@ -224,7 +224,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
On Tue, Apr 11, 2023 at 11:51:33AM +0300, Jani Nikula wrote: > On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Mon, Apr 10, 2023 at 11:32:14AM -0700, José Roberto de Souza wrote: > >> Start to move the initialization of some lock from > >> i915_driver_early_probe(). > >> No dkl function is called prior to intel_setup_outputs(), so this is > >> a good place to initialize it. > > > > I disagree. We don't want to sprinke these all over the place. > > I'm thinking if only foo.c uses a lock, foo.c should initialize it, not > someone else. Perhaps. But I think there should be some consistent place in the higher level code where all such things get called instead of dropping each one individually into some random spot in the overlall display init flow.
On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Tue, Apr 11, 2023 at 11:51:33AM +0300, Jani Nikula wrote: >> On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> > On Mon, Apr 10, 2023 at 11:32:14AM -0700, José Roberto de Souza wrote: >> >> Start to move the initialization of some lock from >> >> i915_driver_early_probe(). >> >> No dkl function is called prior to intel_setup_outputs(), so this is >> >> a good place to initialize it. >> > >> > I disagree. We don't want to sprinke these all over the place. >> >> I'm thinking if only foo.c uses a lock, foo.c should initialize it, not >> someone else. > > Perhaps. But I think there should be some consistent place in the higher > level code where all such things get called instead of dropping each one > individually into some random spot in the overlall display init flow. Agreed.
On Tue, 2023-04-11 at 11:33 +0300, Jani Nikula wrote: > On Tue, 11 Apr 2023, Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Mon, 10 Apr 2023, José Roberto de Souza <jose.souza@intel.com> wrote: > > > Start to move the initialization of some lock from > > > i915_driver_early_probe(). > > > > Please send this as standalone patch to i915. It won't get merged to > > i915 as part of series only parts of which apply to i915. > > Not to mention drm-xe-next is *not* the path to upstream i915 > changes. Each i915 merged there is a potential problem in upstreaming > xe. Already sent it https://patchwork.freedesktop.org/series/116272/ > > BR, > Jani. > >
On Tue, Apr 11, 2023 at 12:14:36PM +0300, Jani Nikula wrote: > On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Tue, Apr 11, 2023 at 11:51:33AM +0300, Jani Nikula wrote: > >> On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > >> > On Mon, Apr 10, 2023 at 11:32:14AM -0700, José Roberto de Souza wrote: > >> >> Start to move the initialization of some lock from > >> >> i915_driver_early_probe(). > >> >> No dkl function is called prior to intel_setup_outputs(), so this is > >> >> a good place to initialize it. > >> > > >> > I disagree. We don't want to sprinke these all over the place. > >> > >> I'm thinking if only foo.c uses a lock, foo.c should initialize it, not > >> someone else. > > > > Perhaps. But I think there should be some consistent place in the higher > > level code where all such things get called instead of dropping each one > > individually into some random spot in the overlall display init flow. > > Agreed. Ops, I just saw this now, right after I cc'ed you in the other thread. So, probably good to hold this and do the entire refactor together of all those locks initialization so we find this common consistent place apparently... > > -- > Jani Nikula, Intel Open Source Graphics Center
On Tue, Apr 11, 2023 at 10:51:04AM -0400, Rodrigo Vivi wrote: >On Tue, Apr 11, 2023 at 12:14:36PM +0300, Jani Nikula wrote: >> On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> > On Tue, Apr 11, 2023 at 11:51:33AM +0300, Jani Nikula wrote: >> >> On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> >> > On Mon, Apr 10, 2023 at 11:32:14AM -0700, José Roberto de Souza wrote: >> >> >> Start to move the initialization of some lock from >> >> >> i915_driver_early_probe(). >> >> >> No dkl function is called prior to intel_setup_outputs(), so this is >> >> >> a good place to initialize it. >> >> > >> >> > I disagree. We don't want to sprinke these all over the place. >> >> >> >> I'm thinking if only foo.c uses a lock, foo.c should initialize it, not >> >> someone else. >> > >> > Perhaps. But I think there should be some consistent place in the higher >> > level code where all such things get called instead of dropping each one >> > individually into some random spot in the overlall display init flow. >> >> Agreed. > >Ops, I just saw this now, right after I cc'ed you in the other thread. > >So, probably good to hold this and do the entire refactor together of all >those locks initialization so we find this common consistent place apparently... "internal" sw initialization of display-related stuff. It doesn't belong in i915_driver_early_probe(), it makes harder to follow the sequence if we sprinkle them around, like here in intel_setup_outputs. But I don't see why this couldn't be done in a higher level "sw initialization of display-related stuff". Should we add an equivalent of i915_driver_early_probe(), e.g. intel_display_early_probe()[1], and move the display-related things from i915_driver_early_probe()? In that case, from xe we would call this function rather than initializing these fields in xe_display_create() Lucas De Marchi [1] I don't like the name, but it follows what is already there > >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
On Tue, 2023-04-11 at 12:59 -0700, Lucas De Marchi wrote: > On Tue, Apr 11, 2023 at 10:51:04AM -0400, Rodrigo Vivi wrote: > > On Tue, Apr 11, 2023 at 12:14:36PM +0300, Jani Nikula wrote: > > > On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > > > On Tue, Apr 11, 2023 at 11:51:33AM +0300, Jani Nikula wrote: > > > > > On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > > > > > On Mon, Apr 10, 2023 at 11:32:14AM -0700, José Roberto de Souza wrote: > > > > > > > Start to move the initialization of some lock from > > > > > > > i915_driver_early_probe(). > > > > > > > No dkl function is called prior to intel_setup_outputs(), so this is > > > > > > > a good place to initialize it. > > > > > > > > > > > > I disagree. We don't want to sprinke these all over the place. > > > > > > > > > > I'm thinking if only foo.c uses a lock, foo.c should initialize it, not > > > > > someone else. > > > > > > > > Perhaps. But I think there should be some consistent place in the higher > > > > level code where all such things get called instead of dropping each one > > > > individually into some random spot in the overlall display init flow. > > > > > > Agreed. > > > > Ops, I just saw this now, right after I cc'ed you in the other thread. > > > > So, probably good to hold this and do the entire refactor together of all > > those locks initialization so we find this common consistent place apparently... > > "internal" sw initialization of display-related stuff. It doesn't belong in > i915_driver_early_probe(), it makes harder to follow the sequence if we sprinkle > them around, like here in intel_setup_outputs. > > But I don't see why this couldn't be done in a higher level "sw > initialization of display-related stuff". Should we add an equivalent > of i915_driver_early_probe(), e.g. intel_display_early_probe()[1], and > move the display-related things from i915_driver_early_probe()? > > In that case, from xe we would call this function rather than > initializing these fields in xe_display_create() Sent another version, added intel_display_locks_init() that is called in the beginning of intel_modeset_init_noirq(). https://patchwork.freedesktop.org/series/116326/ If this is accepted we can then move the other display locks from i915_driver_early_probe(). > > Lucas De Marchi > > [1] I don't like the name, but it follows what is already there > > > > > > > > > -- > > > Jani Nikula, Intel Open Source Graphics Center
On Tue, Apr 11, 2023 at 08:07:12PM +0000, Jose Souza wrote: >On Tue, 2023-04-11 at 12:59 -0700, Lucas De Marchi wrote: >> On Tue, Apr 11, 2023 at 10:51:04AM -0400, Rodrigo Vivi wrote: >> > On Tue, Apr 11, 2023 at 12:14:36PM +0300, Jani Nikula wrote: >> > > On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> > > > On Tue, Apr 11, 2023 at 11:51:33AM +0300, Jani Nikula wrote: >> > > > > On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> > > > > > On Mon, Apr 10, 2023 at 11:32:14AM -0700, José Roberto de Souza wrote: >> > > > > > > Start to move the initialization of some lock from >> > > > > > > i915_driver_early_probe(). >> > > > > > > No dkl function is called prior to intel_setup_outputs(), so this is >> > > > > > > a good place to initialize it. >> > > > > > >> > > > > > I disagree. We don't want to sprinke these all over the place. >> > > > > >> > > > > I'm thinking if only foo.c uses a lock, foo.c should initialize it, not >> > > > > someone else. >> > > > >> > > > Perhaps. But I think there should be some consistent place in the higher >> > > > level code where all such things get called instead of dropping each one >> > > > individually into some random spot in the overlall display init flow. >> > > >> > > Agreed. >> > >> > Ops, I just saw this now, right after I cc'ed you in the other thread. >> > >> > So, probably good to hold this and do the entire refactor together of all >> > those locks initialization so we find this common consistent place apparently... >> >> "internal" sw initialization of display-related stuff. It doesn't belong in >> i915_driver_early_probe(), it makes harder to follow the sequence if we sprinkle >> them around, like here in intel_setup_outputs. >> >> But I don't see why this couldn't be done in a higher level "sw >> initialization of display-related stuff". Should we add an equivalent >> of i915_driver_early_probe(), e.g. intel_display_early_probe()[1], and >> move the display-related things from i915_driver_early_probe()? >> >> In that case, from xe we would call this function rather than >> initializing these fields in xe_display_create() > >Sent another version, added intel_display_locks_init() that is called in the beginning of intel_modeset_init_noirq(). >https://patchwork.freedesktop.org/series/116326/ modeset? why? That is after we are already probing the hw.... and what does that have to do with modeset? Lucas De Marchi > >If this is accepted we can then move the other display locks from i915_driver_early_probe(). > >> >> Lucas De Marchi >> >> [1] I don't like the name, but it follows what is already there >> >> > >> > > >> > > -- >> > > Jani Nikula, Intel Open Source Graphics Center >
On Tue, 2023-04-11 at 14:20 -0700, Lucas De Marchi wrote: > On Tue, Apr 11, 2023 at 08:07:12PM +0000, Jose Souza wrote: > > On Tue, 2023-04-11 at 12:59 -0700, Lucas De Marchi wrote: > > > On Tue, Apr 11, 2023 at 10:51:04AM -0400, Rodrigo Vivi wrote: > > > > On Tue, Apr 11, 2023 at 12:14:36PM +0300, Jani Nikula wrote: > > > > > On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > > > > > On Tue, Apr 11, 2023 at 11:51:33AM +0300, Jani Nikula wrote: > > > > > > > On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > > > > > > > On Mon, Apr 10, 2023 at 11:32:14AM -0700, José Roberto de Souza wrote: > > > > > > > > > Start to move the initialization of some lock from > > > > > > > > > i915_driver_early_probe(). > > > > > > > > > No dkl function is called prior to intel_setup_outputs(), so this is > > > > > > > > > a good place to initialize it. > > > > > > > > > > > > > > > > I disagree. We don't want to sprinke these all over the place. > > > > > > > > > > > > > > I'm thinking if only foo.c uses a lock, foo.c should initialize it, not > > > > > > > someone else. > > > > > > > > > > > > Perhaps. But I think there should be some consistent place in the higher > > > > > > level code where all such things get called instead of dropping each one > > > > > > individually into some random spot in the overlall display init flow. > > > > > > > > > > Agreed. > > > > > > > > Ops, I just saw this now, right after I cc'ed you in the other thread. > > > > > > > > So, probably good to hold this and do the entire refactor together of all > > > > those locks initialization so we find this common consistent place apparently... > > > > > > "internal" sw initialization of display-related stuff. It doesn't belong in > > > i915_driver_early_probe(), it makes harder to follow the sequence if we sprinkle > > > them around, like here in intel_setup_outputs. > > > > > > But I don't see why this couldn't be done in a higher level "sw > > > initialization of display-related stuff". Should we add an equivalent > > > of i915_driver_early_probe(), e.g. intel_display_early_probe()[1], and > > > move the display-related things from i915_driver_early_probe()? > > > > > > In that case, from xe we would call this function rather than > > > initializing these fields in xe_display_create() > > > > Sent another version, added intel_display_locks_init() that is called in the beginning of intel_modeset_init_noirq(). > > https://patchwork.freedesktop.org/series/116326/ > > modeset? why? That is after we are already probing the hw.... That called during probe, called from i915_driver_probe(). > and what does that have to do with modeset? The name is misleading but intel_modeset_init_noirq() is the first generic display initialization function called. There is other display functions called before it but they are very specific(intel_dram_detect(), intel_bw_init_hw() and intel_device_info_runtime_init()). > > Lucas De Marchi > > > > > If this is accepted we can then move the other display locks from i915_driver_early_probe(). > > > > > > > > Lucas De Marchi > > > > > > [1] I don't like the name, but it follows what is already there > > > > > > > > > > > > > > > > > -- > > > > > Jani Nikula, Intel Open Source Graphics Center > >
On Wed, 12 Apr 2023, "Souza, Jose" <jose.souza@intel.com> wrote: > On Tue, 2023-04-11 at 14:20 -0700, Lucas De Marchi wrote: >> On Tue, Apr 11, 2023 at 08:07:12PM +0000, Jose Souza wrote: >> > On Tue, 2023-04-11 at 12:59 -0700, Lucas De Marchi wrote: >> > > On Tue, Apr 11, 2023 at 10:51:04AM -0400, Rodrigo Vivi wrote: >> > > > On Tue, Apr 11, 2023 at 12:14:36PM +0300, Jani Nikula wrote: >> > > > > On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> > > > > > On Tue, Apr 11, 2023 at 11:51:33AM +0300, Jani Nikula wrote: >> > > > > > > On Tue, 11 Apr 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> > > > > > > > On Mon, Apr 10, 2023 at 11:32:14AM -0700, José Roberto de Souza wrote: >> > > > > > > > > Start to move the initialization of some lock from >> > > > > > > > > i915_driver_early_probe(). >> > > > > > > > > No dkl function is called prior to intel_setup_outputs(), so this is >> > > > > > > > > a good place to initialize it. >> > > > > > > > >> > > > > > > > I disagree. We don't want to sprinke these all over the place. >> > > > > > > >> > > > > > > I'm thinking if only foo.c uses a lock, foo.c should initialize it, not >> > > > > > > someone else. >> > > > > > >> > > > > > Perhaps. But I think there should be some consistent place in the higher >> > > > > > level code where all such things get called instead of dropping each one >> > > > > > individually into some random spot in the overlall display init flow. >> > > > > >> > > > > Agreed. >> > > > >> > > > Ops, I just saw this now, right after I cc'ed you in the other thread. >> > > > >> > > > So, probably good to hold this and do the entire refactor together of all >> > > > those locks initialization so we find this common consistent place apparently... >> > > >> > > "internal" sw initialization of display-related stuff. It doesn't belong in >> > > i915_driver_early_probe(), it makes harder to follow the sequence if we sprinkle >> > > them around, like here in intel_setup_outputs. >> > > >> > > But I don't see why this couldn't be done in a higher level "sw >> > > initialization of display-related stuff". Should we add an equivalent >> > > of i915_driver_early_probe(), e.g. intel_display_early_probe()[1], and >> > > move the display-related things from i915_driver_early_probe()? >> > > >> > > In that case, from xe we would call this function rather than >> > > initializing these fields in xe_display_create() >> > >> > Sent another version, added intel_display_locks_init() that is called in the beginning of intel_modeset_init_noirq(). >> > https://patchwork.freedesktop.org/series/116326/ >> >> modeset? why? That is after we are already probing the hw.... > > That called during probe, called from i915_driver_probe(). > >> and what does that have to do with modeset? > > The name is misleading but intel_modeset_init_noirq() is the first generic display initialization function called. > There is other display functions called before it but they are very specific(intel_dram_detect(), intel_bw_init_hw() and > intel_device_info_runtime_init()). See [1]. BR, Jani. [1] https://lore.kernel.org/r/87ile1cjh8.fsf@intel.com > >> >> Lucas De Marchi >> >> > >> > If this is accepted we can then move the other display locks from i915_driver_early_probe(). >> > >> > > >> > > Lucas De Marchi >> > > >> > > [1] I don't like the name, but it follows what is already there >> > > >> > > > >> > > > > >> > > > > -- >> > > > > Jani Nikula, Intel Open Source Graphics Center >> > >
On Thu, 13 Apr 2023, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Wed, 12 Apr 2023, "Souza, Jose" <jose.souza@intel.com> wrote: >> The name is misleading but intel_modeset_init_noirq() is the first generic display initialization function called. >> There is other display functions called before it but they are very specific(intel_dram_detect(), intel_bw_init_hw() and >> intel_device_info_runtime_init()). > > See [1]. > > BR, > Jani. > > > [1] https://lore.kernel.org/r/87ile1cjh8.fsf@intel.com All of this is cleaned up in https://patchwork.freedesktop.org/series/116431/ BR, Jani.
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 24c96f388d521..cf3a3b3afdd13 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -83,6 +83,7 @@ #include "intel_display_debugfs.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_dp_link_training.h" @@ -7883,6 +7884,8 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv) if (!HAS_DISPLAY(dev_priv)) return; + intel_dkl_phy_init(dev_priv); + if (IS_DG2(dev_priv)) { intel_ddi_init(dev_priv, PORT_A); intel_ddi_init(dev_priv, PORT_B); diff --git a/drivers/gpu/drm/i915/display/intel_dkl_phy.c b/drivers/gpu/drm/i915/display/intel_dkl_phy.c index 57cc3edba0163..5bce7b5b27bc7 100644 --- a/drivers/gpu/drm/i915/display/intel_dkl_phy.c +++ b/drivers/gpu/drm/i915/display/intel_dkl_phy.c @@ -104,3 +104,9 @@ 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..615429b6392c5 100644 --- a/drivers/gpu/drm/i915/display/intel_dkl_phy.h +++ b/drivers/gpu/drm/i915/display/intel_dkl_phy.h @@ -20,5 +20,7 @@ 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 066d79c2069c4..6fe1a97cc246d 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -224,7 +224,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);
Start to move the initialization of some lock from i915_driver_early_probe(). No dkl function is called prior to intel_setup_outputs(), so this is a good place to initialize it. This will also 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> Cc: intel-gfx@lists.freedesktop.org Cc: intel-xe@lists.freedesktop.org Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 3 +++ drivers/gpu/drm/i915/display/intel_dkl_phy.c | 6 ++++++ drivers/gpu/drm/i915/display/intel_dkl_phy.h | 2 ++ drivers/gpu/drm/i915/i915_driver.c | 1 - 4 files changed, 11 insertions(+), 1 deletion(-)