[PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2)

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2)
@ 2021-03-09  4:10 Alex Deucher
  2021-03-09  4:10 ` [PATCH 2/7] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags (v2) Alex Deucher
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Alex Deucher @ 2021-03-09  4:10 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 aecf7baf219a..8d4fbee01011 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"
@@ -1293,6 +1294,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);
@@ -1511,6 +1533,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] 16+ messages in thread

* [PATCH 2/7] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags (v2)
  2021-03-09  4:10 [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2) Alex Deucher
@ 2021-03-09  4:10 ` Alex Deucher
  2021-03-09  4:10 ` [PATCH 3/7] drm/amdgpu: disentangle HG systems from vgaswitcheroo Alex Deucher
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2021-03-09  4:10 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Rajneesh Bhardwaj

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.

v2: add DPM_FLAG_SMART_PREPARE as well

Acked-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@amd.com> (v1)
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index f269267a96d3..8e6ef4d8b7ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -205,6 +205,13 @@ 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_PREPARE |
+						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] 16+ messages in thread

* [PATCH 3/7] drm/amdgpu: disentangle HG systems from vgaswitcheroo
  2021-03-09  4:10 [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2) Alex Deucher
  2021-03-09  4:10 ` [PATCH 2/7] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags (v2) Alex Deucher
@ 2021-03-09  4:10 ` Alex Deucher
  2021-03-10  8:42   ` Quan, Evan
  2021-03-09  4:10 ` [PATCH 4/7] drm/amdgpu: track what pmops flow we are in Alex Deucher
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Alex Deucher @ 2021-03-09  4:10 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher

There's no need to keep vgaswitcheroo around for HG
systems.  They don't use muxes and their power control
is handled via ACPI.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 38 +++++++++-------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 34 ++++++++-----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  9 ++---
 4 files changed, 34 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index b5310b35721c..d47626ce9bc5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1260,7 +1260,7 @@ void amdgpu_device_program_register_sequence(struct amdgpu_device *adev,
 					     const u32 *registers,
 					     const u32 array_size);
 
-bool amdgpu_device_supports_atpx(struct drm_device *dev);
+bool amdgpu_device_supports_px(struct drm_device *dev);
 bool amdgpu_device_supports_boco(struct drm_device *dev);
 bool amdgpu_device_supports_baco(struct drm_device *dev);
 bool amdgpu_device_is_peer_accessible(struct amdgpu_device *adev,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 6eb3b4d2c9b2..ac5f7837285b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -211,18 +211,18 @@ static DEVICE_ATTR(serial_number, S_IRUGO,
 		amdgpu_device_get_serial_number, NULL);
 
 /**
- * amdgpu_device_supports_atpx - Is the device a dGPU with HG/PX power control
+ * amdgpu_device_supports_px - Is the device a dGPU with ATPX power control
  *
  * @dev: drm_device pointer
  *
- * Returns true if the device is a dGPU with HG/PX power control,
+ * Returns true if the device is a dGPU with ATPX power control,
  * otherwise return false.
  */
-bool amdgpu_device_supports_atpx(struct drm_device *dev)
+bool amdgpu_device_supports_px(struct drm_device *dev)
 {
 	struct amdgpu_device *adev = drm_to_adev(dev);
 
-	if (adev->flags & AMD_IS_PX)
+	if ((adev->flags & AMD_IS_PX) && !amdgpu_is_atpx_hybrid())
 		return true;
 	return false;
 }
@@ -232,14 +232,15 @@ bool amdgpu_device_supports_atpx(struct drm_device *dev)
  *
  * @dev: drm_device pointer
  *
- * Returns true if the device is a dGPU with HG/PX power control,
+ * Returns true if the device is a dGPU with ACPI power control,
  * otherwise return false.
  */
 bool amdgpu_device_supports_boco(struct drm_device *dev)
 {
 	struct amdgpu_device *adev = drm_to_adev(dev);
 
-	if (adev->has_pr3)
+	if (adev->has_pr3 ||
+	    ((adev->flags & AMD_IS_PX) && amdgpu_is_atpx_hybrid()))
 		return true;
 	return false;
 }
@@ -1429,7 +1430,7 @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev,
 	struct drm_device *dev = pci_get_drvdata(pdev);
 	int r;
 
-	if (amdgpu_device_supports_atpx(dev) && state == VGA_SWITCHEROO_OFF)
+	if (amdgpu_device_supports_px(dev) && state == VGA_SWITCHEROO_OFF)
 		return;
 
 	if (state == VGA_SWITCHEROO_ON) {
@@ -3213,7 +3214,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	struct drm_device *ddev = adev_to_drm(adev);
 	struct pci_dev *pdev = adev->pdev;
 	int r, i;
-	bool atpx = false;
+	bool px = false;
 	u32 max_MBps;
 
 	adev->shutdown = false;
@@ -3385,16 +3386,12 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
 		vga_client_register(adev->pdev, adev, NULL, amdgpu_device_vga_set_decode);
 
-	if (amdgpu_device_supports_atpx(ddev))
-		atpx = true;
-	if (amdgpu_has_atpx() &&
-	    (amdgpu_is_atpx_hybrid() ||
-	     amdgpu_has_atpx_dgpu_power_cntl()) &&
-	    !pci_is_thunderbolt_attached(adev->pdev))
+	if (amdgpu_device_supports_px(ddev)) {
+		px = true;
 		vga_switcheroo_register_client(adev->pdev,
-					       &amdgpu_switcheroo_ops, atpx);
-	if (atpx)
+					       &amdgpu_switcheroo_ops, px);
 		vga_switcheroo_init_domain_pm_ops(adev->dev, &adev->vga_pm_domain);
+	}
 
 	if (amdgpu_emu_mode == 1) {
 		/* post the asic on emulation mode */
@@ -3576,7 +3573,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 
 failed:
 	amdgpu_vf_error_trans_all(adev);
-	if (atpx)
+	if (px)
 		vga_switcheroo_fini_domain_pm_ops(adev->dev);
 
 failed_unmap:
@@ -3636,13 +3633,10 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
 
 	kfree(adev->bios);
 	adev->bios = NULL;
-	if (amdgpu_has_atpx() &&
-	    (amdgpu_is_atpx_hybrid() ||
-	     amdgpu_has_atpx_dgpu_power_cntl()) &&
-	    !pci_is_thunderbolt_attached(adev->pdev))
+	if (amdgpu_device_supports_px(adev_to_drm(adev))) {
 		vga_switcheroo_unregister_client(adev->pdev);
-	if (amdgpu_device_supports_atpx(adev_to_drm(adev)))
 		vga_switcheroo_fini_domain_pm_ops(adev->dev);
+	}
 	if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
 		vga_client_register(adev->pdev, NULL, NULL, NULL);
 	if (adev->rio_mem)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 8d4fbee01011..3e6bb7d79652 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1301,9 +1301,7 @@ static int amdgpu_pmops_prepare(struct device *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))
+	if (amdgpu_device_supports_boco(drm_dev))
 		return pm_runtime_suspended(dev) &&
 			pm_suspend_via_firmware();
 
@@ -1392,7 +1390,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 	}
 
 	adev->in_runpm = true;
-	if (amdgpu_device_supports_atpx(drm_dev))
+	if (amdgpu_device_supports_px(drm_dev))
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
 	ret = amdgpu_device_suspend(drm_dev, false);
@@ -1401,16 +1399,14 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 		return ret;
 	}
 
-	if (amdgpu_device_supports_atpx(drm_dev)) {
+	if (amdgpu_device_supports_px(drm_dev)) {
 		/* Only need to handle PCI state in the driver for ATPX
 		 * PCI core handles it for _PR3.
 		 */
-		if (!amdgpu_is_atpx_hybrid()) {
-			amdgpu_device_cache_pci_state(pdev);
-			pci_disable_device(pdev);
-			pci_ignore_hotplug(pdev);
-			pci_set_power_state(pdev, PCI_D3cold);
-		}
+		amdgpu_device_cache_pci_state(pdev);
+		pci_disable_device(pdev);
+		pci_ignore_hotplug(pdev);
+		pci_set_power_state(pdev, PCI_D3cold);
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;
 	} else if (amdgpu_device_supports_baco(drm_dev)) {
 		amdgpu_device_baco_enter(drm_dev);
@@ -1429,19 +1425,17 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 	if (!adev->runpm)
 		return -EINVAL;
 
-	if (amdgpu_device_supports_atpx(drm_dev)) {
+	if (amdgpu_device_supports_px(drm_dev)) {
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
 		/* Only need to handle PCI state in the driver for ATPX
 		 * PCI core handles it for _PR3.
 		 */
-		if (!amdgpu_is_atpx_hybrid()) {
-			pci_set_power_state(pdev, PCI_D0);
-			amdgpu_device_load_pci_state(pdev);
-			ret = pci_enable_device(pdev);
-			if (ret)
-				return ret;
-		}
+		pci_set_power_state(pdev, PCI_D0);
+		amdgpu_device_load_pci_state(pdev);
+		ret = pci_enable_device(pdev);
+		if (ret)
+			return ret;
 		pci_set_master(pdev);
 	} else if (amdgpu_device_supports_boco(drm_dev)) {
 		/* Only need to handle PCI state in the driver for ATPX
@@ -1452,7 +1446,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 		amdgpu_device_baco_exit(drm_dev);
 	}
 	ret = amdgpu_device_resume(drm_dev, false);
-	if (amdgpu_device_supports_atpx(drm_dev))
+	if (amdgpu_device_supports_px(drm_dev))
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
 	adev->in_runpm = false;
 	return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 8e6ef4d8b7ee..86eeeb4f3513 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -161,7 +161,7 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags)
 		goto out;
 	}
 
-	if (amdgpu_device_supports_atpx(dev) &&
+	if (amdgpu_device_supports_px(dev) &&
 	    (amdgpu_runtime_pm != 0)) { /* enable runpm by default for atpx */
 		adev->runpm = true;
 		dev_info(adev->dev, "Using ATPX for runtime pm\n");
@@ -202,13 +202,10 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags)
 
 	if (adev->runpm) {
 		/* only need to skip on ATPX */
-		if (amdgpu_device_supports_atpx(dev) &&
-		    !amdgpu_is_atpx_hybrid())
+		if (amdgpu_device_supports_px(dev))
 			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))
+		if (amdgpu_device_supports_boco(dev))
 			dev_pm_set_driver_flags(dev->dev, DPM_FLAG_SMART_PREPARE |
 						DPM_FLAG_SMART_SUSPEND |
 						DPM_FLAG_MAY_SKIP_RESUME);
-- 
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] 16+ messages in thread

* RE: [PATCH 3/7] drm/amdgpu: disentangle HG systems from vgaswitcheroo
  2021-03-09  4:10 ` [PATCH 3/7] drm/amdgpu: disentangle HG systems from vgaswitcheroo Alex Deucher
@ 2021-03-10  8:42   ` Quan, Evan
  0 siblings, 0 replies; 16+ messages in thread
From: Quan, Evan @ 2021-03-10  8:42 UTC (permalink / raw)
  To: Deucher, Alexander, 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, March 9, 2021 12:10 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: [PATCH 3/7] drm/amdgpu: disentangle HG systems from vgaswitcheroo

There's no need to keep vgaswitcheroo around for HG systems.  They don't use muxes and their power control is handled via ACPI.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 38 +++++++++-------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 34 ++++++++-----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  9 ++---
 4 files changed, 34 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index b5310b35721c..d47626ce9bc5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1260,7 +1260,7 @@ void amdgpu_device_program_register_sequence(struct amdgpu_device *adev,
 					     const u32 *registers,
 					     const u32 array_size);
 
-bool amdgpu_device_supports_atpx(struct drm_device *dev);
+bool amdgpu_device_supports_px(struct drm_device *dev);
 bool amdgpu_device_supports_boco(struct drm_device *dev);  bool amdgpu_device_supports_baco(struct drm_device *dev);  bool amdgpu_device_is_peer_accessible(struct amdgpu_device *adev, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 6eb3b4d2c9b2..ac5f7837285b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -211,18 +211,18 @@ static DEVICE_ATTR(serial_number, S_IRUGO,
 		amdgpu_device_get_serial_number, NULL);
 
 /**
- * amdgpu_device_supports_atpx - Is the device a dGPU with HG/PX power control
+ * amdgpu_device_supports_px - Is the device a dGPU with ATPX power 
+ control
  *
  * @dev: drm_device pointer
  *
- * Returns true if the device is a dGPU with HG/PX power control,
+ * Returns true if the device is a dGPU with ATPX power control,
  * otherwise return false.
  */
-bool amdgpu_device_supports_atpx(struct drm_device *dev)
+bool amdgpu_device_supports_px(struct drm_device *dev)
 {
 	struct amdgpu_device *adev = drm_to_adev(dev);
 
-	if (adev->flags & AMD_IS_PX)
+	if ((adev->flags & AMD_IS_PX) && !amdgpu_is_atpx_hybrid())
 		return true;
 	return false;
 }
@@ -232,14 +232,15 @@ bool amdgpu_device_supports_atpx(struct drm_device *dev)
  *
  * @dev: drm_device pointer
  *
- * Returns true if the device is a dGPU with HG/PX power control,
+ * Returns true if the device is a dGPU with ACPI power control,
  * otherwise return false.
  */
 bool amdgpu_device_supports_boco(struct drm_device *dev)  {
 	struct amdgpu_device *adev = drm_to_adev(dev);
 
-	if (adev->has_pr3)
+	if (adev->has_pr3 ||
+	    ((adev->flags & AMD_IS_PX) && amdgpu_is_atpx_hybrid()))
 		return true;
 	return false;
 }
@@ -1429,7 +1430,7 @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev,
 	struct drm_device *dev = pci_get_drvdata(pdev);
 	int r;
 
-	if (amdgpu_device_supports_atpx(dev) && state == VGA_SWITCHEROO_OFF)
+	if (amdgpu_device_supports_px(dev) && state == VGA_SWITCHEROO_OFF)
 		return;
 
 	if (state == VGA_SWITCHEROO_ON) {
@@ -3213,7 +3214,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	struct drm_device *ddev = adev_to_drm(adev);
 	struct pci_dev *pdev = adev->pdev;
 	int r, i;
-	bool atpx = false;
+	bool px = false;
 	u32 max_MBps;
 
 	adev->shutdown = false;
@@ -3385,16 +3386,12 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
 		vga_client_register(adev->pdev, adev, NULL, amdgpu_device_vga_set_decode);
 
-	if (amdgpu_device_supports_atpx(ddev))
-		atpx = true;
-	if (amdgpu_has_atpx() &&
-	    (amdgpu_is_atpx_hybrid() ||
-	     amdgpu_has_atpx_dgpu_power_cntl()) &&
-	    !pci_is_thunderbolt_attached(adev->pdev))
+	if (amdgpu_device_supports_px(ddev)) {
[Quan, Evan] Maybe this should be " amdgpu_device_supports_px(ddev) && amdgpu_has_atpx_dgpu_power_cntl() && !pci_is_thunderbolt_attached(adev->pdev)".
+		px = true;
 		vga_switcheroo_register_client(adev->pdev,
-					       &amdgpu_switcheroo_ops, atpx);
-	if (atpx)
+					       &amdgpu_switcheroo_ops, px);
 		vga_switcheroo_init_domain_pm_ops(adev->dev, &adev->vga_pm_domain);
+	}
 
 	if (amdgpu_emu_mode == 1) {
 		/* post the asic on emulation mode */ @@ -3576,7 +3573,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 
 failed:
 	amdgpu_vf_error_trans_all(adev);
-	if (atpx)
+	if (px)
 		vga_switcheroo_fini_domain_pm_ops(adev->dev);
 
 failed_unmap:
@@ -3636,13 +3633,10 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
 
 	kfree(adev->bios);
 	adev->bios = NULL;
-	if (amdgpu_has_atpx() &&
-	    (amdgpu_is_atpx_hybrid() ||
-	     amdgpu_has_atpx_dgpu_power_cntl()) &&
-	    !pci_is_thunderbolt_attached(adev->pdev))
+	if (amdgpu_device_supports_px(adev_to_drm(adev))) {
[Quan, Evan] Same as above
 		vga_switcheroo_unregister_client(adev->pdev);
-	if (amdgpu_device_supports_atpx(adev_to_drm(adev)))
 		vga_switcheroo_fini_domain_pm_ops(adev->dev);
+	}
 	if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
 		vga_client_register(adev->pdev, NULL, NULL, NULL);
 	if (adev->rio_mem)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 8d4fbee01011..3e6bb7d79652 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1301,9 +1301,7 @@ static int amdgpu_pmops_prepare(struct device *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))
+	if (amdgpu_device_supports_boco(drm_dev))
 		return pm_runtime_suspended(dev) &&
 			pm_suspend_via_firmware();
 
@@ -1392,7 +1390,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 	}
 
 	adev->in_runpm = true;
-	if (amdgpu_device_supports_atpx(drm_dev))
+	if (amdgpu_device_supports_px(drm_dev))
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
 	ret = amdgpu_device_suspend(drm_dev, false); @@ -1401,16 +1399,14 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 		return ret;
 	}
 
-	if (amdgpu_device_supports_atpx(drm_dev)) {
+	if (amdgpu_device_supports_px(drm_dev)) {
 		/* Only need to handle PCI state in the driver for ATPX
 		 * PCI core handles it for _PR3.
 		 */
-		if (!amdgpu_is_atpx_hybrid()) {
-			amdgpu_device_cache_pci_state(pdev);
-			pci_disable_device(pdev);
-			pci_ignore_hotplug(pdev);
-			pci_set_power_state(pdev, PCI_D3cold);
-		}
+		amdgpu_device_cache_pci_state(pdev);
+		pci_disable_device(pdev);
+		pci_ignore_hotplug(pdev);
+		pci_set_power_state(pdev, PCI_D3cold);
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;
 	} else if (amdgpu_device_supports_baco(drm_dev)) {
 		amdgpu_device_baco_enter(drm_dev);
@@ -1429,19 +1425,17 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 	if (!adev->runpm)
 		return -EINVAL;
 
-	if (amdgpu_device_supports_atpx(drm_dev)) {
+	if (amdgpu_device_supports_px(drm_dev)) {
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
 		/* Only need to handle PCI state in the driver for ATPX
 		 * PCI core handles it for _PR3.
 		 */
-		if (!amdgpu_is_atpx_hybrid()) {
-			pci_set_power_state(pdev, PCI_D0);
-			amdgpu_device_load_pci_state(pdev);
-			ret = pci_enable_device(pdev);
-			if (ret)
-				return ret;
-		}
+		pci_set_power_state(pdev, PCI_D0);
+		amdgpu_device_load_pci_state(pdev);
+		ret = pci_enable_device(pdev);
+		if (ret)
+			return ret;
 		pci_set_master(pdev);
 	} else if (amdgpu_device_supports_boco(drm_dev)) {
 		/* Only need to handle PCI state in the driver for ATPX @@ -1452,7 +1446,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 		amdgpu_device_baco_exit(drm_dev);
 	}
 	ret = amdgpu_device_resume(drm_dev, false);
-	if (amdgpu_device_supports_atpx(drm_dev))
+	if (amdgpu_device_supports_px(drm_dev))
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
 	adev->in_runpm = false;
 	return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 8e6ef4d8b7ee..86eeeb4f3513 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -161,7 +161,7 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags)
 		goto out;
 	}
 
-	if (amdgpu_device_supports_atpx(dev) &&
+	if (amdgpu_device_supports_px(dev) &&
 	    (amdgpu_runtime_pm != 0)) { /* enable runpm by default for atpx */
 		adev->runpm = true;
 		dev_info(adev->dev, "Using ATPX for runtime pm\n"); @@ -202,13 +202,10 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags)
 
 	if (adev->runpm) {
 		/* only need to skip on ATPX */
-		if (amdgpu_device_supports_atpx(dev) &&
-		    !amdgpu_is_atpx_hybrid())
+		if (amdgpu_device_supports_px(dev))
 			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))
+		if (amdgpu_device_supports_boco(dev))
 			dev_pm_set_driver_flags(dev->dev, DPM_FLAG_SMART_PREPARE |
 						DPM_FLAG_SMART_SUSPEND |
 						DPM_FLAG_MAY_SKIP_RESUME);
--
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%7C51517a035c23422125bd08d8e2b14c1e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637508598407995789%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=zfLM9A%2FtPCjR8xAd%2Faz2ab8Kyk%2FclUnDMKNyoLFkAlU%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] 16+ messages in thread

* [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
  2021-03-09  4:10 [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2) Alex Deucher
  2021-03-09  4:10 ` [PATCH 2/7] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags (v2) Alex Deucher
  2021-03-09  4:10 ` [PATCH 3/7] drm/amdgpu: disentangle HG systems from vgaswitcheroo Alex Deucher
@ 2021-03-09  4:10 ` Alex Deucher
  2021-03-09  6:19   ` Lazar, Lijo
  2021-03-09  4:10 ` [PATCH 5/7] drm/amdgpu: don't evict vram on APUs for suspend to ram Alex Deucher
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Alex Deucher @ 2021-03-09  4:10 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher

We reuse the same suspend and resume functions for
all of the pmops states, so flag what state we are
in so that we can alter behavior deeper in the driver
depending on the current flow.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       | 20 +++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 58 +++++++++++++++++++----
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c |  3 +-
 3 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index d47626ce9bc5..4ddc5cc983c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -347,6 +347,24 @@ int amdgpu_device_ip_block_add(struct amdgpu_device *adev,
 bool amdgpu_get_bios(struct amdgpu_device *adev);
 bool amdgpu_read_bios(struct amdgpu_device *adev);
 
+/*
+ * PM Ops
+ */
+enum amdgpu_pmops_state {
+	AMDGPU_PMOPS_NONE = 0,
+	AMDGPU_PMOPS_PREPARE,
+	AMDGPU_PMOPS_COMPLETE,
+	AMDGPU_PMOPS_SUSPEND,
+	AMDGPU_PMOPS_RESUME,
+	AMDGPU_PMOPS_FREEZE,
+	AMDGPU_PMOPS_THAW,
+	AMDGPU_PMOPS_POWEROFF,
+	AMDGPU_PMOPS_RESTORE,
+	AMDGPU_PMOPS_RUNTIME_SUSPEND,
+	AMDGPU_PMOPS_RUNTIME_RESUME,
+	AMDGPU_PMOPS_RUNTIME_IDLE,
+};
+
 /*
  * Clocks
  */
@@ -1019,8 +1037,8 @@ struct amdgpu_device {
 	u8				reset_magic[AMDGPU_RESET_MAGIC_NUM];
 
 	/* s3/s4 mask */
+	enum amdgpu_pmops_state         pmops_state;
 	bool                            in_suspend;
-	bool				in_hibernate;
 
 	/*
 	 * The combination flag in_poweroff_reboot_com used to identify the poweroff
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 3e6bb7d79652..0312c52bd39d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1297,34 +1297,54 @@ amdgpu_pci_shutdown(struct pci_dev *pdev)
 static int amdgpu_pmops_prepare(struct device *dev)
 {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+	int r;
 
+	adev->pmops_state = AMDGPU_PMOPS_PREPARE;
 	/* Return a positive number here so
 	 * DPM_FLAG_SMART_SUSPEND works properly
 	 */
 	if (amdgpu_device_supports_boco(drm_dev))
-		return pm_runtime_suspended(dev) &&
+		r= pm_runtime_suspended(dev) &&
 			pm_suspend_via_firmware();
-
-	return 0;
+	else
+		r = 0;
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
+	return r;
 }
 
 static void amdgpu_pmops_complete(struct device *dev)
 {
+	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+
+	adev->pmops_state = AMDGPU_PMOPS_COMPLETE;
 	/* nothing to do */
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
 }
 
 static int amdgpu_pmops_suspend(struct device *dev)
 {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+	int r;
 
-	return amdgpu_device_suspend(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_SUSPEND;
+	r = amdgpu_device_suspend(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
+	return r;
 }
 
 static int amdgpu_pmops_resume(struct device *dev)
 {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+	int r;
 
-	return amdgpu_device_resume(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_RESUME;
+	r = amdgpu_device_resume(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
+	return r;
 }
 
 static int amdgpu_pmops_freeze(struct device *dev)
@@ -1333,9 +1353,9 @@ static int amdgpu_pmops_freeze(struct device *dev)
 	struct amdgpu_device *adev = drm_to_adev(drm_dev);
 	int r;
 
-	adev->in_hibernate = true;
+	adev->pmops_state = AMDGPU_PMOPS_FREEZE;
 	r = amdgpu_device_suspend(drm_dev, true);
-	adev->in_hibernate = false;
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
 	if (r)
 		return r;
 	return amdgpu_asic_reset(adev);
@@ -1344,8 +1364,13 @@ static int amdgpu_pmops_freeze(struct device *dev)
 static int amdgpu_pmops_thaw(struct device *dev)
 {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+	int r;
 
-	return amdgpu_device_resume(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_THAW;
+	r = amdgpu_device_resume(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
+	return r;
 }
 
 static int amdgpu_pmops_poweroff(struct device *dev)
@@ -1354,17 +1379,24 @@ static int amdgpu_pmops_poweroff(struct device *dev)
 	struct amdgpu_device *adev = drm_to_adev(drm_dev);
 	int r;
 
+	adev->pmops_state = AMDGPU_PMOPS_POWEROFF;
 	adev->in_poweroff_reboot_com = true;
 	r =  amdgpu_device_suspend(drm_dev, true);
 	adev->in_poweroff_reboot_com = false;
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
 	return r;
 }
 
 static int amdgpu_pmops_restore(struct device *dev)
 {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+	int r;
 
-	return amdgpu_device_resume(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_RESTORE;
+	r = amdgpu_device_resume(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
+	return r;
 }
 
 static int amdgpu_pmops_runtime_suspend(struct device *dev)
@@ -1389,6 +1421,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 		}
 	}
 
+	adev->pmops_state = AMDGPU_PMOPS_RUNTIME_SUSPEND;
 	adev->in_runpm = true;
 	if (amdgpu_device_supports_px(drm_dev))
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
@@ -1396,6 +1429,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 	ret = amdgpu_device_suspend(drm_dev, false);
 	if (ret) {
 		adev->in_runpm = false;
+		adev->pmops_state = AMDGPU_PMOPS_NONE;
 		return ret;
 	}
 
@@ -1412,6 +1446,8 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 		amdgpu_device_baco_enter(drm_dev);
 	}
 
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
+
 	return 0;
 }
 
@@ -1425,6 +1461,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 	if (!adev->runpm)
 		return -EINVAL;
 
+	adev->pmops_state = AMDGPU_PMOPS_RUNTIME_RESUME;
 	if (amdgpu_device_supports_px(drm_dev)) {
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
@@ -1449,6 +1486,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 	if (amdgpu_device_supports_px(drm_dev))
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
 	adev->in_runpm = false;
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
 	return 0;
 }
 
@@ -1464,6 +1502,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
 		return -EBUSY;
 	}
 
+	adev->pmops_state = AMDGPU_PMOPS_RUNTIME_IDLE;
 	if (amdgpu_device_has_dc_support(adev)) {
 		struct drm_crtc *crtc;
 
@@ -1504,6 +1543,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
 
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_autosuspend(dev);
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 502e1b926a06..05a15f858a06 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -1327,7 +1327,8 @@ static int smu_disable_dpms(struct smu_context *smu)
 	bool use_baco = !smu->is_apu &&
 		((amdgpu_in_reset(adev) &&
 		  (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
-		 ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
+		 ((adev->in_runpm || (adev->pmops_state == AMDGPU_PMOPS_FREEZE))
+		  && amdgpu_asic_supports_baco(adev)));
 
 	/*
 	 * For custom pptable uploading, skip the DPM features
-- 
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] 16+ messages in thread

* RE: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
  2021-03-09  4:10 ` [PATCH 4/7] drm/amdgpu: track what pmops flow we are in Alex Deucher
@ 2021-03-09  6:19   ` Lazar, Lijo
  2021-03-09 15:47     ` Alex Deucher
  0 siblings, 1 reply; 16+ messages in thread
From: Lazar, Lijo @ 2021-03-09  6:19 UTC (permalink / raw)
  To: Deucher, Alexander, amd-gfx; +Cc: Deucher, Alexander

[AMD Public Use]

This seems a duplicate of dev_pm_info states. Can't we reuse that?

Thanks,
Lijo

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
Sent: Tuesday, March 9, 2021 9:40 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in

We reuse the same suspend and resume functions for all of the pmops states, so flag what state we are in so that we can alter behavior deeper in the driver depending on the current flow.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       | 20 +++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 58 +++++++++++++++++++----
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c |  3 +-
 3 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index d47626ce9bc5..4ddc5cc983c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -347,6 +347,24 @@ int amdgpu_device_ip_block_add(struct amdgpu_device *adev,  bool amdgpu_get_bios(struct amdgpu_device *adev);  bool amdgpu_read_bios(struct amdgpu_device *adev);
 
+/*
+ * PM Ops
+ */
+enum amdgpu_pmops_state {
+	AMDGPU_PMOPS_NONE = 0,
+	AMDGPU_PMOPS_PREPARE,
+	AMDGPU_PMOPS_COMPLETE,
+	AMDGPU_PMOPS_SUSPEND,
+	AMDGPU_PMOPS_RESUME,
+	AMDGPU_PMOPS_FREEZE,
+	AMDGPU_PMOPS_THAW,
+	AMDGPU_PMOPS_POWEROFF,
+	AMDGPU_PMOPS_RESTORE,
+	AMDGPU_PMOPS_RUNTIME_SUSPEND,
+	AMDGPU_PMOPS_RUNTIME_RESUME,
+	AMDGPU_PMOPS_RUNTIME_IDLE,
+};
+
 /*
  * Clocks
  */
@@ -1019,8 +1037,8 @@ struct amdgpu_device {
 	u8				reset_magic[AMDGPU_RESET_MAGIC_NUM];
 
 	/* s3/s4 mask */
+	enum amdgpu_pmops_state         pmops_state;
 	bool                            in_suspend;
-	bool				in_hibernate;
 
 	/*
 	 * The combination flag in_poweroff_reboot_com used to identify the poweroff diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 3e6bb7d79652..0312c52bd39d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1297,34 +1297,54 @@ amdgpu_pci_shutdown(struct pci_dev *pdev)  static int amdgpu_pmops_prepare(struct device *dev)  {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+	int r;
 
+	adev->pmops_state = AMDGPU_PMOPS_PREPARE;
 	/* Return a positive number here so
 	 * DPM_FLAG_SMART_SUSPEND works properly
 	 */
 	if (amdgpu_device_supports_boco(drm_dev))
-		return pm_runtime_suspended(dev) &&
+		r= pm_runtime_suspended(dev) &&
 			pm_suspend_via_firmware();
-
-	return 0;
+	else
+		r = 0;
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
+	return r;
 }
 
 static void amdgpu_pmops_complete(struct device *dev)  {
+	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+
+	adev->pmops_state = AMDGPU_PMOPS_COMPLETE;
 	/* nothing to do */
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
 }
 
 static int amdgpu_pmops_suspend(struct device *dev)  {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+	int r;
 
-	return amdgpu_device_suspend(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_SUSPEND;
+	r = amdgpu_device_suspend(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
+	return r;
 }
 
 static int amdgpu_pmops_resume(struct device *dev)  {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+	int r;
 
-	return amdgpu_device_resume(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_RESUME;
+	r = amdgpu_device_resume(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
+	return r;
 }
 
 static int amdgpu_pmops_freeze(struct device *dev) @@ -1333,9 +1353,9 @@ static int amdgpu_pmops_freeze(struct device *dev)
 	struct amdgpu_device *adev = drm_to_adev(drm_dev);
 	int r;
 
-	adev->in_hibernate = true;
+	adev->pmops_state = AMDGPU_PMOPS_FREEZE;
 	r = amdgpu_device_suspend(drm_dev, true);
-	adev->in_hibernate = false;
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
 	if (r)
 		return r;
 	return amdgpu_asic_reset(adev);
@@ -1344,8 +1364,13 @@ static int amdgpu_pmops_freeze(struct device *dev)  static int amdgpu_pmops_thaw(struct device *dev)  {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+	int r;
 
-	return amdgpu_device_resume(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_THAW;
+	r = amdgpu_device_resume(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
+	return r;
 }
 
 static int amdgpu_pmops_poweroff(struct device *dev) @@ -1354,17 +1379,24 @@ static int amdgpu_pmops_poweroff(struct device *dev)
 	struct amdgpu_device *adev = drm_to_adev(drm_dev);
 	int r;
 
+	adev->pmops_state = AMDGPU_PMOPS_POWEROFF;
 	adev->in_poweroff_reboot_com = true;
 	r =  amdgpu_device_suspend(drm_dev, true);
 	adev->in_poweroff_reboot_com = false;
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
 	return r;
 }
 
 static int amdgpu_pmops_restore(struct device *dev)  {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+	int r;
 
-	return amdgpu_device_resume(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_RESTORE;
+	r = amdgpu_device_resume(drm_dev, true);
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
+	return r;
 }
 
 static int amdgpu_pmops_runtime_suspend(struct device *dev) @@ -1389,6 +1421,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 		}
 	}
 
+	adev->pmops_state = AMDGPU_PMOPS_RUNTIME_SUSPEND;
 	adev->in_runpm = true;
 	if (amdgpu_device_supports_px(drm_dev))
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING; @@ -1396,6 +1429,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 	ret = amdgpu_device_suspend(drm_dev, false);
 	if (ret) {
 		adev->in_runpm = false;
+		adev->pmops_state = AMDGPU_PMOPS_NONE;
 		return ret;
 	}
 
@@ -1412,6 +1446,8 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 		amdgpu_device_baco_enter(drm_dev);
 	}
 
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
+
 	return 0;
 }
 
@@ -1425,6 +1461,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 	if (!adev->runpm)
 		return -EINVAL;
 
+	adev->pmops_state = AMDGPU_PMOPS_RUNTIME_RESUME;
 	if (amdgpu_device_supports_px(drm_dev)) {
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
@@ -1449,6 +1486,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 	if (amdgpu_device_supports_px(drm_dev))
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
 	adev->in_runpm = false;
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
 	return 0;
 }
 
@@ -1464,6 +1502,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
 		return -EBUSY;
 	}
 
+	adev->pmops_state = AMDGPU_PMOPS_RUNTIME_IDLE;
 	if (amdgpu_device_has_dc_support(adev)) {
 		struct drm_crtc *crtc;
 
@@ -1504,6 +1543,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
 
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_autosuspend(dev);
+	adev->pmops_state = AMDGPU_PMOPS_NONE;
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 502e1b926a06..05a15f858a06 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -1327,7 +1327,8 @@ static int smu_disable_dpms(struct smu_context *smu)
 	bool use_baco = !smu->is_apu &&
 		((amdgpu_in_reset(adev) &&
 		  (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
-		 ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
+		 ((adev->in_runpm || (adev->pmops_state == AMDGPU_PMOPS_FREEZE))
+		  && amdgpu_asic_supports_baco(adev)));
 
 	/*
 	 * For custom pptable uploading, skip the DPM features
--
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%7C522d9fee476f4075753008d8e2b14e6e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637508598450890140%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=pLFkgulTUPmA3C1RRbdJh2mxkGDWxoxTrkMRTs6HfjY%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] 16+ messages in thread

* Re: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
  2021-03-09  6:19   ` Lazar, Lijo
@ 2021-03-09 15:47     ` Alex Deucher
  2021-03-09 17:25       ` Bhardwaj, Rajneesh
  0 siblings, 1 reply; 16+ messages in thread
From: Alex Deucher @ 2021-03-09 15:47 UTC (permalink / raw)
  To: Lazar, Lijo; +Cc: Deucher, Alexander, amd-gfx

On Tue, Mar 9, 2021 at 1:19 AM Lazar, Lijo <Lijo.Lazar@amd.com> wrote:
>
> [AMD Public Use]
>
> This seems a duplicate of dev_pm_info states. Can't we reuse that?

Are you referring to the PM_EVENT_ messages in
dev_pm_info.power_state?  Maybe.  I was not able to find much
documentation on how those should be used.  Do you know?

Alex


>
> Thanks,
> Lijo
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
> Sent: Tuesday, March 9, 2021 9:40 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
>
> We reuse the same suspend and resume functions for all of the pmops states, so flag what state we are in so that we can alter behavior deeper in the driver depending on the current flow.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h       | 20 +++++++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 58 +++++++++++++++++++----
>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c |  3 +-
>  3 files changed, 70 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index d47626ce9bc5..4ddc5cc983c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -347,6 +347,24 @@ int amdgpu_device_ip_block_add(struct amdgpu_device *adev,  bool amdgpu_get_bios(struct amdgpu_device *adev);  bool amdgpu_read_bios(struct amdgpu_device *adev);
>
> +/*
> + * PM Ops
> + */
> +enum amdgpu_pmops_state {
> +       AMDGPU_PMOPS_NONE = 0,
> +       AMDGPU_PMOPS_PREPARE,
> +       AMDGPU_PMOPS_COMPLETE,
> +       AMDGPU_PMOPS_SUSPEND,
> +       AMDGPU_PMOPS_RESUME,
> +       AMDGPU_PMOPS_FREEZE,
> +       AMDGPU_PMOPS_THAW,
> +       AMDGPU_PMOPS_POWEROFF,
> +       AMDGPU_PMOPS_RESTORE,
> +       AMDGPU_PMOPS_RUNTIME_SUSPEND,
> +       AMDGPU_PMOPS_RUNTIME_RESUME,
> +       AMDGPU_PMOPS_RUNTIME_IDLE,
> +};
> +
>  /*
>   * Clocks
>   */
> @@ -1019,8 +1037,8 @@ struct amdgpu_device {
>         u8                              reset_magic[AMDGPU_RESET_MAGIC_NUM];
>
>         /* s3/s4 mask */
> +       enum amdgpu_pmops_state         pmops_state;
>         bool                            in_suspend;
> -       bool                            in_hibernate;
>
>         /*
>          * The combination flag in_poweroff_reboot_com used to identify the poweroff diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 3e6bb7d79652..0312c52bd39d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1297,34 +1297,54 @@ amdgpu_pci_shutdown(struct pci_dev *pdev)  static int amdgpu_pmops_prepare(struct device *dev)  {
>         struct drm_device *drm_dev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> +       int r;
>
> +       adev->pmops_state = AMDGPU_PMOPS_PREPARE;
>         /* Return a positive number here so
>          * DPM_FLAG_SMART_SUSPEND works properly
>          */
>         if (amdgpu_device_supports_boco(drm_dev))
> -               return pm_runtime_suspended(dev) &&
> +               r= pm_runtime_suspended(dev) &&
>                         pm_suspend_via_firmware();
> -
> -       return 0;
> +       else
> +               r = 0;
> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> +       return r;
>  }
>
>  static void amdgpu_pmops_complete(struct device *dev)  {
> +       struct drm_device *drm_dev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> +
> +       adev->pmops_state = AMDGPU_PMOPS_COMPLETE;
>         /* nothing to do */
> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>  }
>
>  static int amdgpu_pmops_suspend(struct device *dev)  {
>         struct drm_device *drm_dev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> +       int r;
>
> -       return amdgpu_device_suspend(drm_dev, true);
> +       adev->pmops_state = AMDGPU_PMOPS_SUSPEND;
> +       r = amdgpu_device_suspend(drm_dev, true);
> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> +       return r;
>  }
>
>  static int amdgpu_pmops_resume(struct device *dev)  {
>         struct drm_device *drm_dev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> +       int r;
>
> -       return amdgpu_device_resume(drm_dev, true);
> +       adev->pmops_state = AMDGPU_PMOPS_RESUME;
> +       r = amdgpu_device_resume(drm_dev, true);
> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> +       return r;
>  }
>
>  static int amdgpu_pmops_freeze(struct device *dev) @@ -1333,9 +1353,9 @@ static int amdgpu_pmops_freeze(struct device *dev)
>         struct amdgpu_device *adev = drm_to_adev(drm_dev);
>         int r;
>
> -       adev->in_hibernate = true;
> +       adev->pmops_state = AMDGPU_PMOPS_FREEZE;
>         r = amdgpu_device_suspend(drm_dev, true);
> -       adev->in_hibernate = false;
> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>         if (r)
>                 return r;
>         return amdgpu_asic_reset(adev);
> @@ -1344,8 +1364,13 @@ static int amdgpu_pmops_freeze(struct device *dev)  static int amdgpu_pmops_thaw(struct device *dev)  {
>         struct drm_device *drm_dev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> +       int r;
>
> -       return amdgpu_device_resume(drm_dev, true);
> +       adev->pmops_state = AMDGPU_PMOPS_THAW;
> +       r = amdgpu_device_resume(drm_dev, true);
> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> +       return r;
>  }
>
>  static int amdgpu_pmops_poweroff(struct device *dev) @@ -1354,17 +1379,24 @@ static int amdgpu_pmops_poweroff(struct device *dev)
>         struct amdgpu_device *adev = drm_to_adev(drm_dev);
>         int r;
>
> +       adev->pmops_state = AMDGPU_PMOPS_POWEROFF;
>         adev->in_poweroff_reboot_com = true;
>         r =  amdgpu_device_suspend(drm_dev, true);
>         adev->in_poweroff_reboot_com = false;
> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>         return r;
>  }
>
>  static int amdgpu_pmops_restore(struct device *dev)  {
>         struct drm_device *drm_dev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> +       int r;
>
> -       return amdgpu_device_resume(drm_dev, true);
> +       adev->pmops_state = AMDGPU_PMOPS_RESTORE;
> +       r = amdgpu_device_resume(drm_dev, true);
> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> +       return r;
>  }
>
>  static int amdgpu_pmops_runtime_suspend(struct device *dev) @@ -1389,6 +1421,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>                 }
>         }
>
> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_SUSPEND;
>         adev->in_runpm = true;
>         if (amdgpu_device_supports_px(drm_dev))
>                 drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING; @@ -1396,6 +1429,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>         ret = amdgpu_device_suspend(drm_dev, false);
>         if (ret) {
>                 adev->in_runpm = false;
> +               adev->pmops_state = AMDGPU_PMOPS_NONE;
>                 return ret;
>         }
>
> @@ -1412,6 +1446,8 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>                 amdgpu_device_baco_enter(drm_dev);
>         }
>
> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> +
>         return 0;
>  }
>
> @@ -1425,6 +1461,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
>         if (!adev->runpm)
>                 return -EINVAL;
>
> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_RESUME;
>         if (amdgpu_device_supports_px(drm_dev)) {
>                 drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>
> @@ -1449,6 +1486,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
>         if (amdgpu_device_supports_px(drm_dev))
>                 drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
>         adev->in_runpm = false;
> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>         return 0;
>  }
>
> @@ -1464,6 +1502,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
>                 return -EBUSY;
>         }
>
> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_IDLE;
>         if (amdgpu_device_has_dc_support(adev)) {
>                 struct drm_crtc *crtc;
>
> @@ -1504,6 +1543,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
>
>         pm_runtime_mark_last_busy(dev);
>         pm_runtime_autosuspend(dev);
> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>         return ret;
>  }
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 502e1b926a06..05a15f858a06 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1327,7 +1327,8 @@ static int smu_disable_dpms(struct smu_context *smu)
>         bool use_baco = !smu->is_apu &&
>                 ((amdgpu_in_reset(adev) &&
>                   (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
> -                ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
> +                ((adev->in_runpm || (adev->pmops_state == AMDGPU_PMOPS_FREEZE))
> +                 && amdgpu_asic_supports_baco(adev)));
>
>         /*
>          * For custom pptable uploading, skip the DPM features
> --
> 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%7C522d9fee476f4075753008d8e2b14e6e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637508598450890140%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=pLFkgulTUPmA3C1RRbdJh2mxkGDWxoxTrkMRTs6HfjY%3D&amp;reserved=0
> _______________________________________________
> 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] 16+ messages in thread

* Re: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
  2021-03-09 15:47     ` Alex Deucher
@ 2021-03-09 17:25       ` Bhardwaj, Rajneesh
  2021-03-10 10:16         ` Liang, Prike
  2021-03-10 20:00         ` Alex Deucher
  0 siblings, 2 replies; 16+ messages in thread
From: Bhardwaj, Rajneesh @ 2021-03-09 17:25 UTC (permalink / raw)
  To: Alex Deucher, Lazar, Lijo; +Cc: Deucher, Alexander, amd-gfx

pm_message_t events seem to be the right thing to use here instead of 
driver's privately managed power states. Please have a look

https://elixir.bootlin.com/linux/v4.7/source/drivers/gpu/drm/i915/i915_drv.c#L714

https://elixir.bootlin.com/linux/v4.7/source/drivers/gpu/drm/drm_sysfs.c#L43

Thanks,

Rajneesh


On 3/9/2021 10:47 AM, Alex Deucher wrote:
> [CAUTION: External Email]
>
> On Tue, Mar 9, 2021 at 1:19 AM Lazar, Lijo <Lijo.Lazar@amd.com> wrote:
>> [AMD Public Use]
>>
>> This seems a duplicate of dev_pm_info states. Can't we reuse that?
> Are you referring to the PM_EVENT_ messages in
> dev_pm_info.power_state?  Maybe.  I was not able to find much
> documentation on how those should be used.  Do you know?
>
> Alex
>
>
>> Thanks,
>> Lijo
>>
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
>> Sent: Tuesday, March 9, 2021 9:40 AM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
>> Subject: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
>>
>> We reuse the same suspend and resume functions for all of the pmops states, so flag what state we are in so that we can alter behavior deeper in the driver depending on the current flow.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h       | 20 +++++++-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 58 +++++++++++++++++++----
>>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c |  3 +-
>>   3 files changed, 70 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index d47626ce9bc5..4ddc5cc983c7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -347,6 +347,24 @@ int amdgpu_device_ip_block_add(struct amdgpu_device *adev,  bool amdgpu_get_bios(struct amdgpu_device *adev);  bool amdgpu_read_bios(struct amdgpu_device *adev);
>>
>> +/*
>> + * PM Ops
>> + */
>> +enum amdgpu_pmops_state {
>> +       AMDGPU_PMOPS_NONE = 0,
>> +       AMDGPU_PMOPS_PREPARE,
>> +       AMDGPU_PMOPS_COMPLETE,
>> +       AMDGPU_PMOPS_SUSPEND,
>> +       AMDGPU_PMOPS_RESUME,
>> +       AMDGPU_PMOPS_FREEZE,
>> +       AMDGPU_PMOPS_THAW,
>> +       AMDGPU_PMOPS_POWEROFF,
>> +       AMDGPU_PMOPS_RESTORE,
>> +       AMDGPU_PMOPS_RUNTIME_SUSPEND,
>> +       AMDGPU_PMOPS_RUNTIME_RESUME,
>> +       AMDGPU_PMOPS_RUNTIME_IDLE,
>> +};
>> +
>>   /*
>>    * Clocks
>>    */
>> @@ -1019,8 +1037,8 @@ struct amdgpu_device {
>>          u8                              reset_magic[AMDGPU_RESET_MAGIC_NUM];
>>
>>          /* s3/s4 mask */
>> +       enum amdgpu_pmops_state         pmops_state;
>>          bool                            in_suspend;
>> -       bool                            in_hibernate;
>>
>>          /*
>>           * The combination flag in_poweroff_reboot_com used to identify the poweroff diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 3e6bb7d79652..0312c52bd39d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -1297,34 +1297,54 @@ amdgpu_pci_shutdown(struct pci_dev *pdev)  static int amdgpu_pmops_prepare(struct device *dev)  {
>>          struct drm_device *drm_dev = dev_get_drvdata(dev);
>> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>> +       int r;
>>
>> +       adev->pmops_state = AMDGPU_PMOPS_PREPARE;
>>          /* Return a positive number here so
>>           * DPM_FLAG_SMART_SUSPEND works properly
>>           */
>>          if (amdgpu_device_supports_boco(drm_dev))
>> -               return pm_runtime_suspended(dev) &&
>> +               r= pm_runtime_suspended(dev) &&
>>                          pm_suspend_via_firmware();
>> -
>> -       return 0;
>> +       else
>> +               r = 0;
>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>> +       return r;
>>   }
>>
>>   static void amdgpu_pmops_complete(struct device *dev)  {
>> +       struct drm_device *drm_dev = dev_get_drvdata(dev);
>> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>> +
>> +       adev->pmops_state = AMDGPU_PMOPS_COMPLETE;
>>          /* nothing to do */
>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>   }
>>
>>   static int amdgpu_pmops_suspend(struct device *dev)  {
>>          struct drm_device *drm_dev = dev_get_drvdata(dev);
>> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>> +       int r;
>>
>> -       return amdgpu_device_suspend(drm_dev, true);
>> +       adev->pmops_state = AMDGPU_PMOPS_SUSPEND;
>> +       r = amdgpu_device_suspend(drm_dev, true);
>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>> +       return r;
>>   }
>>
>>   static int amdgpu_pmops_resume(struct device *dev)  {
>>          struct drm_device *drm_dev = dev_get_drvdata(dev);
>> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>> +       int r;
>>
>> -       return amdgpu_device_resume(drm_dev, true);
>> +       adev->pmops_state = AMDGPU_PMOPS_RESUME;
>> +       r = amdgpu_device_resume(drm_dev, true);
>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>> +       return r;
>>   }
>>
>>   static int amdgpu_pmops_freeze(struct device *dev) @@ -1333,9 +1353,9 @@ static int amdgpu_pmops_freeze(struct device *dev)
>>          struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>          int r;
>>
>> -       adev->in_hibernate = true;
>> +       adev->pmops_state = AMDGPU_PMOPS_FREEZE;
>>          r = amdgpu_device_suspend(drm_dev, true);
>> -       adev->in_hibernate = false;
>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>          if (r)
>>                  return r;
>>          return amdgpu_asic_reset(adev);
>> @@ -1344,8 +1364,13 @@ static int amdgpu_pmops_freeze(struct device *dev)  static int amdgpu_pmops_thaw(struct device *dev)  {
>>          struct drm_device *drm_dev = dev_get_drvdata(dev);
>> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>> +       int r;
>>
>> -       return amdgpu_device_resume(drm_dev, true);
>> +       adev->pmops_state = AMDGPU_PMOPS_THAW;
>> +       r = amdgpu_device_resume(drm_dev, true);
>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>> +       return r;
>>   }
>>
>>   static int amdgpu_pmops_poweroff(struct device *dev) @@ -1354,17 +1379,24 @@ static int amdgpu_pmops_poweroff(struct device *dev)
>>          struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>          int r;
>>
>> +       adev->pmops_state = AMDGPU_PMOPS_POWEROFF;
>>          adev->in_poweroff_reboot_com = true;
>>          r =  amdgpu_device_suspend(drm_dev, true);
>>          adev->in_poweroff_reboot_com = false;
>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>          return r;
>>   }
>>
>>   static int amdgpu_pmops_restore(struct device *dev)  {
>>          struct drm_device *drm_dev = dev_get_drvdata(dev);
>> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>> +       int r;
>>
>> -       return amdgpu_device_resume(drm_dev, true);
>> +       adev->pmops_state = AMDGPU_PMOPS_RESTORE;
>> +       r = amdgpu_device_resume(drm_dev, true);
>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>> +       return r;
>>   }
>>
>>   static int amdgpu_pmops_runtime_suspend(struct device *dev) @@ -1389,6 +1421,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>>                  }
>>          }
>>
>> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_SUSPEND;
>>          adev->in_runpm = true;
>>          if (amdgpu_device_supports_px(drm_dev))
>>                  drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING; @@ -1396,6 +1429,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>>          ret = amdgpu_device_suspend(drm_dev, false);
>>          if (ret) {
>>                  adev->in_runpm = false;
>> +               adev->pmops_state = AMDGPU_PMOPS_NONE;
>>                  return ret;
>>          }
>>
>> @@ -1412,6 +1446,8 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>>                  amdgpu_device_baco_enter(drm_dev);
>>          }
>>
>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>> +
>>          return 0;
>>   }
>>
>> @@ -1425,6 +1461,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
>>          if (!adev->runpm)
>>                  return -EINVAL;
>>
>> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_RESUME;
>>          if (amdgpu_device_supports_px(drm_dev)) {
>>                  drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>>
>> @@ -1449,6 +1486,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
>>          if (amdgpu_device_supports_px(drm_dev))
>>                  drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
>>          adev->in_runpm = false;
>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>          return 0;
>>   }
>>
>> @@ -1464,6 +1502,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
>>                  return -EBUSY;
>>          }
>>
>> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_IDLE;
>>          if (amdgpu_device_has_dc_support(adev)) {
>>                  struct drm_crtc *crtc;
>>
>> @@ -1504,6 +1543,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
>>
>>          pm_runtime_mark_last_busy(dev);
>>          pm_runtime_autosuspend(dev);
>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>          return ret;
>>   }
>>
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> index 502e1b926a06..05a15f858a06 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> @@ -1327,7 +1327,8 @@ static int smu_disable_dpms(struct smu_context *smu)
>>          bool use_baco = !smu->is_apu &&
>>                  ((amdgpu_in_reset(adev) &&
>>                    (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
>> -                ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
>> +                ((adev->in_runpm || (adev->pmops_state == AMDGPU_PMOPS_FREEZE))
>> +                 && amdgpu_asic_supports_baco(adev)));
>>
>>          /*
>>           * For custom pptable uploading, skip the DPM features
>> --
>> 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%7Crajneesh.bhardwaj%40amd.com%7Cd0524482d33848d5909f08d8e312b2e3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637509016743361628%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ZGYK5C%2Flo5BCIqxeEikspQi3Ib1M501mw04evWswAss%3D&amp;reserved=0
>> _______________________________________________
>> 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%7Crajneesh.bhardwaj%40amd.com%7Cd0524482d33848d5909f08d8e312b2e3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637509016743361628%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ZGYK5C%2Flo5BCIqxeEikspQi3Ib1M501mw04evWswAss%3D&amp;reserved=0
> _______________________________________________
> 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%7Crajneesh.bhardwaj%40amd.com%7Cd0524482d33848d5909f08d8e312b2e3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637509016743361628%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ZGYK5C%2Flo5BCIqxeEikspQi3Ib1M501mw04evWswAss%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] 16+ messages in thread

* RE: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
  2021-03-09 17:25       ` Bhardwaj, Rajneesh
@ 2021-03-10 10:16         ` Liang, Prike
  2021-03-10 13:19           ` Bhardwaj, Rajneesh
  2021-03-10 20:00         ` Alex Deucher
  1 sibling, 1 reply; 16+ messages in thread
From: Liang, Prike @ 2021-03-10 10:16 UTC (permalink / raw)
  To: Bhardwaj, Rajneesh, Alex Deucher, Lazar, Lijo; +Cc: Deucher, Alexander, amd-gfx

[AMD Official Use Only - Internal Distribution Only]

Maybe we can use the acpi_target_system_state() interface to identify the system-wide suspend target level Sx and then can parse the return code by the following macro definition. If have bandwidth will update and refine the AMDGPU Sx[0..5] suspend/resume sequence.

#define ACPI_STATE_S0                   (u8) 0
#define ACPI_STATE_S1                   (u8) 1
#define ACPI_STATE_S2                   (u8) 2
#define ACPI_STATE_S3                   (u8) 3
#define ACPI_STATE_S4                   (u8) 4
#define ACPI_STATE_S5                   (u8) 5

Thanks,
Prike
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Bhardwaj, Rajneesh
> Sent: Wednesday, March 10, 2021 1:25 AM
> To: Alex Deucher <alexdeucher@gmail.com>; Lazar, Lijo
> <Lijo.Lazar@amd.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
> gfx@lists.freedesktop.org
> Subject: Re: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
>
> pm_message_t events seem to be the right thing to use here instead of
> driver's privately managed power states. Please have a look
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir
> .bootlin.com%2Flinux%2Fv4.7%2Fsource%2Fdrivers%2Fgpu%2Fdrm%2Fi915
> %2Fi915_drv.c%23L714&amp;data=04%7C01%7CPrike.Liang%40amd.com%7
> C473ede68e7a347ff606b08d8e3204e94%7C3dd8961fe4884e608e11a82d994e
> 183d%7C0%7C0%7C637509075233985095%7CUnknown%7CTWFpbGZsb3d8e
> yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> %7C1000&amp;sdata=Y%2BNKrW2LfzB157fZ%2FuLn7QAu%2BmxVgHjzG8LO
> CH8z6J4%3D&amp;reserved=0
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir
> .bootlin.com%2Flinux%2Fv4.7%2Fsource%2Fdrivers%2Fgpu%2Fdrm%2Fdrm_
> sysfs.c%23L43&amp;data=04%7C01%7CPrike.Liang%40amd.com%7C473ede6
> 8e7a347ff606b08d8e3204e94%7C3dd8961fe4884e608e11a82d994e183d%7C
> 0%7C0%7C637509075233985095%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000
> &amp;sdata=svJsR97DiTwcbYHn3Y9dDV0YQCVzx5HLiebqQ9mTRY8%3D&am
> p;reserved=0
>
> Thanks,
>
> Rajneesh
>
>
> On 3/9/2021 10:47 AM, Alex Deucher wrote:
> > [CAUTION: External Email]
> >
> > On Tue, Mar 9, 2021 at 1:19 AM Lazar, Lijo <Lijo.Lazar@amd.com> wrote:
> >> [AMD Public Use]
> >>
> >> This seems a duplicate of dev_pm_info states. Can't we reuse that?
> > Are you referring to the PM_EVENT_ messages in
> > dev_pm_info.power_state?  Maybe.  I was not able to find much
> > documentation on how those should be used.  Do you know?
> >
> > Alex
> >
> >
> >> Thanks,
> >> Lijo
> >>
> >> -----Original Message-----
> >> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> >> Alex Deucher
> >> Sent: Tuesday, March 9, 2021 9:40 AM
> >> To: amd-gfx@lists.freedesktop.org
> >> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> >> Subject: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
> >>
> >> We reuse the same suspend and resume functions for all of the pmops
> states, so flag what state we are in so that we can alter behavior deeper in
> the driver depending on the current flow.
> >>
> >> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu.h       | 20 +++++++-
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 58
> +++++++++++++++++++----
> >>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c |  3 +-
> >>   3 files changed, 70 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> index d47626ce9bc5..4ddc5cc983c7 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> @@ -347,6 +347,24 @@ int amdgpu_device_ip_block_add(struct
> >> amdgpu_device *adev,  bool amdgpu_get_bios(struct amdgpu_device
> >> *adev);  bool amdgpu_read_bios(struct amdgpu_device *adev);
> >>
> >> +/*
> >> + * PM Ops
> >> + */
> >> +enum amdgpu_pmops_state {
> >> +       AMDGPU_PMOPS_NONE = 0,
> >> +       AMDGPU_PMOPS_PREPARE,
> >> +       AMDGPU_PMOPS_COMPLETE,
> >> +       AMDGPU_PMOPS_SUSPEND,
> >> +       AMDGPU_PMOPS_RESUME,
> >> +       AMDGPU_PMOPS_FREEZE,
> >> +       AMDGPU_PMOPS_THAW,
> >> +       AMDGPU_PMOPS_POWEROFF,
> >> +       AMDGPU_PMOPS_RESTORE,
> >> +       AMDGPU_PMOPS_RUNTIME_SUSPEND,
> >> +       AMDGPU_PMOPS_RUNTIME_RESUME,
> >> +       AMDGPU_PMOPS_RUNTIME_IDLE,
> >> +};
> >> +
> >>   /*
> >>    * Clocks
> >>    */
> >> @@ -1019,8 +1037,8 @@ struct amdgpu_device {
> >>          u8                              reset_magic[AMDGPU_RESET_MAGIC_NUM];
> >>
> >>          /* s3/s4 mask */
> >> +       enum amdgpu_pmops_state         pmops_state;
> >>          bool                            in_suspend;
> >> -       bool                            in_hibernate;
> >>
> >>          /*
> >>           * The combination flag in_poweroff_reboot_com used to
> >> identify the poweroff diff --git
> >> a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> index 3e6bb7d79652..0312c52bd39d 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> @@ -1297,34 +1297,54 @@ amdgpu_pci_shutdown(struct pci_dev *pdev)
> static int amdgpu_pmops_prepare(struct device *dev)  {
> >>          struct drm_device *drm_dev = dev_get_drvdata(dev);
> >> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >> +       int r;
> >>
> >> +       adev->pmops_state = AMDGPU_PMOPS_PREPARE;
> >>          /* Return a positive number here so
> >>           * DPM_FLAG_SMART_SUSPEND works properly
> >>           */
> >>          if (amdgpu_device_supports_boco(drm_dev))
> >> -               return pm_runtime_suspended(dev) &&
> >> +               r= pm_runtime_suspended(dev) &&
> >>                          pm_suspend_via_firmware();
> >> -
> >> -       return 0;
> >> +       else
> >> +               r = 0;
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >> +       return r;
> >>   }
> >>
> >>   static void amdgpu_pmops_complete(struct device *dev)  {
> >> +       struct drm_device *drm_dev = dev_get_drvdata(dev);
> >> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >> +
> >> +       adev->pmops_state = AMDGPU_PMOPS_COMPLETE;
> >>          /* nothing to do */
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >>   }
> >>
> >>   static int amdgpu_pmops_suspend(struct device *dev)  {
> >>          struct drm_device *drm_dev = dev_get_drvdata(dev);
> >> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >> +       int r;
> >>
> >> -       return amdgpu_device_suspend(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_SUSPEND;
> >> +       r = amdgpu_device_suspend(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >> +       return r;
> >>   }
> >>
> >>   static int amdgpu_pmops_resume(struct device *dev)  {
> >>          struct drm_device *drm_dev = dev_get_drvdata(dev);
> >> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >> +       int r;
> >>
> >> -       return amdgpu_device_resume(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_RESUME;
> >> +       r = amdgpu_device_resume(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >> +       return r;
> >>   }
> >>
> >>   static int amdgpu_pmops_freeze(struct device *dev) @@ -1333,9
> +1353,9 @@ static int amdgpu_pmops_freeze(struct device *dev)
> >>          struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >>          int r;
> >>
> >> -       adev->in_hibernate = true;
> >> +       adev->pmops_state = AMDGPU_PMOPS_FREEZE;
> >>          r = amdgpu_device_suspend(drm_dev, true);
> >> -       adev->in_hibernate = false;
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >>          if (r)
> >>                  return r;
> >>          return amdgpu_asic_reset(adev); @@ -1344,8 +1364,13 @@
> >> static int amdgpu_pmops_freeze(struct device *dev)  static int
> amdgpu_pmops_thaw(struct device *dev)  {
> >>          struct drm_device *drm_dev = dev_get_drvdata(dev);
> >> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >> +       int r;
> >>
> >> -       return amdgpu_device_resume(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_THAW;
> >> +       r = amdgpu_device_resume(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >> +       return r;
> >>   }
> >>
> >>   static int amdgpu_pmops_poweroff(struct device *dev) @@ -1354,17
> +1379,24 @@ static int amdgpu_pmops_poweroff(struct device *dev)
> >>          struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >>          int r;
> >>
> >> +       adev->pmops_state = AMDGPU_PMOPS_POWEROFF;
> >>          adev->in_poweroff_reboot_com = true;
> >>          r =  amdgpu_device_suspend(drm_dev, true);
> >>          adev->in_poweroff_reboot_com = false;
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >>          return r;
> >>   }
> >>
> >>   static int amdgpu_pmops_restore(struct device *dev)  {
> >>          struct drm_device *drm_dev = dev_get_drvdata(dev);
> >> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >> +       int r;
> >>
> >> -       return amdgpu_device_resume(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_RESTORE;
> >> +       r = amdgpu_device_resume(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >> +       return r;
> >>   }
> >>
> >>   static int amdgpu_pmops_runtime_suspend(struct device *dev) @@ -
> 1389,6 +1421,7 @@ static int amdgpu_pmops_runtime_suspend(struct
> device *dev)
> >>                  }
> >>          }
> >>
> >> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_SUSPEND;
> >>          adev->in_runpm = true;
> >>          if (amdgpu_device_supports_px(drm_dev))
> >>                  drm_dev->switch_power_state =
> DRM_SWITCH_POWER_CHANGING; @@ -1396,6 +1429,7 @@ static int
> amdgpu_pmops_runtime_suspend(struct device *dev)
> >>          ret = amdgpu_device_suspend(drm_dev, false);
> >>          if (ret) {
> >>                  adev->in_runpm = false;
> >> +               adev->pmops_state = AMDGPU_PMOPS_NONE;
> >>                  return ret;
> >>          }
> >>
> >> @@ -1412,6 +1446,8 @@ static int
> amdgpu_pmops_runtime_suspend(struct device *dev)
> >>                  amdgpu_device_baco_enter(drm_dev);
> >>          }
> >>
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >> +
> >>          return 0;
> >>   }
> >>
> >> @@ -1425,6 +1461,7 @@ static int
> amdgpu_pmops_runtime_resume(struct device *dev)
> >>          if (!adev->runpm)
> >>                  return -EINVAL;
> >>
> >> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_RESUME;
> >>          if (amdgpu_device_supports_px(drm_dev)) {
> >>                  drm_dev->switch_power_state =
> >> DRM_SWITCH_POWER_CHANGING;
> >>
> >> @@ -1449,6 +1486,7 @@ static int
> amdgpu_pmops_runtime_resume(struct device *dev)
> >>          if (amdgpu_device_supports_px(drm_dev))
> >>                  drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
> >>          adev->in_runpm = false;
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >>          return 0;
> >>   }
> >>
> >> @@ -1464,6 +1502,7 @@ static int amdgpu_pmops_runtime_idle(struct
> device *dev)
> >>                  return -EBUSY;
> >>          }
> >>
> >> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_IDLE;
> >>          if (amdgpu_device_has_dc_support(adev)) {
> >>                  struct drm_crtc *crtc;
> >>
> >> @@ -1504,6 +1543,7 @@ static int amdgpu_pmops_runtime_idle(struct
> >> device *dev)
> >>
> >>          pm_runtime_mark_last_busy(dev);
> >>          pm_runtime_autosuspend(dev);
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >>          return ret;
> >>   }
> >>
> >> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >> index 502e1b926a06..05a15f858a06 100644
> >> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >> @@ -1327,7 +1327,8 @@ static int smu_disable_dpms(struct
> smu_context *smu)
> >>          bool use_baco = !smu->is_apu &&
> >>                  ((amdgpu_in_reset(adev) &&
> >>                    (amdgpu_asic_reset_method(adev) ==
> AMD_RESET_METHOD_BACO)) ||
> >> -                ((adev->in_runpm || adev->in_hibernate) &&
> amdgpu_asic_supports_baco(adev)));
> >> +                ((adev->in_runpm || (adev->pmops_state ==
> AMDGPU_PMOPS_FREEZE))
> >> +                 && amdgpu_asic_supports_baco(adev)));
> >>
> >>          /*
> >>           * For custom pptable uploading, skip the DPM features
> >> --
> >> 2.29.2
> >>
> >> _______________________________________________
> >> amd-gfx mailing list
> >> amd-gfx@lists.freedesktop.org
> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> >> ts.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> gfx&amp;data=04%7C01%7C
> >>
> Prike.Liang%40amd.com%7C473ede68e7a347ff606b08d8e3204e94%7C3dd89
> 61fe4
> >>
> 884e608e11a82d994e183d%7C0%7C0%7C637509075233985095%7CUnknow
> n%7CTWFpb
> >>
> GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> Mn
> >>
> 0%3D%7C1000&amp;sdata=yYtPSR7eqLfZzYn1N%2FCDvpp%2Fxr6lERvs8w7d
> uAiaX9g
> >> %3D&amp;reserved=0
> _______________________________________________
> >> amd-gfx mailing list
> >> amd-gfx@lists.freedesktop.org
> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> >> ts.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> gfx&amp;data=04%7C01%7C
> >>
> Prike.Liang%40amd.com%7C473ede68e7a347ff606b08d8e3204e94%7C3dd89
> 61fe4
> >>
> 884e608e11a82d994e183d%7C0%7C0%7C637509075233995092%7CUnknow
> n%7CTWFpb
> >>
> GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> Mn
> >>
> 0%3D%7C1000&amp;sdata=Kl90CvT0F7esbrGUTi1bGhA9Le7H7KZ3umrBcAAb
> Y6o%3D&
> >> amp;reserved=0
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> gfx&amp;data=04%7C01%7CPr
> >
> ike.Liang%40amd.com%7C473ede68e7a347ff606b08d8e3204e94%7C3dd896
> 1fe4884
> >
> e608e11a82d994e183d%7C0%7C0%7C637509075233995092%7CUnknown%7
> CTWFpbGZsb
> >
> 3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> %3D%
> >
> 7C1000&amp;sdata=Kl90CvT0F7esbrGUTi1bGhA9Le7H7KZ3umrBcAAbY6o%3
> D&amp;re
> > served=0
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.f
> reedesktop.org%2Fmailman%2Flistinfo%2Famd-
> gfx&amp;data=04%7C01%7CPrike.Liang%40amd.com%7C473ede68e7a347ff6
> 06b08d8e3204e94%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C
> 637509075233995092%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdat
> a=Kl90CvT0F7esbrGUTi1bGhA9Le7H7KZ3umrBcAAbY6o%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] 16+ messages in thread

* Re: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
  2021-03-10 10:16         ` Liang, Prike
@ 2021-03-10 13:19           ` Bhardwaj, Rajneesh
  0 siblings, 0 replies; 16+ messages in thread
From: Bhardwaj, Rajneesh @ 2021-03-10 13:19 UTC (permalink / raw)
  To: Liang, Prike, Alex Deucher, Lazar, Lijo; +Cc: Deucher, Alexander, amd-gfx

No, those are global system states. Here we are dealing with device pm 
states.

On 3/10/2021 5:16 AM, Liang, Prike wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
> Maybe we can use the acpi_target_system_state() interface to identify the system-wide suspend target level Sx and then can parse the return code by the following macro definition. If have bandwidth will update and refine the AMDGPU Sx[0..5] suspend/resume sequence.
>
> #define ACPI_STATE_S0                   (u8) 0
> #define ACPI_STATE_S1                   (u8) 1
> #define ACPI_STATE_S2                   (u8) 2
> #define ACPI_STATE_S3                   (u8) 3
> #define ACPI_STATE_S4                   (u8) 4
> #define ACPI_STATE_S5                   (u8) 5
>
> Thanks,
> Prike
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>> Bhardwaj, Rajneesh
>> Sent: Wednesday, March 10, 2021 1:25 AM
>> To: Alex Deucher <alexdeucher@gmail.com>; Lazar, Lijo
>> <Lijo.Lazar@amd.com>
>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
>> gfx@lists.freedesktop.org
>> Subject: Re: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
>>
>> pm_message_t events seem to be the right thing to use here instead of
>> driver's privately managed power states. Please have a look
>>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir
>> .bootlin.com%2Flinux%2Fv4.7%2Fsource%2Fdrivers%2Fgpu%2Fdrm%2Fi915
>> %2Fi915_drv.c%23L714&amp;data=04%7C01%7CPrike.Liang%40amd.com%7
>> C473ede68e7a347ff606b08d8e3204e94%7C3dd8961fe4884e608e11a82d994e
>> 183d%7C0%7C0%7C637509075233985095%7CUnknown%7CTWFpbGZsb3d8e
>> yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
>> %7C1000&amp;sdata=Y%2BNKrW2LfzB157fZ%2FuLn7QAu%2BmxVgHjzG8LO
>> CH8z6J4%3D&amp;reserved=0
>>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir
>> .bootlin.com%2Flinux%2Fv4.7%2Fsource%2Fdrivers%2Fgpu%2Fdrm%2Fdrm_
>> sysfs.c%23L43&amp;data=04%7C01%7CPrike.Liang%40amd.com%7C473ede6
>> 8e7a347ff606b08d8e3204e94%7C3dd8961fe4884e608e11a82d994e183d%7C
>> 0%7C0%7C637509075233985095%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
>> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000
>> &amp;sdata=svJsR97DiTwcbYHn3Y9dDV0YQCVzx5HLiebqQ9mTRY8%3D&am
>> p;reserved=0
>>
>> Thanks,
>>
>> Rajneesh
>>
>>
>> On 3/9/2021 10:47 AM, Alex Deucher wrote:
>>> [CAUTION: External Email]
>>>
>>> On Tue, Mar 9, 2021 at 1:19 AM Lazar, Lijo <Lijo.Lazar@amd.com> wrote:
>>>> [AMD Public Use]
>>>>
>>>> This seems a duplicate of dev_pm_info states. Can't we reuse that?
>>> Are you referring to the PM_EVENT_ messages in
>>> dev_pm_info.power_state?  Maybe.  I was not able to find much
>>> documentation on how those should be used.  Do you know?
>>>
>>> Alex
>>>
>>>
>>>> Thanks,
>>>> Lijo
>>>>
>>>> -----Original Message-----
>>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>>> Alex Deucher
>>>> Sent: Tuesday, March 9, 2021 9:40 AM
>>>> To: amd-gfx@lists.freedesktop.org
>>>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
>>>> Subject: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
>>>>
>>>> We reuse the same suspend and resume functions for all of the pmops
>> states, so flag what state we are in so that we can alter behavior deeper in
>> the driver depending on the current flow.
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu.h       | 20 +++++++-
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 58
>> +++++++++++++++++++----
>>>>    drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c |  3 +-
>>>>    3 files changed, 70 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> index d47626ce9bc5..4ddc5cc983c7 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> @@ -347,6 +347,24 @@ int amdgpu_device_ip_block_add(struct
>>>> amdgpu_device *adev,  bool amdgpu_get_bios(struct amdgpu_device
>>>> *adev);  bool amdgpu_read_bios(struct amdgpu_device *adev);
>>>>
>>>> +/*
>>>> + * PM Ops
>>>> + */
>>>> +enum amdgpu_pmops_state {
>>>> +       AMDGPU_PMOPS_NONE = 0,
>>>> +       AMDGPU_PMOPS_PREPARE,
>>>> +       AMDGPU_PMOPS_COMPLETE,
>>>> +       AMDGPU_PMOPS_SUSPEND,
>>>> +       AMDGPU_PMOPS_RESUME,
>>>> +       AMDGPU_PMOPS_FREEZE,
>>>> +       AMDGPU_PMOPS_THAW,
>>>> +       AMDGPU_PMOPS_POWEROFF,
>>>> +       AMDGPU_PMOPS_RESTORE,
>>>> +       AMDGPU_PMOPS_RUNTIME_SUSPEND,
>>>> +       AMDGPU_PMOPS_RUNTIME_RESUME,
>>>> +       AMDGPU_PMOPS_RUNTIME_IDLE,
>>>> +};
>>>> +
>>>>    /*
>>>>     * Clocks
>>>>     */
>>>> @@ -1019,8 +1037,8 @@ struct amdgpu_device {
>>>>           u8                              reset_magic[AMDGPU_RESET_MAGIC_NUM];
>>>>
>>>>           /* s3/s4 mask */
>>>> +       enum amdgpu_pmops_state         pmops_state;
>>>>           bool                            in_suspend;
>>>> -       bool                            in_hibernate;
>>>>
>>>>           /*
>>>>            * The combination flag in_poweroff_reboot_com used to
>>>> identify the poweroff diff --git
>>>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> index 3e6bb7d79652..0312c52bd39d 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> @@ -1297,34 +1297,54 @@ amdgpu_pci_shutdown(struct pci_dev *pdev)
>> static int amdgpu_pmops_prepare(struct device *dev)  {
>>>>           struct drm_device *drm_dev = dev_get_drvdata(dev);
>>>> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>> +       int r;
>>>>
>>>> +       adev->pmops_state = AMDGPU_PMOPS_PREPARE;
>>>>           /* Return a positive number here so
>>>>            * DPM_FLAG_SMART_SUSPEND works properly
>>>>            */
>>>>           if (amdgpu_device_supports_boco(drm_dev))
>>>> -               return pm_runtime_suspended(dev) &&
>>>> +               r= pm_runtime_suspended(dev) &&
>>>>                           pm_suspend_via_firmware();
>>>> -
>>>> -       return 0;
>>>> +       else
>>>> +               r = 0;
>>>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>>> +       return r;
>>>>    }
>>>>
>>>>    static void amdgpu_pmops_complete(struct device *dev)  {
>>>> +       struct drm_device *drm_dev = dev_get_drvdata(dev);
>>>> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>> +
>>>> +       adev->pmops_state = AMDGPU_PMOPS_COMPLETE;
>>>>           /* nothing to do */
>>>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>>>    }
>>>>
>>>>    static int amdgpu_pmops_suspend(struct device *dev)  {
>>>>           struct drm_device *drm_dev = dev_get_drvdata(dev);
>>>> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>> +       int r;
>>>>
>>>> -       return amdgpu_device_suspend(drm_dev, true);
>>>> +       adev->pmops_state = AMDGPU_PMOPS_SUSPEND;
>>>> +       r = amdgpu_device_suspend(drm_dev, true);
>>>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>>> +       return r;
>>>>    }
>>>>
>>>>    static int amdgpu_pmops_resume(struct device *dev)  {
>>>>           struct drm_device *drm_dev = dev_get_drvdata(dev);
>>>> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>> +       int r;
>>>>
>>>> -       return amdgpu_device_resume(drm_dev, true);
>>>> +       adev->pmops_state = AMDGPU_PMOPS_RESUME;
>>>> +       r = amdgpu_device_resume(drm_dev, true);
>>>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>>> +       return r;
>>>>    }
>>>>
>>>>    static int amdgpu_pmops_freeze(struct device *dev) @@ -1333,9
>> +1353,9 @@ static int amdgpu_pmops_freeze(struct device *dev)
>>>>           struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>>           int r;
>>>>
>>>> -       adev->in_hibernate = true;
>>>> +       adev->pmops_state = AMDGPU_PMOPS_FREEZE;
>>>>           r = amdgpu_device_suspend(drm_dev, true);
>>>> -       adev->in_hibernate = false;
>>>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>>>           if (r)
>>>>                   return r;
>>>>           return amdgpu_asic_reset(adev); @@ -1344,8 +1364,13 @@
>>>> static int amdgpu_pmops_freeze(struct device *dev)  static int
>> amdgpu_pmops_thaw(struct device *dev)  {
>>>>           struct drm_device *drm_dev = dev_get_drvdata(dev);
>>>> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>> +       int r;
>>>>
>>>> -       return amdgpu_device_resume(drm_dev, true);
>>>> +       adev->pmops_state = AMDGPU_PMOPS_THAW;
>>>> +       r = amdgpu_device_resume(drm_dev, true);
>>>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>>> +       return r;
>>>>    }
>>>>
>>>>    static int amdgpu_pmops_poweroff(struct device *dev) @@ -1354,17
>> +1379,24 @@ static int amdgpu_pmops_poweroff(struct device *dev)
>>>>           struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>>           int r;
>>>>
>>>> +       adev->pmops_state = AMDGPU_PMOPS_POWEROFF;
>>>>           adev->in_poweroff_reboot_com = true;
>>>>           r =  amdgpu_device_suspend(drm_dev, true);
>>>>           adev->in_poweroff_reboot_com = false;
>>>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>>>           return r;
>>>>    }
>>>>
>>>>    static int amdgpu_pmops_restore(struct device *dev)  {
>>>>           struct drm_device *drm_dev = dev_get_drvdata(dev);
>>>> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>> +       int r;
>>>>
>>>> -       return amdgpu_device_resume(drm_dev, true);
>>>> +       adev->pmops_state = AMDGPU_PMOPS_RESTORE;
>>>> +       r = amdgpu_device_resume(drm_dev, true);
>>>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>>> +       return r;
>>>>    }
>>>>
>>>>    static int amdgpu_pmops_runtime_suspend(struct device *dev) @@ -
>> 1389,6 +1421,7 @@ static int amdgpu_pmops_runtime_suspend(struct
>> device *dev)
>>>>                   }
>>>>           }
>>>>
>>>> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_SUSPEND;
>>>>           adev->in_runpm = true;
>>>>           if (amdgpu_device_supports_px(drm_dev))
>>>>                   drm_dev->switch_power_state =
>> DRM_SWITCH_POWER_CHANGING; @@ -1396,6 +1429,7 @@ static int
>> amdgpu_pmops_runtime_suspend(struct device *dev)
>>>>           ret = amdgpu_device_suspend(drm_dev, false);
>>>>           if (ret) {
>>>>                   adev->in_runpm = false;
>>>> +               adev->pmops_state = AMDGPU_PMOPS_NONE;
>>>>                   return ret;
>>>>           }
>>>>
>>>> @@ -1412,6 +1446,8 @@ static int
>> amdgpu_pmops_runtime_suspend(struct device *dev)
>>>>                   amdgpu_device_baco_enter(drm_dev);
>>>>           }
>>>>
>>>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>>> +
>>>>           return 0;
>>>>    }
>>>>
>>>> @@ -1425,6 +1461,7 @@ static int
>> amdgpu_pmops_runtime_resume(struct device *dev)
>>>>           if (!adev->runpm)
>>>>                   return -EINVAL;
>>>>
>>>> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_RESUME;
>>>>           if (amdgpu_device_supports_px(drm_dev)) {
>>>>                   drm_dev->switch_power_state =
>>>> DRM_SWITCH_POWER_CHANGING;
>>>>
>>>> @@ -1449,6 +1486,7 @@ static int
>> amdgpu_pmops_runtime_resume(struct device *dev)
>>>>           if (amdgpu_device_supports_px(drm_dev))
>>>>                   drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
>>>>           adev->in_runpm = false;
>>>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>>>           return 0;
>>>>    }
>>>>
>>>> @@ -1464,6 +1502,7 @@ static int amdgpu_pmops_runtime_idle(struct
>> device *dev)
>>>>                   return -EBUSY;
>>>>           }
>>>>
>>>> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_IDLE;
>>>>           if (amdgpu_device_has_dc_support(adev)) {
>>>>                   struct drm_crtc *crtc;
>>>>
>>>> @@ -1504,6 +1543,7 @@ static int amdgpu_pmops_runtime_idle(struct
>>>> device *dev)
>>>>
>>>>           pm_runtime_mark_last_busy(dev);
>>>>           pm_runtime_autosuspend(dev);
>>>> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
>>>>           return ret;
>>>>    }
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>> index 502e1b926a06..05a15f858a06 100644
>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>> @@ -1327,7 +1327,8 @@ static int smu_disable_dpms(struct
>> smu_context *smu)
>>>>           bool use_baco = !smu->is_apu &&
>>>>                   ((amdgpu_in_reset(adev) &&
>>>>                     (amdgpu_asic_reset_method(adev) ==
>> AMD_RESET_METHOD_BACO)) ||
>>>> -                ((adev->in_runpm || adev->in_hibernate) &&
>> amdgpu_asic_supports_baco(adev)));
>>>> +                ((adev->in_runpm || (adev->pmops_state ==
>> AMDGPU_PMOPS_FREEZE))
>>>> +                 && amdgpu_asic_supports_baco(adev)));
>>>>
>>>>           /*
>>>>            * For custom pptable uploading, skip the DPM features
>>>> --
>>>> 2.29.2
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
>>>> ts.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
>> gfx&amp;data=04%7C01%7C
>> Prike.Liang%40amd.com%7C473ede68e7a347ff606b08d8e3204e94%7C3dd89
>> 61fe4
>> 884e608e11a82d994e183d%7C0%7C0%7C637509075233985095%7CUnknow
>> n%7CTWFpb
>> GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
>> Mn
>> 0%3D%7C1000&amp;sdata=yYtPSR7eqLfZzYn1N%2FCDvpp%2Fxr6lERvs8w7d
>> uAiaX9g
>>>> %3D&amp;reserved=0
>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
>>>> ts.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
>> gfx&amp;data=04%7C01%7C
>> Prike.Liang%40amd.com%7C473ede68e7a347ff606b08d8e3204e94%7C3dd89
>> 61fe4
>> 884e608e11a82d994e183d%7C0%7C0%7C637509075233995092%7CUnknow
>> n%7CTWFpb
>> GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
>> Mn
>> 0%3D%7C1000&amp;sdata=Kl90CvT0F7esbrGUTi1bGhA9Le7H7KZ3umrBcAAb
>> Y6o%3D&
>>>> amp;reserved=0
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
>>> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
>> gfx&amp;data=04%7C01%7CPr
>> ike.Liang%40amd.com%7C473ede68e7a347ff606b08d8e3204e94%7C3dd896
>> 1fe4884
>> e608e11a82d994e183d%7C0%7C0%7C637509075233995092%7CUnknown%7
>> CTWFpbGZsb
>> 3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
>> %3D%
>> 7C1000&amp;sdata=Kl90CvT0F7esbrGUTi1bGhA9Le7H7KZ3umrBcAAbY6o%3
>> D&amp;re
>>> served=0
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.f
>> reedesktop.org%2Fmailman%2Flistinfo%2Famd-
>> gfx&amp;data=04%7C01%7CPrike.Liang%40amd.com%7C473ede68e7a347ff6
>> 06b08d8e3204e94%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C
>> 637509075233995092%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
>> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdat
>> a=Kl90CvT0F7esbrGUTi1bGhA9Le7H7KZ3umrBcAAbY6o%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] 16+ messages in thread

* Re: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
  2021-03-09 17:25       ` Bhardwaj, Rajneesh
  2021-03-10 10:16         ` Liang, Prike
@ 2021-03-10 20:00         ` Alex Deucher
  1 sibling, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2021-03-10 20:00 UTC (permalink / raw)
  To: Bhardwaj, Rajneesh; +Cc: Deucher, Alexander, Lazar, Lijo, amd-gfx

On Tue, Mar 9, 2021 at 12:25 PM Bhardwaj, Rajneesh
<rajneesh.bhardwaj@amd.com> wrote:
>
> pm_message_t events seem to be the right thing to use here instead of
> driver's privately managed power states. Please have a look
>
> https://elixir.bootlin.com/linux/v4.7/source/drivers/gpu/drm/i915/i915_drv.c#L714
>
> https://elixir.bootlin.com/linux/v4.7/source/drivers/gpu/drm/drm_sysfs.c#L43

Is the driver supposed to track those itself?  When I try and query it
(e.g., adev->ddev.dev->power.power_state.event), I never see it
getting set to anything other than PM_EVENT_ON.  If so, what' the
advantage of using it over a driver flag?

Alex


>
> Thanks,
>
> Rajneesh
>
>
> On 3/9/2021 10:47 AM, Alex Deucher wrote:
> > [CAUTION: External Email]
> >
> > On Tue, Mar 9, 2021 at 1:19 AM Lazar, Lijo <Lijo.Lazar@amd.com> wrote:
> >> [AMD Public Use]
> >>
> >> This seems a duplicate of dev_pm_info states. Can't we reuse that?
> > Are you referring to the PM_EVENT_ messages in
> > dev_pm_info.power_state?  Maybe.  I was not able to find much
> > documentation on how those should be used.  Do you know?
> >
> > Alex
> >
> >
> >> Thanks,
> >> Lijo
> >>
> >> -----Original Message-----
> >> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
> >> Sent: Tuesday, March 9, 2021 9:40 AM
> >> To: amd-gfx@lists.freedesktop.org
> >> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> >> Subject: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
> >>
> >> We reuse the same suspend and resume functions for all of the pmops states, so flag what state we are in so that we can alter behavior deeper in the driver depending on the current flow.
> >>
> >> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu.h       | 20 +++++++-
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 58 +++++++++++++++++++----
> >>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c |  3 +-
> >>   3 files changed, 70 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> index d47626ce9bc5..4ddc5cc983c7 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> @@ -347,6 +347,24 @@ int amdgpu_device_ip_block_add(struct amdgpu_device *adev,  bool amdgpu_get_bios(struct amdgpu_device *adev);  bool amdgpu_read_bios(struct amdgpu_device *adev);
> >>
> >> +/*
> >> + * PM Ops
> >> + */
> >> +enum amdgpu_pmops_state {
> >> +       AMDGPU_PMOPS_NONE = 0,
> >> +       AMDGPU_PMOPS_PREPARE,
> >> +       AMDGPU_PMOPS_COMPLETE,
> >> +       AMDGPU_PMOPS_SUSPEND,
> >> +       AMDGPU_PMOPS_RESUME,
> >> +       AMDGPU_PMOPS_FREEZE,
> >> +       AMDGPU_PMOPS_THAW,
> >> +       AMDGPU_PMOPS_POWEROFF,
> >> +       AMDGPU_PMOPS_RESTORE,
> >> +       AMDGPU_PMOPS_RUNTIME_SUSPEND,
> >> +       AMDGPU_PMOPS_RUNTIME_RESUME,
> >> +       AMDGPU_PMOPS_RUNTIME_IDLE,
> >> +};
> >> +
> >>   /*
> >>    * Clocks
> >>    */
> >> @@ -1019,8 +1037,8 @@ struct amdgpu_device {
> >>          u8                              reset_magic[AMDGPU_RESET_MAGIC_NUM];
> >>
> >>          /* s3/s4 mask */
> >> +       enum amdgpu_pmops_state         pmops_state;
> >>          bool                            in_suspend;
> >> -       bool                            in_hibernate;
> >>
> >>          /*
> >>           * The combination flag in_poweroff_reboot_com used to identify the poweroff diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> index 3e6bb7d79652..0312c52bd39d 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> @@ -1297,34 +1297,54 @@ amdgpu_pci_shutdown(struct pci_dev *pdev)  static int amdgpu_pmops_prepare(struct device *dev)  {
> >>          struct drm_device *drm_dev = dev_get_drvdata(dev);
> >> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >> +       int r;
> >>
> >> +       adev->pmops_state = AMDGPU_PMOPS_PREPARE;
> >>          /* Return a positive number here so
> >>           * DPM_FLAG_SMART_SUSPEND works properly
> >>           */
> >>          if (amdgpu_device_supports_boco(drm_dev))
> >> -               return pm_runtime_suspended(dev) &&
> >> +               r= pm_runtime_suspended(dev) &&
> >>                          pm_suspend_via_firmware();
> >> -
> >> -       return 0;
> >> +       else
> >> +               r = 0;
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >> +       return r;
> >>   }
> >>
> >>   static void amdgpu_pmops_complete(struct device *dev)  {
> >> +       struct drm_device *drm_dev = dev_get_drvdata(dev);
> >> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >> +
> >> +       adev->pmops_state = AMDGPU_PMOPS_COMPLETE;
> >>          /* nothing to do */
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >>   }
> >>
> >>   static int amdgpu_pmops_suspend(struct device *dev)  {
> >>          struct drm_device *drm_dev = dev_get_drvdata(dev);
> >> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >> +       int r;
> >>
> >> -       return amdgpu_device_suspend(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_SUSPEND;
> >> +       r = amdgpu_device_suspend(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >> +       return r;
> >>   }
> >>
> >>   static int amdgpu_pmops_resume(struct device *dev)  {
> >>          struct drm_device *drm_dev = dev_get_drvdata(dev);
> >> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >> +       int r;
> >>
> >> -       return amdgpu_device_resume(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_RESUME;
> >> +       r = amdgpu_device_resume(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >> +       return r;
> >>   }
> >>
> >>   static int amdgpu_pmops_freeze(struct device *dev) @@ -1333,9 +1353,9 @@ static int amdgpu_pmops_freeze(struct device *dev)
> >>          struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >>          int r;
> >>
> >> -       adev->in_hibernate = true;
> >> +       adev->pmops_state = AMDGPU_PMOPS_FREEZE;
> >>          r = amdgpu_device_suspend(drm_dev, true);
> >> -       adev->in_hibernate = false;
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >>          if (r)
> >>                  return r;
> >>          return amdgpu_asic_reset(adev);
> >> @@ -1344,8 +1364,13 @@ static int amdgpu_pmops_freeze(struct device *dev)  static int amdgpu_pmops_thaw(struct device *dev)  {
> >>          struct drm_device *drm_dev = dev_get_drvdata(dev);
> >> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >> +       int r;
> >>
> >> -       return amdgpu_device_resume(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_THAW;
> >> +       r = amdgpu_device_resume(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >> +       return r;
> >>   }
> >>
> >>   static int amdgpu_pmops_poweroff(struct device *dev) @@ -1354,17 +1379,24 @@ static int amdgpu_pmops_poweroff(struct device *dev)
> >>          struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >>          int r;
> >>
> >> +       adev->pmops_state = AMDGPU_PMOPS_POWEROFF;
> >>          adev->in_poweroff_reboot_com = true;
> >>          r =  amdgpu_device_suspend(drm_dev, true);
> >>          adev->in_poweroff_reboot_com = false;
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >>          return r;
> >>   }
> >>
> >>   static int amdgpu_pmops_restore(struct device *dev)  {
> >>          struct drm_device *drm_dev = dev_get_drvdata(dev);
> >> +       struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >> +       int r;
> >>
> >> -       return amdgpu_device_resume(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_RESTORE;
> >> +       r = amdgpu_device_resume(drm_dev, true);
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >> +       return r;
> >>   }
> >>
> >>   static int amdgpu_pmops_runtime_suspend(struct device *dev) @@ -1389,6 +1421,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
> >>                  }
> >>          }
> >>
> >> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_SUSPEND;
> >>          adev->in_runpm = true;
> >>          if (amdgpu_device_supports_px(drm_dev))
> >>                  drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING; @@ -1396,6 +1429,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
> >>          ret = amdgpu_device_suspend(drm_dev, false);
> >>          if (ret) {
> >>                  adev->in_runpm = false;
> >> +               adev->pmops_state = AMDGPU_PMOPS_NONE;
> >>                  return ret;
> >>          }
> >>
> >> @@ -1412,6 +1446,8 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
> >>                  amdgpu_device_baco_enter(drm_dev);
> >>          }
> >>
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >> +
> >>          return 0;
> >>   }
> >>
> >> @@ -1425,6 +1461,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
> >>          if (!adev->runpm)
> >>                  return -EINVAL;
> >>
> >> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_RESUME;
> >>          if (amdgpu_device_supports_px(drm_dev)) {
> >>                  drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
> >>
> >> @@ -1449,6 +1486,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
> >>          if (amdgpu_device_supports_px(drm_dev))
> >>                  drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
> >>          adev->in_runpm = false;
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >>          return 0;
> >>   }
> >>
> >> @@ -1464,6 +1502,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
> >>                  return -EBUSY;
> >>          }
> >>
> >> +       adev->pmops_state = AMDGPU_PMOPS_RUNTIME_IDLE;
> >>          if (amdgpu_device_has_dc_support(adev)) {
> >>                  struct drm_crtc *crtc;
> >>
> >> @@ -1504,6 +1543,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
> >>
> >>          pm_runtime_mark_last_busy(dev);
> >>          pm_runtime_autosuspend(dev);
> >> +       adev->pmops_state = AMDGPU_PMOPS_NONE;
> >>          return ret;
> >>   }
> >>
> >> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >> index 502e1b926a06..05a15f858a06 100644
> >> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >> @@ -1327,7 +1327,8 @@ static int smu_disable_dpms(struct smu_context *smu)
> >>          bool use_baco = !smu->is_apu &&
> >>                  ((amdgpu_in_reset(adev) &&
> >>                    (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
> >> -                ((adev->in_runpm || adev->in_hibernate) && amdgpu_asic_supports_baco(adev)));
> >> +                ((adev->in_runpm || (adev->pmops_state == AMDGPU_PMOPS_FREEZE))
> >> +                 && amdgpu_asic_supports_baco(adev)));
> >>
> >>          /*
> >>           * For custom pptable uploading, skip the DPM features
> >> --
> >> 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%7Crajneesh.bhardwaj%40amd.com%7Cd0524482d33848d5909f08d8e312b2e3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637509016743361628%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ZGYK5C%2Flo5BCIqxeEikspQi3Ib1M501mw04evWswAss%3D&amp;reserved=0
> >> _______________________________________________
> >> 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%7Crajneesh.bhardwaj%40amd.com%7Cd0524482d33848d5909f08d8e312b2e3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637509016743361628%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ZGYK5C%2Flo5BCIqxeEikspQi3Ib1M501mw04evWswAss%3D&amp;reserved=0
> > _______________________________________________
> > 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%7Crajneesh.bhardwaj%40amd.com%7Cd0524482d33848d5909f08d8e312b2e3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637509016743361628%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ZGYK5C%2Flo5BCIqxeEikspQi3Ib1M501mw04evWswAss%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] 16+ messages in thread

* [PATCH 5/7] drm/amdgpu: don't evict vram on APUs for suspend to ram
  2021-03-09  4:10 [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2) Alex Deucher
                   ` (2 preceding siblings ...)
  2021-03-09  4:10 ` [PATCH 4/7] drm/amdgpu: track what pmops flow we are in Alex Deucher
@ 2021-03-09  4:10 ` Alex Deucher
  2021-03-09  4:10 ` [PATCH 6/7] drm/amdgpu: clean up S0ix logic Alex Deucher
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2021-03-09  4:10 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher

Vram is system memory, so no need to evict.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 4b29b8205442..2da3a3480863 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1028,13 +1028,11 @@ int amdgpu_bo_evict_vram(struct amdgpu_device *adev)
 {
 	struct ttm_resource_manager *man;
 
-	/* late 2.6.33 fix IGP hibernate - we need pm ops to do this correct */
-#ifndef CONFIG_HIBERNATION
-	if (adev->flags & AMD_IS_APU) {
-		/* Useless to evict on IGP chips */
+	if ((adev->flags & AMD_IS_APU) &&
+	    (adev->pmops_state == AMDGPU_PMOPS_SUSPEND)) {
+		/* Useless to evict vram on APUs for suspend to ram */
 		return 0;
 	}
-#endif
 
 	man = ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM);
 	return ttm_resource_manager_evict_all(&adev->mman.bdev, man);
-- 
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] 16+ messages in thread

* [PATCH 6/7] drm/amdgpu: clean up S0ix logic
  2021-03-09  4:10 [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2) Alex Deucher
                   ` (3 preceding siblings ...)
  2021-03-09  4:10 ` [PATCH 5/7] drm/amdgpu: don't evict vram on APUs for suspend to ram Alex Deucher
@ 2021-03-09  4:10 ` Alex Deucher
  2021-03-09  4:10 ` [PATCH 7/7] drm/amdgpu: clean up non-DC suspend/resume handling Alex Deucher
  2021-03-10  8:28 ` [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2) Quan, Evan
  6 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2021-03-09  4:10 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher

We only need special handling for the S0ix suspend and resume
cases, legacy S3/S4/shutdown/reboot/reset should use the
standard code pathes.  This should fix systems with S0ix
plus legacy S4.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  6 ------
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 22 ++++++++++++----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  4 ----
 3 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 4ddc5cc983c7..bf9359ccf3da 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1040,12 +1040,6 @@ struct amdgpu_device {
 	enum amdgpu_pmops_state         pmops_state;
 	bool                            in_suspend;
 
-	/*
-	 * The combination flag in_poweroff_reboot_com used to identify the poweroff
-	 * and reboot opt in the s0i3 system-wide suspend.
-	 */
-	bool 				in_poweroff_reboot_com;
-
 	atomic_t 			in_gpu_reset;
 	enum pp_mp1_state               mp1_state;
 	struct rw_semaphore reset_sem;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index ac5f7837285b..2b6e483259f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2680,9 +2680,10 @@ static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
 static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev)
 {
 	int i, r;
+	bool s0ix_suspend = amdgpu_acpi_is_s0ix_supported(adev) &&
+		(adev->pmops_state == AMDGPU_PMOPS_SUSPEND);
 
-	if (adev->in_poweroff_reboot_com ||
-	    !amdgpu_acpi_is_s0ix_supported(adev) || amdgpu_in_reset(adev)) {
+	if (!s0ix_suspend) {
 		amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE);
 		amdgpu_device_set_cg_state(adev, AMD_CG_STATE_UNGATE);
 	}
@@ -3672,13 +3673,13 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
  */
 int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)
 {
-	struct amdgpu_device *adev;
+	struct amdgpu_device *adev = drm_to_adev(dev);
 	struct drm_crtc *crtc;
 	struct drm_connector *connector;
 	struct drm_connector_list_iter iter;
 	int r;
-
-	adev = drm_to_adev(dev);
+	bool s0ix_suspend = amdgpu_acpi_is_s0ix_supported(adev) &&
+		(adev->pmops_state == AMDGPU_PMOPS_SUSPEND);
 
 	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
@@ -3741,11 +3742,10 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)
 
 	amdgpu_fence_driver_suspend(adev);
 
-	if (adev->in_poweroff_reboot_com ||
-	    !amdgpu_acpi_is_s0ix_supported(adev) || amdgpu_in_reset(adev))
-		r = amdgpu_device_ip_suspend_phase2(adev);
-	else
+	if (s0ix_suspend)
 		amdgpu_gfx_state_change_set(adev, sGpuChangeState_D3Entry);
+	else
+		r = amdgpu_device_ip_suspend_phase2(adev);
 	/* evict remaining vram memory
 	 * This second call to evict vram is to evict the gart page table
 	 * using the CPU.
@@ -3772,11 +3772,13 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon)
 	struct amdgpu_device *adev = drm_to_adev(dev);
 	struct drm_crtc *crtc;
 	int r = 0;
+	bool s0ix_resume = amdgpu_acpi_is_s0ix_supported(adev) &&
+		(adev->pmops_state == AMDGPU_PMOPS_RESUME);
 
 	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
 
-	if (amdgpu_acpi_is_s0ix_supported(adev))
+	if (s0ix_resume)
 		amdgpu_gfx_state_change_set(adev, sGpuChangeState_D0Entry);
 
 	/* post card */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0312c52bd39d..dd6d24305b16 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1288,9 +1288,7 @@ amdgpu_pci_shutdown(struct pci_dev *pdev)
 	 */
 	if (!amdgpu_passthrough(adev))
 		adev->mp1_state = PP_MP1_STATE_UNLOAD;
-	adev->in_poweroff_reboot_com = true;
 	amdgpu_device_ip_suspend(adev);
-	adev->in_poweroff_reboot_com = false;
 	adev->mp1_state = PP_MP1_STATE_NONE;
 }
 
@@ -1380,9 +1378,7 @@ static int amdgpu_pmops_poweroff(struct device *dev)
 	int r;
 
 	adev->pmops_state = AMDGPU_PMOPS_POWEROFF;
-	adev->in_poweroff_reboot_com = true;
 	r =  amdgpu_device_suspend(drm_dev, true);
-	adev->in_poweroff_reboot_com = false;
 	adev->pmops_state = AMDGPU_PMOPS_NONE;
 	return r;
 }
-- 
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] 16+ messages in thread

* [PATCH 7/7] drm/amdgpu: clean up non-DC suspend/resume handling
  2021-03-09  4:10 [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2) Alex Deucher
                   ` (4 preceding siblings ...)
  2021-03-09  4:10 ` [PATCH 6/7] drm/amdgpu: clean up S0ix logic Alex Deucher
@ 2021-03-09  4:10 ` Alex Deucher
  2021-03-10  9:05   ` Quan, Evan
  2021-03-10  8:28 ` [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2) Quan, Evan
  6 siblings, 1 reply; 16+ messages in thread
From: Alex Deucher @ 2021-03-09  4:10 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher

Move the non-DC specific code into the DCE IP blocks similar
to how we handle DC.  This cleans up the common suspend
and resume pathes.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 82 +------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 88 +++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.h |  3 +
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c      |  9 ++-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c      |  9 ++-
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c       |  8 +-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c       |  9 ++-
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c    | 15 +++-
 8 files changed, 137 insertions(+), 86 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 2b6e483259f1..c4ccf7a313f8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3674,9 +3674,6 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
 int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)
 {
 	struct amdgpu_device *adev = drm_to_adev(dev);
-	struct drm_crtc *crtc;
-	struct drm_connector *connector;
-	struct drm_connector_list_iter iter;
 	int r;
 	bool s0ix_suspend = amdgpu_acpi_is_s0ix_supported(adev) &&
 		(adev->pmops_state == AMDGPU_PMOPS_SUSPEND);
@@ -3692,45 +3689,6 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)
 
 	cancel_delayed_work_sync(&adev->delayed_init_work);
 
-	if (!amdgpu_device_has_dc_support(adev)) {
-		/* turn off display hw */
-		drm_modeset_lock_all(dev);
-		drm_connector_list_iter_begin(dev, &iter);
-		drm_for_each_connector_iter(connector, &iter)
-			drm_helper_connector_dpms(connector,
-						  DRM_MODE_DPMS_OFF);
-		drm_connector_list_iter_end(&iter);
-		drm_modeset_unlock_all(dev);
-			/* unpin the front buffers and cursors */
-		list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-			struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-			struct drm_framebuffer *fb = crtc->primary->fb;
-			struct amdgpu_bo *robj;
-
-			if (amdgpu_crtc->cursor_bo && !adev->enable_virtual_display) {
-				struct amdgpu_bo *aobj = gem_to_amdgpu_bo(amdgpu_crtc->cursor_bo);
-				r = amdgpu_bo_reserve(aobj, true);
-				if (r == 0) {
-					amdgpu_bo_unpin(aobj);
-					amdgpu_bo_unreserve(aobj);
-				}
-			}
-
-			if (fb == NULL || fb->obj[0] == NULL) {
-				continue;
-			}
-			robj = gem_to_amdgpu_bo(fb->obj[0]);
-			/* don't unpin kernel fb objects */
-			if (!amdgpu_fbdev_robj_is_fb(adev, robj)) {
-				r = amdgpu_bo_reserve(robj, true);
-				if (r == 0) {
-					amdgpu_bo_unpin(robj);
-					amdgpu_bo_unreserve(robj);
-				}
-			}
-		}
-	}
-
 	amdgpu_ras_suspend(adev);
 
 	r = amdgpu_device_ip_suspend_phase1(adev);
@@ -3767,10 +3725,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)
  */
 int amdgpu_device_resume(struct drm_device *dev, bool fbcon)
 {
-	struct drm_connector *connector;
-	struct drm_connector_list_iter iter;
 	struct amdgpu_device *adev = drm_to_adev(dev);
-	struct drm_crtc *crtc;
 	int r = 0;
 	bool s0ix_resume = amdgpu_acpi_is_s0ix_supported(adev) &&
 		(adev->pmops_state == AMDGPU_PMOPS_RESUME);
@@ -3803,24 +3758,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon)
 	queue_delayed_work(system_wq, &adev->delayed_init_work,
 			   msecs_to_jiffies(AMDGPU_RESUME_MS));
 
-	if (!amdgpu_device_has_dc_support(adev)) {
-		/* pin cursors */
-		list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-			struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-
-			if (amdgpu_crtc->cursor_bo && !adev->enable_virtual_display) {
-				struct amdgpu_bo *aobj = gem_to_amdgpu_bo(amdgpu_crtc->cursor_bo);
-				r = amdgpu_bo_reserve(aobj, true);
-				if (r == 0) {
-					r = amdgpu_bo_pin(aobj, AMDGPU_GEM_DOMAIN_VRAM);
-					if (r != 0)
-						dev_err(adev->dev, "Failed to pin cursor BO (%d)\n", r);
-					amdgpu_crtc->cursor_addr = amdgpu_bo_gpu_offset(aobj);
-					amdgpu_bo_unreserve(aobj);
-				}
-			}
-		}
-	}
 	r = amdgpu_amdkfd_resume(adev, adev->in_runpm);
 	if (r)
 		return r;
@@ -3828,25 +3765,8 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon)
 	/* Make sure IB tests flushed */
 	flush_delayed_work(&adev->delayed_init_work);
 
-	/* blat the mode back in */
-	if (fbcon) {
-		if (!amdgpu_device_has_dc_support(adev)) {
-			/* pre DCE11 */
-			drm_helper_resume_force_mode(dev);
-
-			/* turn on display hw */
-			drm_modeset_lock_all(dev);
-
-			drm_connector_list_iter_begin(dev, &iter);
-			drm_for_each_connector_iter(connector, &iter)
-				drm_helper_connector_dpms(connector,
-							  DRM_MODE_DPMS_ON);
-			drm_connector_list_iter_end(&iter);
-
-			drm_modeset_unlock_all(dev);
-		}
+	if (fbcon)
 		amdgpu_fbdev_set_suspend(adev, 0);
-	}
 
 	drm_kms_helper_poll_enable(dev);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 48cb33e5b382..c3797bf3c583 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1310,3 +1310,91 @@ bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc,
 	return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos,
 						  stime, etime, mode);
 }
+
+int amdgpu_display_suspend_helper(struct amdgpu_device *adev)
+{
+	struct drm_device *dev = adev_to_drm(adev);
+	struct drm_crtc *crtc;
+	struct drm_connector *connector;
+	struct drm_connector_list_iter iter;
+	int r;
+
+	/* turn off display hw */
+	drm_modeset_lock_all(dev);
+	drm_connector_list_iter_begin(dev, &iter);
+	drm_for_each_connector_iter(connector, &iter)
+		drm_helper_connector_dpms(connector,
+					  DRM_MODE_DPMS_OFF);
+	drm_connector_list_iter_end(&iter);
+	drm_modeset_unlock_all(dev);
+	/* unpin the front buffers and cursors */
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
+		struct drm_framebuffer *fb = crtc->primary->fb;
+		struct amdgpu_bo *robj;
+
+		if (amdgpu_crtc->cursor_bo && !adev->enable_virtual_display) {
+			struct amdgpu_bo *aobj = gem_to_amdgpu_bo(amdgpu_crtc->cursor_bo);
+			r = amdgpu_bo_reserve(aobj, true);
+			if (r == 0) {
+				amdgpu_bo_unpin(aobj);
+				amdgpu_bo_unreserve(aobj);
+			}
+		}
+
+		if (fb == NULL || fb->obj[0] == NULL) {
+			continue;
+		}
+		robj = gem_to_amdgpu_bo(fb->obj[0]);
+		/* don't unpin kernel fb objects */
+		if (!amdgpu_fbdev_robj_is_fb(adev, robj)) {
+			r = amdgpu_bo_reserve(robj, true);
+			if (r == 0) {
+				amdgpu_bo_unpin(robj);
+				amdgpu_bo_unreserve(robj);
+			}
+		}
+	}
+	return r;
+}
+
+int amdgpu_display_resume_helper(struct amdgpu_device *adev)
+{
+	struct drm_device *dev = adev_to_drm(adev);
+	struct drm_connector *connector;
+	struct drm_connector_list_iter iter;
+	struct drm_crtc *crtc;
+	int r;
+
+	/* pin cursors */
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
+
+		if (amdgpu_crtc->cursor_bo && !adev->enable_virtual_display) {
+			struct amdgpu_bo *aobj = gem_to_amdgpu_bo(amdgpu_crtc->cursor_bo);
+			r = amdgpu_bo_reserve(aobj, true);
+			if (r == 0) {
+				r = amdgpu_bo_pin(aobj, AMDGPU_GEM_DOMAIN_VRAM);
+				if (r != 0)
+					dev_err(adev->dev, "Failed to pin cursor BO (%d)\n", r);
+				amdgpu_crtc->cursor_addr = amdgpu_bo_gpu_offset(aobj);
+				amdgpu_bo_unreserve(aobj);
+			}
+		}
+	}
+
+	drm_helper_resume_force_mode(dev);
+
+	/* turn on display hw */
+	drm_modeset_lock_all(dev);
+
+	drm_connector_list_iter_begin(dev, &iter);
+	drm_for_each_connector_iter(connector, &iter)
+		drm_helper_connector_dpms(connector,
+					  DRM_MODE_DPMS_ON);
+	drm_connector_list_iter_end(&iter);
+
+	drm_modeset_unlock_all(dev);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
index dc7b7d116549..7b6d83e2b13c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
@@ -47,4 +47,7 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
 const struct drm_format_info *
 amdgpu_lookup_format_info(u32 format, uint64_t modifier);
 
+int amdgpu_display_suspend_helper(struct amdgpu_device *adev);
+int amdgpu_display_resume_helper(struct amdgpu_device *adev);
+
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 7944781e1086..19abb740a169 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2897,6 +2897,11 @@ static int dce_v10_0_hw_fini(void *handle)
 static int dce_v10_0_suspend(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	r = amdgpu_display_suspend_helper(adev);
+	if (r)
+		return r;
 
 	adev->mode_info.bl_level =
 		amdgpu_atombios_encoder_get_backlight_level_from_reg(adev);
@@ -2921,8 +2926,10 @@ static int dce_v10_0_resume(void *handle)
 		amdgpu_display_backlight_set_level(adev, adev->mode_info.bl_encoder,
 						    bl_level);
 	}
+	if (ret)
+		return ret;
 
-	return ret;
+	return amdgpu_display_resume_helper(adev);
 }
 
 static bool dce_v10_0_is_idle(void *handle)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 1b6ff0470011..320ec35bfd37 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -3027,6 +3027,11 @@ static int dce_v11_0_hw_fini(void *handle)
 static int dce_v11_0_suspend(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	r = amdgpu_display_suspend_helper(adev);
+	if (r)
+		return r;
 
 	adev->mode_info.bl_level =
 		amdgpu_atombios_encoder_get_backlight_level_from_reg(adev);
@@ -3051,8 +3056,10 @@ static int dce_v11_0_resume(void *handle)
 		amdgpu_display_backlight_set_level(adev, adev->mode_info.bl_encoder,
 						    bl_level);
 	}
+	if (ret)
+		return ret;
 
-	return ret;
+	return amdgpu_display_resume_helper(adev);
 }
 
 static bool dce_v11_0_is_idle(void *handle)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index 83a88385b762..13322000ebd6 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -2770,7 +2770,11 @@ static int dce_v6_0_hw_fini(void *handle)
 static int dce_v6_0_suspend(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
 
+	r = amdgpu_display_suspend_helper(adev);
+	if (r)
+		return r;
 	adev->mode_info.bl_level =
 		amdgpu_atombios_encoder_get_backlight_level_from_reg(adev);
 
@@ -2794,8 +2798,10 @@ static int dce_v6_0_resume(void *handle)
 		amdgpu_display_backlight_set_level(adev, adev->mode_info.bl_encoder,
 						    bl_level);
 	}
+	if (ret)
+		return ret;
 
-	return ret;
+	return amdgpu_display_resume_helper(adev);
 }
 
 static bool dce_v6_0_is_idle(void *handle)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 224b30214427..04ebf02e5b8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -2796,6 +2796,11 @@ static int dce_v8_0_hw_fini(void *handle)
 static int dce_v8_0_suspend(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	r = amdgpu_display_suspend_helper(adev);
+	if (r)
+		return r;
 
 	adev->mode_info.bl_level =
 		amdgpu_atombios_encoder_get_backlight_level_from_reg(adev);
@@ -2820,8 +2825,10 @@ static int dce_v8_0_resume(void *handle)
 		amdgpu_display_backlight_set_level(adev, adev->mode_info.bl_encoder,
 						    bl_level);
 	}
+	if (ret)
+		return ret;
 
-	return ret;
+	return amdgpu_display_resume_helper(adev);
 }
 
 static bool dce_v8_0_is_idle(void *handle)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index 9810af712cc0..5c11144da051 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -39,6 +39,7 @@
 #include "dce_v11_0.h"
 #include "dce_virtual.h"
 #include "ivsrcid/ivsrcid_vislands30.h"
+#include "amdgpu_display.h"
 
 #define DCE_VIRTUAL_VBLANK_PERIOD 16666666
 
@@ -491,12 +492,24 @@ static int dce_virtual_hw_fini(void *handle)
 
 static int dce_virtual_suspend(void *handle)
 {
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	r = amdgpu_display_suspend_helper(adev);
+	if (r)
+		return r;
 	return dce_virtual_hw_fini(handle);
 }
 
 static int dce_virtual_resume(void *handle)
 {
-	return dce_virtual_hw_init(handle);
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	r = dce_virtual_hw_init(handle);
+	if (r)
+		return r;
+	return amdgpu_display_resume_helper(adev);
 }
 
 static bool dce_virtual_is_idle(void *handle)
-- 
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] 16+ messages in thread

* RE: [PATCH 7/7] drm/amdgpu: clean up non-DC suspend/resume handling
  2021-03-09  4:10 ` [PATCH 7/7] drm/amdgpu: clean up non-DC suspend/resume handling Alex Deucher
@ 2021-03-10  9:05   ` Quan, Evan
  0 siblings, 0 replies; 16+ messages in thread
From: Quan, Evan @ 2021-03-10  9:05 UTC (permalink / raw)
  To: Deucher, Alexander, amd-gfx; +Cc: Deucher, Alexander

[AMD Public Use]

Patch5,6,7 are reviewed-by: Evan Quan <evan.quan@amd.com>

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
Sent: Tuesday, March 9, 2021 12:10 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: [PATCH 7/7] drm/amdgpu: clean up non-DC suspend/resume handling

Move the non-DC specific code into the DCE IP blocks similar to how we handle DC.  This cleans up the common suspend and resume pathes.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 82 +------------------  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 88 +++++++++++++++++++++  drivers/gpu/drm/amd/amdgpu/amdgpu_display.h |  3 +
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c      |  9 ++-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c      |  9 ++-
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c       |  8 +-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c       |  9 ++-
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c    | 15 +++-
 8 files changed, 137 insertions(+), 86 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 2b6e483259f1..c4ccf7a313f8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3674,9 +3674,6 @@ void amdgpu_device_fini(struct amdgpu_device *adev)  int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)  {
 	struct amdgpu_device *adev = drm_to_adev(dev);
-	struct drm_crtc *crtc;
-	struct drm_connector *connector;
-	struct drm_connector_list_iter iter;
 	int r;
 	bool s0ix_suspend = amdgpu_acpi_is_s0ix_supported(adev) &&
 		(adev->pmops_state == AMDGPU_PMOPS_SUSPEND); @@ -3692,45 +3689,6 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)
 
 	cancel_delayed_work_sync(&adev->delayed_init_work);
 
-	if (!amdgpu_device_has_dc_support(adev)) {
-		/* turn off display hw */
-		drm_modeset_lock_all(dev);
-		drm_connector_list_iter_begin(dev, &iter);
-		drm_for_each_connector_iter(connector, &iter)
-			drm_helper_connector_dpms(connector,
-						  DRM_MODE_DPMS_OFF);
-		drm_connector_list_iter_end(&iter);
-		drm_modeset_unlock_all(dev);
-			/* unpin the front buffers and cursors */
-		list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-			struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-			struct drm_framebuffer *fb = crtc->primary->fb;
-			struct amdgpu_bo *robj;
-
-			if (amdgpu_crtc->cursor_bo && !adev->enable_virtual_display) {
-				struct amdgpu_bo *aobj = gem_to_amdgpu_bo(amdgpu_crtc->cursor_bo);
-				r = amdgpu_bo_reserve(aobj, true);
-				if (r == 0) {
-					amdgpu_bo_unpin(aobj);
-					amdgpu_bo_unreserve(aobj);
-				}
-			}
-
-			if (fb == NULL || fb->obj[0] == NULL) {
-				continue;
-			}
-			robj = gem_to_amdgpu_bo(fb->obj[0]);
-			/* don't unpin kernel fb objects */
-			if (!amdgpu_fbdev_robj_is_fb(adev, robj)) {
-				r = amdgpu_bo_reserve(robj, true);
-				if (r == 0) {
-					amdgpu_bo_unpin(robj);
-					amdgpu_bo_unreserve(robj);
-				}
-			}
-		}
-	}
-
 	amdgpu_ras_suspend(adev);
 
 	r = amdgpu_device_ip_suspend_phase1(adev);
@@ -3767,10 +3725,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)
  */
 int amdgpu_device_resume(struct drm_device *dev, bool fbcon)  {
-	struct drm_connector *connector;
-	struct drm_connector_list_iter iter;
 	struct amdgpu_device *adev = drm_to_adev(dev);
-	struct drm_crtc *crtc;
 	int r = 0;
 	bool s0ix_resume = amdgpu_acpi_is_s0ix_supported(adev) &&
 		(adev->pmops_state == AMDGPU_PMOPS_RESUME); @@ -3803,24 +3758,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon)
 	queue_delayed_work(system_wq, &adev->delayed_init_work,
 			   msecs_to_jiffies(AMDGPU_RESUME_MS));
 
-	if (!amdgpu_device_has_dc_support(adev)) {
-		/* pin cursors */
-		list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-			struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-
-			if (amdgpu_crtc->cursor_bo && !adev->enable_virtual_display) {
-				struct amdgpu_bo *aobj = gem_to_amdgpu_bo(amdgpu_crtc->cursor_bo);
-				r = amdgpu_bo_reserve(aobj, true);
-				if (r == 0) {
-					r = amdgpu_bo_pin(aobj, AMDGPU_GEM_DOMAIN_VRAM);
-					if (r != 0)
-						dev_err(adev->dev, "Failed to pin cursor BO (%d)\n", r);
-					amdgpu_crtc->cursor_addr = amdgpu_bo_gpu_offset(aobj);
-					amdgpu_bo_unreserve(aobj);
-				}
-			}
-		}
-	}
 	r = amdgpu_amdkfd_resume(adev, adev->in_runpm);
 	if (r)
 		return r;
@@ -3828,25 +3765,8 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon)
 	/* Make sure IB tests flushed */
 	flush_delayed_work(&adev->delayed_init_work);
 
-	/* blat the mode back in */
-	if (fbcon) {
-		if (!amdgpu_device_has_dc_support(adev)) {
-			/* pre DCE11 */
-			drm_helper_resume_force_mode(dev);
-
-			/* turn on display hw */
-			drm_modeset_lock_all(dev);
-
-			drm_connector_list_iter_begin(dev, &iter);
-			drm_for_each_connector_iter(connector, &iter)
-				drm_helper_connector_dpms(connector,
-							  DRM_MODE_DPMS_ON);
-			drm_connector_list_iter_end(&iter);
-
-			drm_modeset_unlock_all(dev);
-		}
+	if (fbcon)
 		amdgpu_fbdev_set_suspend(adev, 0);
-	}
 
 	drm_kms_helper_poll_enable(dev);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 48cb33e5b382..c3797bf3c583 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1310,3 +1310,91 @@ bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc,
 	return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos,
 						  stime, etime, mode);
 }
+
+int amdgpu_display_suspend_helper(struct amdgpu_device *adev) {
+	struct drm_device *dev = adev_to_drm(adev);
+	struct drm_crtc *crtc;
+	struct drm_connector *connector;
+	struct drm_connector_list_iter iter;
+	int r;
+
+	/* turn off display hw */
+	drm_modeset_lock_all(dev);
+	drm_connector_list_iter_begin(dev, &iter);
+	drm_for_each_connector_iter(connector, &iter)
+		drm_helper_connector_dpms(connector,
+					  DRM_MODE_DPMS_OFF);
+	drm_connector_list_iter_end(&iter);
+	drm_modeset_unlock_all(dev);
+	/* unpin the front buffers and cursors */
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
+		struct drm_framebuffer *fb = crtc->primary->fb;
+		struct amdgpu_bo *robj;
+
+		if (amdgpu_crtc->cursor_bo && !adev->enable_virtual_display) {
+			struct amdgpu_bo *aobj = gem_to_amdgpu_bo(amdgpu_crtc->cursor_bo);
+			r = amdgpu_bo_reserve(aobj, true);
+			if (r == 0) {
+				amdgpu_bo_unpin(aobj);
+				amdgpu_bo_unreserve(aobj);
+			}
+		}
+
+		if (fb == NULL || fb->obj[0] == NULL) {
+			continue;
+		}
+		robj = gem_to_amdgpu_bo(fb->obj[0]);
+		/* don't unpin kernel fb objects */
+		if (!amdgpu_fbdev_robj_is_fb(adev, robj)) {
+			r = amdgpu_bo_reserve(robj, true);
+			if (r == 0) {
+				amdgpu_bo_unpin(robj);
+				amdgpu_bo_unreserve(robj);
+			}
+		}
+	}
+	return r;
+}
+
+int amdgpu_display_resume_helper(struct amdgpu_device *adev) {
+	struct drm_device *dev = adev_to_drm(adev);
+	struct drm_connector *connector;
+	struct drm_connector_list_iter iter;
+	struct drm_crtc *crtc;
+	int r;
+
+	/* pin cursors */
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
+
+		if (amdgpu_crtc->cursor_bo && !adev->enable_virtual_display) {
+			struct amdgpu_bo *aobj = gem_to_amdgpu_bo(amdgpu_crtc->cursor_bo);
+			r = amdgpu_bo_reserve(aobj, true);
+			if (r == 0) {
+				r = amdgpu_bo_pin(aobj, AMDGPU_GEM_DOMAIN_VRAM);
+				if (r != 0)
+					dev_err(adev->dev, "Failed to pin cursor BO (%d)\n", r);
+				amdgpu_crtc->cursor_addr = amdgpu_bo_gpu_offset(aobj);
+				amdgpu_bo_unreserve(aobj);
+			}
+		}
+	}
+
+	drm_helper_resume_force_mode(dev);
+
+	/* turn on display hw */
+	drm_modeset_lock_all(dev);
+
+	drm_connector_list_iter_begin(dev, &iter);
+	drm_for_each_connector_iter(connector, &iter)
+		drm_helper_connector_dpms(connector,
+					  DRM_MODE_DPMS_ON);
+	drm_connector_list_iter_end(&iter);
+
+	drm_modeset_unlock_all(dev);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
index dc7b7d116549..7b6d83e2b13c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
@@ -47,4 +47,7 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,  const struct drm_format_info *
 amdgpu_lookup_format_info(u32 format, uint64_t modifier);
 
+int amdgpu_display_suspend_helper(struct amdgpu_device *adev); int 
+amdgpu_display_resume_helper(struct amdgpu_device *adev);
+
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 7944781e1086..19abb740a169 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2897,6 +2897,11 @@ static int dce_v10_0_hw_fini(void *handle)  static int dce_v10_0_suspend(void *handle)  {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	r = amdgpu_display_suspend_helper(adev);
+	if (r)
+		return r;
 
 	adev->mode_info.bl_level =
 		amdgpu_atombios_encoder_get_backlight_level_from_reg(adev);
@@ -2921,8 +2926,10 @@ static int dce_v10_0_resume(void *handle)
 		amdgpu_display_backlight_set_level(adev, adev->mode_info.bl_encoder,
 						    bl_level);
 	}
+	if (ret)
+		return ret;
 
-	return ret;
+	return amdgpu_display_resume_helper(adev);
 }
 
 static bool dce_v10_0_is_idle(void *handle) diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 1b6ff0470011..320ec35bfd37 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -3027,6 +3027,11 @@ static int dce_v11_0_hw_fini(void *handle)  static int dce_v11_0_suspend(void *handle)  {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	r = amdgpu_display_suspend_helper(adev);
+	if (r)
+		return r;
 
 	adev->mode_info.bl_level =
 		amdgpu_atombios_encoder_get_backlight_level_from_reg(adev);
@@ -3051,8 +3056,10 @@ static int dce_v11_0_resume(void *handle)
 		amdgpu_display_backlight_set_level(adev, adev->mode_info.bl_encoder,
 						    bl_level);
 	}
+	if (ret)
+		return ret;
 
-	return ret;
+	return amdgpu_display_resume_helper(adev);
 }
 
 static bool dce_v11_0_is_idle(void *handle) diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index 83a88385b762..13322000ebd6 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -2770,7 +2770,11 @@ static int dce_v6_0_hw_fini(void *handle)  static int dce_v6_0_suspend(void *handle)  {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
 
+	r = amdgpu_display_suspend_helper(adev);
+	if (r)
+		return r;
 	adev->mode_info.bl_level =
 		amdgpu_atombios_encoder_get_backlight_level_from_reg(adev);
 
@@ -2794,8 +2798,10 @@ static int dce_v6_0_resume(void *handle)
 		amdgpu_display_backlight_set_level(adev, adev->mode_info.bl_encoder,
 						    bl_level);
 	}
+	if (ret)
+		return ret;
 
-	return ret;
+	return amdgpu_display_resume_helper(adev);
 }
 
 static bool dce_v6_0_is_idle(void *handle) diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 224b30214427..04ebf02e5b8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -2796,6 +2796,11 @@ static int dce_v8_0_hw_fini(void *handle)  static int dce_v8_0_suspend(void *handle)  {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	r = amdgpu_display_suspend_helper(adev);
+	if (r)
+		return r;
 
 	adev->mode_info.bl_level =
 		amdgpu_atombios_encoder_get_backlight_level_from_reg(adev);
@@ -2820,8 +2825,10 @@ static int dce_v8_0_resume(void *handle)
 		amdgpu_display_backlight_set_level(adev, adev->mode_info.bl_encoder,
 						    bl_level);
 	}
+	if (ret)
+		return ret;
 
-	return ret;
+	return amdgpu_display_resume_helper(adev);
 }
 
 static bool dce_v8_0_is_idle(void *handle) diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index 9810af712cc0..5c11144da051 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -39,6 +39,7 @@
 #include "dce_v11_0.h"
 #include "dce_virtual.h"
 #include "ivsrcid/ivsrcid_vislands30.h"
+#include "amdgpu_display.h"
 
 #define DCE_VIRTUAL_VBLANK_PERIOD 16666666
 
@@ -491,12 +492,24 @@ static int dce_virtual_hw_fini(void *handle)
 
 static int dce_virtual_suspend(void *handle)  {
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	r = amdgpu_display_suspend_helper(adev);
+	if (r)
+		return r;
 	return dce_virtual_hw_fini(handle);
 }
 
 static int dce_virtual_resume(void *handle)  {
-	return dce_virtual_hw_init(handle);
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int r;
+
+	r = dce_virtual_hw_init(handle);
+	if (r)
+		return r;
+	return amdgpu_display_resume_helper(adev);
 }
 
 static bool dce_virtual_is_idle(void *handle)
--
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%7Cde024782e56349cb659608d8e2b14d42%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637508598435530070%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=blFMxp1nrTITbzjXo9yDFs2LsavBg7aFpgvrZu3FYtE%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] 16+ messages in thread

* RE: [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2)
  2021-03-09  4:10 [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2) Alex Deucher
                   ` (5 preceding siblings ...)
  2021-03-09  4:10 ` [PATCH 7/7] drm/amdgpu: clean up non-DC suspend/resume handling Alex Deucher
@ 2021-03-10  8:28 ` Quan, Evan
  6 siblings, 0 replies; 16+ messages in thread
From: Quan, Evan @ 2021-03-10  8:28 UTC (permalink / raw)
  To: Deucher, Alexander, amd-gfx; +Cc: Deucher, Alexander

[AMD Public Use]

Hi Alex,

By checking the document and source code of pci_pm_prepare(), it seems the prepare callback provided by device driver is optional, not must.
- even without the prepare callback provide by device driver, the ->prepare of subsystem/bus can still return positive number(1)
- the result of  the prepare callback provide by device driver may be not honored unless DPM_FLAG_SMART_PREPARE is set.

BR
Evan
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
Sent: Tuesday, March 9, 2021 12:10 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2)

as per:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Flatest%2Fdriver-api%2Fpm%2Fdevices.html&amp;data=04%7C01%7Cevan.quan%40amd.com%7C6e3b5c960f1145c79e1c08d8e2b14a95%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637508598383549752%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=hLOdKKeO7JMk5kU5gmYSrBD53LhsT8vtEH3UivKjx48%3D&amp;reserved=0

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 aecf7baf219a..8d4fbee01011 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"
@@ -1293,6 +1294,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); @@ -1511,6 +1533,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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cevan.quan%40amd.com%7C6e3b5c960f1145c79e1c08d8e2b14a95%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637508598383559745%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=6PfggZhs61N3RO6DKAODOOfKappXUnswvHRXgSHJ%2FtA%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] 16+ messages in thread

end of thread, other threads:[~2021-03-10 20:00 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-09  4:10 [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2) Alex Deucher
2021-03-09  4:10 ` [PATCH 2/7] drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND flags (v2) Alex Deucher
2021-03-09  4:10 ` [PATCH 3/7] drm/amdgpu: disentangle HG systems from vgaswitcheroo Alex Deucher
2021-03-10  8:42   ` Quan, Evan
2021-03-09  4:10 ` [PATCH 4/7] drm/amdgpu: track what pmops flow we are in Alex Deucher
2021-03-09  6:19   ` Lazar, Lijo
2021-03-09 15:47     ` Alex Deucher
2021-03-09 17:25       ` Bhardwaj, Rajneesh
2021-03-10 10:16         ` Liang, Prike
2021-03-10 13:19           ` Bhardwaj, Rajneesh
2021-03-10 20:00         ` Alex Deucher
2021-03-09  4:10 ` [PATCH 5/7] drm/amdgpu: don't evict vram on APUs for suspend to ram Alex Deucher
2021-03-09  4:10 ` [PATCH 6/7] drm/amdgpu: clean up S0ix logic Alex Deucher
2021-03-09  4:10 ` [PATCH 7/7] drm/amdgpu: clean up non-DC suspend/resume handling Alex Deucher
2021-03-10  9:05   ` Quan, Evan
2021-03-10  8:28 ` [PATCH 1/7] drm/amdgpu: add a dev_pm_ops prepare callback (v2) Quan, Evan

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.