On Thu, 2025-11-27 at 15:49 +0200, Jani Nikula wrote: > On Thu, 27 Nov 2025, "Govindapillai, Vinod" > <[email protected]> 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.
Okay. Thanks Jani. I will change that. How about the rest of the changes. Are you ok with those? BR Vinod > > 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 > > > <[email protected]> > > > --- > > > .../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_STA > > > RT_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_C > > > ACHE_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__ */ > > >
