Re: [PATCH 3/4] drm/imagination: Improve some logging in error paths

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

On Wed, 2026-10-14 at 11:13 +0300, Alexandru Dadu wrote:
> From: Matt Coster <matt.coster@imgtec.com>
> 
> Add error messages for attempts to map an already mapped firmware
> object, unmap an already unmapped firmware object, and provide too few
> DMA address slots for the firmware page table.
> 
> Signed-off-by: Matt Coster <matt.coster@imgtec.com>
> Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>

Reviewed-by: Alessio Belle <alessio.belle@imgtec.com>

Thanks,
Alessio

> ---
>  drivers/gpu/drm/imagination/pvr_fw.c      | 6 ++++--
>  drivers/gpu/drm/imagination/pvr_vm_mips.c | 7 ++++++-
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_fw.c b/drivers/gpu/drm/imagination/pvr_fw.c
> index c6c77b152342..4f2b5c4df600 100644
> --- a/drivers/gpu/drm/imagination/pvr_fw.c
> +++ b/drivers/gpu/drm/imagination/pvr_fw.c
> @@ -1165,8 +1165,9 @@ pvr_fw_object_fw_map(struct pvr_device *pvr_dev, struct pvr_fw_object *fw_obj, u
>  	spin_lock(&fw_dev->fw_mm_lock);
>  
>  	if (drm_mm_node_allocated(&fw_obj->fw_mm_node)) {
> -		err = -EINVAL;
> -		goto err_unlock;
> +		spin_unlock(&fw_dev->fw_mm_lock);
> +		drm_err(from_pvr_device(pvr_dev), "FW object is already mapped!\n");
> +		return -EINVAL;
>  	}
>  
>  	if (!dev_addr) {
> @@ -1232,6 +1233,7 @@ pvr_fw_object_fw_unmap(struct pvr_fw_object *fw_obj)
>  
>  	if (!drm_mm_node_allocated(&fw_obj->fw_mm_node)) {
>  		spin_unlock(&fw_dev->fw_mm_lock);
> +		drm_err(from_pvr_device(pvr_dev), "FW object is already unmapped!\n");
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/imagination/pvr_vm_mips.c b/drivers/gpu/drm/imagination/pvr_vm_mips.c
> index 84e023fffe5a..40495b1486a2 100644
> --- a/drivers/gpu/drm/imagination/pvr_vm_mips.c
> +++ b/drivers/gpu/drm/imagination/pvr_vm_mips.c
> @@ -11,6 +11,7 @@
>  #include "pvr_vm_mips.h"
>  
>  #include <drm/drm_managed.h>
> +#include <drm/drm_print.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/err.h>
>  #include <linux/slab.h>
> @@ -278,8 +279,12 @@ pvr_vm_get_pt_phys_addrs(struct pvr_device *pvr_dev, u64 *phys_addrs, int max_nu
>  	struct pvr_fw_device *fw_dev = &pvr_dev->fw_dev;
>  	struct pvr_vm_mips_data *vm_data = fw_dev->processor_data.mips_data.vm_data;
>  
> -	if (ARRAY_SIZE(vm_data->pt_pages) > max_num_addrs)
> +	if (ARRAY_SIZE(vm_data->pt_pages) > max_num_addrs) {
> +		drm_err(from_pvr_device(pvr_dev),
> +			"Minimum %zu pages required for FW PT, but only %d DMA address slots were provided\n",
> +			ARRAY_SIZE(vm_data->pt_pages), max_num_addrs);
>  		return -EINVAL;
> +	}
>  
>  	for (u32 page_nr = 0; page_nr < max_num_addrs; page_nr++) {
>  		/* Firmware expects 4k pages, but host page size might be different. */
>