drm/amd/amdgpu: S4 issue for amdgpu
Subsystems (2)
PATCH v2 1/1 drm/amd/amdgpu: S4 issue for amdgpu
reset the asic if adapter is not powerdown when doing freeze() thaw() and restore(), in order to get a valid state of adapter.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 19 +++++++++++++------ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 18 ++++++++++++++++-- 2 files changed, 29 insertions(+), 8 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +13 -6
@@ -1937,6 +1937,10 @@ int amdgpu_device_suspend(struct drm_device *dev, bool suspend, bool fbcon)/* Shut down the device */ pci_disable_device (dev->pdev);pci_set_power_state (dev->pdev, PCI_D3hot); + }else { + r =amdgpu_asic_reset (adev); +if (r) +DRM_ERROR ("amdgpu asic reset failed\n" ); }if (fbcon) {
@@ -1967,22 +1971,25 @@ int amdgpu_device_resume(struct drm_device *dev, bool resume, bool fbcon) dev->switch_power_state == DRM_SWITCH_POWER_DYNAMIC_OFF)return 0 ; -if (fbcon) { +if (fbcon)console_lock (); - } +if (resume) {pci_set_power_state (dev->pdev,PCI_D0 );pci_restore_state (dev->pdev); -if (pci_enable_device (dev->pdev)) { +if (r =pci_enable_device (dev->pdev)) {if (fbcon)console_unlock (); -return -1 ; +return r; } }/* post card */ -if (!amdgpu_card_posted (adev)) -amdgpu_atom_asic_init (adev->mode_info.atom_context); +if (!amdgpu_card_posted (adev) || !resume) { + r =amdgpu_atom_asic_init (adev->mode_info.atom_context); +if (r) +DRM_ERROR ("amdgpu asic init failed\n" ); + } r =amdgpu_resume (adev);if (r)
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +16 -2
@@ -520,6 +520,20 @@ static int amdgpu_pmops_thaw(struct device *dev)return amdgpu_device_resume (drm_dev, false, true); } +static int amdgpu_pmops_poweroff (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev); +return amdgpu_device_suspend (drm_dev, true, true); +} + +static int amdgpu_pmops_restore (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev); +return amdgpu_device_resume (drm_dev, false, true); +} +static int amdgpu_pmops_runtime_suspend (struct device *dev) {struct pci_dev *pdev =to_pci_dev (dev);
@@ -622,8 +636,8 @@ static const struct dev_pm_ops amdgpu_pm_ops = { .resume = amdgpu_pmops_resume, .freeze = amdgpu_pmops_freeze, .thaw = amdgpu_pmops_thaw, - .poweroff = amdgpu_pmops_freeze, - .restore = amdgpu_pmops_resume, + .poweroff = amdgpu_pmops_poweroff, + .restore = amdgpu_pmops_restore, .runtime_suspend = amdgpu_pmops_runtime_suspend, .runtime_resume = amdgpu_pmops_runtime_resume, .runtime_idle = amdgpu_pmops_runtime_idle,
Signature · 6 lines
1.9.1 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): 1472885811-5210-1-git-send-email-Jim.Qu-5C7GfCeVMHo@public.gmane.org
Quoted 10 lines · EDT jimqu
reset the asic if adapter is not powerdown when doing
freeze()thaw()andrestore(), in order to get a valid state of adapter.Change-Id Ibb03ab3c2ce6ef3c83affbe767308dc22474f563Signed-off-by JimQu <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 19 +++++++++++++------ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 18 ++++++++++++++++-- 2 files changed, 29 insertions(+), 8 deletions(-)
Seems ok on CIK here with 4.8-rc5 + drm-next-4.9-wip merged in, I usually hibernate and reboot into Windows sometimes. I ran pm-hibernate manually.
When you set:
echo reboot > /sys/power/disk
Tested By: Shawn Starr <[email protected]>
Quoted 74 lines
(quoted diff) +0
-0
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.cindex 6510514..f17fc6d 100644drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0@@ -1937,6 +1937,10 @@ int amdgpu_device_suspend(struct drm_device *dev,bool suspend, bool fbcon) /* Shut down the device */
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +4
-0pci_disable_device (dev->pdev);pci_set_power_state (dev->pdev, PCI_D3hot); + }else { + r =amdgpu_asic_reset (adev); +if (r) +DRM_ERROR ("amdgpu asic reset failed\n" ); }if (fbcon) {@@ -1967,22 +1971,25 @@ int amdgpu_device_resume(struct drm_device *dev,bool resume, bool fbcon)
dev->switch_power_state==DRM_SWITCH_POWER_DYNAMIC_OFF)drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +9
-6return 0 ; -if (fbcon) { +if (fbcon)console_lock (); - } +if (resume) {pci_set_power_state (dev->pdev,PCI_D0 );pci_restore_state (dev->pdev); -if (pci_enable_device (dev->pdev)) { +if (r =pci_enable_device (dev->pdev)) {if (fbcon)console_unlock (); -return -1 ; +return r; } }/* post card */ -if (!amdgpu_card_posted (adev)) -amdgpu_atom_asic_init (adev->mode_info.atom_context); +if (!amdgpu_card_posted (adev) || !resume) { + r =amdgpu_atom_asic_init (adev->mode_info.atom_context); +if (r) +DRM_ERROR ("amdgpu asic init failed\n" ); + } r =amdgpu_resume (adev);if (r)(quoted diff) +0
-0
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.cindex 21b8cd6..c00f4a8 100644drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +14
-0@@ -520,6 +520,20 @@ static int amdgpu_pmops_thaw(struct device *dev)return amdgpu_device_resume (drm_dev, false, true); } +static int amdgpu_pmops_poweroff (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev); +return amdgpu_device_suspend (drm_dev, true, true); +} + +static int amdgpu_pmops_restore (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev); +return amdgpu_device_resume (drm_dev, false, true); +} +static int amdgpu_pmops_runtime_suspend (struct device *dev) {struct pci_dev *pdev =to_pci_dev (dev);
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c static const struct dev_pm_ops amdgpu_pm_ops = { · line 643
Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +2
-2@@ -622,8 +636,8 @@ static const struct dev_pm_ops amdgpu_pm_ops = { .resume = amdgpu_pmops_resume, .freeze = amdgpu_pmops_freeze, .thaw = amdgpu_pmops_thaw, - .poweroff = amdgpu_pmops_freeze, - .restore = amdgpu_pmops_resume, + .poweroff = amdgpu_pmops_poweroff, + .restore = amdgpu_pmops_restore, .runtime_suspend = amdgpu_pmops_runtime_suspend, .runtime_resume = amdgpu_pmops_runtime_resume, .runtime_idle = amdgpu_pmops_runtime_idle,
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Am 07.09.2016 um 10:08 schrieb Shawn Starr:
Quoted 11 lines · EDT jimqu
Quoted 10 lines
reset the asic if adapter is not powerdown when doing
freeze()thaw()andrestore(), in order to get a valid state of adapter.Change-Id Ibb03ab3c2ce6ef3c83affbe767308dc22474f563Signed-off-by JimQu <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 19 +++++++++++++------ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 18 ++++++++++++++++-- 2 files changed, 29 insertions(+), 8 deletions(-)
Quoted 7 lines
Seems ok on CIK here with 4.8-rc5 + drm-next-4.9-wip merged in, I usually hibernate and reboot into Windows sometimes. I ran pm-hibernate manually.
When you set:
echo reboot >/sys/power/diskTested By: Shawn Starr <[email protected]>
I unfortunately don't have the slightest idea how this part of PM works and Alex is on vacation. But it sounds important so let's try to get this into out branches ASAP.
One minor nit pick below, apart from that looks good to me.
Quoted 57 lines
Quoted 57 lines
(quoted diff) +0
-0
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.cindex 6510514..f17fc6d 100644drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0@@ -1937,6 +1937,10 @@ int amdgpu_device_suspend(struct drm_device *dev,bool suspend, bool fbcon) /* Shut down the device */
pci_disable_device(dev->pdev);pci_set_power_state(dev->pdev, PCI_D3hot);drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +4
-0+ }else { + r =amdgpu_asic_reset (adev); +if (r) +DRM_ERROR ("amdgpu asic reset failed\n" ); }if (fbcon) {@@ -1967,22 +1971,25 @@ int amdgpu_device_resume(struct drm_device *dev,bool resume, bool fbcon)
dev->switch_power_state==DRM_SWITCH_POWER_DYNAMIC_OFF)
return 0;drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +9
-6-if (fbcon) { +if (fbcon)console_lock (); - } +if (resume) {pci_set_power_state (dev->pdev,PCI_D0 );pci_restore_state (dev->pdev); -if (pci_enable_device (dev->pdev)) { +if (r =pci_enable_device (dev->pdev)) {if (fbcon)console_unlock (); -return -1 ; +return r; } }/* post card */ -if (!amdgpu_card_posted (adev)) -amdgpu_atom_asic_init (adev->mode_info.atom_context); +if (!amdgpu_card_posted (adev) || !resume) { + r =amdgpu_atom_asic_init (adev->mode_info.atom_context); +if (r) +DRM_ERROR ("amdgpu asic init failed\n" ); + } r =amdgpu_resume (adev);if (r)(quoted diff) +0
-0
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.cindex 21b8cd6..c00f4a8 100644drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +0
-0@@ -520,6 +520,20 @@ static int amdgpu_pmops_thaw(struct device *dev)return amdgpu_device_resume (drm_dev, false, true); }
Quoted 4 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c static int amdgpu_pmops_thaw(struct device *dev) · line 526
Quoted 4 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +4
-0+static int amdgpu_pmops_poweroff (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev);
Please add an empty line between the declaration and code.
With that fixed the patch is Acked-by: Christian König <[email protected]>.
Regards,
Christian.
Quoted 21 lines
Quoted 21 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +12
-2+return amdgpu_device_suspend (drm_dev, true, true); +} + +static int amdgpu_pmops_restore (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev); +return amdgpu_device_resume (drm_dev, false, true); +} +static int amdgpu_pmops_runtime_suspend (struct device *dev) {struct pci_dev *pdev =to_pci_dev (dev);@@ -622,8 +636,8 @@ static const struct dev_pm_ops amdgpu_pm_ops = { .resume = amdgpu_pmops_resume, .freeze = amdgpu_pmops_freeze, .thaw = amdgpu_pmops_thaw, - .poweroff = amdgpu_pmops_freeze, - .restore = amdgpu_pmops_resume, + .poweroff = amdgpu_pmops_poweroff, + .restore = amdgpu_pmops_restore,
Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c static const struct dev_pm_ops amdgpu_pm_ops = { · line 643
Quoted 3 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +0
-0.runtime_suspend = amdgpu_pmops_runtime_suspend, .runtime_resume = amdgpu_pmops_runtime_resume, .runtime_idle = amdgpu_pmops_runtime_idle,_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): 6acb4940-7f20-9e8f-c001-0dae7b707460-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org
Thanks to guys, I will correct and push it.
Thanks
JimQu
________________________________________
发件人: amd-gfx <[email protected]> 代表 Christian König <[email protected]> 发送时间: 2016年9月7日 16:15:03
收件人: Shawn Starr; [email protected]
抄送: Qu, Jim
主题: Re: [PATCH v2] drm/amd/amdgpu: S4 issue for amdgpu
Am 07.09.2016 um 10:08 schrieb Shawn Starr:
Quoted 11 lines · EDT jimqu
Quoted 10 lines
reset the asic if adapter is not powerdown when doing
freeze()thaw()andrestore(), in order to get a valid state of adapter.Change-Id Ibb03ab3c2ce6ef3c83affbe767308dc22474f563Signed-off-by JimQu <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 19 +++++++++++++------ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 18 ++++++++++++++++-- 2 files changed, 29 insertions(+), 8 deletions(-)
Quoted 7 lines
Seems ok on CIK here with 4.8-rc5 + drm-next-4.9-wip merged in, I usually hibernate and reboot into Windows sometimes. I ran pm-hibernate manually.
When you set:
echo reboot >/sys/power/diskTested By: Shawn Starr <[email protected]>
I unfortunately don't have the slightest idea how this part of PM works and Alex is on vacation. But it sounds important so let's try to get this into out branches ASAP.
One minor nit pick below, apart from that looks good to me.
Quoted 57 lines
Quoted 57 lines
(quoted diff) +0
-0
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.cindex 6510514..f17fc6d 100644drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0@@ -1937,6 +1937,10 @@ int amdgpu_device_suspend(struct drm_device *dev,bool suspend, bool fbcon) /* Shut down the device */
pci_disable_device(dev->pdev); pci_set_power_state(dev->pdev, PCI_D3hot);drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +4
-0+ }else { + r =amdgpu_asic_reset (adev); +if (r) +DRM_ERROR ("amdgpu asic reset failed\n" ); }if (fbcon) {@@ -1967,22 +1971,25 @@ int amdgpu_device_resume(struct drm_device *dev,bool resume, bool fbcon)
dev->switch_power_state==DRM_SWITCH_POWER_DYNAMIC_OFF)return 0;drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +9
-6-if (fbcon) { +if (fbcon)console_lock (); - } +if (resume) {pci_set_power_state (dev->pdev,PCI_D0 );pci_restore_state (dev->pdev); -if (pci_enable_device (dev->pdev)) { +if (r =pci_enable_device (dev->pdev)) {if (fbcon)console_unlock (); -return -1 ; +return r; } }/* post card */ -if (!amdgpu_card_posted (adev)) -amdgpu_atom_asic_init (adev->mode_info.atom_context); +if (!amdgpu_card_posted (adev) || !resume) { + r =amdgpu_atom_asic_init (adev->mode_info.atom_context); +if (r) +DRM_ERROR ("amdgpu asic init failed\n" ); + } r =amdgpu_resume (adev);if (r)(quoted diff) +0
-0
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.cindex 21b8cd6..c00f4a8 100644drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +0
-0@@ -520,6 +520,20 @@ static int amdgpu_pmops_thaw(struct device *dev)return amdgpu_device_resume (drm_dev, false, true); }
Quoted 4 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c static int amdgpu_pmops_thaw(struct device *dev) · line 526
Quoted 4 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +4
-0+static int amdgpu_pmops_poweroff (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev);
Please add an empty line between the declaration and code.
With that fixed the patch is Acked-by: Christian König <[email protected]>.
Regards,
Christian.
Quoted 21 lines
Quoted 21 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +12
-2+return amdgpu_device_suspend (drm_dev, true, true); +} + +static int amdgpu_pmops_restore (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev); +return amdgpu_device_resume (drm_dev, false, true); +} +static int amdgpu_pmops_runtime_suspend (struct device *dev) {struct pci_dev *pdev =to_pci_dev (dev);@@ -622,8 +636,8 @@ static const struct dev_pm_ops amdgpu_pm_ops = { .resume = amdgpu_pmops_resume, .freeze = amdgpu_pmops_freeze, .thaw = amdgpu_pmops_thaw, - .poweroff = amdgpu_pmops_freeze, - .restore = amdgpu_pmops_resume, + .poweroff = amdgpu_pmops_poweroff, + .restore = amdgpu_pmops_restore,
Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c static const struct dev_pm_ops amdgpu_pm_ops = { · line 643
Quoted 3 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +0
-0.runtime_suspend = amdgpu_pmops_runtime_suspend, .runtime_resume = amdgpu_pmops_runtime_resume, .runtime_idle = amdgpu_pmops_runtime_idle,_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Quoted 25 lines
-----Original Message-----
From: amd-gfx [mailto:[email protected]] On Behalf Of jimqu
Sent: Saturday, September 03, 2016 2:57 AM
To: [email protected]
Cc: Qu, Jim
Subject: [PATCH v2] drm/amd/amdgpu: S4 issue for amdgpureset the asic if adapter is not powerdown when doing
freeze()thaw()andrestore(), in order to get a valid state of adapter.Change-Id Ibb03ab3c2ce6ef3c83affbe767308dc22474f563Signed-off-by JimQu <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 19 +++++++++++++-----(quoted diff) +0
-1- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 18 ++++++++++++++++-- 2 files changed, 29 insertions(+), 8 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c int amdgpu_device_suspend(struct drm_device · line 1944
Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +4
-0@@ -1937,6 +1937,10 @@ int amdgpu_device_suspend(struct drm_device *dev, bool suspend, bool fbcon)/* Shut down the device */ pci_disable_device (dev->pdev);pci_set_power_state (dev->pdev, PCI_D3hot); + }else { + r =amdgpu_asic_reset (adev); +if (r) +DRM_ERROR ("amdgpu asic reset failed\n" );
Do we want to guard this with a separate parameter? Are there use cases where we may not want D3 or GPU reset? Runtime pm for PX comes to mind. That said, it probably won't hurt in that case either.
Alex
Quoted 78 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0}drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +9
-6if (fbcon) {@@ -1967,22 +1971,25 @@ int amdgpu_device_resume(struct drm_device *dev, bool resume, bool fbcon) dev->switch_power_state == DRM_SWITCH_POWER_DYNAMIC_OFF) return 0; -if (fbcon) { +if (fbcon)console_lock (); - } +if (resume) {pci_set_power_state (dev->pdev,PCI_D0 );pci_restore_state (dev->pdev); -if (pci_enable_device (dev->pdev)) { +if (r =pci_enable_device (dev->pdev)) {if (fbcon)console_unlock (); -return -1 ; +return r; } }/* post card */ -if (!amdgpu_card_posted (adev)) -amdgpu_atom_asic_init (adev->mode_info.atom_context); +if (!amdgpu_card_posted (adev) || !resume) { + r =amdgpu_atom_asic_init (adev- >mode_info.atom_context); +if (r) +DRM_ERROR ("amdgpu asic init failed\n" ); + }drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0r =amdgpu_resume (adev);if (r)drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +16
-3@@ -520,6 +520,20 @@ static int amdgpu_pmops_thaw(struct device *dev)return amdgpu_device_resume (drm_dev, false, true); } +static int amdgpu_pmops_poweroff (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev); +return amdgpu_device_suspend (drm_dev, true, true); +} + +static int amdgpu_pmops_restore (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev); +return amdgpu_device_resume (drm_dev, false, true); +} +static int amdgpu_pmops_runtime_suspend (struct device *dev) {struct pci_dev *pdev =to_pci_dev (dev);@@ -622,8 +636,8 @@ static const struct dev_pm_ops amdgpu_pm_ops = { .resume = amdgpu_pmops_resume, .freeze = amdgpu_pmops_freeze, .thaw = amdgpu_pmops_thaw, - .poweroff = amdgpu_pmops_freeze, - .restore = amdgpu_pmops_resume, + .poweroff = amdgpu_pmops_poweroff, + .restore = amdgpu_pmops_restore, .runtime_suspend = amdgpu_pmops_runtime_suspend, .runtime_resume = amdgpu_pmops_runtime_resume, .runtime_idle = amdgpu_pmops_runtime_idle, --1.9.1
Quoted 4 lines
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): MWHPR12MB1694BD707781FE81E77E88C9F7F80-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org
Hi Alex:
According to my understanding, in our driver, amdgpu_device_suspend/resume. the paremeter 'suspend' or 'resume' mean the apdpter is set to D3, and we need power on it at resume. In S4 , the function freeze() , thraw() , poweroff() and restore() is special for hibernation, S3 process could not call these functions. so far, according to my experiment, we must reset asic if adapter is not power down in S4, otherwise, it will be GFX(compute) , SDMA ring test failure on VI(not sure it will be on SI), it seems that aisc reset is not must on CI.
I do not review our runtime PM code, but, based on currently code , I can be sure that asic must be reset if it is suspended without powerdown.
BTW, driver dpm does not work when resume back after hibernation on CI.
Thanks
JimQu
________________________________________
发件人: Deucher, Alexander
发送时间: 2016年9月7日 19:15:22
收件人: Qu, Jim; [email protected] 抄送: Qu, Jim
主题: RE: [PATCH v2] drm/amd/amdgpu: S4 issue for amdgpu
Quoted 25 lines
-----Original Message-----
From: amd-gfx [mailto:[email protected]] On Behalf Of jimqu
Sent: Saturday, September 03, 2016 2:57 AM
To: [email protected]
Cc: Qu, Jim
Subject: [PATCH v2] drm/amd/amdgpu: S4 issue for amdgpureset the asic if adapter is not powerdown when doing
freeze()thaw()andrestore(), in order to get a valid state of adapter.Change-Id Ibb03ab3c2ce6ef3c83affbe767308dc22474f563Signed-off-by JimQu <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 19 +++++++++++++-----(quoted diff) +0
-1- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 18 ++++++++++++++++-- 2 files changed, 29 insertions(+), 8 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c int amdgpu_device_suspend(struct drm_device · line 1944
Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +4
-0@@ -1937,6 +1937,10 @@ int amdgpu_device_suspend(struct drm_device *dev, bool suspend, bool fbcon)/* Shut down the device */ pci_disable_device (dev->pdev);pci_set_power_state (dev->pdev, PCI_D3hot); + }else { + r =amdgpu_asic_reset (adev); +if (r) +DRM_ERROR ("amdgpu asic reset failed\n" );
Do we want to guard this with a separate parameter? Are there use cases where we may not want D3 or GPU reset? Runtime pm for PX comes to mind. That said, it probably won't hurt in that case either.
Alex
Quoted 78 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0}if (fbcon) {drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +9
-6@@ -1967,22 +1971,25 @@ int amdgpu_device_resume(struct drm_device *dev, bool resume, bool fbcon) dev->switch_power_state == DRM_SWITCH_POWER_DYNAMIC_OFF) return 0; -if (fbcon) { +if (fbcon)console_lock (); - } +if (resume) {pci_set_power_state (dev->pdev,PCI_D0 );pci_restore_state (dev->pdev); -if (pci_enable_device (dev->pdev)) { +if (r =pci_enable_device (dev->pdev)) {if (fbcon)console_unlock (); -return -1 ; +return r; } }/* post card */ -if (!amdgpu_card_posted (adev)) -amdgpu_atom_asic_init (adev->mode_info.atom_context); +if (!amdgpu_card_posted (adev) || !resume) { + r =amdgpu_atom_asic_init (adev- >mode_info.atom_context); +if (r) +DRM_ERROR ("amdgpu asic init failed\n" ); + }r = amdgpu_resume(adev); if (r)drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +16
-3@@ -520,6 +520,20 @@ static int amdgpu_pmops_thaw(struct device *dev)return amdgpu_device_resume (drm_dev, false, true); } +static int amdgpu_pmops_poweroff (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev); +return amdgpu_device_suspend (drm_dev, true, true); +} + +static int amdgpu_pmops_restore (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev); +return amdgpu_device_resume (drm_dev, false, true); +} +static int amdgpu_pmops_runtime_suspend (struct device *dev) {struct pci_dev *pdev =to_pci_dev (dev);@@ -622,8 +636,8 @@ static const struct dev_pm_ops amdgpu_pm_ops = { .resume = amdgpu_pmops_resume, .freeze = amdgpu_pmops_freeze, .thaw = amdgpu_pmops_thaw, - .poweroff = amdgpu_pmops_freeze, - .restore = amdgpu_pmops_resume, + .poweroff = amdgpu_pmops_poweroff, + .restore = amdgpu_pmops_restore, .runtime_suspend = amdgpu_pmops_runtime_suspend, .runtime_resume = amdgpu_pmops_runtime_resume, .runtime_idle = amdgpu_pmops_runtime_idle, --1.9.1
Quoted 4 lines
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): BY2PR12MB061459E8F4979DB09440D42B99FB0-K//h7OWB4q5SmL+xRiD5tgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org
Quoted 17 lines
-----Original Message-----
From: Qu, Jim
Sent: Thursday, September 08, 2016 4:51 AM
To: Deucher, Alexander; [email protected]
Subject: 答复: [PATCH v2] drm/amd/amdgpu: S4 issue for amdgpuHi Alex:
According to my understanding, in our driver, amdgpu_device_suspend/resume. the paremeter 'suspend' or 'resume' mean the apdpter is set to D3, and we need power on it at resume. In S4 , the function
freeze(),thraw(),poweroff()andrestore()is special for hibernation, S3 process could not call these functions. so far, according to my experiment, we must reset asic if adapter is not power down in S4, otherwise, it will be GFX(compute) , SDMA ring test failure on VI(not sure it will be on SI), it seems that aisc reset is not must on CI.
Quoted 2 lines
I do not review our runtime PM code, but, based on currently code , I can be sure that asic must be reset if it is suspended without powerdown.
I don't think it will make a difference since the GPU will be powered down anyway via ACPI, but it's worth checking.
Quoted 2 lines
BTW, driver dpm does not work when resume back after hibernation on CI.
We may need to adjust the SMC setup logic to see if the firmware is already loaded or not similar to what we do for VI. I'm not too familiar with how the SMC interacts with a GPU reset since the SMC is the one actually doing the reset on CI and newer.
Alex
Quoted 112 lines
Thanks
JimQu________________________________________
发件人: Deucher, Alexander
发送时间: 2016年9月7日 19:15:22
收件人: Qu, Jim; [email protected] 抄送: Qu, Jim
主题: RE: [PATCH v2] drm/amd/amdgpu: S4 issue for amdgpuQuoted 8 lines
-----Original Message-----
From: amd-gfx [mailto:[email protected]] On Behalf Of jimqu
Sent: Saturday, September 03, 2016 2:57 AM
To: [email protected]
Cc: Qu, Jim
Subject: [PATCH v2] drm/amd/amdgpu: S4 issue for amdgpuQuoted 7 lines
reset the asic if adapter is not powerdown when doing
freeze()thaw()andrestore(), in order to get a valid state of adapter.Change-Id Ibb03ab3c2ce6ef3c83affbe767308dc22474f563Signed-off-by JimQu <[email protected]>--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 19 +++++++++++++-----
Quoted 2 lines
-
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 18++++++++++++++++--
Quoted 8 lines
2 files changed, 29 insertions(+), 8 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0drivers/gpu/drm/amd/amdgpu/amdgpu_device.c int amdgpu_device_suspend(struct drm_device · line 1944
Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +4
-0@@ -1937,6 +1937,10 @@ int amdgpu_device_suspend(struct drm_device *dev, bool suspend, bool fbcon)/* Shut down the device */ pci_disable_device (dev->pdev);pci_set_power_state (dev->pdev, PCI_D3hot); + }else { + r =amdgpu_asic_reset (adev); +if (r) +DRM_ERROR ("amdgpu asic reset failed\n" );Do we want to guard this with a separate parameter? Are there use cases
where we may not want D3 or GPU reset? Runtime pm for PX comes to mind. That said, it probably won't hurt in that case either.Alex
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c static int amdgpu_pmops_thaw(struct device
Quoted 44 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0}if (fbcon) {drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +9
-6@@ -1967,22 +1971,25 @@ int amdgpu_device_resume(struct drm_device *dev, bool resume, bool fbcon) dev->switch_power_state == DRM_SWITCH_POWER_DYNAMIC_OFF) return 0; -if (fbcon) { +if (fbcon)console_lock (); - } +if (resume) {pci_set_power_state (dev->pdev,PCI_D0 );pci_restore_state (dev->pdev); -if (pci_enable_device (dev->pdev)) { +if (r =pci_enable_device (dev->pdev)) {if (fbcon)console_unlock (); -return -1 ; +return r; } }/* post card */ -if (!amdgpu_card_posted (adev)) -amdgpu_atom_asic_init (adev->mode_info.atom_context); +if (!amdgpu_card_posted (adev) || !resume) { + r =amdgpu_atom_asic_init (adev- >mode_info.atom_context); +if (r) +DRM_ERROR ("amdgpu asic init failed\n" ); + }r = amdgpu_resume(adev); if (r)drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +0
-0@@ -520,6 +520,20 @@ static int amdgpu_pmops_thaw(struct device*dev)
Quoted 14 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +11
-0return amdgpu_device_resume (drm_dev, false, true); } +static int amdgpu_pmops_poweroff (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev); +return amdgpu_device_suspend (drm_dev, true, true); +} + +static int amdgpu_pmops_restore (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +struct drm_device *drm_dev =pci_get_drvdata (pdev);
Quoted 25 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c static const struct dev_pm_ops amdgpu_pm_ops =
Quoted 7 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +3
-0+return amdgpu_device_resume (drm_dev, false, true); +} +static int amdgpu_pmops_runtime_suspend (struct device *dev) {struct pci_dev *pdev =to_pci_dev (dev);@@ -622,8 +636,8 @@ static const struct dev_pm_ops amdgpu_pm_ops ={
Quoted 17 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +2
-3.resume = amdgpu_pmops_resume, .freeze = amdgpu_pmops_freeze, .thaw = amdgpu_pmops_thaw, - .poweroff = amdgpu_pmops_freeze, - .restore = amdgpu_pmops_resume, + .poweroff = amdgpu_pmops_poweroff, + .restore = amdgpu_pmops_restore, .runtime_suspend = amdgpu_pmops_runtime_suspend, .runtime_resume = amdgpu_pmops_runtime_resume, .runtime_idle = amdgpu_pmops_runtime_idle, --1.9.1
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx