* [PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware
@ 2020-09-01 6:32 Kai-Heng Feng
2020-09-01 14:19 ` Alex Deucher
0 siblings, 1 reply; 5+ messages in thread
From: Kai-Heng Feng @ 2020-09-01 6:32 UTC (permalink / raw)
To: alexander.deucher, christian.koenig
Cc: Kai-Heng Feng, David Airlie, Daniel Vetter,
open list:RADEON and AMDGPU DRM DRIVERS, open list:DRM DRIVERS,
open list
Suspend with s2idle or by the following steps cause screen frozen:
# echo devices > /sys/power/pm_test
# echo freeze > /sys/power/mem
[ 289.625461] [drm:uvd_v1_0_ib_test [radeon]] *ERROR* radeon: fence wait timed out.
[ 289.625494] [drm:radeon_ib_ring_tests [radeon]] *ERROR* radeon: failed testing IB on ring 5 (-110).
The issue doesn't happen on traditional S3, probably because firmware or
hardware provides extra power management.
Inspired by Daniel Drake's patch [1] on amdgpu, using a similar approach
can fix the issue.
[1] https://patchwork.freedesktop.org/patch/335839/
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
drivers/gpu/drm/radeon/radeon_device.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 266e3cbbd09b..df823b9ad79f 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -33,6 +33,7 @@
#include <linux/slab.h>
#include <linux/vga_switcheroo.h>
#include <linux/vgaarb.h>
+#include <linux/suspend.h>
#include <drm/drm_cache.h>
#include <drm/drm_crtc_helper.h>
@@ -1643,6 +1644,8 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
rdev->asic->asic_reset(rdev, true);
pci_restore_state(dev->pdev);
} else if (suspend) {
+ if (pm_suspend_no_platform())
+ rdev->asic->asic_reset(rdev, true);
/* Shut down the device */
pci_disable_device(dev->pdev);
pci_set_power_state(dev->pdev, PCI_D3hot);
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware
2020-09-01 6:32 [PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware Kai-Heng Feng
@ 2020-09-01 14:19 ` Alex Deucher
2020-09-01 16:20 ` Kai-Heng Feng
0 siblings, 1 reply; 5+ messages in thread
From: Alex Deucher @ 2020-09-01 14:19 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: Deucher, Alexander, Christian Koenig, David Airlie, open list,
open list:DRM DRIVERS, open list:RADEON and AMDGPU DRM DRIVERS
On Tue, Sep 1, 2020 at 3:32 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> Suspend with s2idle or by the following steps cause screen frozen:
> # echo devices > /sys/power/pm_test
> # echo freeze > /sys/power/mem
>
> [ 289.625461] [drm:uvd_v1_0_ib_test [radeon]] *ERROR* radeon: fence wait timed out.
> [ 289.625494] [drm:radeon_ib_ring_tests [radeon]] *ERROR* radeon: failed testing IB on ring 5 (-110).
>
> The issue doesn't happen on traditional S3, probably because firmware or
> hardware provides extra power management.
>
> Inspired by Daniel Drake's patch [1] on amdgpu, using a similar approach
> can fix the issue.
It doesn't actually fix the issue. The device is never powered down
so you are using more power than you would if you did not suspend in
the first place. The reset just works around the fact that the device
is never powered down.
Alex
>
> [1] https://patchwork.freedesktop.org/patch/335839/
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> drivers/gpu/drm/radeon/radeon_device.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index 266e3cbbd09b..df823b9ad79f 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -33,6 +33,7 @@
> #include <linux/slab.h>
> #include <linux/vga_switcheroo.h>
> #include <linux/vgaarb.h>
> +#include <linux/suspend.h>
>
> #include <drm/drm_cache.h>
> #include <drm/drm_crtc_helper.h>
> @@ -1643,6 +1644,8 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
> rdev->asic->asic_reset(rdev, true);
> pci_restore_state(dev->pdev);
> } else if (suspend) {
> + if (pm_suspend_no_platform())
> + rdev->asic->asic_reset(rdev, true);
> /* Shut down the device */
> pci_disable_device(dev->pdev);
> pci_set_power_state(dev->pdev, PCI_D3hot);
> --
> 2.17.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware
2020-09-01 14:19 ` Alex Deucher
@ 2020-09-01 16:20 ` Kai-Heng Feng
2020-09-01 16:30 ` Alex Deucher
0 siblings, 1 reply; 5+ messages in thread
From: Kai-Heng Feng @ 2020-09-01 16:20 UTC (permalink / raw)
To: Alex Deucher
Cc: Deucher, Alexander, Christian Koenig, David Airlie, open list,
open list:DRM DRIVERS, open list:RADEON and AMDGPU DRM DRIVERS
> On Sep 1, 2020, at 22:19, Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Tue, Sep 1, 2020 at 3:32 AM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
>>
>> Suspend with s2idle or by the following steps cause screen frozen:
>> # echo devices > /sys/power/pm_test
>> # echo freeze > /sys/power/mem
>>
>> [ 289.625461] [drm:uvd_v1_0_ib_test [radeon]] *ERROR* radeon: fence wait timed out.
>> [ 289.625494] [drm:radeon_ib_ring_tests [radeon]] *ERROR* radeon: failed testing IB on ring 5 (-110).
>>
>> The issue doesn't happen on traditional S3, probably because firmware or
>> hardware provides extra power management.
>>
>> Inspired by Daniel Drake's patch [1] on amdgpu, using a similar approach
>> can fix the issue.
>
> It doesn't actually fix the issue. The device is never powered down
> so you are using more power than you would if you did not suspend in
> the first place. The reset just works around the fact that the device
> is never powered down.
So how do we properly suspend/resume the device without help from platform firmware?
Kai-Heng
>
> Alex
>
>>
>> [1] https://patchwork.freedesktop.org/patch/335839/
>>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>> drivers/gpu/drm/radeon/radeon_device.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
>> index 266e3cbbd09b..df823b9ad79f 100644
>> --- a/drivers/gpu/drm/radeon/radeon_device.c
>> +++ b/drivers/gpu/drm/radeon/radeon_device.c
>> @@ -33,6 +33,7 @@
>> #include <linux/slab.h>
>> #include <linux/vga_switcheroo.h>
>> #include <linux/vgaarb.h>
>> +#include <linux/suspend.h>
>>
>> #include <drm/drm_cache.h>
>> #include <drm/drm_crtc_helper.h>
>> @@ -1643,6 +1644,8 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
>> rdev->asic->asic_reset(rdev, true);
>> pci_restore_state(dev->pdev);
>> } else if (suspend) {
>> + if (pm_suspend_no_platform())
>> + rdev->asic->asic_reset(rdev, true);
>> /* Shut down the device */
>> pci_disable_device(dev->pdev);
>> pci_set_power_state(dev->pdev, PCI_D3hot);
>> --
>> 2.17.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware
2020-09-01 16:20 ` Kai-Heng Feng
@ 2020-09-01 16:30 ` Alex Deucher
2020-09-02 0:39 ` Kai-Heng Feng
0 siblings, 1 reply; 5+ messages in thread
From: Alex Deucher @ 2020-09-01 16:30 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: Deucher, Alexander, Christian Koenig, David Airlie, open list,
open list:DRM DRIVERS, open list:RADEON and AMDGPU DRM DRIVERS
On Tue, Sep 1, 2020 at 12:21 PM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
>
>
> > On Sep 1, 2020, at 22:19, Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> > On Tue, Sep 1, 2020 at 3:32 AM Kai-Heng Feng
> > <kai.heng.feng@canonical.com> wrote:
> >>
> >> Suspend with s2idle or by the following steps cause screen frozen:
> >> # echo devices > /sys/power/pm_test
> >> # echo freeze > /sys/power/mem
> >>
> >> [ 289.625461] [drm:uvd_v1_0_ib_test [radeon]] *ERROR* radeon: fence wait timed out.
> >> [ 289.625494] [drm:radeon_ib_ring_tests [radeon]] *ERROR* radeon: failed testing IB on ring 5 (-110).
> >>
> >> The issue doesn't happen on traditional S3, probably because firmware or
> >> hardware provides extra power management.
> >>
> >> Inspired by Daniel Drake's patch [1] on amdgpu, using a similar approach
> >> can fix the issue.
> >
> > It doesn't actually fix the issue. The device is never powered down
> > so you are using more power than you would if you did not suspend in
> > the first place. The reset just works around the fact that the device
> > is never powered down.
>
> So how do we properly suspend/resume the device without help from platform firmware?
I guess you don't?
Alex
>
> Kai-Heng
>
> >
> > Alex
> >
> >>
> >> [1] https://patchwork.freedesktop.org/patch/335839/
> >>
> >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> >> ---
> >> drivers/gpu/drm/radeon/radeon_device.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> >> index 266e3cbbd09b..df823b9ad79f 100644
> >> --- a/drivers/gpu/drm/radeon/radeon_device.c
> >> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> >> @@ -33,6 +33,7 @@
> >> #include <linux/slab.h>
> >> #include <linux/vga_switcheroo.h>
> >> #include <linux/vgaarb.h>
> >> +#include <linux/suspend.h>
> >>
> >> #include <drm/drm_cache.h>
> >> #include <drm/drm_crtc_helper.h>
> >> @@ -1643,6 +1644,8 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
> >> rdev->asic->asic_reset(rdev, true);
> >> pci_restore_state(dev->pdev);
> >> } else if (suspend) {
> >> + if (pm_suspend_no_platform())
> >> + rdev->asic->asic_reset(rdev, true);
> >> /* Shut down the device */
> >> pci_disable_device(dev->pdev);
> >> pci_set_power_state(dev->pdev, PCI_D3hot);
> >> --
> >> 2.17.1
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware
2020-09-01 16:30 ` Alex Deucher
@ 2020-09-02 0:39 ` Kai-Heng Feng
0 siblings, 0 replies; 5+ messages in thread
From: Kai-Heng Feng @ 2020-09-02 0:39 UTC (permalink / raw)
To: Alex Deucher
Cc: Deucher, Alexander, Christian Koenig, David Airlie, open list,
open list:DRM DRIVERS, open list:RADEON and AMDGPU DRM DRIVERS
> On Sep 2, 2020, at 00:30, Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Tue, Sep 1, 2020 at 12:21 PM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
>>
>>
>>
>>> On Sep 1, 2020, at 22:19, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>
>>> On Tue, Sep 1, 2020 at 3:32 AM Kai-Heng Feng
>>> <kai.heng.feng@canonical.com> wrote:
>>>>
>>>> Suspend with s2idle or by the following steps cause screen frozen:
>>>> # echo devices > /sys/power/pm_test
>>>> # echo freeze > /sys/power/mem
>>>>
>>>> [ 289.625461] [drm:uvd_v1_0_ib_test [radeon]] *ERROR* radeon: fence wait timed out.
>>>> [ 289.625494] [drm:radeon_ib_ring_tests [radeon]] *ERROR* radeon: failed testing IB on ring 5 (-110).
>>>>
>>>> The issue doesn't happen on traditional S3, probably because firmware or
>>>> hardware provides extra power management.
>>>>
>>>> Inspired by Daniel Drake's patch [1] on amdgpu, using a similar approach
>>>> can fix the issue.
>>>
>>> It doesn't actually fix the issue. The device is never powered down
>>> so you are using more power than you would if you did not suspend in
>>> the first place. The reset just works around the fact that the device
>>> is never powered down.
>>
>> So how do we properly suspend/resume the device without help from platform firmware?
>
> I guess you don't?
Unfortunate but I guess we need to accept reality and use the default suspend method.
Kai-Heng
>
> Alex
>
>
>>
>> Kai-Heng
>>
>>>
>>> Alex
>>>
>>>>
>>>> [1] https://patchwork.freedesktop.org/patch/335839/
>>>>
>>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>>> ---
>>>> drivers/gpu/drm/radeon/radeon_device.c | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
>>>> index 266e3cbbd09b..df823b9ad79f 100644
>>>> --- a/drivers/gpu/drm/radeon/radeon_device.c
>>>> +++ b/drivers/gpu/drm/radeon/radeon_device.c
>>>> @@ -33,6 +33,7 @@
>>>> #include <linux/slab.h>
>>>> #include <linux/vga_switcheroo.h>
>>>> #include <linux/vgaarb.h>
>>>> +#include <linux/suspend.h>
>>>>
>>>> #include <drm/drm_cache.h>
>>>> #include <drm/drm_crtc_helper.h>
>>>> @@ -1643,6 +1644,8 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
>>>> rdev->asic->asic_reset(rdev, true);
>>>> pci_restore_state(dev->pdev);
>>>> } else if (suspend) {
>>>> + if (pm_suspend_no_platform())
>>>> + rdev->asic->asic_reset(rdev, true);
>>>> /* Shut down the device */
>>>> pci_disable_device(dev->pdev);
>>>> pci_set_power_state(dev->pdev, PCI_D3hot);
>>>> --
>>>> 2.17.1
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-02 0:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-01 6:32 [PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware Kai-Heng Feng
2020-09-01 14:19 ` Alex Deucher
2020-09-01 16:20 ` Kai-Heng Feng
2020-09-01 16:30 ` Alex Deucher
2020-09-02 0:39 ` Kai-Heng Feng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).