[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJZ5v0g7ZuY9_6zz6KP7+zhxDb99mdbQULhyNdHOy+Vi19FYtg@mail.gmail.com>
Date: Wed, 16 Mar 2022 19:42:37 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Amit Kucheria <amitk@...nel.org>,
"Zhang, Rui" <rui.zhang@...el.com>,
Linux PM <linux-pm@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
David Laight <David.Laight@...lab.com>
Subject: Re: [PATCH v2] thermal: int340x: Update OS policy capability
On Mon, Mar 14, 2022 at 11:09 PM Srinivas Pandruvada
<srinivas.pandruvada@...ux.intel.com> wrote:
>
> Update the firmware with OS supported policies mask, so that firmware can
> relinquish its internal controls. Without this update several Tiger Lake
> laptops gets performance limited with in few seconds of executing in
> turbo region.
>
> The existing way of enumerating firmware policies via IDSP method and
> selecting policy by directly writing those policy UUIDS via _OSC method
> is not supported in newer generation of hardware.
>
> There is a new UUID "B23BA85D-C8B7-3542-88DE-8DE2FFCFD698" is defined for
> updating policy capabilities. As part of ACPI _OSC method:
>
> Arg0 - UUID: B23BA85D-C8B7-3542-88DE-8DE2FFCFD698
> Arg1 - Rev ID: 1
> Arg2 - Count: 2
> Arg3 - Capability buffers: Array of Arg2 DWORDS
>
> DWORD1: As defined in the ACPI 5.0 Specification
> - Bit 0: Query Flag
> - Bits 1-3: Always 0
> - Bits 4-31: Reserved
>
> DWORD2 and beyond:
> - Bit0: set to 1 to indicate Intel(R) Dynamic Tuning is active, 0 to
> indicate it is disabled and legacy thermal mechanism should
> be enabled.
> - Bit1: set to 1 to indicate Intel(R) Dynamic Tuning is controlling
> active cooling, 0 to indicate bios shall enable legacy thermal
> zone with active trip point.
> - Bit2: set to 1 to indicate Intel(R) Dynamic Tuning is controlling
> passive cooling, 0 to indicate bios shall enable legacy thermal
> zone with passive trip point.
> - Bit3: set to 1 to indicate Intel(R) Dynamic Tuning is handling
> critical trip point, 0 to indicate bios shall enable legacy
> thermal zone with critical trip point.
> - Bits 4:31: Reserved
>
> From sysfs interface, there is an existing interface to update policy
> UUID using attribute "current_uuid". User space can write the same UUID
> for ACTIVE, PASSIVE and CRITICAL policy. Driver converts these UUIDs to
> DWORD2 Bit 1 to Bit 3. When any of the policy is activated by user
> space it is assumed that dynamic tuning is active.
>
> For example
> $cd /sys/bus/platform/devices/INTC1040:00/uuids
> To support active policy
> $echo "3A95C389-E4B8-4629-A526-C52C88626BAE" > current_uuid
> To support passive policy
> $echo "42A441D6-AE6A-462b-A84B-4A8CE79027D3" > current_uuid
> To support critical policy
> $echo "97C68AE7-15FA-499c-B8C9-5DA81D606E0A" > current_uuid
>
> To check all the supported policies
> $cat current_uuid
> 3A95C389-E4B8-4629-A526-C52C88626BAE
> 42A441D6-AE6A-462b-A84B-4A8CE79027D3
> 97C68AE7-15FA-499c-B8C9-5DA81D606E0A
>
> To match the bit format for DWORD2, rearranged enum int3400_thermal_uuid
> and int3400_thermal_uuids[] by swapping current INT3400_THERMAL_ACTIVE
> and INT3400_THERMAL_PASSIVE_1.
>
> If the policies are enumerated via IDSP method then legacy method is
> used, if not the new method is used to update policy support.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Applied as 5.18 material, thanks!
> ---
> v2
> - Changed u16 to u32 for better alignment as suggested by David
>
> .../intel/int340x_thermal/int3400_thermal.c | 146 ++++++++++++------
> 1 file changed, 97 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 348b1f4ef801..b06a70fc6c18 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -17,8 +17,8 @@
> #define INT3400_KEEP_ALIVE 0xA0
>
> enum int3400_thermal_uuid {
> + INT3400_THERMAL_ACTIVE = 0,
> INT3400_THERMAL_PASSIVE_1,
> - INT3400_THERMAL_ACTIVE,
> INT3400_THERMAL_CRITICAL,
> INT3400_THERMAL_ADAPTIVE_PERFORMANCE,
> INT3400_THERMAL_EMERGENCY_CALL_MODE,
> @@ -31,8 +31,8 @@ enum int3400_thermal_uuid {
> };
>
> static char *int3400_thermal_uuids[INT3400_THERMAL_MAXIMUM_UUID] = {
> - "42A441D6-AE6A-462b-A84B-4A8CE79027D3",
> "3A95C389-E4B8-4629-A526-C52C88626BAE",
> + "42A441D6-AE6A-462b-A84B-4A8CE79027D3",
> "97C68AE7-15FA-499c-B8C9-5DA81D606E0A",
> "63BE270F-1C11-48FD-A6F7-3AF253FF3E2D",
> "5349962F-71E6-431D-9AE8-0A635B710AEE",
> @@ -59,6 +59,7 @@ struct int3400_thermal_priv {
> char *data_vault;
> int odvp_count;
> int *odvp;
> + u32 os_uuid_mask;
> struct odvp_attr *odvp_attrs;
> };
>
> @@ -142,12 +143,55 @@ static ssize_t current_uuid_show(struct device *dev,
> struct device_attribute *devattr, char *buf)
> {
> struct int3400_thermal_priv *priv = dev_get_drvdata(dev);
> + int i, length = 0;
> +
> + if (priv->current_uuid_index > 0)
> + return sprintf(buf, "%s\n",
> + int3400_thermal_uuids[priv->current_uuid_index]);
> +
> + for (i = 0; i <= INT3400_THERMAL_CRITICAL; i++) {
> + if (priv->os_uuid_mask & BIT(i))
> + length += scnprintf(&buf[length],
> + PAGE_SIZE - length,
> + "%s\n",
> + int3400_thermal_uuids[i]);
> + }
>
> - if (priv->current_uuid_index == -1)
> - return sprintf(buf, "INVALID\n");
> + if (length)
> + return length;
>
> - return sprintf(buf, "%s\n",
> - int3400_thermal_uuids[priv->current_uuid_index]);
> + return sprintf(buf, "INVALID\n");
> +}
> +
> +static int int3400_thermal_run_osc(acpi_handle handle, char *uuid_str, int *enable)
> +{
> + u32 ret, buf[2];
> + acpi_status status;
> + int result = 0;
> + struct acpi_osc_context context = {
> + .uuid_str = NULL,
> + .rev = 1,
> + .cap.length = 8,
> + };
> +
> + context.uuid_str = uuid_str;
> +
> + buf[OSC_QUERY_DWORD] = 0;
> + buf[OSC_SUPPORT_DWORD] = *enable;
> +
> + context.cap.pointer = buf;
> +
> + status = acpi_run_osc(handle, &context);
> + if (ACPI_SUCCESS(status)) {
> + ret = *((u32 *)(context.ret.pointer + 4));
> + if (ret != *enable)
> + result = -EPERM;
> + } else
> + result = -EPERM;
> +
> + kfree(context.ret.pointer);
> +
> + return result;
> }
>
> static ssize_t current_uuid_store(struct device *dev,
> @@ -164,16 +208,47 @@ static ssize_t current_uuid_store(struct device *dev,
> * If we have a list of supported UUIDs, make sure
> * this one is supported.
> */
> - if (priv->uuid_bitmap &&
> - !(priv->uuid_bitmap & (1 << i)))
> + if (priv->uuid_bitmap & BIT(i)) {
> + priv->current_uuid_index = i;
> + return count;
> + }
> +
> + /*
> + * There is support of only 3 policies via the new
> + * _OSC to inform OS capability:
> + * INT3400_THERMAL_ACTIVE
> + * INT3400_THERMAL_PASSIVE_1
> + * INT3400_THERMAL_CRITICAL
> + */
> +
> + if (i > INT3400_THERMAL_CRITICAL)
> return -EINVAL;
>
> - priv->current_uuid_index = i;
> - return count;
> + priv->os_uuid_mask |= BIT(i);
> +
> + break;
> }
> }
>
> - return -EINVAL;
> + if (priv->os_uuid_mask) {
> + int cap, ret;
> +
> + /*
> + * Capability bits:
> + * Bit 0: set to 1 to indicate DPTF is active
> + * Bi1 1: set to 1 to active cooling is supported by user space daemon
> + * Bit 2: set to 1 to passive cooling is supported by user space daemon
> + * Bit 3: set to 1 to critical trip is handled by user space daemon
> + */
> + cap = ((priv->os_uuid_mask << 1) | 0x01);
> + ret = int3400_thermal_run_osc(priv->adev->handle,
> + "b23ba85d-c8b7-3542-88de-8de2ffcfd698",
> + &cap);
> + if (ret)
> + return ret;
> + }
> +
> + return count;
> }
>
> static DEVICE_ATTR_RW(current_uuid);
> @@ -236,41 +311,6 @@ static int int3400_thermal_get_uuids(struct int3400_thermal_priv *priv)
> return result;
> }
>
> -static int int3400_thermal_run_osc(acpi_handle handle,
> - enum int3400_thermal_uuid uuid, bool enable)
> -{
> - u32 ret, buf[2];
> - acpi_status status;
> - int result = 0;
> - struct acpi_osc_context context = {
> - .uuid_str = NULL,
> - .rev = 1,
> - .cap.length = 8,
> - };
> -
> - if (uuid < 0 || uuid >= INT3400_THERMAL_MAXIMUM_UUID)
> - return -EINVAL;
> -
> - context.uuid_str = int3400_thermal_uuids[uuid];
> -
> - buf[OSC_QUERY_DWORD] = 0;
> - buf[OSC_SUPPORT_DWORD] = enable;
> -
> - context.cap.pointer = buf;
> -
> - status = acpi_run_osc(handle, &context);
> - if (ACPI_SUCCESS(status)) {
> - ret = *((u32 *)(context.ret.pointer + 4));
> - if (ret != enable)
> - result = -EPERM;
> - } else
> - result = -EPERM;
> -
> - kfree(context.ret.pointer);
> -
> - return result;
> -}
> -
> static ssize_t odvp_show(struct kobject *kobj, struct kobj_attribute *attr,
> char *buf)
> {
> @@ -422,10 +462,18 @@ static int int3400_thermal_change_mode(struct thermal_zone_device *thermal,
> if (!priv)
> return -EINVAL;
>
> - if (mode != thermal->mode)
> + if (mode != thermal->mode) {
> + int enabled;
> +
> + if (priv->current_uuid_index < 0 ||
> + priv->current_uuid_index >= INT3400_THERMAL_MAXIMUM_UUID)
> + return -EINVAL;
> +
> + enabled = (mode == THERMAL_DEVICE_ENABLED);
> result = int3400_thermal_run_osc(priv->adev->handle,
> - priv->current_uuid_index,
> - mode == THERMAL_DEVICE_ENABLED);
> + int3400_thermal_uuids[priv->current_uuid_index],
> + &enabled);
> + }
>
>
> evaluate_odvp(priv);
> --
> 2.31.1
>
Powered by blists - more mailing lists