All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: make amdgpu device attr_update() function more efficient
@ 2024-03-26  9:02 Yang Wang
  2024-03-27  6:13 ` Ma, Jun
  2024-03-27  6:21 ` Lazar, Lijo
  0 siblings, 2 replies; 4+ messages in thread
From: Yang Wang @ 2024-03-26  9:02 UTC (permalink / raw)
  To: amd-gfx; +Cc: hawking.zhang, alexander.deucher, Yang Wang

add a new enumeration type to identify device attribute node,
this method is relatively more efficient compared with 'strcmp' in
update_attr() function.

Signed-off-by: Yang Wang <kevinyang.wang@amd.com>
---
 drivers/gpu/drm/amd/pm/amdgpu_pm.c     |  4 +--
 drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h | 41 ++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 85e935556d7d..04f53f2667fe 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2226,16 +2226,16 @@ static int default_attr_update(struct amdgpu_device *adev, struct amdgpu_device_
 			       uint32_t mask, enum amdgpu_device_attr_states *states)
 {
 	struct device_attribute *dev_attr = &attr->dev_attr;
+	enum amdgpu_device_attr_type type = attr->type;
 	uint32_t mp1_ver = amdgpu_ip_version(adev, MP1_HWIP, 0);
 	uint32_t gc_ver = amdgpu_ip_version(adev, GC_HWIP, 0);
-	const char *attr_name = dev_attr->attr.name;
 
 	if (!(attr->flags & mask)) {
 		*states = ATTR_STATE_UNSUPPORTED;
 		return 0;
 	}
 
-#define DEVICE_ATTR_IS(_name)	(!strcmp(attr_name, #_name))
+#define DEVICE_ATTR_IS(_name)		(type == device_attr_type__##_name)
 
 	if (DEVICE_ATTR_IS(pp_dpm_socclk)) {
 		if (gc_ver < IP_VERSION(9, 0, 0))
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
index eec816f0cbf9..157330c379be 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
@@ -43,8 +43,48 @@ enum amdgpu_device_attr_states {
 	ATTR_STATE_SUPPORTED,
 };
 
+enum amdgpu_device_attr_type {
+	device_attr_type__unknown = -1,
+	device_attr_type__power_dpm_state = 0,
+	device_attr_type__power_dpm_force_performance_level,
+	device_attr_type__pp_num_states,
+	device_attr_type__pp_cur_state,
+	device_attr_type__pp_force_state,
+	device_attr_type__pp_table,
+	device_attr_type__pp_dpm_sclk,
+	device_attr_type__pp_dpm_mclk,
+	device_attr_type__pp_dpm_socclk,
+	device_attr_type__pp_dpm_fclk,
+	device_attr_type__pp_dpm_vclk,
+	device_attr_type__pp_dpm_vclk1,
+	device_attr_type__pp_dpm_dclk,
+	device_attr_type__pp_dpm_dclk1,
+	device_attr_type__pp_dpm_dcefclk,
+	device_attr_type__pp_dpm_pcie,
+	device_attr_type__pp_sclk_od,
+	device_attr_type__pp_mclk_od,
+	device_attr_type__pp_power_profile_mode,
+	device_attr_type__pp_od_clk_voltage,
+	device_attr_type__gpu_busy_percent,
+	device_attr_type__mem_busy_percent,
+	device_attr_type__vcn_busy_percent,
+	device_attr_type__pcie_bw,
+	device_attr_type__pp_features,
+	device_attr_type__unique_id,
+	device_attr_type__thermal_throttling_logging,
+	device_attr_type__apu_thermal_cap,
+	device_attr_type__gpu_metrics,
+	device_attr_type__smartshift_apu_power,
+	device_attr_type__smartshift_dgpu_power,
+	device_attr_type__smartshift_bias,
+	device_attr_type__xgmi_plpd_policy,
+	device_attr_type__pm_metrics,
+	device_attr_type__count,
+};
+
 struct amdgpu_device_attr {
 	struct device_attribute dev_attr;
+	enum amdgpu_device_attr_type type;
 	enum amdgpu_device_attr_flags flags;
 	int (*attr_update)(struct amdgpu_device *adev, struct amdgpu_device_attr *attr,
 			   uint32_t mask, enum amdgpu_device_attr_states *states);
@@ -61,6 +101,7 @@ struct amdgpu_device_attr_entry {
 
 #define __AMDGPU_DEVICE_ATTR(_name, _mode, _show, _store, _flags, ...)	\
 	{ .dev_attr = __ATTR(_name, _mode, _show, _store),		\
+	  .type = device_attr_type__##_name,				\
 	  .flags = _flags,						\
 	  ##__VA_ARGS__, }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/amdgpu: make amdgpu device attr_update() function more efficient
  2024-03-26  9:02 [PATCH] drm/amdgpu: make amdgpu device attr_update() function more efficient Yang Wang
@ 2024-03-27  6:13 ` Ma, Jun
  2024-03-27  6:21 ` Lazar, Lijo
  1 sibling, 0 replies; 4+ messages in thread
From: Ma, Jun @ 2024-03-27  6:13 UTC (permalink / raw)
  To: Yang Wang, amd-gfx; +Cc: majun, hawking.zhang, alexander.deucher


Reviewed-by: Ma Jun <majun@amd.com>

On 3/26/2024 5:02 PM, Yang Wang wrote:
> add a new enumeration type to identify device attribute node,
> this method is relatively more efficient compared with 'strcmp' in
> update_attr() function.
> 
> Signed-off-by: Yang Wang <kevinyang.wang@amd.com>
> ---
>  drivers/gpu/drm/amd/pm/amdgpu_pm.c     |  4 +--
>  drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h | 41 ++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 85e935556d7d..04f53f2667fe 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -2226,16 +2226,16 @@ static int default_attr_update(struct amdgpu_device *adev, struct amdgpu_device_
>  			       uint32_t mask, enum amdgpu_device_attr_states *states)
>  {
>  	struct device_attribute *dev_attr = &attr->dev_attr;
> +	enum amdgpu_device_attr_type type = attr->type;
>  	uint32_t mp1_ver = amdgpu_ip_version(adev, MP1_HWIP, 0);
>  	uint32_t gc_ver = amdgpu_ip_version(adev, GC_HWIP, 0);
> -	const char *attr_name = dev_attr->attr.name;
>  
>  	if (!(attr->flags & mask)) {
>  		*states = ATTR_STATE_UNSUPPORTED;
>  		return 0;
>  	}
>  
> -#define DEVICE_ATTR_IS(_name)	(!strcmp(attr_name, #_name))
> +#define DEVICE_ATTR_IS(_name)		(type == device_attr_type__##_name)
>  
>  	if (DEVICE_ATTR_IS(pp_dpm_socclk)) {
>  		if (gc_ver < IP_VERSION(9, 0, 0))
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
> index eec816f0cbf9..157330c379be 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
> @@ -43,8 +43,48 @@ enum amdgpu_device_attr_states {
>  	ATTR_STATE_SUPPORTED,
>  };
>  
> +enum amdgpu_device_attr_type {
> +	device_attr_type__unknown = -1,
> +	device_attr_type__power_dpm_state = 0,
> +	device_attr_type__power_dpm_force_performance_level,
> +	device_attr_type__pp_num_states,
> +	device_attr_type__pp_cur_state,
> +	device_attr_type__pp_force_state,
> +	device_attr_type__pp_table,
> +	device_attr_type__pp_dpm_sclk,
> +	device_attr_type__pp_dpm_mclk,
> +	device_attr_type__pp_dpm_socclk,
> +	device_attr_type__pp_dpm_fclk,
> +	device_attr_type__pp_dpm_vclk,
> +	device_attr_type__pp_dpm_vclk1,
> +	device_attr_type__pp_dpm_dclk,
> +	device_attr_type__pp_dpm_dclk1,
> +	device_attr_type__pp_dpm_dcefclk,
> +	device_attr_type__pp_dpm_pcie,
> +	device_attr_type__pp_sclk_od,
> +	device_attr_type__pp_mclk_od,
> +	device_attr_type__pp_power_profile_mode,
> +	device_attr_type__pp_od_clk_voltage,
> +	device_attr_type__gpu_busy_percent,
> +	device_attr_type__mem_busy_percent,
> +	device_attr_type__vcn_busy_percent,
> +	device_attr_type__pcie_bw,
> +	device_attr_type__pp_features,
> +	device_attr_type__unique_id,
> +	device_attr_type__thermal_throttling_logging,
> +	device_attr_type__apu_thermal_cap,
> +	device_attr_type__gpu_metrics,
> +	device_attr_type__smartshift_apu_power,
> +	device_attr_type__smartshift_dgpu_power,
> +	device_attr_type__smartshift_bias,
> +	device_attr_type__xgmi_plpd_policy,
> +	device_attr_type__pm_metrics,
> +	device_attr_type__count,
> +};
> +
>  struct amdgpu_device_attr {
>  	struct device_attribute dev_attr;
> +	enum amdgpu_device_attr_type type;
>  	enum amdgpu_device_attr_flags flags;
>  	int (*attr_update)(struct amdgpu_device *adev, struct amdgpu_device_attr *attr,
>  			   uint32_t mask, enum amdgpu_device_attr_states *states);
> @@ -61,6 +101,7 @@ struct amdgpu_device_attr_entry {
>  
>  #define __AMDGPU_DEVICE_ATTR(_name, _mode, _show, _store, _flags, ...)	\
>  	{ .dev_attr = __ATTR(_name, _mode, _show, _store),		\
> +	  .type = device_attr_type__##_name,				\
>  	  .flags = _flags,						\
>  	  ##__VA_ARGS__, }
>  

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/amdgpu: make amdgpu device attr_update() function more efficient
  2024-03-26  9:02 [PATCH] drm/amdgpu: make amdgpu device attr_update() function more efficient Yang Wang
  2024-03-27  6:13 ` Ma, Jun
@ 2024-03-27  6:21 ` Lazar, Lijo
  2024-03-27  6:25   ` Wang, Yang(Kevin)
  1 sibling, 1 reply; 4+ messages in thread
From: Lazar, Lijo @ 2024-03-27  6:21 UTC (permalink / raw)
  To: Yang Wang, amd-gfx; +Cc: hawking.zhang, alexander.deucher



On 3/26/2024 2:32 PM, Yang Wang wrote:
> add a new enumeration type to identify device attribute node,
> this method is relatively more efficient compared with 'strcmp' in
> update_attr() function.
> 
> Signed-off-by: Yang Wang <kevinyang.wang@amd.com>
> ---
>  drivers/gpu/drm/amd/pm/amdgpu_pm.c     |  4 +--
>  drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h | 41 ++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 85e935556d7d..04f53f2667fe 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -2226,16 +2226,16 @@ static int default_attr_update(struct amdgpu_device *adev, struct amdgpu_device_
>  			       uint32_t mask, enum amdgpu_device_attr_states *states)
>  {
>  	struct device_attribute *dev_attr = &attr->dev_attr;
> +	enum amdgpu_device_attr_type type = attr->type;
>  	uint32_t mp1_ver = amdgpu_ip_version(adev, MP1_HWIP, 0);
>  	uint32_t gc_ver = amdgpu_ip_version(adev, GC_HWIP, 0);
> -	const char *attr_name = dev_attr->attr.name;
>  
>  	if (!(attr->flags & mask)) {
>  		*states = ATTR_STATE_UNSUPPORTED;
>  		return 0;
>  	}
>  
> -#define DEVICE_ATTR_IS(_name)	(!strcmp(attr_name, #_name))
> +#define DEVICE_ATTR_IS(_name)		(type == device_attr_type__##_name)
>  
>  	if (DEVICE_ATTR_IS(pp_dpm_socclk)) {
>  		if (gc_ver < IP_VERSION(9, 0, 0))
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
> index eec816f0cbf9..157330c379be 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
> @@ -43,8 +43,48 @@ enum amdgpu_device_attr_states {
>  	ATTR_STATE_SUPPORTED,
>  };
>  
> +enum amdgpu_device_attr_type {

Prefer id instead of type. That aside,

Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>

Thanks,
Lijo
	
> +	device_attr_type__unknown = -1,
> +	device_attr_type__power_dpm_state = 0,
> +	device_attr_type__power_dpm_force_performance_level,
> +	device_attr_type__pp_num_states,
> +	device_attr_type__pp_cur_state,
> +	device_attr_type__pp_force_state,
> +	device_attr_type__pp_table,
> +	device_attr_type__pp_dpm_sclk,
> +	device_attr_type__pp_dpm_mclk,
> +	device_attr_type__pp_dpm_socclk,
> +	device_attr_type__pp_dpm_fclk,
> +	device_attr_type__pp_dpm_vclk,
> +	device_attr_type__pp_dpm_vclk1,
> +	device_attr_type__pp_dpm_dclk,
> +	device_attr_type__pp_dpm_dclk1,
> +	device_attr_type__pp_dpm_dcefclk,
> +	device_attr_type__pp_dpm_pcie,
> +	device_attr_type__pp_sclk_od,
> +	device_attr_type__pp_mclk_od,
> +	device_attr_type__pp_power_profile_mode,
> +	device_attr_type__pp_od_clk_voltage,
> +	device_attr_type__gpu_busy_percent,
> +	device_attr_type__mem_busy_percent,
> +	device_attr_type__vcn_busy_percent,
> +	device_attr_type__pcie_bw,
> +	device_attr_type__pp_features,
> +	device_attr_type__unique_id,
> +	device_attr_type__thermal_throttling_logging,
> +	device_attr_type__apu_thermal_cap,
> +	device_attr_type__gpu_metrics,
> +	device_attr_type__smartshift_apu_power,
> +	device_attr_type__smartshift_dgpu_power,
> +	device_attr_type__smartshift_bias,
> +	device_attr_type__xgmi_plpd_policy,
> +	device_attr_type__pm_metrics,
> +	device_attr_type__count,
> +};
> +
>  struct amdgpu_device_attr {
>  	struct device_attribute dev_attr;
> +	enum amdgpu_device_attr_type type;
>  	enum amdgpu_device_attr_flags flags;
>  	int (*attr_update)(struct amdgpu_device *adev, struct amdgpu_device_attr *attr,
>  			   uint32_t mask, enum amdgpu_device_attr_states *states);
> @@ -61,6 +101,7 @@ struct amdgpu_device_attr_entry {
>  
>  #define __AMDGPU_DEVICE_ATTR(_name, _mode, _show, _store, _flags, ...)	\
>  	{ .dev_attr = __ATTR(_name, _mode, _show, _store),		\
> +	  .type = device_attr_type__##_name,				\
>  	  .flags = _flags,						\
>  	  ##__VA_ARGS__, }
>  

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH] drm/amdgpu: make amdgpu device attr_update() function more efficient
  2024-03-27  6:21 ` Lazar, Lijo
@ 2024-03-27  6:25   ` Wang, Yang(Kevin)
  0 siblings, 0 replies; 4+ messages in thread
From: Wang, Yang(Kevin) @ 2024-03-27  6:25 UTC (permalink / raw)
  To: Lazar, Lijo, amd-gfx@lists.freedesktop.org
  Cc: Zhang, Hawking, Deucher, Alexander

[AMD Official Use Only - General]

-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Wednesday, March 27, 2024 2:22 PM
To: Wang, Yang(Kevin) <KevinYang.Wang@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: Re: [PATCH] drm/amdgpu: make amdgpu device attr_update() function more efficient



On 3/26/2024 2:32 PM, Yang Wang wrote:
> add a new enumeration type to identify device attribute node, this
> method is relatively more efficient compared with 'strcmp' in
> update_attr() function.
>
> Signed-off-by: Yang Wang <kevinyang.wang@amd.com>
> ---
>  drivers/gpu/drm/amd/pm/amdgpu_pm.c     |  4 +--
>  drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h | 41
> ++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 85e935556d7d..04f53f2667fe 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -2226,16 +2226,16 @@ static int default_attr_update(struct amdgpu_device *adev, struct amdgpu_device_
>                              uint32_t mask, enum amdgpu_device_attr_states *states)  {
>       struct device_attribute *dev_attr = &attr->dev_attr;
> +     enum amdgpu_device_attr_type type = attr->type;
>       uint32_t mp1_ver = amdgpu_ip_version(adev, MP1_HWIP, 0);
>       uint32_t gc_ver = amdgpu_ip_version(adev, GC_HWIP, 0);
> -     const char *attr_name = dev_attr->attr.name;
>
>       if (!(attr->flags & mask)) {
>               *states = ATTR_STATE_UNSUPPORTED;
>               return 0;
>       }
>
> -#define DEVICE_ATTR_IS(_name)        (!strcmp(attr_name, #_name))
> +#define DEVICE_ATTR_IS(_name)                (type == device_attr_type__##_name)
>
>       if (DEVICE_ATTR_IS(pp_dpm_socclk)) {
>               if (gc_ver < IP_VERSION(9, 0, 0))
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
> b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
> index eec816f0cbf9..157330c379be 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
> @@ -43,8 +43,48 @@ enum amdgpu_device_attr_states {
>       ATTR_STATE_SUPPORTED,
>  };
>
> +enum amdgpu_device_attr_type {

Prefer id instead of type. That aside,

Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>

Thanks,
Lijo

[kevin]:
Yes, agreed, the name 'attribute id' is better on this case.

Best Regards,
Kevin

> +     device_attr_type__unknown = -1,
> +     device_attr_type__power_dpm_state = 0,
> +     device_attr_type__power_dpm_force_performance_level,
> +     device_attr_type__pp_num_states,
> +     device_attr_type__pp_cur_state,
> +     device_attr_type__pp_force_state,
> +     device_attr_type__pp_table,
> +     device_attr_type__pp_dpm_sclk,
> +     device_attr_type__pp_dpm_mclk,
> +     device_attr_type__pp_dpm_socclk,
> +     device_attr_type__pp_dpm_fclk,
> +     device_attr_type__pp_dpm_vclk,
> +     device_attr_type__pp_dpm_vclk1,
> +     device_attr_type__pp_dpm_dclk,
> +     device_attr_type__pp_dpm_dclk1,
> +     device_attr_type__pp_dpm_dcefclk,
> +     device_attr_type__pp_dpm_pcie,
> +     device_attr_type__pp_sclk_od,
> +     device_attr_type__pp_mclk_od,
> +     device_attr_type__pp_power_profile_mode,
> +     device_attr_type__pp_od_clk_voltage,
> +     device_attr_type__gpu_busy_percent,
> +     device_attr_type__mem_busy_percent,
> +     device_attr_type__vcn_busy_percent,
> +     device_attr_type__pcie_bw,
> +     device_attr_type__pp_features,
> +     device_attr_type__unique_id,
> +     device_attr_type__thermal_throttling_logging,
> +     device_attr_type__apu_thermal_cap,
> +     device_attr_type__gpu_metrics,
> +     device_attr_type__smartshift_apu_power,
> +     device_attr_type__smartshift_dgpu_power,
> +     device_attr_type__smartshift_bias,
> +     device_attr_type__xgmi_plpd_policy,
> +     device_attr_type__pm_metrics,
> +     device_attr_type__count,
> +};
> +
>  struct amdgpu_device_attr {
>       struct device_attribute dev_attr;
> +     enum amdgpu_device_attr_type type;
>       enum amdgpu_device_attr_flags flags;
>       int (*attr_update)(struct amdgpu_device *adev, struct amdgpu_device_attr *attr,
>                          uint32_t mask, enum amdgpu_device_attr_states *states); @@
> -61,6 +101,7 @@ struct amdgpu_device_attr_entry {
>
>  #define __AMDGPU_DEVICE_ATTR(_name, _mode, _show, _store, _flags, ...)       \
>       { .dev_attr = __ATTR(_name, _mode, _show, _store),              \
> +       .type = device_attr_type__##_name,                            \
>         .flags = _flags,                                              \
>         ##__VA_ARGS__, }
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-03-27  6:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-26  9:02 [PATCH] drm/amdgpu: make amdgpu device attr_update() function more efficient Yang Wang
2024-03-27  6:13 ` Ma, Jun
2024-03-27  6:21 ` Lazar, Lijo
2024-03-27  6:25   ` Wang, Yang(Kevin)

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.