Re: [PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware
> On Sep 2, 2020, at 00:30, Alex Deucher wrote:
>
> On Tue, Sep 1, 2020 at 12:21 PM Kai-Heng Feng
> wrote:
>>
>>
>>
>>> On Sep 1, 2020, at 22:19, Alex Deucher wrote:
>>>
>>> On Tue, Sep 1, 2020 at 3:32 AM Kai-Heng Feng
>>> 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
---
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
#include
#include
+#include
#include
#include
@@ -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-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
Re: [PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware
On Tue, Sep 1, 2020 at 12:21 PM Kai-Heng Feng
wrote:
>
>
>
> > On Sep 1, 2020, at 22:19, Alex Deucher wrote:
> >
> > On Tue, Sep 1, 2020 at 3:32 AM Kai-Heng Feng
> > 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
> >> ---
> >> 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
> >> #include
> >> #include
> >> +#include
> >>
> >> #include
> >> #include
> >> @@ -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-de...@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Re: [PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware
> On Sep 1, 2020, at 22:19, Alex Deucher wrote:
>
> On Tue, Sep 1, 2020 at 3:32 AM Kai-Heng Feng
> 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
>> ---
>> 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
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> @@ -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-de...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware
On Tue, Sep 1, 2020 at 3:32 AM Kai-Heng Feng
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
> ---
> 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
> #include
> #include
> +#include
>
> #include
> #include
> @@ -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-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware
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
---
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
#include
#include
+#include
#include
#include
@@ -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
