[PATCH v2 0/2] DP audio fixes

[PATCH v2 0/2] DP audio fixes

[Dhinakaran Pandiyan]

Playing audio over DP MST or just enabling it during boot caused underruns
on BDW and SKL. The underruns are followed by displays cycling on-off.
Debugging the underruns led to the understanding this was not a MST
specific issue but specific to the HBR2 configuration. There are two
solutions depending on the platform:

a) BDW - Increase the cdclk to at least 432 MHz in DP HBR2, x4 configs.
b) SKL, BXT and KBL - Enable DP stall fix (increasing the cdclk works
too).

I have tested this only on SKL and it would be great if somebody else can
share the results for other platforms.

Dhinakaran Pandiyan (2):
  drm/i915/dp: Enable DP audio stall fix for gen9 platforms
  drm/i915/dp: BDW cdclk fix for DP audio

 drivers/gpu/drm/i915/i915_reg.h      |  5 +++++
 drivers/gpu/drm/i915/intel_ddi.c     | 38 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 4 files changed, 70 insertions(+), 3 deletions(-)

-- 
2.7.4

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

[PATCH v2 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Dhinakaran Pandiyan]

Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
let's set this bit right before enabling the audio codec. Playing audio
without setting this bit results in pipe FIFO underruns.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  5 +++++
 drivers/gpu/drm/i915/intel_ddi.c | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 3 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 00efaa1..76dac48 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6236,6 +6236,11 @@ enum {
 #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
 #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
 
+#define _CHICKEN_TRANS_A	0x420C0
+#define _CHICKEN_TRANS_B	0x420C4
+#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
+#define SPARE_13	(1<<13)
+
 /* WaCatErrorRejectionIssue */
 #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
 #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index fb18d69..84c91c1 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1858,6 +1858,38 @@ void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
 	I915_WRITE(FDI_RX_CTL(PIPE_A), val);
 }
 
+void gen9_enable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(pipe_config->base.crtc->dev);
+	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
+	uint32_t temp;
+
+	if (intel_crtc_has_dp_encoder(pipe_config) &&
+	    pipe_config->port_clock >= 54000) {
+
+		temp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
+		temp |= SPARE_13;
+		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), temp);
+	}
+}
+
+void gen9_disable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(pipe_config->base.crtc->dev);
+	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
+	uint32_t temp;
+
+	if (intel_crtc_has_dp_encoder(pipe_config) &&
+	    pipe_config->port_clock >= 54000) {
+
+		temp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
+		temp &= ~SPARE_13;
+		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), temp);
+	}
+}
+
 static void intel_enable_ddi(struct intel_encoder *intel_encoder,
 			     struct intel_crtc_state *pipe_config,
 			     struct drm_connector_state *conn_state)
@@ -1893,6 +1925,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
 	}
 
 	if (intel_crtc->config->has_audio) {
+		if (IS_GEN9(dev_priv))
+			gen9_enable_dp_audio_stall_fix(pipe_config);
+
 		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
 		intel_audio_codec_enable(intel_encoder);
 	}
@@ -1912,6 +1947,9 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
 	if (intel_crtc->config->has_audio) {
 		intel_audio_codec_disable(intel_encoder);
 		intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
+
+		if (IS_GEN9(dev_priv))
+			gen9_disable_dp_audio_stall_fix(old_crtc_state);
 	}
 
 	if (type == INTEL_OUTPUT_EDP) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4e90b07..ef02c62 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1189,6 +1189,8 @@ unsigned int intel_fb_align_height(struct drm_device *dev,
 				   uint64_t fb_format_modifier);
 u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
 			      uint64_t fb_modifier, uint32_t pixel_format);
+void gen9_enable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config);
+void gen9_disable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config);
 
 /* intel_audio.c */
 void intel_init_audio_hooks(struct drm_i915_private *dev_priv);
-- 
2.7.4

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

[PATCH v2 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Dhinakaran Pandiyan]

According to BSpec, cdclk has to be not less than 432 MHz with DP audio
enabled, port width x4, and link rate HBR2 (5.4 GHz)

Having a lower cdclk triggers pipe underruns, which then lead to displays
continuously cycling off and on. This is essential for DP MST audio as the
link is trained at HBR2 and 4 lanes by default.

v2: Restrict fix to BDW
    Retain the set cdclk across modesets (Ville)

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a94f7d1..8c59651 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10260,6 +10260,18 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
 	bxt_set_cdclk(to_i915(dev), req_cdclk);
 }
 
+static unsigned int bdw_dp_audio_cdclk(struct intel_crtc_state *crtc_state)
+{
+
+	if (intel_crtc_has_dp_encoder(crtc_state) &&
+	    crtc_state->has_audio &&
+	    crtc_state->port_clock >= 540000 &&
+	    crtc_state->lane_count == 4)
+		return 432000;
+
+	return 0;
+}
+
 /* compute the max rate for new configuration */
 static int ilk_max_pixel_rate(struct drm_atomic_state *state)
 {
@@ -10275,7 +10287,7 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
 	       sizeof(intel_state->min_pixclk));
 
 	for_each_crtc_in_state(state, crtc, cstate, i) {
-		int pixel_rate;
+		unsigned int pixel_rate;
 
 		crtc_state = to_intel_crtc_state(cstate);
 		if (!crtc_state->base.enable) {
@@ -10285,9 +10297,19 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
 
 		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
 
+		if (IS_BROADWELL(dev_priv)) {
 		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
-		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
-			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
+			if (crtc_state->ips_enabled)
+				pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
+
+		/* BSpec says "Do not use DisplayPort with CDCLK less than
+		 * 432 MHz, audio enabled, port width x4, and link rate
+		 * HBR2 (5.4 GHz), or else there may be audio corruption or
+		 * screen corruption."
+		 */
+			pixel_rate = max(pixel_rate,
+					 bdw_dp_audio_cdclk(crtc_state));
+		}
 
 		intel_state->min_pixclk[i] = pixel_rate;
 	}
-- 
2.7.4

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

✗ Fi.CI.BAT: warning for DP audio fixes

[Patchwork]

== Series Details ==

Series: DP audio fixes
URL   : https://patchwork.freedesktop.org/series/14314/
State : warning

== Summary ==

Series 14314v1 DP audio fixes
https://patchwork.freedesktop.org/api/1.0/series/14314/revisions/1/mbox/

Test drv_module_reload_basic:
                skip       -> PASS       (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-skl-6700hq)
Test gem_exec_suspend:
        Subgroup basic-s3:
                dmesg-warn -> PASS       (fi-skl-6700hq)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> SKIP       (fi-ivb-3770)
                pass       -> SKIP       (fi-ivb-3520m)
                pass       -> SKIP       (fi-snb-2600)
                pass       -> SKIP       (fi-hsw-4770r)
                pass       -> SKIP       (fi-snb-2520m)
                pass       -> SKIP       (fi-ilk-650)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-skl-6700hq)
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-skl-6700hq)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (fi-skl-6700hq)
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                fail       -> PASS       (fi-ilk-650)

fi-bdw-5557u     total:246  pass:231  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-n2820     total:246  pass:211  dwarn:0   dfail:0   fail:0   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:223  dwarn:0   dfail:0   fail:0   skip:23 
fi-ilk-650       total:246  pass:185  dwarn:0   dfail:0   fail:0   skip:61 
fi-ivb-3520m     total:246  pass:220  dwarn:0   dfail:0   fail:0   skip:26 
fi-ivb-3770      total:246  pass:220  dwarn:0   dfail:0   fail:0   skip:26 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:246  pass:222  dwarn:1   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:222  dwarn:1   dfail:0   fail:0   skip:23 
fi-skl-6770hq    total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:246  pass:209  dwarn:0   dfail:0   fail:0   skip:37 
fi-snb-2600      total:246  pass:208  dwarn:0   dfail:0   fail:0   skip:38 

Results at /Patchwork_2803/

194359e4a31ff988c7a290093820c5ef28d3752b drm-intel-nightly: 2016y-10m-24d-19h-42m-14s UTC integration manifest
538d5b1 drm/i915/dp: BDW cdclk fix for DP audio
04d3ec8 drm/i915/dp: Enable DP audio stall fix for gen9 platforms

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

Re: [PATCH v2 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Ville Syrjälä]

On Mon, Oct 24, 2016 at 09:18:37PM -0700, Dhinakaran Pandiyan wrote:
> According to BSpec, cdclk has to be not less than 432 MHz with DP audio
> enabled, port width x4, and link rate HBR2 (5.4 GHz)
> 
> Having a lower cdclk triggers pipe underruns, which then lead to displays
> continuously cycling off and on. This is essential for DP MST audio as the
> link is trained at HBR2 and 4 lanes by default.
> 
> v2: Restrict fix to BDW
>     Retain the set cdclk across modesets (Ville)
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a94f7d1..8c59651 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10260,6 +10260,18 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
>  	bxt_set_cdclk(to_i915(dev), req_cdclk);
>  }
>  
> +static unsigned int bdw_dp_audio_cdclk(struct intel_crtc_state *crtc_state)
> +{
> +

Useless blank line.

> +	if (intel_crtc_has_dp_encoder(crtc_state) &&
> +	    crtc_state->has_audio &&
> +	    crtc_state->port_clock >= 540000 &&
> +	    crtc_state->lane_count == 4)
> +		return 432000;
> +
> +	return 0;
> +}
> +
>  /* compute the max rate for new configuration */
>  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>  {
> @@ -10275,7 +10287,7 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>  	       sizeof(intel_state->min_pixclk));
>  
>  	for_each_crtc_in_state(state, crtc, cstate, i) {
> -		int pixel_rate;
> +		unsigned int pixel_rate;
>  
>  		crtc_state = to_intel_crtc_state(cstate);
>  		if (!crtc_state->base.enable) {
> @@ -10285,9 +10297,19 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>  
>  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
>  
> +		if (IS_BROADWELL(dev_priv)) {
>  		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> +			if (crtc_state->ips_enabled)
> +				pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> +
> +		/* BSpec says "Do not use DisplayPort with CDCLK less than
> +		 * 432 MHz, audio enabled, port width x4, and link rate
> +		 * HBR2 (5.4 GHz), or else there may be audio corruption or
> +		 * screen corruption."
> +		 */

Indentation of the comments is wrong.

> +			pixel_rate = max(pixel_rate,
> +					 bdw_dp_audio_cdclk(crtc_state));
> +		}
>  
>  		intel_state->min_pixclk[i] = pixel_rate;

Otherwise I suppose this ought to work.

So with the formatting stuff fixed this is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

The whole min_pixclk vs. pixel_rate vs. cdclk thing is a bit of a mess
though. So it could use a thorough cleaning to make it less confusing.
I'm tinking we might just want to start tracking a minimum acceptable
cdclk per pipe. Probably the main thing would be to pull in the
5%/10% guardband handling here for all platforms.

>  	}
> -- 
> 2.7.4

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

Re: [PATCH v2 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Jani Nikula]

On Tue, 25 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> let's set this bit right before enabling the audio codec. Playing audio
> without setting this bit results in pipe FIFO underruns.
>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

The fix itself looks legit, but please move the enable to beginning of
hsw_audio_codec_enable and disable to end of hsw_audio_codec_disable in
intel_audio.c. Seems to me you don't have to have separate functions for
this, just add it inline there.

Is there a W/A name for this?


BR,
Jani.


> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  5 +++++
>  drivers/gpu/drm/i915/intel_ddi.c | 38 ++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  3 files changed, 45 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 00efaa1..76dac48 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6236,6 +6236,11 @@ enum {
>  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
>  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
>  
> +#define _CHICKEN_TRANS_A	0x420C0
> +#define _CHICKEN_TRANS_B	0x420C4
> +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> +#define SPARE_13	(1<<13)
> +
>  /* WaCatErrorRejectionIssue */
>  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
>  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index fb18d69..84c91c1 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1858,6 +1858,38 @@ void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
>  	I915_WRITE(FDI_RX_CTL(PIPE_A), val);
>  }
>  
> +void gen9_enable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_i915_private *dev_priv =
> +		to_i915(pipe_config->base.crtc->dev);
> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> +	uint32_t temp;
> +
> +	if (intel_crtc_has_dp_encoder(pipe_config) &&
> +	    pipe_config->port_clock >= 54000) {
> +
> +		temp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		temp |= SPARE_13;
> +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), temp);
> +	}
> +}
> +
> +void gen9_disable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_i915_private *dev_priv =
> +		to_i915(pipe_config->base.crtc->dev);
> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> +	uint32_t temp;
> +
> +	if (intel_crtc_has_dp_encoder(pipe_config) &&
> +	    pipe_config->port_clock >= 54000) {
> +
> +		temp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		temp &= ~SPARE_13;
> +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), temp);
> +	}
> +}
> +
>  static void intel_enable_ddi(struct intel_encoder *intel_encoder,
>  			     struct intel_crtc_state *pipe_config,
>  			     struct drm_connector_state *conn_state)
> @@ -1893,6 +1925,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
>  	}
>  
>  	if (intel_crtc->config->has_audio) {
> +		if (IS_GEN9(dev_priv))
> +			gen9_enable_dp_audio_stall_fix(pipe_config);
> +
>  		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
>  		intel_audio_codec_enable(intel_encoder);
>  	}
> @@ -1912,6 +1947,9 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
>  	if (intel_crtc->config->has_audio) {
>  		intel_audio_codec_disable(intel_encoder);
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
> +
> +		if (IS_GEN9(dev_priv))
> +			gen9_disable_dp_audio_stall_fix(old_crtc_state);
>  	}
>  
>  	if (type == INTEL_OUTPUT_EDP) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4e90b07..ef02c62 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1189,6 +1189,8 @@ unsigned int intel_fb_align_height(struct drm_device *dev,
>  				   uint64_t fb_format_modifier);
>  u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
>  			      uint64_t fb_modifier, uint32_t pixel_format);
> +void gen9_enable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config);
> +void gen9_disable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config);
>  
>  /* intel_audio.c */
>  void intel_init_audio_hooks(struct drm_i915_private *dev_priv);

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

Re: [PATCH v2 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Jani Nikula]

On Tue, 25 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> According to BSpec, cdclk has to be not less than 432 MHz with DP audio
> enabled, port width x4, and link rate HBR2 (5.4 GHz)
>
> Having a lower cdclk triggers pipe underruns, which then lead to displays
> continuously cycling off and on. This is essential for DP MST audio as the
> link is trained at HBR2 and 4 lanes by default.
>
> v2: Restrict fix to BDW
>     Retain the set cdclk across modesets (Ville)

Cc: stable@vger.kernel.org

>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a94f7d1..8c59651 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10260,6 +10260,18 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
>  	bxt_set_cdclk(to_i915(dev), req_cdclk);
>  }
>  
> +static unsigned int bdw_dp_audio_cdclk(struct intel_crtc_state *crtc_state)
> +{
> +
> +	if (intel_crtc_has_dp_encoder(crtc_state) &&
> +	    crtc_state->has_audio &&
> +	    crtc_state->port_clock >= 540000 &&
> +	    crtc_state->lane_count == 4)
> +		return 432000;

Where does 432000 come from? 450000 or even (337500 + 1). See below.

> +
> +	return 0;
> +}
> +
>  /* compute the max rate for new configuration */
>  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>  {
> @@ -10275,7 +10287,7 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>  	       sizeof(intel_state->min_pixclk));
>  
>  	for_each_crtc_in_state(state, crtc, cstate, i) {
> -		int pixel_rate;
> +		unsigned int pixel_rate;
>  
>  		crtc_state = to_intel_crtc_state(cstate);
>  		if (!crtc_state->base.enable) {
> @@ -10285,9 +10297,19 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>  
>  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
>  
> +		if (IS_BROADWELL(dev_priv)) {
>  		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> +			if (crtc_state->ips_enabled)
> +				pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> +
> +		/* BSpec says "Do not use DisplayPort with CDCLK less than
> +		 * 432 MHz, audio enabled, port width x4, and link rate

For me the spec says "Do not use DisplayPort with CDCLK 337.5 MHz", not
"less than 432 MHz".

> +		 * HBR2 (5.4 GHz), or else there may be audio corruption or
> +		 * screen corruption."
> +		 */
> +			pixel_rate = max(pixel_rate,
> +					 bdw_dp_audio_cdclk(crtc_state));
> +		}

I'd add a new function

static int bwd_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
					  int pixel_rate)

and do both the IPS adjustment and the audio adjustment there, returning
the original pixel_rate if adjustment is not needed. Move the comments
there as well.

It would be called as

	if (IS_BROADWELL(dev_priv))
        	pixel_rate = bwd_adjust_min_pipe_pixel_rate(crtc_state, pixel_rate);

here.


BR,
Jani.

>  
>  		intel_state->min_pixclk[i] = pixel_rate;
>  	}

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

Re: [PATCH v2 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Jani Nikula]

On Tue, 25 Oct 2016, Jani Nikula <jani.nikula@intel.com> wrote:
> On Tue, 25 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
>> According to BSpec, cdclk has to be not less than 432 MHz with DP audio
>> enabled, port width x4, and link rate HBR2 (5.4 GHz)
>>
>> Having a lower cdclk triggers pipe underruns, which then lead to displays
>> continuously cycling off and on. This is essential for DP MST audio as the
>> link is trained at HBR2 and 4 lanes by default.
>>
>> v2: Restrict fix to BDW
>>     Retain the set cdclk across modesets (Ville)
>
> Cc: stable@vger.kernel.org
>
>>
>> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++++++++++---
>>  1 file changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index a94f7d1..8c59651 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -10260,6 +10260,18 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
>>  	bxt_set_cdclk(to_i915(dev), req_cdclk);
>>  }
>>  
>> +static unsigned int bdw_dp_audio_cdclk(struct intel_crtc_state *crtc_state)
>> +{
>> +
>> +	if (intel_crtc_has_dp_encoder(crtc_state) &&
>> +	    crtc_state->has_audio &&
>> +	    crtc_state->port_clock >= 540000 &&
>> +	    crtc_state->lane_count == 4)
>> +		return 432000;
>
> Where does 432000 come from? 450000 or even (337500 + 1). See below.
>
>> +
>> +	return 0;
>> +}
>> +
>>  /* compute the max rate for new configuration */
>>  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>>  {
>> @@ -10275,7 +10287,7 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>>  	       sizeof(intel_state->min_pixclk));
>>  
>>  	for_each_crtc_in_state(state, crtc, cstate, i) {
>> -		int pixel_rate;
>> +		unsigned int pixel_rate;
>>  
>>  		crtc_state = to_intel_crtc_state(cstate);
>>  		if (!crtc_state->base.enable) {
>> @@ -10285,9 +10297,19 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>>  
>>  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
>>  
>> +		if (IS_BROADWELL(dev_priv)) {
>>  		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
>> -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
>> -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
>> +			if (crtc_state->ips_enabled)
>> +				pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
>> +
>> +		/* BSpec says "Do not use DisplayPort with CDCLK less than
>> +		 * 432 MHz, audio enabled, port width x4, and link rate
>
> For me the spec says "Do not use DisplayPort with CDCLK 337.5 MHz", not
> "less than 432 MHz".

Right, so the spec for *Skylake* mentions 432 MHz. Now, we need this fix
for both Broadwell and Skylake, where's the Skylake part?

BR,
Jani.


>
>> +		 * HBR2 (5.4 GHz), or else there may be audio corruption or
>> +		 * screen corruption."
>> +		 */
>> +			pixel_rate = max(pixel_rate,
>> +					 bdw_dp_audio_cdclk(crtc_state));
>> +		}
>
> I'd add a new function
>
> static int bwd_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> 					  int pixel_rate)
>
> and do both the IPS adjustment and the audio adjustment there, returning
> the original pixel_rate if adjustment is not needed. Move the comments
> there as well.
>
> It would be called as
>
> 	if (IS_BROADWELL(dev_priv))
>         	pixel_rate = bwd_adjust_min_pipe_pixel_rate(crtc_state, pixel_rate);
>
> here.
>
>
> BR,
> Jani.
>
>>  
>>  		intel_state->min_pixclk[i] = pixel_rate;
>>  	}

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

Re: [PATCH v2 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Pandiyan, Dhinakaran]

On Tue, 2016-10-25 at 12:14 +0300, Jani Nikula wrote:
> On Tue, 25 Oct 2016, Jani Nikula <jani.nikula@intel.com> wrote:
> > On Tue, 25 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> >> According to BSpec, cdclk has to be not less than 432 MHz with DP audio
> >> enabled, port width x4, and link rate HBR2 (5.4 GHz)
> >>
> >> Having a lower cdclk triggers pipe underruns, which then lead to displays
> >> continuously cycling off and on. This is essential for DP MST audio as the
> >> link is trained at HBR2 and 4 lanes by default.
> >>
> >> v2: Restrict fix to BDW
> >>     Retain the set cdclk across modesets (Ville)
> >
> > Cc: stable@vger.kernel.org
> >
> >>
> >> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++++++++++---
> >>  1 file changed, 25 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index a94f7d1..8c59651 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -10260,6 +10260,18 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> >>  	bxt_set_cdclk(to_i915(dev), req_cdclk);
> >>  }
> >>  
> >> +static unsigned int bdw_dp_audio_cdclk(struct intel_crtc_state *crtc_state)
> >> +{
> >> +
> >> +	if (intel_crtc_has_dp_encoder(crtc_state) &&
> >> +	    crtc_state->has_audio &&
> >> +	    crtc_state->port_clock >= 540000 &&
> >> +	    crtc_state->lane_count == 4)
> >> +		return 432000;
> >
> > Where does 432000 come from? 450000 or even (337500 + 1). See below.
> >
> >> +
> >> +	return 0;
> >> +}
> >> +
> >>  /* compute the max rate for new configuration */
> >>  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >>  {
> >> @@ -10275,7 +10287,7 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >>  	       sizeof(intel_state->min_pixclk));
> >>  
> >>  	for_each_crtc_in_state(state, crtc, cstate, i) {
> >> -		int pixel_rate;
> >> +		unsigned int pixel_rate;
> >>  
> >>  		crtc_state = to_intel_crtc_state(cstate);
> >>  		if (!crtc_state->base.enable) {
> >> @@ -10285,9 +10297,19 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >>  
> >>  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
> >>  
> >> +		if (IS_BROADWELL(dev_priv)) {
> >>  		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> >> -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)432
> >> -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> >> +			if (crtc_state->ips_enabled)
> >> +				pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> >> +
> >> +		/* BSpec says "Do not use DisplayPort with CDCLK less than
> >> +		 * 432 MHz, audio enabled, port width x4, and link rate
> >
> > For me the spec says "Do not use DisplayPort with CDCLK 337.5 MHz", not
> > "less than 432 MHz".
> 
> Right, so the spec for *Skylake* mentions 432 MHz. Now, we need this fix
> for both Broadwell and Skylake, where's the Skylake part?
> 
> BR,
> Jani.
> 
> 

I believe you are looking at CDCLK_CTL that refers to pre-production SKL
SKU's. See the description for DP_TP_CTL instead. The information seems
to be scattered a bit


> >
> >> +		 * HBR2 (5.4 GHz), or else there may be audio corruption or
> >> +		 * screen corruption."
> >> +		 */
> >> +			pixel_rate = max(pixel_rate,
> >> +					 bdw_dp_audio_cdclk(crtc_state));
> >> +		}
> >
> > I'd add a new function
> >
> > static int bwd_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > 					  int pixel_rate)
> >
> > and do both the IPS adjustment and the audio adjustment there, returning
> > the original pixel_rate if adjustment is not needed. Move the comments
> > there as well.
> >
> > It would be called as
> >
> > 	if (IS_BROADWELL(dev_priv))
> >         	pixel_rate = bwd_adjust_min_pipe_pixel_rate(crtc_state, pixel_rate);
> >
> > here.
> >
> >
> > BR,
> > Jani.
> >

Will do.

-DK
> >>  
> >>  		intel_state->min_pixclk[i] = pixel_rate;
> >>  	}
> 

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

Re: [PATCH v2 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Pandiyan, Dhinakaran]

On Tue, 2016-10-25 at 11:46 +0300, Ville Syrjälä wrote:
> On Mon, Oct 24, 2016 at 09:18:37PM -0700, Dhinakaran Pandiyan wrote:
> > According to BSpec, cdclk has to be not less than 432 MHz with DP audio
> > enabled, port width x4, and link rate HBR2 (5.4 GHz)
> > 
> > Having a lower cdclk triggers pipe underruns, which then lead to displays
> > continuously cycling off and on. This is essential for DP MST audio as the
> > link is trained at HBR2 and 4 lanes by default.
> > 
> > v2: Restrict fix to BDW
> >     Retain the set cdclk across modesets (Ville)
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++++++++++---
> >  1 file changed, 25 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index a94f7d1..8c59651 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10260,6 +10260,18 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> >  	bxt_set_cdclk(to_i915(dev), req_cdclk);
> >  }
> >  
> > +static unsigned int bdw_dp_audio_cdclk(struct intel_crtc_state *crtc_state)
> > +{
> > +
> 
> Useless blank line.
> 
> > +	if (intel_crtc_has_dp_encoder(crtc_state) &&
> > +	    crtc_state->has_audio &&
> > +	    crtc_state->port_clock >= 540000 &&
> > +	    crtc_state->lane_count == 4)
> > +		return 432000;
> > +
> > +	return 0;
> > +}
> > +
> >  /* compute the max rate for new configuration */
> >  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >  {
> > @@ -10275,7 +10287,7 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >  	       sizeof(intel_state->min_pixclk));
> >  
> >  	for_each_crtc_in_state(state, crtc, cstate, i) {
> > -		int pixel_rate;
> > +		unsigned int pixel_rate;
> >  
> >  		crtc_state = to_intel_crtc_state(cstate);
> >  		if (!crtc_state->base.enable) {
> > @@ -10285,9 +10297,19 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >  
> >  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
> >  
> > +		if (IS_BROADWELL(dev_priv)) {
> >  		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> > -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> > -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> > +			if (crtc_state->ips_enabled)
> > +				pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> > +
> > +		/* BSpec says "Do not use DisplayPort with CDCLK less than
> > +		 * 432 MHz, audio enabled, port width x4, and link rate
> > +		 * HBR2 (5.4 GHz), or else there may be audio corruption or
> > +		 * screen corruption."
> > +		 */
> 
> Indentation of the comments is wrong.
> 
> > +			pixel_rate = max(pixel_rate,
> > +					 bdw_dp_audio_cdclk(crtc_state));
> > +		}
> >  
> >  		intel_state->min_pixclk[i] = pixel_rate;
> 
> Otherwise I suppose this ought to work.
> 
> So with the formatting stuff fixed this is
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The whole min_pixclk vs. pixel_rate vs. cdclk thing is a bit of a mess
> though. So it could use a thorough cleaning to make it less confusing.
> I'm tinking we might just want to start tracking a minimum acceptable
> cdclk per pipe. Probably the main thing would be to pull in the
> 5%/10% guardband handling here for all platforms.
> 

Yeah, definitely needs some work. As you wrote in the previous review,
we can get rid of tracking min_pixclk and track min_cdclk instead. I
could not find any other use for min_pixclk other than computing cdclk.
 
Thanks for the review.

-DK

> >  	}
> > -- 
> > 2.7.4
> 

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

Re: [PATCH v2 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Pandiyan, Dhinakaran]

On Tue, 2016-10-25 at 11:47 +0300, Jani Nikula wrote:
> On Tue, 25 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> > let's set this bit right before enabling the audio codec. Playing audio
> > without setting this bit results in pipe FIFO underruns.
> >
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> The fix itself looks legit, but please move the enable to beginning of
> hsw_audio_codec_enable and disable to end of hsw_audio_codec_disable in
> intel_audio.c. Seems to me you don't have to have separate functions for
> this, just add it inline there.
> 

I thought of it, but I didn't find any instances reading/writing of
non-audio registers in intel_audio.c. So, left these functions outside.



> Is there a W/A name for this?
> 
> 
> BR,
> Jani.
> 
> 

Not yet.

-DK

> > ---
> >  drivers/gpu/drm/i915/i915_reg.h  |  5 +++++
> >  drivers/gpu/drm/i915/intel_ddi.c | 38 ++++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h |  2 ++
> >  3 files changed, 45 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 00efaa1..76dac48 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6236,6 +6236,11 @@ enum {
> >  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
> >  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
> >  
> > +#define _CHICKEN_TRANS_A	0x420C0
> > +#define _CHICKEN_TRANS_B	0x420C4
> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> > +#define SPARE_13	(1<<13)
> > +
> >  /* WaCatErrorRejectionIssue */
> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index fb18d69..84c91c1 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1858,6 +1858,38 @@ void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
> >  	I915_WRITE(FDI_RX_CTL(PIPE_A), val);
> >  }
> >  
> > +void gen9_enable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config)
> > +{
> > +	struct drm_i915_private *dev_priv =
> > +		to_i915(pipe_config->base.crtc->dev);
> > +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> > +	uint32_t temp;
> > +
> > +	if (intel_crtc_has_dp_encoder(pipe_config) &&
> > +	    pipe_config->port_clock >= 54000) {
> > +
> > +		temp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +		temp |= SPARE_13;
> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), temp);
> > +	}
> > +}
> > +
> > +void gen9_disable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config)
> > +{
> > +	struct drm_i915_private *dev_priv =
> > +		to_i915(pipe_config->base.crtc->dev);
> > +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> > +	uint32_t temp;
> > +
> > +	if (intel_crtc_has_dp_encoder(pipe_config) &&
> > +	    pipe_config->port_clock >= 54000) {
> > +
> > +		temp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +		temp &= ~SPARE_13;
> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), temp);
> > +	}
> > +}
> > +
> >  static void intel_enable_ddi(struct intel_encoder *intel_encoder,
> >  			     struct intel_crtc_state *pipe_config,
> >  			     struct drm_connector_state *conn_state)
> > @@ -1893,6 +1925,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
> >  	}
> >  
> >  	if (intel_crtc->config->has_audio) {
> > +		if (IS_GEN9(dev_priv))
> > +			gen9_enable_dp_audio_stall_fix(pipe_config);
> > +
> >  		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> >  		intel_audio_codec_enable(intel_encoder);
> >  	}
> > @@ -1912,6 +1947,9 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
> >  	if (intel_crtc->config->has_audio) {
> >  		intel_audio_codec_disable(intel_encoder);
> >  		intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
> > +
> > +		if (IS_GEN9(dev_priv))
> > +			gen9_disable_dp_audio_stall_fix(old_crtc_state);
> >  	}
> >  
> >  	if (type == INTEL_OUTPUT_EDP) {
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 4e90b07..ef02c62 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1189,6 +1189,8 @@ unsigned int intel_fb_align_height(struct drm_device *dev,
> >  				   uint64_t fb_format_modifier);
> >  u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
> >  			      uint64_t fb_modifier, uint32_t pixel_format);
> > +void gen9_enable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config);
> > +void gen9_disable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config);
> >  
> >  /* intel_audio.c */
> >  void intel_init_audio_hooks(struct drm_i915_private *dev_priv);
> 

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

Re: [PATCH v2 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Pandiyan, Dhinakaran]

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

On Tue, 2016-10-25 at 18:19 +0000, Pandiyan, Dhinakaran wrote:
> On Tue, 2016-10-25 at 12:14 +0300, Jani Nikula wrote:
> > On Tue, 25 Oct 2016, Jani Nikula <jani.nikula@intel.com> wrote:
> > > On Tue, 25 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> > >> According to BSpec, cdclk has to be not less than 432 MHz with DP audio
> > >> enabled, port width x4, and link rate HBR2 (5.4 GHz)
> > >>
> > >> Having a lower cdclk triggers pipe underruns, which then lead to displays
> > >> continuously cycling off and on. This is essential for DP MST audio as the
> > >> link is trained at HBR2 and 4 lanes by default.
> > >>
> > >> v2: Restrict fix to BDW
> > >>     Retain the set cdclk across modesets (Ville)
> > >
> > > Cc: stable@vger.kernel.org
> > >
> > >>
> > >> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > >> ---
> > >>  drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++++++++++---
> > >>  1 file changed, 25 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > >> index a94f7d1..8c59651 100644
> > >> --- a/drivers/gpu/drm/i915/intel_display.c
> > >> +++ b/drivers/gpu/drm/i915/intel_display.c
> > >> @@ -10260,6 +10260,18 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> > >>  	bxt_set_cdclk(to_i915(dev), req_cdclk);
> > >>  }
> > >>  
> > >> +static unsigned int bdw_dp_audio_cdclk(struct intel_crtc_state *crtc_state)
> > >> +{
> > >> +
> > >> +	if (intel_crtc_has_dp_encoder(crtc_state) &&
> > >> +	    crtc_state->has_audio &&
> > >> +	    crtc_state->port_clock >= 540000 &&
> > >> +	    crtc_state->lane_count == 4)
> > >> +		return 432000;
> > >
> > > Where does 432000 come from? 450000 or even (337500 + 1). See below.
> > >
> > >> +
> > >> +	return 0;
> > >> +}
> > >> +
> > >>  /* compute the max rate for new configuration */
> > >>  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> > >>  {
> > >> @@ -10275,7 +10287,7 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> > >>  	       sizeof(intel_state->min_pixclk));
> > >>  
> > >>  	for_each_crtc_in_state(state, crtc, cstate, i) {
> > >> -		int pixel_rate;
> > >> +		unsigned int pixel_rate;
> > >>  
> > >>  		crtc_state = to_intel_crtc_state(cstate);
> > >>  		if (!crtc_state->base.enable) {
> > >> @@ -10285,9 +10297,19 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> > >>  
> > >>  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
> > >>  
> > >> +		if (IS_BROADWELL(dev_priv)) {
> > >>  		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> > >> -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)432
> > >> -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> > >> +			if (crtc_state->ips_enabled)
> > >> +				pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> > >> +
> > >> +		/* BSpec says "Do not use DisplayPort with CDCLK less than
> > >> +		 * 432 MHz, audio enabled, port width x4, and link rate
> > >
> > > For me the spec says "Do not use DisplayPort with CDCLK 337.5 MHz", not
> > > "less than 432 MHz".
> > 
> > Right, so the spec for *Skylake* mentions 432 MHz. Now, we need this fix
> > for both Broadwell and Skylake, where's the Skylake part?
> > 
> > BR,
> > Jani.
> > 
> > 
> 
> I believe you are looking at CDCLK_CTL that refers to pre-production SKL
> SKU's. See the description for DP_TP_CTL instead. The information seems
> to be scattered a bit
> 
> 
> > >
> > >> +		 * HBR2 (5.4 GHz), or else there may be audio corruption or
> > >> +		 * screen corruption."
> > >> +		 */
> > >> +			pixel_rate = max(pixel_rate,
> > >> +					 bdw_dp_audio_cdclk(crtc_state));
> > >> +		}
> > >
> > > I'd add a new function
> > >
> > > static int bwd_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > > 					  int pixel_rate)
> > >
> > > and do both the IPS adjustment and the audio adjustment there, returning
> > > the original pixel_rate if adjustment is not needed. Move the comments
> > > there as well.
> > >
> > > It would be called as
> > >
> > > 	if (IS_BROADWELL(dev_priv))
> > >         	pixel_rate = bwd_adjust_min_pipe_pixel_rate(crtc_state, pixel_rate);
> > >
> > > here.
> > >
> > >
> > > BR,
> > > Jani.
> > >
> 
> Will do.
> 
> -DK
> > >>  
> > >>  		intel_state->min_pixclk[i] = pixel_rate;
> > >>  	}
> > 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v3-0002-drm-i915-dp-BDW-cdclk-fix-for-DP-audio.patch --]
[-- Type: text/x-patch; name="v3-0002-drm-i915-dp-BDW-cdclk-fix-for-DP-audio.patch", Size: 2784 bytes --]

From 418d4a849252e8faa6d4e4403f5a99765f343833 Mon Sep 17 00:00:00 2001
From: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Date: Mon, 24 Oct 2016 16:24:20 -0700
Subject: [PATCH v3 2/2] drm/i915/dp: BDW cdclk fix for DP audio
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

According to BSpec, cdclk has to be not less than 432 MHz with DP audio
enabled, port width x4, and link rate HBR2 (5.4 GHz)

Having a lower cdclk triggers pipe underruns, which then lead to displays
continuously cycling off and on. This is essential for DP MST audio as the
link is trained at HBR2 and 4 lanes by default.

v3: Combine BDW pixel rate adjustments into a function (Jani)
v2: Restrict fix to BDW
    Retain the set cdclk across modesets (Ville)
Cc: stable@vger.kernel.org
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a94f7d1..efe46b4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10260,6 +10260,27 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
 	bxt_set_cdclk(to_i915(dev), req_cdclk);
 }
 
+static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
+					  int pixel_rate)
+{
+	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
+	if (crtc_state->ips_enabled)
+		pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
+
+	/* BSpec says "Do not use DisplayPort with CDCLK less than
+	 * 432 MHz, audio enabled, port width x4, and link rate
+	 * HBR2 (5.4 GHz), or else there may be audio corruption or
+	 * screen corruption."
+	 */
+	if (intel_crtc_has_dp_encoder(crtc_state) &&
+	    crtc_state->has_audio &&
+	    crtc_state->port_clock >= 540000 &&
+	    crtc_state->lane_count == 4)
+		pixel_rate = max(432000, pixel_rate);
+
+	return pixel_rate;
+}
+
 /* compute the max rate for new configuration */
 static int ilk_max_pixel_rate(struct drm_atomic_state *state)
 {
@@ -10285,9 +10306,9 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
 
 		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
 
-		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
-		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
-			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
+		if (IS_BROADWELL(dev_priv))
+			pixel_rate = bdw_adjust_min_pipe_pixel_rate(crtc_state,
+								    pixel_rate);
 
 		intel_state->min_pixclk[i] = pixel_rate;
 	}
-- 
2.7.4


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

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

[PATCH v3 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Dhinakaran Pandiyan]

According to BSpec, cdclk has to be not less than 432 MHz with DP audio
enabled, port width x4, and link rate HBR2 (5.4 GHz)

Having a lower cdclk triggers pipe underruns, which then lead to displays
continuously cycling off and on. This is essential for DP MST audio as the
link is trained at HBR2 and 4 lanes by default.

v3: Combine BDW pixel rate adjustments into a function (Jani)
v2: Restrict fix to BDW
    Retain the set cdclk across modesets (Ville)
Cc: stable@vger.kernel.org
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a94f7d1..efe46b4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10260,6 +10260,27 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
 	bxt_set_cdclk(to_i915(dev), req_cdclk);
 }
 
+static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
+					  int pixel_rate)
+{
+	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
+	if (crtc_state->ips_enabled)
+		pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
+
+	/* BSpec says "Do not use DisplayPort with CDCLK less than
+	 * 432 MHz, audio enabled, port width x4, and link rate
+	 * HBR2 (5.4 GHz), or else there may be audio corruption or
+	 * screen corruption."
+	 */
+	if (intel_crtc_has_dp_encoder(crtc_state) &&
+	    crtc_state->has_audio &&
+	    crtc_state->port_clock >= 540000 &&
+	    crtc_state->lane_count == 4)
+		pixel_rate = max(432000, pixel_rate);
+
+	return pixel_rate;
+}
+
 /* compute the max rate for new configuration */
 static int ilk_max_pixel_rate(struct drm_atomic_state *state)
 {
@@ -10285,9 +10306,9 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
 
 		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
 
-		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
-		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
-			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
+		if (IS_BROADWELL(dev_priv))
+			pixel_rate = bdw_adjust_min_pipe_pixel_rate(crtc_state,
+								    pixel_rate);
 
 		intel_state->min_pixclk[i] = pixel_rate;
 	}
-- 
2.7.4

[PATCH v3 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Dhinakaran Pandiyan]

Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
let's set this bit right before enabling the audio codec. Playing audio
without setting this bit results in pipe FIFO underruns.

This workaround is applicable only for audio sample rates up to 96kHz. For
frequencies above 96kHz, this is insufficient and cdclk should be increased
to at least 432 MHz, just like BDW. Since, the audio driver does not
support sample rates < 48 kHz, we are safe with this fix for now.

v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
    Fixed the port clock typo
    Added TODO comment
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  5 +++
 drivers/gpu/drm/i915/intel_audio.c | 62 +++++++++++++++++++++++++++++++++++---
 2 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 00efaa1..76dac48 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6236,6 +6236,11 @@ enum {
 #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
 #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
 
+#define _CHICKEN_TRANS_A	0x420C0
+#define _CHICKEN_TRANS_B	0x420C4
+#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
+#define SPARE_13	(1<<13)
+
 /* WaCatErrorRejectionIssue */
 #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
 #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 7093cfb..413dd50 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	struct intel_crtc_state *crtc_config =  intel_crtc->config;
+	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
 	enum pipe pipe = intel_crtc->pipe;
 	uint32_t tmp;
 
@@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
 
 	mutex_lock(&dev_priv->av_mutex);
 
+	/*Disable DP audio stall fix for HBR2*/
+	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
+	    crtc_config->port_clock >= 540000) {
+		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
+		tmp &= ~SPARE_13;
+		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
+	}
+
 	/* Disable timestamps */
 	tmp = I915_READ(HSW_AUD_CFG(pipe));
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
 	tmp |= AUD_CONFIG_N_PROG_ENABLE;
 	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
 	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
-	if (intel_crtc_has_dp_encoder(intel_crtc->config))
+	if (intel_crtc_has_dp_encoder(crtc_config))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
 
@@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
+	struct intel_crtc_state *crtc_config =  intel_crtc->config;
+	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
 	enum pipe pipe = intel_crtc->pipe;
 	enum port port = intel_encoder->port;
 	const uint8_t *eld = connector->eld;
@@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 
 	mutex_lock(&dev_priv->av_mutex);
 
+	/* Enable DP audio stall fix for HBR2
+	 *
+	 * TODO: This workaround is applicable only for audio sample rates up
+	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
+	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
+	 * the audio driver does not support sample rates < 48 kHz, we are safe
+	 * with this fix for now.
+	 */
+
+	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
+	    crtc_config->port_clock >= 540000) {
+		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
+		tmp |= SPARE_13;
+		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
+	}
+
 	/* Enable audio presence detect, invalidate ELD */
 	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
 	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
@@ -642,25 +670,49 @@ static int i915_audio_component_get_cdclk_freq(struct device *kdev)
 	return dev_priv->cdclk_freq;
 }
 
+/*
+ * get the intel_encoder according to the parameter port and pipe
+ * intel_encoder is saved by the index of pipe
+ * MST & (pipe >= 0): return the av_enc_map[pipe],
+ *   when port is matched
+ * MST & (pipe < 0): this is invalid
+ * Non-MST & (pipe >= 0): only pipe = 0 (the first device entry)
+ *   will get the right intel_encoder with port matched
+ * Non-MST & (pipe < 0): get the right intel_encoder with port matched
+ */
 static struct intel_encoder *get_saved_enc(struct drm_i915_private *dev_priv,
 					       int port, int pipe)
 {
+	struct intel_encoder *encoder;
 
 	if (WARN_ON(pipe >= I915_MAX_PIPES))
 		return NULL;
 
 	/* MST */
-	if (pipe >= 0)
-		return dev_priv->av_enc_map[pipe];
+	if (pipe >= 0) {
+		encoder = dev_priv->av_enc_map[pipe];
+		/*
+		 * when bootup, audio driver may not know it is
+		 * MST or not. So it will poll all the port & pipe
+		 * combinations
+		 */
+		if (encoder != NULL && encoder->port == port &&
+		    encoder->type == INTEL_OUTPUT_DP_MST)
+			return encoder;
+	}
 
 	/* Non-MST */
-	for_each_pipe(dev_priv, pipe) {
-		struct intel_encoder *encoder;
+	if (pipe > 0)
+		return NULL;
 
+	for_each_pipe(dev_priv, pipe) {
 		encoder = dev_priv->av_enc_map[pipe];
 		if (encoder == NULL)
 			continue;
 
+		if (encoder->type == INTEL_OUTPUT_DP_MST)
+			continue;
+
 		if (port == encoder->port)
 			return encoder;
 	}
-- 
2.7.4

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

✓ Fi.CI.BAT: success for DP audio fixes (rev4)

[Patchwork]

== Series Details ==

Series: DP audio fixes (rev4)
URL   : https://patchwork.freedesktop.org/series/14314/
State : success

== Summary ==

Series 14314v4 DP audio fixes
https://patchwork.freedesktop.org/api/1.0/series/14314/revisions/4/mbox/


fi-bdw-5557u     total:246  pass:231  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-n2820     total:246  pass:211  dwarn:0   dfail:0   fail:0   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:223  dwarn:0   dfail:0   fail:0   skip:23 
fi-ilk-650       total:246  pass:185  dwarn:0   dfail:0   fail:0   skip:61 
fi-ivb-3520m     total:246  pass:220  dwarn:0   dfail:0   fail:0   skip:26 
fi-ivb-3770      total:246  pass:220  dwarn:0   dfail:0   fail:0   skip:26 
fi-kbl-7200u     total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:246  pass:222  dwarn:1   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:222  dwarn:1   dfail:0   fail:0   skip:23 
fi-skl-6770hq    total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:246  pass:209  dwarn:0   dfail:0   fail:0   skip:37 
fi-snb-2600      total:246  pass:208  dwarn:0   dfail:0   fail:0   skip:38 

f746a2112fbb563743acc132304075706551d123 drm-intel-nightly: 2016y-10m-25d-20h-02m-34s UTC integration manifest
e90ef85 drm/i915/dp: BDW cdclk fix for DP audio
9eb8630 drm/i915/dp: Enable DP audio stall fix for gen9 platforms

Full results at https://intel-gfx-ci.01.org/CI/Patchwork_2818/

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2818/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Pandiyan, Dhinakaran]

Mixing up git rebase and reset was not a good idea, will send the
corrected patch. Please ignore this.

On Tue, 2016-10-25 at 16:42 -0700, Dhinakaran Pandiyan wrote:
> Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> let's set this bit right before enabling the audio codec. Playing audio
> without setting this bit results in pipe FIFO underruns.
> 
> This workaround is applicable only for audio sample rates up to 96kHz. For
> frequencies above 96kHz, this is insufficient and cdclk should be increased
> to at least 432 MHz, just like BDW. Since, the audio driver does not
> support sample rates < 48 kHz, we are safe with this fix for now.
> 
> v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
>     Fixed the port clock typo
>     Added TODO comment
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  5 +++
>  drivers/gpu/drm/i915/intel_audio.c | 62 +++++++++++++++++++++++++++++++++++---
>  2 files changed, 62 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 00efaa1..76dac48 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6236,6 +6236,11 @@ enum {
>  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
>  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
>  
> +#define _CHICKEN_TRANS_A	0x420C0
> +#define _CHICKEN_TRANS_B	0x420C4
> +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> +#define SPARE_13	(1<<13)
> +
>  /* WaCatErrorRejectionIssue */
>  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
>  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 7093cfb..413dd50 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>  	enum pipe pipe = intel_crtc->pipe;
>  	uint32_t tmp;
>  
> @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  
> +	/*Disable DP audio stall fix for HBR2*/
> +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> +	    crtc_config->port_clock >= 540000) {
> +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		tmp &= ~SPARE_13;
> +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> +	}
> +
>  	/* Disable timestamps */
>  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
>  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> +	if (intel_crtc_has_dp_encoder(crtc_config))
>  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>  
> @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>  	enum pipe pipe = intel_crtc->pipe;
>  	enum port port = intel_encoder->port;
>  	const uint8_t *eld = connector->eld;
> @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  
> +	/* Enable DP audio stall fix for HBR2
> +	 *
> +	 * TODO: This workaround is applicable only for audio sample rates up
> +	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
> +	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> +	 * the audio driver does not support sample rates < 48 kHz, we are safe
> +	 * with this fix for now.
> +	 */
> +
> +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> +	    crtc_config->port_clock >= 540000) {
> +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		tmp |= SPARE_13;
> +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> +	}
> +
>  	/* Enable audio presence detect, invalidate ELD */
>  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
>  	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
> @@ -642,25 +670,49 @@ static int i915_audio_component_get_cdclk_freq(struct device *kdev)
>  	return dev_priv->cdclk_freq;
>  }
>  
> +/*
> + * get the intel_encoder according to the parameter port and pipe
> + * intel_encoder is saved by the index of pipe
> + * MST & (pipe >= 0): return the av_enc_map[pipe],
> + *   when port is matched
> + * MST & (pipe < 0): this is invalid
> + * Non-MST & (pipe >= 0): only pipe = 0 (the first device entry)
> + *   will get the right intel_encoder with port matched
> + * Non-MST & (pipe < 0): get the right intel_encoder with port matched
> + */
>  static struct intel_encoder *get_saved_enc(struct drm_i915_private *dev_priv,
>  					       int port, int pipe)
>  {
> +	struct intel_encoder *encoder;
>  
>  	if (WARN_ON(pipe >= I915_MAX_PIPES))
>  		return NULL;
>  
>  	/* MST */
> -	if (pipe >= 0)
> -		return dev_priv->av_enc_map[pipe];
> +	if (pipe >= 0) {
> +		encoder = dev_priv->av_enc_map[pipe];
> +		/*
> +		 * when bootup, audio driver may not know it is
> +		 * MST or not. So it will poll all the port & pipe
> +		 * combinations
> +		 */
> +		if (encoder != NULL && encoder->port == port &&
> +		    encoder->type == INTEL_OUTPUT_DP_MST)
> +			return encoder;
> +	}
>  
>  	/* Non-MST */
> -	for_each_pipe(dev_priv, pipe) {
> -		struct intel_encoder *encoder;
> +	if (pipe > 0)
> +		return NULL;
>  
> +	for_each_pipe(dev_priv, pipe) {
>  		encoder = dev_priv->av_enc_map[pipe];
>  		if (encoder == NULL)
>  			continue;
>  
> +		if (encoder->type == INTEL_OUTPUT_DP_MST)
> +			continue;
> +
>  		if (port == encoder->port)
>  			return encoder;
>  	}

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

[PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Dhinakaran Pandiyan]

Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
let's set this bit right before enabling the audio codec. Playing audio
without setting this bit results in pipe FIFO underruns.

This workaround is applicable only for audio sample rates up to 96kHz. For
frequencies above 96kHz, this is insufficient and cdclk should be increased
to at least 432 MHz, just like BDW. Since, the audio driver does not
support sample rates > 48 kHz, we are safe with this fix for now.

v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
    Fixed the port clock typo
    Added TODO comment
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
 drivers/gpu/drm/i915/intel_audio.c | 30 +++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 00efaa1..76dac48 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6236,6 +6236,11 @@ enum {
 #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
 #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
 
+#define _CHICKEN_TRANS_A	0x420C0
+#define _CHICKEN_TRANS_B	0x420C4
+#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
+#define SPARE_13	(1<<13)
+
 /* WaCatErrorRejectionIssue */
 #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
 #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 7093cfb..894f11e 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	struct intel_crtc_state *crtc_config =  intel_crtc->config;
+	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
 	enum pipe pipe = intel_crtc->pipe;
 	uint32_t tmp;
 
@@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
 
 	mutex_lock(&dev_priv->av_mutex);
 
+	/*Disable DP audio stall fix for HBR2*/
+	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
+	    crtc_config->port_clock >= 540000) {
+		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
+		tmp &= ~SPARE_13;
+		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
+	}
+
 	/* Disable timestamps */
 	tmp = I915_READ(HSW_AUD_CFG(pipe));
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
 	tmp |= AUD_CONFIG_N_PROG_ENABLE;
 	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
 	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
-	if (intel_crtc_has_dp_encoder(intel_crtc->config))
+	if (intel_crtc_has_dp_encoder(crtc_config))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
 
@@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
+	struct intel_crtc_state *crtc_config =  intel_crtc->config;
+	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
 	enum pipe pipe = intel_crtc->pipe;
 	enum port port = intel_encoder->port;
 	const uint8_t *eld = connector->eld;
@@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 
 	mutex_lock(&dev_priv->av_mutex);
 
+	/* Enable DP audio stall fix for HBR2
+	 *
+	 * TODO: This workaround is applicable only for audio sample rates up
+	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
+	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
+	 * the audio driver does not support sample rates > 48 kHz, we are safe
+	 * with this fix for now.
+	 */
+
+	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
+	    crtc_config->port_clock >= 540000) {
+		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
+		tmp |= SPARE_13;
+		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
+	}
+
 	/* Enable audio presence detect, invalidate ELD */
 	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
 	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
-- 
2.7.4

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

Re: [PATCH v2 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Daniel Vetter]

On Tue, Oct 25, 2016 at 06:19:59PM +0000, Pandiyan, Dhinakaran wrote:
> On Tue, 2016-10-25 at 12:14 +0300, Jani Nikula wrote:
> > On Tue, 25 Oct 2016, Jani Nikula <jani.nikula@intel.com> wrote:
> > > On Tue, 25 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> > >> According to BSpec, cdclk has to be not less than 432 MHz with DP audio
> > >> enabled, port width x4, and link rate HBR2 (5.4 GHz)
> > >>
> > >> Having a lower cdclk triggers pipe underruns, which then lead to displays
> > >> continuously cycling off and on. This is essential for DP MST audio as the
> > >> link is trained at HBR2 and 4 lanes by default.
> > >>
> > >> v2: Restrict fix to BDW
> > >>     Retain the set cdclk across modesets (Ville)
> > >
> > > Cc: stable@vger.kernel.org
> > >
> > >>
> > >> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > >> ---
> > >>  drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++++++++++---
> > >>  1 file changed, 25 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > >> index a94f7d1..8c59651 100644
> > >> --- a/drivers/gpu/drm/i915/intel_display.c
> > >> +++ b/drivers/gpu/drm/i915/intel_display.c
> > >> @@ -10260,6 +10260,18 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> > >>  	bxt_set_cdclk(to_i915(dev), req_cdclk);
> > >>  }
> > >>  
> > >> +static unsigned int bdw_dp_audio_cdclk(struct intel_crtc_state *crtc_state)
> > >> +{
> > >> +
> > >> +	if (intel_crtc_has_dp_encoder(crtc_state) &&
> > >> +	    crtc_state->has_audio &&
> > >> +	    crtc_state->port_clock >= 540000 &&
> > >> +	    crtc_state->lane_count == 4)
> > >> +		return 432000;
> > >
> > > Where does 432000 come from? 450000 or even (337500 + 1). See below.
> > >
> > >> +
> > >> +	return 0;
> > >> +}
> > >> +
> > >>  /* compute the max rate for new configuration */
> > >>  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> > >>  {
> > >> @@ -10275,7 +10287,7 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> > >>  	       sizeof(intel_state->min_pixclk));
> > >>  
> > >>  	for_each_crtc_in_state(state, crtc, cstate, i) {
> > >> -		int pixel_rate;
> > >> +		unsigned int pixel_rate;
> > >>  
> > >>  		crtc_state = to_intel_crtc_state(cstate);
> > >>  		if (!crtc_state->base.enable) {
> > >> @@ -10285,9 +10297,19 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> > >>  
> > >>  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
> > >>  
> > >> +		if (IS_BROADWELL(dev_priv)) {
> > >>  		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> > >> -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)432
> > >> -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> > >> +			if (crtc_state->ips_enabled)
> > >> +				pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> > >> +
> > >> +		/* BSpec says "Do not use DisplayPort with CDCLK less than
> > >> +		 * 432 MHz, audio enabled, port width x4, and link rate
> > >
> > > For me the spec says "Do not use DisplayPort with CDCLK 337.5 MHz", not
> > > "less than 432 MHz".
> > 
> > Right, so the spec for *Skylake* mentions 432 MHz. Now, we need this fix
> > for both Broadwell and Skylake, where's the Skylake part?
> > 
> > BR,
> > Jani.
> > 
> > 
> 
> I believe you are looking at CDCLK_CTL that refers to pre-production SKL
> SKU's. See the description for DP_TP_CTL instead. The information seems
> to be scattered a bit

Please add a detailed citation (mentioning the Bspec hierarchy and all
that) and quote the workaround text in full in the commit message. Because
next month when someone else looks at this code and runs git blame they
will have no chance to find the Bspec notice.

This should be standard practice for any hw workarounds and things like
that which are done due to Bspec late in the platform cycle (personally
I'd say past beta milestone, which we are way past for skl ...).
-Daniel

> 
> 
> > >
> > >> +		 * HBR2 (5.4 GHz), or else there may be audio corruption or
> > >> +		 * screen corruption."
> > >> +		 */
> > >> +			pixel_rate = max(pixel_rate,
> > >> +					 bdw_dp_audio_cdclk(crtc_state));
> > >> +		}
> > >
> > > I'd add a new function
> > >
> > > static int bwd_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > > 					  int pixel_rate)
> > >
> > > and do both the IPS adjustment and the audio adjustment there, returning
> > > the original pixel_rate if adjustment is not needed. Move the comments
> > > there as well.
> > >
> > > It would be called as
> > >
> > > 	if (IS_BROADWELL(dev_priv))
> > >         	pixel_rate = bwd_adjust_min_pipe_pixel_rate(crtc_state, pixel_rate);
> > >
> > > here.
> > >
> > >
> > > BR,
> > > Jani.
> > >
> 
> Will do.
> 
> -DK
> > >>  
> > >>  		intel_state->min_pixclk[i] = pixel_rate;
> > >>  	}
> > 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Daniel Vetter]

On Mon, Oct 24, 2016 at 09:18:36PM -0700, Dhinakaran Pandiyan wrote:
> Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> let's set this bit right before enabling the audio codec. Playing audio
> without setting this bit results in pipe FIFO underruns.

Please insert a full Bspec citation here so that we can find where this is
documented in the future. Both where in Bspec it is, and then just quote
the full text with the explanation and put that into the commit message.

Otherwise there's no way we'll ever find this again in the future.
-Daniel

> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  5 +++++
>  drivers/gpu/drm/i915/intel_ddi.c | 38 ++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  3 files changed, 45 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 00efaa1..76dac48 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6236,6 +6236,11 @@ enum {
>  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
>  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
>  
> +#define _CHICKEN_TRANS_A	0x420C0
> +#define _CHICKEN_TRANS_B	0x420C4
> +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> +#define SPARE_13	(1<<13)
> +
>  /* WaCatErrorRejectionIssue */
>  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
>  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index fb18d69..84c91c1 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1858,6 +1858,38 @@ void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
>  	I915_WRITE(FDI_RX_CTL(PIPE_A), val);
>  }
>  
> +void gen9_enable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_i915_private *dev_priv =
> +		to_i915(pipe_config->base.crtc->dev);
> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> +	uint32_t temp;
> +
> +	if (intel_crtc_has_dp_encoder(pipe_config) &&
> +	    pipe_config->port_clock >= 54000) {
> +
> +		temp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		temp |= SPARE_13;
> +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), temp);
> +	}
> +}
> +
> +void gen9_disable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_i915_private *dev_priv =
> +		to_i915(pipe_config->base.crtc->dev);
> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> +	uint32_t temp;
> +
> +	if (intel_crtc_has_dp_encoder(pipe_config) &&
> +	    pipe_config->port_clock >= 54000) {
> +
> +		temp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		temp &= ~SPARE_13;
> +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), temp);
> +	}
> +}
> +
>  static void intel_enable_ddi(struct intel_encoder *intel_encoder,
>  			     struct intel_crtc_state *pipe_config,
>  			     struct drm_connector_state *conn_state)
> @@ -1893,6 +1925,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
>  	}
>  
>  	if (intel_crtc->config->has_audio) {
> +		if (IS_GEN9(dev_priv))
> +			gen9_enable_dp_audio_stall_fix(pipe_config);
> +
>  		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
>  		intel_audio_codec_enable(intel_encoder);
>  	}
> @@ -1912,6 +1947,9 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
>  	if (intel_crtc->config->has_audio) {
>  		intel_audio_codec_disable(intel_encoder);
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
> +
> +		if (IS_GEN9(dev_priv))
> +			gen9_disable_dp_audio_stall_fix(old_crtc_state);
>  	}
>  
>  	if (type == INTEL_OUTPUT_EDP) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4e90b07..ef02c62 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1189,6 +1189,8 @@ unsigned int intel_fb_align_height(struct drm_device *dev,
>  				   uint64_t fb_format_modifier);
>  u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
>  			      uint64_t fb_modifier, uint32_t pixel_format);
> +void gen9_enable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config);
> +void gen9_disable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config);
>  
>  /* intel_audio.c */
>  void intel_init_audio_hooks(struct drm_i915_private *dev_priv);
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Jani Nikula]

On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> According to BSpec, cdclk has to be not less than 432 MHz with DP audio
> enabled, port width x4, and link rate HBR2 (5.4 GHz)
>
> Having a lower cdclk triggers pipe underruns, which then lead to displays
> continuously cycling off and on. This is essential for DP MST audio as the
> link is trained at HBR2 and 4 lanes by default.
>
> v3: Combine BDW pixel rate adjustments into a function (Jani)
> v2: Restrict fix to BDW
>     Retain the set cdclk across modesets (Ville)
> Cc: stable@vger.kernel.org
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

We'll need this fix for Skylake too, don't we? Maybe Kabylake? Please
send a follow-up patch for Skylake with the changes mentioned inline
below.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a94f7d1..efe46b4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10260,6 +10260,27 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
>  	bxt_set_cdclk(to_i915(dev), req_cdclk);
>  }
>  
> +static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> +					  int pixel_rate)
> +{
> +	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> +	if (crtc_state->ips_enabled)

if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)

> +		pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> +
> +	/* BSpec says "Do not use DisplayPort with CDCLK less than
> +	 * 432 MHz, audio enabled, port width x4, and link rate
> +	 * HBR2 (5.4 GHz), or else there may be audio corruption or
> +	 * screen corruption."
> +	 */
> +	if (intel_crtc_has_dp_encoder(crtc_state) &&
> +	    crtc_state->has_audio &&
> +	    crtc_state->port_clock >= 540000 &&
> +	    crtc_state->lane_count == 4)
> +		pixel_rate = max(432000, pixel_rate);
> +
> +	return pixel_rate;
> +}
> +
>  /* compute the max rate for new configuration */
>  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>  {
> @@ -10285,9 +10306,9 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>  
>  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
>  
> -		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> +		if (IS_BROADWELL(dev_priv))

if (IS_BROADWELL(dev_priv) || IS_SKYLAKE(dev_priv))

> +			pixel_rate = bdw_adjust_min_pipe_pixel_rate(crtc_state,
> +								    pixel_rate);
>  
>  		intel_state->min_pixclk[i] = pixel_rate;
>  	}

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH v3 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Jani Nikula]

On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> According to BSpec, cdclk has to be not less than 432 MHz with DP audio
> enabled, port width x4, and link rate HBR2 (5.4 GHz)
>
> Having a lower cdclk triggers pipe underruns, which then lead to displays
> continuously cycling off and on. This is essential for DP MST audio as the
> link is trained at HBR2 and 4 lanes by default.
>
> v3: Combine BDW pixel rate adjustments into a function (Jani)
> v2: Restrict fix to BDW
>     Retain the set cdclk across modesets (Ville)
> Cc: stable@vger.kernel.org
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

We'll need this fix for Skylake too, don't we? Maybe Kabylake? Please
send a follow-up patch for Skylake with the changes mentioned inline
below.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a94f7d1..efe46b4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10260,6 +10260,27 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
>  	bxt_set_cdclk(to_i915(dev), req_cdclk);
>  }
>  
> +static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> +					  int pixel_rate)
> +{
> +	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> +	if (crtc_state->ips_enabled)

if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)

> +		pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> +
> +	/* BSpec says "Do not use DisplayPort with CDCLK less than
> +	 * 432 MHz, audio enabled, port width x4, and link rate
> +	 * HBR2 (5.4 GHz), or else there may be audio corruption or
> +	 * screen corruption."
> +	 */
> +	if (intel_crtc_has_dp_encoder(crtc_state) &&
> +	    crtc_state->has_audio &&
> +	    crtc_state->port_clock >= 540000 &&
> +	    crtc_state->lane_count == 4)
> +		pixel_rate = max(432000, pixel_rate);
> +
> +	return pixel_rate;
> +}
> +
>  /* compute the max rate for new configuration */
>  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>  {
> @@ -10285,9 +10306,9 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>  
>  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
>  
> -		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> +		if (IS_BROADWELL(dev_priv))

if (IS_BROADWELL(dev_priv) || IS_SKYLAKE(dev_priv))

> +			pixel_rate = bdw_adjust_min_pipe_pixel_rate(crtc_state,
> +								    pixel_rate);
>  
>  		intel_state->min_pixclk[i] = pixel_rate;
>  	}

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Jani Nikula]

On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> let's set this bit right before enabling the audio codec. Playing audio
> without setting this bit results in pipe FIFO underruns.
>
> This workaround is applicable only for audio sample rates up to 96kHz. For
> frequencies above 96kHz, this is insufficient and cdclk should be increased
> to at least 432 MHz, just like BDW. Since, the audio driver does not
> support sample rates > 48 kHz, we are safe with this fix for now.
>
> v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
>     Fixed the port clock typo
>     Added TODO comment
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
>  drivers/gpu/drm/i915/intel_audio.c | 30 +++++++++++++++++++++++++++++-
>  2 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 00efaa1..76dac48 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6236,6 +6236,11 @@ enum {
>  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
>  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
>  
> +#define _CHICKEN_TRANS_A	0x420C0
> +#define _CHICKEN_TRANS_B	0x420C4
> +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> +#define SPARE_13	(1<<13)
> +
>  /* WaCatErrorRejectionIssue */
>  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
>  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 7093cfb..894f11e 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>  	enum pipe pipe = intel_crtc->pipe;
>  	uint32_t tmp;
>  
> @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  
> +	/*Disable DP audio stall fix for HBR2*/

Nitpick, spaces after /* and before */.

> +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> +	    crtc_config->port_clock >= 540000) {
> +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		tmp &= ~SPARE_13;
> +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> +	}

Hmm. Why don't we just do this unconditionally?

> +
>  	/* Disable timestamps */
>  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
>  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> +	if (intel_crtc_has_dp_encoder(crtc_config))
>  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>  
> @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>  	enum pipe pipe = intel_crtc->pipe;
>  	enum port port = intel_encoder->port;
>  	const uint8_t *eld = connector->eld;
> @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  
> +	/* Enable DP audio stall fix for HBR2
> +	 *
> +	 * TODO: This workaround is applicable only for audio sample rates up
> +	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
> +	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> +	 * the audio driver does not support sample rates > 48 kHz, we are safe
> +	 * with this fix for now.
> +	 */

Is this TODO required if you already have that check in patch 2/2?

> +
> +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> +	    crtc_config->port_clock >= 540000) {
> +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		tmp |= SPARE_13;
> +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> +	}
> +
>  	/* Enable audio presence detect, invalidate ELD */
>  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
>  	tmp |= AUDIO_OUTPUT_ENABLE(pipe);

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

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Ville Syrjälä]

On Tue, Oct 25, 2016 at 07:37:36PM -0700, Dhinakaran Pandiyan wrote:
> Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> let's set this bit right before enabling the audio codec. Playing audio
> without setting this bit results in pipe FIFO underruns.
> 
> This workaround is applicable only for audio sample rates up to 96kHz. For
> frequencies above 96kHz, this is insufficient and cdclk should be increased
> to at least 432 MHz, just like BDW. Since, the audio driver does not
> support sample rates > 48 kHz, we are safe with this fix for now.
> 
> v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
>     Fixed the port clock typo
>     Added TODO comment
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
>  drivers/gpu/drm/i915/intel_audio.c | 30 +++++++++++++++++++++++++++++-
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 00efaa1..76dac48 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6236,6 +6236,11 @@ enum {
>  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
>  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
>  
> +#define _CHICKEN_TRANS_A	0x420C0
> +#define _CHICKEN_TRANS_B	0x420C4
> +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> +#define SPARE_13	(1<<13)
> +
>  /* WaCatErrorRejectionIssue */
>  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
>  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 7093cfb..894f11e 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>  	enum pipe pipe = intel_crtc->pipe;
>  	uint32_t tmp;
>  
> @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  
> +	/*Disable DP audio stall fix for HBR2*/
> +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> +	    crtc_config->port_clock >= 540000) {
> +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		tmp &= ~SPARE_13;
> +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> +	}
> +
>  	/* Disable timestamps */
>  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
>  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> +	if (intel_crtc_has_dp_encoder(crtc_config))
>  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>  
> @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>  	enum pipe pipe = intel_crtc->pipe;
>  	enum port port = intel_encoder->port;
>  	const uint8_t *eld = connector->eld;
> @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  
> +	/* Enable DP audio stall fix for HBR2
> +	 *
> +	 * TODO: This workaround is applicable only for audio sample rates up
> +	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
> +	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> +	 * the audio driver does not support sample rates > 48 kHz, we are safe
> +	 * with this fix for now.

Where in the sound driver is this supposed 96kHz limit? I see a lot of
stuff for >96kHz in the code at least.

> +	 */
> +
> +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> +	    crtc_config->port_clock >= 540000) {
> +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		tmp |= SPARE_13;
> +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> +	}
> +
>  	/* Enable audio presence detect, invalidate ELD */
>  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
>  	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
> -- 
> 2.7.4

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

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Pandiyan, Dhinakaran]

On Wed, 2016-10-26 at 11:57 +0300, Jani Nikula wrote:
> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> > let's set this bit right before enabling the audio codec. Playing audio
> > without setting this bit results in pipe FIFO underruns.
> >
> > This workaround is applicable only for audio sample rates up to 96kHz. For
> > frequencies above 96kHz, this is insufficient and cdclk should be increased
> > to at least 432 MHz, just like BDW. Since, the audio driver does not
> > support sample rates > 48 kHz, we are safe with this fix for now.
> >
> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> >     Fixed the port clock typo
> >     Added TODO comment
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
> >  drivers/gpu/drm/i915/intel_audio.c | 30 +++++++++++++++++++++++++++++-
> >  2 files changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 00efaa1..76dac48 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6236,6 +6236,11 @@ enum {
> >  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
> >  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
> >  
> > +#define _CHICKEN_TRANS_A	0x420C0
> > +#define _CHICKEN_TRANS_B	0x420C4
> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> > +#define SPARE_13	(1<<13)
> > +
> >  /* WaCatErrorRejectionIssue */
> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> > index 7093cfb..894f11e 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> >  	enum pipe pipe = intel_crtc->pipe;
> >  	uint32_t tmp;
> >  
> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
> >  
> >  	mutex_lock(&dev_priv->av_mutex);
> >  
> > +	/*Disable DP audio stall fix for HBR2*/
> 
> Nitpick, spaces after /* and before */.
> 
> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > +	    crtc_config->port_clock >= 540000) {
> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +		tmp &= ~SPARE_13;
> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > +	}
> 
> Hmm. Why don't we just do this unconditionally?
> 

That bit is disabled by default, so avoiding  two MMIO ops. that are not
required.

> > +
> >  	/* Disable timestamps */
> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> >  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
> >  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> >  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> > -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> > +	if (intel_crtc_has_dp_encoder(crtc_config))
> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >  
> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> >  	enum pipe pipe = intel_crtc->pipe;
> >  	enum port port = intel_encoder->port;
> >  	const uint8_t *eld = connector->eld;
> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> >  
> >  	mutex_lock(&dev_priv->av_mutex);
> >  
> > +	/* Enable DP audio stall fix for HBR2
> > +	 *
> > +	 * TODO: This workaround is applicable only for audio sample rates up
> > +	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
> > +	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> > +	 * the audio driver does not support sample rates > 48 kHz, we are safe
> > +	 * with this fix for now.
> > +	 */
> 
> Is this TODO required if you already have that check in patch 2/2?
> 

In fact, this patch itself is not required if we are increasing the
cdclk to 432 MHz for gen9 platforms (like patch 2/2). Having this
workaround gives us the option running the pipeline at a lower cdclk
frequency i.e., 337.5 MHz, which I believe should be better in terms of
power.


> > +
> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > +	    crtc_config->port_clock >= 540000) {
> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +		tmp |= SPARE_13;
> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > +	}
> > +
> >  	/* Enable audio presence detect, invalidate ELD */
> >  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> >  	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
> 

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

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Pandiyan, Dhinakaran]

On Wed, 2016-10-26 at 12:11 +0300, Ville Syrjälä wrote:
> On Tue, Oct 25, 2016 at 07:37:36PM -0700, Dhinakaran Pandiyan wrote:
> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> > let's set this bit right before enabling the audio codec. Playing audio
> > without setting this bit results in pipe FIFO underruns.
> > 
> > This workaround is applicable only for audio sample rates up to 96kHz. For
> > frequencies above 96kHz, this is insufficient and cdclk should be increased
> > to at least 432 MHz, just like BDW. Since, the audio driver does not
> > support sample rates > 48 kHz, we are safe with this fix for now.
> > 
> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> >     Fixed the port clock typo
> >     Added TODO comment
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
> >  drivers/gpu/drm/i915/intel_audio.c | 30 +++++++++++++++++++++++++++++-
> >  2 files changed, 34 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 00efaa1..76dac48 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6236,6 +6236,11 @@ enum {
> >  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
> >  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
> >  
> > +#define _CHICKEN_TRANS_A	0x420C0
> > +#define _CHICKEN_TRANS_B	0x420C4
> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> > +#define SPARE_13	(1<<13)
> > +
> >  /* WaCatErrorRejectionIssue */
> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> > index 7093cfb..894f11e 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> >  	enum pipe pipe = intel_crtc->pipe;
> >  	uint32_t tmp;
> >  
> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
> >  
> >  	mutex_lock(&dev_priv->av_mutex);
> >  
> > +	/*Disable DP audio stall fix for HBR2*/
> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > +	    crtc_config->port_clock >= 540000) {
> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +		tmp &= ~SPARE_13;
> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > +	}
> > +
> >  	/* Disable timestamps */
> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> >  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
> >  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> >  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> > -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> > +	if (intel_crtc_has_dp_encoder(crtc_config))
> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >  
> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> >  	enum pipe pipe = intel_crtc->pipe;
> >  	enum port port = intel_encoder->port;
> >  	const uint8_t *eld = connector->eld;
> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> >  
> >  	mutex_lock(&dev_priv->av_mutex);
> >  
> > +	/* Enable DP audio stall fix for HBR2
> > +	 *
> > +	 * TODO: This workaround is applicable only for audio sample rates up
> > +	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
> > +	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> > +	 * the audio driver does not support sample rates > 48 kHz, we are safe
> > +	 * with this fix for now.
> 
> Where in the sound driver is this supposed 96kHz limit? I see a lot of
> stuff for >96kHz in the code at least.
> 

Libin/Jeeja can you help me here?


> > +	 */
> > +
> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > +	    crtc_config->port_clock >= 540000) {
> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +		tmp |= SPARE_13;
> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > +	}
> > +
> >  	/* Enable audio presence detect, invalidate ELD */
> >  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> >  	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
> > -- 
> > 2.7.4
> 

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

Re: [PATCH v3 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Pandiyan, Dhinakaran]

On Wed, 2016-10-26 at 11:54 +0300, Jani Nikula wrote:
> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> > According to BSpec, cdclk has to be not less than 432 MHz with DP audio
> > enabled, port width x4, and link rate HBR2 (5.4 GHz)
> >
> > Having a lower cdclk triggers pipe underruns, which then lead to displays
> > continuously cycling off and on. This is essential for DP MST audio as the
> > link is trained at HBR2 and 4 lanes by default.
> >
> > v3: Combine BDW pixel rate adjustments into a function (Jani)
> > v2: Restrict fix to BDW
> >     Retain the set cdclk across modesets (Ville)
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> We'll need this fix for Skylake too, don't we? Maybe Kabylake? Please
> send a follow-up patch for Skylake with the changes mentioned inline
> below.
> 
> BR,
> Jani.
> 
> 

Patch 1/2 should take care of gen9 platforms when audio sampling rates
are <= 96 kHz. I am still not sure how to increase increase cdclk when
the sampling rates > 96kHZ , that is why I have added the TODO in Patch
1/2.

> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++++++---
> >  1 file changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index a94f7d1..efe46b4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10260,6 +10260,27 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> >  	bxt_set_cdclk(to_i915(dev), req_cdclk);
> >  }
> >  
> > +static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > +					  int pixel_rate)
> > +{
> > +	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> > +	if (crtc_state->ips_enabled)
> 
> if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> 
> > +		pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> > +
> > +	/* BSpec says "Do not use DisplayPort with CDCLK less than
> > +	 * 432 MHz, audio enabled, port width x4, and link rate
> > +	 * HBR2 (5.4 GHz), or else there may be audio corruption or
> > +	 * screen corruption."
> > +	 */
> > +	if (intel_crtc_has_dp_encoder(crtc_state) &&
> > +	    crtc_state->has_audio &&
> > +	    crtc_state->port_clock >= 540000 &&
> > +	    crtc_state->lane_count == 4)
> > +		pixel_rate = max(432000, pixel_rate);
> > +
> > +	return pixel_rate;
> > +}
> > +
> >  /* compute the max rate for new configuration */
> >  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >  {
> > @@ -10285,9 +10306,9 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >  
> >  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
> >  
> > -		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> > -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> > -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> > +		if (IS_BROADWELL(dev_priv))
> 
> if (IS_BROADWELL(dev_priv) || IS_SKYLAKE(dev_priv))
> 
> > +			pixel_rate = bdw_adjust_min_pipe_pixel_rate(crtc_state,
> > +								    pixel_rate);
> >  
> >  		intel_state->min_pixclk[i] = pixel_rate;
> >  	}
> 

Re: [PATCH v3 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Pandiyan, Dhinakaran]

On Wed, 2016-10-26 at 11:54 +0300, Jani Nikula wrote:
> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> > According to BSpec, cdclk has to be not less than 432 MHz with DP audio
> > enabled, port width x4, and link rate HBR2 (5.4 GHz)
> >
> > Having a lower cdclk triggers pipe underruns, which then lead to displays
> > continuously cycling off and on. This is essential for DP MST audio as the
> > link is trained at HBR2 and 4 lanes by default.
> >
> > v3: Combine BDW pixel rate adjustments into a function (Jani)
> > v2: Restrict fix to BDW
> >     Retain the set cdclk across modesets (Ville)
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> We'll need this fix for Skylake too, don't we? Maybe Kabylake? Please
> send a follow-up patch for Skylake with the changes mentioned inline
> below.
> 
> BR,
> Jani.
> 
> 

Patch 1/2 should take care of gen9 platforms when audio sampling rates
are <= 96 kHz. I am still not sure how to increase increase cdclk when
the sampling rates > 96kHZ , that is why I have added the TODO in Patch
1/2.

> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++++++---
> >  1 file changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index a94f7d1..efe46b4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10260,6 +10260,27 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> >  	bxt_set_cdclk(to_i915(dev), req_cdclk);
> >  }
> >  
> > +static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > +					  int pixel_rate)
> > +{
> > +	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> > +	if (crtc_state->ips_enabled)
> 
> if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> 
> > +		pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> > +
> > +	/* BSpec says "Do not use DisplayPort with CDCLK less than
> > +	 * 432 MHz, audio enabled, port width x4, and link rate
> > +	 * HBR2 (5.4 GHz), or else there may be audio corruption or
> > +	 * screen corruption."
> > +	 */
> > +	if (intel_crtc_has_dp_encoder(crtc_state) &&
> > +	    crtc_state->has_audio &&
> > +	    crtc_state->port_clock >= 540000 &&
> > +	    crtc_state->lane_count == 4)
> > +		pixel_rate = max(432000, pixel_rate);
> > +
> > +	return pixel_rate;
> > +}
> > +
> >  /* compute the max rate for new configuration */
> >  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >  {
> > @@ -10285,9 +10306,9 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >  
> >  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
> >  
> > -		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> > -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> > -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> > +		if (IS_BROADWELL(dev_priv))
> 
> if (IS_BROADWELL(dev_priv) || IS_SKYLAKE(dev_priv))
> 
> > +			pixel_rate = bdw_adjust_min_pipe_pixel_rate(crtc_state,
> > +								    pixel_rate);
> >  
> >  		intel_state->min_pixclk[i] = pixel_rate;
> >  	}
> 

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

Re: [PATCH v2 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Pandiyan, Dhinakaran]

On Wed, 2016-10-26 at 08:37 +0200, Daniel Vetter wrote:
> On Mon, Oct 24, 2016 at 09:18:36PM -0700, Dhinakaran Pandiyan wrote:
> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> > let's set this bit right before enabling the audio codec. Playing audio
> > without setting this bit results in pipe FIFO underruns.
> 
> Please insert a full Bspec citation here so that we can find where this is
> documented in the future. Both where in Bspec it is, and then just quote
> the full text with the explanation and put that into the commit message.
> 
> Otherwise there's no way we'll ever find this again in the future.
> -Daniel
> 
This particular update has not landed in BSpec yet. I will update the
commit message and re-send the patches after they are reviewed.

> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h  |  5 +++++
> >  drivers/gpu/drm/i915/intel_ddi.c | 38 ++++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h |  2 ++
> >  3 files changed, 45 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 00efaa1..76dac48 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6236,6 +6236,11 @@ enum {
> >  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
> >  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
> >  
> > +#define _CHICKEN_TRANS_A	0x420C0
> > +#define _CHICKEN_TRANS_B	0x420C4
> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> > +#define SPARE_13	(1<<13)
> > +
> >  /* WaCatErrorRejectionIssue */
> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index fb18d69..84c91c1 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1858,6 +1858,38 @@ void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
> >  	I915_WRITE(FDI_RX_CTL(PIPE_A), val);
> >  }
> >  
> > +void gen9_enable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config)
> > +{
> > +	struct drm_i915_private *dev_priv =
> > +		to_i915(pipe_config->base.crtc->dev);
> > +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> > +	uint32_t temp;
> > +
> > +	if (intel_crtc_has_dp_encoder(pipe_config) &&
> > +	    pipe_config->port_clock >= 54000) {
> > +
> > +		temp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +		temp |= SPARE_13;
> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), temp);
> > +	}
> > +}
> > +
> > +void gen9_disable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config)
> > +{
> > +	struct drm_i915_private *dev_priv =
> > +		to_i915(pipe_config->base.crtc->dev);
> > +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> > +	uint32_t temp;
> > +
> > +	if (intel_crtc_has_dp_encoder(pipe_config) &&
> > +	    pipe_config->port_clock >= 54000) {
> > +
> > +		temp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +		temp &= ~SPARE_13;
> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), temp);
> > +	}
> > +}
> > +
> >  static void intel_enable_ddi(struct intel_encoder *intel_encoder,
> >  			     struct intel_crtc_state *pipe_config,
> >  			     struct drm_connector_state *conn_state)
> > @@ -1893,6 +1925,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
> >  	}
> >  
> >  	if (intel_crtc->config->has_audio) {
> > +		if (IS_GEN9(dev_priv))
> > +			gen9_enable_dp_audio_stall_fix(pipe_config);
> > +
> >  		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> >  		intel_audio_codec_enable(intel_encoder);
> >  	}
> > @@ -1912,6 +1947,9 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
> >  	if (intel_crtc->config->has_audio) {
> >  		intel_audio_codec_disable(intel_encoder);
> >  		intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
> > +
> > +		if (IS_GEN9(dev_priv))
> > +			gen9_disable_dp_audio_stall_fix(old_crtc_state);
> >  	}
> >  
> >  	if (type == INTEL_OUTPUT_EDP) {
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 4e90b07..ef02c62 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1189,6 +1189,8 @@ unsigned int intel_fb_align_height(struct drm_device *dev,
> >  				   uint64_t fb_format_modifier);
> >  u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
> >  			      uint64_t fb_modifier, uint32_t pixel_format);
> > +void gen9_enable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config);
> > +void gen9_disable_dp_audio_stall_fix(struct intel_crtc_state *pipe_config);
> >  
> >  /* intel_audio.c */
> >  void intel_init_audio_hooks(struct drm_i915_private *dev_priv);
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

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

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Jani Nikula]

On Wed, 26 Oct 2016, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote:
> On Wed, 2016-10-26 at 11:57 +0300, Jani Nikula wrote:
>> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
>> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
>> > let's set this bit right before enabling the audio codec. Playing audio
>> > without setting this bit results in pipe FIFO underruns.
>> >
>> > This workaround is applicable only for audio sample rates up to 96kHz. For
>> > frequencies above 96kHz, this is insufficient and cdclk should be increased
>> > to at least 432 MHz, just like BDW. Since, the audio driver does not
>> > support sample rates > 48 kHz, we are safe with this fix for now.
>> >
>> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
>> >     Fixed the port clock typo
>> >     Added TODO comment
>> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
>> >  drivers/gpu/drm/i915/intel_audio.c | 30 +++++++++++++++++++++++++++++-
>> >  2 files changed, 34 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> > index 00efaa1..76dac48 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -6236,6 +6236,11 @@ enum {
>> >  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
>> >  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
>> >  
>> > +#define _CHICKEN_TRANS_A	0x420C0
>> > +#define _CHICKEN_TRANS_B	0x420C4
>> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
>> > +#define SPARE_13	(1<<13)
>> > +
>> >  /* WaCatErrorRejectionIssue */
>> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
>> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>> > index 7093cfb..894f11e 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
>> >  {
>> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> >  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
>> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>> >  	enum pipe pipe = intel_crtc->pipe;
>> >  	uint32_t tmp;
>> >  
>> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
>> >  
>> >  	mutex_lock(&dev_priv->av_mutex);
>> >  
>> > +	/*Disable DP audio stall fix for HBR2*/
>> 
>> Nitpick, spaces after /* and before */.
>> 
>> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
>> > +	    crtc_config->port_clock >= 540000) {
>> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
>> > +		tmp &= ~SPARE_13;
>> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
>> > +	}
>> 
>> Hmm. Why don't we just do this unconditionally?
>> 
>
> That bit is disabled by default, so avoiding  two MMIO ops. that are not
> required.

That makes no difference on the modeset path.

>
>> > +
>> >  	/* Disable timestamps */
>> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>> >  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
>> >  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>> >  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
>> > -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
>> > +	if (intel_crtc_has_dp_encoder(crtc_config))
>> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> >  
>> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>> >  {
>> >  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>> >  	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
>> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
>> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>> >  	enum pipe pipe = intel_crtc->pipe;
>> >  	enum port port = intel_encoder->port;
>> >  	const uint8_t *eld = connector->eld;
>> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>> >  
>> >  	mutex_lock(&dev_priv->av_mutex);
>> >  
>> > +	/* Enable DP audio stall fix for HBR2
>> > +	 *
>> > +	 * TODO: This workaround is applicable only for audio sample rates up
>> > +	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
>> > +	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
>> > +	 * the audio driver does not support sample rates > 48 kHz, we are safe
>> > +	 * with this fix for now.
>> > +	 */
>> 
>> Is this TODO required if you already have that check in patch 2/2?
>> 
>
> In fact, this patch itself is not required if we are increasing the
> cdclk to 432 MHz for gen9 platforms (like patch 2/2). Having this
> workaround gives us the option running the pipeline at a lower cdclk
> frequency i.e., 337.5 MHz, which I believe should be better in terms of
> power.

The conditions for requiring higher cdclk are different in patch 2/2,
right? That one also requires 4 lanes. So with 1 or 2 lanes this is
still needed.

BR,
Jani.


>
>
>> > +
>> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
>> > +	    crtc_config->port_clock >= 540000) {
>> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
>> > +		tmp |= SPARE_13;
>> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
>> > +	}
>> > +
>> >  	/* Enable audio presence detect, invalidate ELD */
>> >  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
>> >  	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
>> 
>

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

Re: [PATCH v3 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Jani Nikula]

On Wed, 26 Oct 2016, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote:
> On Wed, 2016-10-26 at 11:54 +0300, Jani Nikula wrote:
>> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
>> > According to BSpec, cdclk has to be not less than 432 MHz with DP audio
>> > enabled, port width x4, and link rate HBR2 (5.4 GHz)
>> >
>> > Having a lower cdclk triggers pipe underruns, which then lead to displays
>> > continuously cycling off and on. This is essential for DP MST audio as the
>> > link is trained at HBR2 and 4 lanes by default.
>> >
>> > v3: Combine BDW pixel rate adjustments into a function (Jani)
>> > v2: Restrict fix to BDW
>> >     Retain the set cdclk across modesets (Ville)
>> > Cc: stable@vger.kernel.org
>> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> 
>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> We'll need this fix for Skylake too, don't we? Maybe Kabylake? Please
>> send a follow-up patch for Skylake with the changes mentioned inline
>> below.
>> 
>> BR,
>> Jani.
>> 
>> 
>
> Patch 1/2 should take care of gen9 platforms when audio sampling rates
> are <= 96 kHz. I am still not sure how to increase increase cdclk when
> the sampling rates > 96kHZ , that is why I have added the TODO in Patch
> 1/2.

IIUC the two patches and fixes are related, but separate. This one
requires 4 lanes, the other one doesn't. For this one there's no mention
of audio sampling rate.

BR,
Jani.


>
>> > ---
>> >  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++++++---
>> >  1 file changed, 24 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> > index a94f7d1..efe46b4 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -10260,6 +10260,27 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
>> >  	bxt_set_cdclk(to_i915(dev), req_cdclk);
>> >  }
>> >  
>> > +static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
>> > +					  int pixel_rate)
>> > +{
>> > +	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
>> > +	if (crtc_state->ips_enabled)
>> 
>> if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
>> 
>> > +		pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
>> > +
>> > +	/* BSpec says "Do not use DisplayPort with CDCLK less than
>> > +	 * 432 MHz, audio enabled, port width x4, and link rate
>> > +	 * HBR2 (5.4 GHz), or else there may be audio corruption or
>> > +	 * screen corruption."
>> > +	 */
>> > +	if (intel_crtc_has_dp_encoder(crtc_state) &&
>> > +	    crtc_state->has_audio &&
>> > +	    crtc_state->port_clock >= 540000 &&
>> > +	    crtc_state->lane_count == 4)
>> > +		pixel_rate = max(432000, pixel_rate);
>> > +
>> > +	return pixel_rate;
>> > +}
>> > +
>> >  /* compute the max rate for new configuration */
>> >  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>> >  {
>> > @@ -10285,9 +10306,9 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>> >  
>> >  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
>> >  
>> > -		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
>> > -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
>> > -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
>> > +		if (IS_BROADWELL(dev_priv))
>> 
>> if (IS_BROADWELL(dev_priv) || IS_SKYLAKE(dev_priv))
>> 
>> > +			pixel_rate = bdw_adjust_min_pipe_pixel_rate(crtc_state,
>> > +								    pixel_rate);
>> >  
>> >  		intel_state->min_pixclk[i] = pixel_rate;
>> >  	}
>> 
>

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH v3 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Jani Nikula]

On Wed, 26 Oct 2016, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote:
> On Wed, 2016-10-26 at 11:54 +0300, Jani Nikula wrote:
>> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
>> > According to BSpec, cdclk has to be not less than 432 MHz with DP audio
>> > enabled, port width x4, and link rate HBR2 (5.4 GHz)
>> >
>> > Having a lower cdclk triggers pipe underruns, which then lead to displays
>> > continuously cycling off and on. This is essential for DP MST audio as the
>> > link is trained at HBR2 and 4 lanes by default.
>> >
>> > v3: Combine BDW pixel rate adjustments into a function (Jani)
>> > v2: Restrict fix to BDW
>> >     Retain the set cdclk across modesets (Ville)
>> > Cc: stable@vger.kernel.org
>> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> 
>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> We'll need this fix for Skylake too, don't we? Maybe Kabylake? Please
>> send a follow-up patch for Skylake with the changes mentioned inline
>> below.
>> 
>> BR,
>> Jani.
>> 
>> 
>
> Patch 1/2 should take care of gen9 platforms when audio sampling rates
> are <= 96 kHz. I am still not sure how to increase increase cdclk when
> the sampling rates > 96kHZ , that is why I have added the TODO in Patch
> 1/2.

IIUC the two patches and fixes are related, but separate. This one
requires 4 lanes, the other one doesn't. For this one there's no mention
of audio sampling rate.

BR,
Jani.


>
>> > ---
>> >  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++++++---
>> >  1 file changed, 24 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> > index a94f7d1..efe46b4 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -10260,6 +10260,27 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
>> >  	bxt_set_cdclk(to_i915(dev), req_cdclk);
>> >  }
>> >  
>> > +static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
>> > +					  int pixel_rate)
>> > +{
>> > +	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
>> > +	if (crtc_state->ips_enabled)
>> 
>> if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
>> 
>> > +		pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
>> > +
>> > +	/* BSpec says "Do not use DisplayPort with CDCLK less than
>> > +	 * 432 MHz, audio enabled, port width x4, and link rate
>> > +	 * HBR2 (5.4 GHz), or else there may be audio corruption or
>> > +	 * screen corruption."
>> > +	 */
>> > +	if (intel_crtc_has_dp_encoder(crtc_state) &&
>> > +	    crtc_state->has_audio &&
>> > +	    crtc_state->port_clock >= 540000 &&
>> > +	    crtc_state->lane_count == 4)
>> > +		pixel_rate = max(432000, pixel_rate);
>> > +
>> > +	return pixel_rate;
>> > +}
>> > +
>> >  /* compute the max rate for new configuration */
>> >  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>> >  {
>> > @@ -10285,9 +10306,9 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
>> >  
>> >  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
>> >  
>> > -		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
>> > -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
>> > -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
>> > +		if (IS_BROADWELL(dev_priv))
>> 
>> if (IS_BROADWELL(dev_priv) || IS_SKYLAKE(dev_priv))
>> 
>> > +			pixel_rate = bdw_adjust_min_pipe_pixel_rate(crtc_state,
>> > +								    pixel_rate);
>> >  
>> >  		intel_state->min_pixclk[i] = pixel_rate;
>> >  	}
>> 
>

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

Re: [Intel-gfx] [PATCH v3 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Pandiyan, Dhinakaran]

On Wed, 2016-10-26 at 22:08 +0300, Jani Nikula wrote:
> On Wed, 26 Oct 2016, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote:
> > On Wed, 2016-10-26 at 11:54 +0300, Jani Nikula wrote:
> >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> >> > According to BSpec, cdclk has to be not less than 432 MHz with DP audio
> >> > enabled, port width x4, and link rate HBR2 (5.4 GHz)
> >> >
> >> > Having a lower cdclk triggers pipe underruns, which then lead to displays
> >> > continuously cycling off and on. This is essential for DP MST audio as the
> >> > link is trained at HBR2 and 4 lanes by default.
> >> >
> >> > v3: Combine BDW pixel rate adjustments into a function (Jani)
> >> > v2: Restrict fix to BDW
> >> >     Retain the set cdclk across modesets (Ville)
> >> > Cc: stable@vger.kernel.org
> >> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> >> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> 
> >> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> >> 
> >> We'll need this fix for Skylake too, don't we? Maybe Kabylake? Please
> >> send a follow-up patch for Skylake with the changes mentioned inline
> >> below.
> >> 
> >> BR,
> >> Jani.
> >> 
> >> 
> >
> > Patch 1/2 should take care of gen9 platforms when audio sampling rates
> > are <= 96 kHz. I am still not sure how to increase increase cdclk when
> > the sampling rates > 96kHZ , that is why I have added the TODO in Patch
> > 1/2.
> 
> IIUC the two patches and fixes are related, but separate. This one
> requires 4 lanes, the other one doesn't. For this one there's no mention
> of audio sampling rate.
> 
> BR,
> Jani.
> 
> 

That's right, this one is needed on BDW for DP HBR2, 4 lanes
irrespective of the audio sampling rate. Here's the relevant description
that I found in BSpec for this patch. 

[Register] DP_TP_CTL
Project
BDW, SKL:*:A, SKL:*:B, SKL:*:C, SKL:*:D, BXT:*:A
Do not use DisplayPort with CDCLK less than 432 MHz, audio enabled, port
width x4, and link rate HBR2 (5.4 GHz), or else there may be audio
corruption or screen corruption.


-DK
> >
> >> > ---
> >> >  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++++++---
> >> >  1 file changed, 24 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> > index a94f7d1..efe46b4 100644
> >> > --- a/drivers/gpu/drm/i915/intel_display.c
> >> > +++ b/drivers/gpu/drm/i915/intel_display.c
> >> > @@ -10260,6 +10260,27 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> >> >  	bxt_set_cdclk(to_i915(dev), req_cdclk);
> >> >  }
> >> >  
> >> > +static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> >> > +					  int pixel_rate)
> >> > +{
> >> > +	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> >> > +	if (crtc_state->ips_enabled)
> >> 
> >> if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> >> 
> >> > +		pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> >> > +
> >> > +	/* BSpec says "Do not use DisplayPort with CDCLK less than
> >> > +	 * 432 MHz, audio enabled, port width x4, and link rate
> >> > +	 * HBR2 (5.4 GHz), or else there may be audio corruption or
> >> > +	 * screen corruption."
> >> > +	 */
> >> > +	if (intel_crtc_has_dp_encoder(crtc_state) &&
> >> > +	    crtc_state->has_audio &&
> >> > +	    crtc_state->port_clock >= 540000 &&
> >> > +	    crtc_state->lane_count == 4)
> >> > +		pixel_rate = max(432000, pixel_rate);
> >> > +
> >> > +	return pixel_rate;
> >> > +}
> >> > +
> >> >  /* compute the max rate for new configuration */
> >> >  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >> >  {
> >> > @@ -10285,9 +10306,9 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >> >  
> >> >  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
> >> >  
> >> > -		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> >> > -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> >> > -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> >> > +		if (IS_BROADWELL(dev_priv))
> >> 
> >> if (IS_BROADWELL(dev_priv) || IS_SKYLAKE(dev_priv))
> >> 
> >> > +			pixel_rate = bdw_adjust_min_pipe_pixel_rate(crtc_state,
> >> > +								    pixel_rate);
> >> >  
> >> >  		intel_state->min_pixclk[i] = pixel_rate;
> >> >  	}
> >> 
> >
> 

Re: [PATCH v3 2/2] drm/i915/dp: BDW cdclk fix for DP audio

[Pandiyan, Dhinakaran]

On Wed, 2016-10-26 at 22:08 +0300, Jani Nikula wrote:
> On Wed, 26 Oct 2016, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote:
> > On Wed, 2016-10-26 at 11:54 +0300, Jani Nikula wrote:
> >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> >> > According to BSpec, cdclk has to be not less than 432 MHz with DP audio
> >> > enabled, port width x4, and link rate HBR2 (5.4 GHz)
> >> >
> >> > Having a lower cdclk triggers pipe underruns, which then lead to displays
> >> > continuously cycling off and on. This is essential for DP MST audio as the
> >> > link is trained at HBR2 and 4 lanes by default.
> >> >
> >> > v3: Combine BDW pixel rate adjustments into a function (Jani)
> >> > v2: Restrict fix to BDW
> >> >     Retain the set cdclk across modesets (Ville)
> >> > Cc: stable@vger.kernel.org
> >> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> >> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> 
> >> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> >> 
> >> We'll need this fix for Skylake too, don't we? Maybe Kabylake? Please
> >> send a follow-up patch for Skylake with the changes mentioned inline
> >> below.
> >> 
> >> BR,
> >> Jani.
> >> 
> >> 
> >
> > Patch 1/2 should take care of gen9 platforms when audio sampling rates
> > are <= 96 kHz. I am still not sure how to increase increase cdclk when
> > the sampling rates > 96kHZ , that is why I have added the TODO in Patch
> > 1/2.
> 
> IIUC the two patches and fixes are related, but separate. This one
> requires 4 lanes, the other one doesn't. For this one there's no mention
> of audio sampling rate.
> 
> BR,
> Jani.
> 
> 

That's right, this one is needed on BDW for DP HBR2, 4 lanes
irrespective of the audio sampling rate. Here's the relevant description
that I found in BSpec for this patch. 

[Register] DP_TP_CTL
Project
BDW, SKL:*:A, SKL:*:B, SKL:*:C, SKL:*:D, BXT:*:A
Do not use DisplayPort with CDCLK less than 432 MHz, audio enabled, port
width x4, and link rate HBR2 (5.4 GHz), or else there may be audio
corruption or screen corruption.


-DK
> >
> >> > ---
> >> >  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++++++---
> >> >  1 file changed, 24 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> > index a94f7d1..efe46b4 100644
> >> > --- a/drivers/gpu/drm/i915/intel_display.c
> >> > +++ b/drivers/gpu/drm/i915/intel_display.c
> >> > @@ -10260,6 +10260,27 @@ static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> >> >  	bxt_set_cdclk(to_i915(dev), req_cdclk);
> >> >  }
> >> >  
> >> > +static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> >> > +					  int pixel_rate)
> >> > +{
> >> > +	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> >> > +	if (crtc_state->ips_enabled)
> >> 
> >> if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> >> 
> >> > +		pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> >> > +
> >> > +	/* BSpec says "Do not use DisplayPort with CDCLK less than
> >> > +	 * 432 MHz, audio enabled, port width x4, and link rate
> >> > +	 * HBR2 (5.4 GHz), or else there may be audio corruption or
> >> > +	 * screen corruption."
> >> > +	 */
> >> > +	if (intel_crtc_has_dp_encoder(crtc_state) &&
> >> > +	    crtc_state->has_audio &&
> >> > +	    crtc_state->port_clock >= 540000 &&
> >> > +	    crtc_state->lane_count == 4)
> >> > +		pixel_rate = max(432000, pixel_rate);
> >> > +
> >> > +	return pixel_rate;
> >> > +}
> >> > +
> >> >  /* compute the max rate for new configuration */
> >> >  static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >> >  {
> >> > @@ -10285,9 +10306,9 @@ static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> >> >  
> >> >  		pixel_rate = ilk_pipe_pixel_rate(crtc_state);
> >> >  
> >> > -		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> >> > -		if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
> >> > -			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> >> > +		if (IS_BROADWELL(dev_priv))
> >> 
> >> if (IS_BROADWELL(dev_priv) || IS_SKYLAKE(dev_priv))
> >> 
> >> > +			pixel_rate = bdw_adjust_min_pipe_pixel_rate(crtc_state,
> >> > +								    pixel_rate);
> >> >  
> >> >  		intel_state->min_pixclk[i] = pixel_rate;
> >> >  	}
> >> 
> >
> 

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

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Pandiyan, Dhinakaran]

On Wed, 2016-10-26 at 22:06 +0300, Jani Nikula wrote:
> On Wed, 26 Oct 2016, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote:
> > On Wed, 2016-10-26 at 11:57 +0300, Jani Nikula wrote:
> >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> >> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> >> > let's set this bit right before enabling the audio codec. Playing audio
> >> > without setting this bit results in pipe FIFO underruns.
> >> >
> >> > This workaround is applicable only for audio sample rates up to 96kHz. For
> >> > frequencies above 96kHz, this is insufficient and cdclk should be increased
> >> > to at least 432 MHz, just like BDW. Since, the audio driver does not
> >> > support sample rates > 48 kHz, we are safe with this fix for now.
> >> >
> >> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> >> >     Fixed the port clock typo
> >> >     Added TODO comment
> >> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
> >> >  drivers/gpu/drm/i915/intel_audio.c | 30 +++++++++++++++++++++++++++++-
> >> >  2 files changed, 34 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> > index 00efaa1..76dac48 100644
> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> > @@ -6236,6 +6236,11 @@ enum {
> >> >  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
> >> >  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
> >> >  
> >> > +#define _CHICKEN_TRANS_A	0x420C0
> >> > +#define _CHICKEN_TRANS_B	0x420C4
> >> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> >> > +#define SPARE_13	(1<<13)
> >> > +
> >> >  /* WaCatErrorRejectionIssue */
> >> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
> >> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> >> > index 7093cfb..894f11e 100644
> >> > --- a/drivers/gpu/drm/i915/intel_audio.c
> >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> >> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
> >> >  {
> >> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >> >  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> >> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> >> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> >> >  	enum pipe pipe = intel_crtc->pipe;
> >> >  	uint32_t tmp;
> >> >  
> >> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
> >> >  
> >> >  	mutex_lock(&dev_priv->av_mutex);
> >> >  
> >> > +	/*Disable DP audio stall fix for HBR2*/
> >> 
> >> Nitpick, spaces after /* and before */.
> >> 
> >> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> >> > +	    crtc_config->port_clock >= 540000) {
> >> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> >> > +		tmp &= ~SPARE_13;
> >> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> >> > +	}
> >> 
> >> Hmm. Why don't we just do this unconditionally?
> >> 
> >
> > That bit is disabled by default, so avoiding  two MMIO ops. that are not
> > required.
> 
> That makes no difference on the modeset path.
> 

Because we do a lot of MMIO ops. anyway?


> >
> >> > +
> >> >  	/* Disable timestamps */
> >> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> >> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> >> >  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
> >> >  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> >> >  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> >> > -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> >> > +	if (intel_crtc_has_dp_encoder(crtc_config))
> >> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >> >  
> >> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> >> >  {
> >> >  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> >> >  	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> >> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> >> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> >> >  	enum pipe pipe = intel_crtc->pipe;
> >> >  	enum port port = intel_encoder->port;
> >> >  	const uint8_t *eld = connector->eld;
> >> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> >> >  
> >> >  	mutex_lock(&dev_priv->av_mutex);
> >> >  
> >> > +	/* Enable DP audio stall fix for HBR2
> >> > +	 *
> >> > +	 * TODO: This workaround is applicable only for audio sample rates up
> >> > +	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
> >> > +	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> >> > +	 * the audio driver does not support sample rates > 48 kHz, we are safe
> >> > +	 * with this fix for now.
> >> > +	 */
> >> 
> >> Is this TODO required if you already have that check in patch 2/2?
> >> 
> >
> > In fact, this patch itself is not required if we are increasing the
> > cdclk to 432 MHz for gen9 platforms (like patch 2/2). Having this
> > workaround gives us the option running the pipeline at a lower cdclk
> > frequency i.e., 337.5 MHz, which I believe should be better in terms of
> > power.
> 
> The conditions for requiring higher cdclk are different in patch 2/2,
> right? That one also requires 4 lanes. So with 1 or 2 lanes this is
> still needed.
> 
> BR,
> Jani.
> 
> 

I have not tested the 1 or 2 lane, HBR2 configuration myself. And BSpec
doesn't exactly clarify that this is *needed* for 1 or 2 lanes HBR2, but
I see no harm in having this workaround just based on HBR2.


> >
> >
> >> > +
> >> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> >> > +	    crtc_config->port_clock >= 540000) {
> >> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> >> > +		tmp |= SPARE_13;
> >> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> >> > +	}
> >> > +
> >> >  	/* Enable audio presence detect, invalidate ELD */
> >> >  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> >> >  	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
> >> 
> >
> 

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

✗ Fi.CI.BAT: warning for DP audio fixes (rev5)

[Patchwork]

== Series Details ==

Series: DP audio fixes (rev5)
URL   : https://patchwork.freedesktop.org/series/14314/
State : warning

== Summary ==

Series 14314v5 DP audio fixes
https://patchwork.freedesktop.org/api/1.0/series/14314/revisions/5/mbox/

Test gem_ctx_switch:
        Subgroup basic-default:
                timeout    -> PASS       (fi-bsw-n3050)
        Subgroup basic-default-heavy:
                incomplete -> PASS       (fi-bsw-n3050)
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (fi-snb-2600)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-skl-6770hq)

fi-bdw-5557u     total:246  pass:231  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-n2820     total:246  pass:211  dwarn:0   dfail:0   fail:0   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:223  dwarn:0   dfail:0   fail:0   skip:23 
fi-ilk-650       total:246  pass:185  dwarn:0   dfail:0   fail:0   skip:61 
fi-ivb-3520m     total:246  pass:220  dwarn:0   dfail:0   fail:0   skip:26 
fi-ivb-3770      total:246  pass:220  dwarn:0   dfail:0   fail:0   skip:26 
fi-kbl-7200u     total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:246  pass:223  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:222  dwarn:1   dfail:0   fail:0   skip:23 
fi-skl-6770hq    total:246  pass:231  dwarn:1   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:246  pass:209  dwarn:0   dfail:0   fail:0   skip:37 
fi-snb-2600      total:246  pass:207  dwarn:1   dfail:0   fail:0   skip:38 

418b28c2fc2812aff227b273f400bf611c4e7dff drm-intel-nightly: 2016y-10m-27d-08h-38m-08s UTC integration manifest
d7eef28 drm/i915/dp: BDW cdclk fix for DP audio
c589cd8 drm/i915/dp: Enable DP audio stall fix for gen9 platforms

Full results at https://intel-gfx-ci.01.org/CI/Patchwork_2837/

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2837/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Pandiyan, Dhinakaran]

On Wed, 2016-10-26 at 18:14 +0000, Pandiyan, Dhinakaran wrote:
> On Wed, 2016-10-26 at 12:11 +0300, Ville Syrjälä wrote:
> > On Tue, Oct 25, 2016 at 07:37:36PM -0700, Dhinakaran Pandiyan wrote:
> > > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> > > let's set this bit right before enabling the audio codec. Playing audio
> > > without setting this bit results in pipe FIFO underruns.
> > > 
> > > This workaround is applicable only for audio sample rates up to 96kHz. For
> > > frequencies above 96kHz, this is insufficient and cdclk should be increased
> > > to at least 432 MHz, just like BDW. Since, the audio driver does not
> > > support sample rates > 48 kHz, we are safe with this fix for now.
> > > 
> > > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> > >     Fixed the port clock typo
> > >     Added TODO comment
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
> > >  drivers/gpu/drm/i915/intel_audio.c | 30 +++++++++++++++++++++++++++++-
> > >  2 files changed, 34 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 00efaa1..76dac48 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -6236,6 +6236,11 @@ enum {
> > >  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
> > >  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
> > >  
> > > +#define _CHICKEN_TRANS_A	0x420C0
> > > +#define _CHICKEN_TRANS_B	0x420C4
> > > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> > > +#define SPARE_13	(1<<13)
> > > +
> > >  /* WaCatErrorRejectionIssue */
> > >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
> > >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> > > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> > > index 7093cfb..894f11e 100644
> > > --- a/drivers/gpu/drm/i915/intel_audio.c
> > > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
> > >  {
> > >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > >  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> > > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> > >  	enum pipe pipe = intel_crtc->pipe;
> > >  	uint32_t tmp;
> > >  
> > > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
> > >  
> > >  	mutex_lock(&dev_priv->av_mutex);
> > >  
> > > +	/*Disable DP audio stall fix for HBR2*/
> > > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > > +	    crtc_config->port_clock >= 540000) {
> > > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > > +		tmp &= ~SPARE_13;
> > > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > > +	}
> > > +
> > >  	/* Disable timestamps */
> > >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> > >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > >  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
> > >  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> > >  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> > > -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> > > +	if (intel_crtc_has_dp_encoder(crtc_config))
> > >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> > >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > >  
> > > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> > >  {
> > >  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > >  	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> > > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> > >  	enum pipe pipe = intel_crtc->pipe;
> > >  	enum port port = intel_encoder->port;
> > >  	const uint8_t *eld = connector->eld;
> > > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> > >  
> > >  	mutex_lock(&dev_priv->av_mutex);
> > >  
> > > +	/* Enable DP audio stall fix for HBR2
> > > +	 *
> > > +	 * TODO: This workaround is applicable only for audio sample rates up
> > > +	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
> > > +	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> > > +	 * the audio driver does not support sample rates > 48 kHz, we are safe
> > > +	 * with this fix for now.
> > 
> > Where in the sound driver is this supposed 96kHz limit? I see a lot of
> > stuff for >96kHz in the code at least.
> > 
> 
> Libin/Jeeja can you help me here?
> 
> 

I see another problem, we cannot change cdclk without a full modeset. So
choosing the workarounds based on audio sampling rate is going to be
tricky. I guess, it makes sense to use the BDW cdclk workaround to SKL
too.


-DK


> > > +	 */
> > > +
> > > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > > +	    crtc_config->port_clock >= 540000) {
> > > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > > +		tmp |= SPARE_13;
> > > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > > +	}
> > > +
> > >  	/* Enable audio presence detect, invalidate ELD */
> > >  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> > >  	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
> > > -- 
> > > 2.7.4
> > 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Yang, Libin]

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Pandiyan, Dhinakaran
> Sent: Thursday, October 27, 2016 2:14 AM
> To: ville.syrjala@linux.intel.com
> Cc: Nikula, Jani <jani.nikula@intel.com>; Kp, Jeeja <jeeja.kp@intel.com>;
> intel-gfx@lists.freedesktop.org; libin.yang@linux.intel.com
> Subject: Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix
> for gen9 platforms
> 
> On Wed, 2016-10-26 at 12:11 +0300, Ville Syrjälä wrote:
> > On Tue, Oct 25, 2016 at 07:37:36PM -0700, Dhinakaran Pandiyan wrote:
> > > Enabling DP audio stall fix is necessary to play audio over DP HBR2.
> > > So, let's set this bit right before enabling the audio codec.
> > > Playing audio without setting this bit results in pipe FIFO underruns.
> > >
> > > This workaround is applicable only for audio sample rates up to
> > > 96kHz. For frequencies above 96kHz, this is insufficient and cdclk
> > > should be increased to at least 432 MHz, just like BDW. Since, the
> > > audio driver does not support sample rates > 48 kHz, we are safe with
> this fix for now.
> > >
> > > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> > >     Fixed the port clock typo
> > >     Added TODO comment
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
> > >  drivers/gpu/drm/i915/intel_audio.c | 30
> > > +++++++++++++++++++++++++++++-
> > >  2 files changed, 34 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h index 00efaa1..76dac48 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -6236,6 +6236,11 @@ enum {
> > >  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
> > >  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
> > >
> > > +#define _CHICKEN_TRANS_A	0x420C0
> > > +#define _CHICKEN_TRANS_B	0x420C4
> > > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran,
> _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> > > +#define SPARE_13	(1<<13)
> > > +
> > >  /* WaCatErrorRejectionIssue */
> > >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG
> 	_MMIO(0x9030)
> > >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> > > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> > > b/drivers/gpu/drm/i915/intel_audio.c
> > > index 7093cfb..894f11e 100644
> > > --- a/drivers/gpu/drm/i915/intel_audio.c
> > > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct
> > > intel_encoder *encoder)  {
> > >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > >  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> > > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> > >  	enum pipe pipe = intel_crtc->pipe;
> > >  	uint32_t tmp;
> > >
> > > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct
> > > intel_encoder *encoder)
> > >
> > >  	mutex_lock(&dev_priv->av_mutex);
> > >
> > > +	/*Disable DP audio stall fix for HBR2*/
> > > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > > +	    crtc_config->port_clock >= 540000) {
> > > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > > +		tmp &= ~SPARE_13;
> > > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > > +	}
> > > +
> > >  	/* Disable timestamps */
> > >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> > >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > >  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
> > >  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> > >  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> > > -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> > > +	if (intel_crtc_has_dp_encoder(crtc_config))
> > >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> > >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > >
> > > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct
> > > drm_connector *connector,  {
> > >  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > >  	struct intel_crtc *intel_crtc =
> > > to_intel_crtc(intel_encoder->base.crtc);
> > > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> > >  	enum pipe pipe = intel_crtc->pipe;
> > >  	enum port port = intel_encoder->port;
> > >  	const uint8_t *eld = connector->eld; @@ -326,6 +338,22 @@ static
> > > void hsw_audio_codec_enable(struct drm_connector *connector,
> > >
> > >  	mutex_lock(&dev_priv->av_mutex);
> > >
> > > +	/* Enable DP audio stall fix for HBR2
> > > +	 *
> > > +	 * TODO: This workaround is applicable only for audio sample rates
> up
> > > +	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
> > > +	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> > > +	 * the audio driver does not support sample rates > 48 kHz, we are
> safe
> > > +	 * with this fix for now.
> >
> > Where in the sound driver is this supposed 96kHz limit? I see a lot of
> > stuff for >96kHz in the code at least.
> >
> 
> Libin/Jeeja can you help me here?

From audio driver side, 96KHz should be similar with 48KHz. I have test 88KHz,
it works. When I test 96KHz, it will be converted to 48KHz. Our audio QA
didn't test 88KHz and 96KHz and it is not in our support list.

How do you get the 'stuff for >96kHz in the code' message? It seems I can't
reproduce this message.

Regards,
Libin

> 
> 
> > > +	 */
> > > +
> > > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > > +	    crtc_config->port_clock >= 540000) {
> > > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > > +		tmp |= SPARE_13;
> > > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > > +	}
> > > +
> > >  	/* Enable audio presence detect, invalidate ELD */
> > >  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> > >  	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
> > > --
> > > 2.7.4
> >
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Jani Nikula]

On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> let's set this bit right before enabling the audio codec. Playing audio
> without setting this bit results in pipe FIFO underruns.
>
> This workaround is applicable only for audio sample rates up to 96kHz. For
> frequencies above 96kHz, this is insufficient and cdclk should be increased
> to at least 432 MHz, just like BDW. Since, the audio driver does not
> support sample rates > 48 kHz, we are safe with this fix for now.

Do we still need this patch now that these two have been pushed?

b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9 platforms

BR,
Jani.



>
> v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
>     Fixed the port clock typo
>     Added TODO comment
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
>  drivers/gpu/drm/i915/intel_audio.c | 30 +++++++++++++++++++++++++++++-
>  2 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 00efaa1..76dac48 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6236,6 +6236,11 @@ enum {
>  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
>  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
>  
> +#define _CHICKEN_TRANS_A	0x420C0
> +#define _CHICKEN_TRANS_B	0x420C4
> +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> +#define SPARE_13	(1<<13)
> +
>  /* WaCatErrorRejectionIssue */
>  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
>  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 7093cfb..894f11e 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>  	enum pipe pipe = intel_crtc->pipe;
>  	uint32_t tmp;
>  
> @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  
> +	/*Disable DP audio stall fix for HBR2*/
> +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> +	    crtc_config->port_clock >= 540000) {
> +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		tmp &= ~SPARE_13;
> +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> +	}
> +
>  	/* Disable timestamps */
>  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
>  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> +	if (intel_crtc_has_dp_encoder(crtc_config))
>  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>  
> @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>  	enum pipe pipe = intel_crtc->pipe;
>  	enum port port = intel_encoder->port;
>  	const uint8_t *eld = connector->eld;
> @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  
> +	/* Enable DP audio stall fix for HBR2
> +	 *
> +	 * TODO: This workaround is applicable only for audio sample rates up
> +	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
> +	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> +	 * the audio driver does not support sample rates > 48 kHz, we are safe
> +	 * with this fix for now.
> +	 */
> +
> +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> +	    crtc_config->port_clock >= 540000) {
> +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		tmp |= SPARE_13;
> +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> +	}
> +
>  	/* Enable audio presence detect, invalidate ELD */
>  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
>  	tmp |= AUDIO_OUTPUT_ENABLE(pipe);

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

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Pandiyan, Dhinakaran]

On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> > let's set this bit right before enabling the audio codec. Playing audio
> > without setting this bit results in pipe FIFO underruns.
> >
> > This workaround is applicable only for audio sample rates up to 96kHz. For
> > frequencies above 96kHz, this is insufficient and cdclk should be increased
> > to at least 432 MHz, just like BDW. Since, the audio driver does not
> > support sample rates > 48 kHz, we are safe with this fix for now.
> 
> Do we still need this patch now that these two have been pushed?
> 
> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9 platforms
> 
> BR,
> Jani.
> 
> 
> 

No, we are good afaik. This patch would have helped us to make use of a
lower cdclk (337.5 MHz), with constraints on audio bit rate. Operating
at 432 MHz, like we do now, rules out the need for this patch.

-DK

> >
> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> >     Fixed the port clock typo
> >     Added TODO comment
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
> >  drivers/gpu/drm/i915/intel_audio.c | 30 +++++++++++++++++++++++++++++-
> >  2 files changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 00efaa1..76dac48 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6236,6 +6236,11 @@ enum {
> >  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
> >  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
> >  
> > +#define _CHICKEN_TRANS_A	0x420C0
> > +#define _CHICKEN_TRANS_B	0x420C4
> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> > +#define SPARE_13	(1<<13)
> > +
> >  /* WaCatErrorRejectionIssue */
> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> > index 7093cfb..894f11e 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> >  	enum pipe pipe = intel_crtc->pipe;
> >  	uint32_t tmp;
> >  
> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
> >  
> >  	mutex_lock(&dev_priv->av_mutex);
> >  
> > +	/*Disable DP audio stall fix for HBR2*/
> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > +	    crtc_config->port_clock >= 540000) {
> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +		tmp &= ~SPARE_13;
> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > +	}
> > +
> >  	/* Disable timestamps */
> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> >  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
> >  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> >  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> > -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> > +	if (intel_crtc_has_dp_encoder(crtc_config))
> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >  
> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> >  	enum pipe pipe = intel_crtc->pipe;
> >  	enum port port = intel_encoder->port;
> >  	const uint8_t *eld = connector->eld;
> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> >  
> >  	mutex_lock(&dev_priv->av_mutex);
> >  
> > +	/* Enable DP audio stall fix for HBR2
> > +	 *
> > +	 * TODO: This workaround is applicable only for audio sample rates up
> > +	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
> > +	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> > +	 * the audio driver does not support sample rates > 48 kHz, we are safe
> > +	 * with this fix for now.
> > +	 */
> > +
> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > +	    crtc_config->port_clock >= 540000) {
> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +		tmp |= SPARE_13;
> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > +	}
> > +
> >  	/* Enable audio presence detect, invalidate ELD */
> >  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> >  	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
> 

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

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Jani Nikula]

On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote:
> On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
>> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
>> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
>> > let's set this bit right before enabling the audio codec. Playing audio
>> > without setting this bit results in pipe FIFO underruns.
>> >
>> > This workaround is applicable only for audio sample rates up to 96kHz. For
>> > frequencies above 96kHz, this is insufficient and cdclk should be increased
>> > to at least 432 MHz, just like BDW. Since, the audio driver does not
>> > support sample rates > 48 kHz, we are safe with this fix for now.
>> 
>> Do we still need this patch now that these two have been pushed?
>> 
>> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
>> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9 platforms
>> 
>> BR,
>> Jani.
>> 
>> 
>> 
>
> No, we are good afaik. This patch would have helped us to make use of a
> lower cdclk (337.5 MHz), with constraints on audio bit rate. Operating
> at 432 MHz, like we do now, rules out the need for this patch.

Hmm, what about 5.4 Gbps link with 1 or 2 lanes?

BR,
Jani.


>
> -DK
>
>> >
>> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
>> >     Fixed the port clock typo
>> >     Added TODO comment
>> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
>> >  drivers/gpu/drm/i915/intel_audio.c | 30 +++++++++++++++++++++++++++++-
>> >  2 files changed, 34 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> > index 00efaa1..76dac48 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -6236,6 +6236,11 @@ enum {
>> >  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
>> >  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
>> >  
>> > +#define _CHICKEN_TRANS_A	0x420C0
>> > +#define _CHICKEN_TRANS_B	0x420C4
>> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
>> > +#define SPARE_13	(1<<13)
>> > +
>> >  /* WaCatErrorRejectionIssue */
>> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
>> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>> > index 7093cfb..894f11e 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
>> >  {
>> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> >  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
>> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>> >  	enum pipe pipe = intel_crtc->pipe;
>> >  	uint32_t tmp;
>> >  
>> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
>> >  
>> >  	mutex_lock(&dev_priv->av_mutex);
>> >  
>> > +	/*Disable DP audio stall fix for HBR2*/
>> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
>> > +	    crtc_config->port_clock >= 540000) {
>> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
>> > +		tmp &= ~SPARE_13;
>> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
>> > +	}
>> > +
>> >  	/* Disable timestamps */
>> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>> >  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
>> >  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>> >  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
>> > -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
>> > +	if (intel_crtc_has_dp_encoder(crtc_config))
>> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> >  
>> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>> >  {
>> >  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>> >  	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
>> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
>> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>> >  	enum pipe pipe = intel_crtc->pipe;
>> >  	enum port port = intel_encoder->port;
>> >  	const uint8_t *eld = connector->eld;
>> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>> >  
>> >  	mutex_lock(&dev_priv->av_mutex);
>> >  
>> > +	/* Enable DP audio stall fix for HBR2
>> > +	 *
>> > +	 * TODO: This workaround is applicable only for audio sample rates up
>> > +	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
>> > +	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
>> > +	 * the audio driver does not support sample rates > 48 kHz, we are safe
>> > +	 * with this fix for now.
>> > +	 */
>> > +
>> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
>> > +	    crtc_config->port_clock >= 540000) {
>> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
>> > +		tmp |= SPARE_13;
>> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
>> > +	}
>> > +
>> >  	/* Enable audio presence detect, invalidate ELD */
>> >  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
>> >  	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
>> 
>

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

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Pandiyan, Dhinakaran]

On Sat, 2016-11-05 at 21:40 +0200, Jani Nikula wrote:
> On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote:
> > On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
> >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> >> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> >> > let's set this bit right before enabling the audio codec. Playing audio
> >> > without setting this bit results in pipe FIFO underruns.
> >> >
> >> > This workaround is applicable only for audio sample rates up to 96kHz. For
> >> > frequencies above 96kHz, this is insufficient and cdclk should be increased
> >> > to at least 432 MHz, just like BDW. Since, the audio driver does not
> >> > support sample rates > 48 kHz, we are safe with this fix for now.
> >> 
> >> Do we still need this patch now that these two have been pushed?
> >> 
> >> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
> >> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9 platforms
> >> 
> >> BR,
> >> Jani.
> >> 
> >> 
> >> 
> >
> > No, we are good afaik. This patch would have helped us to make use of a
> > lower cdclk (337.5 MHz), with constraints on audio bit rate. Operating
> > at 432 MHz, like we do now, rules out the need for this patch.
> 
> Hmm, what about 5.4 Gbps link with 1 or 2 lanes?
> 
> BR,
> Jani.
> 

Good point, I think it will depend on the audio sampling rate. But, I
have to figure out a way to play high sampling rate audio (> 96 KHz) and
test 5.4 Gbps with 1 or 2 lanes.

The other option is to play safe and apply this patch with even lesser
restrictions, say link rate >= 2.7 Gbps.


-DK

> >
> > -DK
> >
> >> >
> >> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> >> >     Fixed the port clock typo
> >> >     Added TODO comment
> >> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
> >> >  drivers/gpu/drm/i915/intel_audio.c | 30 +++++++++++++++++++++++++++++-
> >> >  2 files changed, 34 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> > index 00efaa1..76dac48 100644
> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> > @@ -6236,6 +6236,11 @@ enum {
> >> >  #define SLICE_ECO_CHICKEN0			_MMIO(0x7308)
> >> >  #define   PIXEL_MASK_CAMMING_DISABLE		(1 << 14)
> >> >  
> >> > +#define _CHICKEN_TRANS_A	0x420C0
> >> > +#define _CHICKEN_TRANS_B	0x420C4
> >> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> >> > +#define SPARE_13	(1<<13)
> >> > +
> >> >  /* WaCatErrorRejectionIssue */
> >> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		_MMIO(0x9030)
> >> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
> >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> >> > index 7093cfb..894f11e 100644
> >> > --- a/drivers/gpu/drm/i915/intel_audio.c
> >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> >> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
> >> >  {
> >> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >> >  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> >> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> >> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> >> >  	enum pipe pipe = intel_crtc->pipe;
> >> >  	uint32_t tmp;
> >> >  
> >> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
> >> >  
> >> >  	mutex_lock(&dev_priv->av_mutex);
> >> >  
> >> > +	/*Disable DP audio stall fix for HBR2*/
> >> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> >> > +	    crtc_config->port_clock >= 540000) {
> >> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> >> > +		tmp &= ~SPARE_13;
> >> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> >> > +	}
> >> > +
> >> >  	/* Disable timestamps */
> >> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> >> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> >> >  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
> >> >  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> >> >  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> >> > -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> >> > +	if (intel_crtc_has_dp_encoder(crtc_config))
> >> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >> >  
> >> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> >> >  {
> >> >  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> >> >  	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> >> > +	struct intel_crtc_state *crtc_config =  intel_crtc->config;
> >> > +	enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> >> >  	enum pipe pipe = intel_crtc->pipe;
> >> >  	enum port port = intel_encoder->port;
> >> >  	const uint8_t *eld = connector->eld;
> >> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> >> >  
> >> >  	mutex_lock(&dev_priv->av_mutex);
> >> >  
> >> > +	/* Enable DP audio stall fix for HBR2
> >> > +	 *
> >> > +	 * TODO: This workaround is applicable only for audio sample rates up
> >> > +	 * to 96kHz. For frequencies above 96kHz, this is insufficient and
> >> > +	 * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> >> > +	 * the audio driver does not support sample rates > 48 kHz, we are safe
> >> > +	 * with this fix for now.
> >> > +	 */
> >> > +
> >> > +	if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> >> > +	    crtc_config->port_clock >= 540000) {
> >> > +		tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> >> > +		tmp |= SPARE_13;
> >> > +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> >> > +	}
> >> > +
> >> >  	/* Enable audio presence detect, invalidate ELD */
> >> >  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> >> >  	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
> >> 
> >
> 

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

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Peter Frühberger]

[-- Attachment #1.1: Type: text/plain, Size: 8129 bytes --]

Hi

On Sun, Nov 6, 2016 at 1:23 AM, Pandiyan, Dhinakaran <
dhinakaran.pandiyan@intel.com> wrote:

> On Sat, 2016-11-05 at 21:40 +0200, Jani Nikula wrote:
> > On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran" <
> dhinakaran.pandiyan@intel.com> wrote:
> > > On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
> > >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <
> dhinakaran.pandiyan@intel.com> wrote:
> > >> > Enabling DP audio stall fix is necessary to play audio over DP
> HBR2. So,
> > >> > let's set this bit right before enabling the audio codec. Playing
> audio
> > >> > without setting this bit results in pipe FIFO underruns.
> > >> >
> > >> > This workaround is applicable only for audio sample rates up to
> 96kHz. For
> > >> > frequencies above 96kHz, this is insufficient and cdclk should be
> increased
> > >> > to at least 432 MHz, just like BDW. Since, the audio driver does not
> > >> > support sample rates > 48 kHz, we are safe with this fix for now.
> > >>
> > >> Do we still need this patch now that these two have been pushed?
> > >>
> > >> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
> > >> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9
> platforms
> > >>
> > >> BR,
> > >> Jani.
> > >>
> > >>
> > >>
> > >
> > > No, we are good afaik. This patch would have helped us to make use of a
> > > lower cdclk (337.5 MHz), with constraints on audio bit rate. Operating
> > > at 432 MHz, like we do now, rules out the need for this patch.
> >
> > Hmm, what about 5.4 Gbps link with 1 or 2 lanes?
> >
> > BR,
> > Jani.
> >
>
> Good point, I think it will depend on the audio sampling rate. But, I
> have to figure out a way to play high sampling rate audio (> 96 KHz) and
> test 5.4 Gbps with 1 or 2 lanes.
>
> The other option is to play safe and apply this patch with even lesser
> restrictions, say link rate >= 2.7 Gbps.
>
>
> -DK
>

as we are currently talking about high samplerates in this context. I
wanted to post a perhaps related issue. On my Apollo Lake (J4205) I have
two outputs. One DVI and one HDMI 2.0 via internal DP. Via DVI the
following works without issues, via DP it fails. As the original commit
mentions HBR, I think there is still something missing. We submit TrueHD,
DTS-HD via 192 khz and 16 bit format while setting AES0=2

You can easily reproduce with (you obviously need a DTS-HD, TrueHD capable
AVR attached to your HDMI 2.0 (DP) out):

#TrueHD
aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
testi.truehd.anssi1.ff.60s.spdif
#DTS-HD
aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
testi.dtshd.anssi1.ma-71-24.spdif
Samples:
http://www.avenard.org/files/media/mediatest/audiotest/HDAUDIO/Passthrough/

For the old HDMI 1.x chips it was fixed via:
https://bugs.freedesktop.org/show_bug.cgi?id=49055

Is this also planned for DP within that patch series?

Best regards
Peter

>
> > >
> > > -DK
> > >
> > >> >
> > >> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> > >> >     Fixed the port clock typo
> > >> >     Added TODO comment
> > >> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > >> > ---
> > >> >  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
> > >> >  drivers/gpu/drm/i915/intel_audio.c | 30
> +++++++++++++++++++++++++++++-
> > >> >  2 files changed, 34 insertions(+), 1 deletion(-)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> > >> > index 00efaa1..76dac48 100644
> > >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > >> > @@ -6236,6 +6236,11 @@ enum {
> > >> >  #define SLICE_ECO_CHICKEN0                      _MMIO(0x7308)
> > >> >  #define   PIXEL_MASK_CAMMING_DISABLE            (1 << 14)
> > >> >
> > >> > +#define _CHICKEN_TRANS_A        0x420C0
> > >> > +#define _CHICKEN_TRANS_B        0x420C4
> > >> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A,
> _CHICKEN_TRANS_B)
> > >> > +#define SPARE_13        (1<<13)
> > >> > +
> > >> >  /* WaCatErrorRejectionIssue */
> > >> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG          _MMIO(0x9030)
> > >> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB       (1<<11)
> > >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> > >> > index 7093cfb..894f11e 100644
> > >> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > >> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct
> intel_encoder *encoder)
> > >> >  {
> > >> >          struct drm_i915_private *dev_priv =
> to_i915(encoder->base.dev);
> > >> >          struct intel_crtc *intel_crtc =
> to_intel_crtc(encoder->base.crtc);
> > >> > +        struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > >> > +        enum transcoder cpu_transcoder =
> crtc_config->cpu_transcoder;
> > >> >          enum pipe pipe = intel_crtc->pipe;
> > >> >          uint32_t tmp;
> > >> >
> > >> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct
> intel_encoder *encoder)
> > >> >
> > >> >          mutex_lock(&dev_priv->av_mutex);
> > >> >
> > >> > +        /*Disable DP audio stall fix for HBR2*/
> > >> > +        if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config)
> &&
> > >> > +            crtc_config->port_clock >= 540000) {
> > >> > +                tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > >> > +                tmp &= ~SPARE_13;
> > >> > +                I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > >> > +        }
> > >> > +
> > >> >          /* Disable timestamps */
> > >> >          tmp = I915_READ(HSW_AUD_CFG(pipe));
> > >> >          tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > >> >          tmp |= AUD_CONFIG_N_PROG_ENABLE;
> > >> >          tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> > >> >          tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> > >> > -        if (intel_crtc_has_dp_encoder(intel_crtc->config))
> > >> > +        if (intel_crtc_has_dp_encoder(crtc_config))
> > >> >                  tmp |= AUD_CONFIG_N_VALUE_INDEX;
> > >> >          I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > >> >
> > >> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> > >> >  {
> > >> >          struct drm_i915_private *dev_priv =
> to_i915(connector->dev);
> > >> >          struct intel_crtc *intel_crtc =
> to_intel_crtc(intel_encoder->base.crtc);
> > >> > +        struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > >> > +        enum transcoder cpu_transcoder =
> crtc_config->cpu_transcoder;
> > >> >          enum pipe pipe = intel_crtc->pipe;
> > >> >          enum port port = intel_encoder->port;
> > >> >          const uint8_t *eld = connector->eld;
> > >> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> > >> >
> > >> >          mutex_lock(&dev_priv->av_mutex);
> > >> >
> > >> > +        /* Enable DP audio stall fix for HBR2
> > >> > +         *
> > >> > +         * TODO: This workaround is applicable only for audio
> sample rates up
> > >> > +         * to 96kHz. For frequencies above 96kHz, this is
> insufficient and
> > >> > +         * cdclk should be increased to at least 432 MHz, just
> like BDW. Since,
> > >> > +         * the audio driver does not support sample rates > 48
> kHz, we are safe
> > >> > +         * with this fix for now.
> > >> > +         */
> > >> > +
> > >> > +        if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config)
> &&
> > >> > +            crtc_config->port_clock >= 540000) {
> > >> > +                tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > >> > +                tmp |= SPARE_13;
> > >> > +                I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > >> > +        }
> > >> > +
> > >> >          /* Enable audio presence detect, invalidate ELD */
> > >> >          tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> > >> >          tmp |= AUDIO_OUTPUT_ENABLE(pipe);
> > >>
> > >
> >
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

[-- Attachment #1.2: Type: text/html, Size: 11837 bytes --]

[-- 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

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Jani Nikula]

On Wed, 04 Jan 2017, Peter Frühberger <fritsch@kodi.tv> wrote:
> Hi
>
> On Sun, Nov 6, 2016 at 1:23 AM, Pandiyan, Dhinakaran <
> dhinakaran.pandiyan@intel.com> wrote:
>
>> On Sat, 2016-11-05 at 21:40 +0200, Jani Nikula wrote:
>> > On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran" <
>> dhinakaran.pandiyan@intel.com> wrote:
>> > > On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
>> > >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <
>> dhinakaran.pandiyan@intel.com> wrote:
>> > >> > Enabling DP audio stall fix is necessary to play audio over DP
>> HBR2. So,
>> > >> > let's set this bit right before enabling the audio codec. Playing
>> audio
>> > >> > without setting this bit results in pipe FIFO underruns.
>> > >> >
>> > >> > This workaround is applicable only for audio sample rates up to
>> 96kHz. For
>> > >> > frequencies above 96kHz, this is insufficient and cdclk should be
>> increased
>> > >> > to at least 432 MHz, just like BDW. Since, the audio driver does not
>> > >> > support sample rates > 48 kHz, we are safe with this fix for now.
>> > >>
>> > >> Do we still need this patch now that these two have been pushed?
>> > >>
>> > >> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
>> > >> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9
>> platforms
>> > >>
>> > >> BR,
>> > >> Jani.
>> > >>
>> > >>
>> > >>
>> > >
>> > > No, we are good afaik. This patch would have helped us to make use of a
>> > > lower cdclk (337.5 MHz), with constraints on audio bit rate. Operating
>> > > at 432 MHz, like we do now, rules out the need for this patch.
>> >
>> > Hmm, what about 5.4 Gbps link with 1 or 2 lanes?
>> >
>> > BR,
>> > Jani.
>> >
>>
>> Good point, I think it will depend on the audio sampling rate. But, I
>> have to figure out a way to play high sampling rate audio (> 96 KHz) and
>> test 5.4 Gbps with 1 or 2 lanes.
>>
>> The other option is to play safe and apply this patch with even lesser
>> restrictions, say link rate >= 2.7 Gbps.
>>
>>
>> -DK
>>
>
> as we are currently talking about high samplerates in this context. I
> wanted to post a perhaps related issue. On my Apollo Lake (J4205) I have
> two outputs. One DVI and one HDMI 2.0 via internal DP. Via DVI the
> following works without issues, via DP it fails. As the original commit
> mentions HBR, I think there is still something missing. We submit TrueHD,
> DTS-HD via 192 khz and 16 bit format while setting AES0=2
>
> You can easily reproduce with (you obviously need a DTS-HD, TrueHD capable
> AVR attached to your HDMI 2.0 (DP) out):

Just to clarify, is the DP -> HDMI2.0 converter internal to the machine?
LSPCON related messages in the dmesg with drm.debug=14? Do you have a DP
or an HDMI physical connector in the chassis?


BR,
Jani.

>
> #TrueHD
> aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
> testi.truehd.anssi1.ff.60s.spdif
> #DTS-HD
> aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
> testi.dtshd.anssi1.ma-71-24.spdif
> Samples:
> http://www.avenard.org/files/media/mediatest/audiotest/HDAUDIO/Passthrough/
>
> For the old HDMI 1.x chips it was fixed via:
> https://bugs.freedesktop.org/show_bug.cgi?id=49055
>
> Is this also planned for DP within that patch series?
>
> Best regards
> Peter
>
>>
>> > >
>> > > -DK
>> > >
>> > >> >
>> > >> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
>> > >> >     Fixed the port clock typo
>> > >> >     Added TODO comment
>> > >> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> > >> > ---
>> > >> >  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
>> > >> >  drivers/gpu/drm/i915/intel_audio.c | 30
>> +++++++++++++++++++++++++++++-
>> > >> >  2 files changed, 34 insertions(+), 1 deletion(-)
>> > >> >
>> > >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> > >> > index 00efaa1..76dac48 100644
>> > >> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > >> > @@ -6236,6 +6236,11 @@ enum {
>> > >> >  #define SLICE_ECO_CHICKEN0                      _MMIO(0x7308)
>> > >> >  #define   PIXEL_MASK_CAMMING_DISABLE            (1 << 14)
>> > >> >
>> > >> > +#define _CHICKEN_TRANS_A        0x420C0
>> > >> > +#define _CHICKEN_TRANS_B        0x420C4
>> > >> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A,
>> _CHICKEN_TRANS_B)
>> > >> > +#define SPARE_13        (1<<13)
>> > >> > +
>> > >> >  /* WaCatErrorRejectionIssue */
>> > >> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG          _MMIO(0x9030)
>> > >> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB       (1<<11)
>> > >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> b/drivers/gpu/drm/i915/intel_audio.c
>> > >> > index 7093cfb..894f11e 100644
>> > >> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > >> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct
>> intel_encoder *encoder)
>> > >> >  {
>> > >> >          struct drm_i915_private *dev_priv =
>> to_i915(encoder->base.dev);
>> > >> >          struct intel_crtc *intel_crtc =
>> to_intel_crtc(encoder->base.crtc);
>> > >> > +        struct intel_crtc_state *crtc_config =  intel_crtc->config;
>> > >> > +        enum transcoder cpu_transcoder =
>> crtc_config->cpu_transcoder;
>> > >> >          enum pipe pipe = intel_crtc->pipe;
>> > >> >          uint32_t tmp;
>> > >> >
>> > >> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct
>> intel_encoder *encoder)
>> > >> >
>> > >> >          mutex_lock(&dev_priv->av_mutex);
>> > >> >
>> > >> > +        /*Disable DP audio stall fix for HBR2*/
>> > >> > +        if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config)
>> &&
>> > >> > +            crtc_config->port_clock >= 540000) {
>> > >> > +                tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
>> > >> > +                tmp &= ~SPARE_13;
>> > >> > +                I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
>> > >> > +        }
>> > >> > +
>> > >> >          /* Disable timestamps */
>> > >> >          tmp = I915_READ(HSW_AUD_CFG(pipe));
>> > >> >          tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>> > >> >          tmp |= AUD_CONFIG_N_PROG_ENABLE;
>> > >> >          tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>> > >> >          tmp &= ~AUD_CONFIG_LOWER_N_MASK;
>> > >> > -        if (intel_crtc_has_dp_encoder(intel_crtc->config))
>> > >> > +        if (intel_crtc_has_dp_encoder(crtc_config))
>> > >> >                  tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> > >> >          I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> > >> >
>> > >> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct
>> drm_connector *connector,
>> > >> >  {
>> > >> >          struct drm_i915_private *dev_priv =
>> to_i915(connector->dev);
>> > >> >          struct intel_crtc *intel_crtc =
>> to_intel_crtc(intel_encoder->base.crtc);
>> > >> > +        struct intel_crtc_state *crtc_config =  intel_crtc->config;
>> > >> > +        enum transcoder cpu_transcoder =
>> crtc_config->cpu_transcoder;
>> > >> >          enum pipe pipe = intel_crtc->pipe;
>> > >> >          enum port port = intel_encoder->port;
>> > >> >          const uint8_t *eld = connector->eld;
>> > >> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct
>> drm_connector *connector,
>> > >> >
>> > >> >          mutex_lock(&dev_priv->av_mutex);
>> > >> >
>> > >> > +        /* Enable DP audio stall fix for HBR2
>> > >> > +         *
>> > >> > +         * TODO: This workaround is applicable only for audio
>> sample rates up
>> > >> > +         * to 96kHz. For frequencies above 96kHz, this is
>> insufficient and
>> > >> > +         * cdclk should be increased to at least 432 MHz, just
>> like BDW. Since,
>> > >> > +         * the audio driver does not support sample rates > 48
>> kHz, we are safe
>> > >> > +         * with this fix for now.
>> > >> > +         */
>> > >> > +
>> > >> > +        if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config)
>> &&
>> > >> > +            crtc_config->port_clock >= 540000) {
>> > >> > +                tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
>> > >> > +                tmp |= SPARE_13;
>> > >> > +                I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
>> > >> > +        }
>> > >> > +
>> > >> >          /* Enable audio presence detect, invalidate ELD */
>> > >> >          tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
>> > >> >          tmp |= AUDIO_OUTPUT_ENABLE(pipe);
>> > >>
>> > >
>> >
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>

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

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Peter Frühberger]

[-- Attachment #1.1: Type: text/plain, Size: 10438 bytes --]

Forgot to CC the list, sorry.

On Wed, Jan 4, 2017 at 11:42 AM, Peter Frühberger <fritsch@kodi.tv> wrote:

> Hi Jani,
> thanks for your reply
>
> On Wed, Jan 4, 2017 at 10:34 AM, Jani Nikula <jani.nikula@intel.com>
> wrote:
>
>> On Wed, 04 Jan 2017, Peter Frühberger <fritsch@kodi.tv> wrote:
>> > Hi
>> >
>> > On Sun, Nov 6, 2016 at 1:23 AM, Pandiyan, Dhinakaran <
>> > dhinakaran.pandiyan@intel.com> wrote:
>> >
>> >> On Sat, 2016-11-05 at 21:40 +0200, Jani Nikula wrote:
>> >> > On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran" <
>> >> dhinakaran.pandiyan@intel.com> wrote:
>> >> > > On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
>> >> > >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <
>> >> dhinakaran.pandiyan@intel.com> wrote:
>> >> > >> > Enabling DP audio stall fix is necessary to play audio over DP
>> >> HBR2. So,
>> >> > >> > let's set this bit right before enabling the audio codec.
>> Playing
>> >> audio
>> >> > >> > without setting this bit results in pipe FIFO underruns.
>> >> > >> >
>> >> > >> > This workaround is applicable only for audio sample rates up to
>> >> 96kHz. For
>> >> > >> > frequencies above 96kHz, this is insufficient and cdclk should
>> be
>> >> increased
>> >> > >> > to at least 432 MHz, just like BDW. Since, the audio driver
>> does not
>> >> > >> > support sample rates > 48 kHz, we are safe with this fix for
>> now.
>> >> > >>
>> >> > >> Do we still need this patch now that these two have been pushed?
>> >> > >>
>> >> > >> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
>> >> > >> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9
>> >> platforms
>> >> > >>
>> >> > >> BR,
>> >> > >> Jani.
>> >> > >>
>> >> > >>
>> >> > >>
>> >> > >
>> >> > > No, we are good afaik. This patch would have helped us to make use
>> of a
>> >> > > lower cdclk (337.5 MHz), with constraints on audio bit rate.
>> Operating
>> >> > > at 432 MHz, like we do now, rules out the need for this patch.
>> >> >
>> >> > Hmm, what about 5.4 Gbps link with 1 or 2 lanes?
>> >> >
>> >> > BR,
>> >> > Jani.
>> >> >
>> >>
>> >> Good point, I think it will depend on the audio sampling rate. But, I
>> >> have to figure out a way to play high sampling rate audio (> 96 KHz)
>> and
>> >> test 5.4 Gbps with 1 or 2 lanes.
>> >>
>> >> The other option is to play safe and apply this patch with even lesser
>> >> restrictions, say link rate >= 2.7 Gbps.
>> >>
>> >>
>> >> -DK
>> >>
>> >
>> > as we are currently talking about high samplerates in this context. I
>> > wanted to post a perhaps related issue. On my Apollo Lake (J4205) I have
>> > two outputs. One DVI and one HDMI 2.0 via internal DP. Via DVI the
>> > following works without issues, via DP it fails. As the original commit
>> > mentions HBR, I think there is still something missing. We submit
>> TrueHD,
>> > DTS-HD via 192 khz and 16 bit format while setting AES0=2
>> >
>> > You can easily reproduce with (you obviously need a DTS-HD, TrueHD
>> capable
>> > AVR attached to your HDMI 2.0 (DP) out):
>>
>> Just to clarify, is the DP -> HDMI2.0 converter internal to the machine?
>> LSPCON related messages in the dmesg with drm.debug=14? Do you have a DP
>> or an HDMI physical connector in the chassis?
>>
>
> The chip used is: https://media.digikey.com/pdf/Data%20Sheets/MegaChips%
> 20PDFs/MCDP28x0_Datasheet.pdf which is the same on all intel nucs,
> including the new Kabilake ones. So it's internal.
> Mainboard: http://www.asrock.com/MB/Intel/J4205-ITX/index.us.asp (DVI-D,
> HDMI (2.0 via this above chip), VGA)
> Here is the output of the a boot up with drm.debug=14:
> http://paste.ubuntu.com/23738282/
> (I used a pastebin site to not spam the ML, is that okay for the future?)
>
> Best regards
> Peter
>
>
>>
>>
>> BR,
>> Jani.
>>
>> >
>> > #TrueHD
>> > aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
>> > testi.truehd.anssi1.ff.60s.spdif
>> > #DTS-HD
>> > aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
>> > testi.dtshd.anssi1.ma-71-24.spdif
>> > Samples:
>> > http://www.avenard.org/files/media/mediatest/audiotest/HDAUD
>> IO/Passthrough/
>> >
>> > For the old HDMI 1.x chips it was fixed via:
>> > https://bugs.freedesktop.org/show_bug.cgi?id=49055
>> >
>> > Is this also planned for DP within that patch series?
>> >
>> > Best regards
>> > Peter
>> >
>> >>
>> >> > >
>> >> > > -DK
>> >> > >
>> >> > >> >
>> >> > >> > v2: Inlined the code change within hsw_audio_codec_enable()
>> (Jani)
>> >> > >> >     Fixed the port clock typo
>> >> > >> >     Added TODO comment
>> >> > >> > Signed-off-by: Dhinakaran Pandiyan <
>> dhinakaran.pandiyan@intel.com>
>> >> > >> > ---
>> >> > >> >  drivers/gpu/drm/i915/i915_reg.h    |  5 +++++
>> >> > >> >  drivers/gpu/drm/i915/intel_audio.c | 30
>> >> +++++++++++++++++++++++++++++-
>> >> > >> >  2 files changed, 34 insertions(+), 1 deletion(-)
>> >> > >> >
>> >> > >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> >> b/drivers/gpu/drm/i915/i915_reg.h
>> >> > >> > index 00efaa1..76dac48 100644
>> >> > >> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> >> > >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> >> > >> > @@ -6236,6 +6236,11 @@ enum {
>> >> > >> >  #define SLICE_ECO_CHICKEN0                      _MMIO(0x7308)
>> >> > >> >  #define   PIXEL_MASK_CAMMING_DISABLE            (1 << 14)
>> >> > >> >
>> >> > >> > +#define _CHICKEN_TRANS_A        0x420C0
>> >> > >> > +#define _CHICKEN_TRANS_B        0x420C4
>> >> > >> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A,
>> >> _CHICKEN_TRANS_B)
>> >> > >> > +#define SPARE_13        (1<<13)
>> >> > >> > +
>> >> > >> >  /* WaCatErrorRejectionIssue */
>> >> > >> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG          _MMIO(0x9030)
>> >> > >> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB       (1<<11)
>> >> > >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> >> b/drivers/gpu/drm/i915/intel_audio.c
>> >> > >> > index 7093cfb..894f11e 100644
>> >> > >> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> >> > >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> >> > >> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct
>> >> intel_encoder *encoder)
>> >> > >> >  {
>> >> > >> >          struct drm_i915_private *dev_priv =
>> >> to_i915(encoder->base.dev);
>> >> > >> >          struct intel_crtc *intel_crtc =
>> >> to_intel_crtc(encoder->base.crtc);
>> >> > >> > +        struct intel_crtc_state *crtc_config =
>> intel_crtc->config;
>> >> > >> > +        enum transcoder cpu_transcoder =
>> >> crtc_config->cpu_transcoder;
>> >> > >> >          enum pipe pipe = intel_crtc->pipe;
>> >> > >> >          uint32_t tmp;
>> >> > >> >
>> >> > >> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct
>> >> intel_encoder *encoder)
>> >> > >> >
>> >> > >> >          mutex_lock(&dev_priv->av_mutex);
>> >> > >> >
>> >> > >> > +        /*Disable DP audio stall fix for HBR2*/
>> >> > >> > +        if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc
>> _config)
>> >> &&
>> >> > >> > +            crtc_config->port_clock >= 540000) {
>> >> > >> > +                tmp = I915_READ(CHICKEN_TRANS(cpu_tr
>> anscoder));
>> >> > >> > +                tmp &= ~SPARE_13;
>> >> > >> > +                I915_WRITE(CHICKEN_TRANS(cpu_transcoder),
>> tmp);
>> >> > >> > +        }
>> >> > >> > +
>> >> > >> >          /* Disable timestamps */
>> >> > >> >          tmp = I915_READ(HSW_AUD_CFG(pipe));
>> >> > >> >          tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>> >> > >> >          tmp |= AUD_CONFIG_N_PROG_ENABLE;
>> >> > >> >          tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>> >> > >> >          tmp &= ~AUD_CONFIG_LOWER_N_MASK;
>> >> > >> > -        if (intel_crtc_has_dp_encoder(intel_crtc->config))
>> >> > >> > +        if (intel_crtc_has_dp_encoder(crtc_config))
>> >> > >> >                  tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> >> > >> >          I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> >> > >> >
>> >> > >> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct
>> >> drm_connector *connector,
>> >> > >> >  {
>> >> > >> >          struct drm_i915_private *dev_priv =
>> >> to_i915(connector->dev);
>> >> > >> >          struct intel_crtc *intel_crtc =
>> >> to_intel_crtc(intel_encoder->base.crtc);
>> >> > >> > +        struct intel_crtc_state *crtc_config =
>> intel_crtc->config;
>> >> > >> > +        enum transcoder cpu_transcoder =
>> >> crtc_config->cpu_transcoder;
>> >> > >> >          enum pipe pipe = intel_crtc->pipe;
>> >> > >> >          enum port port = intel_encoder->port;
>> >> > >> >          const uint8_t *eld = connector->eld;
>> >> > >> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct
>> >> drm_connector *connector,
>> >> > >> >
>> >> > >> >          mutex_lock(&dev_priv->av_mutex);
>> >> > >> >
>> >> > >> > +        /* Enable DP audio stall fix for HBR2
>> >> > >> > +         *
>> >> > >> > +         * TODO: This workaround is applicable only for audio
>> >> sample rates up
>> >> > >> > +         * to 96kHz. For frequencies above 96kHz, this is
>> >> insufficient and
>> >> > >> > +         * cdclk should be increased to at least 432 MHz, just
>> >> like BDW. Since,
>> >> > >> > +         * the audio driver does not support sample rates > 48
>> >> kHz, we are safe
>> >> > >> > +         * with this fix for now.
>> >> > >> > +         */
>> >> > >> > +
>> >> > >> > +        if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc
>> _config)
>> >> &&
>> >> > >> > +            crtc_config->port_clock >= 540000) {
>> >> > >> > +                tmp = I915_READ(CHICKEN_TRANS(cpu_tr
>> anscoder));
>> >> > >> > +                tmp |= SPARE_13;
>> >> > >> > +                I915_WRITE(CHICKEN_TRANS(cpu_transcoder),
>> tmp);
>> >> > >> > +        }
>> >> > >> > +
>> >> > >> >          /* Enable audio presence detect, invalidate ELD */
>> >> > >> >          tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
>> >> > >> >          tmp |= AUDIO_OUTPUT_ENABLE(pipe);
>> >> > >>
>> >> > >
>> >> >
>> >>
>> >> _______________________________________________
>> >> Intel-gfx mailing list
>> >> Intel-gfx@lists.freedesktop.org
>> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >>
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
>>
>
>

[-- Attachment #1.2: Type: text/html, Size: 16114 bytes --]

[-- 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

Re: [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Pandiyan, Dhinakaran]

On Wed, 2017-01-04 at 11:42 +0100, Peter Frühberger wrote:
> Forgot to CC the list, sorry.
> 
> On Wed, Jan 4, 2017 at 11:42 AM, Peter Frühberger <fritsch@kodi.tv>
> wrote:
>         Hi Jani,
>         thanks for your reply
>         
>         On Wed, Jan 4, 2017 at 10:34 AM, Jani Nikula
>         <jani.nikula@intel.com> wrote:
>                 On Wed, 04 Jan 2017, Peter Frühberger
>                 <fritsch@kodi.tv> wrote:
>                 > Hi
>                 >
>                 > On Sun, Nov 6, 2016 at 1:23 AM, Pandiyan, Dhinakaran
>                 <
>                 > dhinakaran.pandiyan@intel.com> wrote:
>                 >
>                 >> On Sat, 2016-11-05 at 21:40 +0200, Jani Nikula
>                 wrote:
>                 >> > On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran" <
>                 >> dhinakaran.pandiyan@intel.com> wrote:
>                 >> > > On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula
>                 wrote:
>                 >> > >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <
>                 >> dhinakaran.pandiyan@intel.com> wrote:
>                 >> > >> > Enabling DP audio stall fix is necessary to
>                 play audio over DP
>                 >> HBR2. So,
>                 >> > >> > let's set this bit right before enabling the
>                 audio codec. Playing
>                 >> audio
>                 >> > >> > without setting this bit results in pipe
>                 FIFO underruns.
>                 >> > >> >
>                 >> > >> > This workaround is applicable only for audio
>                 sample rates up to
>                 >> 96kHz. For
>                 >> > >> > frequencies above 96kHz, this is
>                 insufficient and cdclk should be
>                 >> increased
>                 >> > >> > to at least 432 MHz, just like BDW. Since,
>                 the audio driver does not
>                 >> > >> > support sample rates > 48 kHz, we are safe
>                 with this fix for now.
>                 >> > >>
>                 >> > >> Do we still need this patch now that these two
>                 have been pushed?
>                 >> > >>
>                 >> > >> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP
>                 audio
>                 >> > >> 9c7540241885 drm/i915/dp: Extend BDW DP audio
>                 workaround to GEN9
>                 >> platforms
>                 >> > >>
>                 >> > >> BR,
>                 >> > >> Jani.
>                 >> > >>
>                 >> > >>
>                 >> > >>
>                 >> > >
>                 >> > > No, we are good afaik. This patch would have
>                 helped us to make use of a
>                 >> > > lower cdclk (337.5 MHz), with constraints on
>                 audio bit rate. Operating
>                 >> > > at 432 MHz, like we do now, rules out the need
>                 for this patch.
>                 >> >
>                 >> > Hmm, what about 5.4 Gbps link with 1 or 2 lanes?
>                 >> >
>                 >> > BR,
>                 >> > Jani.
>                 >> >
>                 >>
>                 >> Good point, I think it will depend on the audio
>                 sampling rate. But, I
>                 >> have to figure out a way to play high sampling rate
>                 audio (> 96 KHz) and
>                 >> test 5.4 Gbps with 1 or 2 lanes.
>                 >>
>                 >> The other option is to play safe and apply this
>                 patch with even lesser
>                 >> restrictions, say link rate >= 2.7 Gbps.
>                 >>
>                 >>
>                 >> -DK
>                 >>
>                 >
>                 > as we are currently talking about high samplerates
>                 in this context. I
>                 > wanted to post a perhaps related issue. On my Apollo
>                 Lake (J4205) I have
>                 > two outputs. One DVI and one HDMI 2.0 via internal
>                 DP. Via DVI the
>                 > following works without issues, via DP it fails. As
>                 the original commit
>                 > mentions HBR, I think there is still something
>                 missing. We submit TrueHD,
>                 > DTS-HD via 192 khz and 16 bit format while setting
>                 AES0=2
>                 >
>                 > You can easily reproduce with (you obviously need a
>                 DTS-HD, TrueHD capable
>                 > AVR attached to your HDMI 2.0 (DP) out):
>                 
+Libin


>                 Just to clarify, is the DP -> HDMI2.0 converter
>                 internal to the machine?
>                 LSPCON related messages in the dmesg with
>                 drm.debug=14? Do you have a DP
>                 or an HDMI physical connector in the chassis?
>         
>         
>         The chip used is: https://media.digikey.com/pdf/Data%
>         20Sheets/MegaChips%20PDFs/MCDP28x0_Datasheet.pdf which is the
>         same on all intel nucs, including the new Kabilake ones. So
>         it's internal. 
>         Mainboard: http://www.asrock.com/MB/Intel/J4205-ITX/index.us.asp (DVI-D, HDMI (2.0 via this above chip), VGA)
>         Here is the output of the a boot up with drm.debug=14:
>          http://paste.ubuntu.com/23738282/
>         (I used a pastebin site to not spam the ML, is that okay for
>         the future?)
>         
>         
>         Best regards
>         Peter
>          
>                 
>                 
>                 BR,
>                 Jani.
>                 
>                 >
>                 > #TrueHD
>                 > aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le
>                 -r192000
>                 > testi.truehd.anssi1.ff.60s.spdif
>                 > #DTS-HD
>                 > aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le
>                 -r192000
>                 > testi.dtshd.anssi1.ma-71-24.spdif
>                 > Samples:
>                 >
>                 http://www.avenard.org/files/media/mediatest/audiotest/HDAUDIO/Passthrough/
>                 >
>                 > For the old HDMI 1.x chips it was fixed via:
>                 > https://bugs.freedesktop.org/show_bug.cgi?id=49055
>                 >
>                 > Is this also planned for DP within that patch
>                 series?
>                 >
>                 > Best regards
>                 > Peter
>                 >
>                 >>
>                 >> > >
>                 >> > > -DK
>                 >> > >
>                 >> > >> >
>                 >> > >> > v2: Inlined the code change within
>                 hsw_audio_codec_enable() (Jani)
>                 >> > >> >     Fixed the port clock typo
>                 >> > >> >     Added TODO comment
>                 >> > >> > Signed-off-by: Dhinakaran Pandiyan
>                 <dhinakaran.pandiyan@intel.com>
>                 >> > >> > ---
>                 >> > >> >  drivers/gpu/drm/i915/i915_reg.h    |  5
>                 +++++
>                 >> > >> >  drivers/gpu/drm/i915/intel_audio.c | 30
>                 >> +++++++++++++++++++++++++++++-
>                 >> > >> >  2 files changed, 34 insertions(+), 1
>                 deletion(-)
>                 >> > >> >
>                 >> > >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>                 >> b/drivers/gpu/drm/i915/i915_reg.h
>                 >> > >> > index 00efaa1..76dac48 100644
>                 >> > >> > --- a/drivers/gpu/drm/i915/i915_reg.h
>                 >> > >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>                 >> > >> > @@ -6236,6 +6236,11 @@ enum {
>                 >> > >> >  #define SLICE_ECO_CHICKEN0
>                 _MMIO(0x7308)
>                 >> > >> >  #define   PIXEL_MASK_CAMMING_DISABLE
>                 (1 << 14)
>                 >> > >> >
>                 >> > >> > +#define _CHICKEN_TRANS_A        0x420C0
>                 >> > >> > +#define _CHICKEN_TRANS_B        0x420C4
>                 >> > >> > +#define CHICKEN_TRANS(tran)
>                 _MMIO_TRANS(tran, _CHICKEN_TRANS_A,
>                 >> _CHICKEN_TRANS_B)
>                 >> > >> > +#define SPARE_13        (1<<13)
>                 >> > >> > +
>                 >> > >> >  /* WaCatErrorRejectionIssue */
>                 >> > >> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG
>                 _MMIO(0x9030)
>                 >> > >> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB
>                      (1<<11)
>                 >> > >> > diff --git
>                 a/drivers/gpu/drm/i915/intel_audio.c
>                 >> b/drivers/gpu/drm/i915/intel_audio.c
>                 >> > >> > index 7093cfb..894f11e 100644
>                 >> > >> > --- a/drivers/gpu/drm/i915/intel_audio.c
>                 >> > >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>                 >> > >> > @@ -283,6 +283,8 @@ static void
>                 hsw_audio_codec_disable(struct
>                 >> intel_encoder *encoder)
>                 >> > >> >  {
>                 >> > >> >          struct drm_i915_private *dev_priv =
>                 >> to_i915(encoder->base.dev);
>                 >> > >> >          struct intel_crtc *intel_crtc =
>                 >> to_intel_crtc(encoder->base.crtc);
>                 >> > >> > +        struct intel_crtc_state
>                 *crtc_config =  intel_crtc->config;
>                 >> > >> > +        enum transcoder cpu_transcoder =
>                 >> crtc_config->cpu_transcoder;
>                 >> > >> >          enum pipe pipe = intel_crtc->pipe;
>                 >> > >> >          uint32_t tmp;
>                 >> > >> >
>                 >> > >> > @@ -290,13 +292,21 @@ static void
>                 hsw_audio_codec_disable(struct
>                 >> intel_encoder *encoder)
>                 >> > >> >
>                 >> > >> >          mutex_lock(&dev_priv->av_mutex);
>                 >> > >> >
>                 >> > >> > +        /*Disable DP audio stall fix for
>                 HBR2*/
>                 >> > >> > +        if (IS_GEN9(dev_priv) &&
>                 intel_crtc_has_dp_encoder(crtc_config)
>                 >> &&
>                 >> > >> > +            crtc_config->port_clock >=
>                 540000) {
>                 >> > >> > +                tmp =
>                 I915_READ(CHICKEN_TRANS(cpu_transcoder));
>                 >> > >> > +                tmp &= ~SPARE_13;
>                 >> > >> > +
>                 I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
>                 >> > >> > +        }
>                 >> > >> > +
>                 >> > >> >          /* Disable timestamps */
>                 >> > >> >          tmp = I915_READ(HSW_AUD_CFG(pipe));
>                 >> > >> >          tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>                 >> > >> >          tmp |= AUD_CONFIG_N_PROG_ENABLE;
>                 >> > >> >          tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>                 >> > >> >          tmp &= ~AUD_CONFIG_LOWER_N_MASK;
>                 >> > >> > -        if
>                 (intel_crtc_has_dp_encoder(intel_crtc->config))
>                 >> > >> > +        if
>                 (intel_crtc_has_dp_encoder(crtc_config))
>                 >> > >> >                  tmp |=
>                 AUD_CONFIG_N_VALUE_INDEX;
>                 >> > >> >          I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>                 >> > >> >
>                 >> > >> > @@ -315,6 +325,8 @@ static void
>                 hsw_audio_codec_enable(struct
>                 >> drm_connector *connector,
>                 >> > >> >  {
>                 >> > >> >          struct drm_i915_private *dev_priv =
>                 >> to_i915(connector->dev);
>                 >> > >> >          struct intel_crtc *intel_crtc =
>                 >> to_intel_crtc(intel_encoder->base.crtc);
>                 >> > >> > +        struct intel_crtc_state
>                 *crtc_config =  intel_crtc->config;
>                 >> > >> > +        enum transcoder cpu_transcoder =
>                 >> crtc_config->cpu_transcoder;
>                 >> > >> >          enum pipe pipe = intel_crtc->pipe;
>                 >> > >> >          enum port port =
>                 intel_encoder->port;
>                 >> > >> >          const uint8_t *eld =
>                 connector->eld;
>                 >> > >> > @@ -326,6 +338,22 @@ static void
>                 hsw_audio_codec_enable(struct
>                 >> drm_connector *connector,
>                 >> > >> >
>                 >> > >> >          mutex_lock(&dev_priv->av_mutex);
>                 >> > >> >
>                 >> > >> > +        /* Enable DP audio stall fix for
>                 HBR2
>                 >> > >> > +         *
>                 >> > >> > +         * TODO: This workaround is
>                 applicable only for audio
>                 >> sample rates up
>                 >> > >> > +         * to 96kHz. For frequencies above
>                 96kHz, this is
>                 >> insufficient and
>                 >> > >> > +         * cdclk should be increased to at
>                 least 432 MHz, just
>                 >> like BDW. Since,
>                 >> > >> > +         * the audio driver does not
>                 support sample rates > 48
>                 >> kHz, we are safe
>                 >> > >> > +         * with this fix for now.
>                 >> > >> > +         */
>                 >> > >> > +
>                 >> > >> > +        if (IS_GEN9(dev_priv) &&
>                 intel_crtc_has_dp_encoder(crtc_config)
>                 >> &&
>                 >> > >> > +            crtc_config->port_clock >=
>                 540000) {
>                 >> > >> > +                tmp =
>                 I915_READ(CHICKEN_TRANS(cpu_transcoder));
>                 >> > >> > +                tmp |= SPARE_13;
>                 >> > >> > +
>                 I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
>                 >> > >> > +        }
>                 >> > >> > +
>                 >> > >> >          /* Enable audio presence detect,
>                 invalidate ELD */
>                 >> > >> >          tmp =
>                 I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
>                 >> > >> >          tmp |= AUDIO_OUTPUT_ENABLE(pipe);
>                 >> > >>
>                 >> > >
>                 >> >
>                 >>
>                 >> _______________________________________________
>                 >> Intel-gfx mailing list
>                 >> Intel-gfx@lists.freedesktop.org
>                 >>
>                 https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>                 >>
>                 
>                 
>                 --
>                 Jani Nikula, Intel Open Source Technology Center
>                 
>         
>         
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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