Re: [PATCH v2 3/3] drm/amd: Add special handling for system s0ix state w/ dGPUs

["Lazar, Lijo" <lijo.lazar@amd.com>]

On 2/28/2023 10:13 AM, Mario Limonciello wrote:
> With dGPUs that support BACO or BOCO we want them to go into those
> states when the system goes to s2idle.  Detect that the system will
> be targeting this state and force the call into runtime suspend.
> 
> If the runtime suspend call fails for any reason, then fallback to
> standard suspend flow.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
>   * New patch
>   drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c |  3 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  | 12 +++++++++++-
>   2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index 711f2a1bf525..7c3c6380135a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -1073,8 +1073,7 @@ bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev)
>    */
>   bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev)
>   {
> -	if (!(adev->flags & AMD_IS_APU) ||
> -	    (pm_suspend_target_state != PM_SUSPEND_TO_IDLE))
> +	if (pm_suspend_target_state != PM_SUSPEND_TO_IDLE)
>   		return false;
>

This will set adev->in_s0ix flag to be true for all dGPUs. There are 
many places through out suspend/resume logic where it is assumed that 
adev->in_s0ix is set only for APUs. For ex: it skips suspend of GFX 
assuming GFXOFF is a pre-condition for s0ix.

Basically this will break suspend/resume of dGPUs in s2idle if the 
device is not already suspended.

Thanks,
Lijo


>   	if (adev->asic_type < CHIP_RAVEN)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 750984517192..acc032c4c250 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2415,8 +2415,18 @@ 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);
>   
> -	if (amdgpu_acpi_is_s0ix_active(adev))
> +	if (amdgpu_acpi_is_s0ix_active(adev)) {
> +		/* try to explicitly enter runtime suspend for s2idle on BACO/BOCO */
> +		if (dev_pm_test_driver_flags(drm_dev->dev, DPM_FLAG_SMART_SUSPEND)) {
> +			int ret;
> +
> +			ret = pm_runtime_suspend(dev);
> +			if (!ret)
> +				return 0;
> +			DRM_WARN("failed to enter runtime suspend, running system suspend: %d\n", ret);
> +		}
>   		adev->in_s0ix = true;
> +	}
>   	else if (amdgpu_acpi_is_s3_active(adev))
>   		adev->in_s3 = true;
>   	if (!adev->in_s0ix && !adev->in_s3)