[Intel-xe] [PATCH 6/9] drm/xe/pat: Keep track of relevant indexes
Lucas De Marchi
lucas.demarchi at intel.com
Tue Sep 26 16:55:41 UTC 2023
On Tue, Sep 26, 2023 at 08:34:18AM -0700, Matt Roper wrote:
>On Tue, Sep 26, 2023 at 09:43:51AM -0500, Lucas De Marchi wrote:
>> On Mon, Sep 25, 2023 at 04:14:43PM -0700, Matt Roper wrote:
>> > On Mon, Sep 25, 2023 at 03:10:46PM -0700, Lucas De Marchi wrote:
>> > > Some of the PAT entries are relevant for own driver use, which varies
>> > > per platform. Let the PAT early initialization set what they should
>> > > point to so the rest of the driver can use them where needed.
>> > >
>> > > Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> > > ---
>> > > drivers/gpu/drm/xe/xe_device_types.h | 2 ++
>> > > drivers/gpu/drm/xe/xe_pat.c | 9 +++++++++
>> > > drivers/gpu/drm/xe/xe_pt_types.h | 1 +
>> > > 3 files changed, 12 insertions(+)
>> > >
>> > > diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>> > > index 98835ee058f5..7d0f2109c23a 100644
>> > > --- a/drivers/gpu/drm/xe/xe_device_types.h
>> > > +++ b/drivers/gpu/drm/xe/xe_device_types.h
>> > > @@ -15,6 +15,7 @@
>> > > #include "xe_devcoredump_types.h"
>> > > #include "xe_gt_types.h"
>> > > #include "xe_platform_types.h"
>> > > +#include "xe_pt_types.h"
>> > > #include "xe_pmu.h"
>> > > #include "xe_step_types.h"
>> > >
>> > > @@ -342,6 +343,7 @@ struct xe_device {
>> > > const u32 *table;
>> > > /** Number of PAT entries */
>> > > int n_entries;
>> > > + u32 idx[__XE_CACHE_LEVEL_COUNT];
>> > > } pat;
>> > >
>> > > /** @d3cold: Encapsulate d3cold related stuff */
>> > > diff --git a/drivers/gpu/drm/xe/xe_pat.c b/drivers/gpu/drm/xe/xe_pat.c
>> > > index 86386633e206..966b63252735 100644
>> > > --- a/drivers/gpu/drm/xe/xe_pat.c
>> > > +++ b/drivers/gpu/drm/xe/xe_pat.c
>> > > @@ -108,14 +108,23 @@ void xe_pat_init_early(struct xe_device *xe)
>> > > xe->pat.ops = &mtl_pat_ops;
>> > > xe->pat.table = mtl_pat_table;
>> > > xe->pat.n_entries = ARRAY_SIZE(mtl_pat_table);
>> > > + xe->pat.idx[XE_CACHE_NONE] = 2;
>> > > + xe->pat.idx[XE_CACHE_WT] = 1;
>> > > + xe->pat.idx[XE_CACHE_WB] = 3;
>> > > } else if (xe->info.platform == XE_PVC || xe->info.platform == XE_DG2) {
>> > > xe->pat.ops = &pvc_pat_ops;
>> > > xe->pat.table = pvc_pat_table;
>> > > xe->pat.n_entries = ARRAY_SIZE(pvc_pat_table);
>> > > + xe->pat.idx[XE_CACHE_NONE] = 0;
>> > > + xe->pat.idx[XE_CACHE_WT] = 2;
>> > > + xe->pat.idx[XE_CACHE_WB] = 3;
>> >
>> > These are the PVC indices, but they're not correct for DG2. IIRC, DG2
>> > should be using the same ones as xe_lp below (although it looks like the
>> > bspec tagging might be messed up right now...)
>>
>> But that should imply a different table, otherwise the index doesn't
>> really make much sense. Looking at i915 we have, for DG2:
>>
>> drivers/gpu/drm/i915/i915_pci.c
>> #define TGL_CACHELEVEL \
>> .cachelevel_to_pat = { \
>> [I915_CACHE_NONE] = 0, \
>> [I915_CACHE_LLC] = 1, \
>> [I915_CACHE_L3_LLC] = 2, \
>> [I915_CACHE_WT] = 3, \
>> }
>
>Where (what branch) do you see that on? drm-tip has
>
> #define TGL_CACHELEVEL \
> .cachelevel_to_pat = { \
> [I915_CACHE_NONE] = 3, \
> [I915_CACHE_LLC] = 0, \
> [I915_CACHE_L3_LLC] = 0, \
> [I915_CACHE_WT] = 2, \
>
>which also matches the intel_gtt.c programming below.
>
>The table you pasted should be the LEGACY_CACHELEVEL identity table,
yes, I was just reading the wrong table here. With the right table at
hand it makes much more sense.
Lucas De Marchi
>used for platforms that don't really have/use PAT index.
>
>
>Matt
>
>>
>> drivers/gpu/drm/i915/gt/intel_gtt.c
>> intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(0), GEN8_PPAT_WB);
>> intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(1), GEN8_PPAT_WC);
>> intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(2), GEN8_PPAT_WT);
>> intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(3), GEN8_PPAT_UC);
>> intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(4), GEN8_PPAT_WB);
>> intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(5), GEN8_PPAT_WB);
>> intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(6), GEN8_PPAT_WB);
>> intel_gt_mcr_multicast_write_fw(gt, XEHP_PAT_INDEX(7), GEN8_PPAT_WB);
>>
>> So, it's effectively:
>> I915_CACHE_NONE -> GEN8_PPAT_WB
>> I915_CACHE_LLC -> GEN8_PPAT_WC
>> I915_CACHE_L3_LLC -> GEN8_PPAT_WT
>> I915_CACHE_WT -> GEN8_PPAT_UC
>>
>> Does this even make sense?
>>
>> For TGL it is:
>> I915_CACHE_NONE -> GEN8_PPAT_UC
>> I915_CACHE_LLC -> GEN8_PPAT_WB
>> I915_CACHE_L3_LLC -> GEN8_PPAT_WB
>> I915_CACHE_WT -> GEN8_PPAT_WT
>>
>> .... which seems better. /me confused, maybe I'm reading something wrong
>> here.
>>
>> Lucas De Marchi
>>
>> >
>> > Matt
>> >
>> > > } else if (GRAPHICS_VERx100(xe) <= 1210) {
>> > > xe->pat.ops = &tgl_pat_ops;
>> > > xe->pat.table = tgl_pat_table;
>> > > xe->pat.n_entries = ARRAY_SIZE(tgl_pat_table);
>> > > + xe->pat.idx[XE_CACHE_NONE] = 3;
>> > > + xe->pat.idx[XE_CACHE_WT] = 2;
>> > > + xe->pat.idx[XE_CACHE_WB] = 0;
>> > > } else {
>> > > /*
>> > > * Going forward we expect to need new PAT settings for most
>> > > diff --git a/drivers/gpu/drm/xe/xe_pt_types.h b/drivers/gpu/drm/xe/xe_pt_types.h
>> > > index 64e3921a0f46..bf5000499251 100644
>> > > --- a/drivers/gpu/drm/xe/xe_pt_types.h
>> > > +++ b/drivers/gpu/drm/xe/xe_pt_types.h
>> > > @@ -17,6 +17,7 @@ enum xe_cache_level {
>> > > XE_CACHE_NONE,
>> > > XE_CACHE_WT,
>> > > XE_CACHE_WB,
>> > > + __XE_CACHE_LEVEL_COUNT,
>> > > };
>> > >
>> > > #define XE_VM_MAX_LEVEL 4
>> > > --
>> > > 2.40.1
>> > >
>> >
>> > --
>> > Matt Roper
>> > Graphics Software Engineer
>> > Linux GPU Platform Enablement
>> > Intel Corporation
>
>--
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation
More information about the Intel-xe
mailing list