RE: [PATCH v5 5/7] thermal:boost: Automatic enable/disable of BOOSTfeature
From: R, Durgadoss
Date: Fri Jul 05 2013 - 01:31:58 EST
Hi Lukasz,
>
-----Original Message-----
>
From: Lukasz Majewski [mailto:l.majewski@xxxxxxxxx]
>
Sent: Friday, July 05, 2013 2:28 AM
>
To: R, Durgadoss
>
Cc: Lukasz Majewski; Viresh Kumar; Rafael J. Wysocki; Zhang, Rui; Eduardo
>
Valentin; cpufreq@xxxxxxxxxxxxxxx; Linux PM list; Jonghwa Lee; linux-kernel;
>
Andre Przywara; Daniel Lezcano; Kukjin Kim; Myungjoo Ham
>
Subject: Re: [PATCH v5 5/7] thermal:boost: Automatic enable/disable of BOOST
>
feature
>
>
On Thu, 4 Jul 2013 17:19:04 +0000
>
"R, Durgadoss" <durgadoss.r@xxxxxxxxx> wrote:
>
Hi,
>
[Cut.]
>
> > @@ -326,6 +327,15 @@ static void monitor_thermal_zone(struct
>
> > thermal_zone_device *tz)
>
> > static void handle_non_critical_trips(struct thermal_zone_device
>
> > *tz, int trip, enum thermal_trip_type trip_type)
>
> > {
>
> > + if (cpufreq_boost_supported()) {
>
> > + tz->overheated = true;
>
> > + cpufreq_boost_trigger_state(0);
>
> > + if (!tz->polling_delay) {
>
> > + tz->boost_polling = true;
>
> > + tz->polling_delay = 1000;
>
> > + }
>
> > + }
>
> > +
>
> > if (tz->governor)
>
> > tz->governor->throttle(tz, trip);
>
> > }
>
> > @@ -453,6 +463,27 @@ static void thermal_zone_device_check(struct
>
> > work_struct *work)
>
> > struct thermal_zone_device *tz = container_of(work, struct
>
> > thermal_zone_device,
>
> > poll_queue.work);
>
> > + long trip_temp;
>
> > +
>
> > + if (cpufreq_boost_supported() && tz->overheated) {
>
>
>
> Not all thermal drivers support trip points. So, we first need a
>
> if (tz->ops->get_trip_temp) check here.
>
>
Ok, thanks for tip. Bluntly speaking, I thought, that all SoCs supported
>
by thermal set trip points.
We would wish to get there. But not the reality today ;)
>
>
>
>
> > + tz->ops->get_trip_temp(tz, 0, &trip_temp);
>
> > + /*
>
> > + * Enable boost again only when current
>
> > temperature is less
>
> > + * than 75% of trip_temp[0]
>
> > + */
>
> > + if ((tz->temperature + (trip_temp >> 2)) <
>
> > trip_temp) {
>
>
>
> Another way would be to use the get_trend APIs for this thermal zone.
>
> If the trend is cooling we can re-enable boost otherwise not.
>
>
Trend evaluation seems like a good complementary idea.
>
>
However, I would also like to have the relative temperature drop
>
measurement (if possible) like above (to 75% of the first trip point).
>
>
Then I would be more confident, that everything cooled down and that I
>
can start boost again.
>
>
>
>
> > + tz->overheated = false;
>
> > + if (tz->boost_polling) {
>
> > + tz->boost_polling = false;
>
> > + tz->polling_delay = 0;
>
> > + monitor_thermal_zone(tz);
>
> > + }
>
>
>
> Overall, I believe this will work well only if the thermal zone is
>
> CPU.
>
>
My assumption:
>
>
When I enable boost at CPU, then I also shall cool down the CPU. And
>
the CPU zone seemed a natural choice.
>
>
However I might be missing something, so hints are welcome.
>
>
>
>
> Another suggestion is: We tried hard to remove all throttling logic
>
> from thermal_core.c.
>
>
By throttling logic you mean:
>
if ((tz->temperature + (trip_temp >> 2)) and other conditions (like
>
trend measurement)?
Yes. That is correct.
>
>
> May be we should include this kind of logic in
>
> step_wise.c ?
>
>
It sounds interesting (since ->throttle at thermal_core.c is called
>
always when needed), but I'm afraid of a code duplication when one
>
use Boost with fair_share or other thermal governor.
right. So, for the time being, (as part of this patch series)
I am Okay to have this code in thermal_core.c. From the thermal
subsystem perspective, we will (need to) work out a better/
cleaner/easier approach for this later.
Thanks,
Durga
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
http://vger.kernel.org/majordomo-info.html
Please read the FAQ at
http://www.tux.org/lkml/