On Tuesday, June 16, 2026 1:39:56 PM Central European Summer Time Tvrtko
Ursulin wrote:
> 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 <[email protected]>
> > ---
> >
> > 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.
Sure, we can do that in the future. The reason I chose not to do that is
because that feels like an extremely intrusive refactor across all GPU
generations with a high chance of introducing regressions and not much value
to end users.
For now I would like to just focus on making the current code work well with
minimal refactoring, ie. just get the current soft IH ring implementation to
work reliably with retry faults.
In the meantime if you have a good idea how to refactor this code to make it
cleaner without breaking it, I'm happy to listen and we can make a plan how to
do that in a future series.
Thanks,
Timur
>
> > 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;
> >
> > }
