[RFC,4/4] thermal: Modify thermal governors to do nothing for "cold" trip points
diff mbox series

Message ID 20200710135154.181454-5-thara.gopinath@linaro.org
State New
Headers show
Series
  • thermal: Introduce support for monitoring falling temperature
Related show

Commit Message

Thara Gopinath July 10, 2020, 1:51 p.m. UTC
For now, thermal governors do not support monitoring of falling
temperature. Hence, in case of calls to the governor for trip points marked
as cold, return doing nothing.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 drivers/thermal/gov_bang_bang.c       | 8 ++++++++
 drivers/thermal/gov_fair_share.c      | 8 ++++++++
 drivers/thermal/gov_power_allocator.c | 8 ++++++++
 drivers/thermal/gov_step_wise.c       | 8 ++++++++
 4 files changed, 32 insertions(+)

Comments

Zhang Rui July 15, 2020, 8:35 a.m. UTC | #1
On Fri, 2020-07-10 at 09:51 -0400, Thara Gopinath wrote:
> For now, thermal governors do not support monitoring of falling
> temperature. Hence, in case of calls to the governor for trip points
> marked
> as cold, return doing nothing.
> 
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> ---
>  drivers/thermal/gov_bang_bang.c       | 8 ++++++++
>  drivers/thermal/gov_fair_share.c      | 8 ++++++++
>  drivers/thermal/gov_power_allocator.c | 8 ++++++++
>  drivers/thermal/gov_step_wise.c       | 8 ++++++++
>  4 files changed, 32 insertions(+)

userspace governor does not support cold trip point neither.

So how about adding the check in handle_non_critical_trips first, and
remove the check later, after all the governors support cold trip?

thanks,
rui
> 
> diff --git a/drivers/thermal/gov_bang_bang.c
> b/drivers/thermal/gov_bang_bang.c
> index 991a1c54296d..8324d13de1e7 100644
> --- a/drivers/thermal/gov_bang_bang.c
> +++ b/drivers/thermal/gov_bang_bang.c
> @@ -99,6 +99,14 @@ static void thermal_zone_trip_update(struct
> thermal_zone_device *tz, int trip)
>  static int bang_bang_control(struct thermal_zone_device *tz, int
> trip)
>  {
>  	struct thermal_instance *instance;
> +	enum thermal_trip_type trip_type;
> +
> +	/* Return doing nothing in case of cold trip point */
> +	if (trip != THERMAL_TRIPS_NONE) {
> +		tz->ops->get_trip_type(tz, trip, &trip_type);
> +		if (trip_type == THERMAL_TRIP_COLD)
> +			return 0;
> +	}
>  
>  	thermal_zone_trip_update(tz, trip);
>  
> diff --git a/drivers/thermal/gov_fair_share.c
> b/drivers/thermal/gov_fair_share.c
> index aaa07180ab48..c0adce525faa 100644
> --- a/drivers/thermal/gov_fair_share.c
> +++ b/drivers/thermal/gov_fair_share.c
> @@ -81,6 +81,14 @@ static int fair_share_throttle(struct
> thermal_zone_device *tz, int trip)
>  	int total_weight = 0;
>  	int total_instance = 0;
>  	int cur_trip_level = get_trip_level(tz);
> +	enum thermal_trip_type trip_type;
> +
> +	/* Return doing nothing in case of cold trip point */
> +	if (trip != THERMAL_TRIPS_NONE) {
> +		tz->ops->get_trip_type(tz, trip, &trip_type);
> +		if (trip_type == THERMAL_TRIP_COLD)
> +			return 0;
> +	}
>  
>  	list_for_each_entry(instance, &tz->thermal_instances, tz_node)
> {
>  		if (instance->trip != trip)
> diff --git a/drivers/thermal/gov_power_allocator.c
> b/drivers/thermal/gov_power_allocator.c
> index 44636475b2a3..2644ad4d4032 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -613,8 +613,16 @@ static int power_allocator_throttle(struct
> thermal_zone_device *tz, int trip)
>  {
>  	int ret;
>  	int switch_on_temp, control_temp;
> +	enum thermal_trip_type trip_type;
>  	struct power_allocator_params *params = tz->governor_data;
>  
> +	/* Return doing nothing in case of cold trip point */
> +	if (trip != THERMAL_TRIPS_NONE) {
> +		tz->ops->get_trip_type(tz, trip, &trip_type);
> +		if (trip_type == THERMAL_TRIP_COLD)
> +			return 0;
> +	}
> +
>  	/*
>  	 * We get called for every trip point but we only need to do
>  	 * our calculations once
> diff --git a/drivers/thermal/gov_step_wise.c
> b/drivers/thermal/gov_step_wise.c
> index 2ae7198d3067..009aefda0441 100644
> --- a/drivers/thermal/gov_step_wise.c
> +++ b/drivers/thermal/gov_step_wise.c
> @@ -186,6 +186,14 @@ static void thermal_zone_trip_update(struct
> thermal_zone_device *tz, int trip)
>  static int step_wise_throttle(struct thermal_zone_device *tz, int
> trip)
>  {
>  	struct thermal_instance *instance;
> +	enum thermal_trip_type trip_type;
> +
> +	/* For now, return doing nothing in case of cold trip point */
> +	if (trip != THERMAL_TRIPS_NONE) {
> +		tz->ops->get_trip_type(tz, trip, &trip_type);
> +		if (trip_type == THERMAL_TRIP_COLD)
> +			return 0;
> +	}
>  
>  	thermal_zone_trip_update(tz, trip);
>
Thara Gopinath July 15, 2020, 11:13 p.m. UTC | #2
On 7/15/20 4:35 AM, Zhang Rui wrote:
> On Fri, 2020-07-10 at 09:51 -0400, Thara Gopinath wrote:
>> For now, thermal governors do not support monitoring of falling
>> temperature. Hence, in case of calls to the governor for trip points
>> marked
>> as cold, return doing nothing.
>>
>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
>> ---
>>   drivers/thermal/gov_bang_bang.c       | 8 ++++++++
>>   drivers/thermal/gov_fair_share.c      | 8 ++++++++
>>   drivers/thermal/gov_power_allocator.c | 8 ++++++++
>>   drivers/thermal/gov_step_wise.c       | 8 ++++++++
>>   4 files changed, 32 insertions(+)
> 
> userspace governor does not support cold trip point neither.
> 
> So how about adding the check in handle_non_critical_trips first, and
> remove the check later, after all the governors support cold trip?

Yeah, no governors support cold trip for now. Putting check in 
handle_non_critical_trips is another way to handle this. I can do it and 
come back to this solution when one or more governors start supporting 
cold trip points.

Patch
diff mbox series

diff --git a/drivers/thermal/gov_bang_bang.c b/drivers/thermal/gov_bang_bang.c
index 991a1c54296d..8324d13de1e7 100644
--- a/drivers/thermal/gov_bang_bang.c
+++ b/drivers/thermal/gov_bang_bang.c
@@ -99,6 +99,14 @@  static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
 static int bang_bang_control(struct thermal_zone_device *tz, int trip)
 {
 	struct thermal_instance *instance;
+	enum thermal_trip_type trip_type;
+
+	/* Return doing nothing in case of cold trip point */
+	if (trip != THERMAL_TRIPS_NONE) {
+		tz->ops->get_trip_type(tz, trip, &trip_type);
+		if (trip_type == THERMAL_TRIP_COLD)
+			return 0;
+	}
 
 	thermal_zone_trip_update(tz, trip);
 
diff --git a/drivers/thermal/gov_fair_share.c b/drivers/thermal/gov_fair_share.c
index aaa07180ab48..c0adce525faa 100644
--- a/drivers/thermal/gov_fair_share.c
+++ b/drivers/thermal/gov_fair_share.c
@@ -81,6 +81,14 @@  static int fair_share_throttle(struct thermal_zone_device *tz, int trip)
 	int total_weight = 0;
 	int total_instance = 0;
 	int cur_trip_level = get_trip_level(tz);
+	enum thermal_trip_type trip_type;
+
+	/* Return doing nothing in case of cold trip point */
+	if (trip != THERMAL_TRIPS_NONE) {
+		tz->ops->get_trip_type(tz, trip, &trip_type);
+		if (trip_type == THERMAL_TRIP_COLD)
+			return 0;
+	}
 
 	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
 		if (instance->trip != trip)
diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index 44636475b2a3..2644ad4d4032 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -613,8 +613,16 @@  static int power_allocator_throttle(struct thermal_zone_device *tz, int trip)
 {
 	int ret;
 	int switch_on_temp, control_temp;
+	enum thermal_trip_type trip_type;
 	struct power_allocator_params *params = tz->governor_data;
 
+	/* Return doing nothing in case of cold trip point */
+	if (trip != THERMAL_TRIPS_NONE) {
+		tz->ops->get_trip_type(tz, trip, &trip_type);
+		if (trip_type == THERMAL_TRIP_COLD)
+			return 0;
+	}
+
 	/*
 	 * We get called for every trip point but we only need to do
 	 * our calculations once
diff --git a/drivers/thermal/gov_step_wise.c b/drivers/thermal/gov_step_wise.c
index 2ae7198d3067..009aefda0441 100644
--- a/drivers/thermal/gov_step_wise.c
+++ b/drivers/thermal/gov_step_wise.c
@@ -186,6 +186,14 @@  static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
 static int step_wise_throttle(struct thermal_zone_device *tz, int trip)
 {
 	struct thermal_instance *instance;
+	enum thermal_trip_type trip_type;
+
+	/* For now, return doing nothing in case of cold trip point */
+	if (trip != THERMAL_TRIPS_NONE) {
+		tz->ops->get_trip_type(tz, trip, &trip_type);
+		if (trip_type == THERMAL_TRIP_COLD)
+			return 0;
+	}
 
 	thermal_zone_trip_update(tz, trip);