hybrid graphics in this case refers to systems which use the new platform d3 cold ACPI methods as opposed to ATPX for dGPU power control.
Signed-off-by: Alex Deucher alexander.deucher@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 19d15dc..c6b5ce3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -2394,10 +2394,12 @@ bool amdgpu_device_is_px(struct drm_device *dev); void amdgpu_register_atpx_handler(void); void amdgpu_unregister_atpx_handler(void); bool amdgpu_has_atpx_dgpu_power_cntl(void); +bool amdgpu_is_atpx_hybrid(void); #else static inline void amdgpu_register_atpx_handler(void) {} static inline void amdgpu_unregister_atpx_handler(void) {} static inline bool amdgpu_has_atpx_dgpu_power_cntl(void) { return false; } +static inline bool amdgpu_is_atpx_hybrid(void) { return false; } #endif
/* diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c index 90dfedc..3e973c7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c @@ -28,6 +28,7 @@ struct amdgpu_atpx_functions { struct amdgpu_atpx { acpi_handle handle; struct amdgpu_atpx_functions functions; + bool is_hybrid; };
static struct amdgpu_atpx_priv { @@ -68,6 +69,10 @@ bool amdgpu_has_atpx_dgpu_power_cntl(void) { return amdgpu_atpx_priv.atpx.functions.power_cntl; }
+bool amdgpu_is_atpx_hybrid(void) { + return amdgpu_atpx_priv.atpx.is_hybrid; +} + /** * amdgpu_atpx_call - call an ATPX method * @@ -192,9 +197,11 @@ static int amdgpu_atpx_validate(struct amdgpu_atpx *atpx) ATPX_DYNAMIC_DGPU_POWER_OFF_SUPPORTED)) atpx->functions.power_cntl = true;
+ atpx->is_hybrid = false; if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) { - printk("Hybrid Graphics, ATPX dGPU power cntl disabled\n"); + printk("ATPX Hybrid Graphics\n"); atpx->functions.power_cntl = false; + atpx->is_hybrid = true; }
return 0;
The platform d3 cold is used to power down the dGPU.
Signed-off-by: Alex Deucher alexander.deucher@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 7e49bf4..6c38901 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -417,7 +417,9 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev) pci_save_state(pdev); pci_disable_device(pdev); pci_ignore_hotplug(pdev); - if (amdgpu_has_atpx_dgpu_power_cntl()) + if (amdgpu_is_atpx_hybrid()) + pci_set_power_state(pdev, PCI_D3cold); + else if (amdgpu_has_atpx_dgpu_power_cntl()) pci_set_power_state(pdev, PCI_D3cold); else pci_set_power_state(pdev, PCI_D3hot);
The ATPX power control method does this for you.
Signed-off-by: Alex Deucher alexander.deucher@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 6c38901..e2bf4ef 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -419,9 +419,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev) pci_ignore_hotplug(pdev); if (amdgpu_is_atpx_hybrid()) pci_set_power_state(pdev, PCI_D3cold); - else if (amdgpu_has_atpx_dgpu_power_cntl()) - pci_set_power_state(pdev, PCI_D3cold); - else + else if (!amdgpu_has_atpx_dgpu_power_cntl()) pci_set_power_state(pdev, PCI_D3hot); drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;
@@ -439,7 +437,9 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
- pci_set_power_state(pdev, PCI_D0); + if (amdgpu_is_atpx_hybrid() || + !amdgpu_has_atpx_dgpu_power_cntl()) + pci_set_power_state(pdev, PCI_D0); pci_restore_state(pdev); ret = pci_enable_device(pdev); if (ret)
On Thu, Jun 02, 2016 at 09:33:34AM -0400, Alex Deucher wrote:
The ATPX power control method does this for you.
Signed-off-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 6c38901..e2bf4ef 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -419,9 +419,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev) pci_ignore_hotplug(pdev); if (amdgpu_is_atpx_hybrid()) pci_set_power_state(pdev, PCI_D3cold);
- else if (amdgpu_has_atpx_dgpu_power_cntl())
pci_set_power_state(pdev, PCI_D3cold);- else
- else if (!amdgpu_has_atpx_dgpu_power_cntl())
You're removing code that you added yesterday: https://lists.freedesktop.org/archives/dri-devel/2016-June/109245.html
Why are you doing this?
Lukas
pci_set_power_state(pdev, PCI_D3hot);drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;
@@ -439,7 +437,9 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
- pci_set_power_state(pdev, PCI_D0);
- if (amdgpu_is_atpx_hybrid() ||
!amdgpu_has_atpx_dgpu_power_cntl())pci_set_power_state(pdev, PCI_D0);-- 2.5.5
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jun 2, 2016 at 3:23 PM, Lukas Wunner lukas@wunner.de wrote:
On Thu, Jun 02, 2016 at 09:33:34AM -0400, Alex Deucher wrote:
The ATPX power control method does this for you.
Signed-off-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 6c38901..e2bf4ef 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -419,9 +419,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev) pci_ignore_hotplug(pdev); if (amdgpu_is_atpx_hybrid()) pci_set_power_state(pdev, PCI_D3cold);
else if (amdgpu_has_atpx_dgpu_power_cntl())pci_set_power_state(pdev, PCI_D3cold);else
else if (!amdgpu_has_atpx_dgpu_power_cntl())You're removing code that you added yesterday: https://lists.freedesktop.org/archives/dri-devel/2016-June/109245.html
Why are you doing this?
To avoid changing behavior in intermediate steps to avoid breaking multiple platforms in one patch. The original behavior before this patch set was set d3cold unconditionally. The previous change set d3hot for systems without ATPX power control, and finally this patch removes setting d3cold for systems with ATPX power control.
Alex
Lukas
pci_set_power_state(pdev, PCI_D3hot); drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;@@ -439,7 +437,9 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
pci_set_power_state(pdev, PCI_D0);
if (amdgpu_is_atpx_hybrid() ||!amdgpu_has_atpx_dgpu_power_cntl())pci_set_power_state(pdev, PCI_D0); pci_restore_state(pdev); ret = pci_enable_device(pdev); if (ret)-- 2.5.5
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jun 02, 2016 at 03:26:18PM -0400, Alex Deucher wrote:
On Thu, Jun 2, 2016 at 3:23 PM, Lukas Wunner lukas@wunner.de wrote:
On Thu, Jun 02, 2016 at 09:33:34AM -0400, Alex Deucher wrote:
The ATPX power control method does this for you.
Signed-off-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 6c38901..e2bf4ef 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -419,9 +419,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev) pci_ignore_hotplug(pdev); if (amdgpu_is_atpx_hybrid()) pci_set_power_state(pdev, PCI_D3cold);
else if (amdgpu_has_atpx_dgpu_power_cntl())pci_set_power_state(pdev, PCI_D3cold);else
else if (!amdgpu_has_atpx_dgpu_power_cntl())You're removing code that you added yesterday: https://lists.freedesktop.org/archives/dri-devel/2016-June/109245.html
Why are you doing this?
To avoid changing behavior in intermediate steps to avoid breaking multiple platforms in one patch. The original behavior before this patch set was set d3cold unconditionally. The previous change set d3hot for systems without ATPX power control, and finally this patch removes setting d3cold for systems with ATPX power control.
Thank you for the explanation, I was trying to make sense of this but kept scratching my head. It would be good to have the explanation in the commit message(s), they're fairly terse. (In this patch it's just a one-liner.)
Best regards,
Lukas
pci_set_power_state(pdev, PCI_D3hot); drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;@@ -439,7 +437,9 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
pci_set_power_state(pdev, PCI_D0);
if (amdgpu_is_atpx_hybrid() ||!amdgpu_has_atpx_dgpu_power_cntl())pci_set_power_state(pdev, PCI_D0); pci_restore_state(pdev); ret = pci_enable_device(pdev); if (ret)-- 2.5.5
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
hybrid graphics in this case refers to systems which use the new platform d3 cold ACPI methods as opposed to ATPX for dGPU power control.
Signed-off-by: Alex Deucher alexander.deucher@amd.com --- drivers/gpu/drm/radeon/radeon_atpx_handler.c | 9 ++++++++- drivers/gpu/drm/radeon/radeon_drv.c | 2 ++ 2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/radeon_atpx_handler.c b/drivers/gpu/drm/radeon/radeon_atpx_handler.c index 6996b31..de17b5e 100644 --- a/drivers/gpu/drm/radeon/radeon_atpx_handler.c +++ b/drivers/gpu/drm/radeon/radeon_atpx_handler.c @@ -28,6 +28,7 @@ struct radeon_atpx_functions { struct radeon_atpx { acpi_handle handle; struct radeon_atpx_functions functions; + bool is_hybrid; };
static struct radeon_atpx_priv { @@ -67,6 +68,10 @@ bool radeon_has_atpx_dgpu_power_cntl(void) { return radeon_atpx_priv.atpx.functions.power_cntl; }
+bool radeon_is_atpx_hybrid(void) { + return radeon_atpx_priv.atpx.is_hybrid; +} + /** * radeon_atpx_call - call an ATPX method * @@ -190,9 +195,11 @@ static int radeon_atpx_validate(struct radeon_atpx *atpx) ATPX_DYNAMIC_DGPU_POWER_OFF_SUPPORTED)) atpx->functions.power_cntl = true;
+ atpx->is_hybrid = false; if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) { - printk("Hybrid Graphics, ATPX dGPU power cntl disabled\n"); + printk("ATPX Hybrid Graphics\n"); atpx->functions.power_cntl = false; + atpx->is_hybrid = true; }
return 0; diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index ec80050..af52f10 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -166,10 +166,12 @@ void radeon_debugfs_cleanup(struct drm_minor *minor); void radeon_register_atpx_handler(void); void radeon_unregister_atpx_handler(void); bool radeon_has_atpx_dgpu_power_cntl(void); +bool radeon_is_atpx_hybrid(void); #else static inline void radeon_register_atpx_handler(void) {} static inline void radeon_unregister_atpx_handler(void) {} static inline bool radeon_has_atpx_dgpu_power_cntl(void) { return false; } +static inline bool radeon_is_atpx_hybrid(void) { return false; } #endif
int radeon_no_wb;
The platform d3 cold is used to power down the dGPU.
Signed-off-by: Alex Deucher alexander.deucher@amd.com --- drivers/gpu/drm/radeon/radeon_drv.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index af52f10..f453450 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -417,7 +417,9 @@ static int radeon_pmops_runtime_suspend(struct device *dev) pci_save_state(pdev); pci_disable_device(pdev); pci_ignore_hotplug(pdev); - if (radeon_has_atpx_dgpu_power_cntl()) + if (radeon_is_atpx_hybrid()) + pci_set_power_state(pdev, PCI_D3cold); + else if (radeon_has_atpx_dgpu_power_cntl()) pci_set_power_state(pdev, PCI_D3cold); else pci_set_power_state(pdev, PCI_D3hot);
The ATPX power control method does this for you.
Signed-off-by: Alex Deucher alexander.deucher@amd.com --- drivers/gpu/drm/radeon/radeon_drv.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index f453450..56d15e6 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -419,9 +419,7 @@ static int radeon_pmops_runtime_suspend(struct device *dev) pci_ignore_hotplug(pdev); if (radeon_is_atpx_hybrid()) pci_set_power_state(pdev, PCI_D3cold); - else if (radeon_has_atpx_dgpu_power_cntl()) - pci_set_power_state(pdev, PCI_D3cold); - else + else if (!radeon_has_atpx_dgpu_power_cntl()) pci_set_power_state(pdev, PCI_D3hot); drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;
@@ -439,7 +437,9 @@ static int radeon_pmops_runtime_resume(struct device *dev)
drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
- pci_set_power_state(pdev, PCI_D0); + if (radeon_is_atpx_hybrid() || + !radeon_has_atpx_dgpu_power_cntl()) + pci_set_power_state(pdev, PCI_D0); pci_restore_state(pdev); ret = pci_enable_device(pdev); if (ret)
Am 02.06.2016 um 15:33 schrieb Alex Deucher:
hybrid graphics in this case refers to systems which use the new platform d3 cold ACPI methods as opposed to ATPX for dGPU power control.
Signed-off-by: Alex Deucher alexander.deucher@amd.com
Acked-by: Christian König christian.koenig@amd.com
I'm wondering if I should get deeper into this?
Regards, Christian.
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 19d15dc..c6b5ce3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -2394,10 +2394,12 @@ bool amdgpu_device_is_px(struct drm_device *dev); void amdgpu_register_atpx_handler(void); void amdgpu_unregister_atpx_handler(void); bool amdgpu_has_atpx_dgpu_power_cntl(void); +bool amdgpu_is_atpx_hybrid(void); #else static inline void amdgpu_register_atpx_handler(void) {} static inline void amdgpu_unregister_atpx_handler(void) {} static inline bool amdgpu_has_atpx_dgpu_power_cntl(void) { return false; } +static inline bool amdgpu_is_atpx_hybrid(void) { return false; } #endif
/* diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c index 90dfedc..3e973c7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c @@ -28,6 +28,7 @@ struct amdgpu_atpx_functions { struct amdgpu_atpx { acpi_handle handle; struct amdgpu_atpx_functions functions;
bool is_hybrid; };
static struct amdgpu_atpx_priv {
@@ -68,6 +69,10 @@ bool amdgpu_has_atpx_dgpu_power_cntl(void) { return amdgpu_atpx_priv.atpx.functions.power_cntl; }
+bool amdgpu_is_atpx_hybrid(void) {
- return amdgpu_atpx_priv.atpx.is_hybrid;
+}
- /**
- amdgpu_atpx_call - call an ATPX method
@@ -192,9 +197,11 @@ static int amdgpu_atpx_validate(struct amdgpu_atpx *atpx) ATPX_DYNAMIC_DGPU_POWER_OFF_SUPPORTED)) atpx->functions.power_cntl = true;
- atpx->is_hybrid = false; if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
printk("Hybrid Graphics, ATPX dGPU power cntl disabled\n");
printk("ATPX Hybrid Graphics\n");atpx->functions.power_cntl = false;
atpx->is_hybrid = true;}
return 0;
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Christian König -
Lukas Wunner