diff mbox

[1/2] drm/i915/gen9: Clean up MOCS table definitions

Message ID 1461587001-2463-1-git-send-email-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Imre Deak April 25, 2016, 12:23 p.m. UTC
Use named struct initializers for clarity. Also fix the target cache
definition to reflect its role in GEN9 onwards. On GEN8 a TC value of 0
meant ELLC but on GEN9+ it means the TC and LRU controls are taken from
the PTE.

No functional change.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_mocs.c | 79 ++++++++++++++++++++++++++-------------
 1 file changed, 52 insertions(+), 27 deletions(-)

Comments

Chris Wilson April 25, 2016, 12:30 p.m. UTC | #1
On Mon, Apr 25, 2016 at 03:23:20PM +0300, Imre Deak wrote:
> Use named struct initializers for clarity. Also fix the target cache
> definition to reflect its role in GEN9 onwards. On GEN8 a TC value of 0
> meant ELLC but on GEN9+ it means the TC and LRU controls are taken from
> the PTE.
> 
> No functional change.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_mocs.c | 79 ++++++++++++++++++++++++++-------------
>  1 file changed, 52 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index 23b8545..5006a92 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/intel_mocs.c
> @@ -66,9 +66,10 @@ struct drm_i915_mocs_table {
>  #define L3_WB			3
>  
>  /* Target cache */
> -#define ELLC			0
> -#define LLC			1
> -#define LLC_ELLC		2
> +#define LE_TC_PAGETABLE		0
> +#define LE_TC_LLC		1
> +#define LE_TC_LLC_ELLC		2
> +#define LE_TC_LLC_ELLC_ALT	3
>  
>  /*
>   * MOCS tables
> @@ -96,34 +97,58 @@ struct drm_i915_mocs_table {
>   *       end.
>   */
>  static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
> -	/* { 0x00000009, 0x0010 } */

Hmm, we lose this convenience. Since we have the same table in userspace
to check the ABI is maintained, it would be nice to keep around.

Also mention that you ran igt/gem_mocs_settings to confirm no changes.
-Chris
Imre Deak April 25, 2016, 5:26 p.m. UTC | #2
On ma, 2016-04-25 at 13:30 +0100, Chris Wilson wrote:
> On Mon, Apr 25, 2016 at 03:23:20PM +0300, Imre Deak wrote:
> > Use named struct initializers for clarity. Also fix the target cache
> > definition to reflect its role in GEN9 onwards. On GEN8 a TC value of 0
> > meant ELLC but on GEN9+ it means the TC and LRU controls are taken from
> > the PTE.
> > 
> > No functional change.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_mocs.c | 79 ++++++++++++++++++++++++++-------------
> >  1 file changed, 52 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> > index 23b8545..5006a92 100644
> > --- a/drivers/gpu/drm/i915/intel_mocs.c
> > +++ b/drivers/gpu/drm/i915/intel_mocs.c
> > @@ -66,9 +66,10 @@ struct drm_i915_mocs_table {
> >  #define L3_WB			3
> >  
> >  /* Target cache */
> > -#define ELLC			0
> > -#define LLC			1
> > -#define LLC_ELLC		2
> > +#define LE_TC_PAGETABLE		0
> > +#define LE_TC_LLC		1
> > +#define LE_TC_LLC_ELLC		2
> > +#define LE_TC_LLC_ELLC_ALT	3
> >  
> >  /*
> >   * MOCS tables
> > @@ -96,34 +97,58 @@ struct drm_i915_mocs_table {
> >   *       end.
> >   */
> >  static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
> > -	/* { 0x00000009, 0x0010 } */
> 
> Hmm, we lose this convenience. Since we have the same table in userspace
> to check the ABI is maintained, it would be nice to keep around.

Ok, I can add them back.

> Also mention that you ran igt/gem_mocs_settings to confirm no changes.

Ok, didn't know about this patch. After patch 2/2 there is a change in
the 3rd entry but that's expected based on the explanation of the
commit message there. So I can fix the test case accordingly.

--Imre
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index 23b8545..5006a92 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -66,9 +66,10 @@  struct drm_i915_mocs_table {
 #define L3_WB			3
 
 /* Target cache */
-#define ELLC			0
-#define LLC			1
-#define LLC_ELLC		2
+#define LE_TC_PAGETABLE		0
+#define LE_TC_LLC		1
+#define LE_TC_LLC_ELLC		2
+#define LE_TC_LLC_ELLC_ALT	3
 
 /*
  * MOCS tables
@@ -96,34 +97,58 @@  struct drm_i915_mocs_table {
  *       end.
  */
 static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
-	/* { 0x00000009, 0x0010 } */
-	{ (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(0) |
-	   LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)),
-	  (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) },
-	/* { 0x00000038, 0x0030 } */
-	{ (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) |
-	   LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)),
-	  (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) },
-	/* { 0x0000003b, 0x0030 } */
-	{ (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) |
-	   LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)),
-	  (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }
+	{
+	  .control_value = LE_CACHEABILITY(LE_UC) |
+			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
+			   LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
+			   LE_PFM(0) | LE_SCF(0),
+
+	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
+	},
+	{
+	  .control_value = LE_CACHEABILITY(LE_PAGETABLE) |
+			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
+			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
+			   LE_PFM(0) | LE_SCF(0),
+
+	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+	},
+	{
+	  .control_value = LE_CACHEABILITY(LE_WB) |
+			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
+			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
+			   LE_PFM(0) | LE_SCF(0),
+
+	  .l3cc_value =   L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+	},
 };
 
 /* NOTE: the LE_TGT_CACHE is not used on Broxton */
 static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
-	/* { 0x00000009, 0x0010 } */
-	{ (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(0) |
-	   LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)),
-	  (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) },
-	/* { 0x00000038, 0x0030 } */
-	{ (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) |
-	   LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)),
-	  (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) },
-	/* { 0x0000003b, 0x0030 } */
-	{ (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) |
-	   LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)),
-	  (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }
+	{
+	  .control_value = LE_CACHEABILITY(LE_UC) |
+			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
+			   LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
+			   LE_PFM(0) | LE_SCF(0),
+
+	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
+	},
+	{
+	  .control_value = LE_CACHEABILITY(LE_PAGETABLE) |
+			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
+			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
+			   LE_PFM(0) | LE_SCF(0),
+
+	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+	},
+	{
+	  .control_value = LE_CACHEABILITY(LE_WB) |
+			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
+			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
+			   LE_PFM(0) | LE_SCF(0),
+
+	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+	},
 };
 
 /**