Re: [PATCH v2 10/17] drm/msm/a6xx: Update HFI definitions

[Akhil P Oommen]

On 3/31/2026 1:52 PM, Konrad Dybcio wrote:
> On 3/30/26 10:27 PM, Akhil P Oommen wrote:
>> On 3/30/2026 4:45 PM, Konrad Dybcio wrote:
>>> On 3/27/26 1:13 AM, Akhil P Oommen wrote:
>>>> Update the HFI definitions to support additional GMU based power
>>>> features.
>>>>
>>>> Signed-off-by: Akhil P Oommen <akhilpo@xxxxxxxxxxxxxxxx>
>>>> ---
>>>
>>> Whether you want to proceed with bitfields or not:
>>>
>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
>>
>> Thanks. I still feel it is bitfield layout is a 'bit' complicated.
>>
>> I did an experiment:
>>
>> #include <stdio.h>
>> #include <stdint.h>
>> #include <stddef.h>
>>
>> struct packed_u8 {
>>         uint16_t mitigation_time;
>>         uint8_t current_rating    : 6;
>>         uint8_t num_phases        : 6;
>>         uint8_t path_notification : 1;
>>         uint8_t extra_features    : 3;
>> } __packed;
>>
>>
>> void main() {
>>
>>         struct packed_u8 data = {
>>                  .mitigation_time  = 0xffff,
>>                  .current_rating   = 0x3f,  /* all 6 bits set */
>>                  .num_phases       = 0x3f,
>>                  .path_notification = 1,
>>                  .extra_features   = 0x7,
>>          };
>>
>>         printf("Akhil 0x%x\n", *((uint32_t *) &data));
>> }
>>
>> The output I got in Kaanapali is: Akhil 0x7f3fffff
>>
>> This means that the compiler inserted a padding between current_rating
>> and num_phases.
> 
> That's because __packed doesn't work outside the kernel - you're just
> creating a variable named __packed, so this is effectively the same as:
> 
> struct foo {
>         int a;
>         int b;
>         unsigned char c : 2;
> } __hotdog;
> 
> int main () {
>         printf("Akhil 0x%x\n", *((uint32_t *) &__hotdog));
> }
> 
> Outside the kernel tree, you need to use the full annoying __attribute__((foo))
> syntax:
> 
> include/linux/compiler_attributes.h:#define __packed                        __attribute__((__packed__))
> 
> with that changed, we get:
> 
> Akhil 0xffffffff
> 
> which is the expected behavior

Yeah, I can confirm this. The AAPCS doc which describes this is a bit
difficult to decipher for the case where a bitfield straddles between 2
storage units. Anyway the experiment confirms that your assumption is
correct.

Will address this separately when I post the CLX patches.

-Akhil.

> 
> Konrad