[PATCH 1/2] drm/amd/pp: Add ACP PG support in SMU

[PATCH 1/2] drm/amd/pp: Add ACP PG support in SMU

[Rex Zhu]

when ACP block not enabled, we power off
acp block to save power.

Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c    | 18 ++++++++++++++++++
 drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c | 21 ++++++++++++++++++++-
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h        |  1 +
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
index 7a646f9..da4ebff 100644
--- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
@@ -1181,6 +1181,21 @@ static int pp_dpm_powergate_gfx(void *handle, bool gate)
 	return hwmgr->hwmgr_func->powergate_gfx(hwmgr, gate);
 }
 
+static void pp_dpm_powergate_acp(void *handle, bool gate)
+{
+	struct pp_hwmgr *hwmgr = handle;
+
+	if (!hwmgr || !hwmgr->pm_en)
+		return;
+
+	if (hwmgr->hwmgr_func->powergate_acp == NULL) {
+		pr_info("%s was not implemented.\n", __func__);
+		return;
+	}
+
+	hwmgr->hwmgr_func->powergate_acp(hwmgr, gate);
+}
+
 static int pp_set_powergating_by_smu(void *handle,
 				uint32_t block_type, bool gate)
 {
@@ -1200,6 +1215,9 @@ static int pp_set_powergating_by_smu(void *handle,
 	case AMD_IP_BLOCK_TYPE_GFX:
 		ret = pp_dpm_powergate_gfx(handle, gate);
 		break;
+	case AMD_IP_BLOCK_TYPE_ACP:
+		pp_dpm_powergate_acp(handle, gate);
+		break;
 	default:
 		break;
 	}
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
index 288802f..cafa822 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
@@ -663,8 +663,13 @@ static void smu8_init_power_gate_state(struct pp_hwmgr *hwmgr)
 	data->uvd_power_gated = false;
 	data->vce_power_gated = false;
 	data->samu_power_gated = false;
+#ifdef CONFIG_DRM_AMD_ACP
 	data->acp_power_gated = false;
-	data->pgacpinit = true;
+#else
+	smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ACPPowerOFF);
+	data->acp_power_gated = true;
+#endif
+
 }
 
 static void smu8_init_sclk_threshold(struct pp_hwmgr *hwmgr)
@@ -1885,6 +1890,19 @@ static int smu8_enable_disable_vce_dpm(struct pp_hwmgr *hwmgr, bool enable)
 }
 
 
+static void smu8_dpm_powergate_acp(struct pp_hwmgr *hwmgr, bool bgate)
+{
+	struct smu8_hwmgr *data = hwmgr->backend;
+
+	if (data->acp_power_gated == bgate)
+		return;
+
+	if (bgate)
+		smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ACPPowerOFF);
+	else
+		smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ACPPowerON);
+}
+
 static void smu8_dpm_powergate_uvd(struct pp_hwmgr *hwmgr, bool bgate)
 {
 	struct smu8_hwmgr *data = hwmgr->backend;
@@ -1950,6 +1968,7 @@ static void smu8_dpm_powergate_vce(struct pp_hwmgr *hwmgr, bool bgate)
 	.powerdown_uvd = smu8_dpm_powerdown_uvd,
 	.powergate_uvd = smu8_dpm_powergate_uvd,
 	.powergate_vce = smu8_dpm_powergate_vce,
+	.powergate_acp = smu8_dpm_powergate_acp,
 	.get_mclk = smu8_dpm_get_mclk,
 	.get_sclk = smu8_dpm_get_sclk,
 	.patch_boot_state = smu8_dpm_patch_boot_state,
diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
index d3d9626..7e58a0d 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
@@ -247,6 +247,7 @@ struct pp_hwmgr_func {
 	int (*powerdown_uvd)(struct pp_hwmgr *hwmgr);
 	void (*powergate_vce)(struct pp_hwmgr *hwmgr, bool bgate);
 	void (*powergate_uvd)(struct pp_hwmgr *hwmgr, bool bgate);
+	void (*powergate_acp)(struct pp_hwmgr *hwmgr, bool bgate);
 	uint32_t (*get_mclk)(struct pp_hwmgr *hwmgr, bool low);
 	uint32_t (*get_sclk)(struct pp_hwmgr *hwmgr, bool low);
 	int (*power_state_set)(struct pp_hwmgr *hwmgr,
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amdgpu: Power down acp if board uses AZ

[Rex Zhu]

if board uses AZ rather than ACP, we power down acp
through smu to save power.

Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
index 71efcf3..4e18a88 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
@@ -35,6 +35,7 @@
 
 #include "acp_gfx_if.h"
 
+
 #define ACP_TILE_ON_MASK                	0x03
 #define ACP_TILE_OFF_MASK               	0x02
 #define ACP_TILE_ON_RETAIN_REG_MASK     	0x1f
@@ -289,11 +290,12 @@ static int acp_hw_init(void *handle)
 	r = amd_acp_hw_init(adev->acp.cgs_device,
 			    ip_block->version->major, ip_block->version->minor);
 	/* -ENODEV means board uses AZ rather than ACP */
-	if (r == -ENODEV)
+	if (r == -ENODEV) {
+		amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_ACP, true);
 		return 0;
-	else if (r)
+	} else if (r) {
 		return r;
-
+	}
 	if (adev->rmmio_size == 0 || adev->rmmio_size < 0x5289)
 		return -EINVAL;
 
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: Power down acp if board uses AZ

[Alex Deucher]

On Thu, Jul 19, 2018 at 4:46 AM, Rex Zhu <rex.zhu@amd.com> wrote:
> if board uses AZ rather than ACP, we power down acp
> through smu to save power.
>

We also need to power it back up in hw_fini and suspend and then power
it back down in resume.

Alex

> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> index 71efcf3..4e18a88 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> @@ -35,6 +35,7 @@
>
>  #include "acp_gfx_if.h"
>
> +
>  #define ACP_TILE_ON_MASK                       0x03
>  #define ACP_TILE_OFF_MASK                      0x02
>  #define ACP_TILE_ON_RETAIN_REG_MASK            0x1f
> @@ -289,11 +290,12 @@ static int acp_hw_init(void *handle)
>         r = amd_acp_hw_init(adev->acp.cgs_device,
>                             ip_block->version->major, ip_block->version->minor);
>         /* -ENODEV means board uses AZ rather than ACP */
> -       if (r == -ENODEV)
> +       if (r == -ENODEV) {
> +               amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_ACP, true);
>                 return 0;
> -       else if (r)
> +       } else if (r) {
>                 return r;
> -
> +       }
>         if (adev->rmmio_size == 0 || adev->rmmio_size < 0x5289)
>                 return -EINVAL;
>
> --
> 1.9.1
>
> _______________________________________________
> 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

Re: [PATCH 1/2] drm/amd/pp: Add ACP PG support in SMU

[Alex Deucher]

On Thu, Jul 19, 2018 at 4:46 AM, Rex Zhu <rex.zhu@amd.com> wrote:
> when ACP block not enabled, we power off
> acp block to save power.
>
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c    | 18 ++++++++++++++++++
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c | 21 ++++++++++++++++++++-
>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h        |  1 +
>  3 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> index 7a646f9..da4ebff 100644
> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> @@ -1181,6 +1181,21 @@ static int pp_dpm_powergate_gfx(void *handle, bool gate)
>         return hwmgr->hwmgr_func->powergate_gfx(hwmgr, gate);
>  }
>
> +static void pp_dpm_powergate_acp(void *handle, bool gate)
> +{
> +       struct pp_hwmgr *hwmgr = handle;
> +
> +       if (!hwmgr || !hwmgr->pm_en)
> +               return;
> +
> +       if (hwmgr->hwmgr_func->powergate_acp == NULL) {
> +               pr_info("%s was not implemented.\n", __func__);
> +               return;
> +       }
> +
> +       hwmgr->hwmgr_func->powergate_acp(hwmgr, gate);
> +}
> +
>  static int pp_set_powergating_by_smu(void *handle,
>                                 uint32_t block_type, bool gate)
>  {
> @@ -1200,6 +1215,9 @@ static int pp_set_powergating_by_smu(void *handle,
>         case AMD_IP_BLOCK_TYPE_GFX:
>                 ret = pp_dpm_powergate_gfx(handle, gate);
>                 break;
> +       case AMD_IP_BLOCK_TYPE_ACP:
> +               pp_dpm_powergate_acp(handle, gate);
> +               break;
>         default:
>                 break;
>         }
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
> index 288802f..cafa822 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
> @@ -663,8 +663,13 @@ static void smu8_init_power_gate_state(struct pp_hwmgr *hwmgr)
>         data->uvd_power_gated = false;
>         data->vce_power_gated = false;
>         data->samu_power_gated = false;
> +#ifdef CONFIG_DRM_AMD_ACP
>         data->acp_power_gated = false;
> -       data->pgacpinit = true;
> +#else
> +       smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ACPPowerOFF);
> +       data->acp_power_gated = true;
> +#endif
> +
>  }
>
>  static void smu8_init_sclk_threshold(struct pp_hwmgr *hwmgr)
> @@ -1885,6 +1890,19 @@ static int smu8_enable_disable_vce_dpm(struct pp_hwmgr *hwmgr, bool enable)
>  }
>
>
> +static void smu8_dpm_powergate_acp(struct pp_hwmgr *hwmgr, bool bgate)
> +{
> +       struct smu8_hwmgr *data = hwmgr->backend;
> +
> +       if (data->acp_power_gated == bgate)
> +               return;
> +
> +       if (bgate)
> +               smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ACPPowerOFF);
> +       else
> +               smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ACPPowerON);
> +}
> +
>  static void smu8_dpm_powergate_uvd(struct pp_hwmgr *hwmgr, bool bgate)
>  {
>         struct smu8_hwmgr *data = hwmgr->backend;
> @@ -1950,6 +1968,7 @@ static void smu8_dpm_powergate_vce(struct pp_hwmgr *hwmgr, bool bgate)
>         .powerdown_uvd = smu8_dpm_powerdown_uvd,
>         .powergate_uvd = smu8_dpm_powergate_uvd,
>         .powergate_vce = smu8_dpm_powergate_vce,
> +       .powergate_acp = smu8_dpm_powergate_acp,
>         .get_mclk = smu8_dpm_get_mclk,
>         .get_sclk = smu8_dpm_get_sclk,
>         .patch_boot_state = smu8_dpm_patch_boot_state,
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> index d3d9626..7e58a0d 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> @@ -247,6 +247,7 @@ struct pp_hwmgr_func {
>         int (*powerdown_uvd)(struct pp_hwmgr *hwmgr);
>         void (*powergate_vce)(struct pp_hwmgr *hwmgr, bool bgate);
>         void (*powergate_uvd)(struct pp_hwmgr *hwmgr, bool bgate);
> +       void (*powergate_acp)(struct pp_hwmgr *hwmgr, bool bgate);
>         uint32_t (*get_mclk)(struct pp_hwmgr *hwmgr, bool low);
>         uint32_t (*get_sclk)(struct pp_hwmgr *hwmgr, bool low);
>         int (*power_state_set)(struct pp_hwmgr *hwmgr,
> --
> 1.9.1
>
> _______________________________________________
> 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

Re: [PATCH 2/2] drm/amdgpu: Power down acp if board uses AZ

[Zhu, Rex]

[-- Attachment #1.1: Type: text/plain, Size: 2556 bytes --]

>We also need to power it back up in hw_fini and suspend and then power
>it back down in resume.

Yes, this logic will be added in acp block.
In this patch, we only power down acp when it was not used and have no inpact wen s3.

Best Regards
Rex

________________________________
From: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Thursday, July 19, 2018 9:56 PM
To: Zhu, Rex
Cc: amd-gfx list
Subject: Re: [PATCH 2/2] drm/amdgpu: Power down acp if board uses AZ

On Thu, Jul 19, 2018 at 4:46 AM, Rex Zhu <rex.zhu-5C7GfCeVMHo@public.gmane.org> wrote:
> if board uses AZ rather than ACP, we power down acp
> through smu to save power.
>

We also need to power it back up in hw_fini and suspend and then power
it back down in resume.

Alex

> Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> index 71efcf3..4e18a88 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> @@ -35,6 +35,7 @@
>
>  #include "acp_gfx_if.h"
>
> +
>  #define ACP_TILE_ON_MASK                       0x03
>  #define ACP_TILE_OFF_MASK                      0x02
>  #define ACP_TILE_ON_RETAIN_REG_MASK            0x1f
> @@ -289,11 +290,12 @@ static int acp_hw_init(void *handle)
>         r = amd_acp_hw_init(adev->acp.cgs_device,
>                             ip_block->version->major, ip_block->version->minor);
>         /* -ENODEV means board uses AZ rather than ACP */
> -       if (r == -ENODEV)
> +       if (r == -ENODEV) {
> +               amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_ACP, true);
>                 return 0;
> -       else if (r)
> +       } else if (r) {
>                 return r;
> -
> +       }
>         if (adev->rmmio_size == 0 || adev->rmmio_size < 0x5289)
>                 return -EINVAL;
>
> --
> 1.9.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
lists.freedesktop.org
Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following form. Use of all freedesktop.org lists is subject to our Code of Conduct.




[-- Attachment #1.2: Type: text/html, Size: 7570 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: Power down acp if board uses AZ

[Alex Deucher]

[-- Attachment #1.1: Type: text/plain, Size: 3104 bytes --]

On Thu, Jul 19, 2018 at 11:14 AM, Zhu, Rex <Rex.Zhu-5C7GfCeVMHo@public.gmane.org> wrote:

> >We also need to power it back up in hw_fini and suspend and then power
> >it back down in resume.
>
> Yes, this logic will be added in acp block.
> In this patch, we only power down acp when it was not used and have no
> inpact wen s3.
>

But I think when the asic comes out of S3/S4, the ACP block will be powered
up again so it will have to be powered down again to avoid using power
after resume.  So if the platform uses AZ rather than ACP, we need to
handle that case and to power ACP back up on hw_fini() in case the driver
gets reloaded.  S4 works similarly.  Something like the attached patch.

Alex



>
> Best Regards
> Rex
>
> ------------------------------
> *From:* Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> *Sent:* Thursday, July 19, 2018 9:56 PM
> *To:* Zhu, Rex
> *Cc:* amd-gfx list
> *Subject:* Re: [PATCH 2/2] drm/amdgpu: Power down acp if board uses AZ
>
> On Thu, Jul 19, 2018 at 4:46 AM, Rex Zhu <rex.zhu-5C7GfCeVMHo@public.gmane.org> wrote:
> > if board uses AZ rather than ACP, we power down acp
> > through smu to save power.
> >
>
> We also need to power it back up in hw_fini and suspend and then power
> it back down in resume.
>
> Alex
>
> > Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> > index 71efcf3..4e18a88 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> > @@ -35,6 +35,7 @@
> >
> >  #include "acp_gfx_if.h"
> >
> > +
> >  #define ACP_TILE_ON_MASK                       0x03
> >  #define ACP_TILE_OFF_MASK                      0x02
> >  #define ACP_TILE_ON_RETAIN_REG_MASK            0x1f
> > @@ -289,11 +290,12 @@ static int acp_hw_init(void *handle)
> >         r = amd_acp_hw_init(adev->acp.cgs_device,
> >                             ip_block->version->major,
> ip_block->version->minor);
> >         /* -ENODEV means board uses AZ rather than ACP */
> > -       if (r == -ENODEV)
> > +       if (r == -ENODEV) {
> > +               amdgpu_dpm_set_powergating_by_smu(adev,
> AMD_IP_BLOCK_TYPE_ACP, true);
> >                 return 0;
> > -       else if (r)
> > +       } else if (r) {
> >                 return r;
> > -
> > +       }
> >         if (adev->rmmio_size == 0 || adev->rmmio_size < 0x5289)
> >                 return -EINVAL;
> >
> > --
> > 1.9.1
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - freedesktop.org
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following
> form. Use of all freedesktop.org lists is subject to our Code of Conduct.
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 8261 bytes --]

[-- Attachment #2: 0001-drm-amdgpu-Power-down-acp-if-board-uses-AZ-v2.patch --]
[-- Type: text/x-patch, Size: 2228 bytes --]

From 73f4669a60f0def1c5dc61f99a201f88c6d0e528 Mon Sep 17 00:00:00 2001
From: Rex Zhu <Rex.Zhu@amd.com>
Date: Thu, 19 Jul 2018 11:48:36 -0500
Subject: [PATCH] drm/amdgpu: Power down acp if board uses AZ (v2)

if board uses AZ rather than ACP, we power down acp
through smu to save power.

v2: handle S3/S4 and hw_fini (Alex)

Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
index 71efcf38f11b..d4d1738da3b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
@@ -289,10 +289,12 @@ static int acp_hw_init(void *handle)
 	r = amd_acp_hw_init(adev->acp.cgs_device,
 			    ip_block->version->major, ip_block->version->minor);
 	/* -ENODEV means board uses AZ rather than ACP */
-	if (r == -ENODEV)
+	if (r == -ENODEV) {
+		amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_ACP, true);
 		return 0;
-	else if (r)
+	} else if (r) {
 		return r;
+	}
 
 	if (adev->rmmio_size == 0 || adev->rmmio_size < 0x5289)
 		return -EINVAL;
@@ -497,8 +499,10 @@ static int acp_hw_fini(void *handle)
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
 	/* return early if no ACP */
-	if (!adev->acp.acp_cell)
+	if (!adev->acp.acp_cell) {
+		amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_ACP, false);
 		return 0;
+	}
 
 	/* Assert Soft reset of ACP */
 	val = cgs_read_register(adev->acp.cgs_device, mmACP_SOFT_RESET);
@@ -556,11 +560,21 @@ static int acp_hw_fini(void *handle)
 
 static int acp_suspend(void *handle)
 {
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+	/* power up on suspend */
+	if (!adev->acp.acp_cell)
+		amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_ACP, false);
 	return 0;
 }
 
 static int acp_resume(void *handle)
 {
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+	/* power down again on resume */
+	if (!adev->acp.acp_cell)
+		amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_ACP, true);
 	return 0;
 }
 
-- 
2.13.6


[-- Attachment #3: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: Power down acp if board uses AZ

[Zhu, Rex]

[-- Attachment #1.1: Type: text/plain, Size: 3837 bytes --]

Hi Alex,


I am Ok with the patch in v2.

just one question,


+    if (!adev->acp.acp_cell)
+        amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_ACP, true);


Do we need to check adev->acp.acp_cell before poweron/off ACP?




Best Regards

Rex

________________________________
From: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Friday, July 20, 2018 12:53:37 AM
To: Zhu, Rex
Cc: amd-gfx list
Subject: Re: [PATCH 2/2] drm/amdgpu: Power down acp if board uses AZ


On Thu, Jul 19, 2018 at 11:14 AM, Zhu, Rex <Rex.Zhu-5C7GfCeVMHo@public.gmane.org<mailto:Rex.Zhu@amd.com>> wrote:

>We also need to power it back up in hw_fini and suspend and then power
>it back down in resume.

Yes, this logic will be added in acp block.
In this patch, we only power down acp when it was not used and have no inpact wen s3.

But I think when the asic comes out of S3/S4, the ACP block will be powered up again so it will have to be powered down again to avoid using power after resume.  So if the platform uses AZ rather than ACP, we need to handle that case and to power ACP back up on hw_fini() in case the driver gets reloaded.  S4 works similarly.  Something like the attached patch.

Alex



Best Regards
Rex

________________________________
From: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
Sent: Thursday, July 19, 2018 9:56 PM
To: Zhu, Rex
Cc: amd-gfx list
Subject: Re: [PATCH 2/2] drm/amdgpu: Power down acp if board uses AZ

On Thu, Jul 19, 2018 at 4:46 AM, Rex Zhu <rex.zhu-5C7GfCeVMHo@public.gmane.org<mailto:rex.zhu@amd.com>> wrote:
> if board uses AZ rather than ACP, we power down acp
> through smu to save power.
>

We also need to power it back up in hw_fini and suspend and then power
it back down in resume.

Alex

> Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org<mailto:Rex.Zhu-5C7GfCeVMHo@public.gmane.org>>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> index 71efcf3..4e18a88 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> @@ -35,6 +35,7 @@
>
>  #include "acp_gfx_if.h"
>
> +
>  #define ACP_TILE_ON_MASK                       0x03
>  #define ACP_TILE_OFF_MASK                      0x02
>  #define ACP_TILE_ON_RETAIN_REG_MASK            0x1f
> @@ -289,11 +290,12 @@ static int acp_hw_init(void *handle)
>         r = amd_acp_hw_init(adev->acp.cgs_device,
>                             ip_block->version->major, ip_block->version->minor);
>         /* -ENODEV means board uses AZ rather than ACP */
> -       if (r == -ENODEV)
> +       if (r == -ENODEV) {
> +               amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_ACP, true);
>                 return 0;
> -       else if (r)
> +       } else if (r) {
>                 return r;
> -
> +       }
>         if (adev->rmmio_size == 0 || adev->rmmio_size < 0x5289)
>                 return -EINVAL;
>
> --
> 1.9.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
lists.freedesktop.org<http://lists.freedesktop.org>
Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following form. Use of all freedesktop.org<http://freedesktop.org> lists is subject to our Code of Conduct.





[-- Attachment #1.2: Type: text/html, Size: 11170 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: Power down acp if board uses AZ

[Deucher, Alexander]

[-- Attachment #1.1: Type: text/plain, Size: 4382 bytes --]

We just check that as a way see whether we are using AZ audio (and gpu driver should control ACP power) or i2s audio (and acp driver controls power).  In the former case, there is no structure allocated.


Alex

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Zhu, Rex <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
Sent: Friday, July 20, 2018 6:31:50 AM
To: Alex Deucher
Cc: amd-gfx list
Subject: Re: [PATCH 2/2] drm/amdgpu: Power down acp if board uses AZ


Hi Alex,


I am Ok with the patch in v2.

just one question,


+    if (!adev->acp.acp_cell)
+        amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_ACP, true);


Do we need to check adev->acp.acp_cell before poweron/off ACP?




Best Regards

Rex

________________________________
From: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Friday, July 20, 2018 12:53:37 AM
To: Zhu, Rex
Cc: amd-gfx list
Subject: Re: [PATCH 2/2] drm/amdgpu: Power down acp if board uses AZ


On Thu, Jul 19, 2018 at 11:14 AM, Zhu, Rex <Rex.Zhu-5C7GfCeVMHo@public.gmane.org<mailto:Rex.Zhu@amd.com>> wrote:

>We also need to power it back up in hw_fini and suspend and then power
>it back down in resume.

Yes, this logic will be added in acp block.
In this patch, we only power down acp when it was not used and have no inpact wen s3.

But I think when the asic comes out of S3/S4, the ACP block will be powered up again so it will have to be powered down again to avoid using power after resume.  So if the platform uses AZ rather than ACP, we need to handle that case and to power ACP back up on hw_fini() in case the driver gets reloaded.  S4 works similarly.  Something like the attached patch.

Alex



Best Regards
Rex

________________________________
From: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
Sent: Thursday, July 19, 2018 9:56 PM
To: Zhu, Rex
Cc: amd-gfx list
Subject: Re: [PATCH 2/2] drm/amdgpu: Power down acp if board uses AZ

On Thu, Jul 19, 2018 at 4:46 AM, Rex Zhu <rex.zhu-5C7GfCeVMHo@public.gmane.org<mailto:rex.zhu@amd.com>> wrote:
> if board uses AZ rather than ACP, we power down acp
> through smu to save power.
>

We also need to power it back up in hw_fini and suspend and then power
it back down in resume.

Alex

> Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org<mailto:Rex.Zhu-5C7GfCeVMHo@public.gmane.org>>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> index 71efcf3..4e18a88 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> @@ -35,6 +35,7 @@
>
>  #include "acp_gfx_if.h"
>
> +
>  #define ACP_TILE_ON_MASK                       0x03
>  #define ACP_TILE_OFF_MASK                      0x02
>  #define ACP_TILE_ON_RETAIN_REG_MASK            0x1f
> @@ -289,11 +290,12 @@ static int acp_hw_init(void *handle)
>         r = amd_acp_hw_init(adev->acp.cgs_device,
>                             ip_block->version->major, ip_block->version->minor);
>         /* -ENODEV means board uses AZ rather than ACP */
> -       if (r == -ENODEV)
> +       if (r == -ENODEV) {
> +               amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_ACP, true);
>                 return 0;
> -       else if (r)
> +       } else if (r) {
>                 return r;
> -
> +       }
>         if (adev->rmmio_size == 0 || adev->rmmio_size < 0x5289)
>                 return -EINVAL;
>
> --
> 1.9.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
lists.freedesktop.org<http://lists.freedesktop.org>
Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following form. Use of all freedesktop.org<http://freedesktop.org> lists is subject to our Code of Conduct.





[-- Attachment #1.2: Type: text/html, Size: 12450 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx