* [PATCH] drm/amdgpu: no SMC firmware reloading for non-RAS baco reset
@ 2019-12-18 3:24 Evan Quan
2019-12-18 14:55 ` Alex Deucher
0 siblings, 1 reply; 4+ messages in thread
From: Evan Quan @ 2019-12-18 3:24 UTC (permalink / raw)
To: amd-gfx; +Cc: Evan Quan
For non-RAS baco reset, there is no need to reset the SMC. Thus
the firmware reloading should be avoided.
Change-Id: I73f6284541d0ca0e82761380a27e32484fb0061c
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 ++-
drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 14 ++++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index c14f2ccd0677..9bf7e92394f5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -1439,7 +1439,8 @@ static int psp_np_fw_load(struct psp_context *psp)
continue;
if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
- (psp_smu_reload_quirk(psp) || psp->autoload_supported))
+ ((adev->in_gpu_reset && psp_smu_reload_quirk(psp))
+ || psp->autoload_supported))
continue;
if (amdgpu_sriov_vf(adev) &&
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index c66ca8cc2ebd..ba761e9366e3 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -676,6 +676,19 @@ static bool psp_v11_0_compare_sram_data(struct psp_context *psp,
return true;
}
+/*
+ * Check whether SMU is still alive. If that's true
+ * (e.g. for non-RAS baco reset), we need to skip SMC firmware reloading.
+ */
+static bool psp_v11_0_smu_reload_quirk(struct psp_context *psp)
+{
+ struct amdgpu_device *adev = psp->adev;
+ uint32_t reg;
+
+ reg = RREG32_PCIE(smnMP1_FIRMWARE_FLAGS | 0x03b00000);
+ return (reg & MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED_MASK) ? true : false;
+}
+
static int psp_v11_0_mode1_reset(struct psp_context *psp)
{
int ret;
@@ -1070,6 +1083,7 @@ static const struct psp_funcs psp_v11_0_funcs = {
.ring_stop = psp_v11_0_ring_stop,
.ring_destroy = psp_v11_0_ring_destroy,
.compare_sram_data = psp_v11_0_compare_sram_data,
+ .smu_reload_quirk = psp_v11_0_smu_reload_quirk,
.mode1_reset = psp_v11_0_mode1_reset,
.xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
.xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
--
2.24.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/amdgpu: no SMC firmware reloading for non-RAS baco reset
2019-12-18 3:24 [PATCH] drm/amdgpu: no SMC firmware reloading for non-RAS baco reset Evan Quan
@ 2019-12-18 14:55 ` Alex Deucher
2019-12-19 2:12 ` Quan, Evan
0 siblings, 1 reply; 4+ messages in thread
From: Alex Deucher @ 2019-12-18 14:55 UTC (permalink / raw)
To: Evan Quan; +Cc: amd-gfx list
On Tue, Dec 17, 2019 at 10:25 PM Evan Quan <evan.quan@amd.com> wrote:
>
> For non-RAS baco reset, there is no need to reset the SMC. Thus
> the firmware reloading should be avoided.
>
> Change-Id: I73f6284541d0ca0e82761380a27e32484fb0061c
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 ++-
> drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 14 ++++++++++++++
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index c14f2ccd0677..9bf7e92394f5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -1439,7 +1439,8 @@ static int psp_np_fw_load(struct psp_context *psp)
> continue;
>
> if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
> - (psp_smu_reload_quirk(psp) || psp->autoload_supported))
> + ((adev->in_gpu_reset && psp_smu_reload_quirk(psp))
> + || psp->autoload_supported))
Will this cover the power saving case as well? Do we need to check
adev->in_gpu_reset as well or can we drop that part?
Alex
> continue;
>
> if (amdgpu_sriov_vf(adev) &&
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> index c66ca8cc2ebd..ba761e9366e3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> @@ -676,6 +676,19 @@ static bool psp_v11_0_compare_sram_data(struct psp_context *psp,
> return true;
> }
>
> +/*
> + * Check whether SMU is still alive. If that's true
> + * (e.g. for non-RAS baco reset), we need to skip SMC firmware reloading.
> + */
> +static bool psp_v11_0_smu_reload_quirk(struct psp_context *psp)
> +{
> + struct amdgpu_device *adev = psp->adev;
> + uint32_t reg;
> +
> + reg = RREG32_PCIE(smnMP1_FIRMWARE_FLAGS | 0x03b00000);
> + return (reg & MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED_MASK) ? true : false;
> +}
> +
> static int psp_v11_0_mode1_reset(struct psp_context *psp)
> {
> int ret;
> @@ -1070,6 +1083,7 @@ static const struct psp_funcs psp_v11_0_funcs = {
> .ring_stop = psp_v11_0_ring_stop,
> .ring_destroy = psp_v11_0_ring_destroy,
> .compare_sram_data = psp_v11_0_compare_sram_data,
> + .smu_reload_quirk = psp_v11_0_smu_reload_quirk,
> .mode1_reset = psp_v11_0_mode1_reset,
> .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
> .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
> --
> 2.24.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] drm/amdgpu: no SMC firmware reloading for non-RAS baco reset
2019-12-18 14:55 ` Alex Deucher
@ 2019-12-19 2:12 ` Quan, Evan
2019-12-19 3:23 ` Alex Deucher
0 siblings, 1 reply; 4+ messages in thread
From: Quan, Evan @ 2019-12-19 2:12 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx list
Hi Alex,
"Power saving" means the regular suspend/resume case, right? That was considered.
With current amdgpu code, the MP1 state was not correctly set for the regular suspend case.
More straightforwardly I believe PrepareMP1_for_unload should be issued to MP1 on regular suspend path(excluding gpu reset case).
And with the MP1 state correctly set for all case, we can remove the "adev->in_gpu_reset".
But for now, I do not want to involve too many changes and limit this to the gpu reset case.
P.S. the mp1 state was correctly handled for mode1 reset. So, it's safe to enforce this for all gpu reset case instead of baco reset only.
Regards,
Evan
> -----Original Message-----
> From: Alex Deucher <alexdeucher@gmail.com>
> Sent: Wednesday, December 18, 2019 10:56 PM
> To: Quan, Evan <Evan.Quan@amd.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH] drm/amdgpu: no SMC firmware reloading for non-RAS
> baco reset
>
> On Tue, Dec 17, 2019 at 10:25 PM Evan Quan <evan.quan@amd.com> wrote:
> >
> > For non-RAS baco reset, there is no need to reset the SMC. Thus the
> > firmware reloading should be avoided.
> >
> > Change-Id: I73f6284541d0ca0e82761380a27e32484fb0061c
> > Signed-off-by: Evan Quan <evan.quan@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 ++-
> > drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 14 ++++++++++++++
> > 2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > index c14f2ccd0677..9bf7e92394f5 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > @@ -1439,7 +1439,8 @@ static int psp_np_fw_load(struct psp_context *psp)
> > continue;
> >
> > if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
> > - (psp_smu_reload_quirk(psp) || psp->autoload_supported))
> > + ((adev->in_gpu_reset && psp_smu_reload_quirk(psp))
> > + || psp->autoload_supported))
>
> Will this cover the power saving case as well? Do we need to check
> adev->in_gpu_reset as well or can we drop that part?
>
> Alex
>
> > continue;
> >
> > if (amdgpu_sriov_vf(adev) && diff --git
> > a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > index c66ca8cc2ebd..ba761e9366e3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > @@ -676,6 +676,19 @@ static bool psp_v11_0_compare_sram_data(struct
> psp_context *psp,
> > return true;
> > }
> >
> > +/*
> > + * Check whether SMU is still alive. If that's true
> > + * (e.g. for non-RAS baco reset), we need to skip SMC firmware reloading.
> > + */
> > +static bool psp_v11_0_smu_reload_quirk(struct psp_context *psp) {
> > + struct amdgpu_device *adev = psp->adev;
> > + uint32_t reg;
> > +
> > + reg = RREG32_PCIE(smnMP1_FIRMWARE_FLAGS | 0x03b00000);
> > + return (reg &
> MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED_MASK) ?
> > +true : false; }
> > +
> > static int psp_v11_0_mode1_reset(struct psp_context *psp) {
> > int ret;
> > @@ -1070,6 +1083,7 @@ static const struct psp_funcs psp_v11_0_funcs = {
> > .ring_stop = psp_v11_0_ring_stop,
> > .ring_destroy = psp_v11_0_ring_destroy,
> > .compare_sram_data = psp_v11_0_compare_sram_data,
> > + .smu_reload_quirk = psp_v11_0_smu_reload_quirk,
> > .mode1_reset = psp_v11_0_mode1_reset,
> > .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
> > .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
> > --
> > 2.24.0
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> gfx&data=02%7C01%7Cev
> >
> an.quan%40amd.com%7C8781ad2ef92d4a188c3008d783ca6846%7C3dd8961fe
> 4884e6
> >
> 08e11a82d994e183d%7C0%7C0%7C637122777663939524&sdata=DMLV%
> 2Bz%2FsG
> > nXhpsiOdv9EZrsBcn6HGJ3L7lKdKL2PaPQ%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/amdgpu: no SMC firmware reloading for non-RAS baco reset
2019-12-19 2:12 ` Quan, Evan
@ 2019-12-19 3:23 ` Alex Deucher
0 siblings, 0 replies; 4+ messages in thread
From: Alex Deucher @ 2019-12-19 3:23 UTC (permalink / raw)
To: Quan, Evan; +Cc: amd-gfx list
On Wed, Dec 18, 2019 at 9:12 PM Quan, Evan <Evan.Quan@amd.com> wrote:
>
> Hi Alex,
>
> "Power saving" means the regular suspend/resume case, right? That was considered.
I mean BACO for runtime power management support. I landed the code
to enable BACO for saving power at runtime when the GPU is not in use.
It's still disabled by default until we properly handle KFD support,
but you can enable it with amdgpu.runpm=1.
> With current amdgpu code, the MP1 state was not correctly set for the regular suspend case.
> More straightforwardly I believe PrepareMP1_for_unload should be issued to MP1 on regular suspend path(excluding gpu reset case).
>
> And with the MP1 state correctly set for all case, we can remove the "adev->in_gpu_reset".
> But for now, I do not want to involve too many changes and limit this to the gpu reset case.
>
> P.S. the mp1 state was correctly handled for mode1 reset. So, it's safe to enforce this for all gpu reset case instead of baco reset only.
Ah, good to hear.
Alex
>
> Regards,
> Evan
> > -----Original Message-----
> > From: Alex Deucher <alexdeucher@gmail.com>
> > Sent: Wednesday, December 18, 2019 10:56 PM
> > To: Quan, Evan <Evan.Quan@amd.com>
> > Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
> > Subject: Re: [PATCH] drm/amdgpu: no SMC firmware reloading for non-RAS
> > baco reset
> >
> > On Tue, Dec 17, 2019 at 10:25 PM Evan Quan <evan.quan@amd.com> wrote:
> > >
> > > For non-RAS baco reset, there is no need to reset the SMC. Thus the
> > > firmware reloading should be avoided.
> > >
> > > Change-Id: I73f6284541d0ca0e82761380a27e32484fb0061c
> > > Signed-off-by: Evan Quan <evan.quan@amd.com>
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 ++-
> > > drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 14 ++++++++++++++
> > > 2 files changed, 16 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > index c14f2ccd0677..9bf7e92394f5 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > @@ -1439,7 +1439,8 @@ static int psp_np_fw_load(struct psp_context *psp)
> > > continue;
> > >
> > > if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
> > > - (psp_smu_reload_quirk(psp) || psp->autoload_supported))
> > > + ((adev->in_gpu_reset && psp_smu_reload_quirk(psp))
> > > + || psp->autoload_supported))
> >
> > Will this cover the power saving case as well? Do we need to check
> > adev->in_gpu_reset as well or can we drop that part?
> >
> > Alex
> >
> > > continue;
> > >
> > > if (amdgpu_sriov_vf(adev) && diff --git
> > > a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > > b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > > index c66ca8cc2ebd..ba761e9366e3 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > > @@ -676,6 +676,19 @@ static bool psp_v11_0_compare_sram_data(struct
> > psp_context *psp,
> > > return true;
> > > }
> > >
> > > +/*
> > > + * Check whether SMU is still alive. If that's true
> > > + * (e.g. for non-RAS baco reset), we need to skip SMC firmware reloading.
> > > + */
> > > +static bool psp_v11_0_smu_reload_quirk(struct psp_context *psp) {
> > > + struct amdgpu_device *adev = psp->adev;
> > > + uint32_t reg;
> > > +
> > > + reg = RREG32_PCIE(smnMP1_FIRMWARE_FLAGS | 0x03b00000);
> > > + return (reg &
> > MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED_MASK) ?
> > > +true : false; }
> > > +
> > > static int psp_v11_0_mode1_reset(struct psp_context *psp) {
> > > int ret;
> > > @@ -1070,6 +1083,7 @@ static const struct psp_funcs psp_v11_0_funcs = {
> > > .ring_stop = psp_v11_0_ring_stop,
> > > .ring_destroy = psp_v11_0_ring_destroy,
> > > .compare_sram_data = psp_v11_0_compare_sram_data,
> > > + .smu_reload_quirk = psp_v11_0_smu_reload_quirk,
> > > .mode1_reset = psp_v11_0_mode1_reset,
> > > .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
> > > .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
> > > --
> > > 2.24.0
> > >
> > > _______________________________________________
> > > amd-gfx mailing list
> > > amd-gfx@lists.freedesktop.org
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > > s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> > gfx&data=02%7C01%7Cev
> > >
> > an.quan%40amd.com%7C8781ad2ef92d4a188c3008d783ca6846%7C3dd8961fe
> > 4884e6
> > >
> > 08e11a82d994e183d%7C0%7C0%7C637122777663939524&sdata=DMLV%
> > 2Bz%2FsG
> > > nXhpsiOdv9EZrsBcn6HGJ3L7lKdKL2PaPQ%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-12-19 3:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-18 3:24 [PATCH] drm/amdgpu: no SMC firmware reloading for non-RAS baco reset Evan Quan
2019-12-18 14:55 ` Alex Deucher
2019-12-19 2:12 ` Quan, Evan
2019-12-19 3:23 ` Alex Deucher
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.