[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<PN3PR01MB95970BAFDA466CE04A7297FBB8C02@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM>
Date: Mon, 24 Feb 2025 15:52:03 +0000
From: Aditya Garg <gargaditya08@...e.com>
To: "andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>
CC: "maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
"mripard@...nel.org" <mripard@...nel.org>, "tzimmermann@...e.de"
<tzimmermann@...e.de>, "airlied@...il.com" <airlied@...il.com>,
"simona@...ll.ch" <simona@...ll.ch>, Kerem Karabay <kekrby@...il.com>,
Atharva Tiwari <evepolonium@...il.com>, Aun-Ali Zaidi <admin@...eit.net>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v4 2/2] drm/tiny: add driver for Apple Touch Bars in x86
Macs
> On 24 Feb 2025, at 9:19 PM, andriy.shevchenko@...ux.intel.com wrote:
>
> On Mon, Feb 24, 2025 at 03:39:56PM +0000, Aditya Garg wrote:
>>>> On 24 Feb 2025, at 9:07 PM, andriy.shevchenko@...ux.intel.com wrote:
>>> On Mon, Feb 24, 2025 at 03:20:13PM +0000, Aditya Garg wrote:
>>>>>> On 24 Feb 2025, at 8:41 PM, andriy.shevchenko@...ux.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@...ux.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@...ux.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.
>>
>> 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.
Powered by blists - more mailing lists