[PATCH 4/7] drm/amdgpu: track what pmops flow we are in
Alex Deucher
alexdeucher at gmail.com
Wed Mar 10 20:00:43 UTC 2021
On Tue, Mar 9, 2021 at 12:25 PM Bhardwaj, Rajneesh
<rajneesh.bhardwaj at 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 at 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 at lists.freedesktop.org> On Behalf Of Alex Deucher
> >> Sent: Tuesday, March 9, 2021 9:40 AM
> >> To: amd-gfx at lists.freedesktop.org
> >> Cc: Deucher, Alexander <Alexander.Deucher at 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 at 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 at lists.freedesktop.org
> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Crajneesh.bhardwaj%40amd.com%7Cd0524482d33848d5909f08d8e312b2e3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637509016743361628%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=ZGYK5C%2Flo5BCIqxeEikspQi3Ib1M501mw04evWswAss%3D&reserved=0
> >> _______________________________________________
> >> amd-gfx mailing list
> >> amd-gfx at lists.freedesktop.org
> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Crajneesh.bhardwaj%40amd.com%7Cd0524482d33848d5909f08d8e312b2e3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637509016743361628%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=ZGYK5C%2Flo5BCIqxeEikspQi3Ib1M501mw04evWswAss%3D&reserved=0
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx at lists.freedesktop.org
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Crajneesh.bhardwaj%40amd.com%7Cd0524482d33848d5909f08d8e312b2e3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637509016743361628%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=ZGYK5C%2Flo5BCIqxeEikspQi3Ib1M501mw04evWswAss%3D&reserved=0
More information about the amd-gfx
mailing list