[PATCH v4 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs

[Aditya Garg]

> On 24 Feb 2025, at 9:08 PM, andriy.shevchenko at linux.intel.com wrote:
> 
> On Mon, Feb 24, 2025 at 03:32:56PM +0000, Aditya Garg wrote:
>>> On 24 Feb 2025, at 8:50 PM, Aditya Garg <gargaditya08 at live.com> wrote:
>>>> On 24 Feb 2025, at 8:41 PM, andriy.shevchenko at linux.intel.com wrote:
>>>> On Mon, Feb 24, 2025 at 03:03:40PM +0000, Aditya Garg wrote:
>>>>>>> On 24 Feb 2025, at 8:27 PM, andriy.shevchenko at linux.intel.com wrote:
>>>>>> On Mon, Feb 24, 2025 at 02:32:37PM +0000, Aditya Garg wrote:
>>>>>>>> On 24 Feb 2025, at 7:30 PM, andriy.shevchenko at linux.intel.com wrote:
>>>>>>>>>> On Mon, Feb 24, 2025 at 01:40:20PM +0000, Aditya Garg wrote:
>> 
>> ...
>> 
>>>>>>>>>>> +#define __APPLETBDRM_MSG_STR4(str4) ((__le32 __force)((str4[0] << 24) | (str4[1] << 16) | (str4[2] << 8) | str4[3]))
>>>>>>>>>> 
>>>>>>>>>> As commented previously this is quite strange what's going on with endianess in
>>>>>>>>>> this driver. Especially the above weirdness when get_unaligned_be32() is being
>>>>>>>>>> open coded and force-cast to __le32.
>>>>>>>>> 
>>>>>>>>> I would assume it was also mimicked from the Windows driver, though I haven't
>>>>>>>>> really tried exploring this there.
>>>>>>>>> 
>>>>>>>>> I’d rather be happy if you give me code change suggestions and let me review
>>>>>>>>> and test them
>>>>>>>> 
>>>>>>>> For the starter I would do the following for all related constants and
>>>>>>>> drop that weird and ugly macros at the top (it also has an issue with
>>>>>>>> the str4 length as it is 5 bytes long, not 4, btw):
>>>>>>>> 
>>>>>>>> #define APPLETBDRM_MSG_CLEAR_DISPLAY cpu_to_le32(0x434c5244) /* CLRD */
>>>>>> 
>>>>>> Lemme test this.
>>>>> 
>>>>> Just in case it won't work, reverse bytes in the integer. Because I was lost in
>>>>> this conversion.
>>> 
>>> It works. What I understand is that you used the macro to get the final hex and converted it into little endian, which on the x86 macs would technically remain the same.
>> 
>> And now that I oberved again, %p4cc is actually printing these CLRD, REDY etc
>> in reverse order, probably the reason %p4ch was chosen. And I am unable to
>> find what macro upstream can be used.
> 
> %.4s should work as it technically not DRM 4cc, but specifics of the protocol
> (that reminds me about ACPI that uses 4cc a lot).

I still get reverse order in that.
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
>