Hi Fei,
On Mon, May 08, 2023 at 04:48:54PM -0700, fei.yang@xxxxxxxxx wrote:
> From: Fei Yang <fei.yang@xxxxxxxxx>
>
> Currently the KMD is using enum i915_cache_level to set caching policy for
> buffer objects. This is flaky because the PAT index which really controls
> the caching behavior in PTE has far more levels than what's defined in the
> enum. In addition, the PAT index is platform dependent, having to translate
> between i915_cache_level and PAT index is not reliable, and makes the code
> more complicated.
>
> >From UMD's perspective there is also a necessity to set caching policy for
> performance fine tuning. It's much easier for the UMD to directly use PAT
> index because the behavior of each PAT index is clearly defined in Bspec.
> Having the abstracted i915_cache_level sitting in between would only cause
> more ambiguity. PAT is expected to work much like MOCS already works today,
> and by design userspace is expected to select the index that exactly
> matches the desired behavior described in the hardware specification.
>
> For these reasons this patch replaces i915_cache_level with PAT index. Also
> note, the cache_level is not completely removed yet, because the KMD still
> has the need of creating buffer objects with simple cache settings such as
> cached, uncached, or writethrough. For kernel objects, cache_level is used
> for simplicity and backward compatibility. For Pre-gen12 platforms PAT can
> have 1:1 mapping to i915_cache_level, so these two are interchangeable. see
> the use of LEGACY_CACHELEVEL.
>
> One consequence of this change is that gen8_pte_encode is no longer working
> for gen12 platforms due to the fact that gen12 platforms has different PAT
> definitions. In the meantime the mtl_pte_encode introduced specfically for
> MTL becomes generic for all gen12 platforms. This patch renames the MTL
> PTE encode function into gen12_pte_encode and apply it to all gen12. Even
> though this change looks unrelated, but separating them would temporarily
> break gen12 PTE encoding, thus squash them in one patch.
>
> Special note: this patch changes the way caching behavior is controlled in
> the sense that some objects are left to be managed by userspace. For such
> objects we need to be careful not to change the userspace settings.There
> are kerneldoc and comments added around obj->cache_coherent, cache_dirty,
> and how to bypass the checkings by i915_gem_object_has_cache_level. For
> full understanding, these changes need to be looked at together with the
> two follow-up patches, one disables the {set|get}_caching ioctl's and the
> other adds set_pat extension to the GEM_CREATE uAPI.
>
> Bspec: 63019
>
> Cc: Chris Wilson <chris.p.wilson@xxxxxxxxxxxxxxx>
> Signed-off-by: Fei Yang <fei.yang@xxxxxxxxx>
> Reviewed-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
> Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
>
> To be squashed
Ha! you forgot to remove this... I also do the same :)
No worries, if the patch is right I'll fix it before pushing it.
Tvrtko? Any opinion?
Andi
