[PATCH] drm/i915/fbc: Allow FBC with CCS modifiers on SKL+

[Ville Syrjälä]

On Tue, Jan 23, 2024 at 05:44:06PM -0500, Rodrigo Vivi wrote:
> On Tue, Jan 23, 2024 at 11:02:44AM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Only display workarounds 0391 and 0475 call for disabling
> > FBC with render compression, and those are listed only for
> > pre-prod SKL steppings. So it should be safe to enable
> > FB+CCS on production hardware.
> > 
> > AFAIK CCS is limited to 50% bandwidth reduction (perhaps
> > clear color can do better?). FBC can exceed that number
> > by quite a bit, given the right kind of framebuffer
> > contents. So piling on both kinds of compressions could
> > still make sense.
> 
> yeap, I think so.
> The risk is to hit a workaround that is not ducumented in the BSpec
> cases after gen11...
> 
> Uma, do you recall having seen lately any workaround with FBC
> and render compression?
> 
> > 
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10125
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_fbc.c | 13 +------------
> >  1 file changed, 1 insertion(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index f17a1afb4929..b453fcbd67da 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -1087,18 +1087,7 @@ static bool i8xx_fbc_tiling_valid(const struct intel_plane_state *plane_state)
> >  
> >  static bool skl_fbc_tiling_valid(const struct intel_plane_state *plane_state)
> >  {
> > -	const struct drm_framebuffer *fb = plane_state->hw.fb;
> > -
> > -	switch (fb->modifier) {
> > -	case DRM_FORMAT_MOD_LINEAR:
> > -	case I915_FORMAT_MOD_Y_TILED:
> > -	case I915_FORMAT_MOD_Yf_TILED:
> > -	case I915_FORMAT_MOD_4_TILED:
> > -	case I915_FORMAT_MOD_X_TILED:
> > -		return true;
> > -	default:
> > -		return false;
> > -	}
> > +	return true;
> 
> we could also simply kill this function... the compiler does the right thing,
> but users navigating on the code needs to do an extra ctag/cscope inspections
> to see that it is a simple return.  But well, the code do gets prettier with
> the function :)

I've been thinking of converting a bunch of this stuff to vfuncs,
so keeping the function around in anticipation of that seemed
semi-reasonable.

> So, up to you:
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>

Thanks.

> 
> >  }
> >  
> >  static bool tiling_is_valid(const struct intel_plane_state *plane_state)
> > -- 
> > 2.43.0
> > 

-- 
Ville Syrjälä
Intel