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

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

[Quanxian Wang]

It doesn't work on KBL. Just using automatic N/M.

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;
+
 	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
 	tmp &= ~AUD_CONFIG_M_MASK;
 	tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/audio: not to set N/M value manually for KBL.

[Patchwork]

== Series Details ==

Series: drm/i915/audio: not to set N/M value manually for KBL.
URL   : https://patchwork.freedesktop.org/series/22303/
State : success

== Summary ==

Series 22303v1 drm/i915/audio: not to set N/M value manually for KBL.
https://patchwork.freedesktop.org/api/1.0/series/22303/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 433s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 424s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 580s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 517s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 563s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 486s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 495s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 412s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 411s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 430s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 480s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 486s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 466s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 570s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 451s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 568s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 458s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 486s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 431s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 535s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time: 402s

19ceec7d516a7c4614833ba7a3724a4bea0c59d3 drm-tip: 2017y-03m-31d-20h-10m-03s UTC integration manifest
66240f8 drm/i915/audio: not to set N/M value manually for KBL.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4378/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

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

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

[Wang, Quanxian]

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.

-----Original Message-----
From: Jani Nikula [mailto:jani.nikula@linux.intel.com] 
Sent: Tuesday, April 4, 2017 6:55 PM
To: Wang, Quanxian <quanxian.wang@intel.com>; intel-gfx@lists.freedesktop.org
Cc: Yang, Libin <libin.yang@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@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

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

[Yang, Libin]

>-----Original Message-----
>From: Jani Nikula [mailto:jani.nikula@linux.intel.com]
>Sent: Tuesday, April 4, 2017 6:55 PM
>To: Wang, Quanxian <quanxian.wang@intel.com>; intel-
>gfx@lists.freedesktop.org
>Cc: Yang, Libin <libin.yang@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@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.

Is this a mandatory requirement on the new platform?
We saw "disable and enable transcoder" requirement on old platforms.
But it will dramatically impact on the user experience. After discussion,
we skipped this step and did a lot of test on the old platforms. They all works.

I suggested that we can talk with silicon team to figure out a proper solution,
because on the new platform we may also meet the same situations like old
platforms that some resolution will not work properly.

BTW: please test the playback on 4K resolution to see whether the playback
and delay is normal. 4K is often an issue.

Regards,
Libin

>>
>> 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

Re: [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@linux.intel.com>; intel-gfx@lists.freedesktop.org
>Cc: Yang, Libin <libin.yang@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@linux.intel.com]
>Sent: Tuesday, April 4, 2017 6:55 PM
>To: Wang, Quanxian <quanxian.wang@intel.com>; intel-
>gfx@lists.freedesktop.org
>Cc: Yang, Libin <libin.yang@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@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

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

[Wang, Quanxian]

Hi, Libin

We don't have 4K resolution monitor. We will double check others resolutions.

-----Original Message-----
From: Yang, Libin 
Sent: Wednesday, April 5, 2017 10:22 AM
To: Jani Nikula <jani.nikula@linux.intel.com>; Wang, Quanxian <quanxian.wang@intel.com>; intel-gfx@lists.freedesktop.org
Subject: RE: [Intel-gfx] [PATCH] drm/i915/audio: not to set N/M value manually for KBL.


>-----Original Message-----
>From: Jani Nikula [mailto:jani.nikula@linux.intel.com]
>Sent: Tuesday, April 4, 2017 6:55 PM
>To: Wang, Quanxian <quanxian.wang@intel.com>; intel- 
>gfx@lists.freedesktop.org
>Cc: Yang, Libin <libin.yang@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@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.

Is this a mandatory requirement on the new platform?
[Wang, Quanxian] yes. At least bspec said that with 'NOTE'.
We saw "disable and enable transcoder" requirement on old platforms.
But it will dramatically impact on the user experience. After discussion, we skipped this step and did a lot of test on the old platforms. They all works.
[Wang, Quanxian] Seems to be in old platforms.

I suggested that we can talk with silicon team to figure out a proper solution, because on the new platform we may also meet the same situations like old platforms that some resolution will not work properly.

BTW: please test the playback on 4K resolution to see whether the playback and delay is normal. 4K is often an issue.

Regards,
Libin

>>
>> 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

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

[Wang, Quanxian]

Hi, Jani

After testing on 4K monitor. It is fine to get audio with our patch on 
KBL platform. We have tested with 'speaker' command and "play mp3/mp4 directly", not 
found delay for DP audio.

Libin is fine with this patch.

Any comment, please let me know.

Thanks

Quanxian Wang


-----Original Message-----
From: Wang, Quanxian 
Sent: Wednesday, April 5, 2017 10:36 AM
To: Yang, Libin <libin.yang@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>; intel-gfx@lists.freedesktop.org; Cui, YuepingX <yuepingx.cui@intel.com>; Wang, Quanxian <quanxian.wang@intel.com>
Subject: RE: [Intel-gfx] [PATCH] drm/i915/audio: not to set N/M value manually for KBL.

Hi, Libin

We don't have 4K resolution monitor. We will double check others resolutions.

-----Original Message-----
From: Yang, Libin 
Sent: Wednesday, April 5, 2017 10:22 AM
To: Jani Nikula <jani.nikula@linux.intel.com>; Wang, Quanxian <quanxian.wang@intel.com>; intel-gfx@lists.freedesktop.org
Subject: RE: [Intel-gfx] [PATCH] drm/i915/audio: not to set N/M value manually for KBL.


>-----Original Message-----
>From: Jani Nikula [mailto:jani.nikula@linux.intel.com]
>Sent: Tuesday, April 4, 2017 6:55 PM
>To: Wang, Quanxian <quanxian.wang@intel.com>; intel- 
>gfx@lists.freedesktop.org
>Cc: Yang, Libin <libin.yang@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@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.

Is this a mandatory requirement on the new platform?
[Wang, Quanxian] yes. At least bspec said that with 'NOTE'.
We saw "disable and enable transcoder" requirement on old platforms.
But it will dramatically impact on the user experience. After discussion, we skipped this step and did a lot of test on the old platforms. They all works.
[Wang, Quanxian] Seems to be in old platforms.

I suggested that we can talk with silicon team to figure out a proper solution, because on the new platform we may also meet the same situations like old platforms that some resolution will not work properly.

BTW: please test the playback on 4K resolution to see whether the playback and delay is normal. 4K is often an issue.

Regards,
Libin

>>
>> 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

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

[Wang, Quanxian]

Sorry, I will have a new update based on your suggestion. And do more testing on that. After that, I will resend the patch.

Quanxian

-----Original Message-----
From: Wang, Quanxian 
Sent: Wednesday, May 17, 2017 10:45 AM
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org; Yang, Libin <libin.yang@intel.com>
Subject: RE: [Intel-gfx] [PATCH] drm/i915/audio: not to set N/M value manually for KBL.

Hi, Jani

After testing on 4K monitor. It is fine to get audio with our patch on KBL platform. We have tested with 'speaker' command and "play mp3/mp4 directly", not found delay for DP audio.

Libin is fine with this patch.

Any comment, please let me know.

Thanks

Quanxian Wang


-----Original Message-----
From: Wang, Quanxian
Sent: Wednesday, April 5, 2017 10:36 AM
To: Yang, Libin <libin.yang@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>; intel-gfx@lists.freedesktop.org; Cui, YuepingX <yuepingx.cui@intel.com>; Wang, Quanxian <quanxian.wang@intel.com>
Subject: RE: [Intel-gfx] [PATCH] drm/i915/audio: not to set N/M value manually for KBL.

Hi, Libin

We don't have 4K resolution monitor. We will double check others resolutions.

-----Original Message-----
From: Yang, Libin
Sent: Wednesday, April 5, 2017 10:22 AM
To: Jani Nikula <jani.nikula@linux.intel.com>; Wang, Quanxian <quanxian.wang@intel.com>; intel-gfx@lists.freedesktop.org
Subject: RE: [Intel-gfx] [PATCH] drm/i915/audio: not to set N/M value manually for KBL.


>-----Original Message-----
>From: Jani Nikula [mailto:jani.nikula@linux.intel.com]
>Sent: Tuesday, April 4, 2017 6:55 PM
>To: Wang, Quanxian <quanxian.wang@intel.com>; intel- 
>gfx@lists.freedesktop.org
>Cc: Yang, Libin <libin.yang@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@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.

Is this a mandatory requirement on the new platform?
[Wang, Quanxian] yes. At least bspec said that with 'NOTE'.
We saw "disable and enable transcoder" requirement on old platforms.
But it will dramatically impact on the user experience. After discussion, we skipped this step and did a lot of test on the old platforms. They all works.
[Wang, Quanxian] Seems to be in old platforms.

I suggested that we can talk with silicon team to figure out a proper solution, because on the new platform we may also meet the same situations like old platforms that some resolution will not work properly.

BTW: please test the playback on 4K resolution to see whether the playback and delay is normal. 4K is often an issue.

Regards,
Libin

>>
>> 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