Re: [Intel-gfx] [PATCH 18/22] drm/i915/tgl: Define MOCS entries for Tigerlake

2019-07-25 Thread Lis, Tomasz



On 2019-07-25 00:32, Lucas De Marchi wrote:

On Thu, Jul 18, 2019 at 10:09:27AM -0700, Daniele Ceraolo Spurio wrote:



On 7/18/19 6:08 AM, Ville Syrjälä wrote:

On Fri, Jul 12, 2019 at 06:09:36PM -0700, Lucas De Marchi wrote:

From: Tomasz Lis 

The MOCS table is published as part of bspec, and versioned. Entries
are supposed to never be modified, but new ones can be added. Adding
entries increases table version. The patch includes version 1 entries.

Two of the 3 legacy entries used for gen9 are no longer expected to 
work.
Although we are changing the gen11 table, those changes are 
supposed to

be backward compatible since we are only touching previously undefined
entries.

Cc: Joonas Lahtinen 
Cc: Mika Kuoppala 
Cc: Daniele Ceraolo Spurio 
Signed-off-by: Tomasz Lis 
Signed-off-by: Lucas De Marchi 
---
 drivers/gpu/drm/i915/gt/intel_mocs.c | 25 ++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c 
b/drivers/gpu/drm/i915/gt/intel_mocs.c

index 290a5e9b90b9..259e7bec0a63 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
@@ -62,6 +62,10 @@ struct drm_i915_mocs_table {
 #define GEN11_NUM_MOCS_ENTRIES    64  /* 63-64 are reserved, but 
configured. */

 /* (e)LLC caching options */
+/*
+ * Note: LE_0_PAGETABLE works only up to Gen11; for newer gens it 
means

+ * the same as LE_UC
+ */
 #define LE_0_PAGETABLE    _LE_CACHEABILITY(0)
 #define LE_1_UC    _LE_CACHEABILITY(1)
 #define LE_2_WT    _LE_CACHEABILITY(2)
@@ -100,8 +104,9 @@ struct drm_i915_mocs_table {
  * of bspec.
  *
  * Entries not part of the following tables are undefined as far as
- * userspace is concerned and shouldn't be relied upon. For the time
- * being they will be initialized to PTE.
+ * userspace is concerned and shouldn't be relied upon. For Gen < 12
+ * they will be initialized to PTE. Gen >= 12 onwards don't have a 
setting for

+ * PTE. We use the same value, but that actually means Uncached.
  *
  * The last two entries are reserved by the hardware. For ICL+ they
  * should be initialized according to bspec and never used, for older
@@ -137,11 +142,13 @@ static const struct drm_i915_mocs_entry 
broxton_mocs_table[] = {

 };
 #define GEN11_MOCS_ENTRIES \
-    /* Base - Uncached (Deprecated) */ \
+    /* Gen11: Base - Uncached (Deprecated) */ \
+    /* Gen12+: Base - Error (Reserved for Non-Use) */ \
 MOCS_ENTRY(I915_MOCS_UNCACHED, \
    LE_1_UC | LE_TC_1_LLC, \
    L3_1_UC), \
 /* Base - L3 + LeCC:PAT (Deprecated) */ \
+    /* Gen12+: Base - Reserved */ \
 MOCS_ENTRY(I915_MOCS_PTE, \
    LE_0_PAGETABLE | LE_TC_1_LLC, \
    L3_3_WB), \
@@ -233,6 +240,18 @@ static const struct drm_i915_mocs_entry 
broxton_mocs_table[] = {

 MOCS_ENTRY(23, \
    LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_RSC(1) | 
LE_SCC(7), \

    L3_3_WB), \
+    /* Gen12+: HW Reserved - HDC:L1 + L3 + LLC */ \


Why is this marked as reserved? From the looks of things 48-61 should
just be normal entries that userspace can select to get HDC L1$. And
looks like icl already has that stuff. So someone should probably 
figure

out if Mesa/etc. can make use of the HDC L1$, and if so we should add
the relevant MOCS entries for icl as well.


Here the reserved terminology is indeed misleading. The 48-59 range 
is a "special" range where L1 usage is implicitly enabled by the HW, 
as there is no explicit L1 toggle in the MOCS registers. The reserved 
here means that the range shouldn't be used for "normal" MOCS 
settings, but SW can freely use these entries as needed. Similarly, 
MOCS 60 and 61 are reserved for other special purposes, but are still 
usable by SW. The only entries SW shouldn't touch are 62 and 63.


Regarding ICL, Gen11 HW doesn't have the capability so no new entries 
are required there.
It might be a good idea to find a better word for it than "reserved". 
Not only for this patch, but to be used everywhere, especially the spec.

But that exceeds the scope of this review.





+    MOCS_ENTRY(48, \
+   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+   L3_3_WB), \
+    /* Gen12+: HW Reserved - HW Special Case (CCS) */ \


The specs have MOCS 49-51 defined as well.


humn... it seems they got added later.

I'm not sure anymore if we should update igt so it doesn't expect those
entries to be set to PTE or if we should stop reusing the same table for
ICL and TGL. Spec doesn't mention the compatibility of this table with
gen 11 anymore. Thoughts?



It doesn't sound right to change the implementation decision to keep them
together, only because this allows to keep tests intact.

I don't believe there's a reason to verify undefined entries in IGT.
Sometimes we do want to verify our specific implementation instead of
pure specs compliance; but I don't see a reason for this should be the 
case here.


The existing MOCS entries are supposed to be unchangeable (in 

Re: [Intel-gfx] [PATCH 18/22] drm/i915/tgl: Define MOCS entries for Tigerlake

2019-07-24 Thread Lucas De Marchi

On Thu, Jul 18, 2019 at 10:09:27AM -0700, Daniele Ceraolo Spurio wrote:



On 7/18/19 6:08 AM, Ville Syrjälä wrote:

On Fri, Jul 12, 2019 at 06:09:36PM -0700, Lucas De Marchi wrote:

From: Tomasz Lis 

The MOCS table is published as part of bspec, and versioned. Entries
are supposed to never be modified, but new ones can be added. Adding
entries increases table version. The patch includes version 1 entries.

Two of the 3 legacy entries used for gen9 are no longer expected to work.
Although we are changing the gen11 table, those changes are supposed to
be backward compatible since we are only touching previously undefined
entries.

Cc: Joonas Lahtinen 
Cc: Mika Kuoppala 
Cc: Daniele Ceraolo Spurio 
Signed-off-by: Tomasz Lis 
Signed-off-by: Lucas De Marchi 
---
 drivers/gpu/drm/i915/gt/intel_mocs.c | 25 ++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c 
b/drivers/gpu/drm/i915/gt/intel_mocs.c
index 290a5e9b90b9..259e7bec0a63 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
@@ -62,6 +62,10 @@ struct drm_i915_mocs_table {
 #define GEN11_NUM_MOCS_ENTRIES 64  /* 63-64 are reserved, but configured. */
 /* (e)LLC caching options */
+/*
+ * Note: LE_0_PAGETABLE works only up to Gen11; for newer gens it means
+ * the same as LE_UC
+ */
 #define LE_0_PAGETABLE _LE_CACHEABILITY(0)
 #define LE_1_UC_LE_CACHEABILITY(1)
 #define LE_2_WT_LE_CACHEABILITY(2)
@@ -100,8 +104,9 @@ struct drm_i915_mocs_table {
  * of bspec.
  *
  * Entries not part of the following tables are undefined as far as
- * userspace is concerned and shouldn't be relied upon.  For the time
- * being they will be initialized to PTE.
+ * userspace is concerned and shouldn't be relied upon.  For Gen < 12
+ * they will be initialized to PTE. Gen >= 12 onwards don't have a setting for
+ * PTE. We use the same value, but that actually means Uncached.
  *
  * The last two entries are reserved by the hardware. For ICL+ they
  * should be initialized according to bspec and never used, for older
@@ -137,11 +142,13 @@ static const struct drm_i915_mocs_entry 
broxton_mocs_table[] = {
 };
 #define GEN11_MOCS_ENTRIES \
-   /* Base - Uncached (Deprecated) */ \
+   /* Gen11: Base - Uncached (Deprecated) */ \
+   /* Gen12+: Base - Error (Reserved for Non-Use) */ \
MOCS_ENTRY(I915_MOCS_UNCACHED, \
   LE_1_UC | LE_TC_1_LLC, \
   L3_1_UC), \
/* Base - L3 + LeCC:PAT (Deprecated) */ \
+   /* Gen12+: Base - Reserved */ \
MOCS_ENTRY(I915_MOCS_PTE, \
   LE_0_PAGETABLE | LE_TC_1_LLC, \
   L3_3_WB), \
@@ -233,6 +240,18 @@ static const struct drm_i915_mocs_entry 
broxton_mocs_table[] = {
MOCS_ENTRY(23, \
   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_RSC(1) | LE_SCC(7), \
   L3_3_WB), \
+   /* Gen12+: HW Reserved - HDC:L1 + L3 + LLC */ \


Why is this marked as reserved? From the looks of things 48-61 should
just be normal entries that userspace can select to get HDC L1$. And
looks like icl already has that stuff. So someone should probably figure
out if Mesa/etc. can make use of the HDC L1$, and if so we should add
the relevant MOCS entries for icl as well.


Here the reserved terminology is indeed misleading. The 48-59 range is 
a "special" range where L1 usage is implicitly enabled by the HW, as 
there is no explicit L1 toggle in the MOCS registers. The reserved 
here means that the range shouldn't be used for "normal" MOCS 
settings, but SW can freely use these entries as needed. Similarly, 
MOCS 60 and 61 are reserved for other special purposes, but are still 
usable by SW. The only entries SW shouldn't touch are 62 and 63.


Regarding ICL, Gen11 HW doesn't have the capability so no new entries 
are required there.





+   MOCS_ENTRY(48, \
+  LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+  L3_3_WB), \
+   /* Gen12+: HW Reserved - HW Special Case (CCS) */ \


The specs have MOCS 49-51 defined as well.


humn... it seems they got added later.

I'm not sure anymore if we should update igt so it doesn't expect those
entries to be set to PTE or if we should stop reusing the same table for
ICL and TGL. Spec doesn't mention the compatibility of this table with
gen 11 anymore. Thoughts?


Lucas De Marchi



Daniele


+   MOCS_ENTRY(60, \
+  LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+  L3_1_UC), \
+   /* Gen12+: HW Reserved - HW Special Case (Displayable) */ \
+   MOCS_ENTRY(61, \
+  LE_1_UC | LE_TC_1_LLC | LE_SCF(1), \
+  L3_3_WB), \
/* HW Reserved - SW program but never use */ \
MOCS_ENTRY(62, \
   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
--
2.21.0



___
Intel-gfx mailing list
Intel-gfx@list

Re: [Intel-gfx] [PATCH 18/22] drm/i915/tgl: Define MOCS entries for Tigerlake

2019-07-18 Thread Ville Syrjälä
On Thu, Jul 18, 2019 at 10:09:27AM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 7/18/19 6:08 AM, Ville Syrjälä wrote:
> > On Fri, Jul 12, 2019 at 06:09:36PM -0700, Lucas De Marchi wrote:
> >> From: Tomasz Lis 
> >>
> >> The MOCS table is published as part of bspec, and versioned. Entries
> >> are supposed to never be modified, but new ones can be added. Adding
> >> entries increases table version. The patch includes version 1 entries.
> >>
> >> Two of the 3 legacy entries used for gen9 are no longer expected to work.
> >> Although we are changing the gen11 table, those changes are supposed to
> >> be backward compatible since we are only touching previously undefined
> >> entries.
> >>
> >> Cc: Joonas Lahtinen 
> >> Cc: Mika Kuoppala 
> >> Cc: Daniele Ceraolo Spurio 
> >> Signed-off-by: Tomasz Lis 
> >> Signed-off-by: Lucas De Marchi 
> >> ---
> >>   drivers/gpu/drm/i915/gt/intel_mocs.c | 25 ++---
> >>   1 file changed, 22 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c 
> >> b/drivers/gpu/drm/i915/gt/intel_mocs.c
> >> index 290a5e9b90b9..259e7bec0a63 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> >> @@ -62,6 +62,10 @@ struct drm_i915_mocs_table {
> >>   #define GEN11_NUM_MOCS_ENTRIES   64  /* 63-64 are reserved, but 
> >> configured. */
> >>   
> >>   /* (e)LLC caching options */
> >> +/*
> >> + * Note: LE_0_PAGETABLE works only up to Gen11; for newer gens it means
> >> + * the same as LE_UC
> >> + */
> >>   #define LE_0_PAGETABLE   _LE_CACHEABILITY(0)
> >>   #define LE_1_UC  _LE_CACHEABILITY(1)
> >>   #define LE_2_WT  _LE_CACHEABILITY(2)
> >> @@ -100,8 +104,9 @@ struct drm_i915_mocs_table {
> >>* of bspec.
> >>*
> >>* Entries not part of the following tables are undefined as far as
> >> - * userspace is concerned and shouldn't be relied upon.  For the time
> >> - * being they will be initialized to PTE.
> >> + * userspace is concerned and shouldn't be relied upon.  For Gen < 12
> >> + * they will be initialized to PTE. Gen >= 12 onwards don't have a 
> >> setting for
> >> + * PTE. We use the same value, but that actually means Uncached.
> >>*
> >>* The last two entries are reserved by the hardware. For ICL+ they
> >>* should be initialized according to bspec and never used, for older
> >> @@ -137,11 +142,13 @@ static const struct drm_i915_mocs_entry 
> >> broxton_mocs_table[] = {
> >>   };
> >>   
> >>   #define GEN11_MOCS_ENTRIES \
> >> -  /* Base - Uncached (Deprecated) */ \
> >> +  /* Gen11: Base - Uncached (Deprecated) */ \
> >> +  /* Gen12+: Base - Error (Reserved for Non-Use) */ \
> >>MOCS_ENTRY(I915_MOCS_UNCACHED, \
> >>   LE_1_UC | LE_TC_1_LLC, \
> >>   L3_1_UC), \
> >>/* Base - L3 + LeCC:PAT (Deprecated) */ \
> >> +  /* Gen12+: Base - Reserved */ \
> >>MOCS_ENTRY(I915_MOCS_PTE, \
> >>   LE_0_PAGETABLE | LE_TC_1_LLC, \
> >>   L3_3_WB), \
> >> @@ -233,6 +240,18 @@ static const struct drm_i915_mocs_entry 
> >> broxton_mocs_table[] = {
> >>MOCS_ENTRY(23, \
> >>   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_RSC(1) | LE_SCC(7), \
> >>   L3_3_WB), \
> >> +  /* Gen12+: HW Reserved - HDC:L1 + L3 + LLC */ \
> > 
> > Why is this marked as reserved? From the looks of things 48-61 should
> > just be normal entries that userspace can select to get HDC L1$. And
> > looks like icl already has that stuff. So someone should probably figure
> > out if Mesa/etc. can make use of the HDC L1$, and if so we should add
> > the relevant MOCS entries for icl as well.
> 
> Here the reserved terminology is indeed misleading. The 48-59 range is a 
> "special" range where L1 usage is implicitly enabled by the HW, as there 
> is no explicit L1 toggle in the MOCS registers. The reserved here means 
> that the range shouldn't be used for "normal" MOCS settings, but SW can 
> freely use these entries as needed. Similarly, MOCS 60 and 61 are 
> reserved for other special purposes, but are still usable by SW. The 
> only entries SW shouldn't touch are 62 and 63.
> 
> Regarding ICL, Gen11 HW doesn't have the capability so no new entries 
> are required there.

Hmm. The table doesn't list those entries, but HDC_CHICKEN2 seems to be
saying the features is in there. HDC_MODE also talks about HDC L1$.
Confusing.

> 
> > 
> >> +  MOCS_ENTRY(48, \
> >> + LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
> >> + L3_3_WB), \
> >> +  /* Gen12+: HW Reserved - HW Special Case (CCS) */ \
> 
> The specs have MOCS 49-51 defined as well.
> 
> Daniele
> 
> >> +  MOCS_ENTRY(60, \
> >> + LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
> >> + L3_1_UC), \
> >> +  /* Gen12+: HW Reserved - HW Special Case (Displayable) */ \
> >> +  MOCS_ENTRY(61, \
> >> + LE_1_UC | LE_TC_1_LLC | LE_SCF(1), \
> >> + L3_3_WB), \
> >>/* HW Reserved - SW pr

Re: [Intel-gfx] [PATCH 18/22] drm/i915/tgl: Define MOCS entries for Tigerlake

2019-07-18 Thread Daniele Ceraolo Spurio



On 7/18/19 6:08 AM, Ville Syrjälä wrote:

On Fri, Jul 12, 2019 at 06:09:36PM -0700, Lucas De Marchi wrote:

From: Tomasz Lis 

The MOCS table is published as part of bspec, and versioned. Entries
are supposed to never be modified, but new ones can be added. Adding
entries increases table version. The patch includes version 1 entries.

Two of the 3 legacy entries used for gen9 are no longer expected to work.
Although we are changing the gen11 table, those changes are supposed to
be backward compatible since we are only touching previously undefined
entries.

Cc: Joonas Lahtinen 
Cc: Mika Kuoppala 
Cc: Daniele Ceraolo Spurio 
Signed-off-by: Tomasz Lis 
Signed-off-by: Lucas De Marchi 
---
  drivers/gpu/drm/i915/gt/intel_mocs.c | 25 ++---
  1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c 
b/drivers/gpu/drm/i915/gt/intel_mocs.c
index 290a5e9b90b9..259e7bec0a63 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
@@ -62,6 +62,10 @@ struct drm_i915_mocs_table {
  #define GEN11_NUM_MOCS_ENTRIES64  /* 63-64 are reserved, but 
configured. */
  
  /* (e)LLC caching options */

+/*
+ * Note: LE_0_PAGETABLE works only up to Gen11; for newer gens it means
+ * the same as LE_UC
+ */
  #define LE_0_PAGETABLE_LE_CACHEABILITY(0)
  #define LE_1_UC   _LE_CACHEABILITY(1)
  #define LE_2_WT   _LE_CACHEABILITY(2)
@@ -100,8 +104,9 @@ struct drm_i915_mocs_table {
   * of bspec.
   *
   * Entries not part of the following tables are undefined as far as
- * userspace is concerned and shouldn't be relied upon.  For the time
- * being they will be initialized to PTE.
+ * userspace is concerned and shouldn't be relied upon.  For Gen < 12
+ * they will be initialized to PTE. Gen >= 12 onwards don't have a setting for
+ * PTE. We use the same value, but that actually means Uncached.
   *
   * The last two entries are reserved by the hardware. For ICL+ they
   * should be initialized according to bspec and never used, for older
@@ -137,11 +142,13 @@ static const struct drm_i915_mocs_entry 
broxton_mocs_table[] = {
  };
  
  #define GEN11_MOCS_ENTRIES \

-   /* Base - Uncached (Deprecated) */ \
+   /* Gen11: Base - Uncached (Deprecated) */ \
+   /* Gen12+: Base - Error (Reserved for Non-Use) */ \
MOCS_ENTRY(I915_MOCS_UNCACHED, \
   LE_1_UC | LE_TC_1_LLC, \
   L3_1_UC), \
/* Base - L3 + LeCC:PAT (Deprecated) */ \
+   /* Gen12+: Base - Reserved */ \
MOCS_ENTRY(I915_MOCS_PTE, \
   LE_0_PAGETABLE | LE_TC_1_LLC, \
   L3_3_WB), \
@@ -233,6 +240,18 @@ static const struct drm_i915_mocs_entry 
broxton_mocs_table[] = {
MOCS_ENTRY(23, \
   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_RSC(1) | LE_SCC(7), \
   L3_3_WB), \
+   /* Gen12+: HW Reserved - HDC:L1 + L3 + LLC */ \


Why is this marked as reserved? From the looks of things 48-61 should
just be normal entries that userspace can select to get HDC L1$. And
looks like icl already has that stuff. So someone should probably figure
out if Mesa/etc. can make use of the HDC L1$, and if so we should add
the relevant MOCS entries for icl as well.


Here the reserved terminology is indeed misleading. The 48-59 range is a 
"special" range where L1 usage is implicitly enabled by the HW, as there 
is no explicit L1 toggle in the MOCS registers. The reserved here means 
that the range shouldn't be used for "normal" MOCS settings, but SW can 
freely use these entries as needed. Similarly, MOCS 60 and 61 are 
reserved for other special purposes, but are still usable by SW. The 
only entries SW shouldn't touch are 62 and 63.


Regarding ICL, Gen11 HW doesn't have the capability so no new entries 
are required there.





+   MOCS_ENTRY(48, \
+  LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+  L3_3_WB), \
+   /* Gen12+: HW Reserved - HW Special Case (CCS) */ \


The specs have MOCS 49-51 defined as well.

Daniele


+   MOCS_ENTRY(60, \
+  LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+  L3_1_UC), \
+   /* Gen12+: HW Reserved - HW Special Case (Displayable) */ \
+   MOCS_ENTRY(61, \
+  LE_1_UC | LE_TC_1_LLC | LE_SCF(1), \
+  L3_3_WB), \
/* HW Reserved - SW program but never use */ \
MOCS_ENTRY(62, \
   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
--
2.21.0



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 18/22] drm/i915/tgl: Define MOCS entries for Tigerlake

2019-07-18 Thread Ville Syrjälä
On Fri, Jul 12, 2019 at 06:09:36PM -0700, Lucas De Marchi wrote:
> From: Tomasz Lis 
> 
> The MOCS table is published as part of bspec, and versioned. Entries
> are supposed to never be modified, but new ones can be added. Adding
> entries increases table version. The patch includes version 1 entries.
> 
> Two of the 3 legacy entries used for gen9 are no longer expected to work.
> Although we are changing the gen11 table, those changes are supposed to
> be backward compatible since we are only touching previously undefined
> entries.
> 
> Cc: Joonas Lahtinen 
> Cc: Mika Kuoppala 
> Cc: Daniele Ceraolo Spurio 
> Signed-off-by: Tomasz Lis 
> Signed-off-by: Lucas De Marchi 
> ---
>  drivers/gpu/drm/i915/gt/intel_mocs.c | 25 ++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c 
> b/drivers/gpu/drm/i915/gt/intel_mocs.c
> index 290a5e9b90b9..259e7bec0a63 100644
> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> @@ -62,6 +62,10 @@ struct drm_i915_mocs_table {
>  #define GEN11_NUM_MOCS_ENTRIES   64  /* 63-64 are reserved, but 
> configured. */
>  
>  /* (e)LLC caching options */
> +/*
> + * Note: LE_0_PAGETABLE works only up to Gen11; for newer gens it means
> + * the same as LE_UC
> + */
>  #define LE_0_PAGETABLE   _LE_CACHEABILITY(0)
>  #define LE_1_UC  _LE_CACHEABILITY(1)
>  #define LE_2_WT  _LE_CACHEABILITY(2)
> @@ -100,8 +104,9 @@ struct drm_i915_mocs_table {
>   * of bspec.
>   *
>   * Entries not part of the following tables are undefined as far as
> - * userspace is concerned and shouldn't be relied upon.  For the time
> - * being they will be initialized to PTE.
> + * userspace is concerned and shouldn't be relied upon.  For Gen < 12
> + * they will be initialized to PTE. Gen >= 12 onwards don't have a setting 
> for
> + * PTE. We use the same value, but that actually means Uncached.
>   *
>   * The last two entries are reserved by the hardware. For ICL+ they
>   * should be initialized according to bspec and never used, for older
> @@ -137,11 +142,13 @@ static const struct drm_i915_mocs_entry 
> broxton_mocs_table[] = {
>  };
>  
>  #define GEN11_MOCS_ENTRIES \
> - /* Base - Uncached (Deprecated) */ \
> + /* Gen11: Base - Uncached (Deprecated) */ \
> + /* Gen12+: Base - Error (Reserved for Non-Use) */ \
>   MOCS_ENTRY(I915_MOCS_UNCACHED, \
>  LE_1_UC | LE_TC_1_LLC, \
>  L3_1_UC), \
>   /* Base - L3 + LeCC:PAT (Deprecated) */ \
> + /* Gen12+: Base - Reserved */ \
>   MOCS_ENTRY(I915_MOCS_PTE, \
>  LE_0_PAGETABLE | LE_TC_1_LLC, \
>  L3_3_WB), \
> @@ -233,6 +240,18 @@ static const struct drm_i915_mocs_entry 
> broxton_mocs_table[] = {
>   MOCS_ENTRY(23, \
>  LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_RSC(1) | LE_SCC(7), \
>  L3_3_WB), \
> + /* Gen12+: HW Reserved - HDC:L1 + L3 + LLC */ \

Why is this marked as reserved? From the looks of things 48-61 should
just be normal entries that userspace can select to get HDC L1$. And
looks like icl already has that stuff. So someone should probably figure
out if Mesa/etc. can make use of the HDC L1$, and if so we should add
the relevant MOCS entries for icl as well.

> + MOCS_ENTRY(48, \
> +LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
> +L3_3_WB), \
> + /* Gen12+: HW Reserved - HW Special Case (CCS) */ \
> + MOCS_ENTRY(60, \
> +LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
> +L3_1_UC), \
> + /* Gen12+: HW Reserved - HW Special Case (Displayable) */ \
> + MOCS_ENTRY(61, \
> +LE_1_UC | LE_TC_1_LLC | LE_SCF(1), \
> +L3_3_WB), \
>   /* HW Reserved - SW program but never use */ \
>   MOCS_ENTRY(62, \
>  LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
> -- 
> 2.21.0

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 18/22] drm/i915/tgl: Define MOCS entries for Tigerlake

2019-07-12 Thread Lucas De Marchi
From: Tomasz Lis 

The MOCS table is published as part of bspec, and versioned. Entries
are supposed to never be modified, but new ones can be added. Adding
entries increases table version. The patch includes version 1 entries.

Two of the 3 legacy entries used for gen9 are no longer expected to work.
Although we are changing the gen11 table, those changes are supposed to
be backward compatible since we are only touching previously undefined
entries.

Cc: Joonas Lahtinen 
Cc: Mika Kuoppala 
Cc: Daniele Ceraolo Spurio 
Signed-off-by: Tomasz Lis 
Signed-off-by: Lucas De Marchi 
---
 drivers/gpu/drm/i915/gt/intel_mocs.c | 25 ++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c 
b/drivers/gpu/drm/i915/gt/intel_mocs.c
index 290a5e9b90b9..259e7bec0a63 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
@@ -62,6 +62,10 @@ struct drm_i915_mocs_table {
 #define GEN11_NUM_MOCS_ENTRIES 64  /* 63-64 are reserved, but configured. */
 
 /* (e)LLC caching options */
+/*
+ * Note: LE_0_PAGETABLE works only up to Gen11; for newer gens it means
+ * the same as LE_UC
+ */
 #define LE_0_PAGETABLE _LE_CACHEABILITY(0)
 #define LE_1_UC_LE_CACHEABILITY(1)
 #define LE_2_WT_LE_CACHEABILITY(2)
@@ -100,8 +104,9 @@ struct drm_i915_mocs_table {
  * of bspec.
  *
  * Entries not part of the following tables are undefined as far as
- * userspace is concerned and shouldn't be relied upon.  For the time
- * being they will be initialized to PTE.
+ * userspace is concerned and shouldn't be relied upon.  For Gen < 12
+ * they will be initialized to PTE. Gen >= 12 onwards don't have a setting for
+ * PTE. We use the same value, but that actually means Uncached.
  *
  * The last two entries are reserved by the hardware. For ICL+ they
  * should be initialized according to bspec and never used, for older
@@ -137,11 +142,13 @@ static const struct drm_i915_mocs_entry 
broxton_mocs_table[] = {
 };
 
 #define GEN11_MOCS_ENTRIES \
-   /* Base - Uncached (Deprecated) */ \
+   /* Gen11: Base - Uncached (Deprecated) */ \
+   /* Gen12+: Base - Error (Reserved for Non-Use) */ \
MOCS_ENTRY(I915_MOCS_UNCACHED, \
   LE_1_UC | LE_TC_1_LLC, \
   L3_1_UC), \
/* Base - L3 + LeCC:PAT (Deprecated) */ \
+   /* Gen12+: Base - Reserved */ \
MOCS_ENTRY(I915_MOCS_PTE, \
   LE_0_PAGETABLE | LE_TC_1_LLC, \
   L3_3_WB), \
@@ -233,6 +240,18 @@ static const struct drm_i915_mocs_entry 
broxton_mocs_table[] = {
MOCS_ENTRY(23, \
   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_RSC(1) | LE_SCC(7), \
   L3_3_WB), \
+   /* Gen12+: HW Reserved - HDC:L1 + L3 + LLC */ \
+   MOCS_ENTRY(48, \
+  LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+  L3_3_WB), \
+   /* Gen12+: HW Reserved - HW Special Case (CCS) */ \
+   MOCS_ENTRY(60, \
+  LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
+  L3_1_UC), \
+   /* Gen12+: HW Reserved - HW Special Case (Displayable) */ \
+   MOCS_ENTRY(61, \
+  LE_1_UC | LE_TC_1_LLC | LE_SCF(1), \
+  L3_3_WB), \
/* HW Reserved - SW program but never use */ \
MOCS_ENTRY(62, \
   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
-- 
2.21.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx