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

[Yang, Libin]

Hi Quanxian,

>-----Original Message-----
>From: Wang, Quanxian
>Sent: Wednesday, April 5, 2017 10:21 AM
>To: Jani Nikula <jani.nikula at linux.intel.com>; intel-gfx at lists.freedesktop.org
>Cc: Yang, Libin <libin.yang at intel.com>
>Subject: RE: [Intel-gfx] [PATCH] drm/i915/audio: not to set N/M value
>manually for KBL.
>
>Sorry, I don't find them in bspec. With our testing, SKL/APL works and KBL
>failed.
>
>The commit 6014ac122ed081feca99217bc57b2e15c7fc1a51 mentioned  should
>not do testing on KBL DP audio.
>
>We found the issue in the testing for Ubuntu latest release 17.04. (Almost
>upstream)
>
>But really I don't know the root cause.
>
>Also I have mentioned in commit, for general process of DP audio in bspec,
>transcoder should be restart to make it.
> But no code to do that. Also I don't know why driver doesn't implement that.

Please see my another email. I explained the reason on it. And I strongly suggest
you doing all the resolution and audio format playback test. We did find some
resolution and audio format will fail on the auto setting of N/M.

Regards,
Libin

>
>-----Original Message-----
>From: Jani Nikula [mailto:jani.nikula at linux.intel.com]
>Sent: Tuesday, April 4, 2017 6:55 PM
>To: Wang, Quanxian <quanxian.wang at intel.com>; intel-
>gfx at lists.freedesktop.org
>Cc: Yang, Libin <libin.yang at intel.com>
>Subject: Re: [Intel-gfx] [PATCH] drm/i915/audio: not to set N/M value
>manually for KBL.
>
>On Sat, 01 Apr 2017, Quanxian Wang <quanxian.wang at 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 at 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 at intel.com>
>> Tested-By: Wang Zhijun<zhijunx.wang at intel.com>
>> Tested-By: Cui Yueping<yuepingx.cui at 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