[Intel-xe] [RFC v1 04/17] drm/xe: Add uAPI to query micro-controler firmware version

[Francois Dugast]

On Wed, Oct 11, 2023 at 10:17:41PM +0200, Souza, Jose wrote:
> On Wed, 2023-10-11 at 13:07 -0700, John Harrison wrote:
> > On 10/11/2023 06:59, Francois Dugast wrote:
> > > From: José Roberto de Souza <jose.souza at intel.com>
> > > 
> > > Due to a bug in GuC firmware, Mesa can't enable by default the usage of
> > > compute engines in DG2 and newer.
> > > 
> > > A new GuC firmware fixed the issue but until now there was no way
> > > for Mesa to know if KMD was running with the fixed GuC version or not,
> > > so this uAPI is required.
> > > 
> > > It may be expanded in future to query other firmware versions too.
> > > 
> > > More information: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/23661
> > > Mesa usage: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25233
> > > 
> > > v2:
> > > - changed to submission version
> > > - added branch version to be future proof
> > > - checking if pads and reserved are zero
> > > 
> > > v3:
> > > - add braces around case XE_QUERY_UC_TYPE_GUC to make CI happy
> > > 
> > > Cc: John Harrison <John.C.Harrison at Intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > ---
> > >   drivers/gpu/drm/xe/xe_query.c | 41 +++++++++++++++++++++++++++++++++++
> > >   include/uapi/drm/xe_drm.h     | 31 ++++++++++++++++++++++++++
> > >   2 files changed, 72 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> > > index 10b9878ec95a..efbcf2ef436e 100644
> > > --- a/drivers/gpu/drm/xe/xe_query.c
> > > +++ b/drivers/gpu/drm/xe/xe_query.c
> > > @@ -498,6 +498,46 @@ static int query_gt_topology(struct xe_device *xe,
> > >   	return 0;
> > >   }
> > >   
> > > +static int
> > > +query_uc_fw_version(struct xe_device *xe, struct drm_xe_device_query *query)
> > > +{
> > > +	struct drm_xe_query_uc_fw_version __user *query_ptr = u64_to_user_ptr(query->data);
> > > +	size_t size = sizeof(struct drm_xe_query_uc_fw_version);
> > > +	struct drm_xe_query_uc_fw_version resp;
> > > +
> > > +	if (query->size == 0) {
> > > +		query->size = size;
> > > +		return 0;
> > > +	} else if (XE_IOCTL_DBG(xe, query->size != size)) {
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	if (copy_from_user(&resp, query_ptr, size))
> > > +		return -EFAULT;
> > > +
> > > +	if (XE_IOCTL_DBG(xe, resp.pad || resp.pad2 || resp.reserved))
> > > +		return -EINVAL;
> > > +
> > > +	switch (resp.uc_type) {
> > > +	case XE_QUERY_UC_TYPE_GUC: {
> > > +		struct xe_guc *guc = &xe->tiles[0].primary_gt->uc.guc;
> > > +
> > > +		resp.major_ver = guc->submission_state.version.major;
> > > +		resp.minor_ver = guc->submission_state.version.minor;
> > > +		resp.patch_ver = guc->submission_state.version.patch;
> > > +		resp.branch_ver = 0;
> > > +		break;
> > > +	}
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	if (copy_to_user(query_ptr, &resp, size))
> > > +		return -EFAULT;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >   static int (* const xe_query_funcs[])(struct xe_device *xe,
> > >   				      struct drm_xe_device_query *query) = {
> > >   	query_engines,
> > > @@ -507,6 +547,7 @@ static int (* const xe_query_funcs[])(struct xe_device *xe,
> > >   	query_hwconfig,
> > >   	query_gt_topology,
> > >   	query_engine_cycles,
> > > +	query_uc_fw_version,
> > >   };
> > >   
> > >   int xe_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> > > index 47d2d0468339..f7ca52aba79b 100644
> > > --- a/include/uapi/drm/xe_drm.h
> > > +++ b/include/uapi/drm/xe_drm.h
> > > @@ -466,6 +466,36 @@ struct drm_xe_query_topology_mask {
> > >   	__u8 mask[];
> > >   };
> > >   
> > > +/**
> > > + * struct drm_xe_query_uc_fw_version - query a micro-controller firmware version
> > > + *
> > > + * Given a uc_type this will return the major, minor, patch and branch version
> > > + * of the micro-controller firmware.
> > > + */
> > > +struct drm_xe_query_uc_fw_version {
> > > +	/** @uc: The micro-controller type to query firmware version */
> > > +#define XE_QUERY_UC_TYPE_GUC 0
> > I thought it was agreed to use TYPE_GUC_SUBMISSION to differentiate from 
> > the file version which may need to be added at some point (e.g. for 
> > reporting status in a control panel or something).
> 
> It is being renamed in '[RFC v1 16/17] drm/xe/uapi: Be more specific around GuC Version Type'
> 
> In my opinion it should be squashed but up to Francois and Rodrigo.
> 
> > 
> > > +	__u16 uc_type;
> > > +
> > > +	/** @pad: MBZ */
> > > +	__u16 pad;
> > And wasn't there a comment about just using a u32 for the type? What is 
> > the advantage to splitting it into two u16 values?
> 
> yeah good point or rename to rsvd so it can be used in future.

Sure this will be in the next version.

Francois

> 
> 
> > 
> > John.
> > 
> > > +
> > > +	/* @major_ver: major uc fw version */
> > > +	__u32 major_ver;
> > > +	/* @minor_ver: minor uc fw version */
> > > +	__u32 minor_ver;
> > > +	/* @patch_ver: patch uc fw version */
> > > +	__u32 patch_ver;
> > > +	/* @branch_ver: branch uc fw version */
> > > +	__u32 branch_ver;
> > > +
> > > +	/** @pad2: MBZ */
> > > +	__u32 pad2;
> > > +
> > > +	/** @reserved: Reserved */
> > > +	__u64 reserved;
> > > +};
> > > +
> > >   /**
> > >    * struct drm_xe_device_query - main structure to query device information
> > >    *
> > > @@ -518,6 +548,7 @@ struct drm_xe_device_query {
> > >   #define DRM_XE_DEVICE_QUERY_HWCONFIG		4
> > >   #define DRM_XE_DEVICE_QUERY_GT_TOPOLOGY		5
> > >   #define DRM_XE_DEVICE_QUERY_ENGINE_CYCLES	6
> > > +#define DRM_XE_DEVICE_QUERY_UC_FW_VERSION	7
> > >   	/** @query: The type of data to query */
> > >   	__u32 query;
> > >   
> > 
>