diff mbox

[v3,1/3] drm/i915/gen9: Clean up MOCS table definitions

Message ID 1467380406-11954-2-git-send-email-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Imre Deak July 1, 2016, 1:40 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, igt/gem_mocs_settings still passing after this
change.

v2: (Chris)
- Add back the hexa literals for the entries.
  Add note that igt/gem_mocs_settings still passes.

CC: Rong R Yang <rong.r.yang@intel.com>
CC: Yakui Zhao <yakui.zhao@intel.com>
CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_mocs.c | 88 +++++++++++++++++++++++++++------------
 1 file changed, 61 insertions(+), 27 deletions(-)

Comments

Bish, Jim July 1, 2016, 9:23 p.m. UTC | #1
like the cleanup.  Perhaps someday we will add more entries and have
the user space consume them :)

Jim

On Fri, 2016-07-01 at 16:40 +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, igt/gem_mocs_settings still passing after this

> change.

> 

> v2: (Chris)

> - Add back the hexa literals for the entries.

>   Add note that igt/gem_mocs_settings still passes.

> 

> CC: Rong R Yang <rong.r.yang@intel.com>

> CC: Yakui Zhao <yakui.zhao@intel.com>

> CC: Chris Wilson <chris@chris-wilson.co.uk>

> Signed-off-by: Imre Deak <imre.deak@intel.com>

> ---

>  drivers/gpu/drm/i915/intel_mocs.c | 88 +++++++++++++++++++++++++++

> ------------

>  1 file changed, 61 insertions(+), 27 deletions(-)

> 

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

> b/drivers/gpu/drm/i915/intel_mocs.c

> index 3c1482b..d36e609 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,67 @@ 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)) }

> +	{ /* 0x00000009 */

> +	  .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),

> +

> +	  /* 0x0010 */

> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |

> L3_CACHEABILITY(L3_UC),

> +	},

> +	{

> +	  /* 0x00000038 */

> +	  .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),

> +	  /* 0x0030 */

> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |

> L3_CACHEABILITY(L3_WB),

> +	},

> +	{

> +	  /* 0x0000003b */

> +	  .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),

> +	  /* 0x0030 */

> +	  .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)) }

> +	{

> +	  /* 0x00000009 */

> +	  .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),

> +

> +	  /* 0x0010 */

> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |

> L3_CACHEABILITY(L3_UC),

> +	},

> +	{

> +	  /* 0x00000038 */

> +	  .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),

> +

> +	  /* 0x0030 */

> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |

> L3_CACHEABILITY(L3_WB),

> +	},

> +	{

> +	  /* 0x0000003b */

> +	  .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),

> +

> +	  /* 0x0030 */

> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |

> L3_CACHEABILITY(L3_WB),

> +	},

>  };

>  

>  /**
Francisco Jerez July 1, 2016, 9:47 p.m. UTC | #2
Hi Imre,

Imre Deak <imre.deak@intel.com> writes:

> 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.
>
I vaguely recall bringing up this apparent inconsistency with the
hardware spec during review of the original series of patches
programming the MOCS tables.  IIRC TC value 0 behaved as ELLC-only on
the simulator, but the hardware docs claimed it would take the target
cache from the PTE controls, not sure what the real hardware actually
does.  Maybe Peter can confirm whether this was intentional?

> No functional change, igt/gem_mocs_settings still passing after this
> change.
>
> v2: (Chris)
> - Add back the hexa literals for the entries.
>   Add note that igt/gem_mocs_settings still passes.
>
> CC: Rong R Yang <rong.r.yang@intel.com>
> CC: Yakui Zhao <yakui.zhao@intel.com>
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_mocs.c | 88 +++++++++++++++++++++++++++------------
>  1 file changed, 61 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index 3c1482b..d36e609 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,67 @@ 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)) }
> +	{ /* 0x00000009 */
> +	  .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),
> +
> +	  /* 0x0010 */
> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
> +	},
> +	{
> +	  /* 0x00000038 */
> +	  .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),
> +	  /* 0x0030 */
> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
> +	},
> +	{
> +	  /* 0x0000003b */
> +	  .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),
> +	  /* 0x0030 */
> +	  .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)) }
> +	{
> +	  /* 0x00000009 */
> +	  .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),
> +
> +	  /* 0x0010 */
> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
> +	},
> +	{
> +	  /* 0x00000038 */
> +	  .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),
> +
> +	  /* 0x0030 */
> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
> +	},
> +	{
> +	  /* 0x0000003b */
> +	  .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),
> +
> +	  /* 0x0030 */
> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
> +	},
>  };
>  
>  /**
> -- 
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Imre Deak July 12, 2016, 11:08 a.m. UTC | #3
On pe, 2016-07-01 at 14:47 -0700, Francisco Jerez wrote:
> Hi Imre,
> 
> Imre Deak <imre.deak@intel.com> writes:
> 
> > 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.
> > 
> I vaguely recall bringing up this apparent inconsistency with the
> hardware spec during review of the original series of patches
> programming the MOCS tables.  IIRC TC value 0 behaved as ELLC-only on
> the simulator, but the hardware docs claimed it would take the target
> cache from the PTE controls, not sure what the real hardware actually
> does.

At least the documentation seems to be quite consistent on this. It's
true that you have to set the MOCS TC to 0 for an eLLC-only mapping,
but that depends on the (default) 0 of the PTE [3:2] field (override to
eLLC-only). So maybe during simulation PTE was at the default eLLC-
only, hence your observed behavior.

In any case we don't use TC PTE passthrough for any MOCS index atm, so
I would suggest keeping the code in line with the documentation for
now.

--Imre

> Maybe Peter can confirm whether this was intentional?
> 
> > No functional change, igt/gem_mocs_settings still passing after
> > this
> > change.
> > 
> > v2: (Chris)
> > - Add back the hexa literals for the entries.
> >   Add note that igt/gem_mocs_settings still passes.
> > 
> > CC: Rong R Yang <rong.r.yang@intel.com>
> > CC: Yakui Zhao <yakui.zhao@intel.com>
> > CC: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_mocs.c | 88
> > +++++++++++++++++++++++++++------------
> >  1 file changed, 61 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_mocs.c
> > b/drivers/gpu/drm/i915/intel_mocs.c
> > index 3c1482b..d36e609 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,67 @@ 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)) }
> > +	{ /* 0x00000009 */
> > +	  .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),
> > +
> > +	  /* 0x0010 */
> > +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |
> > L3_CACHEABILITY(L3_UC),
> > +	},
> > +	{
> > +	  /* 0x00000038 */
> > +	  .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),
> > +	  /* 0x0030 */
> > +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |
> > L3_CACHEABILITY(L3_WB),
> > +	},
> > +	{
> > +	  /* 0x0000003b */
> > +	  .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),
> > +	  /* 0x0030 */
> > +	  .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)) }
> > +	{
> > +	  /* 0x00000009 */
> > +	  .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),
> > +
> > +	  /* 0x0010 */
> > +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |
> > L3_CACHEABILITY(L3_UC),
> > +	},
> > +	{
> > +	  /* 0x00000038 */
> > +	  .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),
> > +
> > +	  /* 0x0030 */
> > +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |
> > L3_CACHEABILITY(L3_WB),
> > +	},
> > +	{
> > +	  /* 0x0000003b */
> > +	  .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),
> > +
> > +	  /* 0x0030 */
> > +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) |
> > L3_CACHEABILITY(L3_WB),
> > +	},
> >  };
> >  
> >  /**
Zhao, Yakui July 13, 2016, 2:10 a.m. UTC | #4
On 07/01/2016 09:40 PM, Deak, Imre 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, igt/gem_mocs_settings still passing after this
> change.
>
> v2: (Chris)
> - Add back the hexa literals for the entries.
>    Add note that igt/gem_mocs_settings still passes.
>
> CC: Rong R Yang<rong.r.yang@intel.com>
> CC: Yakui Zhao<yakui.zhao@intel.com>
> CC: Chris Wilson<chris@chris-wilson.co.uk>
> Signed-off-by: Imre Deak<imre.deak@intel.com>

It is helpful to understand the MOCS table definition after cleaning up.

Add: Acked-by: Zhao Yakui <yakui.zhao@intel.com>

Thanks
   Yakui

> ---
>   drivers/gpu/drm/i915/intel_mocs.c | 88 +++++++++++++++++++++++++++------------
>   1 file changed, 61 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index 3c1482b..d36e609 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,67 @@ 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)) }
> +	{ /* 0x00000009 */
> +	  .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),
> +
> +	  /* 0x0010 */
> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
> +	},
> +	{
> +	  /* 0x00000038 */
> +	  .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),
> +	  /* 0x0030 */
> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
> +	},
> +	{
> +	  /* 0x0000003b */
> +	  .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),
> +	  /* 0x0030 */
> +	  .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)) }
> +	{
> +	  /* 0x00000009 */
> +	  .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),
> +
> +	  /* 0x0010 */
> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
> +	},
> +	{
> +	  /* 0x00000038 */
> +	  .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),
> +
> +	  /* 0x0030 */
> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
> +	},
> +	{
> +	  /* 0x0000003b */
> +	  .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),
> +
> +	  /* 0x0030 */
> +	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
> +	},
>   };
>
>   /**
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index 3c1482b..d36e609 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,67 @@  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)) }
+	{ /* 0x00000009 */
+	  .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),
+
+	  /* 0x0010 */
+	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
+	},
+	{
+	  /* 0x00000038 */
+	  .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),
+	  /* 0x0030 */
+	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+	},
+	{
+	  /* 0x0000003b */
+	  .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),
+	  /* 0x0030 */
+	  .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)) }
+	{
+	  /* 0x00000009 */
+	  .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),
+
+	  /* 0x0010 */
+	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
+	},
+	{
+	  /* 0x00000038 */
+	  .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),
+
+	  /* 0x0030 */
+	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+	},
+	{
+	  /* 0x0000003b */
+	  .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),
+
+	  /* 0x0030 */
+	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+	},
 };
 
 /**