Re: [PATCH v4 2/3] drm/i915/xe3p_lpd: Enable display use of system cache for FBC

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

On Thu, 27 Nov 2025, "Govindapillai, Vinod" <vinod.govindapillai@intel.com> wrote:
> Hi,
>
> Ah.. :( Looks like I forgot to git add
> drivers/gpu/drm/i915/display/intel_display_driver.c while commit this
> patch. This patch now miss the mutex init part.

IMO the mutex init should be in intel_fbc_init() anyway.

BR,
Jani.

>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c
> b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index 7e000ba3e08b..4f82b267b086 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -184,6 +184,7 @@ void intel_display_driver_early_probe(struct
> intel_display *display)
>         mutex_init(&display->wm.wm_mutex);
>         mutex_init(&display->pps.mutex);
>         mutex_init(&display->hdcp.hdcp_mutex);
> +       mutex_init(&display->fbc.sys_cache.lock);
>  
>         intel_display_irq_init(display);
>         intel_dkl_phy_init(display);
>
> Will wait for comments before I update this commit with the above
> change. Sorry about that.
>
> BR
> Vinod
>
>
>
> On Thu, 2025-11-27 at 13:53 +0200, Vinod Govindapillai wrote:
>> One of the FBC instances can utilize the reserved area of SoC
>> level cache for the fbc transactions to benefit reduced memory
>> system power especially in idle scenarios. Reserved area of the
>> system cache can be assigned to an fbc instance by configuring
>> the cacheability configuration register with offset of the
>> compressed frame buffer in stolen memoty of that fbc. There is
>> a limit to this reserved area which is programmable and for
>> xe3p_lpd the limit is defined as 2MB.
>> 
>> v2: - better to track fbc sys cache usage from intel_display level,
>>       sanitize the cacheability config register on probe (Matt)
>>     - limit this for integrated graphics solutions, confirmed that
>>       no default value set for cache range by hw (Gustavo)
>> 
>> v3: - changes related to the use of fbc substruct in intel_display
>>     - use intel_de_write() instead of intel_rmw() by hardcoding the
>>       default value fields
>> 
>> v4: - protect sys cache config accesses, sys cache usage status in
>>       debugfs per fbc instance (Jani)
>> 
>> Bspec: 68881, 74722
>> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
>> ---
>>  .../gpu/drm/i915/display/intel_display_core.h |  6 ++
>>  .../drm/i915/display/intel_display_device.h   |  1 +
>>  drivers/gpu/drm/i915/display/intel_fbc.c      | 86
>> +++++++++++++++++++
>>  drivers/gpu/drm/i915/display/intel_fbc_regs.h | 10 +++
>>  4 files changed, 103 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h
>> b/drivers/gpu/drm/i915/display/intel_display_core.h
>> index 58325f530670..3e4bde7fa205 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
>> @@ -400,6 +400,12 @@ struct intel_display {
>>  
>>  	struct {
>>  		struct intel_fbc *instances[I915_MAX_FBCS];
>> +
>> +		/* xe3p_lpd+: FBC instance utilizing the system
>> cache */
>> +		struct sys_cache_cfg {
>> +			struct mutex lock;
>> +			enum intel_fbc_id id;
>> +		} sys_cache;
>>  	} fbc;
>>  
>>  	struct {
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
>> b/drivers/gpu/drm/i915/display/intel_display_device.h
>> index b559ef43d547..b74cb69ccc85 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
>> @@ -173,6 +173,7 @@ struct intel_display_platforms {
>>  #define HAS_DSC_MST(__display)		(DISPLAY_VER(__display) >=
>> 12 && HAS_DSC(__display))
>>  #define
>> HAS_FBC(__display)		(DISPLAY_RUNTIME_INFO(__display)->fbc_mask != 0)
>>  #define HAS_FBC_DIRTY_RECT(__display)	(DISPLAY_VER(__display) >=
>> 30)
>> +#define HAS_FBC_SYS_CACHE(__display)	(DISPLAY_VER(__display) >=
>> 35 && !(__display)->platform.dgfx)
>>  #define
>> HAS_FPGA_DBG_UNCLAIMED(__display)	(DISPLAY_INFO(__display)->has_fpga_dbg)
>>  #define HAS_FW_BLC(__display)		(DISPLAY_VER(__display) >=
>> 3)
>>  #define
>> HAS_GMBUS_BURST_READ(__display)	(DISPLAY_VER(__display) >= 10 || (__display)->platform.kabylake)
>> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
>> b/drivers/gpu/drm/i915/display/intel_fbc.c
>> index dcdfcff80de3..85978196b607 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
>> @@ -71,6 +71,8 @@
>>  	for_each_fbc_id((__display), (__fbc_id)) \
>>  		for_each_if((__fbc) = (__display)-
>> >fbc.instances[(__fbc_id)])
>>  
>> +#define FBC_SYS_CACHE_ID_NONE	I915_MAX_FBCS
>> +
>>  struct intel_fbc_funcs {
>>  	void (*activate)(struct intel_fbc *fbc);
>>  	void (*deactivate)(struct intel_fbc *fbc);
>> @@ -941,6 +943,69 @@ static void intel_fbc_program_workarounds(struct
>> intel_fbc *fbc)
>>  		fbc_compressor_clkgate_disable_wa(fbc, true);
>>  }
>>  
>> +static void fbc_sys_cache_update_config(struct intel_display
>> *display, u32 reg,
>> +					enum intel_fbc_id id)
>> +{
>> +	if (!HAS_FBC_SYS_CACHE(display))
>> +		return;
>> +
>> +	lockdep_assert_held(&display->fbc.sys_cache.lock);
>> +
>> +	/* Cache read enable is set by default */
>> +	reg |= FBC_SYS_CACHE_READ_ENABLE;
>> +
>> +	intel_de_write(display, XE3P_LPD_FBC_SYS_CACHE_USAGE_CFG,
>> reg);
>> +
>> +	display->fbc.sys_cache.id = id;
>> +}
>> +
>> +static void fbc_sys_cache_disable(const struct intel_fbc *fbc)
>> +{
>> +	struct intel_display *display = fbc->display;
>> +	struct sys_cache_cfg *sys_cache = &display->fbc.sys_cache;
>> +
>> +	mutex_lock(&sys_cache->lock);
>> +	/* clear only if "fbc" reserved the cache */
>> +	if (sys_cache->id == fbc->id)
>> +		fbc_sys_cache_update_config(display, 0,
>> FBC_SYS_CACHE_ID_NONE);
>> +	mutex_unlock(&sys_cache->lock);
>> +}
>> +
>> +static int fbc_sys_cache_limit(struct intel_display *display)
>> +{
>> +	/* Default 2MB for xe3p_lpd */
>> +	if (DISPLAY_VER(display) == 35)
>> +		return 2 * 1024 * 1024;
>> +
>> +	return 0;
>> +}
>> +
>> +static void fbc_sys_cache_enable(const struct intel_fbc *fbc)
>> +{
>> +	struct intel_display *display = fbc->display;
>> +	struct sys_cache_cfg *sys_cache = &display->fbc.sys_cache;
>> +	int range, offset;
>> +	u32 cfg;
>> +
>> +	if (!HAS_FBC_SYS_CACHE(display))
>> +		return;
>> +
>> +	/* limit to be configured to the register in 64k byte chunks
>> */
>> +	range = fbc_sys_cache_limit(display) / (64 * 1024);
>> +
>> +	/* offset to be configured to the register in 4K byte chunks
>> */
>> +	offset = i915_gem_stolen_node_offset(fbc->compressed_fb) /
>> (4 * 1024);
>> +
>> +	cfg = FBC_SYS_CACHE_TAG_USE_RES_SPACE |
>> FBC_SYS_CACHEABLE_RANGE(range) |
>> +	      FBC_SYS_CACHE_START_BASE(offset);
>> +
>> +	mutex_lock(&sys_cache->lock);
>> +	/* update sys cache config only if sys cache is unassigned
>> */
>> +	if (sys_cache->id == FBC_SYS_CACHE_ID_NONE)
>> +		fbc_sys_cache_update_config(display, cfg, fbc->id);
>> +	mutex_unlock(&sys_cache->lock);
>> +}
>> +
>>  static void __intel_fbc_cleanup_cfb(struct intel_fbc *fbc)
>>  {
>>  	if (WARN_ON(intel_fbc_hw_is_active(fbc)))
>> @@ -967,6 +1032,11 @@ void intel_fbc_cleanup(struct intel_display
>> *display)
>>  
>>  		kfree(fbc);
>>  	}
>> +
>> +	mutex_lock(&display->fbc.sys_cache.lock);
>> +	drm_WARN_ON(display->drm,
>> +		    display->fbc.sys_cache.id !=
>> FBC_SYS_CACHE_ID_NONE);
>> +	mutex_unlock(&display->fbc.sys_cache.lock);
>>  }
>>  
>>  static bool i8xx_fbc_stride_is_valid(const struct intel_plane_state
>> *plane_state)
>> @@ -1780,6 +1850,8 @@ static void __intel_fbc_disable(struct
>> intel_fbc *fbc)
>>  
>>  	__intel_fbc_cleanup_cfb(fbc);
>>  
>> +	fbc_sys_cache_disable(fbc);
>> +
>>  	/* wa_18038517565 Enable DPFC clock gating after FBC disable
>> */
>>  	if (display->platform.dg2 || DISPLAY_VER(display) >= 14)
>>  		fbc_compressor_clkgate_disable_wa(fbc, false);
>> @@ -1972,6 +2044,8 @@ static void __intel_fbc_enable(struct
>> intel_atomic_state *state,
>>  
>>  	intel_fbc_program_workarounds(fbc);
>>  	intel_fbc_program_cfb(fbc);
>> +
>> +	fbc_sys_cache_enable(fbc);
>>  }
>>  
>>  /**
>> @@ -2212,6 +2286,10 @@ void intel_fbc_init(struct intel_display
>> *display)
>>  
>>  	for_each_fbc_id(display, fbc_id)
>>  		display->fbc.instances[fbc_id] =
>> intel_fbc_create(display, fbc_id);
>> +
>> +	mutex_lock(&display->fbc.sys_cache.lock);
>> +	display->fbc.sys_cache.id = FBC_SYS_CACHE_ID_NONE;
>> +	mutex_unlock(&display->fbc.sys_cache.lock);
>>  }
>>  
>>  /**
>> @@ -2231,6 +2309,9 @@ void intel_fbc_sanitize(struct intel_display
>> *display)
>>  		if (intel_fbc_hw_is_active(fbc))
>>  			intel_fbc_hw_deactivate(fbc);
>>  	}
>> +
>> +	/* Ensure the sys cache usage config is clear as well */
>> +	fbc_sys_cache_update_config(display, 0,
>> FBC_SYS_CACHE_ID_NONE);
>>  }
>>  
>>  static int intel_fbc_debugfs_status_show(struct seq_file *m, void
>> *unused)
>> @@ -2249,6 +2330,11 @@ static int
>> intel_fbc_debugfs_status_show(struct seq_file *m, void *unused)
>>  		seq_puts(m, "FBC enabled\n");
>>  		seq_printf(m, "Compressing: %s\n",
>>  			  
>> str_yes_no(intel_fbc_is_compressing(fbc)));
>> +
>> +		mutex_lock(&display->fbc.sys_cache.lock);
>> +		seq_printf(m, "Using system cache: %s\n",
>> +			   str_yes_no(display->fbc.sys_cache.id ==
>> fbc->id));
>> +		mutex_unlock(&display->fbc.sys_cache.lock);
>>  	} else {
>>  		seq_printf(m, "FBC disabled: %s\n", fbc-
>> >no_fbc_reason);
>>  	}
>> diff --git a/drivers/gpu/drm/i915/display/intel_fbc_regs.h
>> b/drivers/gpu/drm/i915/display/intel_fbc_regs.h
>> index b1d0161a3196..d2d889fa4bed 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fbc_regs.h
>> +++ b/drivers/gpu/drm/i915/display/intel_fbc_regs.h
>> @@ -126,4 +126,14 @@
>>  #define   FBC_REND_NUKE			REG_BIT(2)
>>  #define   FBC_REND_CACHE_CLEAN		REG_BIT(1)
>>  
>> +#define XE3P_LPD_FBC_SYS_CACHE_USAGE_CFG	_MMIO(0x1344E0)
>> +#define  
>> FBC_SYS_CACHE_START_BASE_MASK		REG_GENMASK(31, 16)
>> +#define  
>> FBC_SYS_CACHE_START_BASE(base)	REG_FIELD_PREP(FBC_SYS_CACHE_START_BASE_MASK,(base))
>> +#define   FBC_SYS_CACHEABLE_RANGE_MASK		REG_GENMASK(15, 4)
>> +#define  
>> FBC_SYS_CACHEABLE_RANGE(range)	REG_FIELD_PREP(FBC_SYS_CACHEABLE_RANGE_MASK,(range))
>> +#define   FBC_SYS_CACHE_TAG_MASK		REG_GENMASK(3, 2)
>> +#define  
>> FBC_SYS_CACHE_TAG_DONT_CACHE		REG_FIELD_PREP(FBC_SYS_CACHE_TAG_MASK, 0)
>> +#define  
>> FBC_SYS_CACHE_TAG_USE_RES_SPACE	REG_FIELD_PREP(FBC_SYS_CACHE_TAG_MASK,3)
>> +#define   FBC_SYS_CACHE_READ_ENABLE		REG_BIT(0)
>> +
>>  #endif /* __INTEL_FBC_REGS__ */
>

-- 
Jani Nikula, Intel