* [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform @ 2021-12-24 3:16 Evan Quan 2021-12-24 4:15 ` Alex Deucher ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Evan Quan @ 2021-12-24 3:16 UTC (permalink / raw) To: amd-gfx; +Cc: Alexander.Deucher, Evan Quan, Guchun.Chen 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). 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. -- 2.29.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform 2021-12-24 3:16 [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform Evan Quan @ 2021-12-24 4:15 ` Alex Deucher 2021-12-24 4:41 ` Chen, Guchun 2021-12-24 4:43 ` Lazar, Lijo 2 siblings, 0 replies; 6+ messages in thread From: Alex Deucher @ 2021-12-24 4:15 UTC (permalink / raw) To: Evan Quan; +Cc: Deucher, Alexander, Chen, Guchun, amd-gfx list On Thu, Dec 23, 2021 at 10:17 PM Evan Quan <evan.quan@amd.com> 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). > That can address some random resuming failure observed on BOCO capable platforms. > > Signed-off-by: Evan Quan <evan.quan@amd.com> > Change-Id: I9804c4f04b6d2ef737b076cabf85d2880179efe2 Acked-by: Alex Deucher <alexander.deucher@amd.com> > --- > 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. > -- > 2.29.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform 2021-12-24 3:16 [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform Evan Quan 2021-12-24 4:15 ` Alex Deucher @ 2021-12-24 4:41 ` Chen, Guchun 2021-12-24 5:26 ` Chen, Guchun 2021-12-24 4:43 ` Lazar, Lijo 2 siblings, 1 reply; 6+ messages in thread From: Chen, Guchun @ 2021-12-24 4:41 UTC (permalink / raw) To: Quan, Evan, amd-gfx; +Cc: Deucher, Alexander, Quan, Evan [Public] Thank you Evan. Shall we limit it to dGPU only? With above concern clarified, the patch is: Reviewed-by: Guchun Chen <guchun.chen@amd.com> Regards, Guchun -----Original Message----- From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Evan Quan Sent: Friday, December 24, 2021 11:17 AM To: amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Quan, Evan <Evan.Quan@amd.com>; Chen, Guchun <Guchun.Chen@amd.com> Subject: [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform 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). 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. -- 2.29.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform 2021-12-24 4:41 ` Chen, Guchun @ 2021-12-24 5:26 ` Chen, Guchun 0 siblings, 0 replies; 6+ messages in thread From: Chen, Guchun @ 2021-12-24 5:26 UTC (permalink / raw) To: Quan, Evan, amd-gfx; +Cc: Deucher, Alexander, Quan, Evan [Public] Never mind. I checked the code in amdgpu_device_supports_boco, it's only true on dGPU. So pls skip my question. Regards, Guchun -----Original Message----- From: Chen, Guchun Sent: Friday, December 24, 2021 12:42 PM To: Evan Quan <evan.quan@amd.com>; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Quan, Evan <Evan.Quan@amd.com> Subject: RE: [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform [Public] Thank you Evan. Shall we limit it to dGPU only? With above concern clarified, the patch is: Reviewed-by: Guchun Chen <guchun.chen@amd.com> Regards, Guchun -----Original Message----- From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Evan Quan Sent: Friday, December 24, 2021 11:17 AM To: amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Quan, Evan <Evan.Quan@amd.com>; Chen, Guchun <Guchun.Chen@amd.com> Subject: [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform 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). 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. -- 2.29.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform 2021-12-24 3:16 [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform Evan Quan 2021-12-24 4:15 ` Alex Deucher 2021-12-24 4:41 ` Chen, Guchun @ 2021-12-24 4:43 ` Lazar, Lijo 2021-12-24 12:31 ` Quan, Evan 2 siblings, 1 reply; 6+ messages in thread From: Lazar, Lijo @ 2021-12-24 4:43 UTC (permalink / raw) To: Evan Quan, amd-gfx; +Cc: Alexander.Deucher, Guchun.Chen 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 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. > ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform 2021-12-24 4:43 ` Lazar, Lijo @ 2021-12-24 12:31 ` Quan, Evan 0 siblings, 0 replies; 6+ messages in thread From: Quan, Evan @ 2021-12-24 12:31 UTC (permalink / raw) To: Lazar, Lijo, amd-gfx; +Cc: Deucher, Alexander, Chen, Guchun [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. > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-24 12:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-24 3:16 [PATCH] drm/amdgpu: put SMU into proper state on runpm suspending for BOCO capable platform Evan Quan 2021-12-24 4:15 ` Alex Deucher 2021-12-24 4:41 ` Chen, Guchun 2021-12-24 5:26 ` Chen, Guchun 2021-12-24 4:43 ` Lazar, Lijo 2021-12-24 12:31 ` Quan, Evan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.