[PATCH] drm/panel: ilitek-ili9881c: Use u8 for lane count

[Neil Armstrong]

On 21/06/2025 18:03, Marek Vasut wrote:
> On 6/16/25 6:26 PM, Neil Armstrong wrote:
>> On 16/06/2025 18:05, Marek Vasut wrote:
>>> On 6/16/25 1:45 PM, Neil Armstrong wrote:
>>>> On 13/06/2025 12:54, Marek Vasut wrote:
>>>>> On 6/13/25 11:29 AM, Neil Armstrong wrote:
>>>>>> On 12/06/2025 01:49, Marek Vasut wrote:
>>>>>>> Use u8 to hold lane count in struct ili9881c_desc {} to avoid
>>>>>>> alignment gap between default_address_mode and lanes members.
>>>>>>> The ili9881c controller can only operate up to 4 DSI lanes, so
>>>>>>> there is no chance this value can ever be larger than 4. No
>>>>>>> functional change.
>>>>>>
>>>>>> The u8 will still take at least 4 bytes and cpu will still
>>>>>> do at least a 32bit memory access, so there's no point to change
>>>>>> it to u8.
>>>>> Assuming this layout:
>>>>>
>>>>>    40 struct ili9881c_desc {
>>>>>    41         const struct ili9881c_instr *init;
>>>>>    42         const size_t init_length;
>>>>>    43         const struct drm_display_mode *mode;
>>>>>    44         const unsigned long mode_flags;
>>>>>    45         u8 default_address_mode;
>>>>>    46         u8 lanes;
>>>>>    47 };
>>>>>
>>>>> I wrote a quick test:
>>>>>
>>>>> $ cat test.c
>>>>> #include <stdio.h>
>>>>> #include <stdint.h>
>>>>>
>>>>> struct foo {
>>>>>      void *a;
>>>>>      size_t b;
>>>>>      void *c;
>>>>>      unsigned long d;
>>>>>
>>>>>      uint8_t x;
>>>>>      unsigned long y; // ~= lanes
>>>>> };
>>>>>
>>>>> struct bar {
>>>>>      void *a;
>>>>>      size_t b;
>>>>>      void *c;
>>>>>      unsigned long d;
>>>>>
>>>>>      uint8_t x;
>>>>>      uint8_t y; // ~= lanes
>>>>> };
>>>>>
>>>>> int main(void)
>>>>> {
>>>>>      printf("%d %d\n", sizeof(struct foo), sizeof(struct bar));
>>>>>      return 0;
>>>>> }
>>>>>
>>>>> With which I get these results on x86-64:
>>>>>
>>>>> $ gcc -o test test.c && ./test
>>>>> 48 40
>>>>>
>>>>> And on x86 32bit:
>>>>>
>>>>> $ i686-linux-gnu-gcc -o test test.c && ./test
>>>>> 24 20
>>>>>
>>>>> Maybe there is some improvement ?
>>>>
>>>> Try again with code size included, and other archs since 99% of the users would be an arm/riscv based boards.
>>> Doesn't that mean, that one some systems it wins us a bit of memory utilization improvement, and on other systems it has no impact ?
>>
>> 4 or 8 bytes less in a dynamically allocated struct which is by default aligned
>> on 64 bytes by default on x86, 128 on aarch64, 32/64/128 on arm32, 64 on riscv, sorry this is negligible.
> It is still not zero, so why tolerate the inefficiency when it can be improved ?
> 
> Is this change rejected ?

I won't nack it since it's technically correct, but won't ack it since it's an useless change.

Neil