> -----Original Message-----
> From: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> On Behalf Of Alex
> Deucher
> Sent: 2019/November/26, Tuesday 9:51 AM
> To: amd-gfx list <amd-gfx@lists.freedesktop.org>
> Cc: Deucher, Alexander <alexander.deuc...@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: move pci handling out of pm ops
>
> Ping? I've tested this on all the cards I have access to.
>
> Alex
>
> On Thu, Nov 21, 2019 at 11:55 AM Alex Deucher <alexdeuc...@gmail.com>
> wrote:
> >
> > The documentation says the that PCI core handles this for you unless
> > you choose to implement it. Just rely on the PCI core to handle the
> > pci specific bits.
> >
> > Signed-off-by: Alex Deucher <alexander.deuc...@amd.com>
Reviewed-by: Zhan Liu <zhan....@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 +--
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33 +++++++++---------
> ----
> > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 16 +++++------
> > 3 files changed, 24 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index 97843462c2fb..2e9d0be05f2f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -1191,8 +1191,8 @@ int amdgpu_driver_open_kms(struct drm_device
> > *dev, struct drm_file *file_priv); void
> amdgpu_driver_postclose_kms(struct drm_device *dev,
> > struct drm_file *file_priv); int
> > amdgpu_device_ip_suspend(struct amdgpu_device *adev); -int
> > amdgpu_device_suspend(struct drm_device *dev, bool suspend, bool
> > fbcon); -int amdgpu_device_resume(struct drm_device *dev, bool resume,
> > bool fbcon);
> > +int amdgpu_device_suspend(struct drm_device *dev, bool fbcon); int
> > +amdgpu_device_resume(struct drm_device *dev, bool fbcon);
> > u32 amdgpu_get_vblank_counter_kms(struct drm_device *dev, unsigned
> > int pipe); int amdgpu_enable_vblank_kms(struct drm_device *dev,
> > unsigned int pipe); void amdgpu_disable_vblank_kms(struct drm_device
> > *dev, unsigned int pipe); diff --git
> > a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index b1408c5e4640..d832bd22ba9d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -1090,6 +1090,7 @@ static int
> amdgpu_device_check_arguments(struct
> > amdgpu_device *adev) static void amdgpu_switcheroo_set_state(struct
> > pci_dev *pdev, enum vga_switcheroo_state state) {
> > struct drm_device *dev = pci_get_drvdata(pdev);
> > + int r;
> >
> > if (amdgpu_device_supports_boco(dev) && state ==
> VGA_SWITCHEROO_OFF)
> > return;
> > @@ -1099,7 +1100,12 @@ static void amdgpu_switcheroo_set_state(struct
> pci_dev *pdev, enum vga_switchero
> > /* don't suspend or resume card normally */
> > dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
> >
> > - amdgpu_device_resume(dev, true, true);
> > + pci_set_power_state(dev->pdev, PCI_D0);
> > + pci_restore_state(dev->pdev);
> > + r = pci_enable_device(dev->pdev);
> > + if (r)
> > + DRM_WARN("pci_enable_device failed (%d)\n", r);
> > + amdgpu_device_resume(dev, true);
> >
> > dev->switch_power_state = DRM_SWITCH_POWER_ON;
> > drm_kms_helper_poll_enable(dev); @@ -1107,7 +1113,11
> > @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev,
> enum vga_switchero
> > pr_info("amdgpu: switched off\n");
> > drm_kms_helper_poll_disable(dev);
> > dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
> > - amdgpu_device_suspend(dev, true, true);
> > + amdgpu_device_suspend(dev, true);
> > + pci_save_state(dev->pdev);
> > + /* Shut down the device */
> > + pci_disable_device(dev->pdev);
> > + pci_set_power_state(dev->pdev, PCI_D3cold);
> > dev->switch_power_state = DRM_SWITCH_POWER_OFF;
> > }
> > }
> > @@ -3198,7 +3208,7 @@ void amdgpu_device_fini(struct amdgpu_device
> *adev)
> > * Returns 0 for success or an error on failure.
> > * Called at driver suspend.
> > */
> > -int amdgpu_device_suspend(struct drm_device *dev, bool suspend, bool
> > fbcon)
> > +int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)
> > {
> > struct amdgpu_device *adev;
> > struct drm_crtc *crtc;
> > @@ -3281,13 +3291,6 @@ int amdgpu_device_suspend(struct drm_device
> *dev, bool suspend, bool fbcon)
> > */
> > amdgpu_bo_evict_vram(adev);
> >
> > - if (suspend) {
> > - pci_save_state(dev->pdev);
> > - /* Shut down the device */
> > - pci_disable_device(dev->pdev);
> > - pci_set_power_state(dev->pdev, PCI_D3hot);
> > - }
> > -
> > return 0;
> > }
> >
> > @@ -3302,7 +3305,7 @@ int amdgpu_device_suspend(struct drm_device
> *dev, bool suspend, bool fbcon)
> > * Returns 0 for success or an error on failure.
> > * Called at driver resume.
> > */
> > -int amdgpu_device_resume(struct drm_device *dev, bool resume, bool
> > fbcon)
> > +int amdgpu_device_resume(struct drm_device *dev, bool fbcon)
> > {
> > struct drm_connector *connector;
> > struct drm_connector_list_iter iter; @@ -3313,14 +3316,6 @@
> > int amdgpu_device_resume(struct drm_device *dev, bool resume, bool
> fbcon)
> > if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> > return 0;
> >
> > - if (resume) {
> > - pci_set_power_state(dev->pdev, PCI_D0);
> > - pci_restore_state(dev->pdev);
> > - r = pci_enable_device(dev->pdev);
> > - if (r)
> > - return r;
> > - }
> > -
> > /* post card */
> > if (amdgpu_device_need_post(adev)) {
> > r =
> > amdgpu_atom_asic_init(adev->mode_info.atom_context);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index df2f4720a2f0..2f367146c72c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -1158,7 +1158,7 @@ static int amdgpu_pmops_suspend(struct device
> > *dev) {
> > struct drm_device *drm_dev = dev_get_drvdata(dev);
> >
> > - return amdgpu_device_suspend(drm_dev, true, true);
> > + return amdgpu_device_suspend(drm_dev, true);
> > }
> >
> > static int amdgpu_pmops_resume(struct device *dev) @@ -1173,7
> +1173,7
> > @@ static int amdgpu_pmops_resume(struct device *dev)
> > pm_runtime_enable(dev);
> > }
> >
> > - return amdgpu_device_resume(drm_dev, true, true);
> > + return amdgpu_device_resume(drm_dev, true);
> > }
> >
> > static int amdgpu_pmops_freeze(struct device *dev) @@ -1182,7 +1182,7
> > @@ static int amdgpu_pmops_freeze(struct device *dev)
> > struct amdgpu_device *adev = drm_dev->dev_private;
> > int r;
> >
> > - r = amdgpu_device_suspend(drm_dev, false, true);
> > + r = amdgpu_device_suspend(drm_dev, true);
> > if (r)
> > return r;
> > return amdgpu_asic_reset(adev); @@ -1192,21 +1192,21 @@ static
> > int amdgpu_pmops_thaw(struct device *dev) {
> > struct drm_device *drm_dev = dev_get_drvdata(dev);
> >
> > - return amdgpu_device_resume(drm_dev, false, true);
> > + return amdgpu_device_resume(drm_dev, true);
> > }
> >
> > static int amdgpu_pmops_poweroff(struct device *dev) {
> > struct drm_device *drm_dev = dev_get_drvdata(dev);
> >
> > - return amdgpu_device_suspend(drm_dev, true, true);
> > + return amdgpu_device_suspend(drm_dev, true);
> > }
> >
> > static int amdgpu_pmops_restore(struct device *dev) {
> > struct drm_device *drm_dev = dev_get_drvdata(dev);
> >
> > - return amdgpu_device_resume(drm_dev, false, true);
> > + return amdgpu_device_resume(drm_dev, true);
> > }
> >
> > static int amdgpu_pmops_runtime_suspend(struct device *dev) @@
> > -1225,7 +1225,7 @@ static int amdgpu_pmops_runtime_suspend(struct
> device *dev)
> > drm_dev->switch_power_state =
> DRM_SWITCH_POWER_CHANGING;
> > drm_kms_helper_poll_disable(drm_dev);
> >
> > - ret = amdgpu_device_suspend(drm_dev, false, false);
> > + ret = amdgpu_device_suspend(drm_dev, false);
> > if (amdgpu_device_supports_boco(drm_dev)) {
> > /* Only need to handle PCI state in the driver for ATPX
> > * PCI core handles it for _PR3.
> > @@ -1275,7 +1275,7 @@ static int
> amdgpu_pmops_runtime_resume(struct device *dev)
> > } else if (amdgpu_device_supports_baco(drm_dev)) {
> > amdgpu_device_baco_exit(drm_dev);
> > }
> > - ret = amdgpu_device_resume(drm_dev, false, false);
> > + ret = amdgpu_device_resume(drm_dev, false);
> > drm_kms_helper_poll_enable(drm_dev);
> > if (amdgpu_device_supports_boco(drm_dev))
> > drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
> > --
> > 2.23.0
> >
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
