* [PATCH] drm/amdgpu: fix the hw hang during perform system reboot and reset @ 2020-04-13 15:08 Prike Liang 2020-04-13 15:14 ` Alex Deucher 2020-04-13 16:46 ` Johannes Hirte 0 siblings, 2 replies; 6+ messages in thread From: Prike Liang @ 2020-04-13 15:08 UTC (permalink / raw) To: amd-gfx; +Cc: Xinhui.Pan, Rahul.Kumar1, ray.huang, Prike Liang, johannes.hirte Unify set device CGPG to ungate state before enter poweroff or reset. Signed-off-by: Prike Liang <Prike.Liang@amd.com> Tested-by: Mengbing Wang <Mengbing.Wang@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 87f7c12..bbe090a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2413,6 +2413,8 @@ static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev) { int i, r; + amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE); + amdgpu_device_set_cg_state(adev, AMD_CG_STATE_UNGATE); for (i = adev->num_ip_blocks - 1; i >= 0; i--) { if (!adev->ip_blocks[i].status.valid) -- 2.7.4 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: fix the hw hang during perform system reboot and reset 2020-04-13 15:08 [PATCH] drm/amdgpu: fix the hw hang during perform system reboot and reset Prike Liang @ 2020-04-13 15:14 ` Alex Deucher 2020-04-13 18:17 ` Paul Menzel 2020-04-13 16:46 ` Johannes Hirte 1 sibling, 1 reply; 6+ messages in thread From: Alex Deucher @ 2020-04-13 15:14 UTC (permalink / raw) To: Prike Liang Cc: Rahul.Kumar1, xinhui pan, Huang Rui, amd-gfx list, Johannes Hirte On Mon, Apr 13, 2020 at 11:09 AM Prike Liang <Prike.Liang@amd.com> wrote: > > Unify set device CGPG to ungate state before enter poweroff or reset. > > Signed-off-by: Prike Liang <Prike.Liang@amd.com> > Tested-by: Mengbing Wang <Mengbing.Wang@amd.com> Acked-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 87f7c12..bbe090a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -2413,6 +2413,8 @@ static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev) > { > int i, r; > > + amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE); > + amdgpu_device_set_cg_state(adev, AMD_CG_STATE_UNGATE); > > for (i = adev->num_ip_blocks - 1; i >= 0; i--) { > if (!adev->ip_blocks[i].status.valid) > -- > 2.7.4 > > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: fix the hw hang during perform system reboot and reset 2020-04-13 15:14 ` Alex Deucher @ 2020-04-13 18:17 ` Paul Menzel 2020-04-13 20:32 ` Alex Deucher 0 siblings, 1 reply; 6+ messages in thread From: Paul Menzel @ 2020-04-13 18:17 UTC (permalink / raw) To: Alex Deucher, Prike Liang Cc: Rahul Kumar, Xinhui Pan, Huang Rui, amd-gfx, Johannes Hirte Dear Alex, dear Prike, Am 13.04.20 um 17:14 schrieb Alex Deucher: > On Mon, Apr 13, 2020 at 11:09 AM Prike Liang <Prike.Liang@amd.com> wrote: >> >> Unify set device CGPG to ungate state before enter poweroff or reset. >> >> Signed-off-by: Prike Liang <Prike.Liang@amd.com> >> Tested-by: Mengbing Wang <Mengbing.Wang@amd.com> > > Acked-by: Alex Deucher <alexander.deucher@amd.com> First: Tested-by: Paul Menzel <pmenzel@molgen.mpg.de> (MSI B350M MORTAR (MS-7A37) with an AMD Ryzen 3 2200G) Second, I am having trouble to understand, how you can add your Acked-by tag to a commit with such a commit message? The problem is not described (apparently it only affected certain devices), it is not mentioned that it’s a regression (Fixes: tag/line is missing), and I am having a hard time to understand the commit message at all (and the one from the commit introducing the regression). Why is it more or less reverting part of the other commit, while the issue was not reproducible on Prike’s system? >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> index 87f7c12..bbe090a 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> @@ -2413,6 +2413,8 @@ static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev) >> { >> int i, r; >> >> + amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE); >> + amdgpu_device_set_cg_state(adev, AMD_CG_STATE_UNGATE); >> >> for (i = adev->num_ip_blocks - 1; i >= 0; i--) { >> if (!adev->ip_blocks[i].status.valid) >> -- >> 2.7.4 Kind regards, Paul _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: fix the hw hang during perform system reboot and reset 2020-04-13 18:17 ` Paul Menzel @ 2020-04-13 20:32 ` Alex Deucher 2020-04-14 2:06 ` Liang, Prike 0 siblings, 1 reply; 6+ messages in thread From: Alex Deucher @ 2020-04-13 20:32 UTC (permalink / raw) To: Paul Menzel Cc: Johannes Hirte, Xinhui Pan, Rahul Kumar, amd-gfx list, Prike Liang, Huang Rui On Mon, Apr 13, 2020 at 2:17 PM Paul Menzel <pmenzel+amd-gfx@molgen.mpg.de> wrote: > > Dear Alex, dear Prike, > > > Am 13.04.20 um 17:14 schrieb Alex Deucher: > > On Mon, Apr 13, 2020 at 11:09 AM Prike Liang <Prike.Liang@amd.com> wrote: > >> > >> Unify set device CGPG to ungate state before enter poweroff or reset. > >> > >> Signed-off-by: Prike Liang <Prike.Liang@amd.com> > >> Tested-by: Mengbing Wang <Mengbing.Wang@amd.com> > > > > Acked-by: Alex Deucher <alexander.deucher@amd.com> > > First: > > Tested-by: Paul Menzel <pmenzel@molgen.mpg.de> (MSI B350M MORTAR > (MS-7A37) with an AMD Ryzen 3 2200G) > > Second, I am having trouble to understand, how you can add your Acked-by > tag to a commit with such a commit message? > > The problem is not described (apparently it only affected certain > devices), it is not mentioned that it’s a regression (Fixes: tag/line is > missing), and I am having a hard time to understand the commit message > at all (and the one from the commit introducing the regression). Why is > it more or less reverting part of the other commit, while the issue was > not reproducible on Prike’s system? The original issue was that we were not properly ungating some of the hw blocks in the right order for S3 suspend on renoir. So the fix was to add ungate calls to amdgpu_device_suspend() to handle that case. However, the original fix should not have removed the calls from amdgpu_device_ip_suspend_phase1() since that is called separately for some other use cases (e.g., pci shutdown). It didn't matter for some asics as they don't have different levels of powergating functionality. I'll add the fixes tag before the patch goes upstream. Alex > > >> --- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > >> index 87f7c12..bbe090a 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > >> @@ -2413,6 +2413,8 @@ static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev) > >> { > >> int i, r; > >> > >> + amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE); > >> + amdgpu_device_set_cg_state(adev, AMD_CG_STATE_UNGATE); > >> > >> for (i = adev->num_ip_blocks - 1; i >= 0; i--) { > >> if (!adev->ip_blocks[i].status.valid) > >> -- > >> 2.7.4 > > Kind regards, > > Paul _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: fix the hw hang during perform system reboot and reset 2020-04-13 20:32 ` Alex Deucher @ 2020-04-14 2:06 ` Liang, Prike 0 siblings, 0 replies; 6+ messages in thread From: Liang, Prike @ 2020-04-14 2:06 UTC (permalink / raw) To: Alex Deucher, Paul Menzel Cc: Kumar1, Rahul, Pan, Xinhui, Huang, Ray, amd-gfx list, Johannes Hirte > > On Mon, Apr 13, 2020 at 2:17 PM Paul Menzel <pmenzel+amd- > gfx@molgen.mpg.de> wrote: > > > > Dear Alex, dear Prike, > > > > > > Am 13.04.20 um 17:14 schrieb Alex Deucher: > > > On Mon, Apr 13, 2020 at 11:09 AM Prike Liang <Prike.Liang@amd.com> > wrote: > > >> > > >> Unify set device CGPG to ungate state before enter poweroff or reset. > > >> > > >> Signed-off-by: Prike Liang <Prike.Liang@amd.com> > > >> Tested-by: Mengbing Wang <Mengbing.Wang@amd.com> > > > > > > Acked-by: Alex Deucher <alexander.deucher@amd.com> > > > > First: > > > > Tested-by: Paul Menzel <pmenzel@molgen.mpg.de> (MSI B350M MORTAR > > (MS-7A37) with an AMD Ryzen 3 2200G) > > > > Second, I am having trouble to understand, how you can add your > > Acked-by tag to a commit with such a commit message? > > > > The problem is not described (apparently it only affected certain > > devices), it is not mentioned that it’s a regression (Fixes: tag/line > > is missing), and I am having a hard time to understand the commit > > message at all (and the one from the commit introducing the > > regression). Why is it more or less reverting part of the other > > commit, while the issue was not reproducible on Prike’s system? > > The original issue was that we were not properly ungating some of the hw > blocks in the right order for S3 suspend on renoir. So the fix was to add > ungate calls to amdgpu_device_suspend() to handle that case. > However, the original fix should not have removed the calls from > amdgpu_device_ip_suspend_phase1() since that is called separately for > some other use cases (e.g., pci shutdown). It didn't matter for some asics as > they don't have different levels of powergating functionality. I'll add the fixes > tag before the patch goes upstream. > > Alex > [Prike] Thanks Alex help clarify and I will give more detail in the message. > > > > >> --- > > >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 ++ > > >> 1 file changed, 2 insertions(+) > > >> > > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > >> index 87f7c12..bbe090a 100644 > > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > >> @@ -2413,6 +2413,8 @@ static int > amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev) > > >> { > > >> int i, r; > > >> > > >> + amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE); > > >> + amdgpu_device_set_cg_state(adev, AMD_CG_STATE_UNGATE); > > >> > > >> for (i = adev->num_ip_blocks - 1; i >= 0; i--) { > > >> if (!adev->ip_blocks[i].status.valid) > > >> -- > > >> 2.7.4 > > > > Kind regards, > > > > Paul _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: fix the hw hang during perform system reboot and reset 2020-04-13 15:08 [PATCH] drm/amdgpu: fix the hw hang during perform system reboot and reset Prike Liang 2020-04-13 15:14 ` Alex Deucher @ 2020-04-13 16:46 ` Johannes Hirte 1 sibling, 0 replies; 6+ messages in thread From: Johannes Hirte @ 2020-04-13 16:46 UTC (permalink / raw) To: Prike Liang; +Cc: Xinhui.Pan, Rahul.Kumar1, ray.huang, amd-gfx On 2020 Apr 13, Prike Liang wrote: > Unify set device CGPG to ungate state before enter poweroff or reset. > > Signed-off-by: Prike Liang <Prike.Liang@amd.com> > Tested-by: Mengbing Wang <Mengbing.Wang@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 87f7c12..bbe090a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -2413,6 +2413,8 @@ static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev) > { > int i, r; > > + amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE); > + amdgpu_device_set_cg_state(adev, AMD_CG_STATE_UNGATE); > > for (i = adev->num_ip_blocks - 1; i >= 0; i--) { > if (!adev->ip_blocks[i].status.valid) > -- > 2.7.4 > I can confirm that this fixes the shutdown/reboot hang on my raven. -- Regards, Johannes Hirte _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-14 2:06 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-13 15:08 [PATCH] drm/amdgpu: fix the hw hang during perform system reboot and reset Prike Liang 2020-04-13 15:14 ` Alex Deucher 2020-04-13 18:17 ` Paul Menzel 2020-04-13 20:32 ` Alex Deucher 2020-04-14 2:06 ` Liang, Prike 2020-04-13 16:46 ` Johannes Hirte
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox