[Intel-gfx] [PATCH v6 1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define

[Tvrtko Ursulin]

On 07/11/2018 15:16, Tomasz Lis wrote:
> The MOCS tables are going to be very similar across platforms.
> 
> To reduce the amount of copied code, this patch rips the common part and
> puts it into a definition valid for all gen9 platforms.
> 
> v2: Made defines for or-ing flags. Renamed macros from MOCS_TABLE
>      to MOCS_ENTRIES. (Joonas)
> 
> Signed-off-by: Tomasz Lis <tomasz.lis at intel.com>
> Suggested-by: Lucas De Marchi <lucas.demarchi at intel.com>
> Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com> (v1)

R-b needs to be upgraded to v2 before merge.

> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>   drivers/gpu/drm/i915/intel_mocs.c | 86 ++++++++++++++++-----------------------
>   1 file changed, 36 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index 77e9871..8d08a7b 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/intel_mocs.c
> @@ -96,71 +96,57 @@ struct drm_i915_mocs_table {
>    *       may only be updated incrementally by adding entries at the
>    *       end.
>    */
> -static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
> -	[I915_MOCS_UNCACHED] = {
> -	  /* 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),
> -	},
> -	[I915_MOCS_PTE] = {
> -	  /* 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),
> +
> +#define MOCS_CONTROL_VALUE(lecc, tc, lrum, daom, ersc, scc, pfm, scf) \
> +	(LE_CACHEABILITY(lecc) | LE_TGT_CACHE(tc) | \
> +	LE_LRUM(lrum) | LE_AOM(daom) | LE_RSC(ersc) | LE_SCC(scc) | \
> +	LE_PFM(pfm) | LE_SCF(scf))
> +
> +#define MOCS_L3CC_VALUE(esc, scc, l3cc) \
> +	(L3_ESC(esc) | L3_SCC(scc) | L3_CACHEABILITY(l3cc))

These two macros do not seem more readable than the previous code, since 
one has to reference the macro to remind himself what is what. But never 
mind, I am only here because Tomasz copied me on a ping email. So only a 
reminder to upgrade the r-b.

Regards,

Tvrtko

> +
> +#define GEN9_MOCS_ENTRIES \
> +	[I915_MOCS_UNCACHED] = { \
> +	  /* 0x00000009 */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC_ELLC, \
> +					      0, 0, 0, 0, 0, 0), \
> +	  /* 0x0010 */ \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
> +	}, \
> +	[I915_MOCS_PTE] = { \
> +	  /* 0x00000038 */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_PAGETABLE, LE_TC_LLC_ELLC, \
> +					      3, 0, 0, 0, 0, 0), \
> +	  /* 0x0030 */ \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
>   	},
> +
> +static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
> +	GEN9_MOCS_ENTRIES
>   	[I915_MOCS_CACHED] = {
>   	  /* 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),
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC_ELLC,
> +					      3, 0, 0, 0, 0, 0),
>   	  /* 0x0030 */
> -	  .l3cc_value =   L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB),
>   	},
>   };
>   
>   /* NOTE: the LE_TGT_CACHE is not used on Broxton */
>   static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
> -	[I915_MOCS_UNCACHED] = {
> -	  /* 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),
> -	},
> -	[I915_MOCS_PTE] = {
> -	  /* 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),
> -	},
> +	GEN9_MOCS_ENTRIES
>   	[I915_MOCS_CACHED] = {
>   	  /* 0x00000039 */
> -	  .control_value = LE_CACHEABILITY(LE_UC) |
> -			   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),
> -
> +	  .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC_ELLC,
> +					      3, 0, 0, 0, 0, 0),
>   	  /* 0x0030 */
> -	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB),
>   	},
>   };
>   
> +#undef MOCS_CONTROL_VALUE
> +#undef MOCS_L3CC_VALUE
> +
>   /**
>    * get_mocs_settings()
>    * @dev_priv:	i915 device.
>