On Fri, 05 Jul 2013 05:31:42 +0000, R, Durgadoss wrote:
Hi Durga,

Hi Lukasz,

quoted

-----Original Message-----
From: Lukasz Majewski [mailto:l.majewski@majess.pl]
Sent: Friday, July 05, 2013 2:28 AM
To: R, Durgadoss
Cc: Lukasz Majewski; Viresh Kumar; Rafael J. Wysocki; Zhang, Rui;
Eduardo Valentin; cpufreq@vger.kernel.org; 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" [off-list ref] wrote:
Hi,

[Cut.]

quoted

quoted

quoted

@@ -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 ;)

Ok, I see :-).

quoted

quoted

quoted

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

quoted

quoted

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

quoted

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.

Ok.

quoted

quoted

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 for understanding. I'm going to embed the trend checking in the
next version of this patch (to be more confident that I can reenable
boost).

Thanks,
Durga

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group