On Thu, 4 Jul 2013 17:19:04 +0000
"R, Durgadoss" <durgados...@intel.com> wrote:
Hi,

> Hi Lukasz,
> 
> > -----Original Message-----
> > From: linux-pm-ow...@vger.kernel.org [mailto:linux-pm-
> > ow...@vger.kernel.org] On Behalf Of Lukasz Majewski
> > Sent: Thursday, July 04, 2013 2:20 PM
> > To: Viresh Kumar; Rafael J. Wysocki; Zhang, Rui; Eduardo Valentin
> > Cc: cpuf...@vger.kernel.org; Linux PM list; Jonghwa Lee; Lukasz
> > Majewski; l.majew...@majess.pl; linux-kernel; Andre Przywara;
> > Daniel Lezcano; Kukjin Kim; Myungjoo Ham
> > Subject: [PATCH v5 5/7] thermal:boost: Automatic enable/disable of
> > BOOST feature
> > 
> > This patch provides auto disable/enable operation for boost. When
> > any defined trip point is passed, the boost is disabled.
> > In that moment thermal monitor workqueue is woken up and it monitors
> > if the device temperature drops below 75% of the smallest trip
> > point. When device cools down, the boost is enabled again.
> > 
> > Signed-off-by: Lukasz Majewski <l.majew...@samsung.com>
> > Signed-off-by: Myungjoo Ham <myungjoo....@samsung.com>
> > 
> > ---
> > Changes for v5:
> > - Move boost disable code from cpu_cooling.c to thermal_core.c
> >   (to handle_non_critical_trips)
> > - Extent struct thermal_zone_device by adding overheated bool flag
> > - Implement auto enable of boost after device cools down
> > - Introduce boost_polling flag, which indicates if thermal uses
> > it's predefined pool delay or has woken up thermal workqueue only
> > to wait until device cools down.
> > 
> > Changes for v4:
> > - New patch
> > 
> >  drivers/thermal/thermal_core.c |   31
> > +++++++++++++++++++++++++++++++ include/linux/thermal.h        |
> > 2 ++ 2 files changed, 33 insertions(+)
> > 
> > diff --git a/drivers/thermal/thermal_core.c
> > b/drivers/thermal/thermal_core.c index d755440..12adbad 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -33,6 +33,7 @@
> >  #include <linux/idr.h>
> >  #include <linux/thermal.h>
> >  #include <linux/reboot.h>
> > +#include <linux/cpufreq.h>
> >  #include <net/netlink.h>
> >  #include <net/genetlink.h>
> > 
> > @@ -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.

> 
> > +           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)?

> 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.

> Rui/Eduardo: Any thoughts on this ?
> 
> Thanks,
> Durga
> > +
> > +                   cpufreq_boost_trigger_state(1);
> > +                   return;
> > +           }
> > +   }
> > +
> >     thermal_zone_device_update(tz);
> >  }
> > 
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index a386a1c..f1aa3c2 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -172,6 +172,8 @@ struct thermal_zone_device {
> >     int emul_temperature;
> >     int passive;
> >     unsigned int forced_passive;
> > +   bool overheated;
> > +   bool boost_polling;
> >     const struct thermal_zone_device_ops *ops;
> >     const struct thermal_zone_params *tzp;
> >     struct thermal_governor *governor;
> > --
> > 1.7.10.4
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pm"
> > in the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to