Re: [PATCH 03/14] drm/amdgpu/ih: Don't perturb HW registers when accessing soft IH ring

["Christian König" <christian.koenig@amd.com>]

On 7/1/26 18:17, Timur Kristóf wrote:
> The soft IH ring is implemented entirely in software.
> We shouldn't read (or write) any HW registers when accessing it.
> 
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Reviewed-by: Christian König <christian.koenig@amd.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;
> +
>  	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;
>  	}