* [Patch v1] drm/i915: Add uAPI to query micro-controller FW version
@ 2023-09-27 4:14 ` Balasubrawmanian, Vivaik
0 siblings, 0 replies; 7+ messages in thread
From: Balasubrawmanian, Vivaik @ 2023-09-27 4:14 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel, daniele.ceraolospurio, jose.souza, john.c.harrison
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
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Vivaik Balasubrawmanian <vivaik.balasubrawmanian@intel.com>
---
drivers/gpu/drm/i915/i915_query.c | 47 +++++++++++++++++++++++++++++++
include/uapi/drm/i915_drm.h | 32 +++++++++++++++++++++
2 files changed, 79 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_query.c
b/drivers/gpu/drm/i915/i915_query.c
index 00871ef99792..7f22a49faae7 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -551,6 +551,52 @@ static int query_hwconfig_blob(struct
drm_i915_private *i915,
return hwconfig->size;
}
+static int
+query_uc_fw_version(struct drm_i915_private *i915, struct
drm_i915_query_item *query)
+{
+ struct drm_i915_query_uc_fw_version __user *query_ptr =
u64_to_user_ptr(query->data_ptr);
+ size_t size = sizeof(struct drm_i915_query_uc_fw_version);
+ struct drm_i915_query_uc_fw_version resp;
+
+ if (query->length == 0) {
+ query->length = size;
+ return 0;
+ } else if (query->length != size) {
+ drm_dbg(&i915->drm,
+ "Invalid uc_fw_version query item size=%u expected=%zu\n",
+ query->length, size);
+ return -EINVAL;
+ }
+
+ if (copy_from_user(&resp, query_ptr, size))
+ return -EFAULT;
+
+ if (resp.pad || resp.pad2 || resp.reserved) {
+ drm_dbg(&i915->drm,
+ "Invalid input fw version query structure parameters
received");
+ return -EINVAL;
+ }
+
+ switch (resp.uc_type) {
+ case I915_QUERY_UC_TYPE_GUC: {
+ struct intel_guc *guc = &i915->gt0.uc.guc;
+
+ resp.major_ver = guc->submission_version.major;
+ resp.minor_ver = guc->submission_version.minor;
+ resp.patch_ver = guc->submission_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 i915_query_funcs[])(struct drm_i915_private *dev_priv,
struct drm_i915_query_item *query_item) = {
query_topology_info,
@@ -559,6 +605,7 @@ static int (* const i915_query_funcs[])(struct
drm_i915_private *dev_priv,
query_memregion_info,
query_hwconfig_blob,
query_geometry_subslices,
+ query_uc_fw_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 7000e5910a1d..9be241fb77d8 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -3013,6 +3013,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_UC_FW_VERSION (see struct
drm_i915_query_uc_fw_version)
*/
__u64 query_id;
#define DRM_I915_QUERY_TOPOLOGY_INFO 1
@@ -3021,6 +3022,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_UC_FW_VERSION 7
/* Must be kept compact -- no holes and well documented */
/**
@@ -3213,6 +3215,36 @@ struct drm_i915_query_topology_info {
__u8 data[];
};
+/**
+* struct drm_i915_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_i915_query_uc_fw_version {
+ /** @uc: The micro-controller type to query firmware version */
+#define I915_QUERY_UC_TYPE_GUC 0
+ __u16 uc_type;
+
+ /** @pad: MBZ */
+ __u16 pad;
+
+ /* @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;
+};
+
/**
* DOC: Engine Discovery uAPI
*
base-commit: a42554bf0755b80fdfb8e91ca35ae6835bb3534d
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Add uAPI to query micro-controller FW version
2023-09-27 4:14 ` Balasubrawmanian, Vivaik
(?)
@ 2023-09-27 5:02 ` Patchwork
-1 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2023-09-27 5:02 UTC (permalink / raw)
To: Balasubrawmanian, Vivaik; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Add uAPI to query micro-controller FW version
URL : https://patchwork.freedesktop.org/series/124307/
State : failure
== Summary ==
Error: patch https://patchwork.freedesktop.org/api/1.0/series/124307/revisions/1/mbox/ not applied
Applying: drm/i915: Add uAPI to query micro-controller FW version
error: patch fragment without header at line 9: @@ -3021,6 +3022,7 @@ struct drm_i915_query_item {
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915: Add uAPI to query micro-controller FW version
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Build failed, no error log produced
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Intel-gfx] [Patch v1] drm/i915: Add uAPI to query micro-controller FW version
2023-09-27 4:14 ` Balasubrawmanian, Vivaik
@ 2023-09-27 8:22 ` Jani Nikula
-1 siblings, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2023-09-27 8:22 UTC (permalink / raw)
To: Balasubrawmanian, Vivaik, intel-gfx; +Cc: dri-devel
On Tue, 26 Sep 2023, "Balasubrawmanian, Vivaik" <vivaik.balasubrawmanian@intel.com> wrote:
> 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
>
>
> Cc: John Harrison <John.C.Harrison@Intel.com>
>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>
> Cc: José Roberto de Souza <jose.souza@intel.com>
>
> Signed-off-by: Vivaik Balasubrawmanian <vivaik.balasubrawmanian@intel.com>
Please use git send-email to send patches. This is corrupted.
BR,
Jani.
> ---
> drivers/gpu/drm/i915/i915_query.c | 47 +++++++++++++++++++++++++++++++
> include/uapi/drm/i915_drm.h | 32 +++++++++++++++++++++
> 2 files changed, 79 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_query.c
> b/drivers/gpu/drm/i915/i915_query.c
> index 00871ef99792..7f22a49faae7 100644
> --- a/drivers/gpu/drm/i915/i915_query.c
> +++ b/drivers/gpu/drm/i915/i915_query.c
> @@ -551,6 +551,52 @@ static int query_hwconfig_blob(struct
> drm_i915_private *i915,
> return hwconfig->size;
> }
>
> +static int
> +query_uc_fw_version(struct drm_i915_private *i915, struct
> drm_i915_query_item *query)
> +{
> + struct drm_i915_query_uc_fw_version __user *query_ptr =
> u64_to_user_ptr(query->data_ptr);
> + size_t size = sizeof(struct drm_i915_query_uc_fw_version);
> + struct drm_i915_query_uc_fw_version resp;
> +
> + if (query->length == 0) {
> + query->length = size;
> + return 0;
> + } else if (query->length != size) {
> + drm_dbg(&i915->drm,
> + "Invalid uc_fw_version query item size=%u expected=%zu\n",
> + query->length, size);
> + return -EINVAL;
> + }
> +
> + if (copy_from_user(&resp, query_ptr, size))
> + return -EFAULT;
> +
> + if (resp.pad || resp.pad2 || resp.reserved) {
> + drm_dbg(&i915->drm,
> + "Invalid input fw version query structure parameters
> received");
> + return -EINVAL;
> + }
> +
> + switch (resp.uc_type) {
> + case I915_QUERY_UC_TYPE_GUC: {
> + struct intel_guc *guc = &i915->gt0.uc.guc;
> +
> + resp.major_ver = guc->submission_version.major;
> + resp.minor_ver = guc->submission_version.minor;
> + resp.patch_ver = guc->submission_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 i915_query_funcs[])(struct drm_i915_private *dev_priv,
> struct drm_i915_query_item *query_item) = {
> query_topology_info,
> @@ -559,6 +605,7 @@ static int (* const i915_query_funcs[])(struct
> drm_i915_private *dev_priv,
> query_memregion_info,
> query_hwconfig_blob,
> query_geometry_subslices,
> + query_uc_fw_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 7000e5910a1d..9be241fb77d8 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -3013,6 +3013,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_UC_FW_VERSION (see struct
> drm_i915_query_uc_fw_version)
> */
> __u64 query_id;
> #define DRM_I915_QUERY_TOPOLOGY_INFO 1
> @@ -3021,6 +3022,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_UC_FW_VERSION 7
> /* Must be kept compact -- no holes and well documented */
>
> /**
> @@ -3213,6 +3215,36 @@ struct drm_i915_query_topology_info {
> __u8 data[];
> };
>
> +/**
> +* struct drm_i915_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_i915_query_uc_fw_version {
> + /** @uc: The micro-controller type to query firmware version */
> +#define I915_QUERY_UC_TYPE_GUC 0
> + __u16 uc_type;
> +
> + /** @pad: MBZ */
> + __u16 pad;
> +
> + /* @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;
> +};
> +
> /**
> * DOC: Engine Discovery uAPI
> *
>
> base-commit: a42554bf0755b80fdfb8e91ca35ae6835bb3534d
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Patch v1] drm/i915: Add uAPI to query micro-controller FW version
@ 2023-09-27 8:22 ` Jani Nikula
0 siblings, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2023-09-27 8:22 UTC (permalink / raw)
To: Balasubrawmanian, Vivaik, intel-gfx
Cc: john.c.harrison, daniele.ceraolospurio, jose.souza, dri-devel
On Tue, 26 Sep 2023, "Balasubrawmanian, Vivaik" <vivaik.balasubrawmanian@intel.com> wrote:
> 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
>
>
> Cc: John Harrison <John.C.Harrison@Intel.com>
>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>
> Cc: José Roberto de Souza <jose.souza@intel.com>
>
> Signed-off-by: Vivaik Balasubrawmanian <vivaik.balasubrawmanian@intel.com>
Please use git send-email to send patches. This is corrupted.
BR,
Jani.
> ---
> drivers/gpu/drm/i915/i915_query.c | 47 +++++++++++++++++++++++++++++++
> include/uapi/drm/i915_drm.h | 32 +++++++++++++++++++++
> 2 files changed, 79 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_query.c
> b/drivers/gpu/drm/i915/i915_query.c
> index 00871ef99792..7f22a49faae7 100644
> --- a/drivers/gpu/drm/i915/i915_query.c
> +++ b/drivers/gpu/drm/i915/i915_query.c
> @@ -551,6 +551,52 @@ static int query_hwconfig_blob(struct
> drm_i915_private *i915,
> return hwconfig->size;
> }
>
> +static int
> +query_uc_fw_version(struct drm_i915_private *i915, struct
> drm_i915_query_item *query)
> +{
> + struct drm_i915_query_uc_fw_version __user *query_ptr =
> u64_to_user_ptr(query->data_ptr);
> + size_t size = sizeof(struct drm_i915_query_uc_fw_version);
> + struct drm_i915_query_uc_fw_version resp;
> +
> + if (query->length == 0) {
> + query->length = size;
> + return 0;
> + } else if (query->length != size) {
> + drm_dbg(&i915->drm,
> + "Invalid uc_fw_version query item size=%u expected=%zu\n",
> + query->length, size);
> + return -EINVAL;
> + }
> +
> + if (copy_from_user(&resp, query_ptr, size))
> + return -EFAULT;
> +
> + if (resp.pad || resp.pad2 || resp.reserved) {
> + drm_dbg(&i915->drm,
> + "Invalid input fw version query structure parameters
> received");
> + return -EINVAL;
> + }
> +
> + switch (resp.uc_type) {
> + case I915_QUERY_UC_TYPE_GUC: {
> + struct intel_guc *guc = &i915->gt0.uc.guc;
> +
> + resp.major_ver = guc->submission_version.major;
> + resp.minor_ver = guc->submission_version.minor;
> + resp.patch_ver = guc->submission_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 i915_query_funcs[])(struct drm_i915_private *dev_priv,
> struct drm_i915_query_item *query_item) = {
> query_topology_info,
> @@ -559,6 +605,7 @@ static int (* const i915_query_funcs[])(struct
> drm_i915_private *dev_priv,
> query_memregion_info,
> query_hwconfig_blob,
> query_geometry_subslices,
> + query_uc_fw_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 7000e5910a1d..9be241fb77d8 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -3013,6 +3013,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_UC_FW_VERSION (see struct
> drm_i915_query_uc_fw_version)
> */
> __u64 query_id;
> #define DRM_I915_QUERY_TOPOLOGY_INFO 1
> @@ -3021,6 +3022,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_UC_FW_VERSION 7
> /* Must be kept compact -- no holes and well documented */
>
> /**
> @@ -3213,6 +3215,36 @@ struct drm_i915_query_topology_info {
> __u8 data[];
> };
>
> +/**
> +* struct drm_i915_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_i915_query_uc_fw_version {
> + /** @uc: The micro-controller type to query firmware version */
> +#define I915_QUERY_UC_TYPE_GUC 0
> + __u16 uc_type;
> +
> + /** @pad: MBZ */
> + __u16 pad;
> +
> + /* @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;
> +};
> +
> /**
> * DOC: Engine Discovery uAPI
> *
>
> base-commit: a42554bf0755b80fdfb8e91ca35ae6835bb3534d
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [Patch v1] drm/i915: Add uAPI to query micro-controller FW version
2023-09-27 4:14 ` Balasubrawmanian, Vivaik
` (2 preceding siblings ...)
(?)
@ 2023-09-27 9:20 ` Tvrtko Ursulin
2023-09-28 4:46 ` Balasubrawmanian, Vivaik
-1 siblings, 1 reply; 7+ messages in thread
From: Tvrtko Ursulin @ 2023-09-27 9:20 UTC (permalink / raw)
To: Balasubrawmanian, Vivaik, intel-gfx; +Cc: dri-devel
On 27/09/2023 05:14, Balasubrawmanian, Vivaik wrote:
> 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.
Is the firmware bug making the ccs engines generally useless, or just
not suitable for this specific Mesa use case?
> 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
>
>
> Cc: John Harrison <John.C.Harrison@Intel.com>
>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>
> Cc: José Roberto de Souza <jose.souza@intel.com>
>
> Signed-off-by: Vivaik Balasubrawmanian <vivaik.balasubrawmanian@intel.com>
> ---
> drivers/gpu/drm/i915/i915_query.c | 47 +++++++++++++++++++++++++++++++
> include/uapi/drm/i915_drm.h | 32 +++++++++++++++++++++
> 2 files changed, 79 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_query.c
> b/drivers/gpu/drm/i915/i915_query.c
> index 00871ef99792..7f22a49faae7 100644
> --- a/drivers/gpu/drm/i915/i915_query.c
> +++ b/drivers/gpu/drm/i915/i915_query.c
> @@ -551,6 +551,52 @@ static int query_hwconfig_blob(struct
> drm_i915_private *i915,
> return hwconfig->size;
> }
>
> +static int
> +query_uc_fw_version(struct drm_i915_private *i915, struct
> drm_i915_query_item *query)
> +{
> + struct drm_i915_query_uc_fw_version __user *query_ptr =
> u64_to_user_ptr(query->data_ptr);
> + size_t size = sizeof(struct drm_i915_query_uc_fw_version);
> + struct drm_i915_query_uc_fw_version resp;
> +
> + if (query->length == 0) {
> + query->length = size;
> + return 0;
> + } else if (query->length != size) {
> + drm_dbg(&i915->drm,
> + "Invalid uc_fw_version query item size=%u expected=%zu\n",
> + query->length, size);
> + return -EINVAL;
> + }
> +
> + if (copy_from_user(&resp, query_ptr, size))
> + return -EFAULT;
The above can probably be replaced by using the copy_query_item() helper
and it would work a bit better even since no reason to reject a buffer
too large.
> +
> + if (resp.pad || resp.pad2 || resp.reserved) {
> + drm_dbg(&i915->drm,
> + "Invalid input fw version query structure parameters
> received");
> + return -EINVAL;
> + }
> +
> + switch (resp.uc_type) {
> + case I915_QUERY_UC_TYPE_GUC: {
> + struct intel_guc *guc = &i915->gt0.uc.guc;
> +
> + resp.major_ver = guc->submission_version.major;
> + resp.minor_ver = guc->submission_version.minor;
> + resp.patch_ver = guc->submission_version.patch;
Submission version is not the same as fw version, right? So
DRM_I915_QUERY_UC_FW_VERSION and uapi kerneldoc is misleading.
Name the query type I915_QUERY_UC_TYPE_GUC*_SUBMISSION* and make it clear?
Regards,
Tvrtko
> + resp.branch_ver = 0;
> + break;
> + }
> + default:
> + return -EINVAL;
> + }
> +
> + if (copy_to_user(query_ptr, &resp, 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 +605,7 @@ static int (* const i915_query_funcs[])(struct
> drm_i915_private *dev_priv,
> query_memregion_info,
> query_hwconfig_blob,
> query_geometry_subslices,
> + query_uc_fw_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 7000e5910a1d..9be241fb77d8 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -3013,6 +3013,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_UC_FW_VERSION (see struct
> drm_i915_query_uc_fw_version)
> */
> __u64 query_id;
> #define DRM_I915_QUERY_TOPOLOGY_INFO 1
> @@ -3021,6 +3022,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_UC_FW_VERSION 7
> /* Must be kept compact -- no holes and well documented */
>
> /**
> @@ -3213,6 +3215,36 @@ struct drm_i915_query_topology_info {
> __u8 data[];
> };
>
> +/**
> +* struct drm_i915_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_i915_query_uc_fw_version {
> + /** @uc: The micro-controller type to query firmware version */
> +#define I915_QUERY_UC_TYPE_GUC 0
> + __u16 uc_type;
> +
> + /** @pad: MBZ */
> + __u16 pad;
> +
> + /* @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;
> +};
> +
> /**
> * DOC: Engine Discovery uAPI
> *
>
> base-commit: a42554bf0755b80fdfb8e91ca35ae6835bb3534d
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Intel-gfx] [Patch v1] drm/i915: Add uAPI to query micro-controller FW version
2023-09-27 9:20 ` [Intel-gfx] " Tvrtko Ursulin
@ 2023-09-28 4:46 ` Balasubrawmanian, Vivaik
0 siblings, 0 replies; 7+ messages in thread
From: Balasubrawmanian, Vivaik @ 2023-09-28 4:46 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: dri-devel
On 9/27/2023 2:20 AM, Tvrtko Ursulin wrote:
>
> On 27/09/2023 05:14, Balasubrawmanian, Vivaik wrote:
>> 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.
>
> Is the firmware bug making the ccs engines generally useless, or just
> not suitable for this specific Mesa use case?
My understanding is that this issue was found as part of this specific
use case (enabling async compute queues on DG2).
We are adding the new uAPI to cover for similar situations in the
future. Will clarify in the description above.
>
>> 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
>>
>>
>> Cc: John Harrison <John.C.Harrison@Intel.com>
>>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>
>> Cc: José Roberto de Souza <jose.souza@intel.com>
>>
>> Signed-off-by: Vivaik Balasubrawmanian
>> <vivaik.balasubrawmanian@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_query.c | 47 +++++++++++++++++++++++++++++++
>> include/uapi/drm/i915_drm.h | 32 +++++++++++++++++++++
>> 2 files changed, 79 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_query.c
>> b/drivers/gpu/drm/i915/i915_query.c
>> index 00871ef99792..7f22a49faae7 100644
>> --- a/drivers/gpu/drm/i915/i915_query.c
>> +++ b/drivers/gpu/drm/i915/i915_query.c
>> @@ -551,6 +551,52 @@ static int query_hwconfig_blob(struct
>> drm_i915_private *i915,
>> return hwconfig->size;
>> }
>>
>> +static int
>> +query_uc_fw_version(struct drm_i915_private *i915, struct
>> drm_i915_query_item *query)
>> +{
>> + struct drm_i915_query_uc_fw_version __user *query_ptr =
>> u64_to_user_ptr(query->data_ptr);
>> + size_t size = sizeof(struct drm_i915_query_uc_fw_version);
>> + struct drm_i915_query_uc_fw_version resp;
>> +
>> + if (query->length == 0) {
>> + query->length = size;
>> + return 0;
>> + } else if (query->length != size) {
>> + drm_dbg(&i915->drm,
>> + "Invalid uc_fw_version query item size=%u expected=%zu\n",
>> + query->length, size);
>> + return -EINVAL;
>> + }
>> +
>> + if (copy_from_user(&resp, query_ptr, size))
>> + return -EFAULT;
>
> The above can probably be replaced by using the copy_query_item()
> helper and it would work a bit better even since no reason to reject a
> buffer too large.
>
Yes makes sense. Will send out an updated revision.
>> +
>> + if (resp.pad || resp.pad2 || resp.reserved) {
>> + drm_dbg(&i915->drm,
>> + "Invalid input fw version query structure parameters
>> received");
>> + return -EINVAL;
>> + }
>> +
>> + switch (resp.uc_type) {
>> + case I915_QUERY_UC_TYPE_GUC: {
>> + struct intel_guc *guc = &i915->gt0.uc.guc;
>> +
>> + resp.major_ver = guc->submission_version.major;
>> + resp.minor_ver = guc->submission_version.minor;
>> + resp.patch_ver = guc->submission_version.patch;
>
> Submission version is not the same as fw version, right? So
> DRM_I915_QUERY_UC_FW_VERSION and uapi kerneldoc is misleading.
>
> Name the query type I915_QUERY_UC_TYPE_GUC*_SUBMISSION* and make it
> clear?
>
> Regards,
>
> Tvrtko
I think this makes sense. The use case requires a read of submission
version vs. the file version. Will update in the next revision.
>
>> + resp.branch_ver = 0;
>> + break;
>> + }
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + if (copy_to_user(query_ptr, &resp, 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 +605,7 @@ static int (* const i915_query_funcs[])(struct
>> drm_i915_private *dev_priv,
>> query_memregion_info,
>> query_hwconfig_blob,
>> query_geometry_subslices,
>> + query_uc_fw_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 7000e5910a1d..9be241fb77d8 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -3013,6 +3013,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_UC_FW_VERSION (see struct
>> drm_i915_query_uc_fw_version)
>> */
>> __u64 query_id;
>> #define DRM_I915_QUERY_TOPOLOGY_INFO 1
>> @@ -3021,6 +3022,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_UC_FW_VERSION 7
>> /* Must be kept compact -- no holes and well documented */
>>
>> /**
>> @@ -3213,6 +3215,36 @@ struct drm_i915_query_topology_info {
>> __u8 data[];
>> };
>>
>> +/**
>> +* struct drm_i915_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_i915_query_uc_fw_version {
>> + /** @uc: The micro-controller type to query firmware version */
>> +#define I915_QUERY_UC_TYPE_GUC 0
>> + __u16 uc_type;
>> +
>> + /** @pad: MBZ */
>> + __u16 pad;
>> +
>> + /* @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;
>> +};
>> +
>> /**
>> * DOC: Engine Discovery uAPI
>> *
>>
>> base-commit: a42554bf0755b80fdfb8e91ca35ae6835bb3534d
Thank you Tvrtko for the review and feedback. Deeply appreciated. - VB.
^ permalink raw reply [flat|nested] 7+ messages in thread