Thanks for your suggestion, Lijo. I will follow your suggestion in a new patch set.
Regards,
Guchun
-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Sent: Tuesday, July 12, 2022 2:57 PM
To: Chen, Guchun <Guchun.Chen@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Zhang, Hawking <Hawking.Zhang@xxxxxxx>; Quan, Evan <Evan.Quan@xxxxxxx>; Feng, Kenneth <Kenneth.Feng@xxxxxxx>
Subject: Re: [PATCH 2/2] drm/amdgpu: use cached baco flag as the check in runpm (v2)
Instead of doing this way, suggest to cache the run_pm_mode in
struct amdgpu_pm {
}
You could cache the run_pm modes in kms logic -
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c#L183
Afterwards, you may use the cached value for any check
For ex: adev->pm.run_pm_mode == AMDGPU_RPM_BACO
Thanks,
Lijo
On 7/12/2022 12:08 PM, Guchun Chen wrote:
> SMU will perform dpm disablement when entering BACO, and enable them
> later on, so talking to SMU to get enabled features mask in runpm
> cycle as BACO support check is not reliable. Hence, use a cached baco
> flag to fix it.
>
> v2: cache this flag in load sequence to simplify code (from Evan)
>
> Signed-off-by: Guchun Chen <guchun.chen@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 4 +++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +-
> 4 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 845d6054992a..816f813a5df2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1014,6 +1014,7 @@ struct amdgpu_device {
> bool runpm;
> bool in_runpm;
> bool has_pr3;
> + bool is_baco_supported;
>
> bool pm_sysfs_en;
> bool ucode_sysfs_en;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 1cc9260e75de..c3f870c01c47 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2513,7 +2513,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
> drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;
> } else if (amdgpu_device_supports_boco(drm_dev)) {
> /* nothing to do */
> - } else if (amdgpu_device_supports_baco(drm_dev)) {
> + } else if (adev->is_baco_supported) {
> amdgpu_device_baco_enter(drm_dev);
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 4b663866d33a..532406d32fba 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -188,8 +188,10 @@ int amdgpu_driver_load_kms(struct amdgpu_device
> *adev, unsigned long flags)
>
> amdgpu_runtime_pm_quirk(adev);
>
> - if (adev->runpm)
> + if (adev->runpm) {
> dev_info(adev->dev, "Using BACO for runtime pm\n");
> + adev->is_baco_supported = true;
> + }
> }
>
> /* Call ACPI methods: require modeset init diff --git
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index de59dc051340..f05d7ac03122 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -2353,7 +2353,7 @@ static int psp_load_smu_fw(struct psp_context *psp)
> */
> if (adev->in_runpm &&
> !amdgpu_device_supports_boco(adev_to_drm(adev)) &&
> - amdgpu_device_supports_baco(adev_to_drm(adev)))
> + adev->is_baco_supported)
> return 0;
>
> if (!ucode->fw || amdgpu_sriov_vf(psp->adev))
>
