> On 24 Feb 2025, at 9:45 PM, andriy.shevchenko@xxxxxxxxxxxxxxx wrote: > > On Mon, Feb 24, 2025 at 03:56:36PM +0000, Aditya Garg wrote: >>>> On 24 Feb 2025, at 9:23 PM, andriy.shevchenko@xxxxxxxxxxxxxxx wrote: >>> On Mon, Feb 24, 2025 at 03:40:29PM +0000, Aditya Garg wrote: >>>>>> On 24 Feb 2025, at 9:08 PM, andriy.shevchenko@xxxxxxxxxxxxxxx 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@xxxxxxxx> wrote: >>>>>>>> On 24 Feb 2025, at 8:41 PM, andriy.shevchenko@xxxxxxxxxxxxxxx wrote: >>>>>>>> On Mon, Feb 24, 2025 at 03:03:40PM +0000, Aditya Garg wrote: >>>>>>>>>>> On 24 Feb 2025, at 8:27 PM, andriy.shevchenko@xxxxxxxxxxxxxxx wrote: >>>>>>>>>> On Mon, Feb 24, 2025 at 02:32:37PM +0000, Aditya Garg wrote: >>>>>>>>>>>> On 24 Feb 2025, at 7:30 PM, andriy.shevchenko@xxxxxxxxxxxxxxx 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. >>> >>> Ah, right, it will give you the first letter as LSB, indeed. At the end of the >>> day if it's so important, there are ways how to solve that without using %p4cc. >>> But if others (and esp. PRINTK maintainers) want to have / don't object having >>> that extension, why not? >> >> Right, but what to do about the case of little endian and host endian? I >> remember the statement "for the sake of completeness" for them. Do you think >> just host endian and reverse endian should be just fine? Or you got any "no >> sparse warning" way to get it done? The macros to convert to le32/be32 expect >> a u32 value, but in those cases we actually are passing a le32/be32 value. > > For now I think we better save the energy and wait for PRINTK people to tell > if they are okay in general. Otherwise it makes no sense to develop and review > something that will go to the trash bin. > I'd rather keep %p4cc here for now. I don't know in detail about FOURCC tbh, so probably just let Asahi Linux manage that part if more changes are needed. In case we get better printk formats, a subsequent patch can be sent later. %p4cc still works just fine.
