[Intel-gfx] [PATCH v2 14/14] drm/i915/fbc: Reallocate cfb if we need more of it

[Maarten Lankhorst]

Op 27-11-2019 om 21:12 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> The code assumes we can omit the cfb allocation once fbc
> has been enabled once. That's nonsense. Let's try to
> reallocate it if we need to.
>
> The code is still a mess, but maybe this is enough to get
> fbc going in some cases where it initially underallocates
> the cfb and there's no full modeset to fix it up.
>
> Cc: Daniel Drake <drake at endlessm.com>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Jian-Hong Pan <jian-hong at endlessm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index c976698b0729..928059a5da80 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -672,6 +672,14 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
>  		cache->fence_id = -1;
>  }
>  
> +static bool intel_fbc_cfb_size_changed(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_fbc *fbc = &dev_priv->fbc;
> +
> +	return intel_fbc_calculate_cfb_size(dev_priv, &fbc->state_cache) >
> +		fbc->compressed_fb.size * fbc->threshold;
> +}
> +
>  static bool intel_fbc_can_activate(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -757,8 +765,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
>  	 * we didn't get any invalidate/deactivate calls, but this would require
>  	 * a lot of tracking just for a specific case. If we conclude it's an
>  	 * important case, we can implement it later. */
> -	if (intel_fbc_calculate_cfb_size(dev_priv, &fbc->state_cache) >
> -	    fbc->compressed_fb.size * fbc->threshold) {
> +	if (intel_fbc_cfb_size_changed(dev_priv)) {
>  		fbc->no_fbc_reason = "CFB requirements changed";
>  		return false;
>  	}
> @@ -1112,12 +1119,12 @@ void intel_fbc_enable(struct intel_crtc *crtc,
>  	mutex_lock(&fbc->lock);
>  
>  	if (fbc->crtc) {
> -		WARN_ON(fbc->crtc == crtc && !crtc_state->enable_fbc);
> -		goto out;
> -	}
> +		if (fbc->crtc != crtc ||
> +		    !intel_fbc_cfb_size_changed(dev_priv))
> +			goto out;
>  
> -	if (!crtc_state->enable_fbc)
> -		goto out;
> +		__intel_fbc_disable(dev_priv);
> +	}
>  
>  	WARN_ON(fbc->active);
>  
> @@ -1130,6 +1137,7 @@ void intel_fbc_enable(struct intel_crtc *crtc,
>  	if (intel_fbc_alloc_cfb(dev_priv,
>  				intel_fbc_calculate_cfb_size(dev_priv, cache),
>  				fb->format->cpp[0])) {
> +		cache->plane.visible = false;
>  		fbc->no_fbc_reason = "not enough stolen memory";
>  		goto out;
>  	}

Makes sense, unfortunately kms_cursor_legacy starts failing on this series. :(

For 1-11, 14

Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>

We should probably get rid of the FBC disable on frontbuffer disable as well. I had some patches but nothing upstream-worthy yet. :(

12  and 13 need more thought for now, kms_cursor_legacy is failing.