[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20220211160932.3221873-5-srinivas.pandruvada@linux.intel.com>
Date: Fri, 11 Feb 2022 08:09:30 -0800
From: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
To: rafael@...nel.org, lenb@...nel.org
Cc: linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Subject: [PATCH v4 4/6] ACPI / fan: Properly handle fine grain control
When _FIF object specifies support for fine grain control, then fan speed
can be set from 0 to 100% with the recommended minimum "step size" via
_FSL object. Here the control value doesn't need to match any value from
_FPS object.
Currently we have a simple solution implemented which just pick maximum
control value from _FPS to display the actual state, but this is not
optimal when there is a big window between two control values in
_FPS. Also there is no way to set to any speed which doesn't match
control values in _FPS. The system firmware can start the fan at speed
which doesn't match any control value.
To support fine grain control (when supported) via thermal sysfs:
- cooling device max state is not _FPS state count but it will be
100 / _FIF.step_size
Step size can be from 1 to 9.
- cooling device current state is _FST.control / _FIF.step_size
- cooling device set state will set the control value
cdev.curr_state * _FIF.step_size plus any adjustment for 100%.
By the spec, when control value do not sum to 100% because of
_FIF.step_size, OSPM may select an appropriate ending Level increment
to reach 100%.
There is no rounding during calculation. For example if step size
is 6:
thermal sysfs cooling device max_state = 100/6 = 16
So user can set any value from 0-16.
If the system boots with a _FST.control which is not multiples
of step_size, the thermal sysfs cur_state will be based on the
range. For example for step size = 6:
_FST.control thermal sysfs cur_state
------------------------------------------------
0-5 0
6-11 1
..
..
90-95 15
96-100 16
While setting the _FST.control, the compensation will be at
the last step for cur_state = 16, which will set the _FST.control
to 100.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
---
drivers/acpi/fan.h | 6 +++
drivers/acpi/fan_core.c | 94 +++++++++++++++++++++++++++++------------
2 files changed, 74 insertions(+), 26 deletions(-)
diff --git a/drivers/acpi/fan.h b/drivers/acpi/fan.h
index 6cbb4b028da0..4c01be2e3b77 100644
--- a/drivers/acpi/fan.h
+++ b/drivers/acpi/fan.h
@@ -36,6 +36,12 @@ struct acpi_fan_fif {
u8 low_speed_notification;
};
+struct acpi_fan_fst {
+ u64 revision;
+ u64 control;
+ u64 speed;
+};
+
struct acpi_fan {
bool acpi4;
struct acpi_fan_fif fif;
diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c
index 484cee0fb13e..495f59222881 100644
--- a/drivers/acpi/fan_core.c
+++ b/drivers/acpi/fan_core.c
@@ -63,20 +63,24 @@ static int fan_get_max_state(struct thermal_cooling_device *cdev, unsigned long
struct acpi_device *device = cdev->devdata;
struct acpi_fan *fan = acpi_driver_data(device);
- if (fan->acpi4)
- *state = fan->fps_count - 1;
- else
+ if (fan->acpi4) {
+ if (fan->fif.fine_grain_ctrl)
+ *state = 100 / fan->fif.step_size;
+ else
+ *state = fan->fps_count - 1;
+ } else {
*state = 1;
+ }
+
return 0;
}
-static int fan_get_state_acpi4(struct acpi_device *device, unsigned long *state)
+static int acpi_fan_get_fst(struct acpi_device *device, struct acpi_fan_fst *fst)
{
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
- struct acpi_fan *fan = acpi_driver_data(device);
union acpi_object *obj;
acpi_status status;
- int control, i;
+ int ret = 0;
status = acpi_evaluate_object(device->handle, "_FST", NULL, &buffer);
if (ACPI_FAILURE(status)) {
@@ -89,35 +93,52 @@ static int fan_get_state_acpi4(struct acpi_device *device, unsigned long *state)
obj->package.count != 3 ||
obj->package.elements[1].type != ACPI_TYPE_INTEGER) {
dev_err(&device->dev, "Invalid _FST data\n");
- status = -EINVAL;
+ ret = -EINVAL;
goto err;
}
- control = obj->package.elements[1].integer.value;
+ fst->revision = obj->package.elements[0].integer.value;
+ fst->control = obj->package.elements[1].integer.value;
+ fst->speed = obj->package.elements[2].integer.value;
+
+err:
+ kfree(obj);
+ return ret;
+}
+
+static int fan_get_state_acpi4(struct acpi_device *device, unsigned long *state)
+{
+ struct acpi_fan *fan = acpi_driver_data(device);
+ struct acpi_fan_fst fst;
+ int status, i;
+
+ status = acpi_fan_get_fst(device, &fst);
+ if (status)
+ return status;
+
+ if (fan->fif.fine_grain_ctrl) {
+ /* This control should be same what we set using _FSL by spec */
+ if (fst.control > 100) {
+ dev_dbg(&device->dev, "Invalid control value returned\n");
+ goto match_fps;
+ }
+
+ *state = fst.control / fan->fif.step_size;
+ return 0;
+ }
+
+match_fps:
for (i = 0; i < fan->fps_count; i++) {
- /*
- * When Fine Grain Control is set, return the state
- * corresponding to maximum fan->fps[i].control
- * value compared to the current speed. Here the
- * fan->fps[] is sorted array with increasing speed.
- */
- if (fan->fif.fine_grain_ctrl && control < fan->fps[i].control) {
- i = (i > 0) ? i - 1 : 0;
- break;
- } else if (control == fan->fps[i].control) {
+ if (fst.control == fan->fps[i].control)
break;
- }
}
if (i == fan->fps_count) {
dev_dbg(&device->dev, "Invalid control value returned\n");
- status = -EINVAL;
- goto err;
+ return -EINVAL;
}
*state = i;
-err:
- kfree(obj);
return status;
}
@@ -161,12 +182,27 @@ static int fan_set_state_acpi4(struct acpi_device *device, unsigned long state)
{
struct acpi_fan *fan = acpi_driver_data(device);
acpi_status status;
+ u64 value = state;
+ int max_state;
- if (state >= fan->fps_count)
+ if (fan->fif.fine_grain_ctrl)
+ max_state = 100 / fan->fif.step_size;
+ else
+ max_state = fan->fps_count - 1;
+
+ if (state > max_state)
return -EINVAL;
- status = acpi_execute_simple_method(device->handle, "_FSL",
- fan->fps[state].control);
+ if (fan->fif.fine_grain_ctrl) {
+ value *= fan->fif.step_size;
+ /* Spec allows compensate the last step only */
+ if (value + fan->fif.step_size > 100)
+ value = 100;
+ } else {
+ value = fan->fps[state].control;
+ }
+
+ status = acpi_execute_simple_method(device->handle, "_FSL", value);
if (ACPI_FAILURE(status)) {
dev_dbg(&device->dev, "Failed to set state by _FSL\n");
return -ENODEV;
@@ -238,6 +274,12 @@ static int acpi_fan_get_fif(struct acpi_device *device)
fan->fif.step_size = fields[2];
fan->fif.low_speed_notification = fields[3];
+ /* If there is a bug in step size and set as 0, change to 1 */
+ if (!fan->fif.step_size)
+ fan->fif.step_size = 1;
+ /* If step size > 9, change to 9 (by spec valid values 1-9) */
+ else if (fan->fif.step_size > 9)
+ fan->fif.step_size = 9;
err:
kfree(obj);
return status;
--
2.34.1
Powered by blists - more mailing lists