Re: [PATCH v8 4/7] drm/i915: use a macro to define MOCS entries

From: "Lis, Tomasz" <tomasz.lis@intel.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v8 4/7] drm/i915: use a macro to define MOCS entries
Date: Wed, 23 Jan 2019 19:43:31 +0100	[thread overview]
Message-ID: <45f8e1f1-ce9b-8ed3-9625-d63e47836e5d@intel.com> (raw)
In-Reply-To: <20190122051227.8329-5-lucas.demarchi@intel.com>


On 2019-01-22 06:12, Lucas De Marchi wrote:
> Let's use a macro to make tables smaller and at the same time allow us
> to add fields that apply to all entries in future.
>
> For the sake of readability, I'm calling an exception on 80 chars limit.
> Lines are aligned for easy comparison of the entry values.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Tomasz Lis <tomasz.lis@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_mocs.c | 39 +++++++++++--------------------
>   1 file changed, 14 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index c7a2a8d81d90..faae2eefc5cc 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/intel_mocs.c
> @@ -71,6 +71,12 @@ struct drm_i915_mocs_table {
>   #define L3_2_RESERVED		_L3_CACHEABILITY(2)
>   #define L3_3_WB			_L3_CACHEABILITY(3)
>   
> +#define MOCS_ENTRY(__idx, __control_value, __l3cc_value) \
> +	[__idx] = { \
> +		.control_value = __control_value, \
> +		.l3cc_value = __l3cc_value, \
> +	}
> +
>   /*
>    * MOCS tables
>    *
> @@ -93,40 +99,23 @@ struct drm_i915_mocs_table {
>    *       may only be updated incrementally by adding entries at the
>    *       end.
>    */
> -
The idea behind this EOL was that the comment above relates to several 
statements below, not just the first one.
But I'm not really sure what our commenting rules are in this case - a 
comment which bundles several definitions.
-Tomasz
>   #define GEN9_MOCS_ENTRIES \
> -	[I915_MOCS_UNCACHED] = { \
> -		/* 0x00000009 */ \
> -		.control_value = LE_1_UC | LE_TC_2_LLC_ELLC, \
> -		/* 0x0010 */ \
> -		.l3cc_value = L3_1_UC, \
> -	}, \
> -	[I915_MOCS_PTE] = { \
> -		/* 0x00000038 */ \
> -		.control_value = LE_0_PAGETABLE | LE_TC_2_LLC_ELLC | LE_LRUM(3), \
> -		/* 0x0030 */ \
> -		.l3cc_value = L3_3_WB, \
> -	}
> +	MOCS_ENTRY(I915_MOCS_UNCACHED,	LE_1_UC | LE_TC_2_LLC_ELLC, \
> +					L3_1_UC), \
> +	MOCS_ENTRY(I915_MOCS_PTE,	LE_0_PAGETABLE | LE_TC_2_LLC_ELLC | LE_LRUM(3), \
> +					L3_3_WB) \
>   
>   static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
>   	GEN9_MOCS_ENTRIES,
> -	[I915_MOCS_CACHED] = {
> -		/* 0x0000003b */
> -		.control_value = LE_3_WB | LE_TC_2_LLC_ELLC | LE_LRUM(3),
> -		/* 0x0030 */
> -		.l3cc_value =   L3_3_WB,
> -	},
> +	MOCS_ENTRY(I915_MOCS_CACHED,	LE_3_WB | LE_TC_2_LLC_ELLC | LE_LRUM(3),
> +					L3_3_WB)
>   };
>   
>   /* NOTE: the LE_TGT_CACHE is not used on Broxton */
>   static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
>   	GEN9_MOCS_ENTRIES,
> -	[I915_MOCS_CACHED] = {
> -		/* 0x00000039 */
> -		.control_value = LE_1_UC | LE_TC_2_LLC_ELLC | LE_LRUM(3),
> -		/* 0x0030 */
> -		.l3cc_value = L3_3_WB,
> -	},
> +	MOCS_ENTRY(I915_MOCS_CACHED,	LE_1_UC | LE_TC_2_LLC_ELLC | LE_LRUM(3),
> +					L3_3_WB)
>   };
>   
>   /**

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