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

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Tue, Jan 23, 2024 at 11:02:44AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@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@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 :)
So, up to you:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

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