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