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.ursulin@xxxxxxxxx> > > > > > > > > > > > > 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.souza@xxxxxxxxx> > > > > > Tested-by: José Roberto de Souza <jose.souza@xxxxxxxxx> > > > > > 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.ursulin@xxxxxxxxx> > > > > > > Cc: Kenneth Graunke <kenneth@xxxxxxxxxxxxx> > > > > > > Cc: Jose Souza <jose.souza@xxxxxxxxx> > > > > > > Cc: Sagar Ghuge <sagar.ghuge@xxxxxxxxx> > > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > > > > > Cc: John Harrison <John.C.Harrison@xxxxxxxxx> > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > > > > > Cc: Jani Nikula <jani.nikula@xxxxxxxxx> > > > > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > > > > > Cc: Vivaik Balasubrawmanian <vivaik.balasubrawmanian@xxxxxxxxx> > > > > > > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > > > > > > --- > > > > > > 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 > > > > > > * > > > > > > > > > >
