Re: [Intel-xe] [PATCH 1/3] drm/xe: Add a couple of pcode helpers

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Sep 01, 2023 at 05:45:43PM +0530, Sujaritha Sundaresan wrote:
> Some pcode commands take additional sub-commands and parameters. Add a
> couple of helpers to help formatting these commands to improve code
> readability.
> 
> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pcode.c | 28 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_pcode.h |  3 +++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pcode.c b/drivers/gpu/drm/xe/xe_pcode.c
> index 7f1bf2297f51..e45169f47500 100644
> --- a/drivers/gpu/drm/xe/xe_pcode.c
> +++ b/drivers/gpu/drm/xe/xe_pcode.c
> @@ -104,6 +104,34 @@ int xe_pcode_read(struct xe_gt *gt, u32 mbox, u32 *val, u32 *val1)
>  	return err;
>  }
>  

a doc would be required...

> +int xe_pcode_read_p(struct xe_gt *gt, u32 mbcmd, u32 p1, u32 p2, u32 *val)

a better name would be nice....

> +{
> +	u32 mbox;
> +	int err;
> +
> +	mbox = REG_FIELD_PREP(PCODE_MB_COMMAND, mbcmd)
> +		| REG_FIELD_PREP(PCODE_MB_PARAM1, p1)
> +		| REG_FIELD_PREP(PCODE_MB_PARAM2, p2);
> +
> +	err = xe_pcode_read(gt, mbox, val, NULL);

but why not simply modifying the existent one to accept 2 params?

int xe_pcode_read(struct xe_gt *gt, u32 mbox_param1, u32 mbox_param2,
    		  u32 *val, u32 *val1)

and the equivalent write...

oh, and while doing that, could you please add the missing documentation
to these 2 functions?

Thanks,
Rodrigo.

> +
> +	return err;
> +}
> +
> +int xe_pcode_write_p(struct xe_gt *gt, u32 mbcmd, u32 p1, u32 p2, u32 val)
> +{
> +	u32 mbox;
> +	int err;
> +
> +	mbox = REG_FIELD_PREP(PCODE_MB_COMMAND, mbcmd)
> +		| REG_FIELD_PREP(PCODE_MB_PARAM1, p1)
> +		| REG_FIELD_PREP(PCODE_MB_PARAM2, p2);
> +
> +	err = xe_pcode_write(gt, mbox, val);
> +
> +	return err;
> +}
> +
>  static int xe_pcode_try_request(struct xe_gt *gt, u32 mbox,
>  				u32 request, u32 reply_mask, u32 reply,
>  				u32 *status, bool atomic, int timeout_us)
> diff --git a/drivers/gpu/drm/xe/xe_pcode.h b/drivers/gpu/drm/xe/xe_pcode.h
> index 3b4aa8c1a3ba..8d4103afd7e0 100644
> --- a/drivers/gpu/drm/xe/xe_pcode.h
> +++ b/drivers/gpu/drm/xe/xe_pcode.h
> @@ -19,6 +19,9 @@ int xe_pcode_write_timeout(struct xe_gt *gt, u32 mbox, u32 val,
>  #define xe_pcode_write(gt, mbox, val) \
>  	xe_pcode_write_timeout(gt, mbox, val, 1)
>  
> +int xe_pcode_read_p(struct xe_gt *gt, u32 mbcmd, u32 p1, u32 p2, u32 *val);
> +int xe_pcode_write_p(struct xe_gt *gt, u32 mbcmd, u32 p1, u32 p2, u32 val);
> +
>  int xe_pcode_request(struct xe_gt *gt, u32 mbox, u32 request,
>  		     u32 reply_mask, u32 reply, int timeout_ms);
>  
> -- 
> 2.25.1
>