* [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 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: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 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.