On 1/24/2024 00:55, Tvrtko Ursulin wrote:
On 24/01/2024 08:19, Joonas Lahtinen wrote:
Add reporting of the GuC submissio/VF interface version via GETPARAM
properties. Mesa intends to use this information to check for old
firmware versions with known bugs before enabling features like async
compute.
There was
https://patchwork.freedesktop.org/patch/560704/?series=124592&rev=1
which does everything in one go so would be my preference.
I also think that the original version is a cleaner implementation.
During the time of that patch there was discussion whether firmware
version or submission version was better. I vaguely remember someone
raised an issue with the latter. Adding John in case he remembers.
The file version number should not be exposed to UMDs, only the
submission version. The whole purpose of the submission version is to
track user facing changes. There was a very, very, very long discussion
about that to which all parties did eventually agree on using the
submission version.
The outstanding issues were simply a) whether UMDs should be tracking
version numbers and all the complications that arise with branching and
non-linear numbering, b) should it just be exposed as a feature flag
instead and c) this will prevent hangs in certain specific situations
but it won't prevent the system running slowly and not using the full
capabilities of the hardware, for that we need to be making sure that
distros actually update to a firmware release that is not ancient.
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
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>
---
drivers/gpu/drm/i915/i915_getparam.c | 12 ++++++++++++
include/uapi/drm/i915_drm.h | 13 +++++++++++++
2 files changed, 25 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_getparam.c
b/drivers/gpu/drm/i915/i915_getparam.c
index 5c3fec63cb4c1..f176372debc54 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -113,6 +113,18 @@ int i915_getparam_ioctl(struct drm_device *dev,
void *data,
if (value < 0)
return value;
break;
+ case I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR:
+ case I915_PARAM_GUC_SUBMISSION_VERSION_MINOR:
+ case I915_PARAM_GUC_SUBMISSION_VERSION_PATCH:
+ if (!intel_uc_uses_guc_submission(&to_gt(i915)->uc))
+ return -ENODEV;
+ if (param->param == I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR)
+ value = to_gt(i915)->uc.guc.submission_version.major;
+ else if (param->param ==
I915_PARAM_GUC_SUBMISSION_VERSION_MINOR)
+ value = to_gt(i915)->uc.guc.submission_version.minor;
+ else
+ value = to_gt(i915)->uc.guc.submission_version.patch;
+ break;
case I915_PARAM_MMAP_GTT_VERSION:
/* Though we've started our numbering from 1, and so class all
* earlier versions as 0, in effect their value is
undefined as
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index fd4f9574d177a..7d5a47f182542 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -806,6 +806,19 @@ typedef struct drm_i915_irq_wait {
*/
#define I915_PARAM_PXP_STATUS 58
+/*
+ * Query for the GuC submission/VF interface version number
What is this VF you speak of? :/
Agreed. There is no SRIOV support in i915 so i915 should not be
mentioning SRIOV specific features.
John.
Regards,
Tvrtko
+ *
+ * -ENODEV is returned if GuC submission is not used
+ *
+ * On success, returns the respective GuC submission/VF interface
major,
+ * minor or patch version as per the requested parameter.
+ *
+ */
+#define I915_PARAM_GUC_SUBMISSION_VERSION_MAJOR 59
+#define I915_PARAM_GUC_SUBMISSION_VERSION_MINOR 60
+#define I915_PARAM_GUC_SUBMISSION_VERSION_PATCH 61
+
/* Must be kept compact -- no holes and well documented */
/**
