[PATCH] drm/amdgpu: enable ih1 ih2 for Arcturus only

[Felix Kuehling]

Am 2020-09-02 um 1:01 p.m. schrieb Alex Sierra:
> Enable multi-ring ih1 and ih2 for Arcturus only.
> For Navi10 family multi-ring has been disabled.
> Apparently, having multi-ring enabled in Navi was causing
> continus page fault interrupts.
> Further investigation is needed to get to the root cause.
> Related issue link:
> https://gitlab.freedesktop.org/drm/amd/-/issues/1279
>
> Signed-off-by: Alex Sierra <alex.sierra at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/navi10_ih.c | 30 ++++++++++++++++----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
> index 350f1bf063c6..4d73869870ab 100644
> --- a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
> +++ b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
> @@ -306,7 +306,8 @@ static int navi10_ih_irq_init(struct amdgpu_device *adev)
>  	} else {
>  		WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL, ih_rb_cntl);
>  	}
> -	navi10_ih_reroute_ih(adev);
> +	if (adev->asic_type == CHIP_ARCTURUS)

Instead of looking at the asic_type here, it would be better to check
the IH ring sizes. They are also used further down in this function as
the condition to enable the extra interrupt rings. It would be more
consistent and this way, the ASIC-specific condition would be limited to
one function, navi10_ih_sw_init.


> +		navi10_ih_reroute_ih(adev);
>  
>  	if (unlikely(adev->firmware.load_type == AMDGPU_FW_LOAD_DIRECT)) {
>  		if (ih->use_bus_addr) {
> @@ -668,19 +669,26 @@ static int navi10_ih_sw_init(void *handle)
>  	adev->irq.ih.use_doorbell = true;
>  	adev->irq.ih.doorbell_index = adev->doorbell_index.ih << 1;
>  
> -	r = amdgpu_ih_ring_init(adev, &adev->irq.ih1, PAGE_SIZE, true);
> -	if (r)
> -		return r;
> +	adev->irq.ih1.ring_size = 0;
> +	adev->irq.ih2.ring_size = 0;
>  
> -	adev->irq.ih1.use_doorbell = true;
> -	adev->irq.ih1.doorbell_index = (adev->doorbell_index.ih + 1) << 1;
> +	if (adev->asic_type == CHIP_ARCTURUS) {

This may apply to the Arcturus successor as well. I'd use asic_type <
NAVI10 instead, to be future-proof.

With these two issues fixed, the patch is

Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>


> +		r = amdgpu_ih_ring_init(adev, &adev->irq.ih1, PAGE_SIZE, true);
> +		if (r)
> +			return r;
>  
> -	r = amdgpu_ih_ring_init(adev, &adev->irq.ih2, PAGE_SIZE, true);
> -	if (r)
> -		return r;
> +		adev->irq.ih1.use_doorbell = true;
> +		adev->irq.ih1.doorbell_index =
> +					(adev->doorbell_index.ih + 1) << 1;
> +
> +		r = amdgpu_ih_ring_init(adev, &adev->irq.ih2, PAGE_SIZE, true);
> +		if (r)
> +			return r;
>  
> -	adev->irq.ih2.use_doorbell = true;
> -	adev->irq.ih2.doorbell_index = (adev->doorbell_index.ih + 2) << 1;
> +		adev->irq.ih2.use_doorbell = true;
> +		adev->irq.ih2.doorbell_index =
> +					(adev->doorbell_index.ih + 2) << 1;
> +	}
>  
>  	r = amdgpu_irq_init(adev);
>