[Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK

[Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK

[Kai Vehmanen]

In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
to separate function") the order of force_min_cdclk_changed check and
intel_modeset_checks(), was reversed. This broke the mechanism to
immediately force a new CDCLK minimum, and lead to driver probe
errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
the issue by moving intel_modeset_checks() call later.

Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking to separate function)"
BugLink: https://github.com/thesofproject/linux/issues/2410
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 7d50b7177d40..8caeed23037c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct drm_device *dev,
 	if (dev_priv->wm.distrust_bios_wm)
 		any_ms = true;
 
-	if (any_ms) {
-		ret = intel_modeset_checks(state);
-		if (ret)
-			goto fail;
-	}
-
 	intel_fbc_choose_crtc(dev_priv, state);
 	ret = calc_watermark_data(state);
 	if (ret)
@@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct drm_device *dev,
 		goto fail;
 
 	if (any_ms) {
+		ret = intel_modeset_checks(state);
+		if (ret)
+			goto fail;
+
 		ret = intel_modeset_calc_cdclk(state);
 		if (ret)
 			return ret;
-- 
2.27.0

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: fix regression leading to display audio probe failure on GLK

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 3981 bytes --]

== Series Details ==

Series: drm/i915: fix regression leading to display audio probe failure on GLK
URL   : https://patchwork.freedesktop.org/series/81227/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8951 -> Patchwork_18429
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/index.html

Known issues
------------

  Here are the changes found in Patchwork_18429 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_parallel@engines@contexts:
    - fi-apl-guc:         [PASS][1] -> [INCOMPLETE][2] ([i915#1635] / [i915#2398])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/fi-apl-guc/igt@gem_exec_parallel@engines@contexts.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/fi-apl-guc/igt@gem_exec_parallel@engines@contexts.html

  * igt@gem_exec_parallel@engines@fds:
    - fi-kbl-7500u:       [PASS][3] -> [INCOMPLETE][4] ([i915#2398] / [i915#794])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/fi-kbl-7500u/igt@gem_exec_parallel@engines@fds.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/fi-kbl-7500u/igt@gem_exec_parallel@engines@fds.html
    - fi-skl-lmem:        [PASS][5] -> [INCOMPLETE][6] ([i915#2398])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/fi-skl-lmem/igt@gem_exec_parallel@engines@fds.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/fi-skl-lmem/igt@gem_exec_parallel@engines@fds.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1:
    - fi-icl-u2:          [PASS][7] -> [DMESG-WARN][8] ([i915#1982]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2:
    - fi-skl-guc:         [PASS][9] -> [DMESG-WARN][10] ([i915#2203])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gem_contexts:
    - fi-cml-s:           [INCOMPLETE][11] -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/fi-cml-s/igt@i915_selftest@live@gem_contexts.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/fi-cml-s/igt@i915_selftest@live@gem_contexts.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
  [i915#2398]: https://gitlab.freedesktop.org/drm/intel/issues/2398
  [i915#794]: https://gitlab.freedesktop.org/drm/intel/issues/794


Participating hosts (38 -> 33)
------------------------------

  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper 


Build changes
-------------

  * Linux: CI_DRM_8951 -> Patchwork_18429

  CI-20190529: 20190529
  CI_DRM_8951: 5f8c53e35ef01a1d5e97db6005d3c308d3734bac @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5775: 58cb0aea18fa471af43c11ee9f587554721a7815 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18429: 7db2227d72fdf96d49b2fbb535732d0f0d442f87 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

7db2227d72fd drm/i915: fix regression leading to display audio probe failure on GLK

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/index.html

[-- Attachment #1.2: Type: text/html, Size: 4856 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK

[Ville Syrjälä]

On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
> In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> to separate function") the order of force_min_cdclk_changed check and
> intel_modeset_checks(), was reversed. This broke the mechanism to
> immediately force a new CDCLK minimum, and lead to driver probe
> errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
> the issue by moving intel_modeset_checks() call later.

Yep. I eyeed this same code recently and noticed the same bug.
The one thing I didn't yet figure out is whether there is some
subtle ordering requirement that was the reason for the change.
But considering intel_modeset_checks() doesn't really do much
anymore I think it should be safe.

Sadly CI has been lumping all underrun errors under some ancient
bugs, so no one noticed that things started to fail when this
regression was introduced :(

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking to separate function)"
> BugLink: https://github.com/thesofproject/linux/issues/2410
> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 7d50b7177d40..8caeed23037c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct drm_device *dev,
>  	if (dev_priv->wm.distrust_bios_wm)
>  		any_ms = true;
>  
> -	if (any_ms) {
> -		ret = intel_modeset_checks(state);
> -		if (ret)
> -			goto fail;
> -	}
> -
>  	intel_fbc_choose_crtc(dev_priv, state);
>  	ret = calc_watermark_data(state);
>  	if (ret)
> @@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct drm_device *dev,
>  		goto fail;
>  
>  	if (any_ms) {
> +		ret = intel_modeset_checks(state);
> +		if (ret)
> +			goto fail;
> +
>  		ret = intel_modeset_calc_cdclk(state);
>  		if (ret)
>  			return ret;
> -- 
> 2.27.0

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

Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK

[Lisovskiy, Stanislav]

On Wed, Sep 02, 2020 at 01:31:09PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
> > In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> > to separate function") the order of force_min_cdclk_changed check and
> > intel_modeset_checks(), was reversed. This broke the mechanism to
> > immediately force a new CDCLK minimum, and lead to driver probe
> > errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
> > the issue by moving intel_modeset_checks() call later.
> 
> Yep. I eyeed this same code recently and noticed the same bug.
> The one thing I didn't yet figure out is whether there is some
> subtle ordering requirement that was the reason for the change.
> But considering intel_modeset_checks() doesn't really do much
> anymore I think it should be safe.
> 
> Sadly CI has been lumping all underrun errors under some ancient
> bugs, so no one noticed that things started to fail when this
> regression was introduced :(
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

What surprises me here, is that the actual patch has been sent
and merged during late spring I think and we figure out that there was
a regression only by now. 
For example I figured out this only today. When I was doing that change, 
was actually aware that the change is actually quite significant as 
it changes the way how we deal with CDCLK, however those were necessary 
as we had a massive FIFO underrun issues at the moment. However CI didn't
show any problems, so we went ahead with this.

> 
> > 
> > Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking to separate function)"
> > BugLink: https://github.com/thesofproject/linux/issues/2410
> > Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 7d50b7177d40..8caeed23037c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct drm_device *dev,
> >  	if (dev_priv->wm.distrust_bios_wm)
> >  		any_ms = true;
> >  
> > -	if (any_ms) {
> > -		ret = intel_modeset_checks(state);
> > -		if (ret)
> > -			goto fail;
> > -	}
> > -
> >  	intel_fbc_choose_crtc(dev_priv, state);
> >  	ret = calc_watermark_data(state);
> >  	if (ret)
> > @@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct drm_device *dev,
> >  		goto fail;
> >  
> >  	if (any_ms) {
> > +		ret = intel_modeset_checks(state);
> > +		if (ret)
> > +			goto fail;
> > +
> >  		ret = intel_modeset_calc_cdclk(state);
> >  		if (ret)
> >  			return ret;
> > -- 
> > 2.27.0
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK

[Ville Syrjälä]

On Wed, Sep 02, 2020 at 03:12:01PM +0300, Lisovskiy, Stanislav wrote:
> On Wed, Sep 02, 2020 at 01:31:09PM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
> > > In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> > > to separate function") the order of force_min_cdclk_changed check and
> > > intel_modeset_checks(), was reversed. This broke the mechanism to
> > > immediately force a new CDCLK minimum, and lead to driver probe
> > > errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
> > > the issue by moving intel_modeset_checks() call later.
> > 
> > Yep. I eyeed this same code recently and noticed the same bug.
> > The one thing I didn't yet figure out is whether there is some
> > subtle ordering requirement that was the reason for the change.
> > But considering intel_modeset_checks() doesn't really do much
> > anymore I think it should be safe.
> > 
> > Sadly CI has been lumping all underrun errors under some ancient
> > bugs, so no one noticed that things started to fail when this
> > regression was introduced :(
> > 
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> What surprises me here, is that the actual patch has been sent
> and merged during late spring I think and we figure out that there was
> a regression only by now. 
> For example I figured out this only today. When I was doing that change, 
> was actually aware that the change is actually quite significant as 
> it changes the way how we deal with CDCLK, however those were necessary 
> as we had a massive FIFO underrun issues at the moment. However CI didn't
> show any problems, so we went ahead with this.

I spotted some CI logs that show underruns due to this regression,
but the results just got lumped in with other older underrun bugs,
and thus CI results were always "success" :/

I think we need to kill off all underrun related CI filters and
start from scratch. Otherwise new bugs will keep slipping through.

> 
> > 
> > > 
> > > Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking to separate function)"
> > > BugLink: https://github.com/thesofproject/linux/issues/2410
> > > Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
> > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 7d50b7177d40..8caeed23037c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct drm_device *dev,
> > >  	if (dev_priv->wm.distrust_bios_wm)
> > >  		any_ms = true;
> > >  
> > > -	if (any_ms) {
> > > -		ret = intel_modeset_checks(state);
> > > -		if (ret)
> > > -			goto fail;
> > > -	}
> > > -
> > >  	intel_fbc_choose_crtc(dev_priv, state);
> > >  	ret = calc_watermark_data(state);
> > >  	if (ret)
> > > @@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct drm_device *dev,
> > >  		goto fail;
> > >  
> > >  	if (any_ms) {
> > > +		ret = intel_modeset_checks(state);
> > > +		if (ret)
> > > +			goto fail;
> > > +
> > >  		ret = intel_modeset_calc_cdclk(state);
> > >  		if (ret)
> > >  			return ret;
> > > -- 
> > > 2.27.0
> > 
> > -- 
> > Ville Syrjälä
> > Intel

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

Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK

[Lisovskiy, Stanislav]

On Wed, Sep 02, 2020 at 03:17:08PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 02, 2020 at 03:12:01PM +0300, Lisovskiy, Stanislav wrote:
> > On Wed, Sep 02, 2020 at 01:31:09PM +0300, Ville Syrjälä wrote:
> > > On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
> > > > In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> > > > to separate function") the order of force_min_cdclk_changed check and
> > > > intel_modeset_checks(), was reversed. This broke the mechanism to
> > > > immediately force a new CDCLK minimum, and lead to driver probe
> > > > errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
> > > > the issue by moving intel_modeset_checks() call later.
> > > 
> > > Yep. I eyeed this same code recently and noticed the same bug.
> > > The one thing I didn't yet figure out is whether there is some
> > > subtle ordering requirement that was the reason for the change.
> > > But considering intel_modeset_checks() doesn't really do much
> > > anymore I think it should be safe.
> > > 
> > > Sadly CI has been lumping all underrun errors under some ancient
> > > bugs, so no one noticed that things started to fail when this
> > > regression was introduced :(
> > > 
> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > What surprises me here, is that the actual patch has been sent
> > and merged during late spring I think and we figure out that there was
> > a regression only by now. 
> > For example I figured out this only today. When I was doing that change, 
> > was actually aware that the change is actually quite significant as 
> > it changes the way how we deal with CDCLK, however those were necessary 
> > as we had a massive FIFO underrun issues at the moment. However CI didn't
> > show any problems, so we went ahead with this.
> 
> I spotted some CI logs that show underruns due to this regression,
> but the results just got lumped in with other older underrun bugs,
> and thus CI results were always "success" :/
> 
> I think we need to kill off all underrun related CI filters and
> start from scratch. Otherwise new bugs will keep slipping through.

Another concern I have here, as I understand now intel_modeset_checks
will be put again after wm/ddb and bw calculations - won't this 
cause any additional issues?

Also you now have modeset checks still before that force_min_cdclk
condition check which is now in intel_modeset_calc_cdclk.
My idea was to put all CDCLK related calculations and checks into
same function. However this could be a bad idea, so should you may
be just extract force_min_cdclk check condition back from intel_modeset_calc_cdclk
to the original place where it was?

I'm just now thinking in terms of not breaking anything else now...

Stan

> 
> > 
> > > 
> > > > 
> > > > Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking to separate function)"
> > > > BugLink: https://github.com/thesofproject/linux/issues/2410
> > > > Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
> > > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 7d50b7177d40..8caeed23037c 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct drm_device *dev,
> > > >  	if (dev_priv->wm.distrust_bios_wm)
> > > >  		any_ms = true;
> > > >  
> > > > -	if (any_ms) {
> > > > -		ret = intel_modeset_checks(state);
> > > > -		if (ret)
> > > > -			goto fail;
> > > > -	}
> > > > -
> > > >  	intel_fbc_choose_crtc(dev_priv, state);
> > > >  	ret = calc_watermark_data(state);
> > > >  	if (ret)
> > > > @@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct drm_device *dev,
> > > >  		goto fail;
> > > >  
> > > >  	if (any_ms) {
> > > > +		ret = intel_modeset_checks(state);
> > > > +		if (ret)
> > > > +			goto fail;
> > > > +
> > > >  		ret = intel_modeset_calc_cdclk(state);
> > > >  		if (ret)
> > > >  			return ret;
> > > > -- 
> > > > 2.27.0
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK

[Ville Syrjälä]

On Wed, Sep 02, 2020 at 03:34:50PM +0300, Lisovskiy, Stanislav wrote:
> On Wed, Sep 02, 2020 at 03:17:08PM +0300, Ville Syrjälä wrote:
> > On Wed, Sep 02, 2020 at 03:12:01PM +0300, Lisovskiy, Stanislav wrote:
> > > On Wed, Sep 02, 2020 at 01:31:09PM +0300, Ville Syrjälä wrote:
> > > > On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
> > > > > In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> > > > > to separate function") the order of force_min_cdclk_changed check and
> > > > > intel_modeset_checks(), was reversed. This broke the mechanism to
> > > > > immediately force a new CDCLK minimum, and lead to driver probe
> > > > > errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
> > > > > the issue by moving intel_modeset_checks() call later.
> > > > 
> > > > Yep. I eyeed this same code recently and noticed the same bug.
> > > > The one thing I didn't yet figure out is whether there is some
> > > > subtle ordering requirement that was the reason for the change.
> > > > But considering intel_modeset_checks() doesn't really do much
> > > > anymore I think it should be safe.
> > > > 
> > > > Sadly CI has been lumping all underrun errors under some ancient
> > > > bugs, so no one noticed that things started to fail when this
> > > > regression was introduced :(
> > > > 
> > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > What surprises me here, is that the actual patch has been sent
> > > and merged during late spring I think and we figure out that there was
> > > a regression only by now. 
> > > For example I figured out this only today. When I was doing that change, 
> > > was actually aware that the change is actually quite significant as 
> > > it changes the way how we deal with CDCLK, however those were necessary 
> > > as we had a massive FIFO underrun issues at the moment. However CI didn't
> > > show any problems, so we went ahead with this.
> > 
> > I spotted some CI logs that show underruns due to this regression,
> > but the results just got lumped in with other older underrun bugs,
> > and thus CI results were always "success" :/
> > 
> > I think we need to kill off all underrun related CI filters and
> > start from scratch. Otherwise new bugs will keep slipping through.
> 
> Another concern I have here, as I understand now intel_modeset_checks
> will be put again after wm/ddb and bw calculations - won't this 
> cause any additional issues?

intel_modeset_checks() doesn't really do much anymore.

I think the old global state mess was the reason there was
some ordering requirement between the two. But if you see the
patch I just posted that old global state stuff is now dead
code that can be ripped out.

Oh, the other linkage was the cdclk vs. linetime watermark, but
I moved the linetime wm stuff elsewhere already a while ago. So
also not an issue.

> 
> Also you now have modeset checks still before that force_min_cdclk
> condition check which is now in intel_modeset_calc_cdclk.
> My idea was to put all CDCLK related calculations and checks into
> same function. However this could be a bad idea, so should you may
> be just extract force_min_cdclk check condition back from intel_modeset_calc_cdclk
> to the original place where it was?

The force cdclk stuff looks fine to me. Though there is
https://patchwork.freedesktop.org/patch/377191/?series=79480&rev=1
still unreviewed which would clean it up a bit further.

> 
> I'm just now thinking in terms of not breaking anything else now...
> 
> Stan
> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking to separate function)"
> > > > > BugLink: https://github.com/thesofproject/linux/issues/2410
> > > > > Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
> > > > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > index 7d50b7177d40..8caeed23037c 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > @@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct drm_device *dev,
> > > > >  	if (dev_priv->wm.distrust_bios_wm)
> > > > >  		any_ms = true;
> > > > >  
> > > > > -	if (any_ms) {
> > > > > -		ret = intel_modeset_checks(state);
> > > > > -		if (ret)
> > > > > -			goto fail;
> > > > > -	}
> > > > > -
> > > > >  	intel_fbc_choose_crtc(dev_priv, state);
> > > > >  	ret = calc_watermark_data(state);
> > > > >  	if (ret)
> > > > > @@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct drm_device *dev,
> > > > >  		goto fail;
> > > > >  
> > > > >  	if (any_ms) {
> > > > > +		ret = intel_modeset_checks(state);
> > > > > +		if (ret)
> > > > > +			goto fail;
> > > > > +
> > > > >  		ret = intel_modeset_calc_cdclk(state);
> > > > >  		if (ret)
> > > > >  			return ret;
> > > > > -- 
> > > > > 2.27.0
> > > > 
> > > > -- 
> > > > Ville Syrjälä
> > > > Intel
> > 
> > -- 
> > Ville Syrjälä
> > Intel

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

Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK

[Lisovskiy, Stanislav]

On Wed, Sep 02, 2020 at 01:31:09PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
> > In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> > to separate function") the order of force_min_cdclk_changed check and
> > intel_modeset_checks(), was reversed. This broke the mechanism to
> > immediately force a new CDCLK minimum, and lead to driver probe
> > errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
> > the issue by moving intel_modeset_checks() call later.
> 
> Yep. I eyeed this same code recently and noticed the same bug.
> The one thing I didn't yet figure out is whether there is some
> subtle ordering requirement that was the reason for the change.
> But considering intel_modeset_checks() doesn't really do much
> anymore I think it should be safe.
> 
> Sadly CI has been lumping all underrun errors under some ancient
> bugs, so no one noticed that things started to fail when this
> regression was introduced :(
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Replying to that thread as well:

Another concern I have here, as I understand now intel_modeset_checks
will be put again after wm/ddb and bw calculations - won't this
cause any additional issues?

Also you now have modeset checks still before that force_min_cdclk
condition check which is now in intel_modeset_calc_cdclk.
My idea was to put all CDCLK related calculations and checks into
same function. However this could be a bad idea, so should you may
be just extract force_min_cdclk check condition back from intel_modeset_calc_cdclk
to the original place where it was?

I'm just now thinking in terms of not breaking anything else now...

Stan

> 
> > 
> > Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking to separate function)"
> > BugLink: https://github.com/thesofproject/linux/issues/2410
> > Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 7d50b7177d40..8caeed23037c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct drm_device *dev,
> >  	if (dev_priv->wm.distrust_bios_wm)
> >  		any_ms = true;
> >  
> > -	if (any_ms) {
> > -		ret = intel_modeset_checks(state);
> > -		if (ret)
> > -			goto fail;
> > -	}
> > -
> >  	intel_fbc_choose_crtc(dev_priv, state);
> >  	ret = calc_watermark_data(state);
> >  	if (ret)
> > @@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct drm_device *dev,
> >  		goto fail;
> >  
> >  	if (any_ms) {
> > +		ret = intel_modeset_checks(state);
> > +		if (ret)
> > +			goto fail;
> > +
> >  		ret = intel_modeset_calc_cdclk(state);
> >  		if (ret)
> >  			return ret;
> > -- 
> > 2.27.0
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: fix regression leading to display audio probe failure on GLK

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 16438 bytes --]

== Series Details ==

Series: drm/i915: fix regression leading to display audio probe failure on GLK
URL   : https://patchwork.freedesktop.org/series/81227/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8951_full -> Patchwork_18429_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_18429_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_busy@close-race:
    - shard-skl:          [PASS][1] -> [DMESG-WARN][2] ([i915#1982]) +10 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-skl9/igt@gem_busy@close-race.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-skl6/igt@gem_busy@close-race.html

  * igt@gem_exec_nop@basic-sequential:
    - shard-kbl:          [PASS][3] -> [TIMEOUT][4] ([i915#1958])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-kbl7/igt@gem_exec_nop@basic-sequential.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-kbl4/igt@gem_exec_nop@basic-sequential.html

  * igt@gem_exec_reloc@basic-many-active@vcs0:
    - shard-glk:          [PASS][5] -> [FAIL][6] ([i915#2389])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk8/igt@gem_exec_reloc@basic-many-active@vcs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk9/igt@gem_exec_reloc@basic-many-active@vcs0.html

  * igt@gem_exec_whisper@basic-contexts-forked:
    - shard-glk:          [PASS][7] -> [DMESG-WARN][8] ([i915#118] / [i915#95])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk4/igt@gem_exec_whisper@basic-contexts-forked.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk4/igt@gem_exec_whisper@basic-contexts-forked.html

  * igt@gem_exec_whisper@basic-fds-forked:
    - shard-apl:          [PASS][9] -> [TIMEOUT][10] ([i915#1635] / [i915#1958]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-apl3/igt@gem_exec_whisper@basic-fds-forked.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-apl4/igt@gem_exec_whisper@basic-fds-forked.html

  * igt@gem_exec_whisper@basic-forked:
    - shard-iclb:         [PASS][11] -> [TIMEOUT][12] ([i915#1958])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-iclb8/igt@gem_exec_whisper@basic-forked.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-iclb1/igt@gem_exec_whisper@basic-forked.html
    - shard-skl:          [PASS][13] -> [TIMEOUT][14] ([i915#1958])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-skl9/igt@gem_exec_whisper@basic-forked.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-skl6/igt@gem_exec_whisper@basic-forked.html
    - shard-tglb:         [PASS][15] -> [TIMEOUT][16] ([i915#1958])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-tglb5/igt@gem_exec_whisper@basic-forked.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-tglb7/igt@gem_exec_whisper@basic-forked.html

  * igt@gem_exec_whisper@basic-queues-forked-all:
    - shard-glk:          [PASS][17] -> [TIMEOUT][18] ([i915#1958]) +3 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk5/igt@gem_exec_whisper@basic-queues-forked-all.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk8/igt@gem_exec_whisper@basic-queues-forked-all.html

  * igt@gem_sync@basic-store-all:
    - shard-glk:          [PASS][19] -> [FAIL][20] ([i915#2356])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk7/igt@gem_sync@basic-store-all.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk1/igt@gem_sync@basic-store-all.html
    - shard-tglb:         [PASS][21] -> [FAIL][22] ([i915#2356])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-tglb8/igt@gem_sync@basic-store-all.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-tglb5/igt@gem_sync@basic-store-all.html

  * igt@kms_flip@2x-plain-flip-ts-check@bc-hdmi-a1-hdmi-a2:
    - shard-glk:          [PASS][23] -> [FAIL][24] ([i915#2122])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk8/igt@kms_flip@2x-plain-flip-ts-check@bc-hdmi-a1-hdmi-a2.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk2/igt@kms_flip@2x-plain-flip-ts-check@bc-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@c-hdmi-a1:
    - shard-glk:          [PASS][25] -> [FAIL][26] ([i915#79])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk1/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-hdmi-a1.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk1/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-hdmi-a1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
    - shard-kbl:          [PASS][27] -> [DMESG-WARN][28] ([i915#180]) +7 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-kbl2/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-pwrite:
    - shard-tglb:         [PASS][29] -> [DMESG-WARN][30] ([i915#1982]) +3 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-pwrite.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-tglb3/igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-pwrite.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [PASS][31] -> [FAIL][32] ([fdo#108145] / [i915#265])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [PASS][33] -> [SKIP][34] ([fdo#109441]) +2 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-iclb3/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][35] -> [FAIL][36] ([i915#1635] / [i915#31])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-apl1/igt@kms_setmode@basic.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-apl2/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@gem_exec_nop@basic-sequential:
    - shard-tglb:         [TIMEOUT][37] ([i915#1958]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-tglb7/igt@gem_exec_nop@basic-sequential.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-tglb2/igt@gem_exec_nop@basic-sequential.html

  * igt@gem_exec_whisper@basic-fds-forked:
    - shard-glk:          [TIMEOUT][39] ([i915#1958]) -> [PASS][40] +2 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk1/igt@gem_exec_whisper@basic-fds-forked.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk1/igt@gem_exec_whisper@basic-fds-forked.html

  * igt@gem_exec_whisper@basic-fds-priority-all:
    - shard-glk:          [DMESG-WARN][41] ([i915#118] / [i915#95]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk9/igt@gem_exec_whisper@basic-fds-priority-all.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk3/igt@gem_exec_whisper@basic-fds-priority-all.html

  * igt@gem_exec_whisper@basic-queues:
    - shard-kbl:          [TIMEOUT][43] ([i915#1958]) -> [PASS][44] +4 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-kbl1/igt@gem_exec_whisper@basic-queues.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-kbl2/igt@gem_exec_whisper@basic-queues.html
    - shard-iclb:         [TIMEOUT][45] ([i915#1958]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-iclb7/igt@gem_exec_whisper@basic-queues.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-iclb2/igt@gem_exec_whisper@basic-queues.html

  * igt@gem_partial_pwrite_pread@reads-uncached:
    - shard-apl:          [FAIL][47] ([i915#1635]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-apl7/igt@gem_partial_pwrite_pread@reads-uncached.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-apl1/igt@gem_partial_pwrite_pread@reads-uncached.html

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-iclb:         [FAIL][49] ([i915#1515]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-iclb8/igt@i915_pm_rc6_residency@rc6-idle.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-iclb1/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-edp1:
    - shard-skl:          [INCOMPLETE][51] ([i915#198]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-skl1/igt@kms_flip@flip-vs-suspend-interruptible@a-edp1.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-skl4/igt@kms_flip@flip-vs-suspend-interruptible@a-edp1.html

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - shard-tglb:         [DMESG-WARN][53] ([i915#1982]) -> [PASS][54] +1 similar issue
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-tglb5/igt@kms_frontbuffer_tracking@fbc-stridechange.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-tglb5/igt@kms_frontbuffer_tracking@fbc-stridechange.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [FAIL][55] ([i915#1188]) -> [PASS][56] +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-skl4/igt@kms_hdr@bpc-switch-dpms.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-skl1/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-kbl:          [DMESG-WARN][57] ([i915#180]) -> [PASS][58] +2 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-kbl4/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-kbl1/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-skl:          [DMESG-WARN][59] ([i915#1982]) -> [PASS][60] +2 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-skl4/igt@kms_plane@plane-position-covered-pipe-c-planes.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-skl1/igt@kms_plane@plane-position-covered-pipe-c-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [FAIL][61] ([fdo#108145] / [i915#265]) -> [PASS][62] +1 similar issue
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-skl5/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-skl9/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [SKIP][63] ([fdo#109642] / [fdo#111068]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-iclb4/igt@kms_psr2_su@frontbuffer.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-iclb2/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_suspend:
    - shard-iclb:         [SKIP][65] ([fdo#109441]) -> [PASS][66] +1 similar issue
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-iclb4/igt@kms_psr@psr2_suspend.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-iclb2/igt@kms_psr@psr2_suspend.html

  * igt@kms_setmode@basic:
    - shard-glk:          [FAIL][67] ([i915#31]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk8/igt@kms_setmode@basic.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk9/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-b-wait-forked-busy-hang:
    - shard-apl:          [DMESG-WARN][69] ([i915#1635] / [i915#1982]) -> [PASS][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-apl4/igt@kms_vblank@pipe-b-wait-forked-busy-hang.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-apl6/igt@kms_vblank@pipe-b-wait-forked-busy-hang.html

  
#### Warnings ####

  * igt@gem_exec_whisper@basic-fds-all:
    - shard-glk:          [DMESG-WARN][71] ([i915#118] / [i915#95]) -> [TIMEOUT][72] ([i915#1958])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-glk6/igt@gem_exec_whisper@basic-fds-all.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-glk2/igt@gem_exec_whisper@basic-fds-all.html

  * igt@kms_content_protection@atomic-dpms:
    - shard-apl:          [TIMEOUT][73] ([i915#1319] / [i915#1635] / [i915#1958]) -> [FAIL][74] ([fdo#110321] / [fdo#110336] / [i915#1635])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-apl6/igt@kms_content_protection@atomic-dpms.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-apl8/igt@kms_content_protection@atomic-dpms.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][75] ([fdo#108145] / [i915#265]) -> [DMESG-WARN][76] ([i915#1982])
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8951/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
  [i915#1515]: https://gitlab.freedesktop.org/drm/intel/issues/1515
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2356]: https://gitlab.freedesktop.org/drm/intel/issues/2356
  [i915#2389]: https://gitlab.freedesktop.org/drm/intel/issues/2389
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_8951 -> Patchwork_18429

  CI-20190529: 20190529
  CI_DRM_8951: 5f8c53e35ef01a1d5e97db6005d3c308d3734bac @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5775: 58cb0aea18fa471af43c11ee9f587554721a7815 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18429: 7db2227d72fdf96d49b2fbb535732d0f0d442f87 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18429/index.html

[-- Attachment #1.2: Type: text/html, Size: 20127 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK

[Ville Syrjälä]

On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
> In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
> to separate function") the order of force_min_cdclk_changed check and
> intel_modeset_checks(), was reversed. This broke the mechanism to
> immediately force a new CDCLK minimum, and lead to driver probe
> errors for display audio on GLK platform with 5.9-rc1 kernel. Fix
> the issue by moving intel_modeset_checks() call later.
> 
> Fixes: 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking to separate function)"
> BugLink: https://github.com/thesofproject/linux/issues/2410
> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>

Amended the commit msg with a bit more details:
"[vsyrjala: It also broke the ability of planes to bump up the cdclk
and thus could lead to underruns when eg. flipping from 32bpp to
64bpp framebuffer. To be clear, we still compute the new cdclk
correctly but fail to actually program it to the hardware due to 
intel_set_cdclk_{pre,post}_plane_update() not getting called on 
account of state->modeset==false.]"

The "Fixes" line was also a bit wrong: )" vs ") at the end. Took
me a while to figure out what dim was complaining about :)

Pushed to dinq. Thanks!

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 7d50b7177d40..8caeed23037c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15009,12 +15009,6 @@ static int intel_atomic_check(struct drm_device *dev,
>  	if (dev_priv->wm.distrust_bios_wm)
>  		any_ms = true;
>  
> -	if (any_ms) {
> -		ret = intel_modeset_checks(state);
> -		if (ret)
> -			goto fail;
> -	}
> -
>  	intel_fbc_choose_crtc(dev_priv, state);
>  	ret = calc_watermark_data(state);
>  	if (ret)
> @@ -15029,6 +15023,10 @@ static int intel_atomic_check(struct drm_device *dev,
>  		goto fail;
>  
>  	if (any_ms) {
> +		ret = intel_modeset_checks(state);
> +		if (ret)
> +			goto fail;
> +
>  		ret = intel_modeset_calc_cdclk(state);
>  		if (ret)
>  			return ret;
> -- 
> 2.27.0

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

Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK

[Kai Vehmanen]

[-- Attachment #1: Type: text/plain, Size: 964 bytes --]

Hey,

On Thu, 3 Sep 2020, Ville Syrjälä wrote:

> On Tue, Sep 01, 2020 at 06:10:36PM +0300, Kai Vehmanen wrote:
>> In commit 4f0b4352bd26 ("drm/i915: Extract cdclk requirements checking
>> to separate function") the order of force_min_cdclk_changed check and
>> intel_modeset_checks(), was reversed. This broke the mechanism to
> 
> Amended the commit msg with a bit more details:
> "[vsyrjala: It also broke the ability of planes to bump up the cdclk
> and thus could lead to underruns when eg. flipping from 32bpp to
> 64bpp framebuffer. To be clear, we still compute the new cdclk

ack on that, thanks!

> The "Fixes" line was also a bit wrong: )" vs ") at the end. Took
> me a while to figure out what dim was complaining about :)
> 
> Pushed to dinq. Thanks!

Uh, sorry about that. I actually fixed that in some (?) version due to 
checkpatch warnings, but apparently it slipped though. Sorry about that, 
took me a while to figure it out, as well :)

Br, Kai

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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