2020-07-10 13:52:31

by Thara Gopinath

Subject: [RFC PATCH 4/4] thermal: Modify thermal governors to do nothing for "cold" trip points

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 <[email protected]>
---
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(+)

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

--
2.25.1

2020-07-15 09:11:12

by Zhang, Rui

Subject: Re: [RFC PATCH 4/4] thermal: Modify thermal governors to do nothing for "cold" trip points

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 <[email protected]>
> ---
> 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);
>

2020-07-15 23:14:11

by Thara Gopinath

Subject: Re: [RFC PATCH 4/4] thermal: Modify thermal governors to do nothing for "cold" trip points

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 <[email protected]>
>> ---
>> 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.


--
Warm Regards
Thara