[PATCH] drm/i915/audio: set minimum CD clock to twice the BCLK

[PATCH] drm/i915/audio: set minimum CD clock to twice the BCLK

[Jani Nikula]

From: Abhay Kumar <abhay.kumar@intel.com>

In GLK when the device boots with only 1366x768 panel without audio, HDA
codec doesn't come up. In this case, the CDCLK is less than twice the
BCLK. Even though audio isn't being enabled, having a too low CDCLK
leads to audio probe failing altogether.

Require CDCLK to be at least twice the BLCK regardless of audio. This is
a minimal fix to improve things. Unfortunately, this a) leads to too
high CDCLK being used when audio is not used, and b) is still not enough
to fix audio probe when no outputs are connected at probe time.

The proper fix would be to increase CDCLK dynamically from the audio
component hooks.

v2:
    - Address comment (Jani)
    - New design approach
v3: - Typo fix on top of v1

v4 by Jani: rewrite commit message, add comment in code

Cc: stable@vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com>
Cc: Wenkai Du <wenkai.du@intel.com>
Reviewed-by: Wenkai Du <wenkai.du@intel.com>
Tested-by: Wenkai Du <wenkai.du@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index fc8b2c6e3508..32d24c69da3c 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2140,10 +2140,22 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 		}
 	}
 
-	/* According to BSpec, "The CD clock frequency must be at least twice
+	/*
+	 * According to BSpec, "The CD clock frequency must be at least twice
 	 * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
+	 *
+	 * FIXME: Check the actual, not default, BCLK being used.
+	 *
+	 * FIXME: This does not depend on ->has_audio because the higher CDCLK
+	 * is required for audio probe, also when there are no audio capable
+	 * displays connected at probe time. This leads to unnecessarily high
+	 * CDCLK when audio is not required.
+	 *
+	 * FIXME: This limit is only applied when there are displays connected
+	 * at probe time. If we probe without displays, we'll still end up using
+	 * the platform minimum CDCLK, failing audio probe.
 	 */
-	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
+	if (INTEL_GEN(dev_priv) >= 9)
 		min_cdclk = max(2 * 96000, min_cdclk);
 
 	/*
-- 
2.11.0

[PATCH] drm/i915/audio: set minimum CD clock to twice the BCLK

[Jani Nikula]

From: Abhay Kumar <abhay.kumar@intel.com>

In GLK when the device boots with only 1366x768 panel without audio, HDA
codec doesn't come up. In this case, the CDCLK is less than twice the
BCLK. Even though audio isn't being enabled, having a too low CDCLK
leads to audio probe failing altogether.

Require CDCLK to be at least twice the BLCK regardless of audio. This is
a minimal fix to improve things. Unfortunately, this a) leads to too
high CDCLK being used when audio is not used, and b) is still not enough
to fix audio probe when no outputs are connected at probe time.

The proper fix would be to increase CDCLK dynamically from the audio
component hooks.

v2:
    - Address comment (Jani)
    - New design approach
v3: - Typo fix on top of v1

v4 by Jani: rewrite commit message, add comment in code

Cc: stable@vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com>
Cc: Wenkai Du <wenkai.du@intel.com>
Reviewed-by: Wenkai Du <wenkai.du@intel.com>
Tested-by: Wenkai Du <wenkai.du@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index fc8b2c6e3508..32d24c69da3c 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2140,10 +2140,22 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 		}
 	}
 
-	/* According to BSpec, "The CD clock frequency must be at least twice
+	/*
+	 * According to BSpec, "The CD clock frequency must be at least twice
 	 * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
+	 *
+	 * FIXME: Check the actual, not default, BCLK being used.
+	 *
+	 * FIXME: This does not depend on ->has_audio because the higher CDCLK
+	 * is required for audio probe, also when there are no audio capable
+	 * displays connected at probe time. This leads to unnecessarily high
+	 * CDCLK when audio is not required.
+	 *
+	 * FIXME: This limit is only applied when there are displays connected
+	 * at probe time. If we probe without displays, we'll still end up using
+	 * the platform minimum CDCLK, failing audio probe.
 	 */
-	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
+	if (INTEL_GEN(dev_priv) >= 9)
 		min_cdclk = max(2 * 96000, min_cdclk);
 
 	/*
-- 
2.11.0

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

✗ Fi.CI.SPARSE: warning for drm/i915/audio: set minimum CD clock to twice the BCLK

[Patchwork]

== Series Details ==

Series: drm/i915/audio: set minimum CD clock to twice the BCLK
URL   : https://patchwork.freedesktop.org/series/41888/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/audio: set minimum CD clock to twice the BCLK
-O:drivers/gpu/drm/i915/intel_cdclk.c:2147:29: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_cdclk.c:2159:29: warning: expression using sizeof(void)

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

✓ Fi.CI.BAT: success for drm/i915/audio: set minimum CD clock to twice the BCLK

[Patchwork]

== Series Details ==

Series: drm/i915/audio: set minimum CD clock to twice the BCLK
URL   : https://patchwork.freedesktop.org/series/41888/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4066 -> Patchwork_8735 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/41888/revisions/1/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-ivb-3520m:       PASS -> DMESG-WARN (fdo#106084)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-ivb-3520m:       DMESG-WARN (fdo#106084) -> PASS

    
  fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084


== Participating hosts (33 -> 31) ==

  Additional (2): fi-glk-j4005 fi-bxt-dsi 
  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-glk-1 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4066 -> Patchwork_8735

  CI_DRM_4066: e1fbca4821d0700551df233285a5c28db09fd0f6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4441: 83ba5b7d3bde48b383df41792fc9c955a5a23bdb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8735: bfb500c3510f626c212169ec498ddee1413c9736 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4441: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit


== Linux commits ==

bfb500c3510f drm/i915/audio: set minimum CD clock to twice the BCLK

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915/audio: set minimum CD clock to twice the BCLK

[Patchwork]

== Series Details ==

Series: drm/i915/audio: set minimum CD clock to twice the BCLK
URL   : https://patchwork.freedesktop.org/series/41888/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4066_full -> Patchwork_8735_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8735_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8735_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/41888/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8735_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_mocs_settings@mocs-rc6-bsd1:
      shard-kbl:          PASS -> SKIP

    igt@gem_mocs_settings@mocs-rc6-bsd2:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
      shard-hsw:          PASS -> FAIL (fdo#104873)

    igt@kms_flip@modeset-vs-vblank-race:
      shard-snb:          PASS -> FAIL (fdo#103060)

    igt@kms_flip@modeset-vs-vblank-race-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#103060)

    igt@kms_setmode@basic:
      shard-hsw:          PASS -> FAIL (fdo#99912)

    igt@perf@short-reads:
      shard-kbl:          PASS -> FAIL (fdo#103183)

    
    ==== Possible fixes ====

    igt@drv_suspend@forcewake:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
      shard-hsw:          FAIL (fdo#105767) -> PASS

    igt@kms_flip@2x-dpms-vs-vblank-race:
      shard-hsw:          FAIL (fdo#103060) -> PASS

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-hsw:          FAIL (fdo#102887) -> PASS

    igt@kms_flip@flip-vs-blocking-wf-vblank:
      shard-hsw:          FAIL (fdo#100368) -> PASS

    igt@kms_frontbuffer_tracking@fbc-farfromfence:
      shard-kbl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +2

    igt@kms_sysfs_edid_timing:
      shard-apl:          WARN (fdo#100047) -> PASS

    
  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103183 https://bugs.freedesktop.org/show_bug.cgi?id=103183
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (9 -> 4) ==

  Missing    (5): shard-glk8 shard-glk6 shard-glk7 shard-glk shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4066 -> Patchwork_8735

  CI_DRM_4066: e1fbca4821d0700551df233285a5c28db09fd0f6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4441: 83ba5b7d3bde48b383df41792fc9c955a5a23bdb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8735: bfb500c3510f626c212169ec498ddee1413c9736 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4441: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH] drm/i915/audio: set minimum CD clock to twice the BCLK

[Jani Nikula]

On Wed, 18 Apr 2018, Jani Nikula <jani.nikula@intel.com> wrote:
> From: Abhay Kumar <abhay.kumar@intel.com>
>
> In GLK when the device boots with only 1366x768 panel without audio, HDA
> codec doesn't come up. In this case, the CDCLK is less than twice the
> BCLK. Even though audio isn't being enabled, having a too low CDCLK
> leads to audio probe failing altogether.
>
> Require CDCLK to be at least twice the BLCK regardless of audio. This is
> a minimal fix to improve things. Unfortunately, this a) leads to too
> high CDCLK being used when audio is not used, and b) is still not enough
> to fix audio probe when no outputs are connected at probe time.
>
> The proper fix would be to increase CDCLK dynamically from the audio
> component hooks.
>
> v2:
>     - Address comment (Jani)
>     - New design approach
> v3: - Typo fix on top of v1
>
> v4 by Jani: rewrite commit message, add comment in code
>
> Cc: stable@vger.kernel.org
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com>
> Cc: Wenkai Du <wenkai.du@intel.com>
> Reviewed-by: Wenkai Du <wenkai.du@intel.com>
> Tested-by: Wenkai Du <wenkai.du@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Pushed to dinq with Ville's Ack on another thread.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/intel_cdclk.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index fc8b2c6e3508..32d24c69da3c 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2140,10 +2140,22 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>  		}
>  	}
>  
> -	/* According to BSpec, "The CD clock frequency must be at least twice
> +	/*
> +	 * According to BSpec, "The CD clock frequency must be at least twice
>  	 * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
> +	 *
> +	 * FIXME: Check the actual, not default, BCLK being used.
> +	 *
> +	 * FIXME: This does not depend on ->has_audio because the higher CDCLK
> +	 * is required for audio probe, also when there are no audio capable
> +	 * displays connected at probe time. This leads to unnecessarily high
> +	 * CDCLK when audio is not required.
> +	 *
> +	 * FIXME: This limit is only applied when there are displays connected
> +	 * at probe time. If we probe without displays, we'll still end up using
> +	 * the platform minimum CDCLK, failing audio probe.
>  	 */
> -	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
> +	if (INTEL_GEN(dev_priv) >= 9)
>  		min_cdclk = max(2 * 96000, min_cdclk);
>  
>  	/*

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH] drm/i915/audio: set minimum CD clock to twice the BCLK

[Jani Nikula]

On Wed, 18 Apr 2018, Jani Nikula <jani.nikula@intel.com> wrote:
> From: Abhay Kumar <abhay.kumar@intel.com>
>
> In GLK when the device boots with only 1366x768 panel without audio, HDA
> codec doesn't come up. In this case, the CDCLK is less than twice the
> BCLK. Even though audio isn't being enabled, having a too low CDCLK
> leads to audio probe failing altogether.
>
> Require CDCLK to be at least twice the BLCK regardless of audio. This is
> a minimal fix to improve things. Unfortunately, this a) leads to too
> high CDCLK being used when audio is not used, and b) is still not enough
> to fix audio probe when no outputs are connected at probe time.
>
> The proper fix would be to increase CDCLK dynamically from the audio
> component hooks.
>
> v2:
>     - Address comment (Jani)
>     - New design approach
> v3: - Typo fix on top of v1
>
> v4 by Jani: rewrite commit message, add comment in code
>
> Cc: stable@vger.kernel.org
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com>
> Cc: Wenkai Du <wenkai.du@intel.com>
> Reviewed-by: Wenkai Du <wenkai.du@intel.com>
> Tested-by: Wenkai Du <wenkai.du@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Pushed to dinq with Ville's Ack on another thread.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/intel_cdclk.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index fc8b2c6e3508..32d24c69da3c 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2140,10 +2140,22 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>  		}
>  	}
>  
> -	/* According to BSpec, "The CD clock frequency must be at least twice
> +	/*
> +	 * According to BSpec, "The CD clock frequency must be at least twice
>  	 * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
> +	 *
> +	 * FIXME: Check the actual, not default, BCLK being used.
> +	 *
> +	 * FIXME: This does not depend on ->has_audio because the higher CDCLK
> +	 * is required for audio probe, also when there are no audio capable
> +	 * displays connected at probe time. This leads to unnecessarily high
> +	 * CDCLK when audio is not required.
> +	 *
> +	 * FIXME: This limit is only applied when there are displays connected
> +	 * at probe time. If we probe without displays, we'll still end up using
> +	 * the platform minimum CDCLK, failing audio probe.
>  	 */
> -	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
> +	if (INTEL_GEN(dev_priv) >= 9)
>  		min_cdclk = max(2 * 96000, min_cdclk);
>  
>  	/*

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