All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] drm/amdgpu: use cached SMU feature mask in runpm
@ 2022-07-11 13:58 Guchun Chen
  2022-07-11 15:29 ` Alex Deucher
  0 siblings, 1 reply; 3+ messages in thread
From: Guchun Chen @ 2022-07-11 13:58 UTC (permalink / raw)
  To: amd-gfx, alexander.deucher, hawking.zhang, lijo.lazar, evan.quan,
	kenneth.feng
  Cc: Guchun Chen

SMU will perform dpm disablement when entering BACO,
and enable them later on, so talking to SMU to get
enabled features in runpm cycle as BACO support check
is not reliable. Hence, use a cached value to fix it.

Signed-off-by: Guchun Chen <guchun.chen@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       | 4 ++++
 drivers/gpu/drm/amd/pm/amdgpu_dpm.c           | 9 +++++++++
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h       | 1 +
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     | 5 +++++
 drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 3 +++
 drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c        | 8 +++++++-
 6 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 1cc9260e75de..dc2e78bb7224 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2478,6 +2478,10 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 	}
 
 	adev->in_runpm = true;
+
+	/* cache SMU feature mask */
+	amdgpu_dpm_set_cached_feature_mask(adev);
+
 	if (amdgpu_device_supports_px(drm_dev))
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
index 956b6ce81c84..211f73a987d6 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
@@ -1702,3 +1702,12 @@ int amdgpu_dpm_get_dpm_clock_table(struct amdgpu_device *adev,
 
 	return ret;
 }
+
+void amdgpu_dpm_set_cached_feature_mask(struct amdgpu_device *adev)
+{
+	struct smu_context *smu = adev->powerplay.pp_handle;
+
+	mutex_lock(&adev->pm.mutex);
+	smu_set_cached_enabled_mask(smu);
+	mutex_unlock(&adev->pm.mutex);
+}
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
index 524fb09437e5..e9c002a561c2 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
@@ -543,4 +543,5 @@ enum pp_smu_status amdgpu_dpm_get_uclk_dpm_states(struct amdgpu_device *adev,
 						  unsigned int *num_states);
 int amdgpu_dpm_get_dpm_clock_table(struct amdgpu_device *adev,
 				   struct dpm_clocks *clock_table);
+void amdgpu_dpm_set_cached_feature_mask(struct amdgpu_device *adev);
 #endif
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index fd79b213fab4..e8ead58a00b4 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -3130,3 +3130,8 @@ int smu_send_hbm_bad_channel_flag(struct smu_context *smu, uint32_t size)
 
 	return ret;
 }
+
+void smu_set_cached_enabled_mask(struct smu_context *smu)
+{
+	smu_feature_get_enabled_mask(smu, &smu->cache_enabled_mask);
+}
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
index b81c657c7386..678123b5e2bf 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
@@ -568,6 +568,8 @@ struct smu_context
 	u32 param_reg;
 	u32 msg_reg;
 	u32 resp_reg;
+
+	uint64_t cache_enabled_mask;
 };
 
 struct i2c_adapter;
@@ -1465,5 +1467,6 @@ int smu_stb_collect_info(struct smu_context *smu, void *buff, uint32_t size);
 void amdgpu_smu_stb_debug_fs_init(struct amdgpu_device *adev);
 int smu_send_hbm_bad_pages_num(struct smu_context *smu, uint32_t size);
 int smu_send_hbm_bad_channel_flag(struct smu_context *smu, uint32_t size);
+void smu_set_cached_enabled_mask(struct smu_context *smu);
 #endif
 #endif
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
index 15e4298c7cc8..b3087085622a 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
@@ -499,7 +499,13 @@ int smu_cmn_feature_is_enabled(struct smu_context *smu,
 	uint64_t enabled_features;
 	int feature_id;
 
-	if (__smu_get_enabled_features(smu, &enabled_features)) {
+	/* SMU will perform dpm disablement when entering BACO, and enable
+	 * them later on, so talking to SMU to get enabled features in runpm
+	 * stage is not reliable. Use a cache value for this instead to fix it.
+	 */
+	if (adev->in_runpm) {
+		enabled_features = smu->cache_enabled_mask;
+	} else if (__smu_get_enabled_features(smu, &enabled_features)) {
 		dev_err(adev->dev, "Failed to retrieve enabled ppfeatures!\n");
 		return 0;
 	}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: use cached SMU feature mask in runpm
  2022-07-11 13:58 [PATCH 2/2] drm/amdgpu: use cached SMU feature mask in runpm Guchun Chen
@ 2022-07-11 15:29 ` Alex Deucher
  2022-07-12  3:55   ` Chen, Guchun
  0 siblings, 1 reply; 3+ messages in thread
From: Alex Deucher @ 2022-07-11 15:29 UTC (permalink / raw)
  To: Guchun Chen
  Cc: Lazar, Lijo, amd-gfx list, Deucher, Alexander, Quan, Evan,
	Kenneth Feng, Hawking Zhang

On Mon, Jul 11, 2022 at 9:58 AM Guchun Chen <guchun.chen@amd.com> wrote:
>
> SMU will perform dpm disablement when entering BACO,
> and enable them later on, so talking to SMU to get
> enabled features in runpm cycle as BACO support check
> is not reliable. Hence, use a cached value to fix it.
>
> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       | 4 ++++
>  drivers/gpu/drm/amd/pm/amdgpu_dpm.c           | 9 +++++++++
>  drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h       | 1 +
>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     | 5 +++++
>  drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 3 +++
>  drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c        | 8 +++++++-
>  6 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 1cc9260e75de..dc2e78bb7224 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2478,6 +2478,10 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>         }
>
>         adev->in_runpm = true;
> +
> +       /* cache SMU feature mask */
> +       amdgpu_dpm_set_cached_feature_mask(adev);
> +
>         if (amdgpu_device_supports_px(drm_dev))
>                 drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> index 956b6ce81c84..211f73a987d6 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> @@ -1702,3 +1702,12 @@ int amdgpu_dpm_get_dpm_clock_table(struct amdgpu_device *adev,
>
>         return ret;
>  }
> +
> +void amdgpu_dpm_set_cached_feature_mask(struct amdgpu_device *adev)
> +{
> +       struct smu_context *smu = adev->powerplay.pp_handle;
> +
> +       mutex_lock(&adev->pm.mutex);
> +       smu_set_cached_enabled_mask(smu);
> +       mutex_unlock(&adev->pm.mutex);
> +}
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> index 524fb09437e5..e9c002a561c2 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> @@ -543,4 +543,5 @@ enum pp_smu_status amdgpu_dpm_get_uclk_dpm_states(struct amdgpu_device *adev,
>                                                   unsigned int *num_states);
>  int amdgpu_dpm_get_dpm_clock_table(struct amdgpu_device *adev,
>                                    struct dpm_clocks *clock_table);
> +void amdgpu_dpm_set_cached_feature_mask(struct amdgpu_device *adev);
>  #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index fd79b213fab4..e8ead58a00b4 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -3130,3 +3130,8 @@ int smu_send_hbm_bad_channel_flag(struct smu_context *smu, uint32_t size)
>
>         return ret;
>  }
> +
> +void smu_set_cached_enabled_mask(struct smu_context *smu)
> +{
> +       smu_feature_get_enabled_mask(smu, &smu->cache_enabled_mask);
> +}
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> index b81c657c7386..678123b5e2bf 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -568,6 +568,8 @@ struct smu_context
>         u32 param_reg;
>         u32 msg_reg;
>         u32 resp_reg;
> +
> +       uint64_t cache_enabled_mask;
>  };
>
>  struct i2c_adapter;
> @@ -1465,5 +1467,6 @@ int smu_stb_collect_info(struct smu_context *smu, void *buff, uint32_t size);
>  void amdgpu_smu_stb_debug_fs_init(struct amdgpu_device *adev);
>  int smu_send_hbm_bad_pages_num(struct smu_context *smu, uint32_t size);
>  int smu_send_hbm_bad_channel_flag(struct smu_context *smu, uint32_t size);
> +void smu_set_cached_enabled_mask(struct smu_context *smu);
>  #endif
>  #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> index 15e4298c7cc8..b3087085622a 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> @@ -499,7 +499,13 @@ int smu_cmn_feature_is_enabled(struct smu_context *smu,
>         uint64_t enabled_features;
>         int feature_id;
>
> -       if (__smu_get_enabled_features(smu, &enabled_features)) {
> +       /* SMU will perform dpm disablement when entering BACO, and enable
> +        * them later on, so talking to SMU to get enabled features in runpm
> +        * stage is not reliable. Use a cache value for this instead to fix it.
> +        */
> +       if (adev->in_runpm) {
> +               enabled_features = smu->cache_enabled_mask;

Do we need to handle this differently for BOCO?

Alex

> +       } else if (__smu_get_enabled_features(smu, &enabled_features)) {
>                 dev_err(adev->dev, "Failed to retrieve enabled ppfeatures!\n");
>                 return 0;
>         }
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 3+ messages in thread
* RE: [PATCH 2/2] drm/amdgpu: use cached SMU feature mask in runpm
  2022-07-11 15:29 ` Alex Deucher
@ 2022-07-12  3:55   ` Chen, Guchun
  0 siblings, 0 replies; 3+ messages in thread
From: Chen, Guchun @ 2022-07-12  3:55 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Lazar, Lijo, amd-gfx list, Deucher, Alexander, Quan, Evan, Feng,
	 Kenneth, Zhang, Hawking

Re: Do we need to handle this differently for BOCO?

No. feature mask applies both for BOCO and BACO.

I have a discussion with Evan, we shall focus on BACO bit instead of the whole feature mask to simply this patch. So will try to update it.

Regards,
Guchun

-----Original Message-----
From: Alex Deucher <alexdeucher@gmail.com> 
Sent: Monday, July 11, 2022 11:30 PM
To: Chen, Guchun <Guchun.Chen@amd.com>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Quan, Evan <Evan.Quan@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>
Subject: Re: [PATCH 2/2] drm/amdgpu: use cached SMU feature mask in runpm

On Mon, Jul 11, 2022 at 9:58 AM Guchun Chen <guchun.chen@amd.com> wrote:
>
> SMU will perform dpm disablement when entering BACO, and enable them 
> later on, so talking to SMU to get enabled features in runpm cycle as 
> BACO support check is not reliable. Hence, use a cached value to fix 
> it.
>
> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       | 4 ++++
>  drivers/gpu/drm/amd/pm/amdgpu_dpm.c           | 9 +++++++++
>  drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h       | 1 +
>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     | 5 +++++
>  drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 3 +++
>  drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c        | 8 +++++++-
>  6 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 1cc9260e75de..dc2e78bb7224 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2478,6 +2478,10 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>         }
>
>         adev->in_runpm = true;
> +
> +       /* cache SMU feature mask */
> +       amdgpu_dpm_set_cached_feature_mask(adev);
> +
>         if (amdgpu_device_supports_px(drm_dev))
>                 drm_dev->switch_power_state = 
> DRM_SWITCH_POWER_CHANGING;
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c 
> b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> index 956b6ce81c84..211f73a987d6 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> @@ -1702,3 +1702,12 @@ int amdgpu_dpm_get_dpm_clock_table(struct 
> amdgpu_device *adev,
>
>         return ret;
>  }
> +
> +void amdgpu_dpm_set_cached_feature_mask(struct amdgpu_device *adev) {
> +       struct smu_context *smu = adev->powerplay.pp_handle;
> +
> +       mutex_lock(&adev->pm.mutex);
> +       smu_set_cached_enabled_mask(smu);
> +       mutex_unlock(&adev->pm.mutex); }
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h 
> b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> index 524fb09437e5..e9c002a561c2 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> @@ -543,4 +543,5 @@ enum pp_smu_status amdgpu_dpm_get_uclk_dpm_states(struct amdgpu_device *adev,
>                                                   unsigned int 
> *num_states);  int amdgpu_dpm_get_dpm_clock_table(struct amdgpu_device *adev,
>                                    struct dpm_clocks *clock_table);
> +void amdgpu_dpm_set_cached_feature_mask(struct amdgpu_device *adev);
>  #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index fd79b213fab4..e8ead58a00b4 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -3130,3 +3130,8 @@ int smu_send_hbm_bad_channel_flag(struct 
> smu_context *smu, uint32_t size)
>
>         return ret;
>  }
> +
> +void smu_set_cached_enabled_mask(struct smu_context *smu) {
> +       smu_feature_get_enabled_mask(smu, &smu->cache_enabled_mask); }
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h 
> b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> index b81c657c7386..678123b5e2bf 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -568,6 +568,8 @@ struct smu_context
>         u32 param_reg;
>         u32 msg_reg;
>         u32 resp_reg;
> +
> +       uint64_t cache_enabled_mask;
>  };
>
>  struct i2c_adapter;
> @@ -1465,5 +1467,6 @@ int smu_stb_collect_info(struct smu_context 
> *smu, void *buff, uint32_t size);  void 
> amdgpu_smu_stb_debug_fs_init(struct amdgpu_device *adev);  int 
> smu_send_hbm_bad_pages_num(struct smu_context *smu, uint32_t size);  
> int smu_send_hbm_bad_channel_flag(struct smu_context *smu, uint32_t 
> size);
> +void smu_set_cached_enabled_mask(struct smu_context *smu);
>  #endif
>  #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> index 15e4298c7cc8..b3087085622a 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> @@ -499,7 +499,13 @@ int smu_cmn_feature_is_enabled(struct smu_context *smu,
>         uint64_t enabled_features;
>         int feature_id;
>
> -       if (__smu_get_enabled_features(smu, &enabled_features)) {
> +       /* SMU will perform dpm disablement when entering BACO, and enable
> +        * them later on, so talking to SMU to get enabled features in runpm
> +        * stage is not reliable. Use a cache value for this instead to fix it.
> +        */
> +       if (adev->in_runpm) {
> +               enabled_features = smu->cache_enabled_mask;

Do we need to handle this differently for BOCO?

Alex

> +       } else if (__smu_get_enabled_features(smu, &enabled_features)) 
> + {
>                 dev_err(adev->dev, "Failed to retrieve enabled ppfeatures!\n");
>                 return 0;
>         }
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-07-12  3:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-11 13:58 [PATCH 2/2] drm/amdgpu: use cached SMU feature mask in runpm Guchun Chen
2022-07-11 15:29 ` Alex Deucher
2022-07-12  3:55   ` Chen, Guchun

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.