[v5,4/4] drm/i915: set proper N/CTS in modeset
diff mbox

Message ID 1439880714-40931-4-git-send-email-libin.yang@intel.com
State New
Headers show

Commit Message

Yang, Libin Aug. 18, 2015, 6:51 a.m. UTC
From: Libin Yang <libin.yang@intel.com>

When modeset occurs and the TMDS frequency is set to some
speical values, the N/CTS need to be set manually if audio
is playing.

Signed-off-by: Libin Yang <libin.yang@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  8 ++++++++
 drivers/gpu/drm/i915/intel_audio.c | 40 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 47 insertions(+), 1 deletion(-)

Comments

Ville Syrjälä Aug. 21, 2015, 3:26 p.m. UTC | #1
On Tue, Aug 18, 2015 at 02:51:54PM +0800, libin.yang@intel.com wrote:
> From: Libin Yang <libin.yang@intel.com>
> 
> When modeset occurs and the TMDS frequency is set to some
> speical values, the N/CTS need to be set manually if audio
> is playing.
> 
> Signed-off-by: Libin Yang <libin.yang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  8 ++++++++
>  drivers/gpu/drm/i915/intel_audio.c | 40 +++++++++++++++++++++++++++++++++++++-
>  2 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6786e94..122b5bd 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7035,6 +7035,8 @@ enum skl_disp_power_wells {
>  					_HSW_AUD_MISC_CTRL_A, \
>  					_HSW_AUD_MISC_CTRL_B)
>  
> +#define HSW_AUD_PIPE_CONN_SEL_CTRL  0x650ac
> +
>  #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) _PIPE(pipe, \
> @@ -7049,6 +7051,12 @@ enum skl_disp_power_wells {
>  					_HSW_AUD_DIG_CNVT_2)
>  #define DIP_PORT_SEL_MASK		0x3
>  
> +#define _HSW_AUD_STR_DESC_1		0x65084
> +#define _HSW_AUD_STR_DESC_2		0x65184
> +#define AUD_STR_DESC(pipe)	_PIPE(pipe, \
> +					 _HSW_AUD_STR_DESC_1,	\
> +					 _HSW_AUD_STR_DESC_2)
> +
>  #define _HSW_AUD_EDID_DATA_A		0x65050
>  #define _HSW_AUD_EDID_DATA_B		0x65150
>  #define HSW_AUD_EDID_DATA(pipe) _PIPE(pipe, \
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 96b97be..0dfdc77 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -140,6 +140,27 @@ static bool audio_rate_need_prog(struct intel_crtc *crtc,
>  		return false;
>  }
>  
> +static int audio_config_get_rate(struct drm_i915_private *dev_priv,
> +						enum pipe pipe)
> +{
> +	uint32_t tmp;
> +	int cvt_idx;
> +	int base_rate, mul, div, rate;
> +
> +	tmp = I915_READ(HSW_AUD_PIPE_CONN_SEL_CTRL);
> +	cvt_idx = (tmp >> (pipe * 8)) & 0xff;
> +	tmp = I915_READ(AUD_STR_DESC(cvt_idx));
> +	base_rate = tmp & (1 << 14);
> +	if (base_rate == 0)
> +		rate = 48000;
> +	else
> +		rate = 44100;
> +	mul = (tmp & (0x7 << 11)) + 1;
> +	div = (tmp & (0x7 << 8)) + 1;
> +	rate = rate * mul / div;
> +	return rate;
> +}

Given your new .sync_audio_rate() callback, the audio driver should have
already told us what the current sample rate is, and so we shouldn't
have to go dig it up from registers.

> +
>  static bool intel_eld_uptodate(struct drm_connector *connector,
>  			       int reg_eldv, uint32_t bits_eldv,
>  			       int reg_elda, uint32_t bits_elda,
> @@ -261,6 +282,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  	const uint8_t *eld = connector->eld;
>  	uint32_t tmp;
>  	int len, i;
> +	int n_low, n_up, n;
> +	int rate;
>  
>  	DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
>  		      pipe_name(pipe), drm_eld_size(eld));
> @@ -296,12 +319,27 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>  	/* Enable timestamps */
>  	tmp = I915_READ(HSW_AUD_CFG(pipe));
>  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> -	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>  	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
>  	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
>  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
>  	else
>  		tmp |= audio_config_hdmi_pixel_clock(mode);
> +
> +	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> +	if (audio_rate_eed_prog(intel_crtc, mode)) {
> +		rate = audio_config_get_rate(dev_priv, pipe);
> +		n = audio_config_get_n(mode, rate);
> +		if (n != 0) {
> +			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);
> +		}
> +	}

We should share the code to set this up with .sync_audio_rate() rather
than having two copies of essentically the same code.

> +
>  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>  }
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Yang, Libin Aug. 24, 2015, 1:53 a.m. UTC | #2
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Friday, August 21, 2015 11:26 PM
> To: Yang, Libin
> Cc: alsa-devel@alsa-project.org; tiwai@suse.de; intel-
> gfx@lists.freedesktop.org; daniel.vetter@ffwll.ch;
> jani.nikula@linux.intel.com
> Subject: Re: [Intel-gfx] [PATCH v5 4/4] drm/i915: set proper N/CTS in
> modeset
> 
> On Tue, Aug 18, 2015 at 02:51:54PM +0800, libin.yang@intel.com
> wrote:
> > From: Libin Yang <libin.yang@intel.com>
> >
> > When modeset occurs and the TMDS frequency is set to some
> > speical values, the N/CTS need to be set manually if audio
> > is playing.
> >
> > Signed-off-by: Libin Yang <libin.yang@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h    |  8 ++++++++
> >  drivers/gpu/drm/i915/intel_audio.c | 40
> +++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 47 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> > index 6786e94..122b5bd 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7035,6 +7035,8 @@ enum skl_disp_power_wells {
> >  					_HSW_AUD_MISC_CTRL_A, \
> >  					_HSW_AUD_MISC_CTRL_B)
> >
> > +#define HSW_AUD_PIPE_CONN_SEL_CTRL  0x650ac
> > +
> >  #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) _PIPE(pipe, \
> > @@ -7049,6 +7051,12 @@ enum skl_disp_power_wells {
> >  					_HSW_AUD_DIG_CNVT_2)
> >  #define DIP_PORT_SEL_MASK		0x3
> >
> > +#define _HSW_AUD_STR_DESC_1		0x65084
> > +#define _HSW_AUD_STR_DESC_2		0x65184
> > +#define AUD_STR_DESC(pipe)	_PIPE(pipe, \
> > +					 _HSW_AUD_STR_DESC_1,
> 	\
> > +					 _HSW_AUD_STR_DESC_2)
> > +
> >  #define _HSW_AUD_EDID_DATA_A		0x65050
> >  #define _HSW_AUD_EDID_DATA_B		0x65150
> >  #define HSW_AUD_EDID_DATA(pipe) _PIPE(pipe, \
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> > index 96b97be..0dfdc77 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -140,6 +140,27 @@ static bool audio_rate_need_prog(struct
> intel_crtc *crtc,
> >  		return false;
> >  }
> >
> > +static int audio_config_get_rate(struct drm_i915_private *dev_priv,
> > +						enum pipe pipe)
> > +{
> > +	uint32_t tmp;
> > +	int cvt_idx;
> > +	int base_rate, mul, div, rate;
> > +
> > +	tmp = I915_READ(HSW_AUD_PIPE_CONN_SEL_CTRL);
> > +	cvt_idx = (tmp >> (pipe * 8)) & 0xff;
> > +	tmp = I915_READ(AUD_STR_DESC(cvt_idx));
> > +	base_rate = tmp & (1 << 14);
> > +	if (base_rate == 0)
> > +		rate = 48000;
> > +	else
> > +		rate = 44100;
> > +	mul = (tmp & (0x7 << 11)) + 1;
> > +	div = (tmp & (0x7 << 8)) + 1;
> > +	rate = rate * mul / div;
> > +	return rate;
> > +}
> 
> Given your new .sync_audio_rate() callback, the audio driver should
> have
> already told us what the current sample rate is, and so we shouldn't
> have to go dig it up from registers.

This patch is not calling sync_audio_rate() from audio driver.
It happens when gfx doing the modeset.

> 
> > +
> >  static bool intel_eld_uptodate(struct drm_connector *connector,
> >  			       int reg_eldv, uint32_t bits_eldv,
> >  			       int reg_elda, uint32_t bits_elda,
> > @@ -261,6 +282,8 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> >  	const uint8_t *eld = connector->eld;
> >  	uint32_t tmp;
> >  	int len, i;
> > +	int n_low, n_up, n;
> > +	int rate;
> >
> >  	DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes
> ELD\n",
> >  		      pipe_name(pipe), drm_eld_size(eld));
> > @@ -296,12 +319,27 @@ static void
> hsw_audio_codec_enable(struct drm_connector *connector,
> >  	/* Enable timestamps */
> >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> >  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > -	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> >  	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> >  	if (intel_pipe_has_type(intel_crtc,
> INTEL_OUTPUT_DISPLAYPORT))
> >  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >  	else
> >  		tmp |= audio_config_hdmi_pixel_clock(mode);
> > +
> > +	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> > +	if (audio_rate_eed_prog(intel_crtc, mode)) {
> > +		rate = audio_config_get_rate(dev_priv, pipe);
> > +		n = audio_config_get_n(mode, rate);
> > +		if (n != 0) {
> > +			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);
> > +		}
> > +	}
> 
> We should share the code to set this up with .sync_audio_rate() rather
> than having two copies of essentically the same code.

It's only setting a variable tmp, not sure we really need
a separate function to do the setting?

> 
> > +
> >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >  }
> >
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6786e94..122b5bd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7035,6 +7035,8 @@  enum skl_disp_power_wells {
 					_HSW_AUD_MISC_CTRL_A, \
 					_HSW_AUD_MISC_CTRL_B)
 
+#define HSW_AUD_PIPE_CONN_SEL_CTRL  0x650ac
+
 #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) _PIPE(pipe, \
@@ -7049,6 +7051,12 @@  enum skl_disp_power_wells {
 					_HSW_AUD_DIG_CNVT_2)
 #define DIP_PORT_SEL_MASK		0x3
 
+#define _HSW_AUD_STR_DESC_1		0x65084
+#define _HSW_AUD_STR_DESC_2		0x65184
+#define AUD_STR_DESC(pipe)	_PIPE(pipe, \
+					 _HSW_AUD_STR_DESC_1,	\
+					 _HSW_AUD_STR_DESC_2)
+
 #define _HSW_AUD_EDID_DATA_A		0x65050
 #define _HSW_AUD_EDID_DATA_B		0x65150
 #define HSW_AUD_EDID_DATA(pipe) _PIPE(pipe, \
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 96b97be..0dfdc77 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -140,6 +140,27 @@  static bool audio_rate_need_prog(struct intel_crtc *crtc,
 		return false;
 }
 
+static int audio_config_get_rate(struct drm_i915_private *dev_priv,
+						enum pipe pipe)
+{
+	uint32_t tmp;
+	int cvt_idx;
+	int base_rate, mul, div, rate;
+
+	tmp = I915_READ(HSW_AUD_PIPE_CONN_SEL_CTRL);
+	cvt_idx = (tmp >> (pipe * 8)) & 0xff;
+	tmp = I915_READ(AUD_STR_DESC(cvt_idx));
+	base_rate = tmp & (1 << 14);
+	if (base_rate == 0)
+		rate = 48000;
+	else
+		rate = 44100;
+	mul = (tmp & (0x7 << 11)) + 1;
+	div = (tmp & (0x7 << 8)) + 1;
+	rate = rate * mul / div;
+	return rate;
+}
+
 static bool intel_eld_uptodate(struct drm_connector *connector,
 			       int reg_eldv, uint32_t bits_eldv,
 			       int reg_elda, uint32_t bits_elda,
@@ -261,6 +282,8 @@  static void hsw_audio_codec_enable(struct drm_connector *connector,
 	const uint8_t *eld = connector->eld;
 	uint32_t tmp;
 	int len, i;
+	int n_low, n_up, n;
+	int rate;
 
 	DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
 		      pipe_name(pipe), drm_eld_size(eld));
@@ -296,12 +319,27 @@  static void hsw_audio_codec_enable(struct drm_connector *connector,
 	/* Enable timestamps */
 	tmp = I915_READ(HSW_AUD_CFG(pipe));
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
-	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
 	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
 	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	else
 		tmp |= audio_config_hdmi_pixel_clock(mode);
+
+	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+	if (audio_rate_need_prog(intel_crtc, mode)) {
+		rate = audio_config_get_rate(dev_priv, pipe);
+		n = audio_config_get_n(mode, rate);
+		if (n != 0) {
+			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);
+		}
+	}
+
 	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
 }