Messages in this thread Patch in this message | | | Date | Thu, 26 Feb 2015 17:30:00 -0400 | From | Eduardo Valentin <> | Subject | Re: [PATCH v2 7/7] thermal: export thermal_zone_parameters to sysfs |
| |
On Thu, Feb 26, 2015 at 07:00:33PM +0000, Javi Merino wrote: > It's useful for tuning to be able to edit thermal_zone_parameters from > userspace. Export them to the thermal_zone sysfs so that they can be > easily changed. > > Cc: Zhang Rui <rui.zhang@intel.com> > Cc: Eduardo Valentin <edubezval@gmail.com> > Signed-off-by: Javi Merino <javi.merino@arm.com> > --- > Documentation/thermal/sysfs-api.txt | 52 +++++++++++++++++ > drivers/thermal/thermal_core.c | 110 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 162 insertions(+) > > diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt > index fc7dfe10778b..7d44d7f1a71b 100644 > --- a/Documentation/thermal/sysfs-api.txt > +++ b/Documentation/thermal/sysfs-api.txt > @@ -184,6 +184,12 @@ Thermal zone device sys I/F, created once it's registered: > |---trip_point_[0-*]_type: Trip point type > |---trip_point_[0-*]_hyst: Hysteresis value for this trip point > |---emul_temp: Emulated temperature set node > + |---sustainable_power: Sustainable dissipatable power > + |---k_po: Proportional term during temperature overshoot > + |---k_pu: Proportional term during temperature undershoot > + |---k_i: PID's integral term in the power allocator gov > + |---k_d: PID's derivative term in the power allocator > + |---integral_cutoff: Offset above which errors are accumulated
Can this be under a specific directory?
I thought of something like /sys/class/thermal/thermal_zoneX/governor_params/<gov spec files>
The above node can be handled by the governor code.
> > Thermal cooling device sys I/F, created once it's registered: > /sys/class/thermal/cooling_device[0-*]: > @@ -307,6 +313,52 @@ emul_temp > because userland can easily disable the thermal policy by simply > flooding this sysfs node with low temperature values. > > +sustainable_power > + An estimate of the sustained power that can be dissipated by > + the thermal zone. Used by the power allocator governor. For > + more information see Documentation/thermal/power_allocator.txt > + Unit: milliwatts > + RW, Optional > + > +k_po > + The proportional term of the power allocator governor's PID > + controller during temperature overshoot. Temperature overshoot > + is when the current temperature is above the "desired > + temperature" trip point. For more information see > + Documentation/thermal/power_allocator.txt > + RW, Optional > + > +k_pu > + The proportional term of the power allocator governor's PID > + controller during temperature undershoot. Temperature undershoot > + is when the current temperature is below the "desired > + temperature" trip point. For more information see > + Documentation/thermal/power_allocator.txt > + RW, Optional > + > +k_i > + The integral term of the power allocator governor's PID > + controller. This term allows the PID controller to compensate > + for long term drift. For more information see > + Documentation/thermal/power_allocator.txt > + RW, Optional > + > +k_d > + The derivative term of the power allocator governor's PID > + controller. For more information see > + Documentation/thermal/power_allocator.txt > + RW, Optional > + > +integral_cutoff > + Temperature offset from the desired temperature trip point > + above which the integral term of the power allocator > + governor's PID controller starts accumulating errors. For > + example, if integral_cutoff is 0, then the integral term only > + accumulates error when temperature is above the desired > + temperature trip point. For more information see > + Documentation/thermal/power_allocator.txt > + RW, Optional > + > ***************************** > * Cooling device attributes * > ***************************** > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index df9ba3bf55dc..228e93f8a146 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -873,6 +873,111 @@ emul_temp_store(struct device *dev, struct device_attribute *attr, > static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store); > #endif/*CONFIG_THERMAL_EMULATION*/ > > +#ifdef CONFIG_THERMAL_GOV_POWER_ALLOCATOR > + > +static ssize_t > +sustainable_power_show(struct device *dev, struct device_attribute *devattr, > + char *buf) > +{ > + struct thermal_zone_device *tz = to_thermal_zone(dev); > + > + if (tz->tzp) > + return sprintf(buf, "%u\n", tz->tzp->sustainable_power); > + else > + return -EIO; > +} > + > +static ssize_t > +sustainable_power_store(struct device *dev, struct device_attribute *devattr, > + const char *buf, size_t count) > +{ > + struct thermal_zone_device *tz = to_thermal_zone(dev); > + u32 sustainable_power; > + > + if (!tz->tzp) > + return -EIO; > + > + if (kstrtou32(buf, 10, &sustainable_power)) > + return -EINVAL; > + > + tz->tzp->sustainable_power = sustainable_power; > + > + return count; > +} > +static DEVICE_ATTR(sustainable_power, S_IWUSR | S_IRUGO, sustainable_power_show, > + sustainable_power_store); > + > +#define create_s32_tzp_attr(name) \ > + static ssize_t \ > + name##_show(struct device *dev, struct device_attribute *devattr, \ > + char *buf) \ > + { \ > + struct thermal_zone_device *tz = to_thermal_zone(dev); \ > + \ > + if (tz->tzp) \ > + return sprintf(buf, "%u\n", tz->tzp->name); \ > + else \ > + return -EIO; \ > + } \ > + \ > + static ssize_t \ > + name##_store(struct device *dev, struct device_attribute *devattr, \ > + const char *buf, size_t count) \ > + { \ > + struct thermal_zone_device *tz = to_thermal_zone(dev); \ > + s32 value; \ > + \ > + if (!tz->tzp) \ > + return -EIO; \ > + \ > + if (kstrtos32(buf, 10, &value)) \ > + return -EINVAL; \ > + \ > + tz->tzp->name = value; \ > + \ > + return count; \ > + } \ > + static DEVICE_ATTR(name, S_IWUSR | S_IRUGO, name##_show, name##_store) > + > +create_s32_tzp_attr(k_po); > +create_s32_tzp_attr(k_pu); > +create_s32_tzp_attr(k_i); > +create_s32_tzp_attr(k_d); > +create_s32_tzp_attr(integral_cutoff); > +#undef create_s32_tzp_attr > + > +static struct device_attribute *dev_tzp_attrs[] = { > + &dev_attr_sustainable_power, > + &dev_attr_k_po, > + &dev_attr_k_pu, > + &dev_attr_k_i, > + &dev_attr_k_d, > + &dev_attr_integral_cutoff, > +}; > + > +static int create_power_allocator_tzp_attrs(struct device *dev) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(dev_tzp_attrs); i++) { > + int ret; > + struct device_attribute *dev_attr = dev_tzp_attrs[i]; > + > + ret = device_create_file(dev, dev_attr); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +#else /* !CONFIG_THERMAL_GOV_POWER_ALLOCATOR */ > +static int create_power_allocator_tzp_attrs(struct device *dev) > +{ > + return 0; > +} > +#endif > +
It is better if this code is not part of thermal_core. The governor specific code must be in the governor code. Durga has done a pretty good job splitting governor code out of thermal_core.c. I don't think we want to get them back.
> /** > * power_actor_get_max_power() - get the maximum power that a cdev can consume > * @cdev: pointer to &thermal_cooling_device > @@ -1712,6 +1817,11 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type, > if (result) > goto unregister; > > + /* Add power_allocator specific thermal zone params */ > + result = create_power_allocator_tzp_attrs(&tz->device); > + if (result) > + goto unregister;
Here you could create the governor_params and have a callback from governor to populate it.
> + > /* Update 'this' zone's governor information */ > mutex_lock(&thermal_governor_lock); > > -- > 1.9.1 > [unhandled content-type:application/pgp-signature] | |