Re: [PATCH v3 3/3] drm/tiny: add driver for Apple Touch Bars in x86 Macs

["andriy.shevchenko@linux.intel.com" <andriy.shevchenko@linux.intel.com>]

On Mon, Feb 24, 2025 at 09:41:43AM +0100, Thomas Zimmermann wrote:
> Am 22.02.25 um 10:07 schrieb Aditya Garg:

...

> > > What padding, please? Why TCP UAPI headers do not have these attributes?
> > > Think about it, and think about what actually __packed does and how it affects
> > > (badly) the code generation. Otherwise it looks like a cargo cult.
> > > 
> > > > I tried removing __packed btw and driver no longer works.
> > > So, you need to find a justification why. But definitely not due to padding in
> > > many of them. They can go without __packed as they are naturally aligned.
> > Alright, I did some debugging, basically printk sizeof(struct). Did it for both packed and unpacked with the following results:
> > 
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_msg_request_header is 16
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_msg_request_header_unpacked is 16
> > 
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_msg_response_header is 20
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_msg_response_header_unpacked is 20
> > 
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_msg_simple_request is 32
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_msg_simple_request_unpacked is 32
> > 
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_msg_information is 65
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_msg_information_unpacked is 68
> 
> In the unpacked version, there is a 3-byte gap after the 'bits_per_pixel' to
> align the next field. Using __packed removes those gaps at the expense of
> runtime overhead.
> > 
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_frame is 12
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_frame_unpacked is 12
> > 
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_fb_request_footer is 80
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_fb_request_footer_unpacked is 80
> > 
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_fb_request is 48
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_fb_request_unpacked is 48
> > 
> > Feb 22 13:02:03 MacBook kernel: size of struct appletbdrm_fb_request_response is 40
> > Feb 22 13:02:04 MacBook kernel: size of struct appletbdrm_fb_request_response_unpacked is 40
> > 
> > So, the difference in sizeof in unpacked and packed is only in appletbdrm_msg_information. So, I kept this packed, and removed it from others. The Touch Bar still works.
> > 
> > So maybe keep just this packed?
> 
> The fields in the TCP header are aligned by design.

> Unfortunately, this hardware's protocol is not. And there's no way of fixing
> this now. Just keep all of them packed if you want.

It would be nice to see the difference in the code generation for the all
__packed vs. only those that require it.

> At least it's clear then
> what happens. And if your hardware requires this, you can't do much anyway.

One aspect (member level alignment) is clear but the other is not
(object level alignment). I dunno if it makes sense to be pedantic about this,
but would like to see the binary outcome asked for.

-- 
With Best Regards,
Andy Shevchenko