[Intel-xe] [PATCH v2 10/31] drm/xe/guc: Return the lower part of blocking H2G message

[Matthew Brost]

On Fri, May 05, 2023 at 02:52:45PM -0400, Rodrigo Vivi wrote:
> On Mon, May 01, 2023 at 05:17:06PM -0700, Matthew Brost wrote:
> > The upper layers may need this data, an example of this is allocating
> > DIST doorbell.
> > 
> > Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_guc_ct.c | 6 +++++-
> >  drivers/gpu/drm/xe/xe_guc_pc.c | 6 ++++--
> >  drivers/gpu/drm/xe/xe_huc.c    | 2 +-
> >  3 files changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> > index 6abf1dee95af..60b69fcfac9f 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> > @@ -25,6 +25,7 @@
> >  struct g2h_fence {
> >  	u32 *response_buffer;
> >  	u32 seqno;
> > +	u32 status;
> >  	u16 response_len;
> >  	u16 error;
> >  	u16 hint;
> > @@ -727,7 +728,7 @@ static int guc_ct_send_recv(struct xe_guc_ct *ct, const u32 *action, u32 len,
> >  		ret = -EIO;
> >  	}
> >  
> > -	return ret > 0 ? 0 : ret;
> > +	return ret > 0 ? g2h_fence.status : ret;
> 
> The problem I see here is how the upper level could differentiate
> between and error and a status.
> 

g2h_fence.status is 16 (can't be negative), so 0 or greater is a good
return.

> should we convert the functions to have an &status argument passed in?
>

I like it the way it is but don't really care either way. I'll change
this if you like.

Matt 

> >  }
> >  
> >  int xe_guc_ct_send_recv(struct xe_guc_ct *ct, const u32 *action, u32 len,
> > @@ -793,6 +794,9 @@ static int parse_g2h_response(struct xe_guc_ct *ct, u32 *msg, u32 len)
> >  		g2h_fence->response_len = response_len;
> >  		memcpy(g2h_fence->response_buffer, msg + GUC_CTB_MSG_MIN_LEN,
> >  		       response_len * sizeof(u32));
> > +	} else {
> > +		g2h_fence->status =
> > +			FIELD_GET(GUC_HXG_RESPONSE_MSG_0_DATA0, msg[1]);
> >  	}
> >  
> >  	g2h_release_space(ct, GUC_CTB_HXG_MSG_MAX_LEN);
> > diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
> > index 72d460d5323b..3d2ea723a4a7 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> > @@ -204,11 +204,13 @@ static int pc_action_query_task_state(struct xe_guc_pc *pc)
> >  
> >  	/* Blocking here to ensure the results are ready before reading them */
> >  	ret = xe_guc_ct_send_block(ct, action, ARRAY_SIZE(action));
> > -	if (ret)
> > +	if (ret < 0) {
> >  		drm_err(&pc_to_xe(pc)->drm,
> >  			"GuC PC query task state failed: %pe", ERR_PTR(ret));
> > +		return ret;
> > +	}
> >  
> > -	return ret;
> > +	return 0;
> >  }
> >  
> >  static int pc_action_set_param(struct xe_guc_pc *pc, u8 id, u32 value)
> > diff --git a/drivers/gpu/drm/xe/xe_huc.c b/drivers/gpu/drm/xe/xe_huc.c
> > index 55dcaab34ea4..9c48c3075410 100644
> > --- a/drivers/gpu/drm/xe/xe_huc.c
> > +++ b/drivers/gpu/drm/xe/xe_huc.c
> > @@ -39,7 +39,7 @@ int xe_huc_init(struct xe_huc *huc)
> >  
> >  	huc->fw.type = XE_UC_FW_TYPE_HUC;
> >  	ret = xe_uc_fw_init(&huc->fw);
> > -	if (ret)
> > +	if (ret < 0)
> >  		goto out;
> >  
> >  	xe_uc_fw_change_status(&huc->fw, XE_UC_FIRMWARE_LOADABLE);
> > -- 
> > 2.34.1
> >