Re: [PATCH v3 1/2] drm/i915/display: Use a sub-struct for fbc operations in intel_display

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

On Sun, 23 Nov 2025, Vinod Govindapillai <vinod.govindapillai@intel.com> wrote:
> As FBC can utilze the system cache in xe3p_lpd onwards, we need

*utilize

> a way to track which fbc instance is utilizing this cache. So we
> would need to extend the intel_display with such a functionality.
> Introduce a new fbc substruct and move the current fbc instance
> array into that. Then the following patch can utilize this to
> introduce functionaity to configure and track the system cache
> usage by the fbc instance.

In general, please don't refer to "patches" in commit messages. Just
consider reading this commit message a year from now, wondering what
"the following patch" might refer to.

No need to resend for this.

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

> Suggested-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>  drivers/gpu/drm/i915/display/i9xx_plane.c          | 2 +-
>  drivers/gpu/drm/i915/display/intel_display_core.h  | 5 ++++-
>  drivers/gpu/drm/i915/display/intel_fbc.c           | 6 +++---
>  drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
>  4 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c
> index 51ccc6bd5f21..2c40bc632b3d 100644
> --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> @@ -134,7 +134,7 @@ static struct intel_fbc *i9xx_plane_fbc(struct intel_display *display,
>  					enum i9xx_plane_id i9xx_plane)
>  {
>  	if (i9xx_plane_has_fbc(display, i9xx_plane))
> -		return display->fbc[INTEL_FBC_A];
> +		return display->fbc.instances[INTEL_FBC_A];
>  	else
>  		return NULL;
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> index 9b36654b593d..58325f530670 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -398,6 +398,10 @@ struct intel_display {
>  		const struct dram_info *info;
>  	} dram;
>  
> +	struct {
> +		struct intel_fbc *instances[I915_MAX_FBCS];
> +	} fbc;
> +
>  	struct {
>  		/* list of fbdev register on this device */
>  		struct intel_fbdev *fbdev;
> @@ -615,7 +619,6 @@ struct intel_display {
>  	struct drm_dp_tunnel_mgr *dp_tunnel_mgr;
>  	struct intel_audio audio;
>  	struct intel_dpll_global dpll;
> -	struct intel_fbc *fbc[I915_MAX_FBCS];
>  	struct intel_frontbuffer_tracking fb_tracking;
>  	struct intel_hotplug hotplug;
>  	struct intel_opregion *opregion;
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index d9cab25d414a..dcdfcff80de3 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -69,7 +69,7 @@
>  
>  #define for_each_intel_fbc(__display, __fbc, __fbc_id) \
>  	for_each_fbc_id((__display), (__fbc_id)) \
> -		for_each_if((__fbc) = (__display)->fbc[(__fbc_id)])
> +		for_each_if((__fbc) = (__display)->fbc.instances[(__fbc_id)])
>  
>  struct intel_fbc_funcs {
>  	void (*activate)(struct intel_fbc *fbc);
> @@ -2211,7 +2211,7 @@ void intel_fbc_init(struct intel_display *display)
>  		    display->params.enable_fbc);
>  
>  	for_each_fbc_id(display, fbc_id)
> -		display->fbc[fbc_id] = intel_fbc_create(display, fbc_id);
> +		display->fbc.instances[fbc_id] = intel_fbc_create(display, fbc_id);
>  }
>  
>  /**
> @@ -2330,7 +2330,7 @@ void intel_fbc_debugfs_register(struct intel_display *display)
>  {
>  	struct intel_fbc *fbc;
>  
> -	fbc = display->fbc[INTEL_FBC_A];
> +	fbc = display->fbc.instances[INTEL_FBC_A];
>  	if (fbc)
>  		intel_fbc_debugfs_add(fbc, display->drm->debugfs_root);
>  }
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 89c8003ccfe7..48af74963e74 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -2444,7 +2444,7 @@ static struct intel_fbc *skl_plane_fbc(struct intel_display *display,
>  	enum intel_fbc_id fbc_id = skl_fbc_id_for_pipe(pipe);
>  
>  	if (skl_plane_has_fbc(display, fbc_id, plane_id))
> -		return display->fbc[fbc_id];
> +		return display->fbc.instances[fbc_id];
>  	else
>  		return NULL;
>  }

-- 
Jani Nikula, Intel