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

[Alessio Belle <Alessio.Belle@imgtec.com>]

Hi Brajesh,

On Tue, 2026-10-27 at 20:18 +0530, Brajesh Gupta wrote:
> 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>
> ---
> Changes in v3:
> - Add missed check for padding field added in v2.
> - Link to v2: https://lore.kernel.org/r/20261026-fwversion-v2-1-bb025c93a04a@imgtec.com
> 
> Changes in v2:
> - Add padding field to align new FW query struct to 64-bit.
> - Link to v1: https://patch.msgid.link/20261026-fwversion-v1-1-3ed17c65bf4c@imgtec.com
> 
> To: Alessio Belle <alessio.belle@imgtec.com>
> To: Luigi Santivetti <luigi.santivetti@imgtec.com>
> To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> To: Maxime Ripard <mripard@kernel.org>
> To: Thomas Zimmermann <tzimmermann@suse.de>
> To: David Airlie <airlied@gmail.com>
> To: Simona Vetter <simona@ffwll.ch>
> Cc: imagination@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/gpu/drm/imagination/pvr_drv.c |  5 +++++
>  drivers/gpu/drm/imagination/pvr_drv.h |  4 +++-
>  drivers/gpu/drm/imagination/pvr_fw.c  | 31 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/imagination/pvr_fw.h  |  3 +++
>  include/uapi/drm/pvr_drm.h            | 21 +++++++++++++++++++++
>  5 files changed, 63 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..4e058a6fdd5c 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, _padding_4) \
>  	)
>  
>  /**
> diff --git a/drivers/gpu/drm/imagination/pvr_fw.c b/drivers/gpu/drm/imagination/pvr_fw.c
> index c6c77b152342..34093d29baa0 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,33 @@ 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)

Could you add a description for this function? You can mostly recycle the ones
for pvr_dev_query_gpu_info_get() and pvr_dev_query_runtime_info_get(), except
that this cannot return E2BIG (nor can the others, but different story), instead
it can return EINVAL on non-zero reserved field, or if the input object is
smaller than the minimum required.

Thanks,
Alessio

> +{
> +	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;
> +	}
> +
> +	err = PVR_UOBJ_GET(fw_info, args->size, args->pointer);
> +	if (err < 0)
> +		return err;
> +	if (fw_info._padding_4)
> +		return -EINVAL;
> +
> +	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..1179c1e7d92d 100644
> --- a/include/uapi/drm/pvr_drm.h
> +++ b/include/uapi/drm/pvr_drm.h
> @@ -434,6 +434,21 @@ 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;
> +
> +	/** @_padding_4: Reserved. This field must be zeroed. */
> +	__u32 _padding_4;
> +};
> +
>  /**
>   * 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 +491,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>
>