| Message ID | 20210729170008.2836648-13-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Begin enabling Xe_HP SDV and DG2 platforms | expand |
On Thu, Jul 29, 2021 at 10:00:02AM -0700, Matt Roper wrote: >From: Lucas De Marchi <lucas.demarchi@intel.com> > >Like DG1, XeHP SDV doesn't have LLC/eDRAM control values due to being a >dgfx card. XeHP SDV adds 2 more bits: L3_GLBGO to "push the Go point to >memory for L3 destined transaction" and L3_LKP to "enable Lookup for >uncacheable accesses". > >Bspec: 45101 >Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >Signed-off-by: Stuart Summers <stuart.summers@intel.com> >Signed-off-by: Matt Roper <matthew.d.roper@intel.com> +Ayaz, +Daniel I think this can't land as is since we risk forgetting additional changes that we will have to do. We already made the mistake once of forgetting MOCS changes. There are some patches to initialize unused MOCS entries and similar that should have been sent already to upstream. Ayaz, what's the state of those patches? Lucas De Marchi >--- > drivers/gpu/drm/i915/gt/intel_mocs.c | 33 +++++++++++++++++++++++++++- > 1 file changed, 32 insertions(+), 1 deletion(-) > >diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c >index 17848807f111..0c9d0b936c20 100644 >--- a/drivers/gpu/drm/i915/gt/intel_mocs.c >+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c >@@ -40,6 +40,8 @@ struct drm_i915_mocs_table { > #define L3_ESC(value) ((value) << 0) > #define L3_SCC(value) ((value) << 1) > #define _L3_CACHEABILITY(value) ((value) << 4) >+#define L3_GLBGO(value) ((value) << 6) >+#define L3_LKUP(value) ((value) << 7) > > /* Helper defines */ > #define GEN9_NUM_MOCS_ENTRIES 64 /* 63-64 are reserved, but configured. */ >@@ -314,6 +316,31 @@ static const struct drm_i915_mocs_entry dg1_mocs_table[] = { > MOCS_ENTRY(63, 0, L3_1_UC), > }; > >+static const struct drm_i915_mocs_entry xehpsdv_mocs_table[] = { >+ /* wa_1608975824 */ >+ MOCS_ENTRY(0, 0, L3_3_WB | L3_LKUP(1)), >+ >+ /* UC - Coherent; GO:L3 */ >+ MOCS_ENTRY(1, 0, L3_1_UC | L3_LKUP(1)), >+ /* UC - Coherent; GO:Memory */ >+ MOCS_ENTRY(2, 0, L3_1_UC | L3_GLBGO(1) | L3_LKUP(1)), >+ /* UC - Non-Coherent; GO:Memory */ >+ MOCS_ENTRY(3, 0, L3_1_UC | L3_GLBGO(1)), >+ /* UC - Non-Coherent; GO:L3 */ >+ MOCS_ENTRY(4, 0, L3_1_UC), >+ >+ /* WB */ >+ MOCS_ENTRY(5, 0, L3_3_WB | L3_LKUP(1)), >+ >+ /* HW Reserved - SW program but never use. */ >+ MOCS_ENTRY(48, 0, L3_3_WB | L3_LKUP(1)), >+ MOCS_ENTRY(49, 0, L3_1_UC | L3_LKUP(1)), >+ MOCS_ENTRY(60, 0, L3_1_UC), >+ MOCS_ENTRY(61, 0, L3_1_UC), >+ MOCS_ENTRY(62, 0, L3_1_UC), >+ MOCS_ENTRY(63, 0, L3_1_UC), >+}; >+ > enum { > HAS_GLOBAL_MOCS = BIT(0), > HAS_ENGINE_MOCS = BIT(1), >@@ -340,7 +367,11 @@ static unsigned int get_mocs_settings(const struct drm_i915_private *i915, > { > unsigned int flags; > >- if (IS_DG1(i915)) { >+ if (IS_XEHPSDV(i915)) { >+ table->size = ARRAY_SIZE(xehpsdv_mocs_table); >+ table->table = xehpsdv_mocs_table; >+ table->n_entries = GEN9_NUM_MOCS_ENTRIES; >+ } else if (IS_DG1(i915)) { > table->size = ARRAY_SIZE(dg1_mocs_table); > table->table = dg1_mocs_table; > table->n_entries = GEN9_NUM_MOCS_ENTRIES; >-- >2.25.4 > >_______________________________________________ >Intel-gfx mailing list >Intel-gfx@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -----Original Message----- > From: De Marchi, Lucas <lucas.demarchi@intel.com> > Sent: Thursday, July 29, 2021 11:01 PM > To: Roper, Matthew D <matthew.d.roper@intel.com> > Cc: intel-gfx@lists.freedesktop.org; Siddiqui, Ayaz A > <ayaz.siddiqui@intel.com>; Daniel Vetter <daniel.vetter@ffwll.ch> > Subject: Re: [Intel-gfx] [PATCH v4 12/18] drm/i915/xehpsdv: Define MOCS > table for XeHP SDV > > On Thu, Jul 29, 2021 at 10:00:02AM -0700, Matt Roper wrote: > >From: Lucas De Marchi <lucas.demarchi@intel.com> > > > >Like DG1, XeHP SDV doesn't have LLC/eDRAM control values due to being a > >dgfx card. XeHP SDV adds 2 more bits: L3_GLBGO to "push the Go point to > >memory for L3 destined transaction" and L3_LKP to "enable Lookup for > >uncacheable accesses". > > > >Bspec: 45101 > >Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > >Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > >Signed-off-by: Stuart Summers <stuart.summers@intel.com> > >Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > +Ayaz, +Daniel > > I think this can't land as is since we risk forgetting additional changes that we > will have to do. We already made the mistake once of forgetting MOCS > changes. > > There are some patches to initialize unused MOCS entries and similar that > should have been sent already to upstream. Ayaz, what's the state of those > patches? There are two section of patches . 1. Programming to CMD_CCTL for with UC this patch should sent for upstream. I'll share the patches for CMD_CCTL programming. 2. Programming of unused/unspecified MOCS index to WB, we observed some regression on media use cases. So we decided to delay upstreaming of those patches till MOCS programming are fixed in UMDs. Regards -Ayaz > > Lucas De Marchi > > >--- > > drivers/gpu/drm/i915/gt/intel_mocs.c | 33 > +++++++++++++++++++++++++++- > > 1 file changed, 32 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c > >b/drivers/gpu/drm/i915/gt/intel_mocs.c > >index 17848807f111..0c9d0b936c20 100644 > >--- a/drivers/gpu/drm/i915/gt/intel_mocs.c > >+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c > >@@ -40,6 +40,8 @@ struct drm_i915_mocs_table { > > #define L3_ESC(value) ((value) << 0) > > #define L3_SCC(value) ((value) << 1) > > #define _L3_CACHEABILITY(value) ((value) << 4) > >+#define L3_GLBGO(value) ((value) << 6) > >+#define L3_LKUP(value) ((value) << 7) > > > > /* Helper defines */ > > #define GEN9_NUM_MOCS_ENTRIES 64 /* 63-64 are reserved, but > configured. */ > >@@ -314,6 +316,31 @@ static const struct drm_i915_mocs_entry > dg1_mocs_table[] = { > > MOCS_ENTRY(63, 0, L3_1_UC), > > }; > > > >+static const struct drm_i915_mocs_entry xehpsdv_mocs_table[] = { > >+ /* wa_1608975824 */ > >+ MOCS_ENTRY(0, 0, L3_3_WB | L3_LKUP(1)), > >+ > >+ /* UC - Coherent; GO:L3 */ > >+ MOCS_ENTRY(1, 0, L3_1_UC | L3_LKUP(1)), > >+ /* UC - Coherent; GO:Memory */ > >+ MOCS_ENTRY(2, 0, L3_1_UC | L3_GLBGO(1) | L3_LKUP(1)), > >+ /* UC - Non-Coherent; GO:Memory */ > >+ MOCS_ENTRY(3, 0, L3_1_UC | L3_GLBGO(1)), > >+ /* UC - Non-Coherent; GO:L3 */ > >+ MOCS_ENTRY(4, 0, L3_1_UC), > >+ > >+ /* WB */ > >+ MOCS_ENTRY(5, 0, L3_3_WB | L3_LKUP(1)), > >+ > >+ /* HW Reserved - SW program but never use. */ > >+ MOCS_ENTRY(48, 0, L3_3_WB | L3_LKUP(1)), > >+ MOCS_ENTRY(49, 0, L3_1_UC | L3_LKUP(1)), > >+ MOCS_ENTRY(60, 0, L3_1_UC), > >+ MOCS_ENTRY(61, 0, L3_1_UC), > >+ MOCS_ENTRY(62, 0, L3_1_UC), > >+ MOCS_ENTRY(63, 0, L3_1_UC), > >+}; > >+ > > enum { > > HAS_GLOBAL_MOCS = BIT(0), > > HAS_ENGINE_MOCS = BIT(1), > >@@ -340,7 +367,11 @@ static unsigned int get_mocs_settings(const struct > >drm_i915_private *i915, { > > unsigned int flags; > > > >- if (IS_DG1(i915)) { > >+ if (IS_XEHPSDV(i915)) { > >+ table->size = ARRAY_SIZE(xehpsdv_mocs_table); > >+ table->table = xehpsdv_mocs_table; > >+ table->n_entries = GEN9_NUM_MOCS_ENTRIES; > >+ } else if (IS_DG1(i915)) { > > table->size = ARRAY_SIZE(dg1_mocs_table); > > table->table = dg1_mocs_table; > > table->n_entries = GEN9_NUM_MOCS_ENTRIES; > >-- > >2.25.4 > > > >_______________________________________________ > >Intel-gfx mailing list > >Intel-gfx@lists.freedesktop.org > >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Jul 30, 2021 at 12:16:14AM -0700, Siddiqui, Ayaz A wrote: > > > > -----Original Message----- > > From: De Marchi, Lucas <lucas.demarchi@intel.com> > > Sent: Thursday, July 29, 2021 11:01 PM > > To: Roper, Matthew D <matthew.d.roper@intel.com> > > Cc: intel-gfx@lists.freedesktop.org; Siddiqui, Ayaz A > > <ayaz.siddiqui@intel.com>; Daniel Vetter <daniel.vetter@ffwll.ch> > > Subject: Re: [Intel-gfx] [PATCH v4 12/18] drm/i915/xehpsdv: Define MOCS > > table for XeHP SDV > > > > On Thu, Jul 29, 2021 at 10:00:02AM -0700, Matt Roper wrote: > > >From: Lucas De Marchi <lucas.demarchi@intel.com> > > > > > >Like DG1, XeHP SDV doesn't have LLC/eDRAM control values due to being a > > >dgfx card. XeHP SDV adds 2 more bits: L3_GLBGO to "push the Go point to > > >memory for L3 destined transaction" and L3_LKP to "enable Lookup for > > >uncacheable accesses". > > > > > >Bspec: 45101 > > >Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > >Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > >Signed-off-by: Stuart Summers <stuart.summers@intel.com> > > >Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > > > +Ayaz, +Daniel > > > > I think this can't land as is since we risk forgetting additional changes that we > > will have to do. We already made the mistake once of forgetting MOCS > > changes. > > > > There are some patches to initialize unused MOCS entries and similar that > > should have been sent already to upstream. Ayaz, what's the state of those > > patches? > There are two section of patches . > 1. Programming to CMD_CCTL for with UC this patch should sent for upstream. > I'll share the patches for CMD_CCTL programming. > 2. Programming of unused/unspecified MOCS index to WB, we observed some > regression on media use cases. So we decided to delay upstreaming of > those patches till MOCS programming are fixed in UMDs. I don't follow on #2. At the moment we don't have a complete XeHP SDV driver upstream (the device IDs aren't even there and won't be for a while) so there's nothing that can regress yet from an upstream point of view --- you can't run a UMD at all today. If internal builds of the UMDs run against our internal staging trees are still buggy and won't work properly with the proper MOCS table settings, then that just makes it more important to upstream these patches now so that the UMD's don't come to rely on the incorrect table values. If we wait until later after the driver is "complete" and we've already enabled first contact of the UMDs in general, then we won't be able to update the MOCS programming to the proper values anymore without breaking ABI. Unless you're talking about regressing older platforms? In that case we should be able to restrict the WB programming to just Xe_HP SDV for now; ultimately we'll probably need to figure out some kind of opt-in mechanism for the older platforms, but that can be separate from doing it unconditionally on Xe_HP SDV and beyond I think. Matt > Regards > -Ayaz > > > > > Lucas De Marchi > > > > >--- > > > drivers/gpu/drm/i915/gt/intel_mocs.c | 33 > > +++++++++++++++++++++++++++- > > > 1 file changed, 32 insertions(+), 1 deletion(-) > > > > > >diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c > > >b/drivers/gpu/drm/i915/gt/intel_mocs.c > > >index 17848807f111..0c9d0b936c20 100644 > > >--- a/drivers/gpu/drm/i915/gt/intel_mocs.c > > >+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c > > >@@ -40,6 +40,8 @@ struct drm_i915_mocs_table { > > > #define L3_ESC(value) ((value) << 0) > > > #define L3_SCC(value) ((value) << 1) > > > #define _L3_CACHEABILITY(value) ((value) << 4) > > >+#define L3_GLBGO(value) ((value) << 6) > > >+#define L3_LKUP(value) ((value) << 7) > > > > > > /* Helper defines */ > > > #define GEN9_NUM_MOCS_ENTRIES 64 /* 63-64 are reserved, but > > configured. */ > > >@@ -314,6 +316,31 @@ static const struct drm_i915_mocs_entry > > dg1_mocs_table[] = { > > > MOCS_ENTRY(63, 0, L3_1_UC), > > > }; > > > > > >+static const struct drm_i915_mocs_entry xehpsdv_mocs_table[] = { > > >+ /* wa_1608975824 */ > > >+ MOCS_ENTRY(0, 0, L3_3_WB | L3_LKUP(1)), > > >+ > > >+ /* UC - Coherent; GO:L3 */ > > >+ MOCS_ENTRY(1, 0, L3_1_UC | L3_LKUP(1)), > > >+ /* UC - Coherent; GO:Memory */ > > >+ MOCS_ENTRY(2, 0, L3_1_UC | L3_GLBGO(1) | L3_LKUP(1)), > > >+ /* UC - Non-Coherent; GO:Memory */ > > >+ MOCS_ENTRY(3, 0, L3_1_UC | L3_GLBGO(1)), > > >+ /* UC - Non-Coherent; GO:L3 */ > > >+ MOCS_ENTRY(4, 0, L3_1_UC), > > >+ > > >+ /* WB */ > > >+ MOCS_ENTRY(5, 0, L3_3_WB | L3_LKUP(1)), > > >+ > > >+ /* HW Reserved - SW program but never use. */ > > >+ MOCS_ENTRY(48, 0, L3_3_WB | L3_LKUP(1)), > > >+ MOCS_ENTRY(49, 0, L3_1_UC | L3_LKUP(1)), > > >+ MOCS_ENTRY(60, 0, L3_1_UC), > > >+ MOCS_ENTRY(61, 0, L3_1_UC), > > >+ MOCS_ENTRY(62, 0, L3_1_UC), > > >+ MOCS_ENTRY(63, 0, L3_1_UC), > > >+}; > > >+ > > > enum { > > > HAS_GLOBAL_MOCS = BIT(0), > > > HAS_ENGINE_MOCS = BIT(1), > > >@@ -340,7 +367,11 @@ static unsigned int get_mocs_settings(const struct > > >drm_i915_private *i915, { > > > unsigned int flags; > > > > > >- if (IS_DG1(i915)) { > > >+ if (IS_XEHPSDV(i915)) { > > >+ table->size = ARRAY_SIZE(xehpsdv_mocs_table); > > >+ table->table = xehpsdv_mocs_table; > > >+ table->n_entries = GEN9_NUM_MOCS_ENTRIES; > > >+ } else if (IS_DG1(i915)) { > > > table->size = ARRAY_SIZE(dg1_mocs_table); > > > table->table = dg1_mocs_table; > > > table->n_entries = GEN9_NUM_MOCS_ENTRIES; > > >-- > > >2.25.4 > > > > > >_______________________________________________ > > >Intel-gfx mailing list > > >Intel-gfx@lists.freedesktop.org > > >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c index 17848807f111..0c9d0b936c20 100644 --- a/drivers/gpu/drm/i915/gt/intel_mocs.c +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c @@ -40,6 +40,8 @@ struct drm_i915_mocs_table { #define L3_ESC(value) ((value) << 0) #define L3_SCC(value) ((value) << 1) #define _L3_CACHEABILITY(value) ((value) << 4) +#define L3_GLBGO(value) ((value) << 6) +#define L3_LKUP(value) ((value) << 7) /* Helper defines */ #define GEN9_NUM_MOCS_ENTRIES 64 /* 63-64 are reserved, but configured. */ @@ -314,6 +316,31 @@ static const struct drm_i915_mocs_entry dg1_mocs_table[] = { MOCS_ENTRY(63, 0, L3_1_UC), }; +static const struct drm_i915_mocs_entry xehpsdv_mocs_table[] = { + /* wa_1608975824 */ + MOCS_ENTRY(0, 0, L3_3_WB | L3_LKUP(1)), + + /* UC - Coherent; GO:L3 */ + MOCS_ENTRY(1, 0, L3_1_UC | L3_LKUP(1)), + /* UC - Coherent; GO:Memory */ + MOCS_ENTRY(2, 0, L3_1_UC | L3_GLBGO(1) | L3_LKUP(1)), + /* UC - Non-Coherent; GO:Memory */ + MOCS_ENTRY(3, 0, L3_1_UC | L3_GLBGO(1)), + /* UC - Non-Coherent; GO:L3 */ + MOCS_ENTRY(4, 0, L3_1_UC), + + /* WB */ + MOCS_ENTRY(5, 0, L3_3_WB | L3_LKUP(1)), + + /* HW Reserved - SW program but never use. */ + MOCS_ENTRY(48, 0, L3_3_WB | L3_LKUP(1)), + MOCS_ENTRY(49, 0, L3_1_UC | L3_LKUP(1)), + MOCS_ENTRY(60, 0, L3_1_UC), + MOCS_ENTRY(61, 0, L3_1_UC), + MOCS_ENTRY(62, 0, L3_1_UC), + MOCS_ENTRY(63, 0, L3_1_UC), +}; + enum { HAS_GLOBAL_MOCS = BIT(0), HAS_ENGINE_MOCS = BIT(1), @@ -340,7 +367,11 @@ static unsigned int get_mocs_settings(const struct drm_i915_private *i915, { unsigned int flags; - if (IS_DG1(i915)) { + if (IS_XEHPSDV(i915)) { + table->size = ARRAY_SIZE(xehpsdv_mocs_table); + table->table = xehpsdv_mocs_table; + table->n_entries = GEN9_NUM_MOCS_ENTRIES; + } else if (IS_DG1(i915)) { table->size = ARRAY_SIZE(dg1_mocs_table); table->table = dg1_mocs_table; table->n_entries = GEN9_NUM_MOCS_ENTRIES;