Re: [PATCH v8 06/10] drm/xe/sysctrl: simplify mailbox header handling

["Mallesh, Koujalagi" <mallesh.koujalagi@intel.com>]

On 22-09-2026 04:47 pm, Michal Wajdeczko wrote:
>
> On 9/21/2026 3:39 PM, Mallesh Koujalagi wrote:
>> Use u32 for the mailbox header data field and remove unnecessary
>> cpu_to_le32()/le32_to_cpu() conversions when encoding and decoding
>> header bitfields.
> you still need to add the "why" parts:
> why it is safe to remove those conversions now, or
> why it was wrong/redundant in the past

Sure! will update next revision with below details:

"  The mailbox header was defined as __le32, which required
     cpu_to_le32() and le32_to_cpu() conversions whenever it was
     write or read.

     These conversions are unnecessary because the header is always
     handled as a 32-bit value, not as raw bytes. It is written with
     xe_mmio_write32() and read with xe_mmio_read32(), which already
     handle any required byte-order conversion through the underlying
     writel()/readl() accessors.

     As a result, the header value is already in CPU byte order when
     used by the driver. Keeping the extra conversions had no effect
     on little-endian systems and could cause an incorrect on
     big-endian systems.

     So use u32 for the mailbox header and remove the unnecessary
     endianness conversions."


Thanks,

-/Mallesh

>> Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Signed-off-by: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
>> Tested-by: Anoop Vijay <anoop.c.vijay@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_sysctrl_mailbox.c | 14 +++++++-------
>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
>> index 50bbdab072de..750178a0974b 100644
>> --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
>> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
>> @@ -20,20 +20,20 @@
>>   #include "xe_sysctrl_mailbox_types.h"
>>   
>>   struct xe_sysctrl_mailbox_msg_hdr {
>> -	__le32 data;
>> +	u32 data;
>>   } __packed;
>>   
>>   #define XE_SYSCTRL_HDR_GROUP_ID(hdr) \
>> -	FIELD_GET(SYSCTRL_HDR_GROUP_ID_MASK, le32_to_cpu((hdr)->data))
>> +	FIELD_GET(SYSCTRL_HDR_GROUP_ID_MASK, (hdr)->data)
>>   
>>   #define XE_SYSCTRL_HDR_COMMAND(hdr) \
>> -	FIELD_GET(SYSCTRL_HDR_COMMAND_MASK, le32_to_cpu((hdr)->data))
>> +	FIELD_GET(SYSCTRL_HDR_COMMAND_MASK, (hdr)->data)
>>   
>>   #define XE_SYSCTRL_HDR_IS_RESPONSE(hdr) \
>> -	FIELD_GET(SYSCTRL_HDR_IS_RESPONSE, le32_to_cpu((hdr)->data))
>> +	FIELD_GET(SYSCTRL_HDR_IS_RESPONSE, (hdr)->data)
>>   
>>   #define XE_SYSCTRL_HDR_RESULT(hdr) \
>> -	FIELD_GET(SYSCTRL_HDR_RESULT_MASK, le32_to_cpu((hdr)->data))
>> +	FIELD_GET(SYSCTRL_HDR_RESULT_MASK, (hdr)->data)
>>   
>>   static int sysctrl_wait_bit_clear(struct xe_sysctrl *sc, u32 bit_mask,
>>   				  unsigned int timeout_ms)
>> @@ -112,8 +112,8 @@ static int sysctrl_prepare_command(struct xe_device *xe,
>>   		return -ENOMEM;
>>   
>>   	hdr = (struct xe_sysctrl_mailbox_msg_hdr *)buffer;
>> -	hdr->data = cpu_to_le32(FIELD_PREP(SYSCTRL_HDR_GROUP_ID_MASK, group_id) |
>> -				     FIELD_PREP(SYSCTRL_HDR_COMMAND_MASK, command));
>> +	hdr->data = FIELD_PREP(SYSCTRL_HDR_GROUP_ID_MASK, group_id) |
>> +		    FIELD_PREP(SYSCTRL_HDR_COMMAND_MASK, command);
>>   
>>   	if (data_in && data_in_len)
>>   		memcpy(buffer + sizeof(*hdr), data_in, data_in_len);