Re: [PATCH 2/2] drm/xe/guc: Fix arguments passed to relay G2H handlers

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 11.01.2024 00:07, Matthew Brost wrote:
> On Wed, Jan 10, 2024 at 08:59:51PM +0100, Michal Wajdeczko wrote:
>> By default CT code was passing just payload of the G2H event
>> message, while Relay code expects full G2H message including
>> HXG header which contains DATA0 field. Fix that.
>>
>> Fixes: 152577060697 ("drm/xe/guc: Start handling GuC Relay event messages")
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> 
> FWIW I do think the argument names in xe_guc_relay_process_* functions
> should be changed but not going to hold of this fix.

I can rename message argument in relay g2h handlers to "hxg", but what
about the other g2h handlers, which (IMO wrongly) take just payload, not
a message, while still use "msg" as an argument name:

int xe_guc_sched_done_handler(... u32 *msg, u32 len)
int xe_guc_deregister_done_handler(... u32 *msg, u32 len)
int xe_guc_exec_queue_reset_handler(... u32 *msg, u32 len)
int xe_guc_exec_queue_memory_cat_error_handler(... u32 *msg, u32 len)
int xe_guc_exec_queue_reset_failure_handler(... u32 *msg, u32 len)
int xe_guc_pagefault_handler(... u32 *msg, u32 len)
int xe_guc_tlb_invalidation_done_handler(... u32 *msg, u32 len)
int xe_guc_access_counter_notify_handler(... u32 *msg, u32 len)

> 
> With that:
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> 
>> ---
>>  drivers/gpu/drm/xe/xe_guc_ct.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
>> index d6b7020a2d2f..4a0c9ce13bf8 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
>> @@ -984,10 +984,10 @@ static int process_g2h_msg(struct xe_guc_ct *ct, u32 *msg, u32 len)
>>  							   adj_len);
>>  		break;
>>  	case XE_GUC_ACTION_GUC2PF_RELAY_FROM_VF:
>> -		ret = xe_guc_relay_process_guc2pf(&guc->relay, payload, adj_len);
>> +		ret = xe_guc_relay_process_guc2pf(&guc->relay, hxg, hxg_len);
>>  		break;
>>  	case XE_GUC_ACTION_GUC2VF_RELAY_FROM_PF:
>> -		ret = xe_guc_relay_process_guc2vf(&guc->relay, payload, adj_len);
>> +		ret = xe_guc_relay_process_guc2vf(&guc->relay, hxg, hxg_len);
>>  		break;
>>  	default:
>>  		drm_err(&xe->drm, "unexpected action 0x%04x\n", action);
>> -- 
>> 2.25.1
>>