| Message ID | 1495778837-6722-1-git-send-email-quanxian.wang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 26 May 2017, Quanxian Wang <quanxian.wang@intel.com> wrote: > According to the bspec, when set N/M, should disable > and enable transcoder which attaching DP audio. However > with that implementation will affect performance more. > > But with current implementation, KBL could not work with defined N/M. > By default, KBL will use automatic N/M. > > Signed-off-by: Quanxian Wang <quanxian.wang@intel.com> > Reviewed-by: Libin Yang <libin.yang@intel.com> > Reviewed-by: Jani Nikula <jani.nikula@intel.com> I have *NOT* reviewed this patch. Even if it's based on my suggestion in [1], it does not automatically signify review. The patch subject should start with "drm/i915/audio:". The DP spec always expresses the values as M/N, more specifically Maud/Naud, so it's confusing to see them expressed as N/M. I still have trouble understanding the commit message. I am wondering if something was fixed in Kabylake and beyond, since according to 6014ac122ed0 ("drm/i915/audio: set proper N/M in modeset") we need this for certain modes. In particular, should the condition check for KBL || gen >= 10? BR, Jani. [1] http://mid.mail-archive.com/87pogs1lz7.fsf@intel.com > --- > drivers/gpu/drm/i915/intel_audio.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c > index d805b6e..ba4994e 100644 > --- a/drivers/gpu/drm/i915/intel_audio.c > +++ b/drivers/gpu/drm/i915/intel_audio.c > @@ -289,6 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port, > enum pipe pipe = intel_crtc->pipe; > u32 tmp; > > + /* It doesn't work on KBL and uses automatic N/M. */ > + if (IS_KABYLAKE(dev_priv)) > + nm = NULL; > + > if (nm) > DRM_DEBUG_KMS("using Maud %u, Naud %u\n", nm->m, nm->n); > else
Hi, Jani Thanks for your comment. My comment below. -----Original Message----- From: Jani Nikula [mailto:jani.nikula@linux.intel.com] Sent: Friday, May 26, 2017 4:31 PM To: Wang, Quanxian <quanxian.wang@intel.com>; intel-gfx@lists.freedesktop.org Cc: Yang, Libin <libin.yang@intel.com>; Wang, Quanxian <quanxian.wang@intel.com> Subject: Re: [PATCH] Defined NM doesn't work on KBL and uses automatic N/M. On Fri, 26 May 2017, Quanxian Wang <quanxian.wang@intel.com> wrote: > According to the bspec, when set N/M, should disable and enable > transcoder which attaching DP audio. However with that implementation > will affect performance more. > > But with current implementation, KBL could not work with defined N/M. > By default, KBL will use automatic N/M. > > Signed-off-by: Quanxian Wang <quanxian.wang@intel.com> > Reviewed-by: Libin Yang <libin.yang@intel.com> > Reviewed-by: Jani Nikula <jani.nikula@intel.com> I have *NOT* reviewed this patch. Even if it's based on my suggestion in [1], it does not automatically signify review. [Wang, Quanxian] Sorry, I accept your suggestion. So in order to respect what you said, I input your name here. The patch subject should start with "drm/i915/audio:". [Wang, Quanxian] Ok. The DP spec always expresses the values as M/N, more specifically Maud/Naud, so it's confusing to see them expressed as N/M. [Wang, Quanxian] accept it. I still have trouble understanding the commit message. I am wondering if something was fixed in Kabylake and beyond, since according to [Wang, Quanxian] in Ubuntu 17.04, we do audio testing on KBL/SKL/APL and more, the audio doesn't work on KBL specially, and other platform works fine. Even if with latest upstream kernel, but it works on 4.8 kernel. Therefore It is a regression issue. After investigation, we found it is caused by commit "6014ac122ed081feca99217bc57b2e15c7fc1a51] drm/i915/audio: set proper N/M in modeset". Therefore I provided this patch. After getting suggestion from you and Libin, we do more testing on 4K monitor for audio delay issue. Also talked with Libin why not use transcoder disable/enable process described in bspec, it will affect performance. This is the whole story. You can check LCK number 3791 in Intel JIRA about that. 6014ac122ed0 ("drm/i915/audio: set proper N/M in modeset") we need this for certain modes. In particular, should the condition check for KBL || gen >= 10? [Wang, Quanxian] Basically if upstream could provide a reasonable patch, it will appreciate since you are expert in this area. Thanks for your understanding. BR, Jani. [1] http://mid.mail-archive.com/87pogs1lz7.fsf@intel.com > --- > drivers/gpu/drm/i915/intel_audio.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_audio.c > b/drivers/gpu/drm/i915/intel_audio.c > index d805b6e..ba4994e 100644 > --- a/drivers/gpu/drm/i915/intel_audio.c > +++ b/drivers/gpu/drm/i915/intel_audio.c > @@ -289,6 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port, > enum pipe pipe = intel_crtc->pipe; > u32 tmp; > > + /* It doesn't work on KBL and uses automatic N/M. */ > + if (IS_KABYLAKE(dev_priv)) > + nm = NULL; > + > if (nm) > DRM_DEBUG_KMS("using Maud %u, Naud %u\n", nm->m, nm->n); > else -- Jani Nikula, Intel Open Source Technology Center
On Fri, 26 May 2017, "Wang, Quanxian" <quanxian.wang@intel.com> wrote: > [Wang, Quanxian] in Ubuntu 17.04, we do audio testing on KBL/SKL/APL > and more, the audio doesn't work on KBL specially, and other platform > works fine. Even if with latest upstream kernel, but it works on 4.8 > kernel. Therefore It is a regression issue. After investigation, we > found it is caused by commit > "6014ac122ed081feca99217bc57b2e15c7fc1a51] drm/i915/audio: set proper > N/M in modeset". Therefore I provided this patch. If it's a regression, we need to backport the fix to stable kernels, and the patch needs annotation: Fixes: 6014ac122ed0 ("drm/i915/audio: set proper N/M in modeset") Cc: <stable@vger.kernel.org> # v4.10+ > After getting suggestion from you and Libin, we do more testing on 4K > monitor for audio delay issue. Also talked with Libin why not use > transcoder disable/enable process described in bspec, it will affect > performance. > > This is the whole story. You can check LCK number 3791 in Intel JIRA > about that. Actually, I can't. And the commit message needs to have the explanation. I still do not know what the bug is specifically, or why Kabylake, and only Kabylake, needs this. > [Wang, Quanxian] Basically if upstream could provide a reasonable > patch, it will appreciate since you are expert in this area. > Thanks for your understanding. The patch is probably fine. But I can't invent an explanation if I don't know what the problem is. BR, Jani.
"The patch is probably fine. But I can't invent an explanation if I don't know what the problem is." @Jani @libin, is it possible for you to help contact Intel graphics hardware design engineer to get the reason why M/N doesn't work as you implemented in kernel code in Kabylake platform? I don't find good reason in bspec. Thanks. -----Original Message----- From: Jani Nikula [mailto:jani.nikula@linux.intel.com] Sent: Friday, May 26, 2017 7:01 PM To: Wang, Quanxian <quanxian.wang@intel.com>; intel-gfx@lists.freedesktop.org Cc: Yang, Libin <libin.yang@intel.com> Subject: RE: [PATCH] Defined NM doesn't work on KBL and uses automatic N/M. On Fri, 26 May 2017, "Wang, Quanxian" <quanxian.wang@intel.com> wrote: > [Wang, Quanxian] in Ubuntu 17.04, we do audio testing on KBL/SKL/APL > and more, the audio doesn't work on KBL specially, and other platform > works fine. Even if with latest upstream kernel, but it works on 4.8 > kernel. Therefore It is a regression issue. After investigation, we > found it is caused by commit > "6014ac122ed081feca99217bc57b2e15c7fc1a51] drm/i915/audio: set proper > N/M in modeset". Therefore I provided this patch. If it's a regression, we need to backport the fix to stable kernels, and the patch needs annotation: Fixes: 6014ac122ed0 ("drm/i915/audio: set proper N/M in modeset") Cc: <stable@vger.kernel.org> # v4.10+ > After getting suggestion from you and Libin, we do more testing on 4K > monitor for audio delay issue. Also talked with Libin why not use > transcoder disable/enable process described in bspec, it will affect > performance. > > This is the whole story. You can check LCK number 3791 in Intel JIRA > about that. Actually, I can't. And the commit message needs to have the explanation. I still do not know what the bug is specifically, or why Kabylake, and only Kabylake, needs this. > [Wang, Quanxian] Basically if upstream could provide a reasonable > patch, it will appreciate since you are expert in this area. > Thanks for your understanding. The patch is probably fine. But I can't invent an explanation if I don't know what the problem is. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center
On Fri, 26 May 2017, "Wang, Quanxian" <quanxian.wang@intel.com> wrote: > "The patch is probably fine. But I can't invent an explanation if I > don't know what the problem is." > > @Jani @libin, is it possible for you to help contact Intel graphics > hardware design engineer to get the reason why M/N doesn't work as you > implemented in kernel code in Kabylake platform? I don't find good > reason in bspec. Thanks. How did you come up with the patch then? What made you try this? BR, Jani. > > -----Original Message----- > From: Jani Nikula [mailto:jani.nikula@linux.intel.com] > Sent: Friday, May 26, 2017 7:01 PM > To: Wang, Quanxian <quanxian.wang@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Yang, Libin <libin.yang@intel.com> > Subject: RE: [PATCH] Defined NM doesn't work on KBL and uses automatic N/M. > > On Fri, 26 May 2017, "Wang, Quanxian" <quanxian.wang@intel.com> wrote: >> [Wang, Quanxian] in Ubuntu 17.04, we do audio testing on KBL/SKL/APL >> and more, the audio doesn't work on KBL specially, and other platform >> works fine. Even if with latest upstream kernel, but it works on 4.8 >> kernel. Therefore It is a regression issue. After investigation, we >> found it is caused by commit >> "6014ac122ed081feca99217bc57b2e15c7fc1a51] drm/i915/audio: set proper >> N/M in modeset". Therefore I provided this patch. > > If it's a regression, we need to backport the fix to stable kernels, and the patch needs annotation: > > Fixes: 6014ac122ed0 ("drm/i915/audio: set proper N/M in modeset") > Cc: <stable@vger.kernel.org> # v4.10+ > >> After getting suggestion from you and Libin, we do more testing on 4K >> monitor for audio delay issue. Also talked with Libin why not use >> transcoder disable/enable process described in bspec, it will affect >> performance. >> >> This is the whole story. You can check LCK number 3791 in Intel JIRA >> about that. > > Actually, I can't. And the commit message needs to have the explanation. I still do not know what the bug is specifically, or why Kabylake, and only Kabylake, needs this. > >> [Wang, Quanxian] Basically if upstream could provide a reasonable >> patch, it will appreciate since you are expert in this area. >> Thanks for your understanding. > > The patch is probably fine. But I can't invent an explanation if I don't know what the problem is. > > > BR, > Jani. > > -- > Jani Nikula, Intel Open Source Technology Center
-----Original Message----- From: Jani Nikula [mailto:jani.nikula@linux.intel.com] Sent: Monday, May 29, 2017 4:29 PM To: Wang, Quanxian <quanxian.wang@intel.com>; intel-gfx@lists.freedesktop.org Cc: Yang, Libin <libin.yang@intel.com> Subject: RE: [PATCH] Defined NM doesn't work on KBL and uses automatic N/M. On Fri, 26 May 2017, "Wang, Quanxian" <quanxian.wang@intel.com> wrote: > "The patch is probably fine. But I can't invent an explanation if I > don't know what the problem is." > > @Jani @libin, is it possible for you to help contact Intel graphics > hardware design engineer to get the reason why M/N doesn't work as you > implemented in kernel code in Kabylake platform? I don't find good > reason in bspec. Thanks. How did you come up with the patch then? What made you try this? [Wang, Quanxian] I am from OSV enabling team and responsible for Ubuntu enabling. 17.04 (kernel 4.10) is released, we will do full testing for all Intel platforms including KBL/SKL... With audio testing, we found this regression issue on KBL and find patch caused this regression. Compared with previous implementation, we use automatic M/N to have a try. It works. BR, Jani. > > -----Original Message----- > From: Jani Nikula [mailto:jani.nikula@linux.intel.com] > Sent: Friday, May 26, 2017 7:01 PM > To: Wang, Quanxian <quanxian.wang@intel.com>; > intel-gfx@lists.freedesktop.org > Cc: Yang, Libin <libin.yang@intel.com> > Subject: RE: [PATCH] Defined NM doesn't work on KBL and uses automatic N/M. > > On Fri, 26 May 2017, "Wang, Quanxian" <quanxian.wang@intel.com> wrote: >> [Wang, Quanxian] in Ubuntu 17.04, we do audio testing on KBL/SKL/APL >> and more, the audio doesn't work on KBL specially, and other platform >> works fine. Even if with latest upstream kernel, but it works on 4.8 >> kernel. Therefore It is a regression issue. After investigation, we >> found it is caused by commit >> "6014ac122ed081feca99217bc57b2e15c7fc1a51] drm/i915/audio: set proper >> N/M in modeset". Therefore I provided this patch. > > If it's a regression, we need to backport the fix to stable kernels, and the patch needs annotation: > > Fixes: 6014ac122ed0 ("drm/i915/audio: set proper N/M in modeset") > Cc: <stable@vger.kernel.org> # v4.10+ > >> After getting suggestion from you and Libin, we do more testing on 4K >> monitor for audio delay issue. Also talked with Libin why not use >> transcoder disable/enable process described in bspec, it will affect >> performance. >> >> This is the whole story. You can check LCK number 3791 in Intel JIRA >> about that. > > Actually, I can't. And the commit message needs to have the explanation. I still do not know what the bug is specifically, or why Kabylake, and only Kabylake, needs this. > >> [Wang, Quanxian] Basically if upstream could provide a reasonable >> patch, it will appreciate since you are expert in this area. >> Thanks for your understanding. > > The patch is probably fine. But I can't invent an explanation if I don't know what the problem is. > > > BR, > Jani. > > -- > Jani Nikula, Intel Open Source Technology Center -- Jani Nikula, Intel Open Source Technology Center
Hi, Jani I have created a Bugzilla in free desktop for tracking. https://bugs.freedesktop.org/show_bug.cgi?id=104093 Appreciate find audio expert could help us address root cause. Thank you. Quanxian -----Original Message----- From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Wang, Quanxian Sent: Wednesday, May 31, 2017 11:21 AM To: Jani Nikula <jani.nikula@linux.intel.com>; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] Defined NM doesn't work on KBL and uses automatic N/M. -----Original Message----- From: Jani Nikula [mailto:jani.nikula@linux.intel.com] Sent: Monday, May 29, 2017 4:29 PM To: Wang, Quanxian <quanxian.wang@intel.com>; intel-gfx@lists.freedesktop.org Cc: Yang, Libin <libin.yang@intel.com> Subject: RE: [PATCH] Defined NM doesn't work on KBL and uses automatic N/M. On Fri, 26 May 2017, "Wang, Quanxian" <quanxian.wang@intel.com> wrote: > "The patch is probably fine. But I can't invent an explanation if I > don't know what the problem is." > > @Jani @libin, is it possible for you to help contact Intel graphics > hardware design engineer to get the reason why M/N doesn't work as you > implemented in kernel code in Kabylake platform? I don't find good > reason in bspec. Thanks. How did you come up with the patch then? What made you try this? [Wang, Quanxian] I am from OSV enabling team and responsible for Ubuntu enabling. 17.04 (kernel 4.10) is released, we will do full testing for all Intel platforms including KBL/SKL... With audio testing, we found this regression issue on KBL and find patch caused this regression. Compared with previous implementation, we use automatic M/N to have a try. It works. BR, Jani. > > -----Original Message----- > From: Jani Nikula [mailto:jani.nikula@linux.intel.com] > Sent: Friday, May 26, 2017 7:01 PM > To: Wang, Quanxian <quanxian.wang@intel.com>; > intel-gfx@lists.freedesktop.org > Cc: Yang, Libin <libin.yang@intel.com> > Subject: RE: [PATCH] Defined NM doesn't work on KBL and uses automatic N/M. > > On Fri, 26 May 2017, "Wang, Quanxian" <quanxian.wang@intel.com> wrote: >> [Wang, Quanxian] in Ubuntu 17.04, we do audio testing on KBL/SKL/APL >> and more, the audio doesn't work on KBL specially, and other platform >> works fine. Even if with latest upstream kernel, but it works on 4.8 >> kernel. Therefore It is a regression issue. After investigation, we >> found it is caused by commit >> "6014ac122ed081feca99217bc57b2e15c7fc1a51] drm/i915/audio: set proper >> N/M in modeset". Therefore I provided this patch. > > If it's a regression, we need to backport the fix to stable kernels, and the patch needs annotation: > > Fixes: 6014ac122ed0 ("drm/i915/audio: set proper N/M in modeset") > Cc: <stable@vger.kernel.org> # v4.10+ > >> After getting suggestion from you and Libin, we do more testing on 4K >> monitor for audio delay issue. Also talked with Libin why not use >> transcoder disable/enable process described in bspec, it will affect >> performance. >> >> This is the whole story. You can check LCK number 3791 in Intel JIRA >> about that. > > Actually, I can't. And the commit message needs to have the explanation. I still do not know what the bug is specifically, or why Kabylake, and only Kabylake, needs this. > >> [Wang, Quanxian] Basically if upstream could provide a reasonable >> patch, it will appreciate since you are expert in this area. >> Thanks for your understanding. > > The patch is probably fine. But I can't invent an explanation if I don't know what the problem is. > > > BR, > Jani. > > -- > Jani Nikula, Intel Open Source Technology Center -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index d805b6e..ba4994e 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -289,6 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port, enum pipe pipe = intel_crtc->pipe; u32 tmp; + /* It doesn't work on KBL and uses automatic N/M. */ + if (IS_KABYLAKE(dev_priv)) + nm = NULL; + if (nm) DRM_DEBUG_KMS("using Maud %u, Naud %u\n", nm->m, nm->n); else