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

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

[Marek Vasut]

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.

Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: David Airlie <airlied@gmail.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-renesas-soc@vger.kernel.org
---
 drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
index ac433345a179..84927302957a 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
@@ -43,7 +43,7 @@ struct ili9881c_desc {
 	const struct drm_display_mode *mode;
 	const unsigned long mode_flags;
 	u8 default_address_mode;
-	unsigned int lanes;
+	u8 lanes;
 };
 
 struct ili9881c {
-- 
2.47.2

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

[Neil Armstrong]

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.

Neil

> 
> Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: David Airlie <airlied@gmail.com>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-renesas-soc@vger.kernel.org
> ---
>   drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> index ac433345a179..84927302957a 100644
> --- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> @@ -43,7 +43,7 @@ struct ili9881c_desc {
>   	const struct drm_display_mode *mode;
>   	const unsigned long mode_flags;
>   	u8 default_address_mode;
> -	unsigned int lanes;
> +	u8 lanes;
>   };
>   
>   struct ili9881c {

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

[Marek Vasut]

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 ?

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

[Neil Armstrong]

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.

Neil

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

[Marek Vasut]

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 ?

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

[Neil Armstrong]

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.

Neil

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

[Marek Vasut]

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 ?

Re: [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

Re: [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@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@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