[PATCH 4/7] drm/amdgpu: track what pmops flow we are in

Bhardwaj, Rajneesh rajneesh.bhardwaj at amd.com
Wed Mar 10 13:19:21 UTC 2021


No, those are global system states. Here we are dealing with device pm 
states.

On 3/10/2021 5:16 AM, Liang, Prike wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
> Maybe we can use the acpi_target_system_state() interface to identify the system-wide suspend target level Sx and then can parse the return code by the following macro definition. If have bandwidth will update and refine the AMDGPU Sx[0..5] suspend/resume sequence.
>
> #define ACPI_STATE_S0                   (u8) 0
> #define ACPI_STATE_S1                   (u8) 1
> #define ACPI_STATE_S2                   (u8) 2
> #define ACPI_STATE_S3                   (u8) 3
> #define ACPI_STATE_S4                   (u8) 4
> #define ACPI_STATE_S5                   (u8) 5
>
> Thanks,
> Prike
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of
>> Bhardwaj, Rajneesh
>> Sent: Wednesday, March 10, 2021 1:25 AM
>> To: Alex Deucher <alexdeucher at gmail.com>; Lazar, Lijo
>> <Lijo.Lazar at amd.com>
>> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; amd-
>> gfx at lists.freedesktop.org
>> Subject: Re: [PATCH 4/7] drm/amdgpu: track what pmops flow we are in
>>
>> 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir
>> .bootlin.com%2Flinux%2Fv4.7%2Fsource%2Fdrivers%2Fgpu%2Fdrm%2Fi915
>> %2Fi915_drv.c%23L714&data=04%7C01%7CPrike.Liang%40amd.com%7
>> C473ede68e7a347ff606b08d8e3204e94%7C3dd8961fe4884e608e11a82d994e
>> 183d%7C0%7C0%7C637509075233985095%7CUnknown%7CTWFpbGZsb3d8e
>> yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
>> %7C1000&sdata=Y%2BNKrW2LfzB157fZ%2FuLn7QAu%2BmxVgHjzG8LO
>> CH8z6J4%3D&reserved=0
>>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir
>> .bootlin.com%2Flinux%2Fv4.7%2Fsource%2Fdrivers%2Fgpu%2Fdrm%2Fdrm_
>> sysfs.c%23L43&data=04%7C01%7CPrike.Liang%40amd.com%7C473ede6
>> 8e7a347ff606b08d8e3204e94%7C3dd8961fe4884e608e11a82d994e183d%7C
>> 0%7C0%7C637509075233985095%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
>> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000
>> &sdata=svJsR97DiTwcbYHn3Y9dDV0YQCVzx5HLiebqQ9mTRY8%3D&am
>> p;reserved=0
>>
>> 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%2Flis
>>>> ts.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
>> gfx&data=04%7C01%7C
>> Prike.Liang%40amd.com%7C473ede68e7a347ff606b08d8e3204e94%7C3dd89
>> 61fe4
>> 884e608e11a82d994e183d%7C0%7C0%7C637509075233985095%7CUnknow
>> n%7CTWFpb
>> GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
>> Mn
>> 0%3D%7C1000&sdata=yYtPSR7eqLfZzYn1N%2FCDvpp%2Fxr6lERvs8w7d
>> uAiaX9g
>>>> %3D&reserved=0
>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx at lists.freedesktop.org
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
>>>> ts.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
>> gfx&data=04%7C01%7C
>> Prike.Liang%40amd.com%7C473ede68e7a347ff606b08d8e3204e94%7C3dd89
>> 61fe4
>> 884e608e11a82d994e183d%7C0%7C0%7C637509075233995092%7CUnknow
>> n%7CTWFpb
>> GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
>> Mn
>> 0%3D%7C1000&sdata=Kl90CvT0F7esbrGUTi1bGhA9Le7H7KZ3umrBcAAb
>> Y6o%3D&
>>>> amp;reserved=0
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
>>> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
>> gfx&data=04%7C01%7CPr
>> ike.Liang%40amd.com%7C473ede68e7a347ff606b08d8e3204e94%7C3dd896
>> 1fe4884
>> e608e11a82d994e183d%7C0%7C0%7C637509075233995092%7CUnknown%7
>> CTWFpbGZsb
>> 3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
>> %3D%
>> 7C1000&sdata=Kl90CvT0F7esbrGUTi1bGhA9Le7H7KZ3umrBcAAbY6o%3
>> D&re
>>> served=0
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.f
>> reedesktop.org%2Fmailman%2Flistinfo%2Famd-
>> gfx&data=04%7C01%7CPrike.Liang%40amd.com%7C473ede68e7a347ff6
>> 06b08d8e3204e94%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C
>> 637509075233995092%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
>> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdat
>> a=Kl90CvT0F7esbrGUTi1bGhA9Le7H7KZ3umrBcAAbY6o%3D&reserved=
>> 0


More information about the amd-gfx mailing list