Re: [PATCH 09/10] accel/ivpu: Improve buffer object debug logs

[Jeffrey Hugo <quic_jhugo@xxxxxxxxxxx>]

On 1/5/2024 4:22 AM, Jacek Lawrynowicz wrote:
> Make debug logs more readable and consistent:
>    - don't print handle as it is not always available for all buffers
>    - use hashed ivpu_bo ptr as main buffer identifier
>    - remove unused fields from ivpu_bo_print_info()
>
> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@xxxxxxxxxxxxxxx>
> ---
>   drivers/accel/ivpu/ivpu_gem.c | 72 +++++++++++------------------------
>   drivers/accel/ivpu/ivpu_gem.h |  1 -
>   2 files changed, 23 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_gem.c b/drivers/accel/ivpu/ivpu_gem.c
> index 8cb4d337552e..dd327d7eda0d 100644
> --- a/drivers/accel/ivpu/ivpu_gem.c
> +++ b/drivers/accel/ivpu/ivpu_gem.c
> @@ -24,14 +24,11 @@ static const struct drm_gem_object_funcs ivpu_gem_funcs;
>   
>   static inline void ivpu_dbg_bo(struct ivpu_device *vdev, struct ivpu_bo *bo, const char *action)
>   {
> -	if (bo->ctx)
> -		ivpu_dbg(vdev, BO, "%6s: size %zu has_pages %d dma_mapped %d handle %u ctx %d vpu_addr 0x%llx mmu_mapped %d\n",
> -			 action, ivpu_bo_size(bo), (bool)bo->base.pages, (bool)bo->base.sgt,
> -			 bo->handle, bo->ctx->id, bo->vpu_addr, bo->mmu_mapped);
> -	else
> -		ivpu_dbg(vdev, BO, "%6s: size %zu has_pages %d dma_mapped %d handle %u (not added to context)\n",
> -			 action, ivpu_bo_size(bo), (bool)bo->base.pages, (bool)bo->base.sgt,
> -			 bo->handle);
> +	ivpu_dbg(vdev, BO,
> +		 "%6s: bo %8p vpu_addr %9llx size %8zu ctx %d has_pages %d dma_mapped %d mmu_mapped %d wc %d imported %d\n",
> +		 action, bo, bo->vpu_addr, ivpu_bo_size(bo), bo->ctx ? bo->ctx->id : 0,
> +		 (bool)bo->base.pages, (bool)bo->base.sgt, bo->mmu_mapped, bo->base.map_wc,
> +		 (bool)bo->base.base.import_attach);
>   }
>   
>   /*
> @@ -49,12 +46,7 @@ int __must_check ivpu_bo_pin(struct ivpu_bo *bo)
>   	mutex_lock(&bo->lock);
>   
>   	ivpu_dbg_bo(vdev, bo, "pin");
> -
> -	if (!bo->ctx) {
> -		ivpu_err(vdev, "vpu_addr not allocated for BO %d\n", bo->handle);
> -		ret = -EINVAL;
> -		goto unlock;
> -	}
> +	drm_WARN_ON(&vdev->drm, !bo->ctx);
>   
>   	if (!bo->mmu_mapped) {
>   		struct sg_table *sgt = drm_gem_shmem_get_pages_sgt(&bo->base);
> @@ -108,9 +100,7 @@ static void ivpu_bo_unbind_locked(struct ivpu_bo *bo)
>   {
>   	struct ivpu_device *vdev = ivpu_bo_to_vdev(bo);
>   
> -	lockdep_assert_held(&bo->lock);
> -
> -	ivpu_dbg_bo(vdev, bo, "unbind");
> +	lockdep_assert(lockdep_is_held(&bo->lock) || !kref_read(&bo->base.base.refcount));
>   
>   	if (bo->mmu_mapped) {
>   		drm_WARN_ON(&vdev->drm, !bo->ctx);
> @@ -122,7 +112,6 @@ static void ivpu_bo_unbind_locked(struct ivpu_bo *bo)
>   
>   	if (bo->ctx) {
>   		ivpu_mmu_context_remove_node(bo->ctx, &bo->mm_node);
> -		bo->vpu_addr = 0;
>   		bo->ctx = NULL;
>   	}
>   
> @@ -139,13 +128,6 @@ static void ivpu_bo_unbind_locked(struct ivpu_bo *bo)
>   	dma_resv_unlock(bo->base.base.resv);
>   }
>   
> -static void ivpu_bo_unbind(struct ivpu_bo *bo)
> -{
> -	mutex_lock(&bo->lock);
> -	ivpu_bo_unbind_locked(bo);
> -	mutex_unlock(&bo->lock);
> -}

This does not seem to be related to $SUBJECT

> -
>   void ivpu_bo_remove_all_bos_from_context(struct ivpu_device *vdev, struct ivpu_mmu_context *ctx)
>   {
>   	struct ivpu_bo *bo;
> @@ -156,8 +138,10 @@ void ivpu_bo_remove_all_bos_from_context(struct ivpu_device *vdev, struct ivpu_m
>   	mutex_lock(&vdev->bo_list_lock);
>   	list_for_each_entry(bo, &vdev->bo_list, bo_list_node) {
>   		mutex_lock(&bo->lock);
> -		if (bo->ctx == ctx)
> +		if (bo->ctx == ctx) {
> +			ivpu_dbg_bo(vdev, bo, "unbind");
>   			ivpu_bo_unbind_locked(bo);
> +		}
>   		mutex_unlock(&bo->lock);
>   	}
>   	mutex_unlock(&vdev->bo_list_lock);
> @@ -209,9 +193,6 @@ ivpu_bo_create(struct ivpu_device *vdev, u64 size, u32 flags)
>   	list_add_tail(&bo->bo_list_node, &vdev->bo_list);
>   	mutex_unlock(&vdev->bo_list_lock);
>   
> -	ivpu_dbg(vdev, BO, "create: vpu_addr 0x%llx size %zu flags 0x%x\n",
> -		 bo->vpu_addr, bo->base.base.size, flags);
> -
>   	return bo;
>   }
>   
> @@ -243,15 +224,15 @@ static void ivpu_bo_free(struct drm_gem_object *obj)
>   	struct ivpu_device *vdev = to_ivpu_device(obj->dev);
>   	struct ivpu_bo *bo = to_ivpu_bo(obj);
>   
> +	ivpu_dbg_bo(vdev, bo, "free");
> +
>   	mutex_lock(&vdev->bo_list_lock);
>   	list_del(&bo->bo_list_node);
>   	mutex_unlock(&vdev->bo_list_lock);
>   
>   	drm_WARN_ON(&vdev->drm, !dma_resv_test_signaled(obj->resv, DMA_RESV_USAGE_READ));
>   
> -	ivpu_dbg_bo(vdev, bo, "free");
> -
> -	ivpu_bo_unbind(bo);
> +	ivpu_bo_unbind_locked(bo);

This does not seem to be related to $SUBJECT