RE: [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform

["Quan, Evan" <Evan.Quan@amd.com>]

[AMD Official Use Only]



> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Friday, December 24, 2021 12:44 PM
> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Chen, Guchun
> <Guchun.Chen@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: put SMU into proper state on runpm
> suspending for BOCO capable platform
> 
> 
> 
> On 12/24/2021 8:46 AM, Evan Quan wrote:
> > By setting mp1_state as PP_MP1_STATE_UNLOAD, MP1 will do some
> proper
> > cleanups and put itself into a state ready for PNP(which fits the scenario
> BOCO stands for).
> 
> "BOCO similar to PNP" is not correct. Mention this as a workaround. With that
> changed
[Quan, Evan] Sorry for the confusing. I did not mean "BOCO" == "PNP". What I wanted to express is for BOCO/PNP(unlike BACO), SMU does not have to be alive.
From that perspective, it's reasonable to share the SMU cleanup process designed for PNP for BOCO. Anyway thanks for pointing this out. I will drop that confusing expression.

BR
Evan
> 	Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>
> 
> Thanks,
> Lijo
> 
> > That can address some random resuming failure observed on BOCO
> capable platforms.
> >
> > Signed-off-by: Evan Quan <evan.quan@amd.com>
> > Change-Id: I9804c4f04b6d2ef737b076cabf85d2880179efe2
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 15 +++++++++++++++
> >   1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index e431c7f10755..ad8370b41e74 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -2235,12 +2235,27 @@ static int
> amdgpu_pmops_runtime_suspend(struct device *dev)
> >   	if (amdgpu_device_supports_px(drm_dev))
> >   		drm_dev->switch_power_state =
> DRM_SWITCH_POWER_CHANGING;
> >
> > +	/*
> > +	 * By setting mp1_state as PP_MP1_STATE_UNLOAD, MP1 will do
> some
> > +	 * proper cleanups and put itself into a state ready for PNP. That
> > +	 * can address some random resuming failure observed on BOCO
> capable
> > +	 * platforms.
> > +	 * TODO: this may be also needed for PX capable platform.
> > +	 */
> > +	if (amdgpu_device_supports_boco(drm_dev))
> > +		adev->mp1_state = PP_MP1_STATE_UNLOAD;
> > +
> >   	ret = amdgpu_device_suspend(drm_dev, false);
> >   	if (ret) {
> >   		adev->in_runpm = false;
> > +		if (amdgpu_device_supports_boco(drm_dev))
> > +			adev->mp1_state = PP_MP1_STATE_NONE;
> >   		return ret;
> >   	}
> >
> > +	if (amdgpu_device_supports_boco(drm_dev))
> > +		adev->mp1_state = PP_MP1_STATE_NONE;
> > +
> >   	if (amdgpu_device_supports_px(drm_dev)) {
> >   		/* Only need to handle PCI state in the driver for ATPX
> >   		 * PCI core handles it for _PR3.
> >