Re: [PATCH] drm/i915/audio: not to set N/M value manually for KBL.

From: Jani Nikula <jani.nikula@linux.intel.com>
To: Quanxian Wang <quanxian.wang@intel.com>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/audio: not to set N/M value manually	for KBL.
Date: Tue, 04 Apr 2017 13:54:52 +0300	[thread overview]
Message-ID: <87pogs1lz7.fsf@intel.com> (raw)
In-Reply-To: <1491014281-18853-1-git-send-email-quanxian.wang@intel.com>

On Sat, 01 Apr 2017, Quanxian Wang <quanxian.wang@intel.com> wrote:
> It doesn't work on KBL. Just using automatic N/M.

Please provide a more elaborate commit message. In particular, have a
look at

commit 6014ac122ed081feca99217bc57b2e15c7fc1a51
Author: Libin Yang <libin.yang@linux.intel.com>
Date:   Tue Oct 25 17:54:18 2016 +0300

    drm/i915/audio: set proper N/M in modeset

and explain why that doesn't apply to KBL. And why just KBL, why not
others?

> According to the bspec, when set N/M, should disable
> and enable transcoder which attaching DP audio. but there is no
> such code to do that.
> without this implementation except KBL platforms, seems work well.
>
> Signed-off-by: Quanxian Wang <quanxian.wang@intel.com>
> Tested-By: Wang Zhijun<zhijunx.wang@intel.com>
> Tested-By: Cui Yueping<yuepingx.cui@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 52c207e..0542031 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -300,7 +300,7 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
>  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>  	tmp |= AUD_CONFIG_N_VALUE_INDEX;
>  
> -	if (nm) {
> +	if (!IS_KABYLAKE(dev_priv) && nm) {
>  		tmp &= ~AUD_CONFIG_N_MASK;
>  		tmp |= AUD_CONFIG_N(nm->n);
>  		tmp |= AUD_CONFIG_N_PROG_ENABLE;
> @@ -308,6 +308,9 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
>  
>  	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>  
> +	if (IS_KABYLAKE(dev_priv))
> +		return;
> +

The debug logging near the top of the function will be wrong for KBL,
and you now bypass HSW_AUD_M_CTS_ENABLE() setting for the automatic
values.

Seems to me the simplest would be to do

	if (IS_KABYLAKE(dev_priv))
        	nm = NULL;

*but* this still needs better rationale, especially why this is specific
to KBL.

BR,
Jani.

>  	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
>  	tmp &= ~AUD_CONFIG_M_MASK;
>  	tmp &= ~AUD_M_CTS_M_VALUE_INDEX;

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