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

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Jan 11, 2024 at 10:37:31AM +0100, Michal Wajdeczko wrote:
> 
> 
> 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)
> 

Good point I'd say lets clean all of this up.

msg -> entire G2H
hxg -> HXG + payload
payload -> payload

Ok with a follow up. We can chat off the list with who will post.

Matt

> > 
> > 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
> >>