All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/amdgpu: add a dev_pm_ops prepare callback (v2)
@ 2021-02-02 17:17 Alex Deucher
  2021-02-02 17:17 ` [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags Alex Deucher
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Deucher @ 2021-02-02 17:17 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher

as per:
https://www.kernel.org/doc/html/latest/driver-api/pm/devices.html

The prepare callback is required to support the DPM_FLAG_SMART_SUSPEND
driver flag.  This allows runtime pm to auto complete when the
system goes into suspend avoiding a wake up on suspend and on resume.
Apply this for hybrid gfx and BOCO systems where d3cold is
provided by the ACPI platform.

v2: check if device is runtime suspended in prepare.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 79608949e8e7..05ed82e4d805 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -36,6 +36,7 @@
 #include <linux/vga_switcheroo.h>
 #include <drm/drm_probe_helper.h>
 #include <linux/mmu_notifier.h>
+#include <linux/suspend.h>
 
 #include "amdgpu.h"
 #include "amdgpu_irq.h"
@@ -1271,6 +1272,27 @@ amdgpu_pci_shutdown(struct pci_dev *pdev)
 	adev->mp1_state = PP_MP1_STATE_NONE;
 }
 
+static int amdgpu_pmops_prepare(struct device *dev)
+{
+	struct drm_device *drm_dev = dev_get_drvdata(dev);
+
+	/* Return a positive number here so
+	 * DPM_FLAG_SMART_SUSPEND works properly
+	 */
+	if ((amdgpu_device_supports_atpx(drm_dev) &&
+	    amdgpu_is_atpx_hybrid()) ||
+	    amdgpu_device_supports_boco(drm_dev))
+		return pm_runtime_suspended(dev) &&
+			pm_suspend_via_firmware();
+
+	return 0;
+}
+
+static void amdgpu_pmops_complete(struct device *dev)
+{
+	/* nothing to do */
+}
+
 static int amdgpu_pmops_suspend(struct device *dev)
 {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
@@ -1484,6 +1506,8 @@ long amdgpu_drm_ioctl(struct file *filp,
 }
 
 static const struct dev_pm_ops amdgpu_pm_ops = {
+	.prepare = amdgpu_pmops_prepare,
+	.complete = amdgpu_pmops_complete,
 	.suspend = amdgpu_pmops_suspend,
 	.resume = amdgpu_pmops_resume,
 	.freeze = amdgpu_pmops_freeze,
-- 
2.29.2

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags
  2021-02-02 17:17 [PATCH 1/2] drm/amdgpu: add a dev_pm_ops prepare callback (v2) Alex Deucher
@ 2021-02-02 17:17 ` Alex Deucher
  2021-02-03  4:44   ` Quan, Evan
                     ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Alex Deucher @ 2021-02-02 17:17 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher

Once the device has runtime suspended, we don't need to power it
back up again for system suspend.  Likewise for resume, we don't
to power up the device again on resume only to power it back off
again via runtime pm because it's still idle.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index b4780182f990..b78847fa769b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -206,6 +206,12 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags)
 		if (amdgpu_device_supports_atpx(dev) &&
 		    !amdgpu_is_atpx_hybrid())
 			dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
+		/* we want direct complete for BOCO */
+		if ((amdgpu_device_supports_atpx(dev) &&
+		    amdgpu_is_atpx_hybrid()) ||
+		    amdgpu_device_supports_boco(dev))
+			dev_pm_set_driver_flags(dev->dev, DPM_FLAG_SMART_SUSPEND |
+						DPM_FLAG_MAY_SKIP_RESUME);
 		pm_runtime_use_autosuspend(dev->dev);
 		pm_runtime_set_autosuspend_delay(dev->dev, 5000);
 		pm_runtime_allow(dev->dev);
-- 
2.29.2

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags
  2021-02-02 17:17 ` [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags Alex Deucher
@ 2021-02-03  4:44   ` Quan, Evan
  2021-02-03  7:56   ` Lazar, Lijo
  2021-02-04 10:20   ` Michal Rostecki
  2 siblings, 0 replies; 8+ messages in thread
From: Quan, Evan @ 2021-02-03  4:44 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx; +Cc: Deucher, Alexander

[AMD Official Use Only - Internal Distribution Only]

Series is acked-by: Evan Quan <evan.quan@amd.com>

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
Sent: Wednesday, February 3, 2021 1:18 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags

Once the device has runtime suspended, we don't need to power it
back up again for system suspend.  Likewise for resume, we don't
to power up the device again on resume only to power it back off
again via runtime pm because it's still idle.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index b4780182f990..b78847fa769b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -206,6 +206,12 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags)
 if (amdgpu_device_supports_atpx(dev) &&
     !amdgpu_is_atpx_hybrid())
 dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
+/* we want direct complete for BOCO */
+if ((amdgpu_device_supports_atpx(dev) &&
+    amdgpu_is_atpx_hybrid()) ||
+    amdgpu_device_supports_boco(dev))
+dev_pm_set_driver_flags(dev->dev, DPM_FLAG_SMART_SUSPEND |
+DPM_FLAG_MAY_SKIP_RESUME);
 pm_runtime_use_autosuspend(dev->dev);
 pm_runtime_set_autosuspend_delay(dev->dev, 5000);
 pm_runtime_allow(dev->dev);
--
2.29.2

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cevan.quan%40amd.com%7C8a6f7c5bbab84c6e71c408d8c79e7edd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637478830857036614%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=O8I5HOE3ugNlFpf8f5XI1665h2IQ4VRCj%2FO53yDXGpQ%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags
  2021-02-02 17:17 ` [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags Alex Deucher
  2021-02-03  4:44   ` Quan, Evan
@ 2021-02-03  7:56   ` Lazar, Lijo
  2021-02-04 14:37     ` Alex Deucher
  2021-02-04 10:20   ` Michal Rostecki
  2 siblings, 1 reply; 8+ messages in thread
From: Lazar, Lijo @ 2021-02-03  7:56 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx; +Cc: Deucher, Alexander

[AMD Public Use]


-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
Sent: Tuesday, February 2, 2021 10:48 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags

Once the device has runtime suspended, we don't need to power it back up again for system suspend.  Likewise for resume, we don't to power up the device again on resume only to power it back off again via runtime pm because it's still idle.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index b4780182f990..b78847fa769b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -206,6 +206,12 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags)
 		if (amdgpu_device_supports_atpx(dev) &&
 		    !amdgpu_is_atpx_hybrid())
 			dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
+		/* we want direct complete for BOCO */
+		if ((amdgpu_device_supports_atpx(dev) &&
+		    amdgpu_is_atpx_hybrid()) ||
+		    amdgpu_device_supports_boco(dev))
+			dev_pm_set_driver_flags(dev->dev, DPM_FLAG_SMART_SUSPEND |
+						DPM_FLAG_MAY_SKIP_RESUME);

Device runtime suspend callback does -
	amdgpu_device_suspend(drm_dev, false)

System suspend callback does - 
	amdgpu_device_suspend(drm_dev, true)

One of the effects of this flag is for KFD to decide whether to evict all processes. It is done during system suspend but not during runtime device suspend. Will that have an impact if  the system suspend routine is skipped in this way?

Thanks,
Lijo

 		pm_runtime_use_autosuspend(dev->dev);
 		pm_runtime_set_autosuspend_delay(dev->dev, 5000);
 		pm_runtime_allow(dev->dev);
--
2.29.2

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Clijo.lazar%40amd.com%7C8a6f7c5bbab84c6e71c408d8c79e7edd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637478830856688013%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=55Rfj9pO3cnGBiB1DkJ9yuyMMrKOvxhgYDajhIeUNkI%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags
  2021-02-02 17:17 ` [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags Alex Deucher
  2021-02-03  4:44   ` Quan, Evan
  2021-02-03  7:56   ` Lazar, Lijo
@ 2021-02-04 10:20   ` Michal Rostecki
  2 siblings, 0 replies; 8+ messages in thread
From: Michal Rostecki @ 2021-02-04 10:20 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Alex Deucher, amd-gfx

On Tue, Feb 02, 2021 at 12:17:48PM -0500, Alex Deucher wrote:
> Once the device has runtime suspended, we don't need to power it
> back up again for system suspend.  Likewise for resume, we don't
> to power up the device again on resume only to power it back off
> again via runtime pm because it's still idle.
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

I tested those patches on Dell G5 SE 5505 (RX 5600M / Renoir / 4800H),
which currently suffers from the dc_link_set_backlight_level issue[0].
I have an interesting observation. They don't fix the issue completely,
but after applying them, I'm able to susepnd the laptop once, without
loosing the brightness. Though after suspending the second time, I'm
loosing the brightness and the same usual
amdgpu_dm_backlight_update_status call trace pops out in dmesg.

Everything works properly (with and without patches) on my PC (RX 6900
XT / 5950X).

Since those patches improve the situation a bit on 5600M and work
without issues on 6900 XT, and the backlight race condition needs to be
fully fixes separately, feel free to add:

Tested-by: Michal Rostecki <mrostecki@suse.com>

[0] https://gitlab.freedesktop.org/drm/amd/-/issues/1337
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags
  2021-02-03  7:56   ` Lazar, Lijo
@ 2021-02-04 14:37     ` Alex Deucher
  2021-02-04 15:14       ` Felix Kuehling
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Deucher @ 2021-02-04 14:37 UTC (permalink / raw)
  To: Lazar, Lijo, Rajneesh Bhardwaj; +Cc: Deucher, Alexander, amd-gfx

On Wed, Feb 3, 2021 at 2:56 AM Lazar, Lijo <Lijo.Lazar@amd.com> wrote:
>
> [AMD Public Use]
>
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
> Sent: Tuesday, February 2, 2021 10:48 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags
>
> Once the device has runtime suspended, we don't need to power it back up again for system suspend.  Likewise for resume, we don't to power up the device again on resume only to power it back off again via runtime pm because it's still idle.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index b4780182f990..b78847fa769b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -206,6 +206,12 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags)
>                 if (amdgpu_device_supports_atpx(dev) &&
>                     !amdgpu_is_atpx_hybrid())
>                         dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
> +               /* we want direct complete for BOCO */
> +               if ((amdgpu_device_supports_atpx(dev) &&
> +                   amdgpu_is_atpx_hybrid()) ||
> +                   amdgpu_device_supports_boco(dev))
> +                       dev_pm_set_driver_flags(dev->dev, DPM_FLAG_SMART_SUSPEND |
> +                                               DPM_FLAG_MAY_SKIP_RESUME);
>
> Device runtime suspend callback does -
>         amdgpu_device_suspend(drm_dev, false)
>
> System suspend callback does -
>         amdgpu_device_suspend(drm_dev, true)
>
> One of the effects of this flag is for KFD to decide whether to evict all processes. It is done during system suspend but not during runtime device suspend. Will that have an impact if  the system suspend routine is skipped in this way?

+ Rajneesh

Can you comment on this?  Idea of this patch is to not wake the device
for system suspend and resume if it's already in runtime suspend.

Alex

>
> Thanks,
> Lijo
>
>                 pm_runtime_use_autosuspend(dev->dev);
>                 pm_runtime_set_autosuspend_delay(dev->dev, 5000);
>                 pm_runtime_allow(dev->dev);
> --
> 2.29.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Clijo.lazar%40amd.com%7C8a6f7c5bbab84c6e71c408d8c79e7edd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637478830856688013%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=55Rfj9pO3cnGBiB1DkJ9yuyMMrKOvxhgYDajhIeUNkI%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags
  2021-02-04 14:37     ` Alex Deucher
@ 2021-02-04 15:14       ` Felix Kuehling
  2021-02-05  5:59         ` Bhardwaj, Rajneesh
  0 siblings, 1 reply; 8+ messages in thread
From: Felix Kuehling @ 2021-02-04 15:14 UTC (permalink / raw)
  To: Alex Deucher, Lazar, Lijo, Rajneesh Bhardwaj; +Cc: Deucher, Alexander, amd-gfx


Am 2021-02-04 um 9:37 a.m. schrieb Alex Deucher:
> On Wed, Feb 3, 2021 at 2:56 AM Lazar, Lijo <Lijo.Lazar@amd.com> wrote:
>> [AMD Public Use]
>>
>>
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
>> Sent: Tuesday, February 2, 2021 10:48 PM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
>> Subject: [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags
>>
>> Once the device has runtime suspended, we don't need to power it back up again for system suspend.  Likewise for resume, we don't to power up the device again on resume only to power it back off again via runtime pm because it's still idle.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> index b4780182f990..b78847fa769b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> @@ -206,6 +206,12 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags)
>>                 if (amdgpu_device_supports_atpx(dev) &&
>>                     !amdgpu_is_atpx_hybrid())
>>                         dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
>> +               /* we want direct complete for BOCO */
>> +               if ((amdgpu_device_supports_atpx(dev) &&
>> +                   amdgpu_is_atpx_hybrid()) ||
>> +                   amdgpu_device_supports_boco(dev))
>> +                       dev_pm_set_driver_flags(dev->dev, DPM_FLAG_SMART_SUSPEND |
>> +                                               DPM_FLAG_MAY_SKIP_RESUME);
>>
>> Device runtime suspend callback does -
>>         amdgpu_device_suspend(drm_dev, false)
>>
>> System suspend callback does -
>>         amdgpu_device_suspend(drm_dev, true)
>>
>> One of the effects of this flag is for KFD to decide whether to evict all processes. It is done during system suspend but not during runtime device suspend. Will that have an impact if  the system suspend routine is skipped in this way?
> + Rajneesh
>
> Can you comment on this?  Idea of this patch is to not wake the device
> for system suspend and resume if it's already in runtime suspend.
KFD only allows runtime suspend when there are no processes using the
GPU. Therefore it should be safe (in theory) to skip process eviction if
you're already in runtime suspend. Just make sure all the suspend/resume
calls into KFD are paired up correctly. If you skip suspend but then
later call resume anyway, it will likely cause problems.

For testing this, I'd suggest running some KFD application (e.g. kfdtest
or an OpenCL app with ROCm-based OpenCL) before suspend, then suspend,
then run the app again after resume to make sure KFD is still good.

Regards,
  Felix


>
> Alex
>
>> Thanks,
>> Lijo
>>
>>                 pm_runtime_use_autosuspend(dev->dev);
>>                 pm_runtime_set_autosuspend_delay(dev->dev, 5000);
>>                 pm_runtime_allow(dev->dev);
>> --
>> 2.29.2
>>
>> _______________________________________________
>> 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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags
  2021-02-04 15:14       ` Felix Kuehling
@ 2021-02-05  5:59         ` Bhardwaj, Rajneesh
  0 siblings, 0 replies; 8+ messages in thread
From: Bhardwaj, Rajneesh @ 2021-02-05  5:59 UTC (permalink / raw)
  To: Felix Kuehling, Alex Deucher, Lazar, Lijo; +Cc: Deucher, Alexander, amd-gfx


[-- Attachment #1.1: Type: text/plain, Size: 4477 bytes --]


On 2/4/2021 10:14 AM, Felix Kuehling wrote:
> Am 2021-02-04 um 9:37 a.m. schrieb Alex Deucher:
>> On Wed, Feb 3, 2021 at 2:56 AM Lazar, Lijo <Lijo.Lazar@amd.com> wrote:
>>> [AMD Public Use]
>>>
>>>
>>> -----Original Message-----
>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
>>> Sent: Tuesday, February 2, 2021 10:48 PM
>>> To: amd-gfx@lists.freedesktop.org
>>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
>>> Subject: [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags
>>>
>>> Once the device has runtime suspended, we don't need to power it back up again for system suspend.  Likewise for resume, we don't to power up the device again on resume only to power it back off again via runtime pm because it's still idle.
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> index b4780182f990..b78847fa769b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> @@ -206,6 +206,12 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags)
>>>                  if (amdgpu_device_supports_atpx(dev) &&
>>>                      !amdgpu_is_atpx_hybrid())
>>>                          dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
>>> +               /* we want direct complete for BOCO */
>>> +               if ((amdgpu_device_supports_atpx(dev) &&
>>> +                   amdgpu_is_atpx_hybrid()) ||
>>> +                   amdgpu_device_supports_boco(dev))
>>> +                       dev_pm_set_driver_flags(dev->dev, DPM_FLAG_SMART_SUSPEND |
>>> +                                               DPM_FLAG_MAY_SKIP_RESUME);
>>>
>>> Device runtime suspend callback does -
>>>          amdgpu_device_suspend(drm_dev, false)
>>>
>>> System suspend callback does -
>>>          amdgpu_device_suspend(drm_dev, true)
>>>
>>> One of the effects of this flag is for KFD to decide whether to evict all processes. It is done during system suspend but not during runtime device suspend. Will that have an impact if  the system suspend routine is skipped in this way?
>> + Rajneesh
>>
>> Can you comment on this?  Idea of this patch is to not wake the device
>> for system suspend and resume if it's already in runtime suspend.

KFD doesn't allow amdgpu driver to runtime suspend the device as long as 
the per process device data is valid. This patch only enables direct 
complete path for already runtime suspended amdgpu device so its 
implicit that kfd had no active process. While we may be OK to not 
explicitly cancel scheduled work or suspend all kfd processes (since gpu 
is already runtime suspended) we still leave KFD unlocked for a system 
wide suspend via direct complete path.  I think we should still be ok 
with this since locking is only used for making sure that kfd is not 
open during a gpu reset.

Also i suggest using DPM_FLAG_SMART_PREPARE as well, since we are 
relying on prepare for skipping late/noirq phase during system suspend.


Acked-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@amd.com>


> KFD only allows runtime suspend when there are no processes using the
> GPU. Therefore it should be safe (in theory) to skip process eviction if
> you're already in runtime suspend. Just make sure all the suspend/resume
> calls into KFD are paired up correctly. If you skip suspend but then
> later call resume anyway, it will likely cause problems.
>
> For testing this, I'd suggest running some KFD application (e.g. kfdtest
> or an OpenCL app with ROCm-based OpenCL) before suspend, then suspend,
> then run the app again after resume to make sure KFD is still good.
>
> Regards,
>    Felix
>
>
>> Alex
>>
>>> Thanks,
>>> Lijo
>>>
>>>                  pm_runtime_use_autosuspend(dev->dev);
>>>                  pm_runtime_set_autosuspend_delay(dev->dev, 5000);
>>>                  pm_runtime_allow(dev->dev);
>>> --
>>> 2.29.2
>>>
>>> _______________________________________________
>>> 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

[-- Attachment #1.2: Type: text/html, Size: 8552 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-02-05  5:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02 17:17 [PATCH 1/2] drm/amdgpu: add a dev_pm_ops prepare callback (v2) Alex Deucher
2021-02-02 17:17 ` [PATCH 2/2] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags Alex Deucher
2021-02-03  4:44   ` Quan, Evan
2021-02-03  7:56   ` Lazar, Lijo
2021-02-04 14:37     ` Alex Deucher
2021-02-04 15:14       ` Felix Kuehling
2021-02-05  5:59         ` Bhardwaj, Rajneesh
2021-02-04 10:20   ` Michal Rostecki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.