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

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

On Tue, 25 Nov 2025, "Govindapillai, Vinod" <vinod.govindapillai@intel.com> wrote:
> On Mon, 2025-11-24 at 18:25 +0200, Jani Nikula wrote:
>> On Sun, 23 Nov 2025, Vinod Govindapillai
>> <vinod.govindapillai@intel.com> 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
>> 
>> Overall issue: The fbc mutexes are per fbc instance, but nothing
>> protects display->fbc.sys_cache_id.
>
> The places where this sys_cache_id can be changed to a valid fbc
> instance id + fbc cfb offset are protected by the fbc mutex as part of
> intel_fbc_enable and intel_fbc_disable. That's is what I was mentioning
> in my prev reply. And the places where this sys cache usage register
> reset happens is outside the fbc context - where sanitize and remove
> module gets called. I don't see a need to update the fbc.sys_cache_id
> from anywhere else.

That's not the point. Each FBC instance has its own mutex. Two FBC
instance mutexes could be held at the same time. I don't think this is
the case during enable/disable, though. But the point remains, the
instance mutex can't protect something that's not part of the instance.

BR,
Jani.


-- 
Jani Nikula, Intel