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

[Anoop Vijay <anoop.c.vijay@intel.com>]

On 22-09-2026 16:47, 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
__le32 was added because the header is genuine little-endian wire data,
but xe_mmio_write32()/xe_mmio_read32() already do the
host<->LE conversion at the MMIO boundary. So the extra
cpu_to_le32()/le32_to_cpu() was redundant, and on BE hosts that redundant
conversion doubles up and corrupts the value - safe to remove

Thanks,
Anoop
>> 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);