[PATCH] drm/amdgpu: skipping SDMA IP suspend for S0ix.

[PATCH] drm/amdgpu: skipping SDMA IP suspend for S0ix.

[Rajib Mahapatra]

[Why]
amdgpu error observed if suspend is aborted during S0i3
resume.

[How]
If suspend is aborted for some reason during S0i3 resume
cycle, it follows amdgpu errors in resume.
Skipping SDMA ip in suspend solves the issue on RENOIR
(green sardine apu) chip. This time, the system is
able to resume gracefully even the suspend is aborted.

Signed-off-by: Rajib Mahapatra <rajib.mahapatra@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7931132ce6e3..f01b1dffff7f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2927,6 +2927,16 @@ static int amdgpu_device_ip_suspend_phase2(struct amdgpu_device *adev)
 		     adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GFX))
 			continue;
 
+		/* skip suspend of sdma for S0ix
+		 * Resume has issues if the suspend is aborted during S0i3 cycle.
+		 * Skipping sdma for RN/CZN/BRC chip - green sardine apu.
+		 */
+		if (adev->in_s0ix &&
+		    (adev->asic_type == CHIP_RENOIR &&
+		     (adev->pdev->device == 0x15e7 || adev->pdev->device == 0x1638) &&
+		     adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_SDMA))
+			continue;
+
 		/* XXX handle errors */
 		r = adev->ip_blocks[i].version->funcs->suspend(adev);
 		/* XXX handle errors */
-- 
2.25.1

RE: [PATCH] drm/amdgpu: skipping SDMA IP suspend for S0ix.

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Mahapatra, Rajib <Rajib.Mahapatra@amd.com>
> Sent: Wednesday, February 2, 2022 03:07
> To: Liang, Prike <Prike.Liang@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; S, Shirish <Shirish.S@amd.com>;
> Mahapatra, Rajib <Rajib.Mahapatra@amd.com>
> Subject: [PATCH] drm/amdgpu: skipping SDMA IP suspend for S0ix.
> 
> [Why]
> amdgpu error observed if suspend is aborted during S0i3
> resume.
> 
> [How]
> If suspend is aborted for some reason during S0i3 resume
> cycle, it follows amdgpu errors in resume.
> Skipping SDMA ip in suspend solves the issue on RENOIR
> (green sardine apu) chip. This time, the system is
> able to resume gracefully even the suspend is aborted.
> 
> Signed-off-by: Rajib Mahapatra <rajib.mahapatra@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7931132ce6e3..f01b1dffff7f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2927,6 +2927,16 @@ static int amdgpu_device_ip_suspend_phase2(struct
> amdgpu_device *adev)
>  		     adev->ip_blocks[i].version->type ==
> AMD_IP_BLOCK_TYPE_GFX))
>  			continue;
> 
> +		/* skip suspend of sdma for S0ix
> +		 * Resume has issues if the suspend is aborted during S0i3 cycle.
> +		 * Skipping sdma for RN/CZN/BRC chip - green sardine apu.
> +		 */
> +		if (adev->in_s0ix &&
> +		    (adev->asic_type == CHIP_RENOIR &&
> +		     (adev->pdev->device == 0x15e7 || adev->pdev->device ==
> 0x1638) &&
> +		     adev->ip_blocks[i].version->type ==
> AMD_IP_BLOCK_TYPE_SDMA))
> +			continue;
> +
>  		/* XXX handle errors */
>  		r = adev->ip_blocks[i].version->funcs->suspend(adev);
>  		/* XXX handle errors */
> --
> 2.25.1

As this is specific to RN/CZN I think this check is better suited in
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c perhaps in 
sdma_v4_0_suspend/sdma_v4_0_resume functions.

The aborted suspend case if it's specific to s0ix, can also use adev->in_s0ix as
part of the check.

Re: [PATCH] drm/amdgpu: skipping SDMA IP suspend for S0ix.

[Alex Deucher]

On Wed, Feb 2, 2022 at 4:07 AM Rajib Mahapatra <rajib.mahapatra@amd.com> wrote:
>
> [Why]
> amdgpu error observed if suspend is aborted during S0i3
> resume.
>
> [How]
> If suspend is aborted for some reason during S0i3 resume
> cycle, it follows amdgpu errors in resume.
> Skipping SDMA ip in suspend solves the issue on RENOIR
> (green sardine apu) chip. This time, the system is
> able to resume gracefully even the suspend is aborted.
>
> Signed-off-by: Rajib Mahapatra <rajib.mahapatra@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7931132ce6e3..f01b1dffff7f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2927,6 +2927,16 @@ static int amdgpu_device_ip_suspend_phase2(struct amdgpu_device *adev)
>                      adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GFX))
>                         continue;
>
> +               /* skip suspend of sdma for S0ix
> +                * Resume has issues if the suspend is aborted during S0i3 cycle.
> +                * Skipping sdma for RN/CZN/BRC chip - green sardine apu.
> +                */
> +               if (adev->in_s0ix &&
> +                   (adev->asic_type == CHIP_RENOIR &&
> +                    (adev->pdev->device == 0x15e7 || adev->pdev->device == 0x1638) &&

The check here seems to contradict the comment above.  Is this all
Renoir based APUs or just green sardine?  If it's just green sardine,
you can check the APU flags rather than the PCI ids.  E.g.,
(adev->apu_flags & AMD_APU_IS_GREEN_SARDINE)
Also move this to sdma 4 code as Mario suggested.

Alex

> +                    adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_SDMA))
> +                       continue;
> +
>                 /* XXX handle errors */
>                 r = adev->ip_blocks[i].version->funcs->suspend(adev);
>                 /* XXX handle errors */
> --
> 2.25.1
>

Re: [PATCH] drm/amdgpu: skipping SDMA IP suspend for S0ix.

[Limonciello, Mario]

On 2/2/2022 08:16, Alex Deucher wrote:
> On Wed, Feb 2, 2022 at 4:07 AM Rajib Mahapatra <rajib.mahapatra@amd.com> wrote:
>>
>> [Why]
>> amdgpu error observed if suspend is aborted during S0i3
>> resume.
>>
>> [How]
>> If suspend is aborted for some reason during S0i3 resume
>> cycle, it follows amdgpu errors in resume.
>> Skipping SDMA ip in suspend solves the issue on RENOIR
>> (green sardine apu) chip. This time, the system is
>> able to resume gracefully even the suspend is aborted.
>>
>> Signed-off-by: Rajib Mahapatra <rajib.mahapatra@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 7931132ce6e3..f01b1dffff7f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2927,6 +2927,16 @@ static int amdgpu_device_ip_suspend_phase2(struct amdgpu_device *adev)
>>                       adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GFX))
>>                          continue;
>>
>> +               /* skip suspend of sdma for S0ix
>> +                * Resume has issues if the suspend is aborted during S0i3 cycle.
>> +                * Skipping sdma for RN/CZN/BRC chip - green sardine apu.
>> +                */
>> +               if (adev->in_s0ix &&
>> +                   (adev->asic_type == CHIP_RENOIR &&
>> +                    (adev->pdev->device == 0x15e7 || adev->pdev->device == 0x1638) &&
> 
> The check here seems to contradict the comment above.  Is this all
> Renoir based APUs or just green sardine?  If it's just green sardine,
> you can check the APU flags rather than the PCI ids.  E.g.,
> (adev->apu_flags & AMD_APU_IS_GREEN_SARDINE)
> Also move this to sdma 4 code as Mario suggested.

Both RN and green sardine share the same flows for SMU, I would think it 
should just be match (adev->in_s0xi && (adev->flags & AMD_IS_APU)) when 
it's moved to skip suspend.

> 
> Alex
> 
>> +                    adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_SDMA))
>> +                       continue;
>> +
>>                  /* XXX handle errors */
>>                  r = adev->ip_blocks[i].version->funcs->suspend(adev);
>>                  /* XXX handle errors */
>> --
>> 2.25.1
>>

Re: [PATCH] drm/amdgpu: skipping SDMA IP suspend for S0ix.

[Alex Deucher]

On Wed, Feb 2, 2022 at 10:29 AM Limonciello, Mario
<mario.limonciello@amd.com> wrote:
>
> On 2/2/2022 08:16, Alex Deucher wrote:
> > On Wed, Feb 2, 2022 at 4:07 AM Rajib Mahapatra <rajib.mahapatra@amd.com> wrote:
> >>
> >> [Why]
> >> amdgpu error observed if suspend is aborted during S0i3
> >> resume.
> >>
> >> [How]
> >> If suspend is aborted for some reason during S0i3 resume
> >> cycle, it follows amdgpu errors in resume.
> >> Skipping SDMA ip in suspend solves the issue on RENOIR
> >> (green sardine apu) chip. This time, the system is
> >> able to resume gracefully even the suspend is aborted.
> >>
> >> Signed-off-by: Rajib Mahapatra <rajib.mahapatra@amd.com>
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++++
> >>   1 file changed, 10 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> index 7931132ce6e3..f01b1dffff7f 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> @@ -2927,6 +2927,16 @@ static int amdgpu_device_ip_suspend_phase2(struct amdgpu_device *adev)
> >>                       adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GFX))
> >>                          continue;
> >>
> >> +               /* skip suspend of sdma for S0ix
> >> +                * Resume has issues if the suspend is aborted during S0i3 cycle.
> >> +                * Skipping sdma for RN/CZN/BRC chip - green sardine apu.
> >> +                */
> >> +               if (adev->in_s0ix &&
> >> +                   (adev->asic_type == CHIP_RENOIR &&
> >> +                    (adev->pdev->device == 0x15e7 || adev->pdev->device == 0x1638) &&
> >
> > The check here seems to contradict the comment above.  Is this all
> > Renoir based APUs or just green sardine?  If it's just green sardine,
> > you can check the APU flags rather than the PCI ids.  E.g.,
> > (adev->apu_flags & AMD_APU_IS_GREEN_SARDINE)
> > Also move this to sdma 4 code as Mario suggested.
>
> Both RN and green sardine share the same flows for SMU, I would think it
> should just be match (adev->in_s0xi && (adev->flags & AMD_IS_APU)) when
> it's moved to skip suspend.

The SDMA 4.0 code is shared with Raven1/2 and Picasso as well.  We
should verify that it's required for them as well.

Alex

>
> >
> > Alex
> >
> >> +                    adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_SDMA))
> >> +                       continue;
> >> +
> >>                  /* XXX handle errors */
> >>                  r = adev->ip_blocks[i].version->funcs->suspend(adev);
> >>                  /* XXX handle errors */
> >> --
> >> 2.25.1
> >>
>