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

[Brajesh Gupta <Brajesh.Gupta@imgtec.com>]

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>
>