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

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: fix regression leading to display audio probe failure on GLK
Date: Thu, 3 Sep 2020 16:47:55 +0300	[thread overview]
Message-ID: <20200903134755.GV6112@intel.com> (raw)
In-Reply-To: <20200901151036.1312357-1-kai.vehmanen@linux.intel.com>

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