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

2023-06-26 Thread Pelloux-Prayer, Pierre-Eric 
[Public]

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

Pierre-Eric

From: Koenig, Christian 
Sent: Wednesday, June 21, 2023 5:00 PM
To: Pelloux-Prayer, Pierre-Eric ; 
amd-gfx@lists.freedesktop.org 
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 
> ---
>   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


Regards,
Christian.

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

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

2023-06-21 Thread Christian König 

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



Regards,
Christian.


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

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

2023-06-21 Thread 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 
---
 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";
+   break;
+   case AMDGPU_GEM_DOMAIN_CPU:
+   default:
+   placement = " CPU";
+   break;
+   }
+   dma_resv_unlock(bo->tbo.base.resv);
+   } else {
+   placement = "UNKNOWN";
}
 
size = amdgpu_bo_size(bo);
-- 
2.40.0