diff mbox

drm/i915: set proper N/M in modeset

Message ID 1468479981-738-1-git-send-email-libin.yang@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

libin.yang@linux.intel.com July 14, 2016, 7:06 a.m. UTC 
From: Libin Yang <libin.yang@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.

Also, the patch applies
commit 7e8275c2f2bb ("drm/i915: set proper N/CTS in modeset")
to APL platform.

Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |   6 ++
 drivers/gpu/drm/i915/intel_audio.c | 116 ++++++++++++++++++++++++++++++++-----
 2 files changed, 108 insertions(+), 14 deletions(-)

Comments

Yang, Libin July 19, 2016, 2:24 a.m. UTC | #1 
Hi All,

I submitted a patch and it show to fail in CI.

Does the message means it is failed because of "ro-bdw-i7-5557U failed to connect after reboot". And how can I get the dmesg?

Regards,
Libin


> -----Original Message-----

> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of

> Patchwork

> Sent: Thursday, July 14, 2016 3:39 PM

> To: libin.yang@linux.intel.com

> Cc: intel-gfx@lists.freedesktop.org

> Subject: [Intel-gfx] ✗ Ro.CI.BAT: failure for drm/i915: set proper N/M in

> modeset

> 

> == Series Details ==

> 

> Series: drm/i915: set proper N/M in modeset

> URL   : https://patchwork.freedesktop.org/series/9857/

> State : failure

> 

> == Summary ==

> 

> Series 9857v1 drm/i915: set proper N/M in modeset

> http://patchwork.freedesktop.org/api/1.0/series/9857/revisions/1/mbox

> 

> Test drv_module_reload_basic:

>                 dmesg-warn -> SKIP       (fi-skl-i5-6260u)

> Test kms_pipe_crc_basic:

>         Subgroup suspend-read-crc-pipe-b:

>                 pass       -> DMESG-WARN (ro-skl3-i5-6260u)

> Test prime_self_import:

>         Subgroup basic-with_fd_dup:

>                 pass       -> INCOMPLETE (ro-byt-n2820)

>         Subgroup basic-with_one_bo_two_files:

>                 pass       -> INCOMPLETE (ro-byt-n2820)

>         Subgroup basic-with_two_bos:

>                 pass       -> INCOMPLETE (ro-byt-n2820)

> 

> fi-kbl-qkkr      total:237  pass:174  dwarn:27  dfail:2   fail:7   skip:27

> fi-skl-i5-6260u  total:237  pass:189  dwarn:26  dfail:2   fail:7   skip:13

> fi-skl-i7-6700k  total:237  pass:200  dwarn:2   dfail:0   fail:9   skip:26

> fi-snb-i7-2600   total:237  pass:188  dwarn:0   dfail:0   fail:9   skip:40

> ro-bdw-i5-5250u  total:237  pass:210  dwarn:2   dfail:0   fail:9   skip:16

> ro-bdw-i7-5600u  total:237  pass:195  dwarn:2   dfail:0   fail:9   skip:31

> ro-bsw-n3050     total:217  pass:170  dwarn:0   dfail:0   fail:4   skip:42

> ro-byt-n2820     total:237  pass:174  dwarn:0   dfail:0   fail:7   skip:38

> ro-hsw-i3-4010u  total:237  pass:199  dwarn:5   dfail:2   fail:7   skip:24

> ro-hsw-i7-4770r  total:237  pass:204  dwarn:0   dfail:0   fail:9   skip:24

> ro-ilk-i7-620lm  total:237  pass:166  dwarn:0   dfail:0   fail:8   skip:63

> ro-ilk1-i5-650   total:232  pass:166  dwarn:0   dfail:0   fail:8   skip:58

> ro-ivb-i7-3770   total:237  pass:197  dwarn:0   dfail:0   fail:7   skip:33

> ro-skl3-i5-6260u total:237  pass:212  dwarn:4   dfail:0   fail:9   skip:12

> ro-snb-i7-2620M  total:237  pass:187  dwarn:0   dfail:0   fail:9   skip:41

> ro-bdw-i7-5557U failed to connect after reboot

> 

> Results at /archive/results/CI_IGT_test/RO_Patchwork_1490/

> 

> 5cd2699 drm-intel-nightly: 2016y-07m-13d-14h-43m-27s UTC integration

> manifest 9ea6eab drm/i915: set proper N/M in modeset

> 

> _______________________________________________

> Intel-gfx mailing list

> Intel-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Yang, Libin July 28, 2016, 6:44 a.m. UTC | #2 
Hi all,

Is there anyone can help review this patch? This patch is set N/M for audio in DP 4k resolution. Thanks.

I calculate these values based on the DP1.2 spec chapter 6.6 on page 469:
Maud / Naud = 512 * fs / f_LS_Clk

Regards,
Libin


> -----Original Message-----
> From: libin.yang@linux.intel.com [mailto:libin.yang@linux.intel.com]
> Sent: Thursday, July 14, 2016 3:06 PM
> To: intel-gfx@lists.freedesktop.org; jani.nikula@linux.intel.com;
> ville.syrjala@linux.intel.com; Vetter, Daniel <daniel.vetter@intel.com>;
> tiwai@suse.de
> Cc: Yang, Libin <libin.yang@intel.com>; Libin Yang
> <libin.yang@linux.intel.com>
> Subject: [PATCH] drm/i915: set proper N/M in modeset
> 
> From: Libin Yang <libin.yang@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.
> 
> Also, the patch applies
> commit 7e8275c2f2bb ("drm/i915: set proper N/CTS in modeset") to APL
> platform.
> 
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |   6 ++
>  drivers/gpu/drm/i915/intel_audio.c | 116
> ++++++++++++++++++++++++++++++++-----
>  2 files changed, 108 insertions(+), 14 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)
> +
>  #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..e2e4d4b 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -98,6 +98,22 @@ static const struct {
>  	{ 192000, TMDS_297M, 20480, 247500 },
>  };
> 
> +#define LC_533M 533250
> +#define LC_148M 148500
> +static const struct {
> +	int sample_rate;
> +	int clock;
> +	int n;
> +	int m;
> +} aud_nm[] = {
> +	{48000, LC_533M, 88875, 4096},
> +	{44100, LC_533M, 148125, 6272},
> +	{32000, LC_533M, 266625, 8192},
> +	{48000, LC_148M, 12375, 2048},
> +	{44100, LC_148M, 20625, 3136},
> +	{32000, LC_148M, 37125, 4096},
> +};
> +
>  /* 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 +137,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 *mode, int rate) {
> +	int i;
> +
> +	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) &&
> +				(mode->clock == aud_ncts[i].clock)) {
> +				return aud_ncts[i].n;
> +			}
> +		}
> +	}
> +
> +	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP)) {
> +		for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
> +			if ((rate == aud_nm[i].sample_rate) &&
> +				(mode->clock == aud_nm[i].clock)) {
> +				return aud_nm[i].n;
> +			}
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int audio_config_get_m(struct intel_crtc *crtc,
> +			      const struct drm_display_mode *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_DP)) {
> +		for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
> +			if ((rate == aud_nm[i].sample_rate) &&
> +				(mode->clock == aud_nm[i].clock)) {
> +				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,6 +191,23 @@ 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))
> +		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;
> +
> +	tmp |= m;
> +	tmp |= AUD_M_CTS_M_VALUE_INDEX;
> +	tmp |= AUD_M_CTS_M_PROG_ENABLE;
> +
>  	return tmp;
>  }
> 
> @@ -156,6 +219,10 @@ static bool audio_rate_need_prog(struct intel_crtc
> *crtc,
>  		 (mode->clock == TMDS_296M)) &&
>  		intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
>  		return true;
> +	else if (((mode->clock == LC_533M) ||
> +		   (mode->clock == LC_148M)) &&
> +		  intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP))
> +		return true;
>  	else
>  		return false;
>  }
> @@ -287,7 +354,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 +410,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)) {
> +		m = audio_config_get_m(intel_crtc, adjusted_mode, 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);
> +		}
> +	}
> +
>  	mutex_unlock(&dev_priv->av_mutex);
>  }
> 
> @@ -637,7 +714,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;
> 
> @@ -645,7 +722,8 @@ static int
> i915_audio_component_sync_audio_rate(struct device *dev,
>  	if (!IS_SKYLAKE(dev_priv) &&
>  	    !IS_KABYLAKE(dev_priv) &&
>  	    !IS_BROADWELL(dev_priv) &&
> -	    !IS_HASWELL(dev_priv))
> +	    !IS_HASWELL(dev_priv) &&
> +	    !IS_BROXTON(dev_priv))
>  		return 0;
> 
>  	mutex_lock(&dev_priv->av_mutex);
> @@ -653,7 +731,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))) {
>  		DRM_DEBUG_KMS("no valid port %c\n", port_name(port));
>  		err = -ENODEV;
>  		goto unlock;
> @@ -681,7 +760,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 +772,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)) {
> +		m = audio_config_get_m(crtc, mode, 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ä July 28, 2016, 7:41 a.m. UTC | #3 
On Thu, Jul 14, 2016 at 03:06:21PM +0800, libin.yang@linux.intel.com wrote:
> From: Libin Yang <libin.yang@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.
> 
> Also, the patch applies
> commit 7e8275c2f2bb ("drm/i915: set proper N/CTS in modeset")
> to APL platform.
> 
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |   6 ++
>  drivers/gpu/drm/i915/intel_audio.c | 116 ++++++++++++++++++++++++++++++++-----
>  2 files changed, 108 insertions(+), 14 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)
> +
>  #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..e2e4d4b 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -98,6 +98,22 @@ static const struct {
>  	{ 192000, TMDS_297M, 20480, 247500 },
>  };
>  
> +#define LC_533M 533250
> +#define LC_148M 148500
> +static const struct {
> +	int sample_rate;
> +	int clock;
> +	int n;
> +	int m;
> +} aud_nm[] = {
> +	{48000, LC_533M, 88875, 4096},
> +	{44100, LC_533M, 148125, 6272},
> +	{32000, LC_533M, 266625, 8192},
> +	{48000, LC_148M, 12375, 2048},
> +	{44100, LC_148M, 20625, 3136},
> +	{32000, LC_148M, 37125, 4096},
> +};
> +
>  /* 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 +137,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 *mode, int rate)
> +{
> +	int i;
> +
> +	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) &&
> +				(mode->clock == aud_ncts[i].clock)) {
> +				return aud_ncts[i].n;
> +			}
> +		}
> +	}
> +
> +	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP)) {
> +		for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
> +			if ((rate == aud_nm[i].sample_rate) &&
> +				(mode->clock == aud_nm[i].clock)) {

So the spec says we should have "Maud / Naud = 512 * fs / f_LS_Clk",
where fs is the audio sample rate, and f_LS_CLK is the link symbol clock.
With that in mind this should not be looking at mode->clock but
crtc->config->port_clock.

So I don't actually understand why you say LS_CLK has "special" values.
It shouldn't. It's always either 162, 270, or 540 MHz.

Actually even the HDMI case is wrong in the code, it should be
looking at mode->crtc_clock instead of mode->clock. Or perhaps even
port_clock, if my reading of the HDMI spec is correct, but I never got
any sane answer from any hw folks to my questions about this :(

> +				return aud_nm[i].n;
> +			}
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int audio_config_get_m(struct intel_crtc *crtc,
> +			      const struct drm_display_mode *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_DP)) {
> +		for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
> +			if ((rate == aud_nm[i].sample_rate) &&
> +				(mode->clock == aud_nm[i].clock)) {
> +				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,6 +191,23 @@ 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))
> +		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;
> +
> +	tmp |= m;
> +	tmp |= AUD_M_CTS_M_VALUE_INDEX;
> +	tmp |= AUD_M_CTS_M_PROG_ENABLE;
> +
>  	return tmp;
>  }
>  
> @@ -156,6 +219,10 @@ static bool audio_rate_need_prog(struct intel_crtc *crtc,
>  		 (mode->clock == TMDS_296M)) &&
>  		intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
>  		return true;
> +	else if (((mode->clock == LC_533M) ||
> +		   (mode->clock == LC_148M)) &&
> +		  intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP))
> +		return true;
>  	else
>  		return false;
>  }
> @@ -287,7 +354,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 +410,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)) {
> +		m = audio_config_get_m(intel_crtc, adjusted_mode, 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);
> +		}
> +	}
> +
>  	mutex_unlock(&dev_priv->av_mutex);
>  }
>  
> @@ -637,7 +714,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;
>  
> @@ -645,7 +722,8 @@ static int i915_audio_component_sync_audio_rate(struct device *dev,
>  	if (!IS_SKYLAKE(dev_priv) &&
>  	    !IS_KABYLAKE(dev_priv) &&
>  	    !IS_BROADWELL(dev_priv) &&
> -	    !IS_HASWELL(dev_priv))
> +	    !IS_HASWELL(dev_priv) &&
> +	    !IS_BROXTON(dev_priv))
>  		return 0;
>  
>  	mutex_lock(&dev_priv->av_mutex);
> @@ -653,7 +731,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))) {
>  		DRM_DEBUG_KMS("no valid port %c\n", port_name(port));
>  		err = -ENODEV;
>  		goto unlock;
> @@ -681,7 +760,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 +772,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)) {
> +		m = audio_config_get_m(crtc, mode, 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
Yang, Libin July 29, 2016, 5:54 a.m. UTC | #4 
Hi Ville,

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Thursday, July 28, 2016 3:42 PM
> To: libin.yang@linux.intel.com
> Cc: intel-gfx@lists.freedesktop.org; jani.nikula@linux.intel.com; Vetter,
> Daniel <daniel.vetter@intel.com>; tiwai@suse.de; Yang, Libin
> <libin.yang@intel.com>
> Subject: Re: [PATCH] drm/i915: set proper N/M in modeset
> 
> On Thu, Jul 14, 2016 at 03:06:21PM +0800, libin.yang@linux.intel.com wrote:
> > From: Libin Yang <libin.yang@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.
> >
> > Also, the patch applies
> > commit 7e8275c2f2bb ("drm/i915: set proper N/CTS in modeset") to APL
> > platform.
> >
> > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h    |   6 ++
> >  drivers/gpu/drm/i915/intel_audio.c | 116
> > ++++++++++++++++++++++++++++++++-----
> >  2 files changed, 108 insertions(+), 14 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)
> > +
> >  #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..e2e4d4b 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -98,6 +98,22 @@ static const struct {
> >  	{ 192000, TMDS_297M, 20480, 247500 },  };
> >
> > +#define LC_533M 533250
> > +#define LC_148M 148500
> > +static const struct {
> > +	int sample_rate;
> > +	int clock;
> > +	int n;
> > +	int m;
> > +} aud_nm[] = {
> > +	{48000, LC_533M, 88875, 4096},
> > +	{44100, LC_533M, 148125, 6272},
> > +	{32000, LC_533M, 266625, 8192},
> > +	{48000, LC_148M, 12375, 2048},
> > +	{44100, LC_148M, 20625, 3136},
> > +	{32000, LC_148M, 37125, 4096},
> > +};
> > +
> >  /* 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 +137,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 *mode, int rate) {
> > +	int i;
> > +
> > +	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) &&
> > +				(mode->clock == aud_ncts[i].clock)) {
> > +				return aud_ncts[i].n;
> > +			}
> > +		}
> > +	}
> > +
> > +	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP)) {
> > +		for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
> > +			if ((rate == aud_nm[i].sample_rate) &&
> > +				(mode->clock == aud_nm[i].clock)) {
> 
> So the spec says we should have "Maud / Naud = 512 * fs / f_LS_Clk", where
> fs is the audio sample rate, and f_LS_CLK is the link symbol clock.
> With that in mind this should not be looking at mode->clock but
> crtc->config->port_clock.
> 
> So I don't actually understand why you say LS_CLK has "special" values.
> It shouldn't. It's always either 162, 270, or 540 MHz.

Thanks for the correction. I will use crtc->config->port_clock.

> 
> Actually even the HDMI case is wrong in the code, it should be looking at
> mode->crtc_clock instead of mode->clock. Or perhaps even port_clock, if my
> reading of the HDMI spec is correct, but I never got any sane answer from
> any hw folks to my questions about this :(

I will try mode->crtc_clock later for HDMI. Thanks.

Regards,
Libin

> 
> > +				return aud_nm[i].n;
> > +			}
> > +		}
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int audio_config_get_m(struct intel_crtc *crtc,
> > +			      const struct drm_display_mode *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_DP)) {
> > +		for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
> > +			if ((rate == aud_nm[i].sample_rate) &&
> > +				(mode->clock == aud_nm[i].clock)) {
> > +				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,6 +191,23 @@ 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))
> > +		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;
> > +
> > +	tmp |= m;
> > +	tmp |= AUD_M_CTS_M_VALUE_INDEX;
> > +	tmp |= AUD_M_CTS_M_PROG_ENABLE;
> > +
> >  	return tmp;
> >  }
> >
> > @@ -156,6 +219,10 @@ static bool audio_rate_need_prog(struct intel_crtc
> *crtc,
> >  		 (mode->clock == TMDS_296M)) &&
> >  		intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
> >  		return true;
> > +	else if (((mode->clock == LC_533M) ||
> > +		   (mode->clock == LC_148M)) &&
> > +		  intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP))
> > +		return true;
> >  	else
> >  		return false;
> >  }
> > @@ -287,7 +354,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 +410,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)) {
> > +		m = audio_config_get_m(intel_crtc, adjusted_mode, 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);
> > +		}
> > +	}
> > +
> >  	mutex_unlock(&dev_priv->av_mutex);
> >  }
> >
> > @@ -637,7 +714,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;
> >
> > @@ -645,7 +722,8 @@ static int
> i915_audio_component_sync_audio_rate(struct device *dev,
> >  	if (!IS_SKYLAKE(dev_priv) &&
> >  	    !IS_KABYLAKE(dev_priv) &&
> >  	    !IS_BROADWELL(dev_priv) &&
> > -	    !IS_HASWELL(dev_priv))
> > +	    !IS_HASWELL(dev_priv) &&
> > +	    !IS_BROXTON(dev_priv))
> >  		return 0;
> >
> >  	mutex_lock(&dev_priv->av_mutex);
> > @@ -653,7 +731,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))) {
> >  		DRM_DEBUG_KMS("no valid port %c\n", port_name(port));
> >  		err = -ENODEV;
> >  		goto unlock;
> > @@ -681,7 +760,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 +772,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)) {
> > +		m = audio_config_get_m(crtc, mode, 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ä July 29, 2016, 9:47 a.m. UTC | #5 
On Fri, Jul 29, 2016 at 05:54:23AM +0000, Yang, Libin wrote:
> Hi Ville,
> 
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> > Sent: Thursday, July 28, 2016 3:42 PM
> > To: libin.yang@linux.intel.com
> > Cc: intel-gfx@lists.freedesktop.org; jani.nikula@linux.intel.com; Vetter,
> > Daniel <daniel.vetter@intel.com>; tiwai@suse.de; Yang, Libin
> > <libin.yang@intel.com>
> > Subject: Re: [PATCH] drm/i915: set proper N/M in modeset
> > 
> > On Thu, Jul 14, 2016 at 03:06:21PM +0800, libin.yang@linux.intel.com wrote:
> > > From: Libin Yang <libin.yang@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.
> > >
> > > Also, the patch applies
> > > commit 7e8275c2f2bb ("drm/i915: set proper N/CTS in modeset") to APL
> > > platform.
> > >
> > > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h    |   6 ++
> > >  drivers/gpu/drm/i915/intel_audio.c | 116
> > > ++++++++++++++++++++++++++++++++-----
> > >  2 files changed, 108 insertions(+), 14 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)
> > > +
> > >  #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..e2e4d4b 100644
> > > --- a/drivers/gpu/drm/i915/intel_audio.c
> > > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > > @@ -98,6 +98,22 @@ static const struct {
> > >  	{ 192000, TMDS_297M, 20480, 247500 },  };
> > >
> > > +#define LC_533M 533250
> > > +#define LC_148M 148500
> > > +static const struct {
> > > +	int sample_rate;
> > > +	int clock;
> > > +	int n;
> > > +	int m;
> > > +} aud_nm[] = {
> > > +	{48000, LC_533M, 88875, 4096},
> > > +	{44100, LC_533M, 148125, 6272},
> > > +	{32000, LC_533M, 266625, 8192},
> > > +	{48000, LC_148M, 12375, 2048},
> > > +	{44100, LC_148M, 20625, 3136},
> > > +	{32000, LC_148M, 37125, 4096},
> > > +};
> > > +
> > >  /* 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 +137,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 *mode, int rate) {
> > > +	int i;
> > > +
> > > +	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) &&
> > > +				(mode->clock == aud_ncts[i].clock)) {
> > > +				return aud_ncts[i].n;
> > > +			}
> > > +		}
> > > +	}
> > > +
> > > +	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP)) {
> > > +		for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
> > > +			if ((rate == aud_nm[i].sample_rate) &&
> > > +				(mode->clock == aud_nm[i].clock)) {
> > 
> > So the spec says we should have "Maud / Naud = 512 * fs / f_LS_Clk", where
> > fs is the audio sample rate, and f_LS_CLK is the link symbol clock.
> > With that in mind this should not be looking at mode->clock but
> > crtc->config->port_clock.
> > 
> > So I don't actually understand why you say LS_CLK has "special" values.
> > It shouldn't. It's always either 162, 270, or 540 MHz.
> 
> Thanks for the correction. I will use crtc->config->port_clock.

But why do you need to do it at all? The hardware can't deal with one of
the standard link rates on its own?

> 
> > 
> > Actually even the HDMI case is wrong in the code, it should be looking at
> > mode->crtc_clock instead of mode->clock. Or perhaps even port_clock, if my
> > reading of the HDMI spec is correct, but I never got any sane answer from
> > any hw folks to my questions about this :(
> 
> I will try mode->crtc_clock later for HDMI. Thanks.

While you're at it, can you pls do s/mode/adjusted_mode/ ? A while back
I unified our naming convention for these things, but looks like the
audio code has since gone back to the inconsisten old way.

> 
> Regards,
> Libin
> 
> > 
> > > +				return aud_nm[i].n;
> > > +			}
> > > +		}
> > > +	}
> > > +	return 0;
> > > +}
> > > +
> > > +static int audio_config_get_m(struct intel_crtc *crtc,
> > > +			      const struct drm_display_mode *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_DP)) {
> > > +		for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
> > > +			if ((rate == aud_nm[i].sample_rate) &&
> > > +				(mode->clock == aud_nm[i].clock)) {
> > > +				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,6 +191,23 @@ 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))
> > > +		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;
> > > +
> > > +	tmp |= m;
> > > +	tmp |= AUD_M_CTS_M_VALUE_INDEX;
> > > +	tmp |= AUD_M_CTS_M_PROG_ENABLE;
> > > +
> > >  	return tmp;
> > >  }
> > >
> > > @@ -156,6 +219,10 @@ static bool audio_rate_need_prog(struct intel_crtc
> > *crtc,
> > >  		 (mode->clock == TMDS_296M)) &&
> > >  		intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
> > >  		return true;
> > > +	else if (((mode->clock == LC_533M) ||
> > > +		   (mode->clock == LC_148M)) &&
> > > +		  intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP))
> > > +		return true;
> > >  	else
> > >  		return false;
> > >  }
> > > @@ -287,7 +354,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 +410,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)) {
> > > +		m = audio_config_get_m(intel_crtc, adjusted_mode, 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);
> > > +		}
> > > +	}
> > > +
> > >  	mutex_unlock(&dev_priv->av_mutex);
> > >  }
> > >
> > > @@ -637,7 +714,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;
> > >
> > > @@ -645,7 +722,8 @@ static int
> > i915_audio_component_sync_audio_rate(struct device *dev,
> > >  	if (!IS_SKYLAKE(dev_priv) &&
> > >  	    !IS_KABYLAKE(dev_priv) &&
> > >  	    !IS_BROADWELL(dev_priv) &&
> > > -	    !IS_HASWELL(dev_priv))
> > > +	    !IS_HASWELL(dev_priv) &&
> > > +	    !IS_BROXTON(dev_priv))
> > >  		return 0;
> > >
> > >  	mutex_lock(&dev_priv->av_mutex);
> > > @@ -653,7 +731,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))) {
> > >  		DRM_DEBUG_KMS("no valid port %c\n", port_name(port));
> > >  		err = -ENODEV;
> > >  		goto unlock;
> > > @@ -681,7 +760,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 +772,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)) {
> > > +		m = audio_config_get_m(crtc, mode, 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
Yang, Libin Aug. 1, 2016, 1:31 a.m. UTC | #6 
Hi Ville,

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Friday, July 29, 2016 5:47 PM
> To: Yang, Libin <libin.yang@intel.com>
> Cc: libin.yang@linux.intel.com; intel-gfx@lists.freedesktop.org;
> jani.nikula@linux.intel.com; Vetter, Daniel <daniel.vetter@intel.com>;
> tiwai@suse.de
> Subject: Re: [PATCH] drm/i915: set proper N/M in modeset
> 
> On Fri, Jul 29, 2016 at 05:54:23AM +0000, Yang, Libin wrote:
> > Hi Ville,
> >
> > > -----Original Message-----
> > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> > > Sent: Thursday, July 28, 2016 3:42 PM
> > > To: libin.yang@linux.intel.com
> > > Cc: intel-gfx@lists.freedesktop.org; jani.nikula@linux.intel.com;
> > > Vetter, Daniel <daniel.vetter@intel.com>; tiwai@suse.de; Yang, Libin
> > > <libin.yang@intel.com>
> > > Subject: Re: [PATCH] drm/i915: set proper N/M in modeset
> > >
> > > On Thu, Jul 14, 2016 at 03:06:21PM +0800, libin.yang@linux.intel.com
> wrote:
> > > > From: Libin Yang <libin.yang@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.
> > > >
> > > > Also, the patch applies
> > > > commit 7e8275c2f2bb ("drm/i915: set proper N/CTS in modeset") to
> > > > APL platform.
> > > >
> > > > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_reg.h    |   6 ++
> > > >  drivers/gpu/drm/i915/intel_audio.c | 116
> > > > ++++++++++++++++++++++++++++++++-----
> > > >  2 files changed, 108 insertions(+), 14 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)
> > > > +
> > > >  #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..e2e4d4b 100644
> > > > --- a/drivers/gpu/drm/i915/intel_audio.c
> > > > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > > > @@ -98,6 +98,22 @@ static const struct {
> > > >  	{ 192000, TMDS_297M, 20480, 247500 },  };
> > > >
> > > > +#define LC_533M 533250
> > > > +#define LC_148M 148500
> > > > +static const struct {
> > > > +	int sample_rate;
> > > > +	int clock;
> > > > +	int n;
> > > > +	int m;
> > > > +} aud_nm[] = {
> > > > +	{48000, LC_533M, 88875, 4096},
> > > > +	{44100, LC_533M, 148125, 6272},
> > > > +	{32000, LC_533M, 266625, 8192},
> > > > +	{48000, LC_148M, 12375, 2048},
> > > > +	{44100, LC_148M, 20625, 3136},
> > > > +	{32000, LC_148M, 37125, 4096},
> > > > +};
> > > > +
> > > >  /* 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 +137,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 *mode, int rate) {
> > > > +	int i;
> > > > +
> > > > +	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) &&
> > > > +				(mode->clock == aud_ncts[i].clock)) {
> > > > +				return aud_ncts[i].n;
> > > > +			}
> > > > +		}
> > > > +	}
> > > > +
> > > > +	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP)) {
> > > > +		for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
> > > > +			if ((rate == aud_nm[i].sample_rate) &&
> > > > +				(mode->clock == aud_nm[i].clock)) {
> > >
> > > So the spec says we should have "Maud / Naud = 512 * fs / f_LS_Clk",
> > > where fs is the audio sample rate, and f_LS_CLK is the link symbol clock.
> > > With that in mind this should not be looking at mode->clock but
> > > crtc->config->port_clock.
> > >
> > > So I don't actually understand why you say LS_CLK has "special" values.
> > > It shouldn't. It's always either 162, 270, or 540 MHz.
> >
> > Thanks for the correction. I will use crtc->config->port_clock.
> 
> But why do you need to do it at all? The hardware can't deal with one of the
> standard link rates on its own?

Yes, we found that the HW can't set the N/M automatically. This will cause
there is several seconds of silence at the beginning of audio playback.

> 
> >
> > >
> > > Actually even the HDMI case is wrong in the code, it should be
> > > looking at
> > > mode->crtc_clock instead of mode->clock. Or perhaps even port_clock,
> > > mode->if my
> > > reading of the HDMI spec is correct, but I never got any sane answer
> > > from any hw folks to my questions about this :(
> >
> > I will try mode->crtc_clock later for HDMI. Thanks.
> 
> While you're at it, can you pls do s/mode/adjusted_mode/ ? A while back I
> unified our naming convention for these things, but looks like the audio code
> has since gone back to the inconsisten old way.

Yes, I will do it, replace mode with adjusted_mode.

Regards,
Libin

> 
> >
> > Regards,
> > Libin
> >
> > >
diff mbox

Patch

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)
+
 #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..e2e4d4b 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -98,6 +98,22 @@  static const struct {
 	{ 192000, TMDS_297M, 20480, 247500 },
 };
 
+#define LC_533M 533250
+#define LC_148M 148500
+static const struct {
+	int sample_rate;
+	int clock;
+	int n;
+	int m;
+} aud_nm[] = {
+	{48000, LC_533M, 88875, 4096},
+	{44100, LC_533M, 148125, 6272},
+	{32000, LC_533M, 266625, 8192},
+	{48000, LC_148M, 12375, 2048},
+	{44100, LC_148M, 20625, 3136},
+	{32000, LC_148M, 37125, 4096},
+};
+
 /* 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 +137,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 *mode, int rate)
+{
+	int i;
+
+	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) &&
+				(mode->clock == aud_ncts[i].clock)) {
+				return aud_ncts[i].n;
+			}
+		}
+	}
+
+	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP)) {
+		for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
+			if ((rate == aud_nm[i].sample_rate) &&
+				(mode->clock == aud_nm[i].clock)) {
+				return aud_nm[i].n;
+			}
+		}
+	}
+	return 0;
+}
+
+static int audio_config_get_m(struct intel_crtc *crtc,
+			      const struct drm_display_mode *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_DP)) {
+		for (i = 0; i < ARRAY_SIZE(aud_nm); i++) {
+			if ((rate == aud_nm[i].sample_rate) &&
+				(mode->clock == aud_nm[i].clock)) {
+				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,6 +191,23 @@  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))
+		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;
+
+	tmp |= m;
+	tmp |= AUD_M_CTS_M_VALUE_INDEX;
+	tmp |= AUD_M_CTS_M_PROG_ENABLE;
+
 	return tmp;
 }
 
@@ -156,6 +219,10 @@  static bool audio_rate_need_prog(struct intel_crtc *crtc,
 		 (mode->clock == TMDS_296M)) &&
 		intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
 		return true;
+	else if (((mode->clock == LC_533M) ||
+		   (mode->clock == LC_148M)) &&
+		  intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP))
+		return true;
 	else
 		return false;
 }
@@ -287,7 +354,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 +410,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)) {
+		m = audio_config_get_m(intel_crtc, adjusted_mode, 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);
+		}
+	}
+
 	mutex_unlock(&dev_priv->av_mutex);
 }
 
@@ -637,7 +714,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;
 
@@ -645,7 +722,8 @@  static int i915_audio_component_sync_audio_rate(struct device *dev,
 	if (!IS_SKYLAKE(dev_priv) &&
 	    !IS_KABYLAKE(dev_priv) &&
 	    !IS_BROADWELL(dev_priv) &&
-	    !IS_HASWELL(dev_priv))
+	    !IS_HASWELL(dev_priv) &&
+	    !IS_BROXTON(dev_priv))
 		return 0;
 
 	mutex_lock(&dev_priv->av_mutex);
@@ -653,7 +731,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))) {
 		DRM_DEBUG_KMS("no valid port %c\n", port_name(port));
 		err = -ENODEV;
 		goto unlock;
@@ -681,7 +760,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 +772,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)) {
+		m = audio_config_get_m(crtc, mode, 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);