[Intel-gfx] [PATCH v3 1/3] drm/i915: set proper N/M in modeset

Ville Syrjälä ville.syrjala at linux.intel.com
Fri Aug 5 08:45:02 UTC 2016


On Fri, Aug 05, 2016 at 07:24:14AM +0000, Yang, Libin wrote:
> 
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > Sent: Friday, August 5, 2016 3:17 PM
> > To: Yang, Libin <libin.yang at intel.com>
> > Cc: libin.yang at linux.intel.com; intel-gfx at lists.freedesktop.org;
> > jani.nikula at linux.intel.com; Vetter, Daniel <daniel.vetter at intel.com>;
> > tiwai at suse.de
> > Subject: Re: [PATCH v3 1/3] drm/i915: set proper N/M in modeset
> > 
> > On Fri, Aug 05, 2016 at 06:40:45AM +0000, Yang, Libin wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > > > Sent: Friday, August 5, 2016 2:36 PM
> > > > To: Yang, Libin <libin.yang at intel.com>
> > > > Cc: libin.yang at linux.intel.com; intel-gfx at lists.freedesktop.org;
> > > > jani.nikula at linux.intel.com; Vetter, Daniel
> > > > <daniel.vetter at intel.com>; tiwai at suse.de
> > > > Subject: Re: [PATCH v3 1/3] drm/i915: set proper N/M in modeset
> > > >
> > > > On Fri, Aug 05, 2016 at 06:16:59AM +0000, Yang, Libin wrote:
> > > > > Hi Ville,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > > > > > Sent: Friday, August 5, 2016 1:57 PM
> > > > > > To: Yang, Libin <libin.yang at intel.com>
> > > > > > Cc: libin.yang at linux.intel.com; intel-gfx at lists.freedesktop.org;
> > > > > > jani.nikula at linux.intel.com; Vetter, Daniel
> > > > > > <daniel.vetter at intel.com>; tiwai at suse.de
> > > > > > Subject: Re: [PATCH v3 1/3] drm/i915: set proper N/M in modeset
> > > > > >
> > > > > > On Fri, Aug 05, 2016 at 05:39:19AM +0000, Yang, Libin wrote:
> > > > > > > Hi Ville,
> > > > > > >
> > > > > > > Sorry for delay reply. Please see my comments below.
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > > > > > > > Sent: Thursday, August 4, 2016 4:53 PM
> > > > > > > > To: libin.yang at linux.intel.com
> > > > > > > > Cc: intel-gfx at lists.freedesktop.org;
> > > > > > > > jani.nikula at linux.intel.com; Vetter, Daniel
> > > > > > > > <daniel.vetter at intel.com>; tiwai at suse.de; Yang, Libin
> > > > > > > > <libin.yang at intel.com>
> > > > > > > > Subject: Re: [PATCH v3 1/3] drm/i915: set proper N/M in
> > > > > > > > modeset
> > > > > > > >
> > > > > > > > On Thu, Aug 04, 2016 at 03:58:02PM +0800,
> > > > > > > > libin.yang at linux.intel.com
> > > > > > wrote:
> > > > > > > > > From: Libin Yang <libin.yang at linux.intel.com>
> > > > > > > > >
> > > > > > > > > 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.
> > > > > > > >
> > > > > > > > Pleas include a small changelog here so that I know what has
> > > > > > > > changed with each revision of the patch. Eg:
> > > > > > > >
> > > > > > > > v2: blah
> > > > > > > > v3: whatever
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Signed-off-by: Libin Yang <libin.yang at linux.intel.com>
> > > > > > > > > ---
> > > > > > > > >  drivers/gpu/drm/i915/i915_reg.h    |   6 ++
> > > > > > > > >  drivers/gpu/drm/i915/intel_audio.c | 133
> > > > > > > > > ++++++++++++++++++++++++++++++++-----
> > > > > > > > >  2 files changed, 123 insertions(+), 16 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > > > > > > b/drivers/gpu/drm/i915/i915_reg.h index 8bfde75..2f9d00e
> > > > > > > > > 100644
> > > > > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > > > > > @@ -7351,6 +7351,12 @@ enum {
> > > > > > > > >  #define _HSW_AUD_CONFIG_B		0x65100
> > > > > > > > >  #define HSW_AUD_CFG(pipe)		_MMIO_PIPE(pipe,
> > > > > > > > _HSW_AUD_CONFIG_A, _HSW_AUD_CONFIG_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)
> > > > > > > >
> > > > > > > > Please try to keep the register defines in order by the register offset.
> > > > > > > > Ie. these registers should be placed just after HSW_AUD_MISC_CTRL.
> > > > > > > >
> > > > > > > > > +
> > > > > > > > >  #define _HSW_AUD_MISC_CTRL_A		0x65010
> > > > > > > > >  #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)
> > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> > > > > > > > > b/drivers/gpu/drm/i915/intel_audio.c
> > > > > > > > > index 6700a7b..04e7358 100644
> > > > > > > > > --- a/drivers/gpu/drm/i915/intel_audio.c
> > > > > > > > > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > > > > > > > > @@ -98,6 +98,35 @@ static const struct {
> > > > > > > > >  	{ 192000, TMDS_297M, 20480, 247500 },  };
> > > > > > > > >
> > > > > > > > > +#define LC_540M 540000
> > > > > > > > > +#define LC_270M 270000
> > > > > > > > > +#define LC_162M 162000
> > > > > > > > > +static const struct {
> > > > > > > > > +	int sample_rate;
> > > > > > > > > +	int clock;
> > > > > > > > > +	u16 n;
> > > > > > > > > +	u16 m;
> > > > > > > > > +} aud_nm[] = {
> > > > > > > > > +	{192000, LC_540M, 5625, 1024},
> > > > > > > >          ^                          ^
> > > > > > > >
> > > > > > > > Style nit: please add spaces here
> > > > > > > >
> > > > > > > > > +	{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},
> > > > > > > > > +	{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},
> > > > > > > > > +	{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},
> > > > > > > >
> > > > > > > > The numbers looks good. But looks like you forgot about 176.4 kHz?
> > > > > > > >
> > > > > > > > > +};
> > > > > > > > > +
> > > > > > > > >  /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
> > > > > > > > > static
> > > > > > > > > u32 audio_config_hdmi_pixel_clock(const struct
> > > > > > > > > drm_display_mode
> > > > > > > > > *adjusted_mode)  { @@ -121,20 +150,50 @@ 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(struct intel_crtc *crtc,
> > > > > > > > > +			      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)) {
> > > > > > > > > -			return aud_ncts[i].n;
> > > > > > > > > +	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI)) {
> > > > > > > > > +		for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> > > > > > > > > +			if ((rate == aud_ncts[i].sample_rate) &&
> > > > > > > > > +				(adjusted_mode->clock ==
> > > > aud_ncts[i].clock)) {
> > > > > > > >
> > > > > > > > Indent fail, and a bit too many parens for my taste. Should look like:
> > > > > > > >
> > > > > > > > 		if (rate == aud_ncts[i].sample_rate &&
> > > > > > > > 		    adjusted_mode->clock == aud_ncts[i].clock) {
> > > > > > > >
> > > > > > > >
> > > > > > > > > +				return aud_ncts[i].n;
> > > > > > > > > +			}
> > > > > > > > >  		}
> > > > > > > > >  	}
> > > > > > > > > +
> > > > > > > > > +	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP))
> > > > > > > > > +{
> > > > > > > >
> > > > > > > > intel_has_dp_encoder() will make this work for MST as well.
> > > > > > > >
> > > > > > > > "In MST mode, the Sink device must ignore the Maud and Mvid
> > > > > > > > values sent by an MST Source device unless the MST Sink
> > > > > > > > device is directly connected to the MST Source device via a
> > > > > > > > single link as the Source device generates those values
> > > > > > > > based on the LS_Clk of the link it is driving. An MST Sink
> > > > > > > > device must be able to regenerate a stream clock without
> > > > > > > > depending on Mvid and Mvid time stamp values because the MST
> > > > > > > > Sink device might be plugged into an MST Branch device, not
> > > > > > > > an
> > > > > > MST Source device."
> > > > > > > >
> > > > > > > > So if I'm reading that right, an MST sink can still use the
> > > > > > > > M/N values if it's direclty hooked up to a MST source. Hence
> > > > > > > > we probably want to fill out the M/N even for MST.
> > > > > > >
> > > > > > > My understanding is this is the requirement for sink device.
> > > > > > > Non-mst and MST, source will send the N/M value, and if it is
> > > > > > > Non-mst/single link, sink device should use these data to
> > > > > > > generate the clock. If it is in MST, sink device should ignore
> > > > > > > the data and regenerate the clock without depending on N/M value.
> > > > > > >
> > > > > > > So this code should OK be for both non-mst and MST. If not, I
> > > > > > > think we can make a separate patch for DP MST.
> > > > > >
> > > > > > It's only OK for SST. We could of course delay dealing with MST,
> > > > > > but that seems like unnecessary churn. You're touching all these
> > > > > > places in this patch, so might as well use intel_dp_encoder()
> > > > > > from the start. It will make the MST patch smaller too, which
> > > > > > will make it easier to
> > > > review.
> > > > >
> > > > > OK, I will use intel_crtc_has_dp_encoder(). Hope it still works
> > > > > for DP MST :)
> > > > >
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > > +		for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
> > > > > > > > > +			if ((rate == aud_nm[i].sample_rate) &&
> > > > > > > > > +				(crtc->config->port_clock ==
> > > > aud_nm[i].clock))
> > > > > > > > {
> > > > > > > >
> > > > > > > > indent + parens
> > > > > > > >
> > > > > > > > > +				return aud_nm[i].n;
> > > > > > > > > +			}
> > > > > > > > > +		}
> > > > > > > > > +	}
> > > > > > > > > +	return 0;
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > > +static int audio_config_get_m(struct intel_crtc *crtc, int rate) {
> > > > > > > > > +	int i;
> > > > > > > > > +
> > > > > > > > > +	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP))
> > > > > > > > > +{
> > > > > > > >
> > > > > > > > intel_has_dp_encoder()
> > > > > > > >
> > > > > > > > > +		for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
> > > > > > > > > +			if ((rate == aud_nm[i].sample_rate) &&
> > > > > > > > > +				(crtc->config->port_clock ==
> > > > aud_nm[i].clock))
> > > > > > > > {
> > > > > > > >
> > > > > > > > indent + parens
> > > > > > > >
> > > > > > > > > +				return aud_nm[i].m;
> > > > > > > > > +			}
> > > > > > > > > +		}
> > > > > > > > > +	}
> > > > > > > > > +
> > > > > > > > >  	return 0;
> > > > > > > > >  }
> > > > > > > > >
> > > > > > > > > -static uint32_t audio_config_setup_n_reg(int n, uint32_t
> > > > > > > > > val)
> > > > > > > > > +static uint32_t audio_config_setup_n_reg(struct intel_crtc *crtc,
> > > > > > > > > +					 int n, uint32_t val)
> > > > > > > > >  {
> > > > > > > > >  	int n_low, n_up;
> > > > > > > > >  	uint32_t tmp = val;
> > > > > > > > > @@ -145,17 +204,39 @@ static uint32_t
> > > > > > > > > audio_config_setup_n_reg(int n,
> > > > > > > > uint32_t val)
> > > > > > > > >  	tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
> > > > > > > > >  			(n_low << AUD_CONFIG_LOWER_N_SHIFT) |
> > > > > > > > >  			AUD_CONFIG_N_PROG_ENABLE);
> > > > > > > > > +	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP))
> > > > > > > >
> > > > > > > > intel_has_dp_encoder()
> > > > > > > >
> > > > > > > > > +		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> > > > > > > > > +	return tmp;
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > > +static uint32_t audio_config_setup_m_reg(struct intel_crtc *crtc,
> > > > > > > > > +					 int m, uint32_t val) {
> > > > > > > > > +	uint32_t tmp = val;
> > > > > > > > > +
> > > > > > > > > +	if (!intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP))
> > > > > > > > > +		return 0;
> > > > > > > >
> > > > > > > > The caller already checked that.
> > > > > > > >
> > > > > > > > > +
> > > > > > > > > +	tmp |= m;
> > > > > > > > > +	tmp |= AUD_M_CTS_M_VALUE_INDEX;
> > > > > > > > > +	tmp |= AUD_M_CTS_M_PROG_ENABLE;
> > > > > > > > > +
> > > > > > > > >  	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)
> > > > > > > > > +				 const struct drm_display_mode
> > > > > > > > *adjusted_mode)
> > > > > > > > >  {
> > > > > > > > > -	if (((mode->clock == TMDS_297M) ||
> > > > > > > > > -		 (mode->clock == TMDS_296M)) &&
> > > > > > > > > +	if (((adjusted_mode->clock == TMDS_297M) ||
> > > > > > > > > +		 (adjusted_mode->clock == TMDS_296M)) &&
> > > > > > > > >  		intel_crtc_has_type(crtc->config,
> > > > INTEL_OUTPUT_HDMI))
> > > > > > > > >  		return true;
> > > > > > > > > +	else if (((crtc->config->port_clock == LC_540M) ||
> > > > > > > > > +		  (crtc->config->port_clock == LC_270M) ||
> > > > > > > > > +		  (crtc->config->port_clock == LC_162M)) &&
> > > > > > > > > +		  intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP))
> > > > > > > >
> > > > > > > > intel_has_dp_encoder()
> > > > > > > >
> > > > > > > > > +		return true;
> > > > > > > > >  	else
> > > > > > > > >  		return false;
> > > > > > > > >  }
> > > > > > > > > @@ -287,7 +368,7 @@ static void
> > > > > > > > > hsw_audio_codec_enable(struct
> > > > > > > > drm_connector *connector,
> > > > > > > > >  	struct intel_digital_port *intel_dig_port =
> > > > > > > > >  		enc_to_dig_port(&encoder->base);
> > > > > > > > >  	enum port port = intel_dig_port->port;
> > > > > > > > > -	uint32_t tmp;
> > > > > > > > > +	uint32_t tmp, m;
> > > > > > > > >  	int len, i;
> > > > > > > > >  	int n, rate;
> > > > > > > > >
> > > > > > > > > @@ -343,15 +424,25 @@ static void
> > > > > > > > > hsw_audio_codec_enable(struct
> > > > > > > > drm_connector *connector,
> > > > > > > > >  			DRM_ERROR("invalid port: %d\n", port);
> > > > > > > > >  			rate = 0;
> > > > > > > > >  		}
> > > > > > > > > -		n = audio_config_get_n(adjusted_mode, rate);
> > > > > > > > > +		n = audio_config_get_n(intel_crtc, adjusted_mode,
> > > > rate);
> > > > > > > > >  		if (n != 0)
> > > > > > > > > -			tmp = audio_config_setup_n_reg(n, tmp);
> > > > > > > > > +			tmp = audio_config_setup_n_reg(intel_crtc, n,
> > > > tmp);
> > > > > > > > >  		else
> > > > > > > > >  			DRM_DEBUG_KMS("no suitable N value is
> > > > found\n");
> > > > > > > > >  	}
> > > > > > > > >
> > > > > > > > >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > > > > > > > >
> > > > > > > > > +	/* setup m value for DP */
> > > > > > > > > +	if (intel_crtc_has_type(intel_crtc->config,
> > > > > > > > > +INTEL_OUTPUT_DP)) {
> > > > > > > >
> > > > > > > > intel_has_dp_encoder()
> > > > > > > >
> > > > > > > > > +		m = audio_config_get_m(intel_crtc, rate);
> > > > > > > > > +		if (m != 0) {
> > > > > > > > > +			tmp =
> > > > I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
> > > > > > > > > +			tmp = audio_config_setup_m_reg(intel_crtc,
> > > > m, tmp);
> > > > > > > > > +			I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe),
> > > > tmp);
> > > > > > > >
> > > > > > > > We should program this register even for HDMI, so that we
> > > > > > > > don't leak invalid register values eg. when changing from DP->HDMI.
> > > > > > >
> > > > > > > HDMI doesn't need set these values based on our test. It seems
> > > > > > > silicon can handle smoothly for HDMI.
> > > > > >
> > > > > > Yes, but we nee to make sure we clear whatever we programmed in
> > > > > > for DP previously.
> > > > >
> > > > > The silicon seems to be able to handle the situation of DP -> HDMI > > > > > and HDMI -> DP.
> > > >
> > > > Did you make sure eg. that the power well didn't get toggled in between?
> > > > That would reset the register anyway.
> > >
> > > We have done the test for this case. So far it is OK.
> > 
> > Test what? Checking that the register gets reset w/o any power well toggling
> > and the like? So what event does reset that register?
> 
> The register is used to set m/cts, which will impacts the audio clock sync. We 
> didn't check the register reset or not. But the HDMI audio and DP will both
> works smoothly. And for your consideration, it is for HDMI. Let's make
> another patch for this issue if we really met the issue that hdmi can't
> sync the clock. What do you think?

How about we just always write the register to make sure we won't
get any stupid bugs because of this.

> 
> > 
> > >
> > > Regards,
> > > Libin
> > >
> > > >
> > > > > What's more it can handle sample rate/tmds changes.
> > > > >
> > > > > Regards,
> > > > > Libin
> > > > >
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > > +		}
> > > > > > > > > +	}
> > > > > > > > > +
> > > > > > > > >  	mutex_unlock(&dev_priv->av_mutex);
> > > > > > > > >  }
> > > > > > > > >
> > > > > > > > > @@ -637,7 +728,7 @@ static int
> > > > > > > > i915_audio_component_sync_audio_rate(struct device *dev,
> > > > > > > > >  	struct drm_display_mode *mode;
> > > > > > > > >  	struct i915_audio_component *acomp = dev_priv-
> > > > >audio_component;
> > > > > > > > >  	enum pipe pipe = INVALID_PIPE;
> > > > > > > > > -	u32 tmp;
> > > > > > > > > +	u32 tmp, m;
> > > > > > > > >  	int n;
> > > > > > > > >  	int err = 0;
> > > > > > > > >
> > > > > > > > > @@ -653,7 +744,8 @@ static int
> > > > > > > > i915_audio_component_sync_audio_rate(struct device *dev,
> > > > > > > > >  	intel_encoder = dev_priv->dig_port_map[port];
> > > > > > > > >  	/* intel_encoder might be NULL for DP MST */
> > > > > > > > >  	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))) {
> > > > > > > >
> > > > > > > > Hmm. Should probablt move this stuff later so that we can
> > > > > > > > use check
> > > > > > > > crtc->config->output_types instead. But I guess that's more
> > > > > > > > appropriately left to any MST patch.
> > > > > > >
> > > > > > > The same reason as intel_has_dp_encoder(). And if it is wrong
> > > > > > > for DP MST, let's have a separate patch when we support DP MST.
> > > > > > > At that time, we can do a full test on DP MST.
> > > > > >
> > > > > > Yeah this part we can leave for the MST patch as it'll involve a
> > > > > > bit more than just adding another check to the if statement.
> > > > > >
> > > > > > >
> > > > > > > Regards,
> > > > > > > Libin
> > > > > > >
> > > > > > > >
> > > > > > > > >  		DRM_DEBUG_KMS("no valid port %c\n",
> > > > port_name(port));
> > > > > > > > >  		err = -ENODEV;
> > > > > > > > >  		goto unlock;
> > > > > > > > > @@ -681,7 +773,7 @@ static int
> > > > > > > > i915_audio_component_sync_audio_rate(struct device *dev,
> > > > > > > > >  		goto unlock;
> > > > > > > > >  	}
> > > > > > > > >
> > > > > > > > > -	n = audio_config_get_n(mode, rate);
> > > > > > > > > +	n = audio_config_get_n(crtc, mode, rate);
> > > > > > > > >  	if (n == 0) {
> > > > > > > > >  		DRM_DEBUG_KMS("Using automatic mode for N
> > > > value on
> > > > > > > > port %c\n",
> > > > > > > > >  					  port_name(port));
> > > > > > > > > @@ -693,8 +785,17 @@ static int
> > > > > > > > > i915_audio_component_sync_audio_rate(struct device *dev,
> > > > > > > > >
> > > > > > > > >  	/* 3. set the N/CTS/M */
> > > > > > > > >  	tmp = I915_READ(HSW_AUD_CFG(pipe));
> > > > > > > > > -	tmp = audio_config_setup_n_reg(n, tmp);
> > > > > > > > > +	tmp = audio_config_setup_n_reg(crtc, n, tmp);
> > > > > > > > >  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > > > > > > > > +	/* setup m value for DP */
> > > > > > > > > +	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP))
> > > > > > > > > +{
> > > > > > > >
> > > > > > > > intel_has_dp_encoder()
> > > > > > > >
> > > > > > > > > +		m = audio_config_get_m(crtc, rate);
> > > > > > > > > +		if (m == 0)
> > > > > > > > > +			goto unlock;
> > > > > > > > > +		tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
> > > > > > > > > +		tmp = audio_config_setup_m_reg(crtc, m, tmp);
> > > > > > > > > +		I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
> > > > > > > > > +	}
> > > > > > > > >
> > > > > > > > >   unlock:
> > > > > > > > >  	mutex_unlock(&dev_priv->av_mutex);
> > > > > > > > > --
> > > > > > > > > 1.9.1
> > > > > > > >
> > > > > > > > --
> > > > > > > > Ville Syrjälä
> > > > > > > > Intel OTC
> > > > > >
> > > > > > --
> > > > > > Ville Syrjälä
> > > > > > Intel OTC
> > > >
> > > > --
> > > > Ville Syrjälä
> > > > Intel OTC
> > 
> > --
> > Ville Syrjälä
> > Intel OTC

-- 
Ville Syrjälä
Intel OTC


More information about the Intel-gfx mailing list