[PATCH v2] drm/xe/guc: Fix missing init value and add register order check

Dong, Zhanjun zhanjun.dong at intel.com
Tue Nov 5 23:14:21 UTC 2024



On 2024-11-05 5:04 p.m., Dixit, Ashutosh wrote:
> On Tue, 05 Nov 2024 12:54:49 -0800, Dong, Zhanjun wrote:
>>>> +
> 
> Hi Zhanjun,
> 
>>>> + case REG_64BIT_HI_DW: {
>>> alan: nit: now that you have moved to a switch statement, is this { really required?
>> Few concerns:
>> 1. Have {} here will make it works on most compilers and ANSIC settings.
>> 2. Local variable "u64 value_qw" will be limited within this case
>> statement, I don't want it to be referenced on other case statement.
> 
> Do you have a compiler where it doesn't compile? Looks like you can declare
> the variable following case without the brace, compiles at least with gcc.
> 
I know LLVM/Clang is used for Chrome/Android, but I haven't compile it.
Without brace. vscode with the Clang C syntax parser will report error:

Expected expressionclang(expected_expression)

So I guess it might cause problem for Chrome?

> Also, if you need the brace, the 'break' at the end should be inside the
> brace. This is used in a few places in xe, you can see how it's done :)

Will move it inside.

Regards,
Zhanjun Dong

> 
> Thanks.
> --
> Ashutosh



More information about the Intel-xe mailing list