Re: [PATCH] drm/amdgpu: add VISIBLE info in amdgpu_bo_print_info

["Pelloux-Prayer, Pierre-Eric" <Pierre-eric.Pelloux-prayer@xxxxxxx>]

[Public]

Thanks Christian for the review. I'll remove the leading blanks before submitting the patch.

Pierre-Eric

---

From: Koenig, Christian <Christian.Koenig@xxxxxxx>
Sent: Wednesday, June 21, 2023 5:00 PM
To: Pelloux-Prayer, Pierre-Eric <Pierre-eric.Pelloux-prayer@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: add VISIBLE info in amdgpu_bo_print_info

 

Am 21.06.23 um 16:35 schrieb Pierre-Eric Pelloux-Prayer:
> This allows tools to distinguish between VRAM and visible VRAM.
>
> Use the opportunity to fix locking before accessing bo.
>
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 33 ++++++++++++++--------
>   1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index ff73cc11d47e..f12f019d7f99 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1583,18 +1583,27 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m)
>        unsigned int pin_count;
>        u64 size;
>  
> -     domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
> -     switch (domain) {
> -     case AMDGPU_GEM_DOMAIN_VRAM:
> -             placement = "VRAM";
> -             break;
> -     case AMDGPU_GEM_DOMAIN_GTT:
> -             placement = " GTT";
> -             break;
> -     case AMDGPU_GEM_DOMAIN_CPU:
> -     default:
> -             placement = " CPU";
> -             break;
> +     if (dma_resv_trylock(bo->tbo.base.resv)) {
> +             unsigned int domain;
> +             domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
> +             switch (domain) {
> +             case AMDGPU_GEM_DOMAIN_VRAM:
> +                     if (amdgpu_bo_in_cpu_visible_vram(bo))
> +                             placement = "VRAM VISIBLE";
> +                     else
> +                             placement = "VRAM";
> +                     break;
> +             case AMDGPU_GEM_DOMAIN_GTT:
> +                     placement = " GTT";

We can probably drop the leading blank here and

> +                     break;
> +             case AMDGPU_GEM_DOMAIN_CPU:
> +             default:
> +                     placement = " CPU";

here when we don't keep the strings at the same length anyway.

With that fixed the change is Reviewed-by: Christian König
<christian.koenig@xxxxxxx>

Regards,
Christian.

> +                     break;
> +             }
> +             dma_resv_unlock(bo->tbo.base.resv);
> +     } else {
> +             placement = "UNKNOWN";
>        }
>  
>        size = amdgpu_bo_size(bo);