* [PATCH] drm/i915/audio: not to set N/M value manually for KBL. @ 2017-04-01 2:38 Quanxian Wang 2017-04-01 2:58 ` ✓ Fi.CI.BAT: success for " Patchwork 2017-04-04 10:54 ` [PATCH] " Jani Nikula 0 siblings, 2 replies; 9+ messages in thread From: Quanxian Wang @ 2017-04-01 2:38 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/audio: not to set N/M value manually for KBL. 2017-04-01 2:38 [PATCH] drm/i915/audio: not to set N/M value manually for KBL Quanxian Wang @ 2017-04-01 2:58 ` Patchwork 2017-04-04 10:54 ` [PATCH] " Jani Nikula 1 sibling, 0 replies; 9+ messages in thread From: Patchwork @ 2017-04-01 2:58 UTC (permalink / raw) To: Quanxian Wang; +Cc: intel-gfx == 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/audio: not to set N/M value manually for KBL. 2017-04-01 2:38 [PATCH] drm/i915/audio: not to set N/M value manually for KBL Quanxian Wang 2017-04-01 2:58 ` ✓ Fi.CI.BAT: success for " Patchwork @ 2017-04-04 10:54 ` Jani Nikula 2017-04-05 2:20 ` Wang, Quanxian 2017-04-05 2:22 ` Yang, Libin 1 sibling, 2 replies; 9+ messages in thread From: Jani Nikula @ 2017-04-04 10:54 UTC (permalink / raw) To: Quanxian Wang, intel-gfx 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/audio: not to set N/M value manually for KBL. 2017-04-04 10:54 ` [PATCH] " Jani Nikula @ 2017-04-05 2:20 ` Wang, Quanxian 2017-04-05 2:25 ` Yang, Libin 2017-04-05 2:22 ` Yang, Libin 1 sibling, 1 reply; 9+ messages in thread From: Wang, Quanxian @ 2017-04-05 2:20 UTC (permalink / raw) To: Jani Nikula, intel-gfx@lists.freedesktop.org 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/audio: not to set N/M value manually for KBL. 2017-04-05 2:20 ` Wang, Quanxian @ 2017-04-05 2:25 ` Yang, Libin 0 siblings, 0 replies; 9+ messages in thread From: Yang, Libin @ 2017-04-05 2:25 UTC (permalink / raw) To: Wang, Quanxian, Jani Nikula, intel-gfx@lists.freedesktop.org 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/audio: not to set N/M value manually for KBL. 2017-04-04 10:54 ` [PATCH] " Jani Nikula 2017-04-05 2:20 ` Wang, Quanxian @ 2017-04-05 2:22 ` Yang, Libin 2017-04-05 2:35 ` Wang, Quanxian 1 sibling, 1 reply; 9+ messages in thread From: Yang, Libin @ 2017-04-05 2:22 UTC (permalink / raw) To: Jani Nikula, Wang, Quanxian, intel-gfx@lists.freedesktop.org >-----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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/audio: not to set N/M value manually for KBL. 2017-04-05 2:22 ` Yang, Libin @ 2017-04-05 2:35 ` Wang, Quanxian 2017-05-17 2:45 ` Wang, Quanxian 0 siblings, 1 reply; 9+ messages in thread From: Wang, Quanxian @ 2017-04-05 2:35 UTC (permalink / raw) To: Yang, Libin; +Cc: intel-gfx@lists.freedesktop.org, Cui, YuepingX 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/audio: not to set N/M value manually for KBL. 2017-04-05 2:35 ` Wang, Quanxian @ 2017-05-17 2:45 ` Wang, Quanxian 2017-05-26 1:12 ` Wang, Quanxian 0 siblings, 1 reply; 9+ messages in thread From: Wang, Quanxian @ 2017-05-17 2:45 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx@lists.freedesktop.org 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/audio: not to set N/M value manually for KBL. 2017-05-17 2:45 ` Wang, Quanxian @ 2017-05-26 1:12 ` Wang, Quanxian 0 siblings, 0 replies; 9+ messages in thread From: Wang, Quanxian @ 2017-05-26 1:12 UTC (permalink / raw) To: 'Jani Nikula'; +Cc: 'intel-gfx@lists.freedesktop.org' 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-05-26 1:12 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-01 2:38 [PATCH] drm/i915/audio: not to set N/M value manually for KBL Quanxian Wang 2017-04-01 2:58 ` ✓ Fi.CI.BAT: success for " Patchwork 2017-04-04 10:54 ` [PATCH] " Jani Nikula 2017-04-05 2:20 ` Wang, Quanxian 2017-04-05 2:25 ` Yang, Libin 2017-04-05 2:22 ` Yang, Libin 2017-04-05 2:35 ` Wang, Quanxian 2017-05-17 2:45 ` Wang, Quanxian 2017-05-26 1:12 ` Wang, Quanxian
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.