drm/amdgpu: Correct the irq types' num of sdma

Submitted by Deng, Emily on March 27, 2019, 7:38 a.m.

Details

Message ID 1553672291-31228-1-git-send-email-Emily.Deng@amd.com
State New
Headers show
Series "drm/amdgpu: Correct the irq types' num of sdma" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Deng, Emily March 27, 2019, 7:38 a.m.
Fix the issue about TDR-2 will have "fallback timer expired on ring sdma1".
It is because the wrong number of irq types setting.

Signed-off-by: Emily Deng <Emily.Deng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 3ac5abe..72ec51a 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1908,7 +1908,7 @@  static int sdma_v4_0_set_ecc_irq_state(struct amdgpu_device *adev,
 {
 	u32 sdma_edc_config;
 
-	u32 reg_offset = (type == AMDGPU_SDMA_IRQ_ECC0) ?
+	u32 reg_offset = (type == 0) ?
 		sdma_v4_0_get_reg_offset(adev, 0, mmSDMA0_EDC_CONFIG) :
 		sdma_v4_0_get_reg_offset(adev, 1, mmSDMA0_EDC_CONFIG);
 
@@ -2196,10 +2196,10 @@  static const struct amdgpu_irq_src_funcs sdma_v4_0_ecc_irq_funcs = {
 
 static void sdma_v4_0_set_irq_funcs(struct amdgpu_device *adev)
 {
-	adev->sdma.trap_irq.num_types = AMDGPU_SDMA_IRQ_LAST;
+	adev->sdma.trap_irq.num_types = 2;
 	adev->sdma.trap_irq.funcs = &sdma_v4_0_trap_irq_funcs;
 	adev->sdma.illegal_inst_irq.funcs = &sdma_v4_0_illegal_inst_irq_funcs;
-	adev->sdma.ecc_irq.num_types = AMDGPU_SDMA_IRQ_LAST;
+	adev->sdma.ecc_irq.num_types = 2;
 	adev->sdma.ecc_irq.funcs = &sdma_v4_0_ecc_irq_funcs;
 }
 

Comments

Am 27.03.19 um 08:38 schrieb Emily Deng:
> Fix the issue about TDR-2 will have "fallback timer expired on ring sdma1".
> It is because the wrong number of irq types setting.

Good catch, but the solution is not really clean. The correct approach 
would be to fix the amdgpu_sdma_irq enum.

See the definition:
> enum amdgpu_sdma_irq {
>         AMDGPU_SDMA_IRQ_TRAP0 = 0,
>         AMDGPU_SDMA_IRQ_TRAP1,
>         AMDGPU_SDMA_IRQ_ECC0,
>         AMDGPU_SDMA_IRQ_ECC1,
>
>         AMDGPU_SDMA_IRQ_LAST
> };

As far as I can see that doesn't make any sense, cause it denotes the 
source of the interrupt and not the type.

The AMDGPU_SDMA_IRQ_TRAP0 and AMDGPU_SDMA_IRQ_TRAP1 should be renamed to 
AMDGPU_SDMA_IRQ_INSTANCE0 and AMDGPU_SDMA_IRQ_INSTANCE1 and the _ECC 
values removed altogether.

Christian.

>
> Signed-off-by: Emily Deng <Emily.Deng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 3ac5abe..72ec51a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -1908,7 +1908,7 @@ static int sdma_v4_0_set_ecc_irq_state(struct amdgpu_device *adev,
>   {
>   	u32 sdma_edc_config;
>   
> -	u32 reg_offset = (type == AMDGPU_SDMA_IRQ_ECC0) ?
> +	u32 reg_offset = (type == 0) ?
>   		sdma_v4_0_get_reg_offset(adev, 0, mmSDMA0_EDC_CONFIG) :
>   		sdma_v4_0_get_reg_offset(adev, 1, mmSDMA0_EDC_CONFIG);
>   
> @@ -2196,10 +2196,10 @@ static const struct amdgpu_irq_src_funcs sdma_v4_0_ecc_irq_funcs = {
>   
>   static void sdma_v4_0_set_irq_funcs(struct amdgpu_device *adev)
>   {
> -	adev->sdma.trap_irq.num_types = AMDGPU_SDMA_IRQ_LAST;
> +	adev->sdma.trap_irq.num_types = 2;
>   	adev->sdma.trap_irq.funcs = &sdma_v4_0_trap_irq_funcs;
>   	adev->sdma.illegal_inst_irq.funcs = &sdma_v4_0_illegal_inst_irq_funcs;
> -	adev->sdma.ecc_irq.num_types = AMDGPU_SDMA_IRQ_LAST;
> +	adev->sdma.ecc_irq.num_types = 2;
>   	adev->sdma.ecc_irq.funcs = &sdma_v4_0_ecc_irq_funcs;
>   }
>
Thanks, will modify the patch as your good suggestion.

Best wishes
Emily Deng



>-----Original Message-----

>From: Christian König <ckoenig.leichtzumerken@gmail.com>

>Sent: Wednesday, March 27, 2019 7:40 PM

>To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@lists.freedesktop.org

>Subject: Re: [PATCH] drm/amdgpu: Correct the irq types' num of sdma

>

>Am 27.03.19 um 08:38 schrieb Emily Deng:

>> Fix the issue about TDR-2 will have "fallback timer expired on ring sdma1".

>> It is because the wrong number of irq types setting.

>

>Good catch, but the solution is not really clean. The correct approach would be to

>fix the amdgpu_sdma_irq enum.

>

>See the definition:

>> enum amdgpu_sdma_irq {

>>         AMDGPU_SDMA_IRQ_TRAP0 = 0,

>>         AMDGPU_SDMA_IRQ_TRAP1,

>>         AMDGPU_SDMA_IRQ_ECC0,

>>         AMDGPU_SDMA_IRQ_ECC1,

>>

>>         AMDGPU_SDMA_IRQ_LAST

>> };

>

>As far as I can see that doesn't make any sense, cause it denotes the source of the

>interrupt and not the type.

>

>The AMDGPU_SDMA_IRQ_TRAP0 and AMDGPU_SDMA_IRQ_TRAP1 should be

>renamed to

>AMDGPU_SDMA_IRQ_INSTANCE0 and AMDGPU_SDMA_IRQ_INSTANCE1 and the

>_ECC values removed altogether.

>

>Christian.

>

>>

>> Signed-off-by: Emily Deng <Emily.Deng@amd.com>

>> ---

>>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 6 +++---

>>   1 file changed, 3 insertions(+), 3 deletions(-)

>>

>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

>> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

>> index 3ac5abe..72ec51a 100644

>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

>> @@ -1908,7 +1908,7 @@ static int sdma_v4_0_set_ecc_irq_state(struct

>amdgpu_device *adev,

>>   {

>>   	u32 sdma_edc_config;

>>

>> -	u32 reg_offset = (type == AMDGPU_SDMA_IRQ_ECC0) ?

>> +	u32 reg_offset = (type == 0) ?

>>   		sdma_v4_0_get_reg_offset(adev, 0, mmSDMA0_EDC_CONFIG) :

>>   		sdma_v4_0_get_reg_offset(adev, 1, mmSDMA0_EDC_CONFIG);

>>

>> @@ -2196,10 +2196,10 @@ static const struct amdgpu_irq_src_funcs

>> sdma_v4_0_ecc_irq_funcs = {

>>

>>   static void sdma_v4_0_set_irq_funcs(struct amdgpu_device *adev)

>>   {

>> -	adev->sdma.trap_irq.num_types = AMDGPU_SDMA_IRQ_LAST;

>> +	adev->sdma.trap_irq.num_types = 2;

>>   	adev->sdma.trap_irq.funcs = &sdma_v4_0_trap_irq_funcs;

>>   	adev->sdma.illegal_inst_irq.funcs = &sdma_v4_0_illegal_inst_irq_funcs;

>> -	adev->sdma.ecc_irq.num_types = AMDGPU_SDMA_IRQ_LAST;

>> +	adev->sdma.ecc_irq.num_types = 2;

>>   	adev->sdma.ecc_irq.funcs = &sdma_v4_0_ecc_irq_funcs;

>>   }

>>