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

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

[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@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)
+		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) {
+		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);
 
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [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@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@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);
>  
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Alex Deucher]

On Wed, Sep 2, 2020 at 1:01 PM Alex Sierra <alex.sierra@amd.com> wrote:
>
> 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
>

Before committing, let's verify that it fixes that issue.

Alex


> Signed-off-by: Alex Sierra <alex.sierra@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)
> +               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) {
> +               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);
>
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Felix Kuehling]

Am 2020-09-02 um 2:08 p.m. schrieb Alex Deucher:
> On Wed, Sep 2, 2020 at 1:01 PM Alex Sierra <alex.sierra@amd.com> wrote:
>> 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
>>
> Before committing, let's verify that it fixes that issue.

Has anyone reproduced this in AMD? Or should we ask the gitlab issue
reporter to test the patch?

Thanks,
  Felix


>
> Alex
>
>
>> Signed-off-by: Alex Sierra <alex.sierra@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)
>> +               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) {
>> +               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);
>>
>> --
>> 2.17.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Alex Deucher]

On Wed, Sep 2, 2020 at 2:08 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Wed, Sep 2, 2020 at 1:01 PM Alex Sierra <alex.sierra@amd.com> wrote:
> >
> > 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
> >
>
> Before committing, let's verify that it fixes that issue.

Looking at the bug report, the OSS (presumably IH) block is causing a
write fault so I suspect arcturus may be affected by this as well.  We
should double check the ring sizes and allocations.

Alex


>
> Alex
>
>
> > Signed-off-by: Alex Sierra <alex.sierra@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)
> > +               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) {
> > +               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);
> >
> > --
> > 2.17.1
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Alex Deucher]

On Wed, Sep 2, 2020 at 2:10 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>
> Am 2020-09-02 um 2:08 p.m. schrieb Alex Deucher:
> > On Wed, Sep 2, 2020 at 1:01 PM Alex Sierra <alex.sierra@amd.com> wrote:
> >> 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
> >>
> > Before committing, let's verify that it fixes that issue.
>
> Has anyone reproduced this in AMD? Or should we ask the gitlab issue
> reporter to test the patch?

I've asked on the bug report.  I think Nicolai reported an mmhub error
at some point, but I can't find the reference now.  I haven't heard of
anything else.

Alex

>
> Thanks,
>   Felix
>
>
> >
> > Alex
> >
> >
> >> Signed-off-by: Alex Sierra <alex.sierra@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)
> >> +               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) {
> >> +               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);
> >>
> >> --
> >> 2.17.1
> >>
> >> _______________________________________________
> >> amd-gfx mailing list
> >> amd-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Felix Kuehling]

Am 2020-09-02 um 2:13 p.m. schrieb Alex Deucher:
> On Wed, Sep 2, 2020 at 2:08 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Wed, Sep 2, 2020 at 1:01 PM Alex Sierra <alex.sierra@amd.com> wrote:
>>> 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
>>>
>> Before committing, let's verify that it fixes that issue.
> Looking at the bug report, the OSS (presumably IH) block is causing a
> write fault so I suspect arcturus may be affected by this as well.  We
> should double check the ring sizes and allocations.

Alejandro has been doing a lot of testing on Arcturus and didn't run
into this problem. That's why I suggested only disabling the IH rings on
Navi10 for now. We need the extra rings on Arcturus for our HMM work.

Regards,
  Felix


>
> Alex
>
>
>> Alex
>>
>>
>>> Signed-off-by: Alex Sierra <alex.sierra@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)
>>> +               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) {
>>> +               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);
>>>
>>> --
>>> 2.17.1
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Christian König]

Am 02.09.20 um 20:28 schrieb Felix Kuehling:
> Am 2020-09-02 um 2:13 p.m. schrieb Alex Deucher:
>> On Wed, Sep 2, 2020 at 2:08 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>>> On Wed, Sep 2, 2020 at 1:01 PM Alex Sierra <alex.sierra@amd.com> wrote:
>>>> 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
>>>>
>>> Before committing, let's verify that it fixes that issue.
>> Looking at the bug report, the OSS (presumably IH) block is causing a
>> write fault so I suspect arcturus may be affected by this as well.  We
>> should double check the ring sizes and allocations.
> Alejandro has been doing a lot of testing on Arcturus and didn't run
> into this problem. That's why I suggested only disabling the IH rings on
> Navi10 for now. We need the extra rings on Arcturus for our HMM work.

I think we should further investigate this before applying any patches.

When Navi10 is affected it's likely that other Navi generations are as 
well since AFAIK the OSS hasn't changed much between generations.

Regards,
Christian.

>
> Regards,
>    Felix
>
>
>> Alex
>>
>>
>>> Alex
>>>
>>>
>>>> Signed-off-by: Alex Sierra <alex.sierra@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)
>>>> +               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) {
>>>> +               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);
>>>>
>>>> --
>>>> 2.17.1
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Matt Coffin]

[-- Attachment #1.1.1: Type: text/plain, Size: 972 bytes --]

Hello all,



Thought I'd chime in here. While I can't speak to the issue the GitLab
reporter was experiencing, I can say that I get some performance hit
(expected) from this when using multiple monitors with DRI_PRIME. When
the second monitor is active (with something rendering to it with
relative frequency), then I see a decent performance drop in
applications running on my primary monitor compared to before this.



While this is probably expected, and imperceptible to the standard user,
I thought I'd at least mention it in an effort to keep contributing.



Thanks for the continued open source work. You all make my life.



Cheers,

Matt

On 9/3/20 2:05 AM, Christian König wrote:
> Am 02.09.20 um 20:28 schrieb Felix Kuehling:
>> Am 2020-09-02 um 2:13 p.m. schrieb Alex Deucher:
>>> On Wed, Sep 2, 2020 at 2:08 PM Alex Deucher <alexdeucher@gmail.com>
> I think we should further investigate this before applying any patches.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Felix Kuehling]

Am 2020-09-07 um 1:00 a.m. schrieb Matt Coffin:

> Hello all,
>
>
>
> Thought I'd chime in here. While I can't speak to the issue the GitLab
> reporter was experiencing, I can say that I get some performance hit
> (expected) from this when using multiple monitors with DRI_PRIME. When
> the second monitor is active (with something rendering to it with
> relative frequency), then I see a decent performance drop in
> applications running on my primary monitor compared to before this.

To be clear, are you seeing a performance drop from this partial revert,
or from the original change that enabled ih1 and ih2?

I would not expect this change to affect performance for anything other
than page fault interrupt handling. You should not be seeing GPU page
fault interrupts while running graphics applications.

Regards,
  Felix


>
>
>
> While this is probably expected, and imperceptible to the standard user,
> I thought I'd at least mention it in an effort to keep contributing.
>
>
>
> Thanks for the continued open source work. You all make my life.
>
>
>
> Cheers,
>
> Matt
>
> On 9/3/20 2:05 AM, Christian König wrote:
>> Am 02.09.20 um 20:28 schrieb Felix Kuehling:
>>> Am 2020-09-02 um 2:13 p.m. schrieb Alex Deucher:
>>>> On Wed, Sep 2, 2020 at 2:08 PM Alex Deucher <alexdeucher@gmail.com>
>> I think we should further investigate this before applying any patches.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx