Re: [Intel-gfx] [PATCH 1/7] drm/i915/fbc: Eliminate possible_framebuffer_bits

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 15 Mar 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Not sure what the point of this fbc->possible_frontbuffer_bits is.
> And especially don't see why it's returning all the bits when
> fbc is not even enabled. So let's just get rid of this and only
> say we are interested in the plane's frontbuffer bits when fbc
> is actually enabled.

So, uh, why are we calling this "bits", all over the place? It doesn't
tell anything to a casual reader. It's been percolated to fbc, psr,
plane, frontbuffer...

It's a kind of slot or id mask? I think the naming should reflect that,
and it would be easier to read. For future cleanup.

For the patch at hand, beats me,

Acked-by: Jani Nikula <jani.nikula@intel.com>

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> ---
>drivers/gpu/drm/i915/display/intel_fbc.c | 7 +------ 1 file changed, 1
>insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 142280b6ce6d..e45d64655090 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -89,7 +89,6 @@ struct intel_fbc {
>  	 * with stolen_lock.
>  	 */
>  	struct mutex lock;
> -	unsigned int possible_framebuffer_bits;
>  	unsigned int busy_bits;
>  
>  	struct drm_mm_node compressed_fb;
> @@ -1316,7 +1315,7 @@ static unsigned int intel_fbc_get_frontbuffer_bit(struct intel_fbc *fbc)
>  	if (fbc->state.plane)
>  		return fbc->state.plane->frontbuffer_bit;
>  	else
> -		return fbc->possible_framebuffer_bits;
> +		return 0;
>  }
>  
>  static void __intel_fbc_invalidate(struct intel_fbc *fbc,
> @@ -1655,11 +1654,7 @@ static bool need_fbc_vtd_wa(struct drm_i915_private *i915)
>  
>  void intel_fbc_add_plane(struct intel_fbc *fbc, struct intel_plane *plane)
>  {
> -	if (!fbc)
> -		return;
> -
>  	plane->fbc = fbc;
> -	fbc->possible_framebuffer_bits |= plane->frontbuffer_bit;
>  }
>  
>  static struct intel_fbc *intel_fbc_create(struct drm_i915_private *i915,

-- 
Jani Nikula, Intel Open Source Graphics Center