diff mbox series

[1/5] drm/i915/guc: Don't capture Gen8 regs on Xe devices

Message ID 20230406222617.790484-2-John.C.Harrison@Intel.com (mailing list archive)
State New, archived
Headers show
Series Improvements to GuC error capture list processing | expand

Commit Message

John Harrison April 6, 2023, 10:26 p.m. UTC 
From: John Harrison <John.C.Harrison@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.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Fixes: dce2bd542337 ("drm/i915/guc: Add Gen9 registers for GuC error state capture.")
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Teres Alexis, Alan Previn April 25, 2023, 5:55 p.m. UTC | #1 
On Thu, 2023-04-06 at 15:26 -0700, Harrison, John C wrote:
> From: John Harrison <John.C.Harrison@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.alexis@intel.com>
John Harrison April 26, 2023, 5:22 p.m. UTC | #2 
On 4/25/2023 10:55, Teres Alexis, Alan Previn wrote:
> On Thu, 2023-04-06 at 15:26 -0700, Harrison, John C wrote:
>> From: John Harrison <John.C.Harrison@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.alexis@intel.com>
>
I'm not entirely sure what you are arguing here.

My reading of the original code is that COMMON_GENX_ means the registers 
were introduced on the named device but a are common to later devices. 
Whereas GENX_ means the registers are specific to that device alone. 
That seems a pretty straight forward and simple naming scheme to me.

John.
John Harrison April 26, 2023, 5:23 p.m. UTC | #3 
On 4/25/2023 10:55, Teres Alexis, Alan Previn wrote:
> On Thu, 2023-04-06 at 15:26 -0700, Harrison, John C wrote:
>> From: John Harrison <John.C.Harrison@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.alexis@intel.com>
>
I'm not entirely sure what you are arguing here.

My reading of the original code is that COMMON_GENX_ means the registers 
were introduced on the named device but a are common to later devices. 
Whereas GENX_ means the registers are specific to that device alone. 
That seems a pretty straight forward and simple naming scheme to me.

John.
Teres Alexis, Alan Previn April 26, 2023, 9:14 p.m. UTC | #4 
On Wed, 2023-04-26 at 10:23 -0700, Harrison, John C wrote:
> On 4/25/2023 10:55, Teres Alexis, Alan Previn wrote:
> > On Thu, 2023-04-06 at 15:26 -0700, Harrison, John C wrote:
> > > From: John Harrison <John.C.Harrison@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.alexis@intel.com>
> > 
> I'm not entirely sure what you are arguing here.
> 
> My reading of the original code is that COMMON_GENX_ means the registers 
> were introduced on the named device but a are common to later devices. 
> Whereas GENX_ means the registers are specific to that device alone. 
> That seems a pretty straight forward and simple naming scheme to me.
> 
> John.
> 
alan: you said "reading of the original code is that COMMON_GENX_
means the registers were introduced on the named device but a are
common to later devices".
That wasnt the original intent when authored. The original intent
was list names and its comments to corresponded to what platforms we
supported (thats why the first patch was all of Gen12 registers in a
single list that included Gen8/9 register definitions and Gen9 sublists
got abstracted out later).

I'm okay with changing the intent of the list naming - but your code still
looks a bit off considering you have at least one list with two sublists
that carry the term "GEN9":
  static const struct __guc_mmio_reg_descr default_global_regs[] = {
	COMMON_BASE_GLOBAL,
	COMMON_GEN9BASE_GLOBAL,
	GEN9_GLOBAL,

Again, i feel the name of these two sublists ("COMMON_GEN9BASE_GLOBAL" vs
"GEN9_GLOBAL") dont easily portray differences and why they were separated
out. If they both reflect "when the named register got introduced", then
why not just have them in the same list? Since this patch is not
about "when a register got introduced" but "pre-Xe registers are not
recognized by GuC on Xe", then perhaps we can change the names to:
	COMMON_GEN9BASE_GLOBAL -> [same]
	GEN9_GLOBAL -> PREXE_GEN9_GLOBAL

Either way, i rather not argue about variable names - so here is the Rb
(since patch comment describes the issue and functionality seems correct):
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
John Harrison April 28, 2023, 6:21 p.m. UTC | #5 
On 4/26/2023 14:14, Teres Alexis, Alan Previn wrote:
> On Wed, 2023-04-26 at 10:23 -0700, Harrison, John C wrote:
>> On 4/25/2023 10:55, Teres Alexis, Alan Previn wrote:
>>> On Thu, 2023-04-06 at 15:26 -0700, Harrison, John C wrote:
>>>> From: John Harrison <John.C.Harrison@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.alexis@intel.com>
>>>
>> I'm not entirely sure what you are arguing here.
>>
>> My reading of the original code is that COMMON_GENX_ means the registers
>> were introduced on the named device but a are common to later devices.
>> Whereas GENX_ means the registers are specific to that device alone.
>> That seems a pretty straight forward and simple naming scheme to me.
>>
>> John.
>>
> alan: you said "reading of the original code is that COMMON_GENX_
> means the registers were introduced on the named device but a are
> common to later devices".
> That wasnt the original intent when authored. The original intent
> was list names and its comments to corresponded to what platforms we
> supported (thats why the first patch was all of Gen12 registers in a
> single list that included Gen8/9 register definitions and Gen9 sublists
> got abstracted out later).
>
> I'm okay with changing the intent of the list naming - but your code still
> looks a bit off considering you have at least one list with two sublists
> that carry the term "GEN9":
>    static const struct __guc_mmio_reg_descr default_global_regs[] = {
> 	COMMON_BASE_GLOBAL,
> 	COMMON_GEN9BASE_GLOBAL,
> 	GEN9_GLOBAL,
>
> Again, i feel the name of these two sublists ("COMMON_GEN9BASE_GLOBAL" vs
> "GEN9_GLOBAL") dont easily portray differences and why they were separated
> out. If they both reflect "when the named register got introduced", then
> why not just have them in the same list? Since this patch is not
> about "when a register got introduced" but "pre-Xe registers are not
> recognized by GuC on Xe", then perhaps we can change the names to:
> 	COMMON_GEN9BASE_GLOBAL -> [same]
> 	GEN9_GLOBAL -> PREXE_GEN9_GLOBAL
PREXE_GEN9_... just sounds confused to me. What is Gen9 if it is not PreXe?

The point is to ensure that platform specific register lists are 
constructed from the sublists that apply to that platform. Therefore the 
sublists are named for the platform they apply to.  Thus the gen8 list 
should only contain gen8 sub-lists. The Xe list can contain Xe sublists 
together with gen8 sublists that are still applicable. Those sublists 
have a COMMON_ prefix if they are expected to be multi-platform and 
don't if they are specific to their one named platform.

Note that you can't get hung on 'the end result looks odd' when only 
looking at the first step of a whole series of cleanups. This patch is 
specifically about moving the pre-Xe registers out of the COMMON_ define 
so that they are not used on Xe. It is not trying to fix up all the 
naming and other issues.

John.

>
> Either way, i rather not argue about variable names - so here is the Rb
> (since patch comment describes the issue and functionality seems correct):
> Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
index cf49188db6a6e..e0e793167d61b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
@@ -31,12 +31,14 @@ 
 	{ FORCEWAKE_MT,             0,      0, "FORCEWAKE" }
 
 #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,
 };
 
 static const struct __guc_mmio_reg_descr default_rc_class_regs[] = {