Re: [PATCH] drm/amdgpu: correct the wrong cached state for GMC on PICASSO
On 12/14/2021 1:16 PM, Quan, Evan wrote:
[AMD Official Use Only]
-Original Message-
From: Lazar, Lijo
Sent: Tuesday, December 14, 2021 2:13 PM
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Limonciello, Mario
Subject: Re: [PATCH] drm/amdgpu: correct the wrong cached state for GMC
on PICASSO
On 12/14/2021 7:04 AM, Evan Quan wrote:
Pair the operations did in GMC ->hw_init and ->hw_fini. That can help
to maintain correct cached state for GMC and avoid unintention gate
operation dropping due to wrong cached state.
BUG: https://gitlab.freedesktop.org/drm/amd/-/issues/1828
Signed-off-by: Evan Quan
Change-Id: I9976672a64464b86bb45eed0c25c9599d3bb4c06
---
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c| 8
drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 8
drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 7 ++-
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index db2ec84f7237..c7492db3e189 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1809,6 +1809,14 @@ static int gmc_v9_0_hw_fini(void *handle)
return 0;
}
+ /*
+* Pair the operations did in gmc_v9_0_hw_init and thus maintain
+* a correct cached state for GMC. Otherwise, the "gate" again
+* operation on S3 resuming will fail due to wrong cached state.
+*/
+ if (adev->mmhub.funcs->update_power_gating)
+ adev->mmhub.funcs->update_power_gating(adev, false);
+
amdgpu_irq_put(adev, &adev->gmc.ecc_irq, 0);
amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
index b3bede1dc41d..1da2ec692057 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
@@ -301,10 +301,10 @@ static void
mmhub_v1_0_update_power_gating(struct amdgpu_device *adev,
if (amdgpu_sriov_vf(adev))
return;
- if (enable && adev->pg_flags & AMD_PG_SUPPORT_MMHUB) {
- amdgpu_dpm_set_powergating_by_smu(adev,
AMD_IP_BLOCK_TYPE_GMC, true);
-
- }
+ if (adev->pg_flags & AMD_PG_SUPPORT_MMHUB)
+ amdgpu_dpm_set_powergating_by_smu(adev,
+ AMD_IP_BLOCK_TYPE_GMC,
+ enable);
}
static int mmhub_v1_0_gart_enable(struct amdgpu_device *adev) diff
--git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
index 3656a77baea4..9953a77cb987 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
@@ -1167,7 +1167,12 @@ static int pp_set_powergating_by_smu(void
*handle,
pp_dpm_powergate_vce(handle, gate);
break;
case AMD_IP_BLOCK_TYPE_GMC:
- pp_dpm_powergate_mmhub(handle);
+ /*
+* For now, this is only used on PICASSO.
+* And only "gate" operation is supported.
+*/
+ if (gate)
+ pp_dpm_powergate_mmhub(handle);
This is a generic entry point. Would rather keep PG enable or disable
supported in mmhub_v1_0 rather than than here. And this comment also
should be in mmhub_v1_0.
[Quan, Evan] pp_dpm_powergate_mmhub is not a generic interface. It's customized for
PICASSO which accepts no parameter("bool gate") as other interfaces.
Thus some comments which explain why the interface is so special are proper I
think.
amd_powerplay is the generic entry point and we should avoid ASIC
specific things as much as possible. Ideally this should be handled in
mmhub v1/smu10 hwmgr and not here.
Thanks,
Lijo
BR
Evan
Thanks,
Lijo
break;
case AMD_IP_BLOCK_TYPE_GFX:
ret = pp_dpm_powergate_gfx(handle, gate);
RE: [PATCH] drm/amdgpu: correct the wrong cached state for GMC on PICASSO
[AMD Official Use Only]
> -Original Message-
> From: Lazar, Lijo
> Sent: Tuesday, December 14, 2021 2:13 PM
> To: Quan, Evan ; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Limonciello, Mario
>
> Subject: Re: [PATCH] drm/amdgpu: correct the wrong cached state for GMC
> on PICASSO
>
>
>
> On 12/14/2021 7:04 AM, Evan Quan wrote:
> > Pair the operations did in GMC ->hw_init and ->hw_fini. That can help
> > to maintain correct cached state for GMC and avoid unintention gate
> > operation dropping due to wrong cached state.
> >
> > BUG: https://gitlab.freedesktop.org/drm/amd/-/issues/1828
> >
> > Signed-off-by: Evan Quan
> > Change-Id: I9976672a64464b86bb45eed0c25c9599d3bb4c06
> > ---
> > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c| 8
> > drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 8
> > drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 7 ++-
> > 3 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > index db2ec84f7237..c7492db3e189 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > @@ -1809,6 +1809,14 @@ static int gmc_v9_0_hw_fini(void *handle)
> > return 0;
> > }
> >
> > + /*
> > +* Pair the operations did in gmc_v9_0_hw_init and thus maintain
> > +* a correct cached state for GMC. Otherwise, the "gate" again
> > +* operation on S3 resuming will fail due to wrong cached state.
> > +*/
> > + if (adev->mmhub.funcs->update_power_gating)
> > + adev->mmhub.funcs->update_power_gating(adev, false);
> > +
> > amdgpu_irq_put(adev, &adev->gmc.ecc_irq, 0);
> > amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> > b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> > index b3bede1dc41d..1da2ec692057 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> > @@ -301,10 +301,10 @@ static void
> mmhub_v1_0_update_power_gating(struct amdgpu_device *adev,
> > if (amdgpu_sriov_vf(adev))
> > return;
> >
> > - if (enable && adev->pg_flags & AMD_PG_SUPPORT_MMHUB) {
> > - amdgpu_dpm_set_powergating_by_smu(adev,
> AMD_IP_BLOCK_TYPE_GMC, true);
> > -
> > - }
> > + if (adev->pg_flags & AMD_PG_SUPPORT_MMHUB)
> > + amdgpu_dpm_set_powergating_by_smu(adev,
> > + AMD_IP_BLOCK_TYPE_GMC,
> > + enable);
> > }
> >
> > static int mmhub_v1_0_gart_enable(struct amdgpu_device *adev) diff
> > --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> > b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> > index 3656a77baea4..9953a77cb987 100644
> > --- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> > +++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> > @@ -1167,7 +1167,12 @@ static int pp_set_powergating_by_smu(void
> *handle,
> > pp_dpm_powergate_vce(handle, gate);
> > break;
> > case AMD_IP_BLOCK_TYPE_GMC:
> > - pp_dpm_powergate_mmhub(handle);
> > + /*
> > +* For now, this is only used on PICASSO.
> > +* And only "gate" operation is supported.
> > +*/
> > + if (gate)
> > + pp_dpm_powergate_mmhub(handle);
>
> This is a generic entry point. Would rather keep PG enable or disable
> supported in mmhub_v1_0 rather than than here. And this comment also
> should be in mmhub_v1_0.
[Quan, Evan] pp_dpm_powergate_mmhub is not a generic interface. It's customized
for PICASSO which accepts no parameter("bool gate") as other interfaces.
Thus some comments which explain why the interface is so special are proper I
think.
BR
Evan
>
> Thanks,
> Lijo
>
> > break;
> > case AMD_IP_BLOCK_TYPE_GFX:
> > ret = pp_dpm_powergate_gfx(handle, gate);
> >
Re: [PATCH] drm/amdgpu: correct the wrong cached state for GMC on PICASSO
On 12/14/2021 7:04 AM, Evan Quan wrote:
Pair the operations did in GMC ->hw_init and ->hw_fini. That
can help to maintain correct cached state for GMC and avoid
unintention gate operation dropping due to wrong cached state.
BUG: https://gitlab.freedesktop.org/drm/amd/-/issues/1828
Signed-off-by: Evan Quan
Change-Id: I9976672a64464b86bb45eed0c25c9599d3bb4c06
---
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c| 8
drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 8
drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 7 ++-
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index db2ec84f7237..c7492db3e189 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1809,6 +1809,14 @@ static int gmc_v9_0_hw_fini(void *handle)
return 0;
}
+ /*
+* Pair the operations did in gmc_v9_0_hw_init and thus maintain
+* a correct cached state for GMC. Otherwise, the "gate" again
+* operation on S3 resuming will fail due to wrong cached state.
+*/
+ if (adev->mmhub.funcs->update_power_gating)
+ adev->mmhub.funcs->update_power_gating(adev, false);
+
amdgpu_irq_put(adev, &adev->gmc.ecc_irq, 0);
amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
index b3bede1dc41d..1da2ec692057 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
@@ -301,10 +301,10 @@ static void mmhub_v1_0_update_power_gating(struct
amdgpu_device *adev,
if (amdgpu_sriov_vf(adev))
return;
- if (enable && adev->pg_flags & AMD_PG_SUPPORT_MMHUB) {
- amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GMC,
true);
-
- }
+ if (adev->pg_flags & AMD_PG_SUPPORT_MMHUB)
+ amdgpu_dpm_set_powergating_by_smu(adev,
+ AMD_IP_BLOCK_TYPE_GMC,
+ enable);
}
static int mmhub_v1_0_gart_enable(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
index 3656a77baea4..9953a77cb987 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
@@ -1167,7 +1167,12 @@ static int pp_set_powergating_by_smu(void *handle,
pp_dpm_powergate_vce(handle, gate);
break;
case AMD_IP_BLOCK_TYPE_GMC:
- pp_dpm_powergate_mmhub(handle);
+ /*
+* For now, this is only used on PICASSO.
+* And only "gate" operation is supported.
+*/
+ if (gate)
+ pp_dpm_powergate_mmhub(handle);
This is a generic entry point. Would rather keep PG enable or disable
supported in mmhub_v1_0 rather than than here. And this comment also
should be in mmhub_v1_0.
Thanks,
Lijo
break;
case AMD_IP_BLOCK_TYPE_GFX:
ret = pp_dpm_powergate_gfx(handle, gate);
RE: [PATCH] drm/amdgpu: correct the wrong cached state for GMC on PICASSO
[Public]
> -Original Message-
> From: Limonciello, Mario
> Sent: Tuesday, December 14, 2021 11:50 AM
> To: Chen, Guchun ; Quan, Evan
> ; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Quan, Evan
>
> Subject: RE: [PATCH] drm/amdgpu: correct the wrong cached state for GMC
> on PICASSO
>
> [Public]
>
>
>
> > -Original Message-
> > From: Chen, Guchun
> > Sent: Monday, December 13, 2021 21:49
> > To: Quan, Evan ; amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander ; Quan, Evan
> > ; Limonciello, Mario
> > Subject: RE: [PATCH] drm/amdgpu: correct the wrong cached state for
> > GMC on PICASSO
> >
> > [Public]
> >
> > Acked-by: Guchun Chen
> >
> > Regards,
> > Guchun
> >
> > -Original Message-
> > From: amd-gfx On Behalf Of
> > Evan Quan
> > Sent: Tuesday, December 14, 2021 9:34 AM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander ; Quan, Evan
> > ; Limonciello, Mario
> > Subject: [PATCH] drm/amdgpu: correct the wrong cached state for GMC on
> > PICASSO
> >
> > Pair the operations did in GMC ->hw_init and ->hw_fini. That can help
> > to maintain correct cached state for GMC and avoid unintention gate
> > operation dropping due to wrong cached state.
> >
> > BUG:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > ab.fr
> > eedesktop.org%2Fdrm%2Famd%2F-
> > %2Fissues%2F1828&data=04%7C01%7Cguchun.chen%40amd.com%7C
> 42b
> >
> 00d7e1c4e44c0762908d9bea1ef53%7C3dd8961fe4884e608e11a82d994e183d%
> >
> 7C0%7C0%7C637750424983319967%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> M
> >
> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000
> &a
> >
> mp;sdata=VgBDAbcKN%2FqUz8iBQby9YP8PsG2y93VlnDVhXVaGNBo%3D&a
> mp;r
> > eserved=0
> >
>
> The syntax here should be BugLink:
>
[Quan, Evan] Thanks! Will fix that on code submission.
BR
Evan
> Otherwise
> Reviewed-by: Mario Limonciello
>
> > Signed-off-by: Evan Quan
> > Change-Id: I9976672a64464b86bb45eed0c25c9599d3bb4c06
> > ---
> > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c| 8
> > drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 8
> > drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 7 ++-
> > 3 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > index db2ec84f7237..c7492db3e189 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > @@ -1809,6 +1809,14 @@ static int gmc_v9_0_hw_fini(void *handle)
> > return 0;
> > }
> >
> > + /*
> > +* Pair the operations did in gmc_v9_0_hw_init and thus maintain
> > +* a correct cached state for GMC. Otherwise, the "gate" again
> > +* operation on S3 resuming will fail due to wrong cached state.
> > +*/
> > + if (adev->mmhub.funcs->update_power_gating)
> > + adev->mmhub.funcs->update_power_gating(adev, false);
> > +
> > amdgpu_irq_put(adev, &adev->gmc.ecc_irq, 0);
> > amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> > b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> > index b3bede1dc41d..1da2ec692057 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> > @@ -301,10 +301,10 @@ static void
> > mmhub_v1_0_update_power_gating(struct amdgpu_device *adev,
> > if (amdgpu_sriov_vf(adev))
> > return;
> >
> > - if (enable && adev->pg_flags & AMD_PG_SUPPORT_MMHUB) {
> > - amdgpu_dpm_set_powergating_by_smu(adev,
> > AMD_IP_BLOCK_TYPE_GMC, true);
> > -
> > - }
> > + if (adev->pg_flags & AMD_PG_SUPPORT_MMHUB)
> > + amdgpu_dpm_set_powergating_by_smu(adev,
> > + AMD_IP_BLOCK_TYPE_GMC,
> > + enable);
> > }
> >
> > static int mmhub_v1_0_gart_enable(struct amdgpu_device *adev) diff
> > --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> > b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> > index 3656a77baea4..9953a77cb987 100644
> > --- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> > +++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> > @@ -1167,7 +1167,12 @@ static int pp_set_powergating_by_smu(void
> > *handle,
> > pp_dpm_powergate_vce(handle, gate);
> > break;
> > case AMD_IP_BLOCK_TYPE_GMC:
> > - pp_dpm_powergate_mmhub(handle);
> > + /*
> > +* For now, this is only used on PICASSO.
> > +* And only "gate" operation is supported.
> > +*/
> > + if (gate)
> > + pp_dpm_powergate_mmhub(handle);
> > break;
> > case AMD_IP_BLOCK_TYPE_GFX:
> > ret = pp_dpm_powergate_gfx(handle, gate);
> > --
> > 2.29.0
RE: [PATCH] drm/amdgpu: correct the wrong cached state for GMC on PICASSO
[Public]
> -Original Message-
> From: Chen, Guchun
> Sent: Monday, December 13, 2021 21:49
> To: Quan, Evan ; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Quan, Evan
> ; Limonciello, Mario
> Subject: RE: [PATCH] drm/amdgpu: correct the wrong cached state for GMC on
> PICASSO
>
> [Public]
>
> Acked-by: Guchun Chen
>
> Regards,
> Guchun
>
> -Original Message-
> From: amd-gfx On Behalf Of Evan
> Quan
> Sent: Tuesday, December 14, 2021 9:34 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Quan, Evan
> ; Limonciello, Mario
> Subject: [PATCH] drm/amdgpu: correct the wrong cached state for GMC on
> PICASSO
>
> Pair the operations did in GMC ->hw_init and ->hw_fini. That can help to
> maintain correct cached state for GMC and avoid unintention gate operation
> dropping due to wrong cached state.
>
> BUG:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.fr
> eedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1828&data=04%7C01%7Cguchun.chen%40amd.com%7C42b
> 00d7e1c4e44c0762908d9bea1ef53%7C3dd8961fe4884e608e11a82d994e183d%
> 7C0%7C0%7C637750424983319967%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&a
> mp;sdata=VgBDAbcKN%2FqUz8iBQby9YP8PsG2y93VlnDVhXVaGNBo%3D&r
> eserved=0
>
The syntax here should be BugLink:
Otherwise
Reviewed-by: Mario Limonciello
> Signed-off-by: Evan Quan
> Change-Id: I9976672a64464b86bb45eed0c25c9599d3bb4c06
> ---
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c| 8
> drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 8
> drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 7 ++-
> 3 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index db2ec84f7237..c7492db3e189 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1809,6 +1809,14 @@ static int gmc_v9_0_hw_fini(void *handle)
> return 0;
> }
>
> + /*
> + * Pair the operations did in gmc_v9_0_hw_init and thus maintain
> + * a correct cached state for GMC. Otherwise, the "gate" again
> + * operation on S3 resuming will fail due to wrong cached state.
> + */
> + if (adev->mmhub.funcs->update_power_gating)
> + adev->mmhub.funcs->update_power_gating(adev, false);
> +
> amdgpu_irq_put(adev, &adev->gmc.ecc_irq, 0);
> amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> index b3bede1dc41d..1da2ec692057 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> @@ -301,10 +301,10 @@ static void
> mmhub_v1_0_update_power_gating(struct amdgpu_device *adev,
> if (amdgpu_sriov_vf(adev))
> return;
>
> - if (enable && adev->pg_flags & AMD_PG_SUPPORT_MMHUB) {
> - amdgpu_dpm_set_powergating_by_smu(adev,
> AMD_IP_BLOCK_TYPE_GMC, true);
> -
> - }
> + if (adev->pg_flags & AMD_PG_SUPPORT_MMHUB)
> + amdgpu_dpm_set_powergating_by_smu(adev,
> + AMD_IP_BLOCK_TYPE_GMC,
> + enable);
> }
>
> static int mmhub_v1_0_gart_enable(struct amdgpu_device *adev) diff --git
> a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> index 3656a77baea4..9953a77cb987 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> @@ -1167,7 +1167,12 @@ static int pp_set_powergating_by_smu(void
> *handle,
> pp_dpm_powergate_vce(handle, gate);
> break;
> case AMD_IP_BLOCK_TYPE_GMC:
> - pp_dpm_powergate_mmhub(handle);
> + /*
> + * For now, this is only used on PICASSO.
> + * And only "gate" operation is supported.
> + */
> + if (gate)
> + pp_dpm_powergate_mmhub(handle);
> break;
> case AMD_IP_BLOCK_TYPE_GFX:
> ret = pp_dpm_powergate_gfx(handle, gate);
> --
> 2.29.0
RE: [PATCH] drm/amdgpu: correct the wrong cached state for GMC on PICASSO
[Public]
Acked-by: Guchun Chen
Regards,
Guchun
-Original Message-
From: amd-gfx On Behalf Of Evan Quan
Sent: Tuesday, December 14, 2021 9:34 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Quan, Evan
; Limonciello, Mario
Subject: [PATCH] drm/amdgpu: correct the wrong cached state for GMC on PICASSO
Pair the operations did in GMC ->hw_init and ->hw_fini. That can help to
maintain correct cached state for GMC and avoid unintention gate operation
dropping due to wrong cached state.
BUG:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1828&data=04%7C01%7Cguchun.chen%40amd.com%7C42b00d7e1c4e44c0762908d9bea1ef53%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637750424983319967%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=VgBDAbcKN%2FqUz8iBQby9YP8PsG2y93VlnDVhXVaGNBo%3D&reserved=0
Signed-off-by: Evan Quan
Change-Id: I9976672a64464b86bb45eed0c25c9599d3bb4c06
---
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c| 8
drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 8
drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 7 ++-
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index db2ec84f7237..c7492db3e189 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1809,6 +1809,14 @@ static int gmc_v9_0_hw_fini(void *handle)
return 0;
}
+ /*
+* Pair the operations did in gmc_v9_0_hw_init and thus maintain
+* a correct cached state for GMC. Otherwise, the "gate" again
+* operation on S3 resuming will fail due to wrong cached state.
+*/
+ if (adev->mmhub.funcs->update_power_gating)
+ adev->mmhub.funcs->update_power_gating(adev, false);
+
amdgpu_irq_put(adev, &adev->gmc.ecc_irq, 0);
amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
index b3bede1dc41d..1da2ec692057 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
@@ -301,10 +301,10 @@ static void mmhub_v1_0_update_power_gating(struct
amdgpu_device *adev,
if (amdgpu_sriov_vf(adev))
return;
- if (enable && adev->pg_flags & AMD_PG_SUPPORT_MMHUB) {
- amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GMC,
true);
-
- }
+ if (adev->pg_flags & AMD_PG_SUPPORT_MMHUB)
+ amdgpu_dpm_set_powergating_by_smu(adev,
+ AMD_IP_BLOCK_TYPE_GMC,
+ enable);
}
static int mmhub_v1_0_gart_enable(struct amdgpu_device *adev) diff --git
a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
index 3656a77baea4..9953a77cb987 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
@@ -1167,7 +1167,12 @@ static int pp_set_powergating_by_smu(void *handle,
pp_dpm_powergate_vce(handle, gate);
break;
case AMD_IP_BLOCK_TYPE_GMC:
- pp_dpm_powergate_mmhub(handle);
+ /*
+* For now, this is only used on PICASSO.
+* And only "gate" operation is supported.
+*/
+ if (gate)
+ pp_dpm_powergate_mmhub(handle);
break;
case AMD_IP_BLOCK_TYPE_GFX:
ret = pp_dpm_powergate_gfx(handle, gate);
--
2.29.0
[PATCH] drm/amdgpu: correct the wrong cached state for GMC on PICASSO
Pair the operations did in GMC ->hw_init and ->hw_fini. That
can help to maintain correct cached state for GMC and avoid
unintention gate operation dropping due to wrong cached state.
BUG: https://gitlab.freedesktop.org/drm/amd/-/issues/1828
Signed-off-by: Evan Quan
Change-Id: I9976672a64464b86bb45eed0c25c9599d3bb4c06
---
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c| 8
drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 8
drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 7 ++-
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index db2ec84f7237..c7492db3e189 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1809,6 +1809,14 @@ static int gmc_v9_0_hw_fini(void *handle)
return 0;
}
+ /*
+* Pair the operations did in gmc_v9_0_hw_init and thus maintain
+* a correct cached state for GMC. Otherwise, the "gate" again
+* operation on S3 resuming will fail due to wrong cached state.
+*/
+ if (adev->mmhub.funcs->update_power_gating)
+ adev->mmhub.funcs->update_power_gating(adev, false);
+
amdgpu_irq_put(adev, &adev->gmc.ecc_irq, 0);
amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
index b3bede1dc41d..1da2ec692057 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
@@ -301,10 +301,10 @@ static void mmhub_v1_0_update_power_gating(struct
amdgpu_device *adev,
if (amdgpu_sriov_vf(adev))
return;
- if (enable && adev->pg_flags & AMD_PG_SUPPORT_MMHUB) {
- amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GMC,
true);
-
- }
+ if (adev->pg_flags & AMD_PG_SUPPORT_MMHUB)
+ amdgpu_dpm_set_powergating_by_smu(adev,
+ AMD_IP_BLOCK_TYPE_GMC,
+ enable);
}
static int mmhub_v1_0_gart_enable(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
index 3656a77baea4..9953a77cb987 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
@@ -1167,7 +1167,12 @@ static int pp_set_powergating_by_smu(void *handle,
pp_dpm_powergate_vce(handle, gate);
break;
case AMD_IP_BLOCK_TYPE_GMC:
- pp_dpm_powergate_mmhub(handle);
+ /*
+* For now, this is only used on PICASSO.
+* And only "gate" operation is supported.
+*/
+ if (gate)
+ pp_dpm_powergate_mmhub(handle);
break;
case AMD_IP_BLOCK_TYPE_GFX:
ret = pp_dpm_powergate_gfx(handle, gate);
--
2.29.0
