[13/26] drm/amd/powerplay: add limit of pp_feature for smu

Submitted by Huang, Ray on Feb. 25, 2019, 12:12 p.m.

Details

Message ID 1551096752-18205-14-git-send-email-ray.huang@amd.com
State New
Headers show
Series "Updates for SW SMU driver" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Huang, Ray Feb. 25, 2019, 12:12 p.m.
From: Likun Gao <Likun.Gao@amd.com>

Move pp_feature from the struct of amd_powerplay to amdgpu_device.
Add pp_feature limit for overdrive interface.

Signed-off-by: Likun Gao <Likun.Gao@amd.com>
Reviewed-by: Kevin Wang <kevin1.wang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h            | 4 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c     | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c        | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c         | 6 ++++--
 drivers/gpu/drm/amd/amdgpu/kv_dpm.c            | 2 +-
 drivers/gpu/drm/amd/amdgpu/soc15.c             | 2 +-
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 2 +-
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c     | 4 ++++
 drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 3 +++
 9 files changed, 19 insertions(+), 8 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index d1c02fa..f96b6e9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -706,7 +706,6 @@  enum amd_hw_ip_block_type {
 struct amd_powerplay {
 	void *pp_handle;
 	const struct amd_pm_funcs *pp_funcs;
-	uint32_t pp_feature;
 };
 
 #define AMDGPU_RESET_MAGIC_NUM 64
@@ -842,6 +841,9 @@  struct amdgpu_device {
 	/* interrupts */
 	struct amdgpu_irq		irq;
 
+	/* powerplay feature */
+	uint32_t pp_feature;
+
 	/* powerplay */
 	struct amd_powerplay		powerplay;
 	bool				pp_force_state_enabled;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index fcab1fe..c8fe5e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1506,7 +1506,7 @@  static int amdgpu_device_ip_early_init(struct amdgpu_device *adev)
 			return -EAGAIN;
 	}
 
-	adev->powerplay.pp_feature = amdgpu_pp_feature_mask;
+	adev->pp_feature = amdgpu_pp_feature_mask;
 
 	for (i = 0; i < adev->num_ip_blocks; i++) {
 		if ((amdgpu_ip_block_mask & (1 << i)) == 0) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 97a60da..bcc732d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -390,7 +390,7 @@  void amdgpu_gfx_compute_mqd_sw_fini(struct amdgpu_device *adev)
 
 void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
 {
-	if (!(adev->powerplay.pp_feature & PP_GFXOFF_MASK))
+	if (!(adev->pp_feature & PP_GFXOFF_MASK))
 		return;
 
 	if (!adev->powerplay.pp_funcs || !adev->powerplay.pp_funcs->set_powergating_by_smu)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 6bc80c1..fe1b0c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -2558,7 +2558,8 @@  int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
 				"pp_power_profile_mode\n");
 		return ret;
 	}
-	if (is_support_sw_smu(adev) || hwmgr->od_enabled) {
+	if ((is_support_sw_smu(adev) && adev->smu.od_enabled) ||
+	    (!is_support_sw_smu(adev) && hwmgr->od_enabled)) {
 		ret = device_create_file(adev->dev,
 				&dev_attr_pp_od_clk_voltage);
 		if (ret) {
@@ -2634,7 +2635,8 @@  void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
 	device_remove_file(adev->dev, &dev_attr_pp_mclk_od);
 	device_remove_file(adev->dev,
 			&dev_attr_pp_power_profile_mode);
-	if (hwmgr->od_enabled)
+	if ((is_support_sw_smu(adev) && adev->smu.od_enabled) ||
+	    (!is_support_sw_smu(adev) && hwmgr->od_enabled))
 		device_remove_file(adev->dev,
 				&dev_attr_pp_od_clk_voltage);
 	device_remove_file(adev->dev, &dev_attr_gpu_busy_percent);
diff --git a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c
index 0c9a2c0..9022f42 100644
--- a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c
+++ b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c
@@ -2824,7 +2824,7 @@  static int kv_dpm_init(struct amdgpu_device *adev)
 		pi->caps_tcp_ramping = true;
 	}
 
-	if (adev->powerplay.pp_feature & PP_SCLK_DEEP_SLEEP_MASK)
+	if (adev->pp_feature & PP_SCLK_DEEP_SLEEP_MASK)
 		pi->caps_sclk_ds = true;
 	else
 		pi->caps_sclk_ds = false;
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
index 9f6ce6e..c296f6c 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -933,7 +933,7 @@  static int soc15_common_early_init(void *handle)
 			adev->pg_flags = AMD_PG_SUPPORT_SDMA | AMD_PG_SUPPORT_VCN;
 		}
 
-		if (adev->powerplay.pp_feature & PP_GFXOFF_MASK)
+		if (adev->pp_feature & PP_GFXOFF_MASK)
 			adev->pg_flags |= AMD_PG_SUPPORT_GFX_PG |
 				AMD_PG_SUPPORT_CP |
 				AMD_PG_SUPPORT_RLC_SMU_HS;
diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
index 3f73f7c..ea5689a 100644
--- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
@@ -53,7 +53,7 @@  static int amd_powerplay_create(struct amdgpu_device *adev)
 	mutex_init(&hwmgr->smu_lock);
 	hwmgr->chip_family = adev->family;
 	hwmgr->chip_id = adev->asic_type;
-	hwmgr->feature_mask = adev->powerplay.pp_feature;
+	hwmgr->feature_mask = adev->pp_feature;
 	hwmgr->display_config = &adev->pm.pm_display_cfg;
 	adev->powerplay.pp_handle = hwmgr;
 	adev->powerplay.pp_funcs = &pp_dpm_funcs;
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 9cb45fe..fa0af71 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -290,6 +290,9 @@  static int smu_set_funcs(struct amdgpu_device *adev)
 
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
+		smu->feature_mask &= ~PP_GFXOFF_MASK;
+		if (smu->feature_mask & PP_OVERDRIVE_MASK)
+			smu->od_enabled = true;
 		smu_v11_0_set_smu_funcs(smu);
 		break;
 	default:
@@ -306,6 +309,7 @@  static int smu_early_init(void *handle)
 
 	smu->adev = adev;
 	mutex_init(&smu->mutex);
+	smu->feature_mask = adev->pp_feature;
 
 	return smu_set_funcs(adev);
 }
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
index 00ef6f1..8c7eac9 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -384,6 +384,7 @@  struct smu_context
 	uint32_t pstate_sclk;
 	uint32_t pstate_mclk;
 
+	bool od_enabled;
 	uint32_t power_limit;
 	uint32_t default_power_limit;
 
@@ -399,6 +400,8 @@  struct smu_context
 	uint32_t workload_setting[WORKLOAD_POLICY_MAX];
 	uint32_t power_profile_mode;
 	uint32_t default_power_profile_mode;
+
+	uint32_t feature_mask;
 };
 
 struct pptable_funcs {

Comments

On Mon, Feb 25, 2019 at 7:13 AM Huang Rui <ray.huang@amd.com> wrote:
>
> From: Likun Gao <Likun.Gao@amd.com>
>
> Move pp_feature from the struct of amd_powerplay to amdgpu_device.

I think we can probably drop this change.  If you do want to move it
out of powerplay, maybe put it in struct amdgpu_pm?  I don't like
adding random stuff to amdgpu_device.

> Add pp_feature limit for overdrive interface.
>
> Signed-off-by: Likun Gao <Likun.Gao@amd.com>
> Reviewed-by: Kevin Wang <kevin1.wang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h            | 4 +++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c     | 2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c        | 2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c         | 6 ++++--
>  drivers/gpu/drm/amd/amdgpu/kv_dpm.c            | 2 +-
>  drivers/gpu/drm/amd/amdgpu/soc15.c             | 2 +-
>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 2 +-
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c     | 4 ++++
>  drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 3 +++
>  9 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index d1c02fa..f96b6e9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -706,7 +706,6 @@ enum amd_hw_ip_block_type {
>  struct amd_powerplay {
>         void *pp_handle;
>         const struct amd_pm_funcs *pp_funcs;
> -       uint32_t pp_feature;
>  };
>
>  #define AMDGPU_RESET_MAGIC_NUM 64
> @@ -842,6 +841,9 @@ struct amdgpu_device {
>         /* interrupts */
>         struct amdgpu_irq               irq;
>
> +       /* powerplay feature */
> +       uint32_t pp_feature;
> +
>         /* powerplay */
>         struct amd_powerplay            powerplay;
>         bool                            pp_force_state_enabled;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index fcab1fe..c8fe5e5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1506,7 +1506,7 @@ static int amdgpu_device_ip_early_init(struct amdgpu_device *adev)
>                         return -EAGAIN;
>         }
>
> -       adev->powerplay.pp_feature = amdgpu_pp_feature_mask;
> +       adev->pp_feature = amdgpu_pp_feature_mask;
>
>         for (i = 0; i < adev->num_ip_blocks; i++) {
>                 if ((amdgpu_ip_block_mask & (1 << i)) == 0) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 97a60da..bcc732d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -390,7 +390,7 @@ void amdgpu_gfx_compute_mqd_sw_fini(struct amdgpu_device *adev)
>
>  void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
>  {
> -       if (!(adev->powerplay.pp_feature & PP_GFXOFF_MASK))
> +       if (!(adev->pp_feature & PP_GFXOFF_MASK))
>                 return;
>
>         if (!adev->powerplay.pp_funcs || !adev->powerplay.pp_funcs->set_powergating_by_smu)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 6bc80c1..fe1b0c4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -2558,7 +2558,8 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
>                                 "pp_power_profile_mode\n");
>                 return ret;
>         }
> -       if (is_support_sw_smu(adev) || hwmgr->od_enabled) {
> +       if ((is_support_sw_smu(adev) && adev->smu.od_enabled) ||
> +           (!is_support_sw_smu(adev) && hwmgr->od_enabled)) {
>                 ret = device_create_file(adev->dev,
>                                 &dev_attr_pp_od_clk_voltage);
>                 if (ret) {
> @@ -2634,7 +2635,8 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
>         device_remove_file(adev->dev, &dev_attr_pp_mclk_od);
>         device_remove_file(adev->dev,
>                         &dev_attr_pp_power_profile_mode);
> -       if (hwmgr->od_enabled)
> +       if ((is_support_sw_smu(adev) && adev->smu.od_enabled) ||
> +           (!is_support_sw_smu(adev) && hwmgr->od_enabled))
>                 device_remove_file(adev->dev,
>                                 &dev_attr_pp_od_clk_voltage);
>         device_remove_file(adev->dev, &dev_attr_gpu_busy_percent);
> diff --git a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c
> index 0c9a2c0..9022f42 100644
> --- a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c
> @@ -2824,7 +2824,7 @@ static int kv_dpm_init(struct amdgpu_device *adev)
>                 pi->caps_tcp_ramping = true;
>         }
>
> -       if (adev->powerplay.pp_feature & PP_SCLK_DEEP_SLEEP_MASK)
> +       if (adev->pp_feature & PP_SCLK_DEEP_SLEEP_MASK)
>                 pi->caps_sclk_ds = true;
>         else
>                 pi->caps_sclk_ds = false;
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index 9f6ce6e..c296f6c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -933,7 +933,7 @@ static int soc15_common_early_init(void *handle)
>                         adev->pg_flags = AMD_PG_SUPPORT_SDMA | AMD_PG_SUPPORT_VCN;
>                 }
>
> -               if (adev->powerplay.pp_feature & PP_GFXOFF_MASK)
> +               if (adev->pp_feature & PP_GFXOFF_MASK)
>                         adev->pg_flags |= AMD_PG_SUPPORT_GFX_PG |
>                                 AMD_PG_SUPPORT_CP |
>                                 AMD_PG_SUPPORT_RLC_SMU_HS;
> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> index 3f73f7c..ea5689a 100644
> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> @@ -53,7 +53,7 @@ static int amd_powerplay_create(struct amdgpu_device *adev)
>         mutex_init(&hwmgr->smu_lock);
>         hwmgr->chip_family = adev->family;
>         hwmgr->chip_id = adev->asic_type;
> -       hwmgr->feature_mask = adev->powerplay.pp_feature;
> +       hwmgr->feature_mask = adev->pp_feature;
>         hwmgr->display_config = &adev->pm.pm_display_cfg;
>         adev->powerplay.pp_handle = hwmgr;
>         adev->powerplay.pp_funcs = &pp_dpm_funcs;
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 9cb45fe..fa0af71 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -290,6 +290,9 @@ static int smu_set_funcs(struct amdgpu_device *adev)
>
>         switch (adev->asic_type) {
>         case CHIP_VEGA20:
> +               smu->feature_mask &= ~PP_GFXOFF_MASK;
> +               if (smu->feature_mask & PP_OVERDRIVE_MASK)
> +                       smu->od_enabled = true;
>                 smu_v11_0_set_smu_funcs(smu);
>                 break;
>         default:
> @@ -306,6 +309,7 @@ static int smu_early_init(void *handle)
>
>         smu->adev = adev;
>         mutex_init(&smu->mutex);
> +       smu->feature_mask = adev->pp_feature;
>
>         return smu_set_funcs(adev);
>  }
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index 00ef6f1..8c7eac9 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -384,6 +384,7 @@ struct smu_context
>         uint32_t pstate_sclk;
>         uint32_t pstate_mclk;
>
> +       bool od_enabled;
>         uint32_t power_limit;
>         uint32_t default_power_limit;
>
> @@ -399,6 +400,8 @@ struct smu_context
>         uint32_t workload_setting[WORKLOAD_POLICY_MAX];
>         uint32_t power_profile_mode;
>         uint32_t default_power_profile_mode;
> +
> +       uint32_t feature_mask;
>  };
>
>  struct pptable_funcs {
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> -----Original Message-----

> From: Alex Deucher [mailto:alexdeucher@gmail.com]

> Sent: Tuesday, February 26, 2019 12:08 PM

> To: Huang, Ray <Ray.Huang@amd.com>

> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Gao, Likun

> <Likun.Gao@amd.com>; Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; Gui,

> Jack <Jack.Gui@amd.com>

> Subject: Re: [PATCH 13/26] drm/amd/powerplay: add limit of pp_feature for

> smu

> 

> On Mon, Feb 25, 2019 at 7:13 AM Huang Rui <ray.huang@amd.com> wrote:

> >

> > From: Likun Gao <Likun.Gao@amd.com>

> >

> > Move pp_feature from the struct of amd_powerplay to amdgpu_device.

> 

> I think we can probably drop this change.  If you do want to move it out of

> powerplay, maybe put it in struct amdgpu_pm?  I don't like adding random

> stuff to amdgpu_device.


While we init sw smu, the powerplay structure won't be initialized. So we move the pp_feature out of powerplay.
And yes, putting it in struct amdgpu_pm is better than in struct amdgpu_device.

Thanks,
Ray

> 

> > Add pp_feature limit for overdrive interface.

> >

> > Signed-off-by: Likun Gao <Likun.Gao@amd.com>

> > Reviewed-by: Kevin Wang <kevin1.wang@amd.com>

> > ---

> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h            | 4 +++-

> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c     | 2 +-

> >  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c        | 2 +-

> >  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c         | 6 ++++--

> >  drivers/gpu/drm/amd/amdgpu/kv_dpm.c            | 2 +-

> >  drivers/gpu/drm/amd/amdgpu/soc15.c             | 2 +-

> >  drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 2 +-

> >  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c     | 4 ++++

> >  drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 3 +++

> >  9 files changed, 19 insertions(+), 8 deletions(-)

> >

> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> > index d1c02fa..f96b6e9 100644

> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> > @@ -706,7 +706,6 @@ enum amd_hw_ip_block_type {  struct

> amd_powerplay

> > {

> >         void *pp_handle;

> >         const struct amd_pm_funcs *pp_funcs;

> > -       uint32_t pp_feature;

> >  };

> >

> >  #define AMDGPU_RESET_MAGIC_NUM 64

> > @@ -842,6 +841,9 @@ struct amdgpu_device {

> >         /* interrupts */

> >         struct amdgpu_irq               irq;

> >

> > +       /* powerplay feature */

> > +       uint32_t pp_feature;

> > +

> >         /* powerplay */

> >         struct amd_powerplay            powerplay;

> >         bool                            pp_force_state_enabled;

> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> > index fcab1fe..c8fe5e5 100644

> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> > @@ -1506,7 +1506,7 @@ static int amdgpu_device_ip_early_init(struct

> amdgpu_device *adev)

> >                         return -EAGAIN;

> >         }

> >

> > -       adev->powerplay.pp_feature = amdgpu_pp_feature_mask;

> > +       adev->pp_feature = amdgpu_pp_feature_mask;

> >

> >         for (i = 0; i < adev->num_ip_blocks; i++) {

> >                 if ((amdgpu_ip_block_mask & (1 << i)) == 0) { diff

> > --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c

> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c

> > index 97a60da..bcc732d 100644

> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c

> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c

> > @@ -390,7 +390,7 @@ void amdgpu_gfx_compute_mqd_sw_fini(struct

> > amdgpu_device *adev)

> >

> >  void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)  {

> > -       if (!(adev->powerplay.pp_feature & PP_GFXOFF_MASK))

> > +       if (!(adev->pp_feature & PP_GFXOFF_MASK))

> >                 return;

> >

> >         if (!adev->powerplay.pp_funcs ||

> > !adev->powerplay.pp_funcs->set_powergating_by_smu)

> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c

> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c

> > index 6bc80c1..fe1b0c4 100644

> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c

> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c

> > @@ -2558,7 +2558,8 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device

> *adev)

> >                                 "pp_power_profile_mode\n");

> >                 return ret;

> >         }

> > -       if (is_support_sw_smu(adev) || hwmgr->od_enabled) {

> > +       if ((is_support_sw_smu(adev) && adev->smu.od_enabled) ||

> > +           (!is_support_sw_smu(adev) && hwmgr->od_enabled)) {

> >                 ret = device_create_file(adev->dev,

> >                                 &dev_attr_pp_od_clk_voltage);

> >                 if (ret) {

> > @@ -2634,7 +2635,8 @@ void amdgpu_pm_sysfs_fini(struct

> amdgpu_device *adev)

> >         device_remove_file(adev->dev, &dev_attr_pp_mclk_od);

> >         device_remove_file(adev->dev,

> >                         &dev_attr_pp_power_profile_mode);

> > -       if (hwmgr->od_enabled)

> > +       if ((is_support_sw_smu(adev) && adev->smu.od_enabled) ||

> > +           (!is_support_sw_smu(adev) && hwmgr->od_enabled))

> >                 device_remove_file(adev->dev,

> >                                 &dev_attr_pp_od_clk_voltage);

> >         device_remove_file(adev->dev, &dev_attr_gpu_busy_percent);

> > diff --git a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c

> > b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c

> > index 0c9a2c0..9022f42 100644

> > --- a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c

> > +++ b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c

> > @@ -2824,7 +2824,7 @@ static int kv_dpm_init(struct amdgpu_device

> *adev)

> >                 pi->caps_tcp_ramping = true;

> >         }

> >

> > -       if (adev->powerplay.pp_feature & PP_SCLK_DEEP_SLEEP_MASK)

> > +       if (adev->pp_feature & PP_SCLK_DEEP_SLEEP_MASK)

> >                 pi->caps_sclk_ds = true;

> >         else

> >                 pi->caps_sclk_ds = false; diff --git

> > a/drivers/gpu/drm/amd/amdgpu/soc15.c

> > b/drivers/gpu/drm/amd/amdgpu/soc15.c

> > index 9f6ce6e..c296f6c 100644

> > --- a/drivers/gpu/drm/amd/amdgpu/soc15.c

> > +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c

> > @@ -933,7 +933,7 @@ static int soc15_common_early_init(void *handle)

> >                         adev->pg_flags = AMD_PG_SUPPORT_SDMA |

> AMD_PG_SUPPORT_VCN;

> >                 }

> >

> > -               if (adev->powerplay.pp_feature & PP_GFXOFF_MASK)

> > +               if (adev->pp_feature & PP_GFXOFF_MASK)

> >                         adev->pg_flags |= AMD_PG_SUPPORT_GFX_PG |

> >                                 AMD_PG_SUPPORT_CP |

> >                                 AMD_PG_SUPPORT_RLC_SMU_HS; diff --git

> > a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c

> > b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c

> > index 3f73f7c..ea5689a 100644

> > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c

> > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c

> > @@ -53,7 +53,7 @@ static int amd_powerplay_create(struct

> amdgpu_device *adev)

> >         mutex_init(&hwmgr->smu_lock);

> >         hwmgr->chip_family = adev->family;

> >         hwmgr->chip_id = adev->asic_type;

> > -       hwmgr->feature_mask = adev->powerplay.pp_feature;

> > +       hwmgr->feature_mask = adev->pp_feature;

> >         hwmgr->display_config = &adev->pm.pm_display_cfg;

> >         adev->powerplay.pp_handle = hwmgr;

> >         adev->powerplay.pp_funcs = &pp_dpm_funcs; diff --git

> > a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c

> > b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c

> > index 9cb45fe..fa0af71 100644

> > --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c

> > +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c

> > @@ -290,6 +290,9 @@ static int smu_set_funcs(struct amdgpu_device

> > *adev)

> >

> >         switch (adev->asic_type) {

> >         case CHIP_VEGA20:

> > +               smu->feature_mask &= ~PP_GFXOFF_MASK;

> > +               if (smu->feature_mask & PP_OVERDRIVE_MASK)

> > +                       smu->od_enabled = true;

> >                 smu_v11_0_set_smu_funcs(smu);

> >                 break;

> >         default:

> > @@ -306,6 +309,7 @@ static int smu_early_init(void *handle)

> >

> >         smu->adev = adev;

> >         mutex_init(&smu->mutex);

> > +       smu->feature_mask = adev->pp_feature;

> >

> >         return smu_set_funcs(adev);

> >  }

> > diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h

> > b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h

> > index 00ef6f1..8c7eac9 100644

> > --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h

> > +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h

> > @@ -384,6 +384,7 @@ struct smu_context

> >         uint32_t pstate_sclk;

> >         uint32_t pstate_mclk;

> >

> > +       bool od_enabled;

> >         uint32_t power_limit;

> >         uint32_t default_power_limit;

> >

> > @@ -399,6 +400,8 @@ struct smu_context

> >         uint32_t workload_setting[WORKLOAD_POLICY_MAX];

> >         uint32_t power_profile_mode;

> >         uint32_t default_power_profile_mode;

> > +

> > +       uint32_t feature_mask;

> >  };

> >

> >  struct pptable_funcs {

> > --

> > 2.7.4

> >

> > _______________________________________________

> > amd-gfx mailing list

> > amd-gfx@lists.freedesktop.org

> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
OK, thanks for the advice. I will try to put pp_feature in the struct amdgpu_pm and send out the changed patch latter.

Regards,
Likun

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Huang, Ray

Sent: Tuesday, February 26, 2019 2:02 PM
To: Alex Deucher <alexdeucher@gmail.com>
Cc: Gao, Likun <Likun.Gao@amd.com>; Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; Gui, Jack <Jack.Gui@amd.com>; amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: RE: [PATCH 13/26] drm/amd/powerplay: add limit of pp_feature for smu

> -----Original Message-----

> From: Alex Deucher [mailto:alexdeucher@gmail.com]

> Sent: Tuesday, February 26, 2019 12:08 PM

> To: Huang, Ray <Ray.Huang@amd.com>

> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Gao, Likun 

> <Likun.Gao@amd.com>; Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; Gui, 

> Jack <Jack.Gui@amd.com>

> Subject: Re: [PATCH 13/26] drm/amd/powerplay: add limit of pp_feature 

> for smu

> 

> On Mon, Feb 25, 2019 at 7:13 AM Huang Rui <ray.huang@amd.com> wrote:

> >

> > From: Likun Gao <Likun.Gao@amd.com>

> >

> > Move pp_feature from the struct of amd_powerplay to amdgpu_device.

> 

> I think we can probably drop this change.  If you do want to move it 

> out of powerplay, maybe put it in struct amdgpu_pm?  I don't like 

> adding random stuff to amdgpu_device.


While we init sw smu, the powerplay structure won't be initialized. So we move the pp_feature out of powerplay.
And yes, putting it in struct amdgpu_pm is better than in struct amdgpu_device.

Thanks,
Ray

> 

> > Add pp_feature limit for overdrive interface.

> >

> > Signed-off-by: Likun Gao <Likun.Gao@amd.com>

> > Reviewed-by: Kevin Wang <kevin1.wang@amd.com>

> > ---

> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h            | 4 +++-

> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c     | 2 +-

> >  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c        | 2 +-

> >  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c         | 6 ++++--

> >  drivers/gpu/drm/amd/amdgpu/kv_dpm.c            | 2 +-

> >  drivers/gpu/drm/amd/amdgpu/soc15.c             | 2 +-

> >  drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 2 +-

> >  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c     | 4 ++++

> >  drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 3 +++

> >  9 files changed, 19 insertions(+), 8 deletions(-)

> >

> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> > index d1c02fa..f96b6e9 100644

> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

> > @@ -706,7 +706,6 @@ enum amd_hw_ip_block_type {  struct

> amd_powerplay

> > {

> >         void *pp_handle;

> >         const struct amd_pm_funcs *pp_funcs;

> > -       uint32_t pp_feature;

> >  };

> >

> >  #define AMDGPU_RESET_MAGIC_NUM 64

> > @@ -842,6 +841,9 @@ struct amdgpu_device {

> >         /* interrupts */

> >         struct amdgpu_irq               irq;

> >

> > +       /* powerplay feature */

> > +       uint32_t pp_feature;

> > +

> >         /* powerplay */

> >         struct amd_powerplay            powerplay;

> >         bool                            pp_force_state_enabled;

> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> > index fcab1fe..c8fe5e5 100644

> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

> > @@ -1506,7 +1506,7 @@ static int amdgpu_device_ip_early_init(struct

> amdgpu_device *adev)

> >                         return -EAGAIN;

> >         }

> >

> > -       adev->powerplay.pp_feature = amdgpu_pp_feature_mask;

> > +       adev->pp_feature = amdgpu_pp_feature_mask;

> >

> >         for (i = 0; i < adev->num_ip_blocks; i++) {

> >                 if ((amdgpu_ip_block_mask & (1 << i)) == 0) { diff 

> > --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c

> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c

> > index 97a60da..bcc732d 100644

> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c

> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c

> > @@ -390,7 +390,7 @@ void amdgpu_gfx_compute_mqd_sw_fini(struct

> > amdgpu_device *adev)

> >

> >  void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)  {

> > -       if (!(adev->powerplay.pp_feature & PP_GFXOFF_MASK))

> > +       if (!(adev->pp_feature & PP_GFXOFF_MASK))

> >                 return;

> >

> >         if (!adev->powerplay.pp_funcs ||

> > !adev->powerplay.pp_funcs->set_powergating_by_smu)

> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c

> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c

> > index 6bc80c1..fe1b0c4 100644

> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c

> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c

> > @@ -2558,7 +2558,8 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device

> *adev)

> >                                 "pp_power_profile_mode\n");

> >                 return ret;

> >         }

> > -       if (is_support_sw_smu(adev) || hwmgr->od_enabled) {

> > +       if ((is_support_sw_smu(adev) && adev->smu.od_enabled) ||

> > +           (!is_support_sw_smu(adev) && hwmgr->od_enabled)) {

> >                 ret = device_create_file(adev->dev,

> >                                 &dev_attr_pp_od_clk_voltage);

> >                 if (ret) {

> > @@ -2634,7 +2635,8 @@ void amdgpu_pm_sysfs_fini(struct

> amdgpu_device *adev)

> >         device_remove_file(adev->dev, &dev_attr_pp_mclk_od);

> >         device_remove_file(adev->dev,

> >                         &dev_attr_pp_power_profile_mode);

> > -       if (hwmgr->od_enabled)

> > +       if ((is_support_sw_smu(adev) && adev->smu.od_enabled) ||

> > +           (!is_support_sw_smu(adev) && hwmgr->od_enabled))

> >                 device_remove_file(adev->dev,

> >                                 &dev_attr_pp_od_clk_voltage);

> >         device_remove_file(adev->dev, &dev_attr_gpu_busy_percent); 

> > diff --git a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c

> > b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c

> > index 0c9a2c0..9022f42 100644

> > --- a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c

> > +++ b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c

> > @@ -2824,7 +2824,7 @@ static int kv_dpm_init(struct amdgpu_device

> *adev)

> >                 pi->caps_tcp_ramping = true;

> >         }

> >

> > -       if (adev->powerplay.pp_feature & PP_SCLK_DEEP_SLEEP_MASK)

> > +       if (adev->pp_feature & PP_SCLK_DEEP_SLEEP_MASK)

> >                 pi->caps_sclk_ds = true;

> >         else

> >                 pi->caps_sclk_ds = false; diff --git 

> > a/drivers/gpu/drm/amd/amdgpu/soc15.c

> > b/drivers/gpu/drm/amd/amdgpu/soc15.c

> > index 9f6ce6e..c296f6c 100644

> > --- a/drivers/gpu/drm/amd/amdgpu/soc15.c

> > +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c

> > @@ -933,7 +933,7 @@ static int soc15_common_early_init(void *handle)

> >                         adev->pg_flags = AMD_PG_SUPPORT_SDMA |

> AMD_PG_SUPPORT_VCN;

> >                 }

> >

> > -               if (adev->powerplay.pp_feature & PP_GFXOFF_MASK)

> > +               if (adev->pp_feature & PP_GFXOFF_MASK)

> >                         adev->pg_flags |= AMD_PG_SUPPORT_GFX_PG |

> >                                 AMD_PG_SUPPORT_CP |

> >                                 AMD_PG_SUPPORT_RLC_SMU_HS; diff 

> > --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c

> > b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c

> > index 3f73f7c..ea5689a 100644

> > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c

> > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c

> > @@ -53,7 +53,7 @@ static int amd_powerplay_create(struct

> amdgpu_device *adev)

> >         mutex_init(&hwmgr->smu_lock);

> >         hwmgr->chip_family = adev->family;

> >         hwmgr->chip_id = adev->asic_type;

> > -       hwmgr->feature_mask = adev->powerplay.pp_feature;

> > +       hwmgr->feature_mask = adev->pp_feature;

> >         hwmgr->display_config = &adev->pm.pm_display_cfg;

> >         adev->powerplay.pp_handle = hwmgr;

> >         adev->powerplay.pp_funcs = &pp_dpm_funcs; diff --git 

> > a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c

> > b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c

> > index 9cb45fe..fa0af71 100644

> > --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c

> > +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c

> > @@ -290,6 +290,9 @@ static int smu_set_funcs(struct amdgpu_device

> > *adev)

> >

> >         switch (adev->asic_type) {

> >         case CHIP_VEGA20:

> > +               smu->feature_mask &= ~PP_GFXOFF_MASK;

> > +               if (smu->feature_mask & PP_OVERDRIVE_MASK)

> > +                       smu->od_enabled = true;

> >                 smu_v11_0_set_smu_funcs(smu);

> >                 break;

> >         default:

> > @@ -306,6 +309,7 @@ static int smu_early_init(void *handle)

> >

> >         smu->adev = adev;

> >         mutex_init(&smu->mutex);

> > +       smu->feature_mask = adev->pp_feature;

> >

> >         return smu_set_funcs(adev);

> >  }

> > diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h

> > b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h

> > index 00ef6f1..8c7eac9 100644

> > --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h

> > +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h

> > @@ -384,6 +384,7 @@ struct smu_context

> >         uint32_t pstate_sclk;

> >         uint32_t pstate_mclk;

> >

> > +       bool od_enabled;

> >         uint32_t power_limit;

> >         uint32_t default_power_limit;

> >

> > @@ -399,6 +400,8 @@ struct smu_context

> >         uint32_t workload_setting[WORKLOAD_POLICY_MAX];

> >         uint32_t power_profile_mode;

> >         uint32_t default_power_profile_mode;

> > +

> > +       uint32_t feature_mask;

> >  };

> >

> >  struct pptable_funcs {

> > --

> > 2.7.4

> >

> > _______________________________________________

> > 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