[PATCH] drm/imagination: Check virtual address space size only once

[PATCH] drm/imagination: Check virtual address space size only once

[Alexandru Dadu]

From: Alessio Belle <alessio.belle@imgtec.com>

Every time a virtual memory context is created, the driver reads the
size of the virtual address space, as reported by the firmware binary,
and compares it against a define for the same, but both values are fixed
on a given core, and on top of that, all known Imagination GPU cores use
the same 40-bit virtual address space.

Move the check above to a new function for GPU device info validation
code, so it's performed only once.
This also allows other areas of the driver to reference the relevant
defines without having to perform other checks at runtime, as long as
all cores keep using the same virtual address space.

Signed-off-by: Alessio Belle <alessio.belle@imgtec.com>
Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_device.c | 43 +++++++++++++++++++++++++++++---
 drivers/gpu/drm/imagination/pvr_mmu.h    |  2 +-
 drivers/gpu/drm/imagination/pvr_vm.c     | 22 +---------------
 3 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
index 35eaa54f75ed..992509d95d28 100644
--- a/drivers/gpu/drm/imagination/pvr_device.c
+++ b/drivers/gpu/drm/imagination/pvr_device.c
@@ -5,6 +5,7 @@
 #include "pvr_device_info.h"
 
 #include "pvr_fw.h"
+#include "pvr_mmu.h"
 #include "pvr_power.h"
 #include "pvr_queue.h"
 #include "pvr_rogue_cr_defs.h"
@@ -646,6 +647,36 @@ pvr_set_dma_info(struct pvr_device *pvr_dev)
 	return 0;
 }
 
+/**
+ * pvr_device_info_validate() - Validate PowerVR device information, read from
+ * the firmware binary, against what is supported by the driver.
+ * @pvr_dev: Target PowerVR device.
+ *
+ * Returns:
+ *  * 0 on success, or
+ *  * -%EINVAL if the validation failed.
+ */
+static int
+pvr_device_info_validate(struct pvr_device *pvr_dev)
+{
+	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
+	u16 device_addr_bits;
+	int err;
+
+	err = PVR_FEATURE_VALUE(pvr_dev, virtual_address_space_bits, &device_addr_bits);
+	if (err) {
+		drm_err(drm_dev, "Failed to get device virtual address space bits\n");
+		return err;
+	}
+
+	if (device_addr_bits != PVR_PAGE_TABLE_ADDR_BITS) {
+		drm_err(drm_dev, "Device has unsupported virtual address space size\n");
+		return -EINVAL;
+	}
+
+	return err;
+}
+
 /**
  * pvr_device_gpu_init() - GPU-specific initialization for a PowerVR device
  * @pvr_dev: Target PowerVR device.
@@ -661,9 +692,11 @@ pvr_set_dma_info(struct pvr_device *pvr_dev)
  * Return:
  *  * 0 on success,
  *  * -%ENODEV if the GPU is not supported,
- *  * Any error returned by pvr_set_dma_info(),
- *  * Any error returned by pvr_memory_context_init(), or
- *  * Any error returned by pvr_request_firmware().
+ *  * Any error returned by pvr_request_firmware(),
+ *  * Any error returned by pvr_fw_validate_init_device_info(),
+ *  * Any error returned by pvr_device_info_validate(),
+ *  * Any error returned by pvr_set_dma_info(), or
+ *  * Any error returned by pvr_fw_init().
  */
 static int
 pvr_device_gpu_init(struct pvr_device *pvr_dev)
@@ -691,6 +724,10 @@ pvr_device_gpu_init(struct pvr_device *pvr_dev)
 	else
 		return -EINVAL;
 
+	err = pvr_device_info_validate(pvr_dev);
+	if (err)
+		return err;
+
 	pvr_stream_create_musthave_masks(pvr_dev);
 
 	err = pvr_set_dma_info(pvr_dev);
diff --git a/drivers/gpu/drm/imagination/pvr_mmu.h b/drivers/gpu/drm/imagination/pvr_mmu.h
index a8ecd460168d..1f9ae00a8b17 100644
--- a/drivers/gpu/drm/imagination/pvr_mmu.h
+++ b/drivers/gpu/drm/imagination/pvr_mmu.h
@@ -62,7 +62,7 @@ struct sg_table;
  *
  *    This value is checked at runtime against
  *    &pvr_device_features.virtual_address_space_bits by
- *    pvr_vm_create_context(), which will return an error if the feature value
+ *    pvr_device_info_validate(), which will return an error if the feature value
  *    does not match this constant.
  *
  *    .. admonition:: Future work
diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
index ceb78694cd98..304d0743c5aa 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.c
+++ b/drivers/gpu/drm/imagination/pvr_vm.c
@@ -535,8 +535,6 @@ fw_mem_context_init(void *cpu_ptr, void *priv)
  *
  * Return:
  *  * A handle to the newly-minted VM context on success,
- *  * -%EINVAL if the feature "virtual address space bits" on @pvr_dev is
- *    missing or has an unsupported value,
  *  * -%ENOMEM if allocation of the structure behind the opaque handle fails,
  *    or
  *  * Any error encountered while setting up internal structures.
@@ -544,27 +542,9 @@ fw_mem_context_init(void *cpu_ptr, void *priv)
 struct pvr_vm_context *
 pvr_vm_create_context(struct pvr_device *pvr_dev, bool is_userspace_context)
 {
-	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
-
 	struct pvr_vm_context *vm_ctx;
-	u16 device_addr_bits;
-
 	int err;
 
-	err = PVR_FEATURE_VALUE(pvr_dev, virtual_address_space_bits,
-				&device_addr_bits);
-	if (err) {
-		drm_err(drm_dev,
-			"Failed to get device virtual address space bits\n");
-		return ERR_PTR(err);
-	}
-
-	if (device_addr_bits != PVR_PAGE_TABLE_ADDR_BITS) {
-		drm_err(drm_dev,
-			"Device has unsupported virtual address space size\n");
-		return ERR_PTR(-EINVAL);
-	}
-
 	vm_ctx = kzalloc_obj(*vm_ctx);
 	if (!vm_ctx)
 		return ERR_PTR(-ENOMEM);
@@ -589,7 +569,7 @@ pvr_vm_create_context(struct pvr_device *pvr_dev, bool is_userspace_context)
 	drm_gpuvm_init(&vm_ctx->gpuvm_mgr,
 		       is_userspace_context ? "PowerVR-user-VM" : "PowerVR-FW-VM",
 		       0, &pvr_dev->base, &vm_ctx->dummy_gem,
-		       0, 1ULL << device_addr_bits, 0, 0, &pvr_vm_gpuva_ops);
+		       0, PVR_PAGE_TABLE_ADDR_SPACE_SIZE, 0, 0, &pvr_vm_gpuva_ops);
 
 	mutex_init(&vm_ctx->lock);
 	kref_init(&vm_ctx->ref_count);

---
base-commit: 6e375de99d0c420169481fcd36064177ab55b09d
change-id: 20261006-b4-check-virtual-addr-space-size-once-044265ce3fa3

Best regards,
--  
Alexandru Dadu <alexandru.dadu@imgtec.com>

Re: [PATCH] drm/imagination: Check virtual address space size only once

[Brajesh Gupta]

On Tue, 2026-10-06 at 11:08 +0300, Alexandru Dadu wrote:
Hi Dadu,
> From: Alessio Belle <alessio.belle@imgtec.com>
> 
> Every time a virtual memory context is created, the driver reads the
> size of the virtual address space, as reported by the firmware binary,
> and compares it against a define for the same, but both values are fixed
> on a given core, and on top of that, all known Imagination GPU cores use
> the same 40-bit virtual address space.
> 
> Move the check above to a new function for GPU device info validation
> code, so it's performed only once.
> This also allows other areas of the driver to reference the relevant
> defines without having to perform other checks at runtime, as long as
> all cores keep using the same virtual address space.
> 
> Signed-off-by: Alessio Belle <alessio.belle@imgtec.com>
> Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
> ---
>  drivers/gpu/drm/imagination/pvr_device.c | 43 +++++++++++++++++++++++++++++---
>  drivers/gpu/drm/imagination/pvr_mmu.h    |  2 +-
>  drivers/gpu/drm/imagination/pvr_vm.c     | 22 +---------------
>  3 files changed, 42 insertions(+), 25 deletions(-)
> 
Reviewed-by: Brajesh Gupta <brajesh.gupta@imgtec.com>

Thanks,
Brajesh
> diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
> index 35eaa54f75ed..992509d95d28 100644
> --- a/drivers/gpu/drm/imagination/pvr_device.c
> +++ b/drivers/gpu/drm/imagination/pvr_device.c
> @@ -5,6 +5,7 @@
>  #include "pvr_device_info.h"
>  
>  #include "pvr_fw.h"
> +#include "pvr_mmu.h"
>  #include "pvr_power.h"
>  #include "pvr_queue.h"
>  #include "pvr_rogue_cr_defs.h"
> @@ -646,6 +647,36 @@ pvr_set_dma_info(struct pvr_device *pvr_dev)
>  	return 0;
>  }
>  
> +/**
> + * pvr_device_info_validate() - Validate PowerVR device information, read from
> + * the firmware binary, against what is supported by the driver.
> + * @pvr_dev: Target PowerVR device.
> + *
> + * Returns:
> + *  * 0 on success, or
> + *  * -%EINVAL if the validation failed.
> + */
> +static int
> +pvr_device_info_validate(struct pvr_device *pvr_dev)
> +{
> +	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
> +	u16 device_addr_bits;
> +	int err;
> +
> +	err = PVR_FEATURE_VALUE(pvr_dev, virtual_address_space_bits, &device_addr_bits);
> +	if (err) {
> +		drm_err(drm_dev, "Failed to get device virtual address space bits\n");
> +		return err;
> +	}
> +
> +	if (device_addr_bits != PVR_PAGE_TABLE_ADDR_BITS) {
> +		drm_err(drm_dev, "Device has unsupported virtual address space size\n");
> +		return -EINVAL;
> +	}
> +
> +	return err;
> +}
> +
>  /**
>   * pvr_device_gpu_init() - GPU-specific initialization for a PowerVR device
>   * @pvr_dev: Target PowerVR device.
> @@ -661,9 +692,11 @@ pvr_set_dma_info(struct pvr_device *pvr_dev)
>   * Return:
>   *  * 0 on success,
>   *  * -%ENODEV if the GPU is not supported,
> - *  * Any error returned by pvr_set_dma_info(),
> - *  * Any error returned by pvr_memory_context_init(), or
> - *  * Any error returned by pvr_request_firmware().
> + *  * Any error returned by pvr_request_firmware(),
> + *  * Any error returned by pvr_fw_validate_init_device_info(),
> + *  * Any error returned by pvr_device_info_validate(),
> + *  * Any error returned by pvr_set_dma_info(), or
> + *  * Any error returned by pvr_fw_init().
>   */
>  static int
>  pvr_device_gpu_init(struct pvr_device *pvr_dev)
> @@ -691,6 +724,10 @@ pvr_device_gpu_init(struct pvr_device *pvr_dev)
>  	else
>  		return -EINVAL;
>  
> +	err = pvr_device_info_validate(pvr_dev);
> +	if (err)
> +		return err;
> +
>  	pvr_stream_create_musthave_masks(pvr_dev);
>  
>  	err = pvr_set_dma_info(pvr_dev);
> diff --git a/drivers/gpu/drm/imagination/pvr_mmu.h b/drivers/gpu/drm/imagination/pvr_mmu.h
> index a8ecd460168d..1f9ae00a8b17 100644
> --- a/drivers/gpu/drm/imagination/pvr_mmu.h
> +++ b/drivers/gpu/drm/imagination/pvr_mmu.h
> @@ -62,7 +62,7 @@ struct sg_table;
>   *
>   *    This value is checked at runtime against
>   *    &pvr_device_features.virtual_address_space_bits by
> - *    pvr_vm_create_context(), which will return an error if the feature value
> + *    pvr_device_info_validate(), which will return an error if the feature value
>   *    does not match this constant.
>   *
>   *    .. admonition:: Future work
> diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
> index ceb78694cd98..304d0743c5aa 100644
> --- a/drivers/gpu/drm/imagination/pvr_vm.c
> +++ b/drivers/gpu/drm/imagination/pvr_vm.c
> @@ -535,8 +535,6 @@ fw_mem_context_init(void *cpu_ptr, void *priv)
>   *
>   * Return:
>   *  * A handle to the newly-minted VM context on success,
> - *  * -%EINVAL if the feature "virtual address space bits" on @pvr_dev is
> - *    missing or has an unsupported value,
>   *  * -%ENOMEM if allocation of the structure behind the opaque handle fails,
>   *    or
>   *  * Any error encountered while setting up internal structures.
> @@ -544,27 +542,9 @@ fw_mem_context_init(void *cpu_ptr, void *priv)
>  struct pvr_vm_context *
>  pvr_vm_create_context(struct pvr_device *pvr_dev, bool is_userspace_context)
>  {
> -	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
> -
>  	struct pvr_vm_context *vm_ctx;
> -	u16 device_addr_bits;
> -
>  	int err;
>  
> -	err = PVR_FEATURE_VALUE(pvr_dev, virtual_address_space_bits,
> -				&device_addr_bits);
> -	if (err) {
> -		drm_err(drm_dev,
> -			"Failed to get device virtual address space bits\n");
> -		return ERR_PTR(err);
> -	}
> -
> -	if (device_addr_bits != PVR_PAGE_TABLE_ADDR_BITS) {
> -		drm_err(drm_dev,
> -			"Device has unsupported virtual address space size\n");
> -		return ERR_PTR(-EINVAL);
> -	}
> -
>  	vm_ctx = kzalloc_obj(*vm_ctx);
>  	if (!vm_ctx)
>  		return ERR_PTR(-ENOMEM);
> @@ -589,7 +569,7 @@ pvr_vm_create_context(struct pvr_device *pvr_dev, bool is_userspace_context)
>  	drm_gpuvm_init(&vm_ctx->gpuvm_mgr,
>  		       is_userspace_context ? "PowerVR-user-VM" : "PowerVR-FW-VM",
>  		       0, &pvr_dev->base, &vm_ctx->dummy_gem,
> -		       0, 1ULL << device_addr_bits, 0, 0, &pvr_vm_gpuva_ops);
> +		       0, PVR_PAGE_TABLE_ADDR_SPACE_SIZE, 0, 0, &pvr_vm_gpuva_ops);
>  
>  	mutex_init(&vm_ctx->lock);
>  	kref_init(&vm_ctx->ref_count);
> 
> ---
> base-commit: 6e375de99d0c420169481fcd36064177ab55b09d
> change-id: 20261006-b4-check-virtual-addr-space-size-once-044265ce3fa3
> 
> Best regards,
> --  
> Alexandru Dadu <alexandru.dadu@imgtec.com>
>