[PATCH][next] drm/amd/pm: Avoid multiple -Wflex-array-member-not-at-end warnings

[PATCH][next] drm/amd/pm: Avoid multiple -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are
getting ready to enable it, globally.

So, in order to avoid ending up with a flexible-array member in the
middle of other structs, we use the `struct_group_tagged()` helper
to create a new tagged `struct NISLANDS_SMC_SWSTATE_HDR` (and `struct
SISLANDS_SMC_SWSTATE_HDR`). This structures group together all the
members of the flexible `struct NISLANDS_SMC_SWSTATE` (and `struct
SISLANDS_SMC_SWSTATE`) except the flexible array.

As a result, the array is effectively separated from the rest of the
members without modifying the memory layout of the flexible structure.
We then change the type of the middle struct members currently causing
trouble from `struct NISLANDS_SMC_SWSTATE` to `struct
NISLANDS_SMC_SWSTATE_HDR` (and from `struct SISLANDS_SMC_SWSTATE` to
`struct SISLANDS_SMC_SWSTATE_HDR`).

We also want to ensure that when new members need to be added to the
flexible structure, they are always included within the newly created
tagged struct. For this, we use `static_assert()`. This ensures that
the memory layout for both the flexible structure and the new tagged
struct is the same after any changes.

This approach avoids having to implement `struct NISLANDS_SMC_SWSTATE_HDR`
(and `struct SISLANDS_SMC_SWSTATE_HDR`) as a completely separate structure,
thus preventing having to maintain two independent but basically identical
structures, closing the door to potential bugs in the future.

We also use `container_of()` whenever we need to retrieve a pointer to
the flexible structure, through which we can access the flexible-array
member, if necessary.

So, with this changes, fix the following warnings:

drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/sislands_smc.h:218:49: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:819:41: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:818:41: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:817:41: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:816:41: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c    |  7 ++++--
 drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h    | 23 +++++++++++--------
 .../gpu/drm/amd/pm/legacy-dpm/sislands_smc.h  | 15 ++++++++----
 3 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
index a87dcf0974bc..2c9d473d122f 100644
--- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
+++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
@@ -5234,7 +5234,8 @@ static int si_init_smc_table(struct amdgpu_device *adev)
 
 	table->driverState.flags = table->initialState.flags;
 	table->driverState.levelCount = table->initialState.levelCount;
-	table->driverState.levels[0] = table->initialState.level;
+	container_of(&table->driverState, SISLANDS_SMC_SWSTATE, __hdr)->levels[0] =
+								table->initialState.level;
 
 	ret = si_do_program_memory_timing_parameters(adev, amdgpu_boot_state,
 						     SISLANDS_INITIAL_STATE_ARB_INDEX);
@@ -5755,7 +5756,9 @@ static int si_upload_sw_state(struct amdgpu_device *adev,
 	int ret;
 	u32 address = si_pi->state_table_start +
 		offsetof(SISLANDS_SMC_STATETABLE, driverState);
-	SISLANDS_SMC_SWSTATE *smc_state = &si_pi->smc_statetable.driverState;
+	SISLANDS_SMC_SWSTATE *smc_state =
+		container_of(&si_pi->smc_statetable.driverState,
+			     SISLANDS_SMC_SWSTATE, __hdr);
 	size_t state_size = struct_size(smc_state, levels,
 					new_state->performance_level_count);
 	memset(smc_state, 0, state_size);
diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h
index 11cb7874a6bb..62530f89ebdf 100644
--- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h
+++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h
@@ -784,12 +784,17 @@ typedef struct NISLANDS_SMC_HW_PERFORMANCE_LEVEL NISLANDS_SMC_HW_PERFORMANCE_LEV
 
 struct NISLANDS_SMC_SWSTATE
 {
-    uint8_t                             flags;
-    uint8_t                             levelCount;
-    uint8_t                             padding2;
-    uint8_t                             padding3;
-    NISLANDS_SMC_HW_PERFORMANCE_LEVEL   levels[];
+	/* New members MUST be added within the struct_group() macro below. */
+	struct_group_tagged(NISLANDS_SMC_SWSTATE_HDR, __hdr,
+		uint8_t                             flags;
+		uint8_t                             levelCount;
+		uint8_t                             padding2;
+		uint8_t                             padding3;
+	);
+	NISLANDS_SMC_HW_PERFORMANCE_LEVEL   levels[];
 };
+static_assert(offsetof(struct NISLANDS_SMC_SWSTATE, levels) == sizeof(struct NISLANDS_SMC_SWSTATE_HDR),
+	      "struct member likely outside of struct_group_tagged()");
 
 typedef struct NISLANDS_SMC_SWSTATE NISLANDS_SMC_SWSTATE;
 
@@ -813,10 +818,10 @@ struct NISLANDS_SMC_STATETABLE
     uint32_t                            lowSMIO[NISLANDS_MAX_NO_VREG_STEPS];
     NISLANDS_SMC_VOLTAGEMASKTABLE       voltageMaskTable;
     PP_NIslands_DPM2Parameters          dpm2Params;
-    NISLANDS_SMC_SWSTATE                initialState;
-    NISLANDS_SMC_SWSTATE                ACPIState;
-    NISLANDS_SMC_SWSTATE                ULVState;
-    NISLANDS_SMC_SWSTATE                driverState;
+    struct NISLANDS_SMC_SWSTATE_HDR		initialState;
+    struct NISLANDS_SMC_SWSTATE_HDR		ACPIState;
+    struct NISLANDS_SMC_SWSTATE_HDR		ULVState;
+    struct NISLANDS_SMC_SWSTATE_HDR		driverState;
     NISLANDS_SMC_HW_PERFORMANCE_LEVEL   dpmLevels[NISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE - 1];
 };
 
diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h b/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h
index 90ec411c5029..1711e3e35e80 100644
--- a/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h
+++ b/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h
@@ -172,12 +172,17 @@ struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL {
 typedef struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL SISLANDS_SMC_HW_PERFORMANCE_LEVEL;
 
 struct SISLANDS_SMC_SWSTATE {
-	uint8_t                             flags;
-	uint8_t                             levelCount;
-	uint8_t                             padding2;
-	uint8_t                             padding3;
+	/* New members MUST be added within the struct_group() macro below. */
+	struct_group_tagged(SISLANDS_SMC_SWSTATE_HDR, __hdr,
+		uint8_t                             flags;
+		uint8_t                             levelCount;
+		uint8_t                             padding2;
+		uint8_t                             padding3;
+	);
 	SISLANDS_SMC_HW_PERFORMANCE_LEVEL   levels[];
 };
+static_assert(offsetof(struct SISLANDS_SMC_SWSTATE, levels) == sizeof(struct SISLANDS_SMC_SWSTATE_HDR),
+	      "struct member likely outside of struct_group_tagged()");
 
 typedef struct SISLANDS_SMC_SWSTATE SISLANDS_SMC_SWSTATE;
 
@@ -215,7 +220,7 @@ struct SISLANDS_SMC_STATETABLE {
 	struct SISLANDS_SMC_SWSTATE_SINGLE	initialState;
 	struct SISLANDS_SMC_SWSTATE_SINGLE	ACPIState;
 	struct SISLANDS_SMC_SWSTATE_SINGLE	ULVState;
-	SISLANDS_SMC_SWSTATE			driverState;
+	struct SISLANDS_SMC_SWSTATE_HDR		driverState;
 	SISLANDS_SMC_HW_PERFORMANCE_LEVEL	dpmLevels[SISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE];
 };
 
-- 
2.43.0

Re: [PATCH][next] drm/amd/pm: Avoid multiple -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

Hi all,

Friendly ping: who can take this, please? :)

Thanks!
--
Gustavo

On 14/02/25 18:48, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> getting ready to enable it, globally.
> 
> So, in order to avoid ending up with a flexible-array member in the
> middle of other structs, we use the `struct_group_tagged()` helper
> to create a new tagged `struct NISLANDS_SMC_SWSTATE_HDR` (and `struct
> SISLANDS_SMC_SWSTATE_HDR`). This structures group together all the
> members of the flexible `struct NISLANDS_SMC_SWSTATE` (and `struct
> SISLANDS_SMC_SWSTATE`) except the flexible array.
> 
> As a result, the array is effectively separated from the rest of the
> members without modifying the memory layout of the flexible structure.
> We then change the type of the middle struct members currently causing
> trouble from `struct NISLANDS_SMC_SWSTATE` to `struct
> NISLANDS_SMC_SWSTATE_HDR` (and from `struct SISLANDS_SMC_SWSTATE` to
> `struct SISLANDS_SMC_SWSTATE_HDR`).
> 
> We also want to ensure that when new members need to be added to the
> flexible structure, they are always included within the newly created
> tagged struct. For this, we use `static_assert()`. This ensures that
> the memory layout for both the flexible structure and the new tagged
> struct is the same after any changes.
> 
> This approach avoids having to implement `struct NISLANDS_SMC_SWSTATE_HDR`
> (and `struct SISLANDS_SMC_SWSTATE_HDR`) as a completely separate structure,
> thus preventing having to maintain two independent but basically identical
> structures, closing the door to potential bugs in the future.
> 
> We also use `container_of()` whenever we need to retrieve a pointer to
> the flexible structure, through which we can access the flexible-array
> member, if necessary.
> 
> So, with this changes, fix the following warnings:
> 
> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/sislands_smc.h:218:49: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:819:41: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:818:41: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:817:41: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:816:41: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>   drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c    |  7 ++++--
>   drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h    | 23 +++++++++++--------
>   .../gpu/drm/amd/pm/legacy-dpm/sislands_smc.h  | 15 ++++++++----
>   3 files changed, 29 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> index a87dcf0974bc..2c9d473d122f 100644
> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> @@ -5234,7 +5234,8 @@ static int si_init_smc_table(struct amdgpu_device *adev)
>   
>   	table->driverState.flags = table->initialState.flags;
>   	table->driverState.levelCount = table->initialState.levelCount;
> -	table->driverState.levels[0] = table->initialState.level;
> +	container_of(&table->driverState, SISLANDS_SMC_SWSTATE, __hdr)->levels[0] =
> +								table->initialState.level;
>   
>   	ret = si_do_program_memory_timing_parameters(adev, amdgpu_boot_state,
>   						     SISLANDS_INITIAL_STATE_ARB_INDEX);
> @@ -5755,7 +5756,9 @@ static int si_upload_sw_state(struct amdgpu_device *adev,
>   	int ret;
>   	u32 address = si_pi->state_table_start +
>   		offsetof(SISLANDS_SMC_STATETABLE, driverState);
> -	SISLANDS_SMC_SWSTATE *smc_state = &si_pi->smc_statetable.driverState;
> +	SISLANDS_SMC_SWSTATE *smc_state =
> +		container_of(&si_pi->smc_statetable.driverState,
> +			     SISLANDS_SMC_SWSTATE, __hdr);
>   	size_t state_size = struct_size(smc_state, levels,
>   					new_state->performance_level_count);
>   	memset(smc_state, 0, state_size);
> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h
> index 11cb7874a6bb..62530f89ebdf 100644
> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h
> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h
> @@ -784,12 +784,17 @@ typedef struct NISLANDS_SMC_HW_PERFORMANCE_LEVEL NISLANDS_SMC_HW_PERFORMANCE_LEV
>   
>   struct NISLANDS_SMC_SWSTATE
>   {
> -    uint8_t                             flags;
> -    uint8_t                             levelCount;
> -    uint8_t                             padding2;
> -    uint8_t                             padding3;
> -    NISLANDS_SMC_HW_PERFORMANCE_LEVEL   levels[];
> +	/* New members MUST be added within the struct_group() macro below. */
> +	struct_group_tagged(NISLANDS_SMC_SWSTATE_HDR, __hdr,
> +		uint8_t                             flags;
> +		uint8_t                             levelCount;
> +		uint8_t                             padding2;
> +		uint8_t                             padding3;
> +	);
> +	NISLANDS_SMC_HW_PERFORMANCE_LEVEL   levels[];
>   };
> +static_assert(offsetof(struct NISLANDS_SMC_SWSTATE, levels) == sizeof(struct NISLANDS_SMC_SWSTATE_HDR),
> +	      "struct member likely outside of struct_group_tagged()");
>   
>   typedef struct NISLANDS_SMC_SWSTATE NISLANDS_SMC_SWSTATE;
>   
> @@ -813,10 +818,10 @@ struct NISLANDS_SMC_STATETABLE
>       uint32_t                            lowSMIO[NISLANDS_MAX_NO_VREG_STEPS];
>       NISLANDS_SMC_VOLTAGEMASKTABLE       voltageMaskTable;
>       PP_NIslands_DPM2Parameters          dpm2Params;
> -    NISLANDS_SMC_SWSTATE                initialState;
> -    NISLANDS_SMC_SWSTATE                ACPIState;
> -    NISLANDS_SMC_SWSTATE                ULVState;
> -    NISLANDS_SMC_SWSTATE                driverState;
> +    struct NISLANDS_SMC_SWSTATE_HDR		initialState;
> +    struct NISLANDS_SMC_SWSTATE_HDR		ACPIState;
> +    struct NISLANDS_SMC_SWSTATE_HDR		ULVState;
> +    struct NISLANDS_SMC_SWSTATE_HDR		driverState;
>       NISLANDS_SMC_HW_PERFORMANCE_LEVEL   dpmLevels[NISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE - 1];
>   };
>   
> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h b/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h
> index 90ec411c5029..1711e3e35e80 100644
> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h
> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h
> @@ -172,12 +172,17 @@ struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL {
>   typedef struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL SISLANDS_SMC_HW_PERFORMANCE_LEVEL;
>   
>   struct SISLANDS_SMC_SWSTATE {
> -	uint8_t                             flags;
> -	uint8_t                             levelCount;
> -	uint8_t                             padding2;
> -	uint8_t                             padding3;
> +	/* New members MUST be added within the struct_group() macro below. */
> +	struct_group_tagged(SISLANDS_SMC_SWSTATE_HDR, __hdr,
> +		uint8_t                             flags;
> +		uint8_t                             levelCount;
> +		uint8_t                             padding2;
> +		uint8_t                             padding3;
> +	);
>   	SISLANDS_SMC_HW_PERFORMANCE_LEVEL   levels[];
>   };
> +static_assert(offsetof(struct SISLANDS_SMC_SWSTATE, levels) == sizeof(struct SISLANDS_SMC_SWSTATE_HDR),
> +	      "struct member likely outside of struct_group_tagged()");
>   
>   typedef struct SISLANDS_SMC_SWSTATE SISLANDS_SMC_SWSTATE;
>   
> @@ -215,7 +220,7 @@ struct SISLANDS_SMC_STATETABLE {
>   	struct SISLANDS_SMC_SWSTATE_SINGLE	initialState;
>   	struct SISLANDS_SMC_SWSTATE_SINGLE	ACPIState;
>   	struct SISLANDS_SMC_SWSTATE_SINGLE	ULVState;
> -	SISLANDS_SMC_SWSTATE			driverState;
> +	struct SISLANDS_SMC_SWSTATE_HDR		driverState;
>   	SISLANDS_SMC_HW_PERFORMANCE_LEVEL	dpmLevels[SISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE];
>   };
>   

Re: [PATCH][next] drm/amd/pm: Avoid multiple -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

Hi all,

Friendly ping (second one): who can take this patch, please? 🙂

Thanks!
-Gustavo

On 11/03/25 02:10, Gustavo A. R. Silva wrote:
> Hi all,
> 
> Friendly ping: who can take this, please? :)
> 
> Thanks!
> -- 
> Gustavo
> 
> On 14/02/25 18:48, Gustavo A. R. Silva wrote:
>> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
>> getting ready to enable it, globally.
>>
>> So, in order to avoid ending up with a flexible-array member in the
>> middle of other structs, we use the `struct_group_tagged()` helper
>> to create a new tagged `struct NISLANDS_SMC_SWSTATE_HDR` (and `struct
>> SISLANDS_SMC_SWSTATE_HDR`). This structures group together all the
>> members of the flexible `struct NISLANDS_SMC_SWSTATE` (and `struct
>> SISLANDS_SMC_SWSTATE`) except the flexible array.
>>
>> As a result, the array is effectively separated from the rest of the
>> members without modifying the memory layout of the flexible structure.
>> We then change the type of the middle struct members currently causing
>> trouble from `struct NISLANDS_SMC_SWSTATE` to `struct
>> NISLANDS_SMC_SWSTATE_HDR` (and from `struct SISLANDS_SMC_SWSTATE` to
>> `struct SISLANDS_SMC_SWSTATE_HDR`).
>>
>> We also want to ensure that when new members need to be added to the
>> flexible structure, they are always included within the newly created
>> tagged struct. For this, we use `static_assert()`. This ensures that
>> the memory layout for both the flexible structure and the new tagged
>> struct is the same after any changes.
>>
>> This approach avoids having to implement `struct NISLANDS_SMC_SWSTATE_HDR`
>> (and `struct SISLANDS_SMC_SWSTATE_HDR`) as a completely separate structure,
>> thus preventing having to maintain two independent but basically identical
>> structures, closing the door to potential bugs in the future.
>>
>> We also use `container_of()` whenever we need to retrieve a pointer to
>> the flexible structure, through which we can access the flexible-array
>> member, if necessary.
>>
>> So, with this changes, fix the following warnings:
>>
>> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/sislands_smc.h:218:49: warning: structure containing a flexible array member is not at the end of another 
>> structure [-Wflex-array-member-not-at-end]
>> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:819:41: warning: structure containing a flexible array member is not at the end of another structure [- 
>> Wflex-array-member-not-at-end]
>> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:818:41: warning: structure containing a flexible array member is not at the end of another structure [- 
>> Wflex-array-member-not-at-end]
>> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:817:41: warning: structure containing a flexible array member is not at the end of another structure [- 
>> Wflex-array-member-not-at-end]
>> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:816:41: warning: structure containing a flexible array member is not at the end of another structure [- 
>> Wflex-array-member-not-at-end]
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>> ---
>>   drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c    |  7 ++++--
>>   drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h    | 23 +++++++++++--------
>>   .../gpu/drm/amd/pm/legacy-dpm/sislands_smc.h  | 15 ++++++++----
>>   3 files changed, 29 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
>> index a87dcf0974bc..2c9d473d122f 100644
>> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
>> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
>> @@ -5234,7 +5234,8 @@ static int si_init_smc_table(struct amdgpu_device *adev)
>>       table->driverState.flags = table->initialState.flags;
>>       table->driverState.levelCount = table->initialState.levelCount;
>> -    table->driverState.levels[0] = table->initialState.level;
>> +    container_of(&table->driverState, SISLANDS_SMC_SWSTATE, __hdr)->levels[0] =
>> +                                table->initialState.level;
>>       ret = si_do_program_memory_timing_parameters(adev, amdgpu_boot_state,
>>                                SISLANDS_INITIAL_STATE_ARB_INDEX);
>> @@ -5755,7 +5756,9 @@ static int si_upload_sw_state(struct amdgpu_device *adev,
>>       int ret;
>>       u32 address = si_pi->state_table_start +
>>           offsetof(SISLANDS_SMC_STATETABLE, driverState);
>> -    SISLANDS_SMC_SWSTATE *smc_state = &si_pi->smc_statetable.driverState;
>> +    SISLANDS_SMC_SWSTATE *smc_state =
>> +        container_of(&si_pi->smc_statetable.driverState,
>> +                 SISLANDS_SMC_SWSTATE, __hdr);
>>       size_t state_size = struct_size(smc_state, levels,
>>                       new_state->performance_level_count);
>>       memset(smc_state, 0, state_size);
>> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h
>> index 11cb7874a6bb..62530f89ebdf 100644
>> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h
>> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h
>> @@ -784,12 +784,17 @@ typedef struct NISLANDS_SMC_HW_PERFORMANCE_LEVEL NISLANDS_SMC_HW_PERFORMANCE_LEV
>>   struct NISLANDS_SMC_SWSTATE
>>   {
>> -    uint8_t                             flags;
>> -    uint8_t                             levelCount;
>> -    uint8_t                             padding2;
>> -    uint8_t                             padding3;
>> -    NISLANDS_SMC_HW_PERFORMANCE_LEVEL   levels[];
>> +    /* New members MUST be added within the struct_group() macro below. */
>> +    struct_group_tagged(NISLANDS_SMC_SWSTATE_HDR, __hdr,
>> +        uint8_t                             flags;
>> +        uint8_t                             levelCount;
>> +        uint8_t                             padding2;
>> +        uint8_t                             padding3;
>> +    );
>> +    NISLANDS_SMC_HW_PERFORMANCE_LEVEL   levels[];
>>   };
>> +static_assert(offsetof(struct NISLANDS_SMC_SWSTATE, levels) == sizeof(struct NISLANDS_SMC_SWSTATE_HDR),
>> +          "struct member likely outside of struct_group_tagged()");
>>   typedef struct NISLANDS_SMC_SWSTATE NISLANDS_SMC_SWSTATE;
>> @@ -813,10 +818,10 @@ struct NISLANDS_SMC_STATETABLE
>>       uint32_t                            lowSMIO[NISLANDS_MAX_NO_VREG_STEPS];
>>       NISLANDS_SMC_VOLTAGEMASKTABLE       voltageMaskTable;
>>       PP_NIslands_DPM2Parameters          dpm2Params;
>> -    NISLANDS_SMC_SWSTATE                initialState;
>> -    NISLANDS_SMC_SWSTATE                ACPIState;
>> -    NISLANDS_SMC_SWSTATE                ULVState;
>> -    NISLANDS_SMC_SWSTATE                driverState;
>> +    struct NISLANDS_SMC_SWSTATE_HDR        initialState;
>> +    struct NISLANDS_SMC_SWSTATE_HDR        ACPIState;
>> +    struct NISLANDS_SMC_SWSTATE_HDR        ULVState;
>> +    struct NISLANDS_SMC_SWSTATE_HDR        driverState;
>>       NISLANDS_SMC_HW_PERFORMANCE_LEVEL   dpmLevels[NISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE - 1];
>>   };
>> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h b/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h
>> index 90ec411c5029..1711e3e35e80 100644
>> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h
>> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h
>> @@ -172,12 +172,17 @@ struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL {
>>   typedef struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL SISLANDS_SMC_HW_PERFORMANCE_LEVEL;
>>   struct SISLANDS_SMC_SWSTATE {
>> -    uint8_t                             flags;
>> -    uint8_t                             levelCount;
>> -    uint8_t                             padding2;
>> -    uint8_t                             padding3;
>> +    /* New members MUST be added within the struct_group() macro below. */
>> +    struct_group_tagged(SISLANDS_SMC_SWSTATE_HDR, __hdr,
>> +        uint8_t                             flags;
>> +        uint8_t                             levelCount;
>> +        uint8_t                             padding2;
>> +        uint8_t                             padding3;
>> +    );
>>       SISLANDS_SMC_HW_PERFORMANCE_LEVEL   levels[];
>>   };
>> +static_assert(offsetof(struct SISLANDS_SMC_SWSTATE, levels) == sizeof(struct SISLANDS_SMC_SWSTATE_HDR),
>> +          "struct member likely outside of struct_group_tagged()");
>>   typedef struct SISLANDS_SMC_SWSTATE SISLANDS_SMC_SWSTATE;
>> @@ -215,7 +220,7 @@ struct SISLANDS_SMC_STATETABLE {
>>       struct SISLANDS_SMC_SWSTATE_SINGLE    initialState;
>>       struct SISLANDS_SMC_SWSTATE_SINGLE    ACPIState;
>>       struct SISLANDS_SMC_SWSTATE_SINGLE    ULVState;
>> -    SISLANDS_SMC_SWSTATE            driverState;
>> +    struct SISLANDS_SMC_SWSTATE_HDR        driverState;
>>       SISLANDS_SMC_HW_PERFORMANCE_LEVEL    dpmLevels[SISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE];
>>   };
> 

Re: [PATCH][next] drm/amd/pm: Avoid multiple -Wflex-array-member-not-at-end warnings

[Alex Deucher]

Can you resend, I can't seem to find the original emails.
Additionally, all of the NISLANDS structures are unused in amdgpu, so
those could be removed.

Alex
On Wed, Apr 16, 2025 at 12:48 AM Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
>
> Hi all,
>
> Friendly ping (second one): who can take this patch, please? 🙂
>
> Thanks!
> -Gustavo
>
> On 11/03/25 02:10, Gustavo A. R. Silva wrote:
> > Hi all,
> >
> > Friendly ping: who can take this, please? :)
> >
> > Thanks!
> > --
> > Gustavo
> >
> > On 14/02/25 18:48, Gustavo A. R. Silva wrote:
> >> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> >> getting ready to enable it, globally.
> >>
> >> So, in order to avoid ending up with a flexible-array member in the
> >> middle of other structs, we use the `struct_group_tagged()` helper
> >> to create a new tagged `struct NISLANDS_SMC_SWSTATE_HDR` (and `struct
> >> SISLANDS_SMC_SWSTATE_HDR`). This structures group together all the
> >> members of the flexible `struct NISLANDS_SMC_SWSTATE` (and `struct
> >> SISLANDS_SMC_SWSTATE`) except the flexible array.
> >>
> >> As a result, the array is effectively separated from the rest of the
> >> members without modifying the memory layout of the flexible structure.
> >> We then change the type of the middle struct members currently causing
> >> trouble from `struct NISLANDS_SMC_SWSTATE` to `struct
> >> NISLANDS_SMC_SWSTATE_HDR` (and from `struct SISLANDS_SMC_SWSTATE` to
> >> `struct SISLANDS_SMC_SWSTATE_HDR`).
> >>
> >> We also want to ensure that when new members need to be added to the
> >> flexible structure, they are always included within the newly created
> >> tagged struct. For this, we use `static_assert()`. This ensures that
> >> the memory layout for both the flexible structure and the new tagged
> >> struct is the same after any changes.
> >>
> >> This approach avoids having to implement `struct NISLANDS_SMC_SWSTATE_HDR`
> >> (and `struct SISLANDS_SMC_SWSTATE_HDR`) as a completely separate structure,
> >> thus preventing having to maintain two independent but basically identical
> >> structures, closing the door to potential bugs in the future.
> >>
> >> We also use `container_of()` whenever we need to retrieve a pointer to
> >> the flexible structure, through which we can access the flexible-array
> >> member, if necessary.
> >>
> >> So, with this changes, fix the following warnings:
> >>
> >> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/sislands_smc.h:218:49: warning: structure containing a flexible array member is not at the end of another
> >> structure [-Wflex-array-member-not-at-end]
> >> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:819:41: warning: structure containing a flexible array member is not at the end of another structure [-
> >> Wflex-array-member-not-at-end]
> >> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:818:41: warning: structure containing a flexible array member is not at the end of another structure [-
> >> Wflex-array-member-not-at-end]
> >> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:817:41: warning: structure containing a flexible array member is not at the end of another structure [-
> >> Wflex-array-member-not-at-end]
> >> drivers/gpu/drm/amd/amdgpu/../pm/legacy-dpm/si_dpm.h:816:41: warning: structure containing a flexible array member is not at the end of another structure [-
> >> Wflex-array-member-not-at-end]
> >>
> >> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> >> ---
> >>   drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c    |  7 ++++--
> >>   drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h    | 23 +++++++++++--------
> >>   .../gpu/drm/amd/pm/legacy-dpm/sislands_smc.h  | 15 ++++++++----
> >>   3 files changed, 29 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> >> index a87dcf0974bc..2c9d473d122f 100644
> >> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> >> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> >> @@ -5234,7 +5234,8 @@ static int si_init_smc_table(struct amdgpu_device *adev)
> >>       table->driverState.flags = table->initialState.flags;
> >>       table->driverState.levelCount = table->initialState.levelCount;
> >> -    table->driverState.levels[0] = table->initialState.level;
> >> +    container_of(&table->driverState, SISLANDS_SMC_SWSTATE, __hdr)->levels[0] =
> >> +                                table->initialState.level;
> >>       ret = si_do_program_memory_timing_parameters(adev, amdgpu_boot_state,
> >>                                SISLANDS_INITIAL_STATE_ARB_INDEX);
> >> @@ -5755,7 +5756,9 @@ static int si_upload_sw_state(struct amdgpu_device *adev,
> >>       int ret;
> >>       u32 address = si_pi->state_table_start +
> >>           offsetof(SISLANDS_SMC_STATETABLE, driverState);
> >> -    SISLANDS_SMC_SWSTATE *smc_state = &si_pi->smc_statetable.driverState;
> >> +    SISLANDS_SMC_SWSTATE *smc_state =
> >> +        container_of(&si_pi->smc_statetable.driverState,
> >> +                 SISLANDS_SMC_SWSTATE, __hdr);
> >>       size_t state_size = struct_size(smc_state, levels,
> >>                       new_state->performance_level_count);
> >>       memset(smc_state, 0, state_size);
> >> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h
> >> index 11cb7874a6bb..62530f89ebdf 100644
> >> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h
> >> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.h
> >> @@ -784,12 +784,17 @@ typedef struct NISLANDS_SMC_HW_PERFORMANCE_LEVEL NISLANDS_SMC_HW_PERFORMANCE_LEV
> >>   struct NISLANDS_SMC_SWSTATE
> >>   {
> >> -    uint8_t                             flags;
> >> -    uint8_t                             levelCount;
> >> -    uint8_t                             padding2;
> >> -    uint8_t                             padding3;
> >> -    NISLANDS_SMC_HW_PERFORMANCE_LEVEL   levels[];
> >> +    /* New members MUST be added within the struct_group() macro below. */
> >> +    struct_group_tagged(NISLANDS_SMC_SWSTATE_HDR, __hdr,
> >> +        uint8_t                             flags;
> >> +        uint8_t                             levelCount;
> >> +        uint8_t                             padding2;
> >> +        uint8_t                             padding3;
> >> +    );
> >> +    NISLANDS_SMC_HW_PERFORMANCE_LEVEL   levels[];
> >>   };
> >> +static_assert(offsetof(struct NISLANDS_SMC_SWSTATE, levels) == sizeof(struct NISLANDS_SMC_SWSTATE_HDR),
> >> +          "struct member likely outside of struct_group_tagged()");
> >>   typedef struct NISLANDS_SMC_SWSTATE NISLANDS_SMC_SWSTATE;
> >> @@ -813,10 +818,10 @@ struct NISLANDS_SMC_STATETABLE
> >>       uint32_t                            lowSMIO[NISLANDS_MAX_NO_VREG_STEPS];
> >>       NISLANDS_SMC_VOLTAGEMASKTABLE       voltageMaskTable;
> >>       PP_NIslands_DPM2Parameters          dpm2Params;
> >> -    NISLANDS_SMC_SWSTATE                initialState;
> >> -    NISLANDS_SMC_SWSTATE                ACPIState;
> >> -    NISLANDS_SMC_SWSTATE                ULVState;
> >> -    NISLANDS_SMC_SWSTATE                driverState;
> >> +    struct NISLANDS_SMC_SWSTATE_HDR        initialState;
> >> +    struct NISLANDS_SMC_SWSTATE_HDR        ACPIState;
> >> +    struct NISLANDS_SMC_SWSTATE_HDR        ULVState;
> >> +    struct NISLANDS_SMC_SWSTATE_HDR        driverState;
> >>       NISLANDS_SMC_HW_PERFORMANCE_LEVEL   dpmLevels[NISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE - 1];
> >>   };
> >> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h b/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h
> >> index 90ec411c5029..1711e3e35e80 100644
> >> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h
> >> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/sislands_smc.h
> >> @@ -172,12 +172,17 @@ struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL {
> >>   typedef struct SISLANDS_SMC_HW_PERFORMANCE_LEVEL SISLANDS_SMC_HW_PERFORMANCE_LEVEL;
> >>   struct SISLANDS_SMC_SWSTATE {
> >> -    uint8_t                             flags;
> >> -    uint8_t                             levelCount;
> >> -    uint8_t                             padding2;
> >> -    uint8_t                             padding3;
> >> +    /* New members MUST be added within the struct_group() macro below. */
> >> +    struct_group_tagged(SISLANDS_SMC_SWSTATE_HDR, __hdr,
> >> +        uint8_t                             flags;
> >> +        uint8_t                             levelCount;
> >> +        uint8_t                             padding2;
> >> +        uint8_t                             padding3;
> >> +    );
> >>       SISLANDS_SMC_HW_PERFORMANCE_LEVEL   levels[];
> >>   };
> >> +static_assert(offsetof(struct SISLANDS_SMC_SWSTATE, levels) == sizeof(struct SISLANDS_SMC_SWSTATE_HDR),
> >> +          "struct member likely outside of struct_group_tagged()");
> >>   typedef struct SISLANDS_SMC_SWSTATE SISLANDS_SMC_SWSTATE;
> >> @@ -215,7 +220,7 @@ struct SISLANDS_SMC_STATETABLE {
> >>       struct SISLANDS_SMC_SWSTATE_SINGLE    initialState;
> >>       struct SISLANDS_SMC_SWSTATE_SINGLE    ACPIState;
> >>       struct SISLANDS_SMC_SWSTATE_SINGLE    ULVState;
> >> -    SISLANDS_SMC_SWSTATE            driverState;
> >> +    struct SISLANDS_SMC_SWSTATE_HDR        driverState;
> >>       SISLANDS_SMC_HW_PERFORMANCE_LEVEL    dpmLevels[SISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE];
> >>   };
> >
>

Re: [PATCH][next] drm/amd/pm: Avoid multiple -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

On 16/04/25 09:04, Alex Deucher wrote:
> Can you resend, I can't seem to find the original emails.
> Additionally, all of the NISLANDS structures are unused in amdgpu, so
> those could be removed.

Okay, I'll take a look.

Thanks

-Gustavo

Re: [PATCH][next] drm/amd/pm: Avoid multiple -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

Hi!

On 22/04/25 23:58, Gustavo A. R. Silva wrote:
> 
> 
> On 16/04/25 09:04, Alex Deucher wrote:
>> Can you resend, I can't seem to find the original emails.
>> Additionally, all of the NISLANDS structures are unused in amdgpu, so
>> those could be removed.

I'm taking a look at this, and it seems that those NISLANDS structs are actually
needed in amdgpu code. For instance, `struct si_power_info` contains a member
of the type of `struct ni_power_info`, and this latter struct contains a
member of the type of `NISLANDS_SMC_STATETABLE`, thus `NISLANDS_SMC_SWSTATE`
and `NISLANDS_SMC_HW_PERFORMANCE_LEVEL` are needed, and so on.

So, it seems that all those structs should stay. What do you think?

Thanks!
-Gustavo

Re: [PATCH][next] drm/amd/pm: Avoid multiple -Wflex-array-member-not-at-end warnings

[Alex Deucher]

On Wed, Aug 13, 2025 at 1:12 AM Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
>
> Hi!
>
> On 22/04/25 23:58, Gustavo A. R. Silva wrote:
> >
> >
> > On 16/04/25 09:04, Alex Deucher wrote:
> >> Can you resend, I can't seem to find the original emails.
> >> Additionally, all of the NISLANDS structures are unused in amdgpu, so
> >> those could be removed.
>
> I'm taking a look at this, and it seems that those NISLANDS structs are actually
> needed in amdgpu code. For instance, `struct si_power_info` contains a member
> of the type of `struct ni_power_info`, and this latter struct contains a
> member of the type of `NISLANDS_SMC_STATETABLE`, thus `NISLANDS_SMC_SWSTATE`
> and `NISLANDS_SMC_HW_PERFORMANCE_LEVEL` are needed, and so on.
>
> So, it seems that all those structs should stay. What do you think?

They are not used for programming the hardware.  They were just
inherited from radeon.  All of the NI SMC stuff can be dropped.

Alex

>
> Thanks!
> -Gustavo