On Fri, 2024-02-09 at 09:06 +0000, Tvrtko Ursulin wrote:
> On 08/02/2024 17:55, Souza, Jose wrote:
> > On Thu, 2024-02-08 at 07:19 -0800, José Roberto de Souza wrote:
> > > On Thu, 2024-02-08 at 14:59 +0000, Tvrtko Ursulin wrote:
> > > > On 08/02/2024 14:30, Souza, Jose wrote:
> > > > > On Thu, 2024-02-08 at 08:25 +0000, Tvrtko Ursulin wrote:
> > > > > > From: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
> > > > > >
> > > > > > Add a new query to the GuC submission interface version.
> > > > > >
> > > > > > Mesa intends to use this information to check for old firmware
> > > > > > versions
> > > > > > with a known bug where using the render and compute command
> > > > > > streamers
> > > > > > simultaneously can cause GPU hangs due issues in firmware
> > > > > > scheduling.
> > > > > >
> > > > > > Based on patches from Vivaik and Joonas.
> > > > > >
> > > > > > Compile tested only.
> > > > > >
> > > > > > v2:
> > > > > > * Added branch version.
> > > > >
> > > > > Reviewed-by: José Roberto de Souza <jose.so...@intel.com>
> > > > > Tested-by: José Roberto de Souza <jose.so...@intel.com>
> > > > > UMD: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25233
> > > >
> > > > Thanks, but please we also need to close down on the branch number
> > > > situation. I.e. be sure what is the failure mode in shipping Mesa with
> > > > the change as it stands in the MR linked. What platforms could start
> > > > failing and when, depending on GuC FW release eventualities.
> > >
> > > yes, I have asked John Harrison for a documentation link about the
> > > firmware versioning.
> >
> > Got the documentation link, MR updated.
> > Will ask for reviews in Mesa side.
>
> Is it then understood and accepted that should GuC ever update the
> branch number on any given platform, that platform, for all deployed
> Mesa's in the field, will automatically revert to no async queues and so
> cause a silent performance regression?
yes, understood and accepted.
The Mesa MR is now reviewed, thank you Sagar.
>
> Regards,
>
> Tvrtko
>
> >
> > >
> > > >
> > > > Regards,
> > > >
> > > > Tvrtko
> > > >
> > > > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
> > > > > > Cc: Kenneth Graunke <kenn...@whitecape.org>
> > > > > > Cc: Jose Souza <jose.so...@intel.com>
> > > > > > Cc: Sagar Ghuge <sagar.gh...@intel.com>
> > > > > > Cc: Paulo Zanoni <paulo.r.zan...@intel.com>
> > > > > > Cc: John Harrison <john.c.harri...@intel.com>
> > > > > > Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
> > > > > > Cc: Jani Nikula <jani.nik...@intel.com>
> > > > > > Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
> > > > > > Cc: Vivaik Balasubrawmanian <vivaik.balasubrawman...@intel.com>
> > > > > > Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/i915_query.c | 33
> > > > > > +++++++++++++++++++++++++++++++
> > > > > > include/uapi/drm/i915_drm.h | 12 +++++++++++
> > > > > > 2 files changed, 45 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_query.c
> > > > > > b/drivers/gpu/drm/i915/i915_query.c
> > > > > > index 00871ef99792..d4dba1240b40 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_query.c
> > > > > > +++ b/drivers/gpu/drm/i915/i915_query.c
> > > > > > @@ -551,6 +551,38 @@ static int query_hwconfig_blob(struct
> > > > > > drm_i915_private *i915,
> > > > > > return hwconfig->size;
> > > > > > }
> > > > > >
> > > > > > +static int
> > > > > > +query_guc_submission_version(struct drm_i915_private *i915,
> > > > > > + struct drm_i915_query_item *query)
> > > > > > +{
> > > > > > + struct drm_i915_query_guc_submission_version __user *query_ptr =
> > > > > > +
> > > > > > u64_to_user_ptr(query->data_ptr);
> > > > > > + struct drm_i915_query_guc_submission_version ver;
> > > > > > + struct intel_guc *guc = &to_gt(i915)->uc.guc;
> > > > > > + const size_t size = sizeof(ver);
> > > > > > + int ret;
> > > > > > +
> > > > > > + if (!intel_uc_uses_guc_submission(&to_gt(i915)->uc))
> > > > > > + return -ENODEV;
> > > > > > +
> > > > > > + ret = copy_query_item(&ver, size, size, query);
> > > > > > + if (ret != 0)
> > > > > > + return ret;
> > > > > > +
> > > > > > + if (ver.branch || ver.major || ver.minor || ver.patch)
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + ver.branch = 0;
> > > > > > + ver.major = guc->submission_version.major;
> > > > > > + ver.minor = guc->submission_version.minor;
> > > > > > + ver.patch = guc->submission_version.patch;
> > > > > > +
> > > > > > + if (copy_to_user(query_ptr, &ver, size))
> > > > > > + return -EFAULT;
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > static int (* const i915_query_funcs[])(struct drm_i915_private
> > > > > > *dev_priv,
> > > > > > struct drm_i915_query_item
> > > > > > *query_item) = {
> > > > > > query_topology_info,
> > > > > > @@ -559,6 +591,7 @@ static int (* const i915_query_funcs[])(struct
> > > > > > drm_i915_private *dev_priv,
> > > > > > query_memregion_info,
> > > > > > query_hwconfig_blob,
> > > > > > query_geometry_subslices,
> > > > > > + query_guc_submission_version,
> > > > > > };
> > > > > >
> > > > > > int i915_query_ioctl(struct drm_device *dev, void *data, struct
> > > > > > drm_file *file)
> > > > > > diff --git a/include/uapi/drm/i915_drm.h
> > > > > > b/include/uapi/drm/i915_drm.h
> > > > > > index 550c496ce76d..84fb7f7ea834 100644
> > > > > > --- a/include/uapi/drm/i915_drm.h
> > > > > > +++ b/include/uapi/drm/i915_drm.h
> > > > > > @@ -3038,6 +3038,7 @@ struct drm_i915_query_item {
> > > > > > * - %DRM_I915_QUERY_MEMORY_REGIONS (see struct
> > > > > > drm_i915_query_memory_regions)
> > > > > > * - %DRM_I915_QUERY_HWCONFIG_BLOB (see `GuC HWCONFIG blob
> > > > > > uAPI`)
> > > > > > * - %DRM_I915_QUERY_GEOMETRY_SUBSLICES (see struct
> > > > > > drm_i915_query_topology_info)
> > > > > > + * - %DRM_I915_QUERY_GUC_SUBMISSION_VERSION (see struct
> > > > > > drm_i915_query_guc_submission_version)
> > > > > > */
> > > > > > __u64 query_id;
> > > > > > #define DRM_I915_QUERY_TOPOLOGY_INFO 1
> > > > > > @@ -3046,6 +3047,7 @@ struct drm_i915_query_item {
> > > > > > #define DRM_I915_QUERY_MEMORY_REGIONS 4
> > > > > > #define DRM_I915_QUERY_HWCONFIG_BLOB 5
> > > > > > #define DRM_I915_QUERY_GEOMETRY_SUBSLICES 6
> > > > > > +#define DRM_I915_QUERY_GUC_SUBMISSION_VERSION 7
> > > > > > /* Must be kept compact -- no holes and well documented */
> > > > > >
> > > > > > /**
> > > > > > @@ -3591,6 +3593,16 @@ struct drm_i915_query_memory_regions {
> > > > > > struct drm_i915_memory_region_info regions[];
> > > > > > };
> > > > > >
> > > > > > +/**
> > > > > > +* struct drm_i915_query_guc_submission_version - query GuC
> > > > > > submission interface version
> > > > > > +*/
> > > > > > +struct drm_i915_query_guc_submission_version {
> > > > > > + __u32 branch;
> > > > > > + __u32 major;
> > > > > > + __u32 minor;
> > > > > > + __u32 patch;
> > > > > > +};
> > > > > > +
> > > > > > /**
> > > > > > * DOC: GuC HWCONFIG blob uAPI
> > > > > > *
> > > > >
> > >
> >
