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

[Ville Syrjälä]

On Thu, Nov 28, 2019 at 04:48:04PM +0100, Maarten Lankhorst wrote:
> 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>

Entire series pushed with Maarten's irc r-b for the rest (thanks).

I also tracked down the lag I'm seeing on my laptop. It's caused by
the intel ddx never issuing dirtyfb ioctls because it fails to
correctly detect the capability. I'll post fixes for that shortly.
I suspect it was working better before we got WC mmap because then
the hw gtt tracking should have dealt with it. But with WC mmap
that no longer works and we actually depend on dirtyfb to flush
things.

-- 
Ville Syrjälä
Intel