All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] drm/i915/audio: Skip the cdclk modeset if no pipes attached
@ 2020-02-01  9:46 Chris Wilson
  2020-02-01 10:26 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Chris Wilson @ 2020-02-01  9:46 UTC (permalink / raw)
  To: intel-gfx

If the display is not driving any pipes, we cannot change the bclk and
doing so risks chasing NULL pointers:

<6> [278.907105] snd_hda_intel 0000:00:0e.0: DSP detected with PCI class/subclass/prog-if info 0x040100
<6> [278.909936] snd_hda_intel 0000:00:0e.0: bound 0000:00:02.0 (ops i915_audio_component_bind_ops [i915])
<7> [278.910078] i915 0000:00:02.0: [drm:intel_power_well_enable [i915]] enabling power well 2
<1> [278.910228] BUG: kernel NULL pointer dereference, address: 0000000000000080
<1> [278.910243] #PF: supervisor read access in kernel mode
<1> [278.910251] #PF: error_code(0x0000) - not-present page
<6> [278.910260] PGD 0 P4D 0
<4> [278.910267] Oops: 0000 [#1] PREEMPT SMP PTI
<4> [278.910276] CPU: 0 PID: 5 Comm: kworker/0:0 Tainted: G     U            5.5.0-CI-CI_DRM_7853+ #1
<4> [278.910289] Hardware name: Intel Corp. Geminilake/GLK RVP2 LP4SD (07), BIOS GELKRVPA.X64.0062.B30.1708222146 08/22/2017
<4> [278.910312] Workqueue: events azx_probe_work [snd_hda_intel]
<4> [278.910327] RIP: 0010:__ww_mutex_lock.constprop.15+0x5e/0x1090
<4> [278.910338] Code: 75 88 be a7 03 00 00 65 48 8b 04 25 28 00 00 00 48 89 45 c8 31 c0 4c 89 c3 e8 5e b3 6d ff 44 8b 3d 2f 24 37 02 45 85 ff 75 0a <4d> 3b 6d 58 0f 85 3f 07 00 00 48 85 db 74 22 49 8b 95 80 00 00 00
<4> [278.910362] RSP: 0018:ffffc9000008bc10 EFLAGS: 00010246
<4> [278.910371] RAX: 0000000000000246 RBX: ffffc9000008bd30 RCX: 0000000000000001
<4> [278.910382] RDX: 0000000000000000 RSI: ffffffff82647c60 RDI: ffff88817b27d848
<4> [278.910393] RBP: ffffc9000008bcc0 R08: 0000000000000000 R09: 0000000000000001
<4> [278.910404] R10: ffffc9000008bce0 R11: 0000000000000000 R12: ffffffff8168f0fc
<4> [278.910414] R13: 0000000000000028 R14: ffffc9000008bd60 R15: 0000000000000000
<4> [278.910425] FS:  0000000000000000(0000) GS:ffff88817bc00000(0000) knlGS:0000000000000000
<4> [278.910437] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4> [278.910446] CR2: 0000000000000080 CR3: 00000001650da000 CR4: 0000000000340ef0
<4> [278.910456] Call Trace:
<4> [278.910468]  ? mark_held_locks+0x49/0x70
<4> [278.910479]  ? ww_mutex_lock+0x39/0x70
<4> [278.910487]  ww_mutex_lock+0x39/0x70
<4> [278.910497]  drm_modeset_lock+0x6c/0x120
<4> [278.910575]  glk_force_audio_cdclk+0x7d/0x140 [i915]
<4> [278.910656]  i915_audio_component_get_power+0xf2/0x110 [i915]
<4> [278.910673]  snd_hdac_display_power+0x7d/0x120 [snd_hda_core]
<4> [278.910686]  azx_probe_work+0x88/0x7e0 [snd_hda_intel]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/display/intel_audio.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index e3efd81c5855..3d92849811e1 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -810,16 +810,14 @@ void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
 	}
 }
 
-static int glk_force_audio_cdclk_commit(struct intel_atomic_state *state,
+static int glk_force_audio_cdclk_commit(struct intel_crtc *crtc,
+					struct intel_atomic_state *state,
 					bool enable)
 {
-	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 	struct intel_cdclk_state *cdclk_state;
-	struct intel_crtc *crtc;
 	int ret;
 
 	/* need to hold at least one crtc lock for the global state */
-	crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
 	ret = drm_modeset_lock(&crtc->base.mutex, state->base.acquire_ctx);
 	if (ret)
 		return ret;
@@ -843,8 +841,13 @@ static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
 {
 	struct drm_modeset_acquire_ctx ctx;
 	struct drm_atomic_state *state;
+	struct intel_crtc *crtc;
 	int ret;
 
+	crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
+	if (!crtc)
+		return;
+
 	drm_modeset_acquire_init(&ctx, 0);
 	state = drm_atomic_state_alloc(&dev_priv->drm);
 	if (WARN_ON(!state))
@@ -853,7 +856,9 @@ static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
 	state->acquire_ctx = &ctx;
 
 retry:
-	ret = glk_force_audio_cdclk_commit(to_intel_atomic_state(state), enable);
+	ret = glk_force_audio_cdclk_commit(crtc,
+					   to_intel_atomic_state(state),
+					   enable);
 	if (ret == -EDEADLK) {
 		drm_atomic_state_clear(state);
 		drm_modeset_backoff(&ctx);
-- 
2.25.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/audio: Skip the cdclk modeset if no pipes attached
  2020-02-01  9:46 [Intel-gfx] [PATCH] drm/i915/audio: Skip the cdclk modeset if no pipes attached Chris Wilson
@ 2020-02-01 10:26 ` Patchwork
  2020-02-01 10:45 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2020-02-01 10:26 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/audio: Skip the cdclk modeset if no pipes attached
URL   : https://patchwork.freedesktop.org/series/72863/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
74fe991ad914 drm/i915/audio: Skip the cdclk modeset if no pipes attached
-:9: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#9: 
<6> [278.907105] snd_hda_intel 0000:00:0e.0: DSP detected with PCI class/subclass/prog-if info 0x040100

total: 0 errors, 1 warnings, 0 checks, 41 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/audio: Skip the cdclk modeset if no pipes attached
  2020-02-01  9:46 [Intel-gfx] [PATCH] drm/i915/audio: Skip the cdclk modeset if no pipes attached Chris Wilson
  2020-02-01 10:26 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2020-02-01 10:45 ` Patchwork
  2020-02-01 12:36 ` [Intel-gfx] [PATCH] " Jani Nikula
  2020-02-03 13:23 ` Ville Syrjälä
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2020-02-01 10:45 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/audio: Skip the cdclk modeset if no pipes attached
URL   : https://patchwork.freedesktop.org/series/72863/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7854 -> Patchwork_16373
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16373/index.html

Known issues
------------

  Here are the changes found in Patchwork_16373 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload:
    - fi-icl-u2:          [PASS][1] -> [DMESG-WARN][2] ([i915#289]) +2 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-icl-u2/igt@i915_module_load@reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16373/fi-icl-u2/igt@i915_module_load@reload.html

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770r:       [PASS][3] -> [DMESG-FAIL][4] ([i915#770])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16373/fi-hsw-4770r/igt@i915_selftest@live_blt.html
    - fi-hsw-4770:        [PASS][5] -> [DMESG-FAIL][6] ([i915#725])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-hsw-4770/igt@i915_selftest@live_blt.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16373/fi-hsw-4770/igt@i915_selftest@live_blt.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-cml-u2:          [PASS][7] -> [FAIL][8] ([i915#262])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16373/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [PASS][9] -> [FAIL][10] ([fdo#111407])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16373/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload-no-display:
    - fi-glk-dsi:         [TIMEOUT][11] -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-glk-dsi/igt@i915_module_load@reload-no-display.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16373/fi-glk-dsi/igt@i915_module_load@reload-no-display.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-atomic:
    - fi-icl-u2:          [DMESG-WARN][13] ([i915#263]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16373/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html

  
#### Warnings ####

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-icl-u2:          [FAIL][15] ([i915#323]) -> [DMESG-WARN][16] ([IGT#4] / [i915#263])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-icl-u2/igt@kms_chamelium@common-hpd-after-suspend.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16373/fi-icl-u2/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@vga-edid-read:
    - fi-icl-u2:          [SKIP][17] ([fdo#109309]) -> [FAIL][18] ([i915#217])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7854/fi-icl-u2/igt@kms_chamelium@vga-edid-read.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16373/fi-icl-u2/igt@kms_chamelium@vga-edid-read.html

  
  [IGT#4]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/4
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [i915#217]: https://gitlab.freedesktop.org/drm/intel/issues/217
  [i915#262]: https://gitlab.freedesktop.org/drm/intel/issues/262
  [i915#263]: https://gitlab.freedesktop.org/drm/intel/issues/263
  [i915#289]: https://gitlab.freedesktop.org/drm/intel/issues/289
  [i915#323]: https://gitlab.freedesktop.org/drm/intel/issues/323
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
  [i915#770]: https://gitlab.freedesktop.org/drm/intel/issues/770


Participating hosts (48 -> 44)
------------------------------

  Additional (4): fi-hsw-peppy fi-skl-lmem fi-gdg-551 fi-ivb-3770 
  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ilk-650 fi-byt-n2820 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7854 -> Patchwork_16373

  CI-20190529: 20190529
  CI_DRM_7854: 727605cdef77d1e7eafb7e4c05b0ee74132a0930 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5410: 9d3872ede14307ef4adb0866f8474f5c41e6b1c1 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16373: 74fe991ad91484f836665768ff3264723ff00e0f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

74fe991ad914 drm/i915/audio: Skip the cdclk modeset if no pipes attached

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16373/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/audio: Skip the cdclk modeset if no pipes attached
  2020-02-01  9:46 [Intel-gfx] [PATCH] drm/i915/audio: Skip the cdclk modeset if no pipes attached Chris Wilson
  2020-02-01 10:26 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
  2020-02-01 10:45 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-02-01 12:36 ` Jani Nikula
  2020-02-01 19:09   ` Chris Wilson
  2020-02-03 13:23 ` Ville Syrjälä
  3 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2020-02-01 12:36 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Vehmanen, Kai

On Sat, 01 Feb 2020, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> If the display is not driving any pipes, we cannot change the bclk and
> doing so risks chasing NULL pointers:

Does this mean we can't probe hda if there are no displays attached at
boot on GLK?

BR,
Jani.



>
> <6> [278.907105] snd_hda_intel 0000:00:0e.0: DSP detected with PCI class/subclass/prog-if info 0x040100
> <6> [278.909936] snd_hda_intel 0000:00:0e.0: bound 0000:00:02.0 (ops i915_audio_component_bind_ops [i915])
> <7> [278.910078] i915 0000:00:02.0: [drm:intel_power_well_enable [i915]] enabling power well 2
> <1> [278.910228] BUG: kernel NULL pointer dereference, address: 0000000000000080
> <1> [278.910243] #PF: supervisor read access in kernel mode
> <1> [278.910251] #PF: error_code(0x0000) - not-present page
> <6> [278.910260] PGD 0 P4D 0
> <4> [278.910267] Oops: 0000 [#1] PREEMPT SMP PTI
> <4> [278.910276] CPU: 0 PID: 5 Comm: kworker/0:0 Tainted: G     U            5.5.0-CI-CI_DRM_7853+ #1
> <4> [278.910289] Hardware name: Intel Corp. Geminilake/GLK RVP2 LP4SD (07), BIOS GELKRVPA.X64.0062.B30.1708222146 08/22/2017
> <4> [278.910312] Workqueue: events azx_probe_work [snd_hda_intel]
> <4> [278.910327] RIP: 0010:__ww_mutex_lock.constprop.15+0x5e/0x1090
> <4> [278.910338] Code: 75 88 be a7 03 00 00 65 48 8b 04 25 28 00 00 00 48 89 45 c8 31 c0 4c 89 c3 e8 5e b3 6d ff 44 8b 3d 2f 24 37 02 45 85 ff 75 0a <4d> 3b 6d 58 0f 85 3f 07 00 00 48 85 db 74 22 49 8b 95 80 00 00 00
> <4> [278.910362] RSP: 0018:ffffc9000008bc10 EFLAGS: 00010246
> <4> [278.910371] RAX: 0000000000000246 RBX: ffffc9000008bd30 RCX: 0000000000000001
> <4> [278.910382] RDX: 0000000000000000 RSI: ffffffff82647c60 RDI: ffff88817b27d848
> <4> [278.910393] RBP: ffffc9000008bcc0 R08: 0000000000000000 R09: 0000000000000001
> <4> [278.910404] R10: ffffc9000008bce0 R11: 0000000000000000 R12: ffffffff8168f0fc
> <4> [278.910414] R13: 0000000000000028 R14: ffffc9000008bd60 R15: 0000000000000000
> <4> [278.910425] FS:  0000000000000000(0000) GS:ffff88817bc00000(0000) knlGS:0000000000000000
> <4> [278.910437] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> <4> [278.910446] CR2: 0000000000000080 CR3: 00000001650da000 CR4: 0000000000340ef0
> <4> [278.910456] Call Trace:
> <4> [278.910468]  ? mark_held_locks+0x49/0x70
> <4> [278.910479]  ? ww_mutex_lock+0x39/0x70
> <4> [278.910487]  ww_mutex_lock+0x39/0x70
> <4> [278.910497]  drm_modeset_lock+0x6c/0x120
> <4> [278.910575]  glk_force_audio_cdclk+0x7d/0x140 [i915]
> <4> [278.910656]  i915_audio_component_get_power+0xf2/0x110 [i915]
> <4> [278.910673]  snd_hdac_display_power+0x7d/0x120 [snd_hda_core]
> <4> [278.910686]  azx_probe_work+0x88/0x7e0 [snd_hda_intel]
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/display/intel_audio.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index e3efd81c5855..3d92849811e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -810,16 +810,14 @@ void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
>  	}
>  }
>  
> -static int glk_force_audio_cdclk_commit(struct intel_atomic_state *state,
> +static int glk_force_audio_cdclk_commit(struct intel_crtc *crtc,
> +					struct intel_atomic_state *state,
>  					bool enable)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>  	struct intel_cdclk_state *cdclk_state;
> -	struct intel_crtc *crtc;
>  	int ret;
>  
>  	/* need to hold at least one crtc lock for the global state */
> -	crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
>  	ret = drm_modeset_lock(&crtc->base.mutex, state->base.acquire_ctx);
>  	if (ret)
>  		return ret;
> @@ -843,8 +841,13 @@ static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
>  {
>  	struct drm_modeset_acquire_ctx ctx;
>  	struct drm_atomic_state *state;
> +	struct intel_crtc *crtc;
>  	int ret;
>  
> +	crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
> +	if (!crtc)
> +		return;
> +
>  	drm_modeset_acquire_init(&ctx, 0);
>  	state = drm_atomic_state_alloc(&dev_priv->drm);
>  	if (WARN_ON(!state))
> @@ -853,7 +856,9 @@ static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
>  	state->acquire_ctx = &ctx;
>  
>  retry:
> -	ret = glk_force_audio_cdclk_commit(to_intel_atomic_state(state), enable);
> +	ret = glk_force_audio_cdclk_commit(crtc,
> +					   to_intel_atomic_state(state),
> +					   enable);
>  	if (ret == -EDEADLK) {
>  		drm_atomic_state_clear(state);
>  		drm_modeset_backoff(&ctx);

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/audio: Skip the cdclk modeset if no pipes attached
  2020-02-01 12:36 ` [Intel-gfx] [PATCH] " Jani Nikula
@ 2020-02-01 19:09   ` Chris Wilson
  2020-02-02 10:00     ` Jani Nikula
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2020-02-01 19:09 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx; +Cc: Vehmanen, Kai

Quoting Jani Nikula (2020-02-01 12:36:46)
> On Sat, 01 Feb 2020, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > If the display is not driving any pipes, we cannot change the bclk and
> > doing so risks chasing NULL pointers:
> 
> Does this mean we can't probe hda if there are no displays attached at
> boot on GLK?

We do, but the question is should we. If we say there are no pipes, then
how can there be a HDMI/other audio link?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/audio: Skip the cdclk modeset if no pipes attached
  2020-02-01 19:09   ` Chris Wilson
@ 2020-02-02 10:00     ` Jani Nikula
  0 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2020-02-02 10:00 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Vehmanen, Kai

On Sat, 01 Feb 2020, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Jani Nikula (2020-02-01 12:36:46)
>> On Sat, 01 Feb 2020, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > If the display is not driving any pipes, we cannot change the bclk and
>> > doing so risks chasing NULL pointers:
>> 
>> Does this mean we can't probe hda if there are no displays attached at
>> boot on GLK?
>
> We do, but the question is should we. If we say there are no pipes, then
> how can there be a HDMI/other audio link?

I'll let Kai fill in the gaps, but my understanding is that you can't
even probe it then. I.e. it won't work when you *do* attach the cable.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/audio: Skip the cdclk modeset if no pipes attached
  2020-02-01  9:46 [Intel-gfx] [PATCH] drm/i915/audio: Skip the cdclk modeset if no pipes attached Chris Wilson
                   ` (2 preceding siblings ...)
  2020-02-01 12:36 ` [Intel-gfx] [PATCH] " Jani Nikula
@ 2020-02-03 13:23 ` Ville Syrjälä
  2020-02-03 13:28   ` Chris Wilson
  2020-02-03 14:46   ` Kai Vehmanen
  3 siblings, 2 replies; 9+ messages in thread
From: Ville Syrjälä @ 2020-02-03 13:23 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Sat, Feb 01, 2020 at 09:46:41AM +0000, Chris Wilson wrote:
> If the display is not driving any pipes, we cannot change the bclk and
> doing so risks chasing NULL pointers:
> 
> <6> [278.907105] snd_hda_intel 0000:00:0e.0: DSP detected with PCI class/subclass/prog-if info 0x040100
> <6> [278.909936] snd_hda_intel 0000:00:0e.0: bound 0000:00:02.0 (ops i915_audio_component_bind_ops [i915])
> <7> [278.910078] i915 0000:00:02.0: [drm:intel_power_well_enable [i915]] enabling power well 2
> <1> [278.910228] BUG: kernel NULL pointer dereference, address: 0000000000000080
> <1> [278.910243] #PF: supervisor read access in kernel mode
> <1> [278.910251] #PF: error_code(0x0000) - not-present page
> <6> [278.910260] PGD 0 P4D 0
> <4> [278.910267] Oops: 0000 [#1] PREEMPT SMP PTI
> <4> [278.910276] CPU: 0 PID: 5 Comm: kworker/0:0 Tainted: G     U            5.5.0-CI-CI_DRM_7853+ #1
> <4> [278.910289] Hardware name: Intel Corp. Geminilake/GLK RVP2 LP4SD (07), BIOS GELKRVPA.X64.0062.B30.1708222146 08/22/2017
> <4> [278.910312] Workqueue: events azx_probe_work [snd_hda_intel]
> <4> [278.910327] RIP: 0010:__ww_mutex_lock.constprop.15+0x5e/0x1090
> <4> [278.910338] Code: 75 88 be a7 03 00 00 65 48 8b 04 25 28 00 00 00 48 89 45 c8 31 c0 4c 89 c3 e8 5e b3 6d ff 44 8b 3d 2f 24 37 02 45 85 ff 75 0a <4d> 3b 6d 58 0f 85 3f 07 00 00 48 85 db 74 22 49 8b 95 80 00 00 00
> <4> [278.910362] RSP: 0018:ffffc9000008bc10 EFLAGS: 00010246
> <4> [278.910371] RAX: 0000000000000246 RBX: ffffc9000008bd30 RCX: 0000000000000001
> <4> [278.910382] RDX: 0000000000000000 RSI: ffffffff82647c60 RDI: ffff88817b27d848
> <4> [278.910393] RBP: ffffc9000008bcc0 R08: 0000000000000000 R09: 0000000000000001
> <4> [278.910404] R10: ffffc9000008bce0 R11: 0000000000000000 R12: ffffffff8168f0fc
> <4> [278.910414] R13: 0000000000000028 R14: ffffc9000008bd60 R15: 0000000000000000
> <4> [278.910425] FS:  0000000000000000(0000) GS:ffff88817bc00000(0000) knlGS:0000000000000000
> <4> [278.910437] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> <4> [278.910446] CR2: 0000000000000080 CR3: 00000001650da000 CR4: 0000000000340ef0
> <4> [278.910456] Call Trace:
> <4> [278.910468]  ? mark_held_locks+0x49/0x70
> <4> [278.910479]  ? ww_mutex_lock+0x39/0x70
> <4> [278.910487]  ww_mutex_lock+0x39/0x70
> <4> [278.910497]  drm_modeset_lock+0x6c/0x120
> <4> [278.910575]  glk_force_audio_cdclk+0x7d/0x140 [i915]
> <4> [278.910656]  i915_audio_component_get_power+0xf2/0x110 [i915]
> <4> [278.910673]  snd_hdac_display_power+0x7d/0x120 [snd_hda_core]
> <4> [278.910686]  azx_probe_work+0x88/0x7e0 [snd_hda_intel]
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/display/intel_audio.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index e3efd81c5855..3d92849811e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -810,16 +810,14 @@ void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
>  	}
>  }
>  
> -static int glk_force_audio_cdclk_commit(struct intel_atomic_state *state,
> +static int glk_force_audio_cdclk_commit(struct intel_crtc *crtc,
> +					struct intel_atomic_state *state,
>  					bool enable)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>  	struct intel_cdclk_state *cdclk_state;
> -	struct intel_crtc *crtc;
>  	int ret;
>  
>  	/* need to hold at least one crtc lock for the global state */
> -	crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);

Was thinking a simple 'return 0' would do the trick, but maybe we
don't want to call this at all. OTOH not sure why we would even
register the audio component if there are no pipes. Does the audio
driver get confused if we don't do that?

>  	ret = drm_modeset_lock(&crtc->base.mutex, state->base.acquire_ctx);
>  	if (ret)
>  		return ret;
> @@ -843,8 +841,13 @@ static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
>  {
>  	struct drm_modeset_acquire_ctx ctx;
>  	struct drm_atomic_state *state;
> +	struct intel_crtc *crtc;
>  	int ret;
>  
> +	crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
> +	if (!crtc)
> +		return;
> +
>  	drm_modeset_acquire_init(&ctx, 0);
>  	state = drm_atomic_state_alloc(&dev_priv->drm);
>  	if (WARN_ON(!state))
> @@ -853,7 +856,9 @@ static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
>  	state->acquire_ctx = &ctx;
>  
>  retry:
> -	ret = glk_force_audio_cdclk_commit(to_intel_atomic_state(state), enable);
> +	ret = glk_force_audio_cdclk_commit(crtc,
> +					   to_intel_atomic_state(state),
> +					   enable);

I guess I'd order these state,crtc,enable to match more what we've been
doing elsewhere.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	if (ret == -EDEADLK) {
>  		drm_atomic_state_clear(state);
>  		drm_modeset_backoff(&ctx);
> -- 
> 2.25.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/audio: Skip the cdclk modeset if no pipes attached
  2020-02-03 13:23 ` Ville Syrjälä
@ 2020-02-03 13:28   ` Chris Wilson
  2020-02-03 14:46   ` Kai Vehmanen
  1 sibling, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2020-02-03 13:28 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

Quoting Ville Syrjälä (2020-02-03 13:23:17)
> On Sat, Feb 01, 2020 at 09:46:41AM +0000, Chris Wilson wrote:
> > -static int glk_force_audio_cdclk_commit(struct intel_atomic_state *state,
> > +static int glk_force_audio_cdclk_commit(struct intel_crtc *crtc,
> > +                                     struct intel_atomic_state *state,
> >                                       bool enable)
> >  {
> > -     struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> >       struct intel_cdclk_state *cdclk_state;
> > -     struct intel_crtc *crtc;
> >       int ret;
> >  
> >       /* need to hold at least one crtc lock for the global state */
> > -     crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
> 
> Was thinking a simple 'return 0' would do the trick, but maybe we
> don't want to call this at all. OTOH not sure why we would even
> register the audio component if there are no pipes. Does the audio
> driver get confused if we don't do that?

I expect that is the most correct answer: don't register display audio
if the display insists on there being no pipes. I'll leave that to the
intrepid observers though :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/audio: Skip the cdclk modeset if no pipes attached
  2020-02-03 13:23 ` Ville Syrjälä
  2020-02-03 13:28   ` Chris Wilson
@ 2020-02-03 14:46   ` Kai Vehmanen
  1 sibling, 0 replies; 9+ messages in thread
From: Kai Vehmanen @ 2020-02-03 14:46 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 554 bytes --]

Hey,

On Mon, 3 Feb 2020, Ville Syrjälä wrote:

> >  	/* need to hold at least one crtc lock for the global state */
> > -	crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
> 
> Was thinking a simple 'return 0' would do the trick, but maybe we
> don't want to call this at all. OTOH not sure why we would even
> register the audio component if there are no pipes. Does the audio
> driver get confused if we don't do that?

that's ok as well, audio driver will return fail for the probe, but in 
this case I guess this is ok (and even expected).

Br, Kai

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-02-03 14:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-01  9:46 [Intel-gfx] [PATCH] drm/i915/audio: Skip the cdclk modeset if no pipes attached Chris Wilson
2020-02-01 10:26 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2020-02-01 10:45 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-02-01 12:36 ` [Intel-gfx] [PATCH] " Jani Nikula
2020-02-01 19:09   ` Chris Wilson
2020-02-02 10:00     ` Jani Nikula
2020-02-03 13:23 ` Ville Syrjälä
2020-02-03 13:28   ` Chris Wilson
2020-02-03 14:46   ` Kai Vehmanen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.