[PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes

[PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes

[Jani Nikula]

Resend of [1] due to no review. Patches 1-8 should be easy stuff to
review. *wink*.

BR,
Jani.


[1] https://patchwork.freedesktop.org/series/12754/


Jani Nikula (7):
  drm/i915/audio: abstract audio config update
  drm/i915/audio: port is going to be just fine, simplify checks
  drm/i915/audio: use the same code for updating audio config
  drm/i915/audio: split dp and hdmi audio config update
  drm/i915/audio: add register macros for audio config N value
  drm/i915/audio: rename N value getter to emphasize it's for hdmi
  drm/i915: set proper N/M in modeset

Libin Yang (2):
  drm/i915/audio: set proper N/MCTS on more platforms
  drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock

 drivers/gpu/drm/i915/i915_reg.h    |  11 ++
 drivers/gpu/drm/i915/intel_audio.c | 260 ++++++++++++++++++++++++-------------
 2 files changed, 179 insertions(+), 92 deletions(-)

-- 
2.1.4

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

[PATCH RESEND 1/9] drm/i915/audio: abstract audio config update

[Jani Nikula]

Prepare for using the same code for updating HSW_AUD_CFG register. No
functional changes.

Cc: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 68 ++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 9583f432e02e..0a54f7cdce37 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -245,6 +245,45 @@ static void g4x_audio_codec_enable(struct drm_connector *connector,
 	I915_WRITE(G4X_AUD_CNTL_ST, tmp);
 }
 
+static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
+				    enum port port,
+				    const struct drm_display_mode *adjusted_mode)
+{
+	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+	struct i915_audio_component *acomp = dev_priv->audio_component;
+	enum pipe pipe = intel_crtc->pipe;
+	int n, rate;
+	u32 tmp;
+
+	tmp = I915_READ(HSW_AUD_CFG(pipe));
+	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
+	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
+	if (intel_crtc_has_dp_encoder(intel_crtc->config))
+		tmp |= AUD_CONFIG_N_VALUE_INDEX;
+	else
+		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
+
+	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+	if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
+		if (!acomp)
+			rate = 0;
+		else if (port >= PORT_A && port <= PORT_E)
+			rate = acomp->aud_sample_rate[port];
+		else {
+			DRM_ERROR("invalid port: %d\n", port);
+			rate = 0;
+		}
+
+		n = audio_config_get_n(adjusted_mode, rate);
+		if (n != 0)
+			tmp = audio_config_setup_n_reg(n, tmp);
+		else
+			DRM_DEBUG_KMS("no suitable N value is found\n");
+	}
+
+	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+}
+
 static void hsw_audio_codec_disable(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -283,11 +322,9 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
 	enum pipe pipe = intel_crtc->pipe;
 	enum port port = intel_encoder->port;
-	struct i915_audio_component *acomp = dev_priv->audio_component;
 	const uint8_t *eld = connector->eld;
 	uint32_t tmp;
 	int len, i;
-	int n, rate;
 
 	DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
 		      pipe_name(pipe), drm_eld_size(eld));
@@ -323,32 +360,7 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 	I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
 
 	/* Enable timestamps */
-	tmp = I915_READ(HSW_AUD_CFG(pipe));
-	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
-	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
-	if (intel_crtc_has_dp_encoder(intel_crtc->config))
-		tmp |= AUD_CONFIG_N_VALUE_INDEX;
-	else
-		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
-
-	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
-	if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
-		if (!acomp)
-			rate = 0;
-		else if (port >= PORT_A && port <= PORT_E)
-			rate = acomp->aud_sample_rate[port];
-		else {
-			DRM_ERROR("invalid port: %d\n", port);
-			rate = 0;
-		}
-		n = audio_config_get_n(adjusted_mode, rate);
-		if (n != 0)
-			tmp = audio_config_setup_n_reg(n, tmp);
-		else
-			DRM_DEBUG_KMS("no suitable N value is found\n");
-	}
-
-	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+	hsw_audio_config_update(intel_crtc, port, adjusted_mode);
 
 	mutex_unlock(&dev_priv->av_mutex);
 }
-- 
2.1.4

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

Re: [PATCH RESEND 1/9] drm/i915/audio: abstract audio config update

[Yang, Libin]

Reviewed-by: Libin Yang <libin.yang@intel.com>

Regards,
Libin


> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [PATCH RESEND 1/9] drm/i915/audio: abstract audio
> config update
> 
> Prepare for using the same code for updating HSW_AUD_CFG register. No
> functional changes.
> 
> Cc: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 68 ++++++++++++++++++++++------------
> ----
>  1 file changed, 40 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 9583f432e02e..0a54f7cdce37 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -245,6 +245,45 @@ static void g4x_audio_codec_enable(struct
> drm_connector *connector,
>  	I915_WRITE(G4X_AUD_CNTL_ST, tmp);
>  }
> 
> +static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
> +				    enum port port,
> +				    const struct drm_display_mode
> *adjusted_mode) {
> +	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> +	struct i915_audio_component *acomp = dev_priv-
> >audio_component;
> +	enum pipe pipe = intel_crtc->pipe;
> +	int n, rate;
> +	u32 tmp;
> +
> +	tmp = I915_READ(HSW_AUD_CFG(pipe));
> +	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> +	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> +	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> +		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> +	else
> +		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> +
> +	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> +	if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
> +		if (!acomp)
> +			rate = 0;
> +		else if (port >= PORT_A && port <= PORT_E)
> +			rate = acomp->aud_sample_rate[port];
> +		else {
> +			DRM_ERROR("invalid port: %d\n", port);
> +			rate = 0;
> +		}
> +
> +		n = audio_config_get_n(adjusted_mode, rate);
> +		if (n != 0)
> +			tmp = audio_config_setup_n_reg(n, tmp);
> +		else
> +			DRM_DEBUG_KMS("no suitable N value is found\n");
> +	}
> +
> +	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> +}
> +
>  static void hsw_audio_codec_disable(struct intel_encoder *encoder)  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> @@ -283,11 +322,9 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
>  	enum pipe pipe = intel_crtc->pipe;
>  	enum port port = intel_encoder->port;
> -	struct i915_audio_component *acomp = dev_priv-
> >audio_component;
>  	const uint8_t *eld = connector->eld;
>  	uint32_t tmp;
>  	int len, i;
> -	int n, rate;
> 
>  	DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
>  		      pipe_name(pipe), drm_eld_size(eld)); @@ -323,32 +360,7
> @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  	I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
> 
>  	/* Enable timestamps */
> -	tmp = I915_READ(HSW_AUD_CFG(pipe));
> -	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> -	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> -		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> -	else
> -		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> -
> -	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> -	if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
> -		if (!acomp)
> -			rate = 0;
> -		else if (port >= PORT_A && port <= PORT_E)
> -			rate = acomp->aud_sample_rate[port];
> -		else {
> -			DRM_ERROR("invalid port: %d\n", port);
> -			rate = 0;
> -		}
> -		n = audio_config_get_n(adjusted_mode, rate);
> -		if (n != 0)
> -			tmp = audio_config_setup_n_reg(n, tmp);
> -		else
> -			DRM_DEBUG_KMS("no suitable N value is found\n");
> -	}
> -
> -	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> +	hsw_audio_config_update(intel_crtc, port, adjusted_mode);
> 
>  	mutex_unlock(&dev_priv->av_mutex);
>  }
> --
> 2.1.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

[PATCH RESEND 2/9] drm/i915/audio: port is going to be just fine, simplify checks

[Jani Nikula]

If it was wrong, we'd be screwed already.

Cc: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 0a54f7cdce37..5d0bd07afa51 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -251,8 +251,9 @@ static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
 	struct i915_audio_component *acomp = dev_priv->audio_component;
+	int rate = acomp ? acomp->aud_sample_rate[port] : 0;
 	enum pipe pipe = intel_crtc->pipe;
-	int n, rate;
+	int n;
 	u32 tmp;
 
 	tmp = I915_READ(HSW_AUD_CFG(pipe));
@@ -265,15 +266,6 @@ static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
 
 	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
 	if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
-		if (!acomp)
-			rate = 0;
-		else if (port >= PORT_A && port <= PORT_E)
-			rate = acomp->aud_sample_rate[port];
-		else {
-			DRM_ERROR("invalid port: %d\n", port);
-			rate = 0;
-		}
-
 		n = audio_config_get_n(adjusted_mode, rate);
 		if (n != 0)
 			tmp = audio_config_setup_n_reg(n, tmp);
-- 
2.1.4

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

Re: [PATCH RESEND 2/9] drm/i915/audio: port is going to be just fine, simplify checks

[Yang, Libin]

Reviewed-by: Libin Yang <libin.yang@intel.com>

Regards,
Libin


> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [PATCH RESEND 2/9] drm/i915/audio: port is going to be
> just fine, simplify checks
> 
> If it was wrong, we'd be screwed already.
> 
> Cc: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 0a54f7cdce37..5d0bd07afa51 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -251,8 +251,9 @@ static void hsw_audio_config_update(struct intel_crtc
> *intel_crtc,  {
>  	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
>  	struct i915_audio_component *acomp = dev_priv-
> >audio_component;
> +	int rate = acomp ? acomp->aud_sample_rate[port] : 0;
>  	enum pipe pipe = intel_crtc->pipe;
> -	int n, rate;
> +	int n;
>  	u32 tmp;
> 
>  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> @@ -265,15 +266,6 @@ static void hsw_audio_config_update(struct
> intel_crtc *intel_crtc,
> 
>  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>  	if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
> -		if (!acomp)
> -			rate = 0;
> -		else if (port >= PORT_A && port <= PORT_E)
> -			rate = acomp->aud_sample_rate[port];
> -		else {
> -			DRM_ERROR("invalid port: %d\n", port);
> -			rate = 0;
> -		}
> -
>  		n = audio_config_get_n(adjusted_mode, rate);
>  		if (n != 0)
>  			tmp = audio_config_setup_n_reg(n, tmp);
> --
> 2.1.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

[PATCH RESEND 3/9] drm/i915/audio: use the same code for updating audio config

[Jani Nikula]

It gets fragile to duplicate the code for updating HSW_AUD_CFG. The only
change should be that the hdmi pixel clock is also updated in
i915_audio_component_sync_audio_rate(), but it should not be any
different.

Cc: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 29 +++--------------------------
 1 file changed, 3 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 5d0bd07afa51..4d62b3e8ac19 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -671,10 +671,8 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
 	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
 	struct intel_encoder *intel_encoder;
 	struct intel_crtc *crtc;
-	struct drm_display_mode *mode;
+	struct drm_display_mode *adjusted_mode;
 	struct i915_audio_component *acomp = dev_priv->audio_component;
-	u32 tmp;
-	int n;
 	int err = 0;
 
 	/* HSW, BDW, SKL, KBL need this fix */
@@ -700,33 +698,12 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
 	crtc = to_intel_crtc(intel_encoder->base.crtc);
 	pipe = crtc->pipe;
 
-	mode = &crtc->config->base.adjusted_mode;
+	adjusted_mode = &crtc->config->base.adjusted_mode;
 
 	/* port must be valid now, otherwise the pipe will be invalid */
 	acomp->aud_sample_rate[port] = rate;
 
-	/* 2. check whether to set the N/CTS/M manually or not */
-	if (!audio_rate_need_prog(crtc, mode)) {
-		tmp = I915_READ(HSW_AUD_CFG(pipe));
-		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
-		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
-		goto unlock;
-	}
-
-	n = audio_config_get_n(mode, rate);
-	if (n == 0) {
-		DRM_DEBUG_KMS("Using automatic mode for N value on port %c\n",
-					  port_name(port));
-		tmp = I915_READ(HSW_AUD_CFG(pipe));
-		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
-		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
-		goto unlock;
-	}
-
-	/* 3. set the N/CTS/M */
-	tmp = I915_READ(HSW_AUD_CFG(pipe));
-	tmp = audio_config_setup_n_reg(n, tmp);
-	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+	hsw_audio_config_update(crtc, port, adjusted_mode);
 
  unlock:
 	mutex_unlock(&dev_priv->av_mutex);
-- 
2.1.4

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

Re: [PATCH RESEND 3/9] drm/i915/audio: use the same code for updating audio config

[Yang, Libin]

Reviewed-by: Libin Yang <libin.yang@intel.com>

Regards,
Libin


> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [PATCH RESEND 3/9] drm/i915/audio: use the same code
> for updating audio config
> 
> It gets fragile to duplicate the code for updating HSW_AUD_CFG. The only
> change should be that the hdmi pixel clock is also updated in
> i915_audio_component_sync_audio_rate(), but it should not be any
> different.
> 
> Cc: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 29 +++--------------------------
>  1 file changed, 3 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 5d0bd07afa51..4d62b3e8ac19 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -671,10 +671,8 @@ static int
> i915_audio_component_sync_audio_rate(struct device *kdev, int port,
>  	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
>  	struct intel_encoder *intel_encoder;
>  	struct intel_crtc *crtc;
> -	struct drm_display_mode *mode;
> +	struct drm_display_mode *adjusted_mode;
>  	struct i915_audio_component *acomp = dev_priv-
> >audio_component;
> -	u32 tmp;
> -	int n;
>  	int err = 0;
> 
>  	/* HSW, BDW, SKL, KBL need this fix */ @@ -700,33 +698,12 @@
> static int i915_audio_component_sync_audio_rate(struct device *kdev, int
> port,
>  	crtc = to_intel_crtc(intel_encoder->base.crtc);
>  	pipe = crtc->pipe;
> 
> -	mode = &crtc->config->base.adjusted_mode;
> +	adjusted_mode = &crtc->config->base.adjusted_mode;
> 
>  	/* port must be valid now, otherwise the pipe will be invalid */
>  	acomp->aud_sample_rate[port] = rate;
> 
> -	/* 2. check whether to set the N/CTS/M manually or not */
> -	if (!audio_rate_need_prog(crtc, mode)) {
> -		tmp = I915_READ(HSW_AUD_CFG(pipe));
> -		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> -		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> -		goto unlock;
> -	}
> -
> -	n = audio_config_get_n(mode, rate);
> -	if (n == 0) {
> -		DRM_DEBUG_KMS("Using automatic mode for N value on
> port %c\n",
> -					  port_name(port));
> -		tmp = I915_READ(HSW_AUD_CFG(pipe));
> -		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> -		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> -		goto unlock;
> -	}
> -
> -	/* 3. set the N/CTS/M */
> -	tmp = I915_READ(HSW_AUD_CFG(pipe));
> -	tmp = audio_config_setup_n_reg(n, tmp);
> -	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> +	hsw_audio_config_update(crtc, port, adjusted_mode);
> 
>   unlock:
>  	mutex_unlock(&dev_priv->av_mutex);
> --
> 2.1.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

[PATCH RESEND 4/9] drm/i915/audio: split dp and hdmi audio config update

[Jani Nikula]

The code for dp and hdmi are already different, and they're about to
diverge even more. Split them for clarity in future work. No functional
changes.

Cc: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 55 +++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 4d62b3e8ac19..db7fd6d8333f 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -148,18 +148,6 @@ static uint32_t audio_config_setup_n_reg(int n, uint32_t val)
 	return tmp;
 }
 
-/* check whether N/CTS/M need be set manually */
-static bool audio_rate_need_prog(struct intel_crtc *crtc,
-				 const struct drm_display_mode *mode)
-{
-	if (((mode->clock == TMDS_297M) ||
-		 (mode->clock == TMDS_296M)) &&
-		intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
-		return true;
-	else
-		return false;
-}
-
 static bool intel_eld_uptodate(struct drm_connector *connector,
 			       i915_reg_t reg_eldv, uint32_t bits_eldv,
 			       i915_reg_t reg_elda, uint32_t bits_elda,
@@ -245,9 +233,26 @@ static void g4x_audio_codec_enable(struct drm_connector *connector,
 	I915_WRITE(G4X_AUD_CNTL_ST, tmp);
 }
 
-static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
-				    enum port port,
-				    const struct drm_display_mode *adjusted_mode)
+static void
+hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
+			   const struct drm_display_mode *adjusted_mode)
+{
+	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+	enum pipe pipe = intel_crtc->pipe;
+	u32 tmp;
+
+	tmp = I915_READ(HSW_AUD_CFG(pipe));
+	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
+	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
+	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+	tmp |= AUD_CONFIG_N_VALUE_INDEX;
+
+	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+}
+
+static void
+hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
+			     const struct drm_display_mode *adjusted_mode)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
 	struct i915_audio_component *acomp = dev_priv->audio_component;
@@ -259,13 +264,11 @@ static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
 	tmp = I915_READ(HSW_AUD_CFG(pipe));
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
 	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
-	if (intel_crtc_has_dp_encoder(intel_crtc->config))
-		tmp |= AUD_CONFIG_N_VALUE_INDEX;
-	else
-		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
-
 	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
-	if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
+	tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
+
+	if (adjusted_mode->clock == TMDS_296M ||
+	    adjusted_mode->clock == TMDS_297M) {
 		n = audio_config_get_n(adjusted_mode, rate);
 		if (n != 0)
 			tmp = audio_config_setup_n_reg(n, tmp);
@@ -276,6 +279,16 @@ static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
 	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
 }
 
+static void
+hsw_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
+			const struct drm_display_mode *adjusted_mode)
+{
+	if (intel_crtc_has_dp_encoder(intel_crtc->config))
+		hsw_dp_audio_config_update(intel_crtc, port, adjusted_mode);
+	else
+		hsw_hdmi_audio_config_update(intel_crtc, port, adjusted_mode);
+}
+
 static void hsw_audio_codec_disable(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-- 
2.1.4

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

Re: [PATCH RESEND 4/9] drm/i915/audio: split dp and hdmi audio config update

[Yang, Libin]

Reviewed-by: Libin Yang <libin.yang@intel.com>

Regards,
Libin


> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [PATCH RESEND 4/9] drm/i915/audio: split dp and hdmi
> audio config update
> 
> The code for dp and hdmi are already different, and they're about to diverge
> even more. Split them for clarity in future work. No functional changes.
> 
> Cc: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 55 +++++++++++++++++++++++----------
> -----
>  1 file changed, 34 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 4d62b3e8ac19..db7fd6d8333f 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -148,18 +148,6 @@ static uint32_t audio_config_setup_n_reg(int n,
> uint32_t val)
>  	return tmp;
>  }
> 
> -/* check whether N/CTS/M need be set manually */ -static bool
> audio_rate_need_prog(struct intel_crtc *crtc,
> -				 const struct drm_display_mode *mode)
> -{
> -	if (((mode->clock == TMDS_297M) ||
> -		 (mode->clock == TMDS_296M)) &&
> -		intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
> -		return true;
> -	else
> -		return false;
> -}
> -
>  static bool intel_eld_uptodate(struct drm_connector *connector,
>  			       i915_reg_t reg_eldv, uint32_t bits_eldv,
>  			       i915_reg_t reg_elda, uint32_t bits_elda, @@ -
> 245,9 +233,26 @@ static void g4x_audio_codec_enable(struct
> drm_connector *connector,
>  	I915_WRITE(G4X_AUD_CNTL_ST, tmp);
>  }
> 
> -static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
> -				    enum port port,
> -				    const struct drm_display_mode
> *adjusted_mode)
> +static void
> +hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
> +			   const struct drm_display_mode *adjusted_mode) {
> +	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> +	enum pipe pipe = intel_crtc->pipe;
> +	u32 tmp;
> +
> +	tmp = I915_READ(HSW_AUD_CFG(pipe));
> +	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> +	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> +	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> +	tmp |= AUD_CONFIG_N_VALUE_INDEX;
> +
> +	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> +}
> +
> +static void
> +hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port
> port,
> +			     const struct drm_display_mode *adjusted_mode)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
>  	struct i915_audio_component *acomp = dev_priv-
> >audio_component; @@ -259,13 +264,11 @@ static void
> hsw_audio_config_update(struct intel_crtc *intel_crtc,
>  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>  	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> -		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> -	else
> -		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> -
>  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> -	if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
> +	tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> +
> +	if (adjusted_mode->clock == TMDS_296M ||
> +	    adjusted_mode->clock == TMDS_297M) {
>  		n = audio_config_get_n(adjusted_mode, rate);
>  		if (n != 0)
>  			tmp = audio_config_setup_n_reg(n, tmp); @@ -276,6
> +279,16 @@ static void hsw_audio_config_update(struct intel_crtc
> *intel_crtc,
>  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>  }
> 
> +static void
> +hsw_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
> +			const struct drm_display_mode *adjusted_mode) {
> +	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> +		hsw_dp_audio_config_update(intel_crtc, port,
> adjusted_mode);
> +	else
> +		hsw_hdmi_audio_config_update(intel_crtc, port,
> adjusted_mode); }
> +
>  static void hsw_audio_codec_disable(struct intel_encoder *encoder)  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> --
> 2.1.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

[PATCH RESEND 5/9] drm/i915/audio: set proper N/MCTS on more platforms

[Jani Nikula]

From: Libin Yang <libin.yang@linux.intel.com>

This patch applies setting proper N/M, N/CTS on more platforms.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index db7fd6d8333f..bce3ad2eb86d 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -688,11 +688,7 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
 	struct i915_audio_component *acomp = dev_priv->audio_component;
 	int err = 0;
 
-	/* HSW, BDW, SKL, KBL need this fix */
-	if (!IS_SKYLAKE(dev_priv) &&
-	    !IS_KABYLAKE(dev_priv) &&
-	    !IS_BROADWELL(dev_priv) &&
-	    !IS_HASWELL(dev_priv))
+	if (!HAS_DDI(dev_priv))
 		return 0;
 
 	i915_audio_component_get_power(kdev);
-- 
2.1.4

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

[PATCH RESEND 6/9] drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock

[Jani Nikula]

From: Libin Yang <libin.yang@linux.intel.com>

HDMI audio should use crtc_clock to get the TMDS clock.

This patch renames mode to adjusted_mode to unify the name.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index bce3ad2eb86d..6aa619a84439 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -121,13 +121,14 @@ static u32 audio_config_hdmi_pixel_clock(const struct drm_display_mode *adjusted
 	return hdmi_audio_clock[i].config;
 }
 
-static int audio_config_get_n(const struct drm_display_mode *mode, int rate)
+static int audio_config_get_n(const struct drm_display_mode *adjusted_mode,
+			      int rate)
 {
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
 		if ((rate == aud_ncts[i].sample_rate) &&
-			(mode->clock == aud_ncts[i].clock)) {
+			(adjusted_mode->crtc_clock == aud_ncts[i].clock)) {
 			return aud_ncts[i].n;
 		}
 	}
@@ -267,8 +268,8 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
 	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
 	tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
 
-	if (adjusted_mode->clock == TMDS_296M ||
-	    adjusted_mode->clock == TMDS_297M) {
+	if (adjusted_mode->crtc_clock == TMDS_296M ||
+	    adjusted_mode->crtc_clock == TMDS_297M) {
 		n = audio_config_get_n(adjusted_mode, rate);
 		if (n != 0)
 			tmp = audio_config_setup_n_reg(n, tmp);
-- 
2.1.4

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

[PATCH RESEND 7/9] drm/i915/audio: add register macros for audio config N value

[Jani Nikula]

Have generic macros in line with the rest of the register bit definition
macros instead of a dedicated function in intel_audio.c, and use them.
No functional changes.

Cc: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  4 ++++
 drivers/gpu/drm/i915/intel_audio.c | 23 ++++++-----------------
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index acc767a52d8e..595d196f753f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7332,6 +7332,10 @@ enum {
 #define   AUD_CONFIG_UPPER_N_MASK		(0xff << 20)
 #define   AUD_CONFIG_LOWER_N_SHIFT		4
 #define   AUD_CONFIG_LOWER_N_MASK		(0xfff << 4)
+#define   AUD_CONFIG_N_MASK			(AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK)
+#define   AUD_CONFIG_N(n) \
+	(((((n) >> 12) & 0xff) << AUD_CONFIG_UPPER_N_SHIFT) |	\
+	 (((n) & 0xfff) << AUD_CONFIG_LOWER_N_SHIFT))
 #define   AUD_CONFIG_PIXEL_CLOCK_HDMI_SHIFT	16
 #define   AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK	(0xf << 16)
 #define   AUD_CONFIG_PIXEL_CLOCK_HDMI_25175	(0 << 16)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 6aa619a84439..d2c6227f72b8 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -135,20 +135,6 @@ static int audio_config_get_n(const struct drm_display_mode *adjusted_mode,
 	return 0;
 }
 
-static uint32_t audio_config_setup_n_reg(int n, uint32_t val)
-{
-	int n_low, n_up;
-	uint32_t tmp = val;
-
-	n_low = n & 0xfff;
-	n_up = (n >> 12) & 0xff;
-	tmp &= ~(AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK);
-	tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
-			(n_low << AUD_CONFIG_LOWER_N_SHIFT) |
-			AUD_CONFIG_N_PROG_ENABLE);
-	return tmp;
-}
-
 static bool intel_eld_uptodate(struct drm_connector *connector,
 			       i915_reg_t reg_eldv, uint32_t bits_eldv,
 			       i915_reg_t reg_elda, uint32_t bits_elda,
@@ -271,10 +257,13 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
 	if (adjusted_mode->crtc_clock == TMDS_296M ||
 	    adjusted_mode->crtc_clock == TMDS_297M) {
 		n = audio_config_get_n(adjusted_mode, rate);
-		if (n != 0)
-			tmp = audio_config_setup_n_reg(n, tmp);
-		else
+		if (n != 0) {
+			tmp &= ~AUD_CONFIG_N_MASK;
+			tmp |= AUD_CONFIG_N(n);
+			tmp |= AUD_CONFIG_N_PROG_ENABLE;
+		} else {
 			DRM_DEBUG_KMS("no suitable N value is found\n");
+		}
 	}
 
 	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
-- 
2.1.4

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

Re: [PATCH RESEND 7/9] drm/i915/audio: add register macros for audio config N value

[Yang, Libin]

Reviewed-by: Libin Yang <libin.yang@intel.com>

Regards,
Libin


> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [PATCH RESEND 7/9] drm/i915/audio: add register macros
> for audio config N value
> 
> Have generic macros in line with the rest of the register bit definition macros
> instead of a dedicated function in intel_audio.c, and use them.
> No functional changes.
> 
> Cc: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  4 ++++
>  drivers/gpu/drm/i915/intel_audio.c | 23 ++++++-----------------
>  2 files changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index acc767a52d8e..595d196f753f
> 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7332,6 +7332,10 @@ enum {
>  #define   AUD_CONFIG_UPPER_N_MASK		(0xff << 20)
>  #define   AUD_CONFIG_LOWER_N_SHIFT		4
>  #define   AUD_CONFIG_LOWER_N_MASK		(0xfff << 4)
> +#define   AUD_CONFIG_N_MASK
> 	(AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK)
> +#define   AUD_CONFIG_N(n) \
> +	(((((n) >> 12) & 0xff) << AUD_CONFIG_UPPER_N_SHIFT) |	\
> +	 (((n) & 0xfff) << AUD_CONFIG_LOWER_N_SHIFT))
>  #define   AUD_CONFIG_PIXEL_CLOCK_HDMI_SHIFT	16
>  #define   AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK	(0xf << 16)
>  #define   AUD_CONFIG_PIXEL_CLOCK_HDMI_25175	(0 << 16)
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 6aa619a84439..d2c6227f72b8 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -135,20 +135,6 @@ static int audio_config_get_n(const struct
> drm_display_mode *adjusted_mode,
>  	return 0;
>  }
> 
> -static uint32_t audio_config_setup_n_reg(int n, uint32_t val) -{
> -	int n_low, n_up;
> -	uint32_t tmp = val;
> -
> -	n_low = n & 0xfff;
> -	n_up = (n >> 12) & 0xff;
> -	tmp &= ~(AUD_CONFIG_UPPER_N_MASK |
> AUD_CONFIG_LOWER_N_MASK);
> -	tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
> -			(n_low << AUD_CONFIG_LOWER_N_SHIFT) |
> -			AUD_CONFIG_N_PROG_ENABLE);
> -	return tmp;
> -}
> -
>  static bool intel_eld_uptodate(struct drm_connector *connector,
>  			       i915_reg_t reg_eldv, uint32_t bits_eldv,
>  			       i915_reg_t reg_elda, uint32_t bits_elda, @@ -
> 271,10 +257,13 @@ hsw_hdmi_audio_config_update(struct intel_crtc
> *intel_crtc, enum port port,
>  	if (adjusted_mode->crtc_clock == TMDS_296M ||
>  	    adjusted_mode->crtc_clock == TMDS_297M) {
>  		n = audio_config_get_n(adjusted_mode, rate);
> -		if (n != 0)
> -			tmp = audio_config_setup_n_reg(n, tmp);
> -		else
> +		if (n != 0) {
> +			tmp &= ~AUD_CONFIG_N_MASK;
> +			tmp |= AUD_CONFIG_N(n);
> +			tmp |= AUD_CONFIG_N_PROG_ENABLE;
> +		} else {
>  			DRM_DEBUG_KMS("no suitable N value is found\n");
> +		}
>  	}
> 
>  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> --
> 2.1.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

[PATCH RESEND 8/9] drm/i915/audio: rename N value getter to emphasize it's for hdmi

[Jani Nikula]

We'll be getting a function and a table for dp parameters soon enough,
so rename the function and table for hdmi. No functional changes.

Cc: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index d2c6227f72b8..81df29ca4947 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -81,7 +81,7 @@ static const struct {
 	int clock;
 	int n;
 	int cts;
-} aud_ncts[] = {
+} hdmi_aud_ncts[] = {
 	{ 44100, TMDS_296M, 4459, 234375 },
 	{ 44100, TMDS_297M, 4704, 247500 },
 	{ 48000, TMDS_296M, 5824, 281250 },
@@ -121,15 +121,15 @@ static u32 audio_config_hdmi_pixel_clock(const struct drm_display_mode *adjusted
 	return hdmi_audio_clock[i].config;
 }
 
-static int audio_config_get_n(const struct drm_display_mode *adjusted_mode,
-			      int rate)
+static int audio_config_hdmi_get_n(const struct drm_display_mode *adjusted_mode,
+				   int rate)
 {
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
-		if ((rate == aud_ncts[i].sample_rate) &&
-			(adjusted_mode->crtc_clock == aud_ncts[i].clock)) {
-			return aud_ncts[i].n;
+	for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
+		if (rate == hdmi_aud_ncts[i].sample_rate &&
+		    adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
+			return hdmi_aud_ncts[i].n;
 		}
 	}
 	return 0;
@@ -256,7 +256,7 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
 
 	if (adjusted_mode->crtc_clock == TMDS_296M ||
 	    adjusted_mode->crtc_clock == TMDS_297M) {
-		n = audio_config_get_n(adjusted_mode, rate);
+		n = audio_config_hdmi_get_n(adjusted_mode, rate);
 		if (n != 0) {
 			tmp &= ~AUD_CONFIG_N_MASK;
 			tmp |= AUD_CONFIG_N(n);
-- 
2.1.4

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

Re: [PATCH RESEND 8/9] drm/i915/audio: rename N value getter to emphasize it's for hdmi

[Yang, Libin]

Reviewed-by: Libin Yang <libin.yang@intel.com>

Regards,
Libin


> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [PATCH RESEND 8/9] drm/i915/audio: rename N value
> getter to emphasize it's for hdmi
> 
> We'll be getting a function and a table for dp parameters soon enough, so
> rename the function and table for hdmi. No functional changes.
> 
> Cc: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index d2c6227f72b8..81df29ca4947 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -81,7 +81,7 @@ static const struct {
>  	int clock;
>  	int n;
>  	int cts;
> -} aud_ncts[] = {
> +} hdmi_aud_ncts[] = {
>  	{ 44100, TMDS_296M, 4459, 234375 },
>  	{ 44100, TMDS_297M, 4704, 247500 },
>  	{ 48000, TMDS_296M, 5824, 281250 },
> @@ -121,15 +121,15 @@ static u32 audio_config_hdmi_pixel_clock(const
> struct drm_display_mode *adjusted
>  	return hdmi_audio_clock[i].config;
>  }
> 
> -static int audio_config_get_n(const struct drm_display_mode
> *adjusted_mode,
> -			      int rate)
> +static int audio_config_hdmi_get_n(const struct drm_display_mode
> *adjusted_mode,
> +				   int rate)
>  {
>  	int i;
> 
> -	for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> -		if ((rate == aud_ncts[i].sample_rate) &&
> -			(adjusted_mode->crtc_clock == aud_ncts[i].clock)) {
> -			return aud_ncts[i].n;
> +	for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
> +		if (rate == hdmi_aud_ncts[i].sample_rate &&
> +		    adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
> +			return hdmi_aud_ncts[i].n;
>  		}
>  	}
>  	return 0;
> @@ -256,7 +256,7 @@ hsw_hdmi_audio_config_update(struct intel_crtc
> *intel_crtc, enum port port,
> 
>  	if (adjusted_mode->crtc_clock == TMDS_296M ||
>  	    adjusted_mode->crtc_clock == TMDS_297M) {
> -		n = audio_config_get_n(adjusted_mode, rate);
> +		n = audio_config_hdmi_get_n(adjusted_mode, rate);
>  		if (n != 0) {
>  			tmp &= ~AUD_CONFIG_N_MASK;
>  			tmp |= AUD_CONFIG_N(n);
> --
> 2.1.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 RESEND 8/9] drm/i915/audio: rename N value getter to emphasize it's for hdmi

[Jani Nikula]

On Tue, 11 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> Reviewed-by: Libin Yang <libin.yang@intel.com>

Pushed patches 1-8 to drm-intel-next-queued, thanks for the review.

BR,
Jani.

>
> Regards,
> Libin
>
>
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>> Jani Nikula
>> Sent: Monday, October 10, 2016 11:04 PM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
>> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
>> Subject: [Intel-gfx] [PATCH RESEND 8/9] drm/i915/audio: rename N value
>> getter to emphasize it's for hdmi
>> 
>> We'll be getting a function and a table for dp parameters soon enough, so
>> rename the function and table for hdmi. No functional changes.
>> 
>> Cc: Libin Yang <libin.yang@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_audio.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> b/drivers/gpu/drm/i915/intel_audio.c
>> index d2c6227f72b8..81df29ca4947 100644
>> --- a/drivers/gpu/drm/i915/intel_audio.c
>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>> @@ -81,7 +81,7 @@ static const struct {
>>  	int clock;
>>  	int n;
>>  	int cts;
>> -} aud_ncts[] = {
>> +} hdmi_aud_ncts[] = {
>>  	{ 44100, TMDS_296M, 4459, 234375 },
>>  	{ 44100, TMDS_297M, 4704, 247500 },
>>  	{ 48000, TMDS_296M, 5824, 281250 },
>> @@ -121,15 +121,15 @@ static u32 audio_config_hdmi_pixel_clock(const
>> struct drm_display_mode *adjusted
>>  	return hdmi_audio_clock[i].config;
>>  }
>> 
>> -static int audio_config_get_n(const struct drm_display_mode
>> *adjusted_mode,
>> -			      int rate)
>> +static int audio_config_hdmi_get_n(const struct drm_display_mode
>> *adjusted_mode,
>> +				   int rate)
>>  {
>>  	int i;
>> 
>> -	for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
>> -		if ((rate == aud_ncts[i].sample_rate) &&
>> -			(adjusted_mode->crtc_clock == aud_ncts[i].clock)) {
>> -			return aud_ncts[i].n;
>> +	for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
>> +		if (rate == hdmi_aud_ncts[i].sample_rate &&
>> +		    adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
>> +			return hdmi_aud_ncts[i].n;
>>  		}
>>  	}
>>  	return 0;
>> @@ -256,7 +256,7 @@ hsw_hdmi_audio_config_update(struct intel_crtc
>> *intel_crtc, enum port port,
>> 
>>  	if (adjusted_mode->crtc_clock == TMDS_296M ||
>>  	    adjusted_mode->crtc_clock == TMDS_297M) {
>> -		n = audio_config_get_n(adjusted_mode, rate);
>> +		n = audio_config_hdmi_get_n(adjusted_mode, rate);
>>  		if (n != 0) {
>>  			tmp &= ~AUD_CONFIG_N_MASK;
>>  			tmp |= AUD_CONFIG_N(n);
>> --
>> 2.1.4
>> 
>> _______________________________________________
>> 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

[PATCH RESEND 9/9] drm/i915: set proper N/M in modeset

[Jani Nikula]

When modeset occurs and the LS_CLK is set to some
special values in DP mode, the N/M need to be set
manually if audio is playing. Otherwise the first
several seconds may be silent in audio playback.

The relationship of Maud and Naud is expressed in
the following equation:
Maud/Naud = 512 * fs / f_LS_Clk

Please refer VESA DisplayPort Standard spec for details.

Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |   7 +++
 drivers/gpu/drm/i915/intel_audio.c | 100 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 595d196f753f..8d9dbc7d5b32 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7359,6 +7359,13 @@ enum {
 #define _HSW_AUD_MISC_CTRL_B		0x65110
 #define HSW_AUD_MISC_CTRL(pipe)		_MMIO_PIPE(pipe, _HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
 
+#define _HSW_AUD_M_CTS_ENABLE_A		0x65028
+#define _HSW_AUD_M_CTS_ENABLE_B		0x65128
+#define HSW_AUD_M_CTS_ENABLE(pipe)	_MMIO_PIPE(pipe, _HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
+#define   AUD_M_CTS_M_VALUE_INDEX	(1 << 21)
+#define   AUD_M_CTS_M_PROG_ENABLE	(1 << 20)
+#define   AUD_CONFIG_M_MASK		0xfffff
+
 #define _HSW_AUD_DIP_ELD_CTRL_ST_A	0x650b4
 #define _HSW_AUD_DIP_ELD_CTRL_ST_B	0x651b4
 #define HSW_AUD_DIP_ELD_CTRL(pipe)	_MMIO_PIPE(pipe, _HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 81df29ca4947..0bc2701b6c9c 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -57,6 +57,70 @@
  * struct &i915_audio_component_audio_ops @audio_ops is called from i915 driver.
  */
 
+/* DP N/M table */
+#define LC_540M 540000
+#define LC_270M 270000
+#define LC_162M 162000
+
+struct dp_aud_n_m {
+	int sample_rate;
+	int clock;
+	u16 n;
+	u16 m;
+};
+
+static const struct dp_aud_n_m dp_aud_n_m[] = {
+	{ 192000, LC_540M, 5625, 1024 },
+	{ 176400, LC_540M, 9375, 1568 },
+	{ 96000, LC_540M, 5625, 512 },
+	{ 88200, LC_540M, 9375, 784 },
+	{ 48000, LC_540M, 5625, 256 },
+	{ 44100, LC_540M, 9375, 392 },
+	{ 32000, LC_540M, 16875, 512 },
+	{ 192000, LC_270M, 5625, 2048 },
+	{ 176400, LC_270M, 9375, 3136 },
+	{ 96000, LC_270M, 5625, 1024 },
+	{ 88200, LC_270M, 9375, 1568 },
+	{ 48000, LC_270M, 5625, 512 },
+	{ 44100, LC_270M, 9375, 784 },
+	{ 32000, LC_270M, 16875, 1024 },
+	{ 192000, LC_162M, 3375, 2048 },
+	{ 176400, LC_162M, 5625, 3136 },
+	{ 96000, LC_162M, 3375, 1024 },
+	{ 88200, LC_162M, 5625, 1568 },
+	{ 48000, LC_162M, 3375, 512 },
+	{ 44100, LC_162M, 5625, 784 },
+	{ 32000, LC_162M, 10125, 1024 },
+};
+
+static const struct dp_aud_n_m *
+audio_config_dp_get_n_m(struct intel_crtc *intel_crtc, int rate)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
+		if (rate == dp_aud_n_m[i].sample_rate &&
+		    intel_crtc->config->port_clock == dp_aud_n_m[i].clock)
+			return &dp_aud_n_m[i];
+	}
+
+	return NULL;
+}
+
+static int audio_config_dp_get_m(struct intel_crtc *intel_crtc, int rate)
+{
+	const struct dp_aud_n_m *nm = audio_config_dp_get_n_m(intel_crtc, rate);
+
+	return nm ? nm->m : 0;
+}
+
+static int audio_config_dp_get_n(struct intel_crtc *intel_crtc, int rate)
+{
+	const struct dp_aud_n_m *nm = audio_config_dp_get_n_m(intel_crtc, rate);
+
+	return nm ? nm->n : 0;
+}
+
 static const struct {
 	int clock;
 	u32 config;
@@ -225,8 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
 			   const struct drm_display_mode *adjusted_mode)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+	struct i915_audio_component *acomp = dev_priv->audio_component;
+	int rate = acomp ? acomp->aud_sample_rate[port] : 0;
 	enum pipe pipe = intel_crtc->pipe;
-	u32 tmp;
+	u32 tmp, n, m;
 
 	tmp = I915_READ(HSW_AUD_CFG(pipe));
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
@@ -234,7 +300,30 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
 	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
 	tmp |= AUD_CONFIG_N_VALUE_INDEX;
 
+	if (intel_crtc->config->port_clock == LC_540M ||
+	    intel_crtc->config->port_clock == LC_270M ||
+	    intel_crtc->config->port_clock == LC_162M) {
+		n = audio_config_dp_get_n(intel_crtc, rate);
+		if (n != 0) {
+			tmp &= ~AUD_CONFIG_N_MASK;
+			tmp |= AUD_CONFIG_N(n);
+			tmp |= AUD_CONFIG_N_PROG_ENABLE;
+		} else {
+			DRM_DEBUG_KMS("no suitable N value is found\n");
+		}
+	}
+
 	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+
+	m = audio_config_dp_get_m(intel_crtc, rate);
+	if (m) {
+		tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
+		tmp &= ~AUD_CONFIG_M_MASK;
+		tmp |= m;
+		tmp |= AUD_M_CTS_M_VALUE_INDEX;
+		tmp |= AUD_M_CTS_M_PROG_ENABLE;
+		I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
+	}
 }
 
 static void
@@ -267,6 +356,12 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
 	}
 
 	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+
+	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
+	tmp &= ~AUD_CONFIG_M_MASK;
+	tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
+	tmp |= AUD_M_CTS_M_PROG_ENABLE;
+	I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
 }
 
 static void
@@ -687,7 +782,8 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
 	/* 1. get the pipe */
 	intel_encoder = get_saved_enc(dev_priv, port, pipe);
 	if (!intel_encoder || !intel_encoder->base.crtc ||
-	    intel_encoder->type != INTEL_OUTPUT_HDMI) {
+	    (intel_encoder->type != INTEL_OUTPUT_HDMI &&
+	     intel_encoder->type != INTEL_OUTPUT_DP)) {
 		DRM_DEBUG_KMS("Not valid for port %c\n", port_name(port));
 		err = -ENODEV;
 		goto unlock;
-- 
2.1.4

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

Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset

[Lin, Mengdong]

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> modeset
> 
> When modeset occurs and the LS_CLK is set to some special values in DP
> mode, the N/M need to be set manually if audio is playing. Otherwise the
> first several seconds may be silent in audio playback.
> 
> The relationship of Maud and Naud is expressed in the following equation:
> Maud/Naud = 512 * fs / f_LS_Clk
> 
> Please refer VESA DisplayPort Standard spec for details.
> 
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |   7 +++
>  drivers/gpu/drm/i915/intel_audio.c | 100
> ++++++++++++++++++++++++++++++++++++-
>  2 files changed, 105 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index 595d196f753f..8d9dbc7d5b32
> 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7359,6 +7359,13 @@ enum {
>  #define _HSW_AUD_MISC_CTRL_B		0x65110
>  #define HSW_AUD_MISC_CTRL(pipe)		_MMIO_PIPE(pipe,
> _HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
> 
> +#define _HSW_AUD_M_CTS_ENABLE_A		0x65028
> +#define _HSW_AUD_M_CTS_ENABLE_B		0x65128
> +#define HSW_AUD_M_CTS_ENABLE(pipe)	_MMIO_PIPE(pipe,
> _HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
> +#define   AUD_M_CTS_M_VALUE_INDEX	(1 << 21)
> +#define   AUD_M_CTS_M_PROG_ENABLE	(1 << 20)
> +#define   AUD_CONFIG_M_MASK		0xfffff

The last line cause misalignment after applying the patch.

> +
>  #define _HSW_AUD_DIP_ELD_CTRL_ST_A	0x650b4
>  #define _HSW_AUD_DIP_ELD_CTRL_ST_B	0x651b4
>  #define HSW_AUD_DIP_ELD_CTRL(pipe)	_MMIO_PIPE(pipe,
> _HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B)
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 81df29ca4947..0bc2701b6c9c 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -57,6 +57,70 @@
>   * struct &i915_audio_component_audio_ops @audio_ops is called from
> i915 driver.
>   */
> 
> +/* DP N/M table */
> +#define LC_540M 540000
> +#define LC_270M 270000
> +#define LC_162M 162000
> +
> +struct dp_aud_n_m {
> +	int sample_rate;
> +	int clock;
> +	u16 n;
> +	u16 m;
> +};
> +
> +static const struct dp_aud_n_m dp_aud_n_m[] = {
> +	{ 192000, LC_540M, 5625, 1024 },
> +	{ 176400, LC_540M, 9375, 1568 },
> +	{ 96000, LC_540M, 5625, 512 },
> +	{ 88200, LC_540M, 9375, 784 },
> +	{ 48000, LC_540M, 5625, 256 },
> +	{ 44100, LC_540M, 9375, 392 },
> +	{ 32000, LC_540M, 16875, 512 },
> +	{ 192000, LC_270M, 5625, 2048 },
> +	{ 176400, LC_270M, 9375, 3136 },
> +	{ 96000, LC_270M, 5625, 1024 },
> +	{ 88200, LC_270M, 9375, 1568 },
> +	{ 48000, LC_270M, 5625, 512 },
> +	{ 44100, LC_270M, 9375, 784 },
> +	{ 32000, LC_270M, 16875, 1024 },
> +	{ 192000, LC_162M, 3375, 2048 },
> +	{ 176400, LC_162M, 5625, 3136 },
> +	{ 96000, LC_162M, 3375, 1024 },
> +	{ 88200, LC_162M, 5625, 1568 },
> +	{ 48000, LC_162M, 3375, 512 },
> +	{ 44100, LC_162M, 5625, 784 },
> +	{ 32000, LC_162M, 10125, 1024 },
> +};
> +
> +static const struct dp_aud_n_m *
> +audio_config_dp_get_n_m(struct intel_crtc *intel_crtc, int rate) {
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
> +		if (rate == dp_aud_n_m[i].sample_rate &&
> +		    intel_crtc->config->port_clock == dp_aud_n_m[i].clock)
> +			return &dp_aud_n_m[i];
> +	}
> +
> +	return NULL;
> +}
> +
> +static int audio_config_dp_get_m(struct intel_crtc *intel_crtc, int
> +rate) {
> +	const struct dp_aud_n_m *nm =
> audio_config_dp_get_n_m(intel_crtc,
> +rate);
> +
> +	return nm ? nm->m : 0;
> +}
> +
> +static int audio_config_dp_get_n(struct intel_crtc *intel_crtc, int
> +rate) {
> +	const struct dp_aud_n_m *nm =
> audio_config_dp_get_n_m(intel_crtc,
> +rate);
> +
> +	return nm ? nm->n : 0;
> +}
> +
>  static const struct {
>  	int clock;
>  	u32 config;
> @@ -225,8 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc
> *intel_crtc, enum port port,
>  			   const struct drm_display_mode *adjusted_mode)  {
>  	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> +	struct i915_audio_component *acomp = dev_priv-
> >audio_component;
> +	int rate = acomp ? acomp->aud_sample_rate[port] : 0;
>  	enum pipe pipe = intel_crtc->pipe;
> -	u32 tmp;
> +	u32 tmp, n, m;
> 
>  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> @@ -234,7 +300,30 @@ hsw_dp_audio_config_update(struct intel_crtc
> *intel_crtc, enum port port,
>  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>  	tmp |= AUD_CONFIG_N_VALUE_INDEX;
> 
> +	if (intel_crtc->config->port_clock == LC_540M ||
> +	    intel_crtc->config->port_clock == LC_270M ||
> +	    intel_crtc->config->port_clock == LC_162M) {
> +		n = audio_config_dp_get_n(intel_crtc, rate);
> +		if (n != 0) {
> +			tmp &= ~AUD_CONFIG_N_MASK;
> +			tmp |= AUD_CONFIG_N(n);
> +			tmp |= AUD_CONFIG_N_PROG_ENABLE;
> +		} else {
> +			DRM_DEBUG_KMS("no suitable N value is found\n");

Could some comments be added here why we don't return if no valid N value is found?
Do we still need to program write the register to let HW calculate N by itself?

> +		}
> +	}
> +
>  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);

And if no valid N value is found, need we go ahead to check M value?

> +
> +	m = audio_config_dp_get_m(intel_crtc, rate);
> +	if (m) {
> +		tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
> +		tmp &= ~AUD_CONFIG_M_MASK;
> +		tmp |= m;
> +		tmp |= AUD_M_CTS_M_VALUE_INDEX;
> +		tmp |= AUD_M_CTS_M_PROG_ENABLE;
> +		I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
> +	}
>  }

Thanks
Mengdong
 
>  static void
> @@ -267,6 +356,12 @@ hsw_hdmi_audio_config_update(struct intel_crtc
> *intel_crtc, enum port port,
>  	}
> 
>  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> +
> +	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
> +	tmp &= ~AUD_CONFIG_M_MASK;
> +	tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
> +	tmp |= AUD_M_CTS_M_PROG_ENABLE;
> +	I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
>  }
> 
>  static void
> @@ -687,7 +782,8 @@ static int
> i915_audio_component_sync_audio_rate(struct device *kdev, int port,
>  	/* 1. get the pipe */
>  	intel_encoder = get_saved_enc(dev_priv, port, pipe);
>  	if (!intel_encoder || !intel_encoder->base.crtc ||
> -	    intel_encoder->type != INTEL_OUTPUT_HDMI) {
> +	    (intel_encoder->type != INTEL_OUTPUT_HDMI &&
> +	     intel_encoder->type != INTEL_OUTPUT_DP)) {
>  		DRM_DEBUG_KMS("Not valid for port %c\n",
> port_name(port));
>  		err = -ENODEV;
>  		goto unlock;
> --
> 2.1.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 RESEND 9/9] drm/i915: set proper N/M in modeset

[Yang, Libin]

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Lin, Mengdong
> Sent: Wednesday, October 12, 2016 10:46 AM
> To: Nikula, Jani <jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: Re: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> modeset
> 
> 
> 
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
> > Behalf Of Jani Nikula
> > Sent: Monday, October 10, 2016 11:04 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> > Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> > Subject: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> > modeset
> >
> > When modeset occurs and the LS_CLK is set to some special values in DP
> > mode, the N/M need to be set manually if audio is playing. Otherwise
> > the first several seconds may be silent in audio playback.
> >
> > The relationship of Maud and Naud is expressed in the following equation:
> > Maud/Naud = 512 * fs / f_LS_Clk
> >
> > Please refer VESA DisplayPort Standard spec for details.
> >
> > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h    |   7 +++
> >  drivers/gpu/drm/i915/intel_audio.c | 100
> > ++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 105 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 595d196f753f..8d9dbc7d5b32
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7359,6 +7359,13 @@ enum {
> >  #define _HSW_AUD_MISC_CTRL_B		0x65110
> >  #define HSW_AUD_MISC_CTRL(pipe)		_MMIO_PIPE(pipe,
> > _HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
> >
> > +#define _HSW_AUD_M_CTS_ENABLE_A		0x65028
> > +#define _HSW_AUD_M_CTS_ENABLE_B		0x65128
> > +#define HSW_AUD_M_CTS_ENABLE(pipe)	_MMIO_PIPE(pipe,
> > _HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
> > +#define   AUD_M_CTS_M_VALUE_INDEX	(1 << 21)
> > +#define   AUD_M_CTS_M_PROG_ENABLE	(1 << 20)
> > +#define   AUD_CONFIG_M_MASK		0xfffff
> 
> The last line cause misalignment after applying the patch.
> 
> > +
> >  #define _HSW_AUD_DIP_ELD_CTRL_ST_A	0x650b4
> >  #define _HSW_AUD_DIP_ELD_CTRL_ST_B	0x651b4
> >  #define HSW_AUD_DIP_ELD_CTRL(pipe)	_MMIO_PIPE(pipe,
> > _HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B) diff --
> git
> > a/drivers/gpu/drm/i915/intel_audio.c
> > b/drivers/gpu/drm/i915/intel_audio.c
> > index 81df29ca4947..0bc2701b6c9c 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -57,6 +57,70 @@
> >   * struct &i915_audio_component_audio_ops @audio_ops is called from
> > i915 driver.
> >   */
> >
> > +/* DP N/M table */
> > +#define LC_540M 540000
> > +#define LC_270M 270000
> > +#define LC_162M 162000
> > +
> > +struct dp_aud_n_m {
> > +	int sample_rate;
> > +	int clock;
> > +	u16 n;
> > +	u16 m;
> > +};
> > +
> > +static const struct dp_aud_n_m dp_aud_n_m[] = {
> > +	{ 192000, LC_540M, 5625, 1024 },
> > +	{ 176400, LC_540M, 9375, 1568 },
> > +	{ 96000, LC_540M, 5625, 512 },
> > +	{ 88200, LC_540M, 9375, 784 },
> > +	{ 48000, LC_540M, 5625, 256 },
> > +	{ 44100, LC_540M, 9375, 392 },
> > +	{ 32000, LC_540M, 16875, 512 },
> > +	{ 192000, LC_270M, 5625, 2048 },
> > +	{ 176400, LC_270M, 9375, 3136 },
> > +	{ 96000, LC_270M, 5625, 1024 },
> > +	{ 88200, LC_270M, 9375, 1568 },
> > +	{ 48000, LC_270M, 5625, 512 },
> > +	{ 44100, LC_270M, 9375, 784 },
> > +	{ 32000, LC_270M, 16875, 1024 },
> > +	{ 192000, LC_162M, 3375, 2048 },
> > +	{ 176400, LC_162M, 5625, 3136 },
> > +	{ 96000, LC_162M, 3375, 1024 },
> > +	{ 88200, LC_162M, 5625, 1568 },
> > +	{ 48000, LC_162M, 3375, 512 },
> > +	{ 44100, LC_162M, 5625, 784 },
> > +	{ 32000, LC_162M, 10125, 1024 },
> > +};
> > +
> > +static const struct dp_aud_n_m *
> > +audio_config_dp_get_n_m(struct intel_crtc *intel_crtc, int rate) {
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
> > +		if (rate == dp_aud_n_m[i].sample_rate &&
> > +		    intel_crtc->config->port_clock == dp_aud_n_m[i].clock)
> > +			return &dp_aud_n_m[i];
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> > +static int audio_config_dp_get_m(struct intel_crtc *intel_crtc, int
> > +rate) {
> > +	const struct dp_aud_n_m *nm =
> > audio_config_dp_get_n_m(intel_crtc,
> > +rate);
> > +
> > +	return nm ? nm->m : 0;
> > +}
> > +
> > +static int audio_config_dp_get_n(struct intel_crtc *intel_crtc, int
> > +rate) {
> > +	const struct dp_aud_n_m *nm =
> > audio_config_dp_get_n_m(intel_crtc,
> > +rate);
> > +
> > +	return nm ? nm->n : 0;
> > +}
> > +
> >  static const struct {
> >  	int clock;
> >  	u32 config;
> > @@ -225,8 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc
> > *intel_crtc, enum port port,
> >  			   const struct drm_display_mode *adjusted_mode)  {
> >  	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> > +	struct i915_audio_component *acomp = dev_priv-
> > >audio_component;
> > +	int rate = acomp ? acomp->aud_sample_rate[port] : 0;
> >  	enum pipe pipe = intel_crtc->pipe;
> > -	u32 tmp;
> > +	u32 tmp, n, m;
> >
> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > @@ -234,7 +300,30 @@ hsw_dp_audio_config_update(struct intel_crtc
> > *intel_crtc, enum port port,
> >  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> >  	tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >
> > +	if (intel_crtc->config->port_clock == LC_540M ||
> > +	    intel_crtc->config->port_clock == LC_270M ||
> > +	    intel_crtc->config->port_clock == LC_162M) {
> > +		n = audio_config_dp_get_n(intel_crtc, rate);
> > +		if (n != 0) {
> > +			tmp &= ~AUD_CONFIG_N_MASK;
> > +			tmp |= AUD_CONFIG_N(n);
> > +			tmp |= AUD_CONFIG_N_PROG_ENABLE;
> > +		} else {
> > +			DRM_DEBUG_KMS("no suitable N value is found\n");
> 
> Could some comments be added here why we don't return if no valid N value
> is found?

This is because although we don't need set N value here, but there are some
other bits to be set. So you can see we call 
I915_WRITE(HSW_AUD_CFG(pipe), tmp);
Later. 

> Do we still need to program write the register to let HW calculate N by itself?

Ideally, all the frequency should be in the table. But if the frequency is not
in our table, we will let HW calculate N by itself.

> 
> > +		}
> > +	}
> > +
> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> 
> And if no valid N value is found, need we go ahead to check M value?

If there is no valid N value, audio_config_dp_get_m(intel_crtc, rate);
will also return 0 and skip the m setting.

Regards,
Libin

> 
> > +
> > +	m = audio_config_dp_get_m(intel_crtc, rate);
> > +	if (m) {
> > +		tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
> > +		tmp &= ~AUD_CONFIG_M_MASK;
> > +		tmp |= m;
> > +		tmp |= AUD_M_CTS_M_VALUE_INDEX;
> > +		tmp |= AUD_M_CTS_M_PROG_ENABLE;
> > +		I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
> > +	}
> >  }
> 
> Thanks
> Mengdong
> 
> >  static void
> > @@ -267,6 +356,12 @@ hsw_hdmi_audio_config_update(struct intel_crtc
> > *intel_crtc, enum port port,
> >  	}
> >
> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > +
> > +	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
> > +	tmp &= ~AUD_CONFIG_M_MASK;
> > +	tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
> > +	tmp |= AUD_M_CTS_M_PROG_ENABLE;
> > +	I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
> >  }
> >
> >  static void
> > @@ -687,7 +782,8 @@ static int
> > i915_audio_component_sync_audio_rate(struct device *kdev, int port,
> >  	/* 1. get the pipe */
> >  	intel_encoder = get_saved_enc(dev_priv, port, pipe);
> >  	if (!intel_encoder || !intel_encoder->base.crtc ||
> > -	    intel_encoder->type != INTEL_OUTPUT_HDMI) {
> > +	    (intel_encoder->type != INTEL_OUTPUT_HDMI &&
> > +	     intel_encoder->type != INTEL_OUTPUT_DP)) {
> >  		DRM_DEBUG_KMS("Not valid for port %c\n",
> port_name(port));
> >  		err = -ENODEV;
> >  		goto unlock;
> > --
> > 2.1.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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset

[Jani Nikula]

Sorry it's taken me forever to get back to this. Some comments inline.

BR,
Jani.

On Wed, 12 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>> Lin, Mengdong
>> Sent: Wednesday, October 12, 2016 10:46 AM
>> To: Nikula, Jani <jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org
>> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
>> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
>> Subject: Re: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
>> modeset
>> 
>> 
>> 
>> > -----Original Message-----
>> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
>> > Behalf Of Jani Nikula
>> > Sent: Monday, October 10, 2016 11:04 PM
>> > To: intel-gfx@lists.freedesktop.org
>> > Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
>> > Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
>> > Subject: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
>> > modeset
>> >
>> > When modeset occurs and the LS_CLK is set to some special values in DP
>> > mode, the N/M need to be set manually if audio is playing. Otherwise
>> > the first several seconds may be silent in audio playback.
>> >
>> > The relationship of Maud and Naud is expressed in the following equation:
>> > Maud/Naud = 512 * fs / f_LS_Clk
>> >
>> > Please refer VESA DisplayPort Standard spec for details.
>> >
>> > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
>> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_reg.h    |   7 +++
>> >  drivers/gpu/drm/i915/intel_audio.c | 100
>> > ++++++++++++++++++++++++++++++++++++-
>> >  2 files changed, 105 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> > b/drivers/gpu/drm/i915/i915_reg.h index 595d196f753f..8d9dbc7d5b32
>> > 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -7359,6 +7359,13 @@ enum {
>> >  #define _HSW_AUD_MISC_CTRL_B		0x65110
>> >  #define HSW_AUD_MISC_CTRL(pipe)		_MMIO_PIPE(pipe,
>> > _HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
>> >
>> > +#define _HSW_AUD_M_CTS_ENABLE_A		0x65028
>> > +#define _HSW_AUD_M_CTS_ENABLE_B		0x65128
>> > +#define HSW_AUD_M_CTS_ENABLE(pipe)	_MMIO_PIPE(pipe,
>> > _HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
>> > +#define   AUD_M_CTS_M_VALUE_INDEX	(1 << 21)
>> > +#define   AUD_M_CTS_M_PROG_ENABLE	(1 << 20)
>> > +#define   AUD_CONFIG_M_MASK		0xfffff
>> 
>> The last line cause misalignment after applying the patch.
>> 
>> > +
>> >  #define _HSW_AUD_DIP_ELD_CTRL_ST_A	0x650b4
>> >  #define _HSW_AUD_DIP_ELD_CTRL_ST_B	0x651b4
>> >  #define HSW_AUD_DIP_ELD_CTRL(pipe)	_MMIO_PIPE(pipe,
>> > _HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B) diff --
>> git
>> > a/drivers/gpu/drm/i915/intel_audio.c
>> > b/drivers/gpu/drm/i915/intel_audio.c
>> > index 81df29ca4947..0bc2701b6c9c 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -57,6 +57,70 @@
>> >   * struct &i915_audio_component_audio_ops @audio_ops is called from
>> > i915 driver.
>> >   */
>> >
>> > +/* DP N/M table */
>> > +#define LC_540M 540000
>> > +#define LC_270M 270000
>> > +#define LC_162M 162000
>> > +
>> > +struct dp_aud_n_m {
>> > +	int sample_rate;
>> > +	int clock;
>> > +	u16 n;
>> > +	u16 m;
>> > +};
>> > +
>> > +static const struct dp_aud_n_m dp_aud_n_m[] = {
>> > +	{ 192000, LC_540M, 5625, 1024 },
>> > +	{ 176400, LC_540M, 9375, 1568 },
>> > +	{ 96000, LC_540M, 5625, 512 },
>> > +	{ 88200, LC_540M, 9375, 784 },
>> > +	{ 48000, LC_540M, 5625, 256 },
>> > +	{ 44100, LC_540M, 9375, 392 },
>> > +	{ 32000, LC_540M, 16875, 512 },

Any particular reason these M/N values are half of what they're in table
2-104 of DP 1.4 spec? (Admittedly the table is an informative example.)

>> > +	{ 192000, LC_270M, 5625, 2048 },
>> > +	{ 176400, LC_270M, 9375, 3136 },
>> > +	{ 96000, LC_270M, 5625, 1024 },
>> > +	{ 88200, LC_270M, 9375, 1568 },
>> > +	{ 48000, LC_270M, 5625, 512 },
>> > +	{ 44100, LC_270M, 9375, 784 },
>> > +	{ 32000, LC_270M, 16875, 1024 },
>> > +	{ 192000, LC_162M, 3375, 2048 },
>> > +	{ 176400, LC_162M, 5625, 3136 },
>> > +	{ 96000, LC_162M, 3375, 1024 },
>> > +	{ 88200, LC_162M, 5625, 1568 },
>> > +	{ 48000, LC_162M, 3375, 512 },
>> > +	{ 44100, LC_162M, 5625, 784 },
>> > +	{ 32000, LC_162M, 10125, 1024 },

Do we not support 128 or 64 kHz audio?

>> > +};
>> > +
>> > +static const struct dp_aud_n_m *
>> > +audio_config_dp_get_n_m(struct intel_crtc *intel_crtc, int rate) {
>> > +	int i;
>> > +
>> > +	for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
>> > +		if (rate == dp_aud_n_m[i].sample_rate &&
>> > +		    intel_crtc->config->port_clock == dp_aud_n_m[i].clock)
>> > +			return &dp_aud_n_m[i];
>> > +	}
>> > +
>> > +	return NULL;
>> > +}
>> > +
>> > +static int audio_config_dp_get_m(struct intel_crtc *intel_crtc, int
>> > +rate) {
>> > +	const struct dp_aud_n_m *nm =
>> > audio_config_dp_get_n_m(intel_crtc,
>> > +rate);
>> > +
>> > +	return nm ? nm->m : 0;
>> > +}
>> > +
>> > +static int audio_config_dp_get_n(struct intel_crtc *intel_crtc, int
>> > +rate) {
>> > +	const struct dp_aud_n_m *nm =
>> > audio_config_dp_get_n_m(intel_crtc,
>> > +rate);
>> > +
>> > +	return nm ? nm->n : 0;
>> > +}
>> > +
>> >  static const struct {
>> >  	int clock;
>> >  	u32 config;
>> > @@ -225,8 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc
>> > *intel_crtc, enum port port,
>> >  			   const struct drm_display_mode *adjusted_mode)  {
>> >  	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
>> > +	struct i915_audio_component *acomp = dev_priv-
>> > >audio_component;
>> > +	int rate = acomp ? acomp->aud_sample_rate[port] : 0;
>> >  	enum pipe pipe = intel_crtc->pipe;
>> > -	u32 tmp;
>> > +	u32 tmp, n, m;
>> >
>> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>> > @@ -234,7 +300,30 @@ hsw_dp_audio_config_update(struct intel_crtc
>> > *intel_crtc, enum port port,
>> >  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>> >  	tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> >
>> > +	if (intel_crtc->config->port_clock == LC_540M ||
>> > +	    intel_crtc->config->port_clock == LC_270M ||
>> > +	    intel_crtc->config->port_clock == LC_162M) {

Actually, this check could be removed. audio_config_dp_get_n_m will
always return 0 if this doesn't hold. But not a big deal.

>> > +		n = audio_config_dp_get_n(intel_crtc, rate);
>> > +		if (n != 0) {
>> > +			tmp &= ~AUD_CONFIG_N_MASK;
>> > +			tmp |= AUD_CONFIG_N(n);
>> > +			tmp |= AUD_CONFIG_N_PROG_ENABLE;
>> > +		} else {
>> > +			DRM_DEBUG_KMS("no suitable N value is found\n");
>> 
>> Could some comments be added here why we don't return if no valid N value
>> is found?
>
> This is because although we don't need set N value here, but there are some
> other bits to be set. So you can see we call 
> I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> Later. 
>
>> Do we still need to program write the register to let HW calculate N by itself?
>
> Ideally, all the frequency should be in the table. But if the frequency is not
> in our table, we will let HW calculate N by itself.
>
>> 
>> > +		}
>> > +	}
>> > +
>> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> 
>> And if no valid N value is found, need we go ahead to check M value?
>
> If there is no valid N value, audio_config_dp_get_m(intel_crtc, rate);
> will also return 0 and skip the m setting.

If we can't find M/N values, the HSW_AUD_CFG register is set to use
automatic N value. However, the HSW_AUD_M_CTS_ENABLE register is not
updated at all, and could potentially have stale stuff. Do we need to
clear the AUD_M_CTS_M_VALUE_INDEX and/or AUD_M_CTS_M_PROG_ENABLE bits?

We only do this stuff in hsw_audio_codec_enable now. Do we need to do
something in hsw_audio_codec_disable too?


BR,
Jani.


>
> Regards,
> Libin
>
>> 
>> > +
>> > +	m = audio_config_dp_get_m(intel_crtc, rate);
>> > +	if (m) {
>> > +		tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
>> > +		tmp &= ~AUD_CONFIG_M_MASK;
>> > +		tmp |= m;
>> > +		tmp |= AUD_M_CTS_M_VALUE_INDEX;
>> > +		tmp |= AUD_M_CTS_M_PROG_ENABLE;
>> > +		I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
>> > +	}
>> >  }
>> 
>> Thanks
>> Mengdong
>> 
>> >  static void
>> > @@ -267,6 +356,12 @@ hsw_hdmi_audio_config_update(struct intel_crtc
>> > *intel_crtc, enum port port,
>> >  	}
>> >
>> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> > +
>> > +	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
>> > +	tmp &= ~AUD_CONFIG_M_MASK;
>> > +	tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
>> > +	tmp |= AUD_M_CTS_M_PROG_ENABLE;
>> > +	I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
>> >  }
>> >
>> >  static void
>> > @@ -687,7 +782,8 @@ static int
>> > i915_audio_component_sync_audio_rate(struct device *kdev, int port,
>> >  	/* 1. get the pipe */
>> >  	intel_encoder = get_saved_enc(dev_priv, port, pipe);
>> >  	if (!intel_encoder || !intel_encoder->base.crtc ||
>> > -	    intel_encoder->type != INTEL_OUTPUT_HDMI) {
>> > +	    (intel_encoder->type != INTEL_OUTPUT_HDMI &&
>> > +	     intel_encoder->type != INTEL_OUTPUT_DP)) {
>> >  		DRM_DEBUG_KMS("Not valid for port %c\n",
>> port_name(port));
>> >  		err = -ENODEV;
>> >  		goto unlock;
>> > --
>> > 2.1.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

-- 
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 RESEND 9/9] drm/i915: set proper N/M in modeset

[Yang, Libin]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Wednesday, October 19, 2016 11:09 PM
> To: Yang, Libin <libin.yang@intel.com>; Lin, Mengdong
> <mengdong.lin@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>; Zhang, Keqiao
> <keqiao.zhang@intel.com>; Zhao, Juan J <juan.j.zhao@intel.com>
> Subject: RE: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> modeset
> 
> 
> Sorry it's taken me forever to get back to this. Some comments inline.
> 
> BR,
> Jani.
> 
> On Wed, 12 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> >> -----Original Message-----
> >> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
> >> Behalf Of Lin, Mengdong
> >> Sent: Wednesday, October 12, 2016 10:46 AM
> >> To: Nikula, Jani <jani.nikula@intel.com>;
> >> intel-gfx@lists.freedesktop.org
> >> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> >> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> >> Subject: Re: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M
> >> in modeset
> >>
> >>
> >>
> >> > -----Original Message-----
> >> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
> >> > Behalf Of Jani Nikula
> >> > Sent: Monday, October 10, 2016 11:04 PM
> >> > To: intel-gfx@lists.freedesktop.org
> >> > Cc: Nikula, Jani <jani.nikula@intel.com>;
> >> > libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> >> > <dhinakaran.pandiyan@intel.com>
> >> > Subject: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> >> > modeset
> >> >
> >> > When modeset occurs and the LS_CLK is set to some special values in
> >> > DP mode, the N/M need to be set manually if audio is playing.
> >> > Otherwise the first several seconds may be silent in audio playback.
> >> >
> >> > The relationship of Maud and Naud is expressed in the following
> equation:
> >> > Maud/Naud = 512 * fs / f_LS_Clk
> >> >
> >> > Please refer VESA DisplayPort Standard spec for details.
> >> >
> >> > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> >> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/i915_reg.h    |   7 +++
> >> >  drivers/gpu/drm/i915/intel_audio.c | 100
> >> > ++++++++++++++++++++++++++++++++++++-
> >> >  2 files changed, 105 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> > b/drivers/gpu/drm/i915/i915_reg.h index 595d196f753f..8d9dbc7d5b32
> >> > 100644
> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> > @@ -7359,6 +7359,13 @@ enum {
> >> >  #define _HSW_AUD_MISC_CTRL_B		0x65110
> >> >  #define HSW_AUD_MISC_CTRL(pipe)		_MMIO_PIPE(pipe,
> >> > _HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
> >> >
> >> > +#define _HSW_AUD_M_CTS_ENABLE_A		0x65028
> >> > +#define _HSW_AUD_M_CTS_ENABLE_B		0x65128
> >> > +#define HSW_AUD_M_CTS_ENABLE(pipe)	_MMIO_PIPE(pipe,
> >> > _HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
> >> > +#define   AUD_M_CTS_M_VALUE_INDEX	(1 << 21)
> >> > +#define   AUD_M_CTS_M_PROG_ENABLE	(1 << 20)
> >> > +#define   AUD_CONFIG_M_MASK		0xfffff
> >>
> >> The last line cause misalignment after applying the patch.
> >>
> >> > +
> >> >  #define _HSW_AUD_DIP_ELD_CTRL_ST_A	0x650b4
> >> >  #define _HSW_AUD_DIP_ELD_CTRL_ST_B	0x651b4
> >> >  #define HSW_AUD_DIP_ELD_CTRL(pipe)	_MMIO_PIPE(pipe,
> >> > _HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B)
> diff --
> >> git
> >> > a/drivers/gpu/drm/i915/intel_audio.c
> >> > b/drivers/gpu/drm/i915/intel_audio.c
> >> > index 81df29ca4947..0bc2701b6c9c 100644
> >> > --- a/drivers/gpu/drm/i915/intel_audio.c
> >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> >> > @@ -57,6 +57,70 @@
> >> >   * struct &i915_audio_component_audio_ops @audio_ops is called
> >> > from
> >> > i915 driver.
> >> >   */
> >> >
> >> > +/* DP N/M table */
> >> > +#define LC_540M 540000
> >> > +#define LC_270M 270000
> >> > +#define LC_162M 162000
> >> > +
> >> > +struct dp_aud_n_m {
> >> > +	int sample_rate;
> >> > +	int clock;
> >> > +	u16 n;
> >> > +	u16 m;
> >> > +};
> >> > +
> >> > +static const struct dp_aud_n_m dp_aud_n_m[] = {
> >> > +	{ 192000, LC_540M, 5625, 1024 },
> >> > +	{ 176400, LC_540M, 9375, 1568 },
> >> > +	{ 96000, LC_540M, 5625, 512 },
> >> > +	{ 88200, LC_540M, 9375, 784 },
> >> > +	{ 48000, LC_540M, 5625, 256 },
> >> > +	{ 44100, LC_540M, 9375, 392 },
> >> > +	{ 32000, LC_540M, 16875, 512 },
> 
> Any particular reason these M/N values are half of what they're in table
> 2-104 of DP 1.4 spec? (Admittedly the table is an informative example.)

For HDMI, we found only set N is enough. HW then can handle the remaining.

> 
> >> > +	{ 192000, LC_270M, 5625, 2048 },
> >> > +	{ 176400, LC_270M, 9375, 3136 },
> >> > +	{ 96000, LC_270M, 5625, 1024 },
> >> > +	{ 88200, LC_270M, 9375, 1568 },
> >> > +	{ 48000, LC_270M, 5625, 512 },
> >> > +	{ 44100, LC_270M, 9375, 784 },
> >> > +	{ 32000, LC_270M, 16875, 1024 },
> >> > +	{ 192000, LC_162M, 3375, 2048 },
> >> > +	{ 176400, LC_162M, 5625, 3136 },
> >> > +	{ 96000, LC_162M, 3375, 1024 },
> >> > +	{ 88200, LC_162M, 5625, 1568 },
> >> > +	{ 48000, LC_162M, 3375, 512 },
> >> > +	{ 44100, LC_162M, 5625, 784 },
> >> > +	{ 32000, LC_162M, 10125, 1024 },
> 
> Do we not support 128 or 64 kHz audio?

No, we don't support HBR audio.

> 
> >> > +};
> >> > +
> >> > +static const struct dp_aud_n_m *
> >> > +audio_config_dp_get_n_m(struct intel_crtc *intel_crtc, int rate) {
> >> > +	int i;
> >> > +
> >> > +	for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
> >> > +		if (rate == dp_aud_n_m[i].sample_rate &&
> >> > +		    intel_crtc->config->port_clock == dp_aud_n_m[i].clock)
> >> > +			return &dp_aud_n_m[i];
> >> > +	}
> >> > +
> >> > +	return NULL;
> >> > +}
> >> > +
> >> > +static int audio_config_dp_get_m(struct intel_crtc *intel_crtc,
> >> > +int
> >> > +rate) {
> >> > +	const struct dp_aud_n_m *nm =
> >> > audio_config_dp_get_n_m(intel_crtc,
> >> > +rate);
> >> > +
> >> > +	return nm ? nm->m : 0;
> >> > +}
> >> > +
> >> > +static int audio_config_dp_get_n(struct intel_crtc *intel_crtc,
> >> > +int
> >> > +rate) {
> >> > +	const struct dp_aud_n_m *nm =
> >> > audio_config_dp_get_n_m(intel_crtc,
> >> > +rate);
> >> > +
> >> > +	return nm ? nm->n : 0;
> >> > +}
> >> > +
> >> >  static const struct {
> >> >  	int clock;
> >> >  	u32 config;
> >> > @@ -225,8 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc
> >> > *intel_crtc, enum port port,
> >> >  			   const struct drm_display_mode *adjusted_mode)  {
> >> >  	struct drm_i915_private *dev_priv =
> >> > to_i915(intel_crtc->base.dev);
> >> > +	struct i915_audio_component *acomp = dev_priv-
> >> > >audio_component;
> >> > +	int rate = acomp ? acomp->aud_sample_rate[port] : 0;
> >> >  	enum pipe pipe = intel_crtc->pipe;
> >> > -	u32 tmp;
> >> > +	u32 tmp, n, m;
> >> >
> >> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> >> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX; @@ -234,7 +300,30 @@
> >> > hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port
> >> > port,
> >> >  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> >> >  	tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >> >
> >> > +	if (intel_crtc->config->port_clock == LC_540M ||
> >> > +	    intel_crtc->config->port_clock == LC_270M ||
> >> > +	    intel_crtc->config->port_clock == LC_162M) {
> 
> Actually, this check could be removed. audio_config_dp_get_n_m will always
> return 0 if this doesn't hold. But not a big deal.

Yes, we can remove it.

> 
> >> > +		n = audio_config_dp_get_n(intel_crtc, rate);
> >> > +		if (n != 0) {
> >> > +			tmp &= ~AUD_CONFIG_N_MASK;
> >> > +			tmp |= AUD_CONFIG_N(n);
> >> > +			tmp |= AUD_CONFIG_N_PROG_ENABLE;
> >> > +		} else {
> >> > +			DRM_DEBUG_KMS("no suitable N value is found\n");
> >>
> >> Could some comments be added here why we don't return if no valid N
> >> value is found?
> >
> > This is because although we don't need set N value here, but there are
> > some other bits to be set. So you can see we call
> > I915_WRITE(HSW_AUD_CFG(pipe), tmp); Later.
> >
> >> Do we still need to program write the register to let HW calculate N by
> itself?
> >
> > Ideally, all the frequency should be in the table. But if the
> > frequency is not in our table, we will let HW calculate N by itself.
> >
> >>
> >> > +		}
> >> > +	}
> >> > +
> >> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >>
> >> And if no valid N value is found, need we go ahead to check M value?
> >
> > If there is no valid N value, audio_config_dp_get_m(intel_crtc, rate);
> > will also return 0 and skip the m setting.
> 
> If we can't find M/N values, the HSW_AUD_CFG register is set to use
> automatic N value. However, the HSW_AUD_M_CTS_ENABLE register is not
> updated at all, and could potentially have stale stuff. Do we need to clear the
> AUD_M_CTS_M_VALUE_INDEX and/or AUD_M_CTS_M_PROG_ENABLE bits?
> 
> We only do this stuff in hsw_audio_codec_enable now. Do we need to do
> something in hsw_audio_codec_disable too?

In _disable(), the audio doesn't work. So the value will not impact audio.
It will not be worse that we do not reset the value in _disable().
Without reset, the value is "cached". Next time, it may be a little quicker
for monitor response if we are using the same parameter, I think.

Regards
Libin

> 
> 
> BR,
> Jani.
> 
> 
> >
> > Regards,
> > Libin
> >
> >>
> >> > +
> >> > +	m = audio_config_dp_get_m(intel_crtc, rate);
> >> > +	if (m) {
> >> > +		tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
> >> > +		tmp &= ~AUD_CONFIG_M_MASK;
> >> > +		tmp |= m;
> >> > +		tmp |= AUD_M_CTS_M_VALUE_INDEX;
> >> > +		tmp |= AUD_M_CTS_M_PROG_ENABLE;
> >> > +		I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
> >> > +	}
> >> >  }
> >>
> >> Thanks
> >> Mengdong
> >>
> >> >  static void
> >> > @@ -267,6 +356,12 @@ hsw_hdmi_audio_config_update(struct
> intel_crtc
> >> > *intel_crtc, enum port port,
> >> >  	}
> >> >
> >> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >> > +
> >> > +	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
> >> > +	tmp &= ~AUD_CONFIG_M_MASK;
> >> > +	tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
> >> > +	tmp |= AUD_M_CTS_M_PROG_ENABLE;
> >> > +	I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
> >> >  }
> >> >
> >> >  static void
> >> > @@ -687,7 +782,8 @@ static int
> >> > i915_audio_component_sync_audio_rate(struct device *kdev, int port,
> >> >  	/* 1. get the pipe */
> >> >  	intel_encoder = get_saved_enc(dev_priv, port, pipe);
> >> >  	if (!intel_encoder || !intel_encoder->base.crtc ||
> >> > -	    intel_encoder->type != INTEL_OUTPUT_HDMI) {
> >> > +	    (intel_encoder->type != INTEL_OUTPUT_HDMI &&
> >> > +	     intel_encoder->type != INTEL_OUTPUT_DP)) {
> >> >  		DRM_DEBUG_KMS("Not valid for port %c\n",
> >> port_name(port));
> >> >  		err = -ENODEV;
> >> >  		goto unlock;
> >> > --
> >> > 2.1.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
> 
> --
> 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 RESEND 9/9] drm/i915: set proper N/M in modeset

[Jani Nikula]

On Thu, 20 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani
>> Sent: Wednesday, October 19, 2016 11:09 PM
>> To: Yang, Libin <libin.yang@intel.com>; Lin, Mengdong
>> <mengdong.lin@intel.com>; intel-gfx@lists.freedesktop.org
>> Cc: libin.yang@linux.intel.com; Pandiyan, Dhinakaran
>> <dhinakaran.pandiyan@intel.com>; Zhang, Keqiao
>> <keqiao.zhang@intel.com>; Zhao, Juan J <juan.j.zhao@intel.com>
>> Subject: RE: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
>> modeset
>> 
>> 
>> Sorry it's taken me forever to get back to this. Some comments inline.
>> 
>> BR,
>> Jani.
>> 
>> On Wed, 12 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
>> >> -----Original Message-----
>> >> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
>> >> Behalf Of Lin, Mengdong
>> >> Sent: Wednesday, October 12, 2016 10:46 AM
>> >> To: Nikula, Jani <jani.nikula@intel.com>;
>> >> intel-gfx@lists.freedesktop.org
>> >> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
>> >> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
>> >> Subject: Re: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M
>> >> in modeset
>> >>
>> >>
>> >>
>> >> > -----Original Message-----
>> >> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
>> >> > Behalf Of Jani Nikula
>> >> > Sent: Monday, October 10, 2016 11:04 PM
>> >> > To: intel-gfx@lists.freedesktop.org
>> >> > Cc: Nikula, Jani <jani.nikula@intel.com>;
>> >> > libin.yang@linux.intel.com; Pandiyan, Dhinakaran
>> >> > <dhinakaran.pandiyan@intel.com>
>> >> > Subject: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
>> >> > modeset
>> >> >
>> >> > When modeset occurs and the LS_CLK is set to some special values in
>> >> > DP mode, the N/M need to be set manually if audio is playing.
>> >> > Otherwise the first several seconds may be silent in audio playback.
>> >> >
>> >> > The relationship of Maud and Naud is expressed in the following
>> equation:
>> >> > Maud/Naud = 512 * fs / f_LS_Clk
>> >> >
>> >> > Please refer VESA DisplayPort Standard spec for details.
>> >> >
>> >> > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
>> >> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> >> > ---
>> >> >  drivers/gpu/drm/i915/i915_reg.h    |   7 +++
>> >> >  drivers/gpu/drm/i915/intel_audio.c | 100
>> >> > ++++++++++++++++++++++++++++++++++++-
>> >> >  2 files changed, 105 insertions(+), 2 deletions(-)
>> >> >
>> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> >> > b/drivers/gpu/drm/i915/i915_reg.h index 595d196f753f..8d9dbc7d5b32
>> >> > 100644
>> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> >> > @@ -7359,6 +7359,13 @@ enum {
>> >> >  #define _HSW_AUD_MISC_CTRL_B		0x65110
>> >> >  #define HSW_AUD_MISC_CTRL(pipe)		_MMIO_PIPE(pipe,
>> >> > _HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
>> >> >
>> >> > +#define _HSW_AUD_M_CTS_ENABLE_A		0x65028
>> >> > +#define _HSW_AUD_M_CTS_ENABLE_B		0x65128
>> >> > +#define HSW_AUD_M_CTS_ENABLE(pipe)	_MMIO_PIPE(pipe,
>> >> > _HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
>> >> > +#define   AUD_M_CTS_M_VALUE_INDEX	(1 << 21)
>> >> > +#define   AUD_M_CTS_M_PROG_ENABLE	(1 << 20)
>> >> > +#define   AUD_CONFIG_M_MASK		0xfffff
>> >>
>> >> The last line cause misalignment after applying the patch.
>> >>
>> >> > +
>> >> >  #define _HSW_AUD_DIP_ELD_CTRL_ST_A	0x650b4
>> >> >  #define _HSW_AUD_DIP_ELD_CTRL_ST_B	0x651b4
>> >> >  #define HSW_AUD_DIP_ELD_CTRL(pipe)	_MMIO_PIPE(pipe,
>> >> > _HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B)
>> diff --
>> >> git
>> >> > a/drivers/gpu/drm/i915/intel_audio.c
>> >> > b/drivers/gpu/drm/i915/intel_audio.c
>> >> > index 81df29ca4947..0bc2701b6c9c 100644
>> >> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> >> > @@ -57,6 +57,70 @@
>> >> >   * struct &i915_audio_component_audio_ops @audio_ops is called
>> >> > from
>> >> > i915 driver.
>> >> >   */
>> >> >
>> >> > +/* DP N/M table */
>> >> > +#define LC_540M 540000
>> >> > +#define LC_270M 270000
>> >> > +#define LC_162M 162000
>> >> > +
>> >> > +struct dp_aud_n_m {
>> >> > +	int sample_rate;
>> >> > +	int clock;
>> >> > +	u16 n;
>> >> > +	u16 m;
>> >> > +};
>> >> > +
>> >> > +static const struct dp_aud_n_m dp_aud_n_m[] = {
>> >> > +	{ 192000, LC_540M, 5625, 1024 },
>> >> > +	{ 176400, LC_540M, 9375, 1568 },
>> >> > +	{ 96000, LC_540M, 5625, 512 },
>> >> > +	{ 88200, LC_540M, 9375, 784 },
>> >> > +	{ 48000, LC_540M, 5625, 256 },
>> >> > +	{ 44100, LC_540M, 9375, 392 },
>> >> > +	{ 32000, LC_540M, 16875, 512 },
>> 
>> Any particular reason these M/N values are half of what they're in table
>> 2-104 of DP 1.4 spec? (Admittedly the table is an informative example.)
>
> For HDMI, we found only set N is enough. HW then can handle the remaining.

I meant, the M and N values in this part of the dp_aud_n_m table are 1/2
of what they are in the DP spec table. Why?

Jani.


>
>> 
>> >> > +	{ 192000, LC_270M, 5625, 2048 },
>> >> > +	{ 176400, LC_270M, 9375, 3136 },
>> >> > +	{ 96000, LC_270M, 5625, 1024 },
>> >> > +	{ 88200, LC_270M, 9375, 1568 },
>> >> > +	{ 48000, LC_270M, 5625, 512 },
>> >> > +	{ 44100, LC_270M, 9375, 784 },
>> >> > +	{ 32000, LC_270M, 16875, 1024 },
>> >> > +	{ 192000, LC_162M, 3375, 2048 },
>> >> > +	{ 176400, LC_162M, 5625, 3136 },
>> >> > +	{ 96000, LC_162M, 3375, 1024 },
>> >> > +	{ 88200, LC_162M, 5625, 1568 },
>> >> > +	{ 48000, LC_162M, 3375, 512 },
>> >> > +	{ 44100, LC_162M, 5625, 784 },
>> >> > +	{ 32000, LC_162M, 10125, 1024 },
>> 
>> Do we not support 128 or 64 kHz audio?
>
> No, we don't support HBR audio.
>
>> 
>> >> > +};
>> >> > +
>> >> > +static const struct dp_aud_n_m *
>> >> > +audio_config_dp_get_n_m(struct intel_crtc *intel_crtc, int rate) {
>> >> > +	int i;
>> >> > +
>> >> > +	for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
>> >> > +		if (rate == dp_aud_n_m[i].sample_rate &&
>> >> > +		    intel_crtc->config->port_clock == dp_aud_n_m[i].clock)
>> >> > +			return &dp_aud_n_m[i];
>> >> > +	}
>> >> > +
>> >> > +	return NULL;
>> >> > +}
>> >> > +
>> >> > +static int audio_config_dp_get_m(struct intel_crtc *intel_crtc,
>> >> > +int
>> >> > +rate) {
>> >> > +	const struct dp_aud_n_m *nm =
>> >> > audio_config_dp_get_n_m(intel_crtc,
>> >> > +rate);
>> >> > +
>> >> > +	return nm ? nm->m : 0;
>> >> > +}
>> >> > +
>> >> > +static int audio_config_dp_get_n(struct intel_crtc *intel_crtc,
>> >> > +int
>> >> > +rate) {
>> >> > +	const struct dp_aud_n_m *nm =
>> >> > audio_config_dp_get_n_m(intel_crtc,
>> >> > +rate);
>> >> > +
>> >> > +	return nm ? nm->n : 0;
>> >> > +}
>> >> > +
>> >> >  static const struct {
>> >> >  	int clock;
>> >> >  	u32 config;
>> >> > @@ -225,8 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc
>> >> > *intel_crtc, enum port port,
>> >> >  			   const struct drm_display_mode *adjusted_mode)  {
>> >> >  	struct drm_i915_private *dev_priv =
>> >> > to_i915(intel_crtc->base.dev);
>> >> > +	struct i915_audio_component *acomp = dev_priv-
>> >> > >audio_component;
>> >> > +	int rate = acomp ? acomp->aud_sample_rate[port] : 0;
>> >> >  	enum pipe pipe = intel_crtc->pipe;
>> >> > -	u32 tmp;
>> >> > +	u32 tmp, n, m;
>> >> >
>> >> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>> >> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX; @@ -234,7 +300,30 @@
>> >> > hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port
>> >> > port,
>> >> >  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>> >> >  	tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> >> >
>> >> > +	if (intel_crtc->config->port_clock == LC_540M ||
>> >> > +	    intel_crtc->config->port_clock == LC_270M ||
>> >> > +	    intel_crtc->config->port_clock == LC_162M) {
>> 
>> Actually, this check could be removed. audio_config_dp_get_n_m will always
>> return 0 if this doesn't hold. But not a big deal.
>
> Yes, we can remove it.
>
>> 
>> >> > +		n = audio_config_dp_get_n(intel_crtc, rate);
>> >> > +		if (n != 0) {
>> >> > +			tmp &= ~AUD_CONFIG_N_MASK;
>> >> > +			tmp |= AUD_CONFIG_N(n);
>> >> > +			tmp |= AUD_CONFIG_N_PROG_ENABLE;
>> >> > +		} else {
>> >> > +			DRM_DEBUG_KMS("no suitable N value is found\n");
>> >>
>> >> Could some comments be added here why we don't return if no valid N
>> >> value is found?
>> >
>> > This is because although we don't need set N value here, but there are
>> > some other bits to be set. So you can see we call
>> > I915_WRITE(HSW_AUD_CFG(pipe), tmp); Later.
>> >
>> >> Do we still need to program write the register to let HW calculate N by
>> itself?
>> >
>> > Ideally, all the frequency should be in the table. But if the
>> > frequency is not in our table, we will let HW calculate N by itself.
>> >
>> >>
>> >> > +		}
>> >> > +	}
>> >> > +
>> >> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> >>
>> >> And if no valid N value is found, need we go ahead to check M value?
>> >
>> > If there is no valid N value, audio_config_dp_get_m(intel_crtc, rate);
>> > will also return 0 and skip the m setting.
>> 
>> If we can't find M/N values, the HSW_AUD_CFG register is set to use
>> automatic N value. However, the HSW_AUD_M_CTS_ENABLE register is not
>> updated at all, and could potentially have stale stuff. Do we need to clear the
>> AUD_M_CTS_M_VALUE_INDEX and/or AUD_M_CTS_M_PROG_ENABLE bits?
>> 
>> We only do this stuff in hsw_audio_codec_enable now. Do we need to do
>> something in hsw_audio_codec_disable too?
>
> In _disable(), the audio doesn't work. So the value will not impact audio.
> It will not be worse that we do not reset the value in _disable().
> Without reset, the value is "cached". Next time, it may be a little quicker
> for monitor response if we are using the same parameter, I think.
>
> Regards
> Libin
>
>> 
>> 
>> BR,
>> Jani.
>> 
>> 
>> >
>> > Regards,
>> > Libin
>> >
>> >>
>> >> > +
>> >> > +	m = audio_config_dp_get_m(intel_crtc, rate);
>> >> > +	if (m) {
>> >> > +		tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
>> >> > +		tmp &= ~AUD_CONFIG_M_MASK;
>> >> > +		tmp |= m;
>> >> > +		tmp |= AUD_M_CTS_M_VALUE_INDEX;
>> >> > +		tmp |= AUD_M_CTS_M_PROG_ENABLE;
>> >> > +		I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
>> >> > +	}
>> >> >  }
>> >>
>> >> Thanks
>> >> Mengdong
>> >>
>> >> >  static void
>> >> > @@ -267,6 +356,12 @@ hsw_hdmi_audio_config_update(struct
>> intel_crtc
>> >> > *intel_crtc, enum port port,
>> >> >  	}
>> >> >
>> >> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> >> > +
>> >> > +	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
>> >> > +	tmp &= ~AUD_CONFIG_M_MASK;
>> >> > +	tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
>> >> > +	tmp |= AUD_M_CTS_M_PROG_ENABLE;
>> >> > +	I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
>> >> >  }
>> >> >
>> >> >  static void
>> >> > @@ -687,7 +782,8 @@ static int
>> >> > i915_audio_component_sync_audio_rate(struct device *kdev, int port,
>> >> >  	/* 1. get the pipe */
>> >> >  	intel_encoder = get_saved_enc(dev_priv, port, pipe);
>> >> >  	if (!intel_encoder || !intel_encoder->base.crtc ||
>> >> > -	    intel_encoder->type != INTEL_OUTPUT_HDMI) {
>> >> > +	    (intel_encoder->type != INTEL_OUTPUT_HDMI &&
>> >> > +	     intel_encoder->type != INTEL_OUTPUT_DP)) {
>> >> >  		DRM_DEBUG_KMS("Not valid for port %c\n",
>> >> port_name(port));
>> >> >  		err = -ENODEV;
>> >> >  		goto unlock;
>> >> > --
>> >> > 2.1.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
>> 
>> --
>> Jani Nikula, Intel Open Source Technology Center

-- 
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 RESEND 9/9] drm/i915: set proper N/M in modeset

[Yang, Libin]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Thursday, October 20, 2016 4:34 PM
> To: Yang, Libin <libin.yang@intel.com>; Lin, Mengdong
> <mengdong.lin@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>; Zhang, Keqiao
> <keqiao.zhang@intel.com>; Zhao, Juan J <juan.j.zhao@intel.com>
> Subject: RE: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> modeset
> 
> On Thu, 20 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani
> >> Sent: Wednesday, October 19, 2016 11:09 PM
> >> To: Yang, Libin <libin.yang@intel.com>; Lin, Mengdong
> >> <mengdong.lin@intel.com>; intel-gfx@lists.freedesktop.org
> >> Cc: libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> >> <dhinakaran.pandiyan@intel.com>; Zhang, Keqiao
> >> <keqiao.zhang@intel.com>; Zhao, Juan J <juan.j.zhao@intel.com>
> >> Subject: RE: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M
> >> in modeset
> >>
> >>
> >> Sorry it's taken me forever to get back to this. Some comments inline.
> >>
> >> BR,
> >> Jani.
> >>
> >> On Wed, 12 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> >> >> -----Original Message-----
> >> >> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org]
> >> >> On Behalf Of Lin, Mengdong
> >> >> Sent: Wednesday, October 12, 2016 10:46 AM
> >> >> To: Nikula, Jani <jani.nikula@intel.com>;
> >> >> intel-gfx@lists.freedesktop.org
> >> >> Cc: Nikula, Jani <jani.nikula@intel.com>;
> >> >> libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> >> >> <dhinakaran.pandiyan@intel.com>
> >> >> Subject: Re: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper
> >> >> N/M in modeset
> >> >>
> >> >>
> >> >>
> >> >> > -----Original Message-----
> >> >> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org]
> >> >> > On Behalf Of Jani Nikula
> >> >> > Sent: Monday, October 10, 2016 11:04 PM
> >> >> > To: intel-gfx@lists.freedesktop.org
> >> >> > Cc: Nikula, Jani <jani.nikula@intel.com>;
> >> >> > libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> >> >> > <dhinakaran.pandiyan@intel.com>
> >> >> > Subject: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M
> >> >> > in modeset
> >> >> >
> >> >> > When modeset occurs and the LS_CLK is set to some special values
> >> >> > in DP mode, the N/M need to be set manually if audio is playing.
> >> >> > Otherwise the first several seconds may be silent in audio playback.
> >> >> >
> >> >> > The relationship of Maud and Naud is expressed in the following
> >> equation:
> >> >> > Maud/Naud = 512 * fs / f_LS_Clk
> >> >> >
> >> >> > Please refer VESA DisplayPort Standard spec for details.
> >> >> >
> >> >> > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> >> >> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> >> > ---
> >> >> >  drivers/gpu/drm/i915/i915_reg.h    |   7 +++
> >> >> >  drivers/gpu/drm/i915/intel_audio.c | 100
> >> >> > ++++++++++++++++++++++++++++++++++++-
> >> >> >  2 files changed, 105 insertions(+), 2 deletions(-)
> >> >> >
> >> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> >> > b/drivers/gpu/drm/i915/i915_reg.h index
> >> >> > 595d196f753f..8d9dbc7d5b32
> >> >> > 100644
> >> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> >> > @@ -7359,6 +7359,13 @@ enum {
> >> >> >  #define _HSW_AUD_MISC_CTRL_B		0x65110
> >> >> >  #define HSW_AUD_MISC_CTRL(pipe)		_MMIO_PIPE(pipe,
> >> >> > _HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
> >> >> >
> >> >> > +#define _HSW_AUD_M_CTS_ENABLE_A		0x65028
> >> >> > +#define _HSW_AUD_M_CTS_ENABLE_B		0x65128
> >> >> > +#define HSW_AUD_M_CTS_ENABLE(pipe)	_MMIO_PIPE(pipe,
> >> >> > _HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
> >> >> > +#define   AUD_M_CTS_M_VALUE_INDEX	(1 << 21)
> >> >> > +#define   AUD_M_CTS_M_PROG_ENABLE	(1 << 20)
> >> >> > +#define   AUD_CONFIG_M_MASK		0xfffff
> >> >>
> >> >> The last line cause misalignment after applying the patch.
> >> >>
> >> >> > +
> >> >> >  #define _HSW_AUD_DIP_ELD_CTRL_ST_A	0x650b4
> >> >> >  #define _HSW_AUD_DIP_ELD_CTRL_ST_B	0x651b4
> >> >> >  #define HSW_AUD_DIP_ELD_CTRL(pipe)	_MMIO_PIPE(pipe,
> >> >> > _HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B)
> >> diff --
> >> >> git
> >> >> > a/drivers/gpu/drm/i915/intel_audio.c
> >> >> > b/drivers/gpu/drm/i915/intel_audio.c
> >> >> > index 81df29ca4947..0bc2701b6c9c 100644
> >> >> > --- a/drivers/gpu/drm/i915/intel_audio.c
> >> >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> >> >> > @@ -57,6 +57,70 @@
> >> >> >   * struct &i915_audio_component_audio_ops @audio_ops is called
> >> >> > from
> >> >> > i915 driver.
> >> >> >   */
> >> >> >
> >> >> > +/* DP N/M table */
> >> >> > +#define LC_540M 540000
> >> >> > +#define LC_270M 270000
> >> >> > +#define LC_162M 162000
> >> >> > +
> >> >> > +struct dp_aud_n_m {
> >> >> > +	int sample_rate;
> >> >> > +	int clock;
> >> >> > +	u16 n;
> >> >> > +	u16 m;
> >> >> > +};
> >> >> > +
> >> >> > +static const struct dp_aud_n_m dp_aud_n_m[] = {
> >> >> > +	{ 192000, LC_540M, 5625, 1024 },
> >> >> > +	{ 176400, LC_540M, 9375, 1568 },
> >> >> > +	{ 96000, LC_540M, 5625, 512 },
> >> >> > +	{ 88200, LC_540M, 9375, 784 },
> >> >> > +	{ 48000, LC_540M, 5625, 256 },
> >> >> > +	{ 44100, LC_540M, 9375, 392 },
> >> >> > +	{ 32000, LC_540M, 16875, 512 },
> >>
> >> Any particular reason these M/N values are half of what they're in
> >> table
> >> 2-104 of DP 1.4 spec? (Admittedly the table is an informative
> >> example.)
> >
> > For HDMI, we found only set N is enough. HW then can handle the
> remaining.
> 
> I meant, the M and N values in this part of the dp_aud_n_m table are 1/2 of
> what they are in the DP spec table. Why?

Which table are you meaning? I calculate the values myself. I didn't find the
full table in DP spec. I only find the table for 270MHz and 162MHz in 
Table 2-50: Examples of Maud and Naud Values

Regards,
Libin

> 
> Jani.
> 
> 
> >
> >>
> >> >> > +	{ 192000, LC_270M, 5625, 2048 },
> >> >> > +	{ 176400, LC_270M, 9375, 3136 },
> >> >> > +	{ 96000, LC_270M, 5625, 1024 },
> >> >> > +	{ 88200, LC_270M, 9375, 1568 },
> >> >> > +	{ 48000, LC_270M, 5625, 512 },
> >> >> > +	{ 44100, LC_270M, 9375, 784 },
> >> >> > +	{ 32000, LC_270M, 16875, 1024 },
> >> >> > +	{ 192000, LC_162M, 3375, 2048 },
> >> >> > +	{ 176400, LC_162M, 5625, 3136 },
> >> >> > +	{ 96000, LC_162M, 3375, 1024 },
> >> >> > +	{ 88200, LC_162M, 5625, 1568 },
> >> >> > +	{ 48000, LC_162M, 3375, 512 },
> >> >> > +	{ 44100, LC_162M, 5625, 784 },
> >> >> > +	{ 32000, LC_162M, 10125, 1024 },
> >>
> >> Do we not support 128 or 64 kHz audio?
> >
> > No, we don't support HBR audio.
> >
> >>
> >> >> > +};
> >> >> > +
> >> >> > +static const struct dp_aud_n_m * audio_config_dp_get_n_m(struct
> >> >> > +intel_crtc *intel_crtc, int rate) {
> >> >> > +	int i;
> >> >> > +
> >> >> > +	for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
> >> >> > +		if (rate == dp_aud_n_m[i].sample_rate &&
> >> >> > +		    intel_crtc->config->port_clock ==
> dp_aud_n_m[i].clock)
> >> >> > +			return &dp_aud_n_m[i];
> >> >> > +	}
> >> >> > +
> >> >> > +	return NULL;
> >> >> > +}
> >> >> > +
> >> >> > +static int audio_config_dp_get_m(struct intel_crtc *intel_crtc,
> >> >> > +int
> >> >> > +rate) {
> >> >> > +	const struct dp_aud_n_m *nm =
> >> >> > audio_config_dp_get_n_m(intel_crtc,
> >> >> > +rate);
> >> >> > +
> >> >> > +	return nm ? nm->m : 0;
> >> >> > +}
> >> >> > +
> >> >> > +static int audio_config_dp_get_n(struct intel_crtc *intel_crtc,
> >> >> > +int
> >> >> > +rate) {
> >> >> > +	const struct dp_aud_n_m *nm =
> >> >> > audio_config_dp_get_n_m(intel_crtc,
> >> >> > +rate);
> >> >> > +
> >> >> > +	return nm ? nm->n : 0;
> >> >> > +}
> >> >> > +
> >> >> >  static const struct {
> >> >> >  	int clock;
> >> >> >  	u32 config;
> >> >> > @@ -225,8 +289,10 @@ hsw_dp_audio_config_update(struct
> >> >> > intel_crtc *intel_crtc, enum port port,
> >> >> >  			   const struct drm_display_mode
> *adjusted_mode)  {
> >> >> >  	struct drm_i915_private *dev_priv =
> >> >> > to_i915(intel_crtc->base.dev);
> >> >> > +	struct i915_audio_component *acomp = dev_priv-
> >> >> > >audio_component;
> >> >> > +	int rate = acomp ? acomp->aud_sample_rate[port] : 0;
> >> >> >  	enum pipe pipe = intel_crtc->pipe;
> >> >> > -	u32 tmp;
> >> >> > +	u32 tmp, n, m;
> >> >> >
> >> >> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> >> >> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX; @@ -234,7 +300,30
> @@
> >> >> > hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum
> >> >> > port port,
> >> >> >  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> >> >> >  	tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >> >> >
> >> >> > +	if (intel_crtc->config->port_clock == LC_540M ||
> >> >> > +	    intel_crtc->config->port_clock == LC_270M ||
> >> >> > +	    intel_crtc->config->port_clock == LC_162M) {
> >>
> >> Actually, this check could be removed. audio_config_dp_get_n_m will
> >> always return 0 if this doesn't hold. But not a big deal.
> >
> > Yes, we can remove it.
> >
> >>
> >> >> > +		n = audio_config_dp_get_n(intel_crtc, rate);
> >> >> > +		if (n != 0) {
> >> >> > +			tmp &= ~AUD_CONFIG_N_MASK;
> >> >> > +			tmp |= AUD_CONFIG_N(n);
> >> >> > +			tmp |= AUD_CONFIG_N_PROG_ENABLE;
> >> >> > +		} else {
> >> >> > +			DRM_DEBUG_KMS("no suitable N value is
> found\n");
> >> >>
> >> >> Could some comments be added here why we don't return if no valid
> >> >> N value is found?
> >> >
> >> > This is because although we don't need set N value here, but there
> >> > are some other bits to be set. So you can see we call
> >> > I915_WRITE(HSW_AUD_CFG(pipe), tmp); Later.
> >> >
> >> >> Do we still need to program write the register to let HW calculate
> >> >> N by
> >> itself?
> >> >
> >> > Ideally, all the frequency should be in the table. But if the
> >> > frequency is not in our table, we will let HW calculate N by itself.
> >> >
> >> >>
> >> >> > +		}
> >> >> > +	}
> >> >> > +
> >> >> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >> >>
> >> >> And if no valid N value is found, need we go ahead to check M value?
> >> >
> >> > If there is no valid N value, audio_config_dp_get_m(intel_crtc,
> >> > rate); will also return 0 and skip the m setting.
> >>
> >> If we can't find M/N values, the HSW_AUD_CFG register is set to use
> >> automatic N value. However, the HSW_AUD_M_CTS_ENABLE register is
> not
> >> updated at all, and could potentially have stale stuff. Do we need to
> >> clear the AUD_M_CTS_M_VALUE_INDEX and/or
> AUD_M_CTS_M_PROG_ENABLE bits?
> >>
> >> We only do this stuff in hsw_audio_codec_enable now. Do we need to do
> >> something in hsw_audio_codec_disable too?
> >
> > In _disable(), the audio doesn't work. So the value will not impact audio.
> > It will not be worse that we do not reset the value in _disable().
> > Without reset, the value is "cached". Next time, it may be a little
> > quicker for monitor response if we are using the same parameter, I think.
> >
> > Regards
> > Libin
> >
> >>
> >>
> >> BR,
> >> Jani.
> >>
> >>
> >> >
> >> > Regards,
> >> > Libin
> >> >
> >> >>
> >> >> > +
> >> >> > +	m = audio_config_dp_get_m(intel_crtc, rate);
> >> >> > +	if (m) {
> >> >> > +		tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
> >> >> > +		tmp &= ~AUD_CONFIG_M_MASK;
> >> >> > +		tmp |= m;
> >> >> > +		tmp |= AUD_M_CTS_M_VALUE_INDEX;
> >> >> > +		tmp |= AUD_M_CTS_M_PROG_ENABLE;
> >> >> > +		I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
> >> >> > +	}
> >> >> >  }
> >> >>
> >> >> Thanks
> >> >> Mengdong
> >> >>
> >> >> >  static void
> >> >> > @@ -267,6 +356,12 @@ hsw_hdmi_audio_config_update(struct
> >> intel_crtc
> >> >> > *intel_crtc, enum port port,
> >> >> >  	}
> >> >> >
> >> >> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >> >> > +
> >> >> > +	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
> >> >> > +	tmp &= ~AUD_CONFIG_M_MASK;
> >> >> > +	tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
> >> >> > +	tmp |= AUD_M_CTS_M_PROG_ENABLE;
> >> >> > +	I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
> >> >> >  }
> >> >> >
> >> >> >  static void
> >> >> > @@ -687,7 +782,8 @@ static int
> >> >> > i915_audio_component_sync_audio_rate(struct device *kdev, int
> port,
> >> >> >  	/* 1. get the pipe */
> >> >> >  	intel_encoder = get_saved_enc(dev_priv, port, pipe);
> >> >> >  	if (!intel_encoder || !intel_encoder->base.crtc ||
> >> >> > -	    intel_encoder->type != INTEL_OUTPUT_HDMI) {
> >> >> > +	    (intel_encoder->type != INTEL_OUTPUT_HDMI &&
> >> >> > +	     intel_encoder->type != INTEL_OUTPUT_DP)) {
> >> >> >  		DRM_DEBUG_KMS("Not valid for port %c\n",
> >> >> port_name(port));
> >> >> >  		err = -ENODEV;
> >> >> >  		goto unlock;
> >> >> > --
> >> >> > 2.1.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
> >>
> >> --
> >> Jani Nikula, Intel Open Source Technology Center
> 
> --
> 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 RESEND 9/9] drm/i915: set proper N/M in modeset

[Jani Nikula]

On Thu, 20 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
>> >> Any particular reason these M/N values are half of what they're in
>> >> table
>> >> 2-104 of DP 1.4 spec? (Admittedly the table is an informative
>> >> example.)
>> >
>> > For HDMI, we found only set N is enough. HW then can handle the
>> remaining.
>> 
>> I meant, the M and N values in this part of the dp_aud_n_m table are 1/2 of
>> what they are in the DP spec table. Why?
>
> Which table are you meaning? I calculate the values myself. I didn't find the
> full table in DP spec. I only find the table for 270MHz and 162MHz in 
> Table 2-50: Examples of Maud and Naud Values

Table 2-104 in DP 1.4. Maybe you're looking at an older version of the
spec.

BR,
Jani.


-- 
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 RESEND 9/9] drm/i915: set proper N/M in modeset

[Ville Syrjälä]

On Thu, Oct 20, 2016 at 02:34:34PM +0300, Jani Nikula wrote:
> On Thu, 20 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> >> >> Any particular reason these M/N values are half of what they're in
> >> >> table
> >> >> 2-104 of DP 1.4 spec? (Admittedly the table is an informative
> >> >> example.)
> >> >
> >> > For HDMI, we found only set N is enough. HW then can handle the
> >> remaining.
> >> 
> >> I meant, the M and N values in this part of the dp_aud_n_m table are 1/2 of
> >> what they are in the DP spec table. Why?
> >
> > Which table are you meaning? I calculate the values myself. I didn't find the
> > full table in DP spec. I only find the table for 270MHz and 162MHz in 
> > Table 2-50: Examples of Maud and Naud Values
> 
> Table 2-104 in DP 1.4. Maybe you're looking at an older version of the
> spec.

So it looks like they used the same M value for all link frequencies
in that example, which for 540M ends up being double what the minimal
accurate value is. But as both M and N are doubled the ratio is still
exactly the same.

And there is indeed a 64k and 128k bitrates added as well. Those are
not classified under HBR audio. But as those can't be expressed via
short audio descriptors in the EDID, I don't think they would be
accepted by ALSA. Or am I wrong? I do wonder how they are supposed to be
used though since there must be some way for the sink to advertise
them, otherwise I can't see the point in adding them.

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

Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset

[Yang, Libin]

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Thursday, October 20, 2016 8:03 PM
> To: Nikula, Jani <jani.nikula@intel.com>
> Cc: Yang, Libin <libin.yang@intel.com>; Lin, Mengdong
> <mengdong.lin@intel.com>; intel-gfx@lists.freedesktop.org; Zhang, Keqiao
> <keqiao.zhang@intel.com>; libin.yang@linux.intel.com; Pandiyan,
> Dhinakaran <dhinakaran.pandiyan@intel.com>; Zhao, Juan J
> <juan.j.zhao@intel.com>
> Subject: Re: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> modeset
> 
> On Thu, Oct 20, 2016 at 02:34:34PM +0300, Jani Nikula wrote:
> > On Thu, 20 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> > >> >> Any particular reason these M/N values are half of what they're
> > >> >> in table
> > >> >> 2-104 of DP 1.4 spec? (Admittedly the table is an informative
> > >> >> example.)
> > >> >
> > >> > For HDMI, we found only set N is enough. HW then can handle the
> > >> remaining.
> > >>
> > >> I meant, the M and N values in this part of the dp_aud_n_m table
> > >> are 1/2 of what they are in the DP spec table. Why?
> > >
> > > Which table are you meaning? I calculate the values myself. I didn't
> > > find the full table in DP spec. I only find the table for 270MHz and
> > > 162MHz in Table 2-50: Examples of Maud and Naud Values
> >
> > Table 2-104 in DP 1.4. Maybe you're looking at an older version of the
> > spec.
> 
> So it looks like they used the same M value for all link frequencies in that
> example, which for 540M ends up being double what the minimal accurate
> value is. But as both M and N are doubled the ratio is still exactly the same.

There is a formula in DP 1.2 spec:
Maud/Naud = 512 * fs / f_LS_Clk

Suppose all the values meeting the formula is acceptable.

> 
> And there is indeed a 64k and 128k bitrates added as well. Those are not
> classified under HBR audio. But as those can't be expressed via short audio
> descriptors in the EDID, I don't think they would be accepted by ALSA. Or am
> I wrong? I do wonder how they are supposed to be used though since there
> must be some way for the sink to advertise them, otherwise I can't see the
> point in adding them.

Oh, sorry, I was thinking 128k is HBR. I didn't see 64k and 128k description
in DP 1.2 spec. 

Regards,
Libin

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

Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset

[Yang, Libin]

Hi Jani,

> -----Original Message-----
> From: Nikula, Jani
> Sent: Thursday, October 20, 2016 7:35 PM
> To: Yang, Libin <libin.yang@intel.com>; Lin, Mengdong
> <mengdong.lin@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>; Zhang, Keqiao
> <keqiao.zhang@intel.com>; Zhao, Juan J <juan.j.zhao@intel.com>
> Subject: RE: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> modeset
> 
> On Thu, 20 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> >> >> Any particular reason these M/N values are half of what they're in
> >> >> table
> >> >> 2-104 of DP 1.4 spec? (Admittedly the table is an informative
> >> >> example.)
> >> >
> >> > For HDMI, we found only set N is enough. HW then can handle the
> >> remaining.
> >>
> >> I meant, the M and N values in this part of the dp_aud_n_m table are
> >> 1/2 of what they are in the DP spec table. Why?
> >
> > Which table are you meaning? I calculate the values myself. I didn't
> > find the full table in DP spec. I only find the table for 270MHz and
> > 162MHz in Table 2-50: Examples of Maud and Naud Values
> 
> Table 2-104 in DP 1.4. Maybe you're looking at an older version of the spec.

I was referring DP1.2. I don't have DP 1.4 spec on hand. But suppose
both table should be OK?

Regards,
Libin

> 
> BR,
> Jani.
> 
> 
> --
> 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 RESEND 9/9] drm/i915: set proper N/M in modeset

[Zhang, Keqiao]

Tested by: Keqiao, Zhang <keqiao.zhang@intel.com>

-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Nikula, Jani
Sent: Monday, October 10, 2016 11:04 PM
To: intel-gfx@lists.freedesktop.org
Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com; Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
Subject: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset

When modeset occurs and the LS_CLK is set to some special values in DP mode, the N/M need to be set manually if audio is playing. Otherwise the first several seconds may be silent in audio playback.

The relationship of Maud and Naud is expressed in the following equation:
Maud/Naud = 512 * fs / f_LS_Clk

Please refer VESA DisplayPort Standard spec for details.

Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |   7 +++
 drivers/gpu/drm/i915/intel_audio.c | 100 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 595d196f753f..8d9dbc7d5b32 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7359,6 +7359,13 @@ enum {
 #define _HSW_AUD_MISC_CTRL_B		0x65110
 #define HSW_AUD_MISC_CTRL(pipe)		_MMIO_PIPE(pipe, _HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
 
+#define _HSW_AUD_M_CTS_ENABLE_A		0x65028
+#define _HSW_AUD_M_CTS_ENABLE_B		0x65128
+#define HSW_AUD_M_CTS_ENABLE(pipe)	_MMIO_PIPE(pipe, _HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
+#define   AUD_M_CTS_M_VALUE_INDEX	(1 << 21)
+#define   AUD_M_CTS_M_PROG_ENABLE	(1 << 20)
+#define   AUD_CONFIG_M_MASK		0xfffff
+
 #define _HSW_AUD_DIP_ELD_CTRL_ST_A	0x650b4
 #define _HSW_AUD_DIP_ELD_CTRL_ST_B	0x651b4
 #define HSW_AUD_DIP_ELD_CTRL(pipe)	_MMIO_PIPE(pipe, _HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 81df29ca4947..0bc2701b6c9c 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -57,6 +57,70 @@
  * struct &i915_audio_component_audio_ops @audio_ops is called from i915 driver.
  */
 
+/* DP N/M table */
+#define LC_540M 540000
+#define LC_270M 270000
+#define LC_162M 162000
+
+struct dp_aud_n_m {
+	int sample_rate;
+	int clock;
+	u16 n;
+	u16 m;
+};
+
+static const struct dp_aud_n_m dp_aud_n_m[] = {
+	{ 192000, LC_540M, 5625, 1024 },
+	{ 176400, LC_540M, 9375, 1568 },
+	{ 96000, LC_540M, 5625, 512 },
+	{ 88200, LC_540M, 9375, 784 },
+	{ 48000, LC_540M, 5625, 256 },
+	{ 44100, LC_540M, 9375, 392 },
+	{ 32000, LC_540M, 16875, 512 },
+	{ 192000, LC_270M, 5625, 2048 },
+	{ 176400, LC_270M, 9375, 3136 },
+	{ 96000, LC_270M, 5625, 1024 },
+	{ 88200, LC_270M, 9375, 1568 },
+	{ 48000, LC_270M, 5625, 512 },
+	{ 44100, LC_270M, 9375, 784 },
+	{ 32000, LC_270M, 16875, 1024 },
+	{ 192000, LC_162M, 3375, 2048 },
+	{ 176400, LC_162M, 5625, 3136 },
+	{ 96000, LC_162M, 3375, 1024 },
+	{ 88200, LC_162M, 5625, 1568 },
+	{ 48000, LC_162M, 3375, 512 },
+	{ 44100, LC_162M, 5625, 784 },
+	{ 32000, LC_162M, 10125, 1024 },
+};
+
+static const struct dp_aud_n_m *
+audio_config_dp_get_n_m(struct intel_crtc *intel_crtc, int rate) {
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
+		if (rate == dp_aud_n_m[i].sample_rate &&
+		    intel_crtc->config->port_clock == dp_aud_n_m[i].clock)
+			return &dp_aud_n_m[i];
+	}
+
+	return NULL;
+}
+
+static int audio_config_dp_get_m(struct intel_crtc *intel_crtc, int 
+rate) {
+	const struct dp_aud_n_m *nm = audio_config_dp_get_n_m(intel_crtc, 
+rate);
+
+	return nm ? nm->m : 0;
+}
+
+static int audio_config_dp_get_n(struct intel_crtc *intel_crtc, int 
+rate) {
+	const struct dp_aud_n_m *nm = audio_config_dp_get_n_m(intel_crtc, 
+rate);
+
+	return nm ? nm->n : 0;
+}
+
 static const struct {
 	int clock;
 	u32 config;
@@ -225,8 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
 			   const struct drm_display_mode *adjusted_mode)  {
 	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+	struct i915_audio_component *acomp = dev_priv->audio_component;
+	int rate = acomp ? acomp->aud_sample_rate[port] : 0;
 	enum pipe pipe = intel_crtc->pipe;
-	u32 tmp;
+	u32 tmp, n, m;
 
 	tmp = I915_READ(HSW_AUD_CFG(pipe));
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
@@ -234,7 +300,30 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
 	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
 	tmp |= AUD_CONFIG_N_VALUE_INDEX;
 
+	if (intel_crtc->config->port_clock == LC_540M ||
+	    intel_crtc->config->port_clock == LC_270M ||
+	    intel_crtc->config->port_clock == LC_162M) {
+		n = audio_config_dp_get_n(intel_crtc, rate);
+		if (n != 0) {
+			tmp &= ~AUD_CONFIG_N_MASK;
+			tmp |= AUD_CONFIG_N(n);
+			tmp |= AUD_CONFIG_N_PROG_ENABLE;
+		} else {
+			DRM_DEBUG_KMS("no suitable N value is found\n");
+		}
+	}
+
 	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+
+	m = audio_config_dp_get_m(intel_crtc, rate);
+	if (m) {
+		tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
+		tmp &= ~AUD_CONFIG_M_MASK;
+		tmp |= m;
+		tmp |= AUD_M_CTS_M_VALUE_INDEX;
+		tmp |= AUD_M_CTS_M_PROG_ENABLE;
+		I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
+	}
 }
 
 static void
@@ -267,6 +356,12 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
 	}
 
 	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+
+	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
+	tmp &= ~AUD_CONFIG_M_MASK;
+	tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
+	tmp |= AUD_M_CTS_M_PROG_ENABLE;
+	I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
 }
 
 static void
@@ -687,7 +782,8 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
 	/* 1. get the pipe */
 	intel_encoder = get_saved_enc(dev_priv, port, pipe);
 	if (!intel_encoder || !intel_encoder->base.crtc ||
-	    intel_encoder->type != INTEL_OUTPUT_HDMI) {
+	    (intel_encoder->type != INTEL_OUTPUT_HDMI &&
+	     intel_encoder->type != INTEL_OUTPUT_DP)) {
 		DRM_DEBUG_KMS("Not valid for port %c\n", port_name(port));
 		err = -ENODEV;
 		goto unlock;
--
2.1.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

✗ Fi.CI.BAT: warning for drm/i915/audio: audio cleanups, 4k fixes (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/audio: audio cleanups, 4k fixes (rev3)
URL   : https://patchwork.freedesktop.org/series/12754/
State : warning

== Summary ==

Series 12754v3 drm/i915/audio: audio cleanups, 4k fixes
https://patchwork.freedesktop.org/api/1.0/series/12754/revisions/3/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-WARN (fi-skl-6700k)
Test kms_psr_sink_crc:
        Subgroup psr_basic:
                pass       -> DMESG-WARN (fi-skl-6700hq)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-WARN (fi-skl-6770hq)
Test vgem_basic:
        Subgroup unload:
                skip       -> PASS       (fi-bsw-n3050)
                skip       -> PASS       (fi-hsw-4770)
                pass       -> SKIP       (fi-ilk-650)

fi-bdw-5557u     total:248  pass:231  dwarn:0   dfail:0   fail:0   skip:17 
fi-bsw-n3050     total:248  pass:205  dwarn:0   dfail:0   fail:0   skip:43 
fi-bxt-t5700     total:248  pass:217  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-j1900     total:248  pass:214  dwarn:1   dfail:0   fail:1   skip:32 
fi-hsw-4770      total:248  pass:225  dwarn:0   dfail:0   fail:0   skip:23 
fi-hsw-4770r     total:248  pass:224  dwarn:0   dfail:0   fail:0   skip:24 
fi-ilk-650       total:248  pass:184  dwarn:0   dfail:0   fail:2   skip:62 
fi-ivb-3520m     total:248  pass:221  dwarn:0   dfail:0   fail:0   skip:27 
fi-ivb-3770      total:248  pass:207  dwarn:0   dfail:0   fail:0   skip:41 
fi-kbl-7200u     total:248  pass:222  dwarn:0   dfail:0   fail:0   skip:26 
fi-skl-6260u     total:248  pass:232  dwarn:0   dfail:0   fail:0   skip:16 
fi-skl-6700hq    total:248  pass:223  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6700k     total:248  pass:220  dwarn:2   dfail:0   fail:0   skip:26 
fi-skl-6770hq    total:248  pass:230  dwarn:2   dfail:0   fail:1   skip:15 
fi-snb-2520m     total:248  pass:211  dwarn:0   dfail:0   fail:0   skip:37 
fi-snb-2600      total:248  pass:209  dwarn:0   dfail:0   fail:0   skip:39 

Results at /archive/results/CI_IGT_test/Patchwork_2663/

e37a15c8d775e79dddc8345a0f6afdcfe1f607d9 drm-intel-nightly: 2016y-10m-10d-14h-33m-29s UTC integration manifest
ceae7cb drm/i915: set proper N/M in modeset
831ac5b drm/i915/audio: rename N value getter to emphasize it's for hdmi
a12ba34 drm/i915/audio: add register macros for audio config N value
d887d42 drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock
ba64534 drm/i915/audio: set proper N/MCTS on more platforms
cc137fd drm/i915/audio: split dp and hdmi audio config update
293d738 drm/i915/audio: use the same code for updating audio config
f04978a drm/i915/audio: port is going to be just fine, simplify checks
5c152ee drm/i915/audio: abstract audio config update

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