On Thu, 2023-04-06 at 15:26 -0700, Harrison, John C wrote:
> From: John Harrison <john.c.harri...@intel.com>
> 
> A pair of pre-Xe registers were being included in the Xe capture list.
> GuC was rejecting those as being invalid and logging errors about
> them. So, stop doing it.
> 
alan:snip
>  #define COMMON_GEN9BASE_GLOBAL \
> -     { GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
> -     { GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }, \
>       { ERROR_GEN6,               0,      0, "ERROR_GEN6" }, \
>       { DONE_REG,                 0,      0, "DONE_REG" }, \
>       { HSW_GTT_CACHE_EN,         0,      0, "HSW_GTT_CACHE_EN" }
>  
> +#define GEN9_GLOBAL \
> +     { GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
> +     { GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }
> +
>  #define COMMON_GEN12BASE_GLOBAL \
>       { GEN12_FAULT_TLB_DATA0,    0,      0, "GEN12_FAULT_TLB_DATA0" }, \
>       { GEN12_FAULT_TLB_DATA1,    0,      0, "GEN12_FAULT_TLB_DATA1" }, \
> @@ -142,6 +144,7 @@ static const struct __guc_mmio_reg_descr 
> xe_lpd_gsc_inst_regs[] = {
>  static const struct __guc_mmio_reg_descr default_global_regs[] = {
>       COMMON_BASE_GLOBAL,
>       COMMON_GEN9BASE_GLOBAL,
> +     GEN9_GLOBAL,
>  };
>  
alan: splitting out a couple registers from COMMON_GEN9BASE_GLOBAL into 
GEN9_GLOBAL
doesn't seem to communicate the intent of fix for this patch. This is more of a 
naming,
thing and i am not sure what counter-proposal will work well in terms of 
readibility.
One idea: perhaps we rename "COMMON_GEN9BASE_GLOBAL" to 
"COMMON_GEN9PLUS_BASE_GLOBAL"
and rename GEN9_GLOBAL to COMMON_GEN9LEGACY_GLOBAL. so we would have two 
gen9-global
with a clear distinction in naming where one is "GEN9PLUS" and the other is 
"GEN9LEGACY".

But since this is a list-naming thing, i am okay either above change... OR...
keeping the same but with the condition of adding a comment under
COMMON_GEN9BASE_GLOBAL and GEN9_GLOBAL names that explain the differences where 
one
is gen9-legacy and the other is gen9-and-future that carries over to beyond 
Gen9.
(side note: coding style wise, is it possible to add the comment right under 
the #define
line as opposed to under the entire list?)

(conditional) Reviewed-by: Alan Previn <alan.previn.teres.ale...@intel.com>

Reply via email to