| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9cbc3ee82561859acbd6f4787c82f453f8a7afeb.camel@linux.intel.com>
Date: Wed, 04 Jun 2025 14:40:01 -0700
From: srinivas pandruvada <srinivas.pandruvada@...ux.intel.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: rui.zhang@...el.com, daniel.lezcano@...aro.org, lukasz.luba@....com,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH 1/2] thermal: intel: int340x: Add performance control
for platform temperature control
On Wed, 2025-06-04 at 22:55 +0200, Rafael J. Wysocki wrote:
> On Wed, Jun 4, 2025 at 10:35 PM Srinivas Pandruvada
> <srinivas.pandruvada@...ux.intel.com> wrote:
> >
> > Add additional attribute to control performance of platform
> > temperature
> > control feature. Two attributes are added:
>
> It would be good to explain why they are needed.
This is to give user space some influence in the aggressiveness of
control loops in the firmware. Firmware may be aggressive but there
can be some tolerance based on current conditions like ambient
temperature or whether the laptop is placed not on lap. Similar to
current adaptive thermal controls changing temperature limits and power
limits, with the PTC these controls can be used.
>
> > gain: 0-7 levels, with 0 being most aggressive.
> > 7 – graceful, favors performance at the expense of
> > temperature
> > overshoots
> > 0 – aggressive, favors tight regulation over performance
> >
> > min_performance_level: A value from 0-255. Specifies minimum
> > Performance
> > level below which the there will be no throttling.
> > 0 - all levels of throttling allowed including
> > survivability
> > actions.
> > 255 - no throttling allowed.
>
> The description of min_performance_level above doesn't seem to be
> consistent to me. Specifically, the descriptions of the 0 and 255
> values appear to indicate that this really is about what kind of
> throttling can be applied.
At the end this is some power/frequency throttling to various sub
systems. 0 means that to meet temperature threshold, the power can be
lower than the lowest package power limit which is survivability mode.
>
> Also, I gather that the units of this number are arbitrary and it is
> not tied to anything specific. I mean, 127 doesn't mean 50% of CPU
> performance, for instance.
It is unit-less. It may not scale linearly to assume any value to
percent.
Thanks,
Srinivas
>
> > Signed-off-by: Srinivas Pandruvada
> > <srinivas.pandruvada@...ux.intel.com>
> > ---
> > Documentation/driver-api/thermal/intel_dptf.rst | 10 ++++++++++
> > .../platform_temperature_control.c | 15
> > ++++++++++++++-
> > 2 files changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/driver-api/thermal/intel_dptf.rst
> > b/Documentation/driver-api/thermal/intel_dptf.rst
> > index ec5769accae0..794f5cce548e 100644
> > --- a/Documentation/driver-api/thermal/intel_dptf.rst
> > +++ b/Documentation/driver-api/thermal/intel_dptf.rst
> > @@ -206,6 +206,16 @@ All these controls needs admin privilege to
> > update.
> > Update a new temperature target in milli degree celsius for
> > hardware to
> > use for the temperature control.
> >
> > +``gain`` (RW)
> > + A value in the range 0-7. Sets the aggressiveness of
> > control loop.
> > + 7 – graceful, favors performance at the expense of
> > temperature overshoots.
> > + 0 – aggressive, favors tight regulation over performance.
> > +
> > +``min_performance_level`` (RW)
> > + Minimum Performance level below which the there will be no
> > throttling.
> > + 0 - all levels of throttling allowed including
> > survivability actions.
> > + 256 - no throttling allowed.
> > +
> > Given that this is platform temperature control, it is expected
> > that a
> > single user-level manager owns and manages the controls. If
> > multiple
> > user-level software applications attempt to write different
> > targets, it
> > diff --git
> > a/drivers/thermal/intel/int340x_thermal/platform_temperature_contro
> > l.c
> > b/drivers/thermal/intel/int340x_thermal/platform_temperature_contro
> > l.c
> > index 2d6504514893..6cd05783a52d 100644
> > ---
> > a/drivers/thermal/intel/int340x_thermal/platform_temperature_contro
> > l.c
> > +++
> > b/drivers/thermal/intel/int340x_thermal/platform_temperature_contro
> > l.c
> > @@ -49,7 +49,7 @@ struct mmio_reg {
> > };
> >
> > #define MAX_ATTR_GROUP_NAME_LEN 32
> > -#define PTC_MAX_ATTRS 3
> > +#define PTC_MAX_ATTRS 5
> >
> > struct ptc_data {
> > u32 offset;
> > @@ -57,6 +57,8 @@ struct ptc_data {
> > struct attribute *ptc_attrs[PTC_MAX_ATTRS];
> > struct device_attribute temperature_target_attr;
> > struct device_attribute enable_attr;
> > + struct device_attribute gain_attr;
> > + struct device_attribute min_performance_level_attr;
> > char group_name[MAX_ATTR_GROUP_NAME_LEN];
> > };
> >
> > @@ -78,6 +80,8 @@ static u32 ptc_offsets[PTC_MAX_INSTANCES] =
> > {0x5B20, 0x5B28, 0x5B30};
> > static const char * const ptc_strings[] = {
> > "temperature_target",
> > "enable",
> > + "gain",
> > + "min_performance_level",
> > NULL
> > };
> >
> > @@ -177,6 +181,11 @@ PTC_SHOW(temperature_target);
> > PTC_STORE(temperature_target);
> > PTC_SHOW(enable);
> > PTC_STORE(enable);
> > +PTC_SHOW(gain);
> > +PTC_STORE(gain);
> > +PTC_SHOW(min_performance_level);
> > +PTC_STORE(min_performance_level);
> > +
> >
> > #define ptc_init_attribute(_name)\
> > do {\
> > @@ -193,9 +202,13 @@ static int ptc_create_groups(struct pci_dev
> > *pdev, int instance, struct ptc_data
> >
> > ptc_init_attribute(temperature_target);
> > ptc_init_attribute(enable);
> > + ptc_init_attribute(gain);
> > + ptc_init_attribute(min_performance_level);
> >
> > data->ptc_attrs[index++] = &data-
> > >temperature_target_attr.attr;
> > data->ptc_attrs[index++] = &data->enable_attr.attr;
> > + data->ptc_attrs[index++] = &data->gain_attr.attr;
> > + data->ptc_attrs[index++] = &data-
> > >min_performance_level_attr.attr;
> > data->ptc_attrs[index] = NULL;
> >
> > snprintf(data->group_name, MAX_ATTR_GROUP_NAME_LEN,
> > --
> > 2.49.0
> >
Powered by blists - more mailing lists