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. 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_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
