diff mbox series

[v3,2/3] platform/x86/amd/pmf: Add sysfs to toggle CnQF

Message ID 20220921102455.1792336-3-Shyam-sundar.S-k@amd.com (mailing list archive)
State Changes Requested, archived
Headers show
Series platform/x86/amd/pmf: Introduce CnQF feature for AMD PMF | expand

Commit Message

Shyam Sundar S K Sept. 21, 2022, 10:24 a.m. UTC 
Whether to turn CnQF on/off by default upon driver load would be decided
by a BIOS flag. Add a sysfs node to provide a way to the user whether to
use static slider or CnQF .

Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmf/cnqf.c | 62 +++++++++++++++++++++++++++++
 drivers/platform/x86/amd/pmf/core.c |  6 +++
 drivers/platform/x86/amd/pmf/pmf.h  |  1 +
 3 files changed, 69 insertions(+)

Comments

Hans de Goede Sept. 21, 2022, 2:41 p.m. UTC | #1 
Hi,

On 9/21/22 12:24, Shyam Sundar S K wrote:
> Whether to turn CnQF on/off by default upon driver load would be decided
> by a BIOS flag. Add a sysfs node to provide a way to the user whether to
> use static slider or CnQF .
> 
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
>  drivers/platform/x86/amd/pmf/cnqf.c | 62 +++++++++++++++++++++++++++++
>  drivers/platform/x86/amd/pmf/core.c |  6 +++
>  drivers/platform/x86/amd/pmf/pmf.h  |  1 +
>  3 files changed, 69 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd/pmf/cnqf.c b/drivers/platform/x86/amd/pmf/cnqf.c
> index fa994800692a..34021bb22064 100644
> --- a/drivers/platform/x86/amd/pmf/cnqf.c
> +++ b/drivers/platform/x86/amd/pmf/cnqf.c
> @@ -298,6 +298,68 @@ static int amd_pmf_load_defaults_cnqf(struct amd_pmf_dev *dev)
>  	return 0;
>  }
>  
> +static ssize_t cnqf_enable_store(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf, size_t count)
> +{
> +	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
> +	int mode, result, src;
> +	bool input;
> +
> +	mode = amd_pmf_get_pprof_modes(pdev);
> +	if (mode < 0)
> +		return mode;
> +
> +	result = kstrtobool(buf, &input);
> +	if (result)
> +		return result;
> +
> +	src = amd_pmf_get_power_source();
> +	pdev->cnqf_enabled = input;
> +
> +	if (pdev->cnqf_enabled && pdev->current_profile == PLATFORM_PROFILE_BALANCED) {
> +		amd_pmf_set_cnqf(pdev, src, config_store.current_mode, NULL);
> +	} else {
> +		if (is_apmf_func_supported(pdev, APMF_FUNC_STATIC_SLIDER_GRANULAR)) {
> +			amd_pmf_init_sps(pdev);

This amd_pmf_init_sps() calls is not necessary, this is already done at probe time
when is_apmf_func_supported(pdev, APMF_FUNC_STATIC_SLIDER_GRANULAR) returns true.

Please drop this.

> +			amd_pmf_update_slider(pdev, SLIDER_OP_SET, mode, NULL);
> +		}
> +	}
> +
> +	dev_dbg(pdev->dev, "Received CnQF %s\n", input ? "on" : "off");
> +	return count;
> +}
> +
> +static ssize_t cnqf_enable_show(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
> +
> +	return sysfs_emit(buf, "%s\n", pdev->cnqf_enabled ? "on" : "off");
> +}
> +
> +static DEVICE_ATTR_RW(cnqf_enable);
> +
> +static umode_t cnqf_feature_is_visible(struct kobject *kobj,
> +				       struct attribute *attr, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
> +
> +	return pdev->cnqf_supported ? attr->mode : 0;
> +}
> +
> +static struct attribute *cnqf_feature_attrs[] = {
> +	&dev_attr_cnqf_enable.attr,
> +	NULL
> +};
> +
> +const struct attribute_group cnqf_feature_attribute_group = {
> +	.is_visible = cnqf_feature_is_visible,
> +	.attrs = cnqf_feature_attrs,
> +};
> +
>  void amd_pmf_deinit_cnqf(struct amd_pmf_dev *dev)
>  {
>  	cancel_delayed_work_sync(&dev->work_buffer);
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index 99be1e6d57d3..44fe30726b62 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -391,10 +391,16 @@ static int amd_pmf_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct attribute_group *amd_pmf_driver_groups[] = {
> +	&cnqf_feature_attribute_group,
> +	NULL,
> +};
> +
>  static struct platform_driver amd_pmf_driver = {
>  	.driver = {
>  		.name = "amd-pmf",
>  		.acpi_match_table = amd_pmf_acpi_ids,
> +		.dev_groups = amd_pmf_driver_groups,
>  	},
>  	.probe = amd_pmf_probe,
>  	.remove = amd_pmf_remove,
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 24dd6fbff24c..097f3f5d33a3 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -422,5 +422,6 @@ int apmf_get_dyn_slider_def_dc(struct amd_pmf_dev *pdev, struct apmf_dyn_slider_
>  int amd_pmf_init_cnqf(struct amd_pmf_dev *dev);
>  void amd_pmf_deinit_cnqf(struct amd_pmf_dev *dev);
>  int amd_pmf_trans_cnqf(struct amd_pmf_dev *dev, int socket_power, ktime_t time_lapsed_ms);
> +extern const struct attribute_group cnqf_feature_attribute_group;
>  
>  #endif /* PMF_H */

Other then the remark about dropping the amd_pmf_init_sps() call this
looks good to me, with that fixed this is:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans
Shyam Sundar S K Sept. 21, 2022, 5:28 p.m. UTC | #2 
On 9/21/2022 8:11 PM, Hans de Goede wrote:
> Hi,
> 
> On 9/21/22 12:24, Shyam Sundar S K wrote:
>> Whether to turn CnQF on/off by default upon driver load would be decided
>> by a BIOS flag. Add a sysfs node to provide a way to the user whether to
>> use static slider or CnQF .
>>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>>  drivers/platform/x86/amd/pmf/cnqf.c | 62 +++++++++++++++++++++++++++++
>>  drivers/platform/x86/amd/pmf/core.c |  6 +++
>>  drivers/platform/x86/amd/pmf/pmf.h  |  1 +
>>  3 files changed, 69 insertions(+)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/cnqf.c b/drivers/platform/x86/amd/pmf/cnqf.c
>> index fa994800692a..34021bb22064 100644
>> --- a/drivers/platform/x86/amd/pmf/cnqf.c
>> +++ b/drivers/platform/x86/amd/pmf/cnqf.c
>> @@ -298,6 +298,68 @@ static int amd_pmf_load_defaults_cnqf(struct amd_pmf_dev *dev)
>>  	return 0;
>>  }
>>  
>> +static ssize_t cnqf_enable_store(struct device *dev,
>> +				 struct device_attribute *attr,
>> +				 const char *buf, size_t count)
>> +{
>> +	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
>> +	int mode, result, src;
>> +	bool input;
>> +
>> +	mode = amd_pmf_get_pprof_modes(pdev);
>> +	if (mode < 0)
>> +		return mode;
>> +
>> +	result = kstrtobool(buf, &input);
>> +	if (result)
>> +		return result;
>> +
>> +	src = amd_pmf_get_power_source();
>> +	pdev->cnqf_enabled = input;
>> +
>> +	if (pdev->cnqf_enabled && pdev->current_profile == PLATFORM_PROFILE_BALANCED) {
>> +		amd_pmf_set_cnqf(pdev, src, config_store.current_mode, NULL);
>> +	} else {
>> +		if (is_apmf_func_supported(pdev, APMF_FUNC_STATIC_SLIDER_GRANULAR)) {
>> +			amd_pmf_init_sps(pdev);
> 
> This amd_pmf_init_sps() calls is not necessary, this is already done at probe time
> when is_apmf_func_supported(pdev, APMF_FUNC_STATIC_SLIDER_GRANULAR) returns true.
> 
> Please drop this.
> 
>> +			amd_pmf_update_slider(pdev, SLIDER_OP_SET, mode, NULL);
>> +		}
>> +	}
>> +
>> +	dev_dbg(pdev->dev, "Received CnQF %s\n", input ? "on" : "off");
>> +	return count;
>> +}
>> +
>> +static ssize_t cnqf_enable_show(struct device *dev,
>> +				struct device_attribute *attr,
>> +				char *buf)
>> +{
>> +	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
>> +
>> +	return sysfs_emit(buf, "%s\n", pdev->cnqf_enabled ? "on" : "off");
>> +}
>> +
>> +static DEVICE_ATTR_RW(cnqf_enable);
>> +
>> +static umode_t cnqf_feature_is_visible(struct kobject *kobj,
>> +				       struct attribute *attr, int n)
>> +{
>> +	struct device *dev = kobj_to_dev(kobj);
>> +	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
>> +
>> +	return pdev->cnqf_supported ? attr->mode : 0;
>> +}
>> +
>> +static struct attribute *cnqf_feature_attrs[] = {
>> +	&dev_attr_cnqf_enable.attr,
>> +	NULL
>> +};
>> +
>> +const struct attribute_group cnqf_feature_attribute_group = {
>> +	.is_visible = cnqf_feature_is_visible,
>> +	.attrs = cnqf_feature_attrs,
>> +};
>> +
>>  void amd_pmf_deinit_cnqf(struct amd_pmf_dev *dev)
>>  {
>>  	cancel_delayed_work_sync(&dev->work_buffer);
>> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
>> index 99be1e6d57d3..44fe30726b62 100644
>> --- a/drivers/platform/x86/amd/pmf/core.c
>> +++ b/drivers/platform/x86/amd/pmf/core.c
>> @@ -391,10 +391,16 @@ static int amd_pmf_remove(struct platform_device *pdev)
>>  	return 0;
>>  }
>>  
>> +static const struct attribute_group *amd_pmf_driver_groups[] = {
>> +	&cnqf_feature_attribute_group,
>> +	NULL,
>> +};
>> +
>>  static struct platform_driver amd_pmf_driver = {
>>  	.driver = {
>>  		.name = "amd-pmf",
>>  		.acpi_match_table = amd_pmf_acpi_ids,
>> +		.dev_groups = amd_pmf_driver_groups,
>>  	},
>>  	.probe = amd_pmf_probe,
>>  	.remove = amd_pmf_remove,
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
>> index 24dd6fbff24c..097f3f5d33a3 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -422,5 +422,6 @@ int apmf_get_dyn_slider_def_dc(struct amd_pmf_dev *pdev, struct apmf_dyn_slider_
>>  int amd_pmf_init_cnqf(struct amd_pmf_dev *dev);
>>  void amd_pmf_deinit_cnqf(struct amd_pmf_dev *dev);
>>  int amd_pmf_trans_cnqf(struct amd_pmf_dev *dev, int socket_power, ktime_t time_lapsed_ms);
>> +extern const struct attribute_group cnqf_feature_attribute_group;
>>  
>>  #endif /* PMF_H */
> 
> Other then the remark about dropping the amd_pmf_init_sps() call this
> looks good to me, with that fixed this is:

Agreed. Will drop the call and fix that in v4.

Thanks,
Shyam

> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> Regards,
> 
> Hans
>
diff mbox series

Patch

diff --git a/drivers/platform/x86/amd/pmf/cnqf.c b/drivers/platform/x86/amd/pmf/cnqf.c
index fa994800692a..34021bb22064 100644
--- a/drivers/platform/x86/amd/pmf/cnqf.c
+++ b/drivers/platform/x86/amd/pmf/cnqf.c
@@ -298,6 +298,68 @@  static int amd_pmf_load_defaults_cnqf(struct amd_pmf_dev *dev)
 	return 0;
 }
 
+static ssize_t cnqf_enable_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
+	int mode, result, src;
+	bool input;
+
+	mode = amd_pmf_get_pprof_modes(pdev);
+	if (mode < 0)
+		return mode;
+
+	result = kstrtobool(buf, &input);
+	if (result)
+		return result;
+
+	src = amd_pmf_get_power_source();
+	pdev->cnqf_enabled = input;
+
+	if (pdev->cnqf_enabled && pdev->current_profile == PLATFORM_PROFILE_BALANCED) {
+		amd_pmf_set_cnqf(pdev, src, config_store.current_mode, NULL);
+	} else {
+		if (is_apmf_func_supported(pdev, APMF_FUNC_STATIC_SLIDER_GRANULAR)) {
+			amd_pmf_init_sps(pdev);
+			amd_pmf_update_slider(pdev, SLIDER_OP_SET, mode, NULL);
+		}
+	}
+
+	dev_dbg(pdev->dev, "Received CnQF %s\n", input ? "on" : "off");
+	return count;
+}
+
+static ssize_t cnqf_enable_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%s\n", pdev->cnqf_enabled ? "on" : "off");
+}
+
+static DEVICE_ATTR_RW(cnqf_enable);
+
+static umode_t cnqf_feature_is_visible(struct kobject *kobj,
+				       struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
+
+	return pdev->cnqf_supported ? attr->mode : 0;
+}
+
+static struct attribute *cnqf_feature_attrs[] = {
+	&dev_attr_cnqf_enable.attr,
+	NULL
+};
+
+const struct attribute_group cnqf_feature_attribute_group = {
+	.is_visible = cnqf_feature_is_visible,
+	.attrs = cnqf_feature_attrs,
+};
+
 void amd_pmf_deinit_cnqf(struct amd_pmf_dev *dev)
 {
 	cancel_delayed_work_sync(&dev->work_buffer);
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index 99be1e6d57d3..44fe30726b62 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -391,10 +391,16 @@  static int amd_pmf_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct attribute_group *amd_pmf_driver_groups[] = {
+	&cnqf_feature_attribute_group,
+	NULL,
+};
+
 static struct platform_driver amd_pmf_driver = {
 	.driver = {
 		.name = "amd-pmf",
 		.acpi_match_table = amd_pmf_acpi_ids,
+		.dev_groups = amd_pmf_driver_groups,
 	},
 	.probe = amd_pmf_probe,
 	.remove = amd_pmf_remove,
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 24dd6fbff24c..097f3f5d33a3 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -422,5 +422,6 @@  int apmf_get_dyn_slider_def_dc(struct amd_pmf_dev *pdev, struct apmf_dyn_slider_
 int amd_pmf_init_cnqf(struct amd_pmf_dev *dev);
 void amd_pmf_deinit_cnqf(struct amd_pmf_dev *dev);
 int amd_pmf_trans_cnqf(struct amd_pmf_dev *dev, int socket_power, ktime_t time_lapsed_ms);
+extern const struct attribute_group cnqf_feature_attribute_group;
 
 #endif /* PMF_H */