Re: [PATCH 2/3] amdgpu/ih: Don't perturb HW registers when accessing soft IH ring

[Tvrtko Ursulin]

On 13/05/2026 18:08, Timur Kristóf wrote:
The soft IH ring is implemented entirely in software.
We shouldn't read (or write) and HW registers when accessing it.

Signed-off-by: Timur Kristóf <timur.kris...@gmail.com>
---
  drivers/gpu/drm/amd/amdgpu/ih_v6_0.c   | 7 +++++++
  drivers/gpu/drm/amd/amdgpu/ih_v6_1.c   | 7 +++++++
  drivers/gpu/drm/amd/amdgpu/ih_v7_0.c   | 7 +++++++
  drivers/gpu/drm/amd/amdgpu/navi10_ih.c | 4 ++++
  4 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/ih_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/ih_v6_0.c
index 333e9c30c091..65e5d21753f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/ih_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/ih_v6_0.c
@@ -439,6 +439,10 @@ static u32 ih_v6_0_get_wptr(struct amdgpu_device *adev,
        struct amdgpu_ih_regs *ih_regs;
  
  	wptr = le32_to_cpu(*ih->wptr_cpu);
+
+       if (ih == &adev->irq.ih_soft)
+               goto out;
+

Would it be feasible to move amdgpu_ih_funcs from device global into the 
IH rings themselves? Then we could have soft IH ops and it would be very 
clean.

Possibly also cleanup all the protoptyes to operate only on ih and not 
the adev + ih pair.

Regards,

Tvrtko

        ih_regs = &ih->ih_regs;
  
  	if (!REG_GET_FIELD(wptr, IH_RB_WPTR, RB_OVERFLOW))
@@ -514,6 +518,9 @@ static void ih_v6_0_set_rptr(struct amdgpu_device *adev,
  {
        struct amdgpu_ih_regs *ih_regs;
  
+	if (ih == &adev->irq.ih_soft)
+               return;
+
        if (ih->use_doorbell) {
                /* XXX check if swapping is necessary on BE */
                *ih->rptr_cpu = ih->rptr;
diff --git a/drivers/gpu/drm/amd/amdgpu/ih_v6_1.c 
b/drivers/gpu/drm/amd/amdgpu/ih_v6_1.c
index 699c274d357e..9dbc20131410 100644
--- a/drivers/gpu/drm/amd/amdgpu/ih_v6_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/ih_v6_1.c
@@ -410,6 +410,10 @@ static u32 ih_v6_1_get_wptr(struct amdgpu_device *adev,
        struct amdgpu_ih_regs *ih_regs;
  
  	wptr = le32_to_cpu(*ih->wptr_cpu);
+
+       if (ih == &adev->irq.ih_soft)
+               goto out;
+
        ih_regs = &ih->ih_regs;
  
  	if (!REG_GET_FIELD(wptr, IH_RB_WPTR, RB_OVERFLOW))
@@ -481,6 +485,9 @@ static void ih_v6_1_irq_rearm(struct amdgpu_device *adev,
  static void ih_v6_1_set_rptr(struct amdgpu_device *adev,
                               struct amdgpu_ih_ring *ih)
  {
+       if (ih == &adev->irq.ih_soft)
+               return;
+
        struct amdgpu_ih_regs *ih_regs;
  
  	if (ih->use_doorbell) {
diff --git a/drivers/gpu/drm/amd/amdgpu/ih_v7_0.c 
b/drivers/gpu/drm/amd/amdgpu/ih_v7_0.c
index 6de9e87e04e1..bd332e8cc5bf 100644
--- a/drivers/gpu/drm/amd/amdgpu/ih_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/ih_v7_0.c
@@ -457,6 +457,10 @@ static u32 ih_v7_0_get_wptr(struct amdgpu_device *adev,
        struct amdgpu_ih_regs *ih_regs;
  
  	wptr = le32_to_cpu(*ih->wptr_cpu);
+
+       if (ih == &adev->irq.ih_soft)
+               goto out;
+
        ih_regs = &ih->ih_regs;
  
  	if (!REG_GET_FIELD(wptr, IH_RB_WPTR, RB_OVERFLOW))
@@ -527,6 +531,9 @@ static void ih_v7_0_set_rptr(struct amdgpu_device *adev,
  {
        struct amdgpu_ih_regs *ih_regs;
  
+	if (ih == &adev->irq.ih_soft)
+               return;
+
        if (ih->use_doorbell) {
                /* XXX check if swapping is necessary on BE */
                *ih->rptr_cpu = ih->rptr;
diff --git a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c 
b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
index 4cd325149b63..e7ed37bb48e0 100644
--- a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
@@ -417,6 +417,10 @@ static u32 navi10_ih_get_wptr(struct amdgpu_device *adev,
                 */
                wptr = le32_to_cpu(*ih->wptr_cpu);
  
+		if (ih == &adev->irq.ih_soft)
+                       goto out;
+
+
                if (!REG_GET_FIELD(wptr, IH_RB_WPTR, RB_OVERFLOW))
                        goto out;
        }