Re: [PATCH] drm/imagination: Manage FW VM context from its init and fini callbacks

[Brajesh Gupta]

On Mon, 2026-08-10 at 15:31 +0300, Alexandru Dadu wrote:
Hi Alexandru,
> From: Alessio Belle <alessio.belle@xxxxxxxxxx>
> 
> While the MIPS firmware virtual memory context is created and destroyed
> from within the MIPS firmware init and fini callbacks, the META and
> RISC-V firmware contexts are created and destroyed from within blocks
> right before or after those callbacks.
> 
> Match the logic for all firmware processors by moving those blocks to
> the META and RISC-V init and fini callbacks.
> 
> Signed-off-by: Alessio Belle <alessio.belle@xxxxxxxxxx>
> ---
> Signed-off-by: Alexandru Dadu <alexandru.dadu@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/imagination/pvr_device.c   | 25 +------------------------
>  drivers/gpu/drm/imagination/pvr_fw.c       |  6 ++----
>  drivers/gpu/drm/imagination/pvr_fw.h       |  2 +-
>  drivers/gpu/drm/imagination/pvr_fw_meta.c  | 12 ++++++++++++
>  drivers/gpu/drm/imagination/pvr_fw_riscv.c | 12 ++++++++++++
>  5 files changed, 28 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
> index 54fe4180c73c..35eaa54f75ed 100644
> --- a/drivers/gpu/drm/imagination/pvr_device.c
> +++ b/drivers/gpu/drm/imagination/pvr_device.c
> @@ -697,25 +697,7 @@ pvr_device_gpu_init(struct pvr_device *pvr_dev)
>  	if (err)
>  		return err;
>  
> -	if (pvr_dev->fw_dev.processor_type != PVR_FW_PROCESSOR_TYPE_MIPS) {
> -		pvr_dev->kernel_vm_ctx = pvr_vm_create_context(pvr_dev, false);
> -		if (IS_ERR(pvr_dev->kernel_vm_ctx))
> -			return PTR_ERR(pvr_dev->kernel_vm_ctx);
> -	}
> -
> -	err = pvr_fw_init(pvr_dev);
> -	if (err)
> -		goto err_vm_ctx_put;
> -
> -	return 0;
> -
> -err_vm_ctx_put:
> -	if (pvr_dev->fw_dev.processor_type != PVR_FW_PROCESSOR_TYPE_MIPS) {
> -		pvr_vm_context_put(pvr_dev->kernel_vm_ctx);
> -		pvr_dev->kernel_vm_ctx = NULL;
> -	}
> -
> -	return err;
> +	return pvr_fw_init(pvr_dev);
>  }
>  
>  /**
> @@ -726,11 +708,6 @@ static void
>  pvr_device_gpu_fini(struct pvr_device *pvr_dev)
>  {
>  	pvr_fw_fini(pvr_dev);
> -
> -	if (pvr_dev->fw_dev.processor_type != PVR_FW_PROCESSOR_TYPE_MIPS) {
> -		WARN_ON(!pvr_vm_context_put(pvr_dev->kernel_vm_ctx));
> -		pvr_dev->kernel_vm_ctx = NULL;
> -	}
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/imagination/pvr_fw.c b/drivers/gpu/drm/imagination/pvr_fw.c
> index 850a3ec8e775..58ee51cd65df 100644
> --- a/drivers/gpu/drm/imagination/pvr_fw.c
> +++ b/drivers/gpu/drm/imagination/pvr_fw.c
> @@ -1029,8 +1029,7 @@ pvr_fw_init(struct pvr_device *pvr_dev)
>  err_mm_takedown:
>  	drm_mm_takedown(&fw_dev->fw_mm);
>  
> -	if (fw_dev->defs->fini)
> -		fw_dev->defs->fini(pvr_dev);
> +	fw_dev->defs->fini(pvr_dev);
>  
>  	return err;
>  }
> @@ -1063,8 +1062,7 @@ pvr_fw_fini(struct pvr_device *pvr_dev)
>  
>  	drm_mm_takedown(&fw_dev->fw_mm);
>  
> -	if (fw_dev->defs->fini)
> -		fw_dev->defs->fini(pvr_dev);
> +	fw_dev->defs->fini(pvr_dev);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/imagination/pvr_fw.h b/drivers/gpu/drm/imagination/pvr_fw.h
> index 3390c84e4fd3..4b25291135b6 100644
> --- a/drivers/gpu/drm/imagination/pvr_fw.h
> +++ b/drivers/gpu/drm/imagination/pvr_fw.h
> @@ -86,7 +86,7 @@ struct pvr_fw_defs {
>  	 * FW processor specific finalisation.
>  	 * @pvr_dev: Target PowerVR device.
>  	 *
> -	 * This function is optional.
> +	 * This function is mandatory.
>  	 */
>  	void (*fini)(struct pvr_device *pvr_dev);
>  
> diff --git a/drivers/gpu/drm/imagination/pvr_fw_meta.c b/drivers/gpu/drm/imagination/pvr_fw_meta.c
> index 9ff03bc60a08..6c5dc711e81e 100644
> --- a/drivers/gpu/drm/imagination/pvr_fw_meta.c
> +++ b/drivers/gpu/drm/imagination/pvr_fw_meta.c
> @@ -500,9 +500,20 @@ pvr_meta_init(struct pvr_device *pvr_dev)
>  {
>  	pvr_fw_heap_info_init(pvr_dev, ROGUE_FW_HEAP_META_SHIFT, 0);
>  
> +	pvr_dev->kernel_vm_ctx = pvr_vm_create_context(pvr_dev, false);
> +	if (IS_ERR(pvr_dev->kernel_vm_ctx))
> +		return PTR_ERR(pvr_dev->kernel_vm_ctx);
> +
>  	return 0;
>  }
>  
> +static void
> +pvr_meta_fini(struct pvr_device *pvr_dev)
> +{
> +	WARN_ON(!pvr_vm_context_put(pvr_dev->kernel_vm_ctx));
> +	pvr_dev->kernel_vm_ctx = NULL;
> +}
> +
>  static u32
>  pvr_meta_get_fw_addr_with_offset(struct pvr_fw_object *fw_obj, u32 offset)
>  {
> @@ -550,6 +561,7 @@ pvr_meta_irq_clear(struct pvr_device *pvr_dev)
>  
>  const struct pvr_fw_defs pvr_fw_defs_meta = {
>  	.init = pvr_meta_init,
> +	.fini = pvr_meta_fini,
>  	.fw_process = pvr_meta_fw_process,
>  	.vm_map = pvr_meta_vm_map,
>  	.vm_unmap = pvr_meta_vm_unmap,
> diff --git a/drivers/gpu/drm/imagination/pvr_fw_riscv.c b/drivers/gpu/drm/imagination/pvr_fw_riscv.c
> index fc13d483be9a..58bacc522e72 100644
> --- a/drivers/gpu/drm/imagination/pvr_fw_riscv.c
> +++ b/drivers/gpu/drm/imagination/pvr_fw_riscv.c
> @@ -103,9 +103,20 @@ pvr_riscv_init(struct pvr_device *pvr_dev)
>  {
>  	pvr_fw_heap_info_init(pvr_dev, ROGUE_FW_HEAP_RISCV_SHIFT, 0);
>  
> +	pvr_dev->kernel_vm_ctx = pvr_vm_create_context(pvr_dev, false);
> +	if (IS_ERR(pvr_dev->kernel_vm_ctx))
> +		return PTR_ERR(pvr_dev->kernel_vm_ctx);
> +
>  	return 0;
>  }
>  
> +static void
> +pvr_riscv_fini(struct pvr_device *pvr_dev)
> +{
> +	WARN_ON(!pvr_vm_context_put(pvr_dev->kernel_vm_ctx));
> +	pvr_dev->kernel_vm_ctx = NULL;
> +}
> +
>  static u32
>  pvr_riscv_get_fw_addr_with_offset(struct pvr_fw_object *fw_obj, u32 offset)
>  {
> @@ -154,6 +165,7 @@ pvr_riscv_irq_clear(struct pvr_device *pvr_dev)
>  
>  const struct pvr_fw_defs pvr_fw_defs_riscv = {
>  	.init = pvr_riscv_init,
> +	.fini = pvr_riscv_fini,
>  	.fw_process = pvr_riscv_fw_process,
>  	.vm_map = pvr_riscv_vm_map,
>  	.vm_unmap = pvr_riscv_vm_unmap,
> 
> ---
> base-commit: e55fead22ff9ee047ab9f1903860c4b43043514e
> change-id: 20260810-b4-upstream-manage-fw-vm-context-from-init-and-fini-195cdd01cc60
> 
> Best regards,
> --  
> Alexandru Dadu <alexandru.dadu@xxxxxxxxxx>
> 
Reviewed by: Brajesh Gupta <brajesh.gupta@xxxxxxxxxx>

Thanks,
Brajesh