[PATCH] drm/imagination: Support for querying Firmware information

[PATCH] drm/imagination: Support for querying Firmware information

[Brajesh Gupta]

Add a new subtype DRM_PVR_DEV_QUERY_FW_INFO_GET under
DRM_IOCTL_PVR_DEV_QUERY to query Firmware information. This allows the
userspace to detect any bug fixes and features present in the Firmware
and take appropriate action.
Currently it supports querying major and minor version of the Firmware.

Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_drv.c |  5 +++++
 drivers/gpu/drm/imagination/pvr_drv.h |  4 +++-
 drivers/gpu/drm/imagination/pvr_fw.c  | 25 +++++++++++++++++++++++++
 drivers/gpu/drm/imagination/pvr_fw.h  |  3 +++
 include/uapi/drm/pvr_drm.h            | 18 ++++++++++++++++++
 5 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
index 5c965ef0274f..2d522a8e46ad 100644
--- a/drivers/gpu/drm/imagination/pvr_drv.c
+++ b/drivers/gpu/drm/imagination/pvr_drv.c
@@ -6,6 +6,7 @@
 #include "pvr_device.h"
 #include "pvr_drv.h"
 #include "pvr_free_list.h"
+#include "pvr_fw.h"
 #include "pvr_gem.h"
 #include "pvr_hwrt.h"
 #include "pvr_job.h"
@@ -658,6 +659,10 @@ pvr_ioctl_dev_query(struct drm_device *drm_dev, void *raw_args,
 	case DRM_PVR_DEV_QUERY_STATIC_DATA_AREAS_GET:
 		ret = pvr_static_data_areas_get(pvr_dev, args);
 		break;
+
+	case DRM_PVR_DEV_QUERY_FW_INFO_GET:
+		ret = pvr_fw_info_get(pvr_dev, args);
+		break;
 	}
 
 	drm_dev_exit(idx);
diff --git a/drivers/gpu/drm/imagination/pvr_drv.h b/drivers/gpu/drm/imagination/pvr_drv.h
index 7fa147312dd1..fd01feeeda1d 100644
--- a/drivers/gpu/drm/imagination/pvr_drv.h
+++ b/drivers/gpu/drm/imagination/pvr_drv.h
@@ -13,9 +13,10 @@
 /*
  * Driver interface version:
  *  - 1.0: Initial interface
+ *  - 1.1: Add DRM_PVR_DEV_QUERY_FW_INFO_GET query
  */
 #define PVR_DRIVER_MAJOR 1
-#define PVR_DRIVER_MINOR 0
+#define PVR_DRIVER_MINOR 1
 #define PVR_DRIVER_PATCHLEVEL 0
 
 int pvr_get_uobj(u64 usr_ptr, u32 usr_size, u32 min_size, u32 obj_size, void *out);
@@ -68,6 +69,7 @@ int pvr_set_uobj_array(const struct drm_pvr_obj_array *out, u32 min_stride, u32
 	PVR_UOBJ_DECL(struct drm_pvr_dev_query_heap_info, heaps) \
 	PVR_UOBJ_DECL(struct drm_pvr_static_data_area, offset) \
 	PVR_UOBJ_DECL(struct drm_pvr_dev_query_static_data_areas, static_data_areas) \
+	PVR_UOBJ_DECL(struct drm_pvr_dev_query_fw_info, minor_version) \
 	)
 
 /**
diff --git a/drivers/gpu/drm/imagination/pvr_fw.c b/drivers/gpu/drm/imagination/pvr_fw.c
index c6c77b152342..1f21e6adaff2 100644
--- a/drivers/gpu/drm/imagination/pvr_fw.c
+++ b/drivers/gpu/drm/imagination/pvr_fw.c
@@ -4,6 +4,7 @@
 #include "pvr_ccb.h"
 #include "pvr_device.h"
 #include "pvr_device_info.h"
+#include "pvr_drv.h"
 #include "pvr_fw.h"
 #include "pvr_fw_info.h"
 #include "pvr_fw_startstop.h"
@@ -1496,3 +1497,27 @@ pvr_fw_hard_reset(struct pvr_device *pvr_dev)
 
 	return 0;
 }
+
+int
+pvr_fw_info_get(const struct pvr_device *pvr_dev,
+		struct drm_pvr_ioctl_dev_query_args *args)
+{
+	struct drm_pvr_dev_query_fw_info fw_info = {0};
+	int err;
+
+	if (!args->pointer) {
+		args->size = sizeof(struct drm_pvr_dev_query_fw_info);
+		return 0;
+	}
+
+	fw_info.major_version = pvr_dev->fw_version.major;
+	fw_info.minor_version = pvr_dev->fw_version.minor;
+
+	err = PVR_UOBJ_SET(args->pointer, args->size, fw_info);
+	if (err < 0)
+		return err;
+
+	if (args->size > sizeof(fw_info))
+		args->size = sizeof(fw_info);
+	return 0;
+}
diff --git a/drivers/gpu/drm/imagination/pvr_fw.h b/drivers/gpu/drm/imagination/pvr_fw.h
index 4b25291135b6..7e18f3e7d4be 100644
--- a/drivers/gpu/drm/imagination/pvr_fw.h
+++ b/drivers/gpu/drm/imagination/pvr_fw.h
@@ -496,4 +496,7 @@ int
 pvr_fw_process_elf_command_stream(struct pvr_device *pvr_dev, const u8 *fw, u8 *fw_code_ptr,
 				  u8 *fw_data_ptr, u8 *fw_core_code_ptr, u8 *fw_core_data_ptr);
 
+int
+pvr_fw_info_get(const struct pvr_device *pvr_dev, struct drm_pvr_ioctl_dev_query_args *args);
+
 #endif /* PVR_FW_H */
diff --git a/include/uapi/drm/pvr_drm.h b/include/uapi/drm/pvr_drm.h
index ccf6c2112468..94281af974c7 100644
--- a/include/uapi/drm/pvr_drm.h
+++ b/include/uapi/drm/pvr_drm.h
@@ -434,6 +434,18 @@ struct drm_pvr_dev_query_static_data_areas {
 	struct drm_pvr_obj_array static_data_areas;
 };
 
+/**
+ * struct drm_pvr_dev_query_fw_info - Container used to fetch information
+ * about the GPU Firmware like its version.
+ */
+struct drm_pvr_dev_query_fw_info {
+	/** @major_version: Major version of the Firmware. */
+	__u16 major_version;
+
+	/** @minor_version: Minor version of the Firmware. */
+	__u16 minor_version;
+};
+
 /**
  * enum drm_pvr_dev_query - For use with &drm_pvr_ioctl_dev_query_args.type to
  * indicate the type of the receiving container.
@@ -476,6 +488,12 @@ enum drm_pvr_dev_query {
 	 * a pointer to &struct drm_pvr_dev_query_static_data_areas.
 	 */
 	DRM_PVR_DEV_QUERY_STATIC_DATA_AREAS_GET,
+
+	/**
+	 * @DRM_PVR_DEV_QUERY_FW_INFO_GET: The dev query args contain
+	 * a pointer to &struct drm_pvr_dev_query_fw_info.
+	 */
+	DRM_PVR_DEV_QUERY_FW_INFO_GET,
 };
 
 /**

---
base-commit: a8fe4e162ba8212b7e51fdb024b37800f36b496c
change-id: 20261026-fwversion-95a82a91f9f7

Best regards,
--  
Brajesh Gupta <brajesh.gupta@imgtec.com>

Re: [PATCH] drm/imagination: Support for querying Firmware information

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/imagination: missing 64-bit padding in struct drm_pvr_dev_query_fw_info
--

commit 1360941b5bdd5b35c5e7870e3bae3cb8d37c65fd
Author: Brajesh Gupta <brajesh.gupta@imgtec.com>

drm/imagination: Support for querying Firmware information

This commit adds a new dev query subtype to query firmware version
information from the driver.

> diff --git a/drivers/gpu/drm/imagination/pvr_drv.h b/drivers/gpu/drm/imagination/pvr_drv.h
> index 7fa147312dd11..fd01feeeda1d4 100644
> --- a/drivers/gpu/drm/imagination/pvr_drv.h
> +++ b/drivers/gpu/drm/imagination/pvr_drv.h
[ ... ]
> @@ -68,6 +69,7 @@ int pvr_set_uobj_array(const struct drm_pvr_obj_array *out, u32 min_stride, u32
>  	PVR_UOBJ_DECL(struct drm_pvr_dev_query_heap_info, heaps) \
>  	PVR_UOBJ_DECL(struct drm_pvr_static_data_area, offset) \
>  	PVR_UOBJ_DECL(struct drm_pvr_dev_query_static_data_areas, static_data_areas) \
> +	PVR_UOBJ_DECL(struct drm_pvr_dev_query_fw_info, minor_version) \
>  	)
>  
>  /**
[ ... ]
> diff --git a/include/uapi/drm/pvr_drm.h b/include/uapi/drm/pvr_drm.h
> index ccf6c2112468f..94281af974c78 100644
> --- a/include/uapi/drm/pvr_drm.h
> +++ b/include/uapi/drm/pvr_drm.h
> @@ -434,6 +434,18 @@ struct drm_pvr_dev_query_static_data_areas {
>  	struct drm_pvr_obj_array static_data_areas;
>  };
>  
> +/**
> + * struct drm_pvr_dev_query_fw_info - Container used to fetch information
> + * about the GPU Firmware like its version.
> + */
> +struct drm_pvr_dev_query_fw_info {
> +	/** @major_version: Major version of the Firmware. */
> +	__u16 major_version;
> +
> +	/** @minor_version: Minor version of the Firmware. */
> +	__u16 minor_version;
> +};

[Severity: Medium]
Should struct drm_pvr_dev_query_fw_info include an explicit __u32 _padding_4
field to pad the structure to 64-bit alignment, and pass _padding_4 as the
last member in the pvr_drv.h declaration macro?

The documentation at the top of include/uapi/drm/pvr_drm.h states that all
ioctl and dev query structs must be padded to 64-bit alignment using explicit
_padding_[x] fields:

include/uapi/drm/pvr_drm.h:
 *  - All members must be type-aligned.
 *  - The overall struct must be padded to 64-bit alignment.
 *  - Explicit padding is almost always required. This takes the form of
 *    ``_padding_[x]`` members of sufficient size to pad to the next power-of-two
 *    alignment, where [x] is the offset into the struct in hexadecimal. Arrays
 *    are never used for alignment. Padding fields must be zeroed; this is
 *    always checked.
 ...
 * In addition to the IOCTL argument structs, the PowerVR UAPI makes use of
 * DEV_QUERY argument structs. These are used to fetch information about the
 * device and runtime. These structs are subject to the same rules set out
 * above.

As defined, struct drm_pvr_dev_query_fw_info is only 4 bytes, so
pvr_ioctl_dev_query() -> pvr_fw_info_get() reports a size of 4 when
args->pointer is zero and copies out a 4-byte structure.

If a 64-bit field is appended to struct drm_pvr_dev_query_fw_info in the
future, leaving it at 4 bytes now would introduce implicit compiler padding at
offset 4 on 64-bit architectures and cause a 32-bit versus 64-bit layout
mismatch.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-fwversion-v1-1-3ed17c65bf4c@imgtec.com?part=1