Re: [PATCH 14/14] drm/amdgpu: introduce doorbell bo in kernel

[Shashank Sharma]

On 06/02/2023 17:57, Christian König wrote:

Am 03.02.23 um 20:08 schrieb Shashank Sharma:

From: Alex Deucher 

This patch does the following:
- Removes doorbell ptr from adev.
- Moves doorbell.num_doorbells into mman.num_doorbells.
- Introduces a kernel bo for doorbell management in form of 
mman.doorbell_kernel_bo.

   This bo holds the doorbell space now.
- Introduces mman.doorbell_bo_size: to save total doorbell memory size.
- Also introduces mman.doorbell_va to save CPU address of doorbell BO
   mapping. This ptr will be used now for doorbell read/write from 
doorbell BAR.


Please don't call any CPU address VA, this is a term we usually use 
with GPUVM.


Rather use doorbell_cpu_addr for this.

Christian.


Noted,

- Shashank





Signed-off-by: Alex Deucher 
Signed-off-by: Shashank Sharma 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h  |  1 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   |  5 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   | 16 ++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h |  4 
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 24 
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h  |  6 +
  6 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

index e1c1a360614e..dd3875ae1ad3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -834,7 +834,6 @@ struct amdgpu_device {
  spinlock_t audio_endpt_idx_lock;
  amdgpu_block_rreg_t    audio_endpt_rreg;
  amdgpu_block_wreg_t    audio_endpt_wreg;
-    struct amdgpu_doorbell    doorbell;
    /* clock/pll info */
  struct amdgpu_clock    clock;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c

index 28076da2258f..aea943e337df 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -109,11 +109,10 @@ static void amdgpu_doorbell_get_kfd_info(struct 
amdgpu_device *adev,

  *aperture_base = adev->gmc.doorbell_aper_base;
  *aperture_size = 0;
  *start_offset = 0;
-    } else if (adev->gmc.doorbell_aper_size > 
adev->doorbell.num_doorbells *

-    sizeof(u32)) {
+    } else if (adev->gmc.doorbell_aper_size > 
adev->mman.doorbell_kernel_bo_size) {

  *aperture_base = adev->gmc.doorbell_aper_base;
  *aperture_size = adev->gmc.doorbell_aper_size;
-    *start_offset = adev->doorbell.num_doorbells * sizeof(u32);
+    *start_offset = adev->mman.doorbell_kernel_bo_size;
  } else {
  *aperture_base = 0;
  *aperture_size = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

index 7c21ffe63ebc..9c3ce1ae66ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -593,8 +593,8 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device 
*adev, u32 index)

  if (amdgpu_device_skip_hw_access(adev))
  return 0;
  -    if (index < adev->doorbell.num_doorbells) {
-    return readl(adev->mman.doorbell_aper_base_kaddr + index);
+    if (index < adev->mman.num_doorbells) {
+    return readl(adev->mman.doorbell_va + index);
  } else {
  DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", 
index);

  return 0;
@@ -616,8 +616,8 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device 
*adev, u32 index, u32 v)

  if (amdgpu_device_skip_hw_access(adev))
  return;
  -    if (index < adev->doorbell.num_doorbells) {
-    writel(v, adev->mman.doorbell_aper_base_kaddr + index);
+    if (index < adev->mman.num_doorbells) {
+    writel(v, adev->mman.doorbell_va + index);
  } else {
  DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", 
index);

  }
@@ -637,8 +637,8 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device 
*adev, u32 index)

  if (amdgpu_device_skip_hw_access(adev))
  return 0;
  -    if (index < adev->doorbell.num_doorbells) {
-    return atomic64_read((atomic64_t 
*)(adev->mman.doorbell_aper_base_kaddr + index));

+    if (index < adev->mman.num_doorbells) {
+    return atomic64_read((atomic64_t *)(adev->mman.doorbell_va + 
index));

  } else {
  DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", 
index);

  return 0;
@@ -660,8 +660,8 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device 
*adev, u32 index, u64 v)

  if (amdgpu_device_skip_hw_access(adev))
  return;
  -    if (index < adev->doorbell.num_doorbells) {
-    atomic64_set((atomic64_t 
*)(adev->mman.doorbell_aper_base_kaddr + index), v);

+    if (index < adev->mman.num_doorbells) {
+    atomic64_set((atomic64_t *)(adev->mman.doorbell_va + index), 
v);

  } else {
  DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", 
index);

  }
diff --git 

Re: [PATCH 14/14] drm/amdgpu: introduce doorbell bo in kernel

[Christian König]

Am 03.02.23 um 20:08 schrieb Shashank Sharma:

From: Alex Deucher 

This patch does the following:
- Removes doorbell ptr from adev.
- Moves doorbell.num_doorbells into mman.num_doorbells.
- Introduces a kernel bo for doorbell management in form of 
mman.doorbell_kernel_bo.
   This bo holds the doorbell space now.
- Introduces mman.doorbell_bo_size: to save total doorbell memory size.
- Also introduces mman.doorbell_va to save CPU address of doorbell BO
   mapping. This ptr will be used now for doorbell read/write from doorbell BAR.


Please don't call any CPU address VA, this is a term we usually use with 
GPUVM.


Rather use doorbell_cpu_addr for this.

Christian.



Signed-off-by: Alex Deucher 
Signed-off-by: Shashank Sharma 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h  |  1 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   |  5 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   | 16 ++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h |  4 
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 24 
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h  |  6 +
  6 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e1c1a360614e..dd3875ae1ad3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -834,7 +834,6 @@ struct amdgpu_device {
spinlock_t audio_endpt_idx_lock;
amdgpu_block_rreg_t audio_endpt_rreg;
amdgpu_block_wreg_t audio_endpt_wreg;
-   struct amdgpu_doorbell  doorbell;
  
  	/* clock/pll info */

struct amdgpu_clockclock;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 28076da2258f..aea943e337df 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -109,11 +109,10 @@ static void amdgpu_doorbell_get_kfd_info(struct 
amdgpu_device *adev,
*aperture_base = adev->gmc.doorbell_aper_base;
*aperture_size = 0;
*start_offset = 0;
-   } else if (adev->gmc.doorbell_aper_size > adev->doorbell.num_doorbells *
-   sizeof(u32)) {
+   } else if (adev->gmc.doorbell_aper_size > 
adev->mman.doorbell_kernel_bo_size) {
*aperture_base = adev->gmc.doorbell_aper_base;
*aperture_size = adev->gmc.doorbell_aper_size;
-   *start_offset = adev->doorbell.num_doorbells * sizeof(u32);
+   *start_offset = adev->mman.doorbell_kernel_bo_size;
} else {
*aperture_base = 0;
*aperture_size = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7c21ffe63ebc..9c3ce1ae66ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -593,8 +593,8 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device *adev, u32 
index)
if (amdgpu_device_skip_hw_access(adev))
return 0;
  
-	if (index < adev->doorbell.num_doorbells) {

-   return readl(adev->mman.doorbell_aper_base_kaddr + index);
+   if (index < adev->mman.num_doorbells) {
+   return readl(adev->mman.doorbell_va + index);
} else {
DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
return 0;
@@ -616,8 +616,8 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device *adev, u32 
index, u32 v)
if (amdgpu_device_skip_hw_access(adev))
return;
  
-	if (index < adev->doorbell.num_doorbells) {

-   writel(v, adev->mman.doorbell_aper_base_kaddr + index);
+   if (index < adev->mman.num_doorbells) {
+   writel(v, adev->mman.doorbell_va + index);
} else {
DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
}
@@ -637,8 +637,8 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device *adev, u32 
index)
if (amdgpu_device_skip_hw_access(adev))
return 0;
  
-	if (index < adev->doorbell.num_doorbells) {

-   return atomic64_read((atomic64_t 
*)(adev->mman.doorbell_aper_base_kaddr + index));
+   if (index < adev->mman.num_doorbells) {
+   return atomic64_read((atomic64_t *)(adev->mman.doorbell_va + 
index));
} else {
DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
return 0;
@@ -660,8 +660,8 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device *adev, u32 
index, u64 v)
if (amdgpu_device_skip_hw_access(adev))
return;
  
-	if (index < adev->doorbell.num_doorbells) {

-   atomic64_set((atomic64_t *)(adev->mman.doorbell_aper_base_kaddr 
+ index), v);
+   if (index < adev->mman.num_doorbells) {
+   atomic64_set((atomic64_t 

[PATCH 14/14] drm/amdgpu: introduce doorbell bo in kernel

[Shashank Sharma]

From: Alex Deucher 

This patch does the following:
- Removes doorbell ptr from adev.
- Moves doorbell.num_doorbells into mman.num_doorbells.
- Introduces a kernel bo for doorbell management in form of 
mman.doorbell_kernel_bo.
  This bo holds the doorbell space now.
- Introduces mman.doorbell_bo_size: to save total doorbell memory size.
- Also introduces mman.doorbell_va to save CPU address of doorbell BO
  mapping. This ptr will be used now for doorbell read/write from doorbell BAR.

Signed-off-by: Alex Deucher 
Signed-off-by: Shashank Sharma 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h  |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   |  5 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   | 16 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h |  4 
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 24 
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h  |  6 +
 6 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e1c1a360614e..dd3875ae1ad3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -834,7 +834,6 @@ struct amdgpu_device {
spinlock_t audio_endpt_idx_lock;
amdgpu_block_rreg_t audio_endpt_rreg;
amdgpu_block_wreg_t audio_endpt_wreg;
-   struct amdgpu_doorbell  doorbell;
 
/* clock/pll info */
struct amdgpu_clockclock;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 28076da2258f..aea943e337df 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -109,11 +109,10 @@ static void amdgpu_doorbell_get_kfd_info(struct 
amdgpu_device *adev,
*aperture_base = adev->gmc.doorbell_aper_base;
*aperture_size = 0;
*start_offset = 0;
-   } else if (adev->gmc.doorbell_aper_size > adev->doorbell.num_doorbells *
-   sizeof(u32)) {
+   } else if (adev->gmc.doorbell_aper_size > 
adev->mman.doorbell_kernel_bo_size) {
*aperture_base = adev->gmc.doorbell_aper_base;
*aperture_size = adev->gmc.doorbell_aper_size;
-   *start_offset = adev->doorbell.num_doorbells * sizeof(u32);
+   *start_offset = adev->mman.doorbell_kernel_bo_size;
} else {
*aperture_base = 0;
*aperture_size = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7c21ffe63ebc..9c3ce1ae66ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -593,8 +593,8 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device *adev, u32 
index)
if (amdgpu_device_skip_hw_access(adev))
return 0;
 
-   if (index < adev->doorbell.num_doorbells) {
-   return readl(adev->mman.doorbell_aper_base_kaddr + index);
+   if (index < adev->mman.num_doorbells) {
+   return readl(adev->mman.doorbell_va + index);
} else {
DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
return 0;
@@ -616,8 +616,8 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device *adev, u32 
index, u32 v)
if (amdgpu_device_skip_hw_access(adev))
return;
 
-   if (index < adev->doorbell.num_doorbells) {
-   writel(v, adev->mman.doorbell_aper_base_kaddr + index);
+   if (index < adev->mman.num_doorbells) {
+   writel(v, adev->mman.doorbell_va + index);
} else {
DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
}
@@ -637,8 +637,8 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device *adev, u32 
index)
if (amdgpu_device_skip_hw_access(adev))
return 0;
 
-   if (index < adev->doorbell.num_doorbells) {
-   return atomic64_read((atomic64_t 
*)(adev->mman.doorbell_aper_base_kaddr + index));
+   if (index < adev->mman.num_doorbells) {
+   return atomic64_read((atomic64_t *)(adev->mman.doorbell_va + 
index));
} else {
DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
return 0;
@@ -660,8 +660,8 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device *adev, u32 
index, u64 v)
if (amdgpu_device_skip_hw_access(adev))
return;
 
-   if (index < adev->doorbell.num_doorbells) {
-   atomic64_set((atomic64_t *)(adev->mman.doorbell_aper_base_kaddr 
+ index), v);
+   if (index < adev->mman.num_doorbells) {
+   atomic64_set((atomic64_t *)(adev->mman.doorbell_va + index), v);
} else {
DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
}
diff --git