On Wed, 2023-06-28 at 21:44 +0000, Brost, Matthew wrote:
> On Wed, Jun 28, 2023 at 11:17:18AM -0700, Alan Previn wrote:
> > In the ABI header, GUC_CTB_MSG_MIN_LEN is '1' because
> > GUC_CTB_HDR_LEN is 1. This aligns with H2G/G2H CTB specification
> > where all command formats are defined in units of dwords so that '1'
> > is a dword. Accordingly, GUC_CTB_MSG_MAX_LEN is 256-1 (i.e. 255
> > dwords). However, h2g_write was incorrectly assuming that
> > GUC_CTB_MSG_MAX_LEN was in bytes. Fix this.
>
alan:snip
> > diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> > index 22bc9ce846db..aa04b5c4822f 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> > @@ -401,19 +401,21 @@ static int h2g_write(struct xe_guc_ct *ct, const u32 *action, u32 len,
> > {
> > struct xe_device *xe = ct_to_xe(ct);
> > struct guc_ctb *h2g = &ct->ctbs.h2g;
> > - u32 cmd[GUC_CTB_MSG_MAX_LEN / sizeof(u32)];
> > - u32 cmd_len = len + GUC_CTB_HDR_LEN;
> > - u32 cmd_idx = 0, i;
> > +#define H2G_CT_HEADERS (GUC_CTB_HDR_LEN + 1) /* one DW CTB header and one DW HxG header */
>
> Hate to nit pick but again this should be above the h2g_write per
> feedback from Oden on Xe in general.
>
> Otherwise LGTM.
>
> With the nit addressed:
>
> Reviewed-by: Matthew Brost <matthew.brost@xxxxxxxxx>
Thanks for reviewing. My bad on the #define - you mentioned that before.
Will fix that now.
...alan
