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

[Geert Uytterhoeven]

Hi Neil,

On Mon, 30 Jun 2025 at 17:34, Neil Armstrong <neil.armstrong at linaro.org> wrote:
> 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

These structures are static, and not allocated dynamically.

> >> 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.

On arm32:

    $ bloat-o-meter drivers/gpu/drm/panel/panel-ilitek-ili9881c.o{.orig,}
    add/remove: 0/0 grow/shrink: 0/8 up/down: 0/-32 (-32)

Surprisingly, even on arm64:

    $ bloat-o-meter drivers/gpu/drm/panel/panel-ilitek-ili9881c.o{.orig,}
    add/remove: 0/0 grow/shrink: 0/8 up/down: 0/-64 (-64)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds