| Message ID | 1464874417-27272-3-git-send-email-alexander.deucher@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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); > 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 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
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)
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(-)