On Mon, Feb 24, 2025 at 03:52:03PM +0000, Aditya Garg wrote: > > On 24 Feb 2025, at 9:19 PM, andriy.shevchenko@xxxxxxxxxxxxxxx wrote: > > On Mon, Feb 24, 2025 at 03:39:56PM +0000, Aditya Garg wrote: > >>>> On 24 Feb 2025, at 9:07 PM, andriy.shevchenko@xxxxxxxxxxxxxxx wrote: > >>> On Mon, Feb 24, 2025 at 03:20:13PM +0000, Aditya Garg 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. > >> > >> The Macro is just converting the letters into their hex form, but simply > >> calculating them and putting the letters in comments is equally good. > > > > Again, it does it in most confusing and weird way. Just kill it. > > #define APPLETBDRM_PIXEL_FORMAT cpu_to_le32(0x52474241) /* RGBA, the actual format is BGR888 */ > #define APPLETBDRM_BITS_PER_PIXEL 24 > > #define APPLETBDRM_MSG_CLEAR_DISPLAY cpu_to_le32(0x434c5244) /* CLRD */ > #define APPLETBDRM_MSG_GET_INFORMATION cpu_to_le32(0x47494e46) /* GINF */ > #define APPLETBDRM_MSG_UPDATE_COMPLETE cpu_to_le32(0x5544434c) /* UDCL */ > #define APPLETBDRM_MSG_SIGNAL_READINESS cpu_to_le32(0x52454459) /* REDY */ > > This should be good. Yep, way better than the original code! -- With Best Regards, Andy Shevchenko
