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 cpufreq" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
