[PATCH v3 0/4] add sysfs_notify on some attributes

[PATCH v3 0/4] add sysfs_notify on some attributes

[Srikar Srimath Tirumala]

This patchset adds support to send certain thermal notifications and
facilitate workload throttling in userspace. Userspace apps that
want to reduce workload when thermal throttling starts, will
have to register for these events and interpret them as per their
requirement. This workload throttling allows userspace to react before
any hardware clocks are throttled and can keep some critical apps
running reliably longer.

v3
 * Reduced the number of sysfs_notify calls, Notify once per trip only
   when the temperature crosses the trip point.
 * Use hysteresis to avoid sending too many notifications when
   temperatures are bouncing around.
 * Add trip state tracking.
 * Add support to disable trip point notifications.

v2
 * Call sysfs_notify for thermal_zone*/temp only when a trip has
   violated.

v1
 * Base version, added two sysfs_notify calls.

Srikar Srimath Tirumala (4):
  thermal: send notifications for cdev state change
  thermal: send notifications for tz temperature
  thermal: of: enable temperature notifications
  thermal: update documentation for new device ops

 Documentation/thermal/sysfs-api.txt |  5 +++++
 drivers/thermal/of-thermal.c        | 43 +++++++++++++++++++++++++++++++++++++
 drivers/thermal/thermal_core.c      | 29 +++++++++++++++++++++++++
 include/linux/thermal.h             | 12 +++++++++++
 4 files changed, 89 insertions(+)

-- 
2.1.4

[PATCH v3 2/4] thermal: send notifications for tz temperature

[Srikar Srimath Tirumala]

* Update the 'temp' attr of a thermal_zone sysfs node, when the
  temperature goes above\below a trip point. Use sysfs_notify to send
  the notifications.

* Send the notification only if the temperature notifications are
  enabled. Use the hysteresis property to avoid sending too many
  notifications when the temperatures are bouncing around.

* Add ops that will allow thermal framework to:
  * find out if a temperature notification can be sent when a given
    trip point has been crossed.
  * get/set the trip state.

* Add a new state type for the trip points. A trip point can either
  be 'TRIPPED' or 'NOT_TRIPPED'.

Change-Id: If5b530b4045cdb2840eef855dc158cbce239ce79
Signed-off-by: Srikar Srimath Tirumala <srikars@nvidia.com>
---
 drivers/thermal/thermal_core.c | 28 ++++++++++++++++++++++++++++
 include/linux/thermal.h        | 12 ++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index b09fff1..37bdc32 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -419,6 +419,32 @@ static void monitor_thermal_zone(struct thermal_zone_device *tz)
 	mutex_unlock(&tz->lock);
 }
 
+static void handle_temp_notify(struct thermal_zone_device *tz, int trip)
+{
+	int trip_temp, hyst = 0;
+	enum thermal_trip_state s, ns;
+
+	s = ns = THERMAL_TRIP_NOT_TRIPPED;
+	tz->ops->get_trip_temp(tz, trip, &trip_temp);
+	tz->ops->get_trip_hyst(tz, trip, &hyst);
+
+	if (tz->ops->get_trip_state)
+		tz->ops->get_trip_state(tz, trip, &s);
+
+	if (s == THERMAL_TRIP_TRIPPED && tz->temperature < trip_temp - hyst)
+		ns = THERMAL_TRIP_NOT_TRIPPED;
+	else if (s == THERMAL_TRIP_NOT_TRIPPED && tz->temperature >= trip_temp)
+		ns = THERMAL_TRIP_TRIPPED;
+
+	if (tz->ops->set_trip_state)
+		tz->ops->set_trip_state(tz, trip, ns);
+
+	/* send if notifications are enabled and state has changed */
+	if (s != ns && tz->ops->enb_temp_notify &&
+		tz->ops->enb_temp_notify(tz, trip))
+		sysfs_notify(&tz->device.kobj, NULL, "temp");
+}
+
 static void handle_non_critical_trips(struct thermal_zone_device *tz,
 			int trip, enum thermal_trip_type trip_type)
 {
@@ -464,6 +490,8 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
 		handle_critical_trips(tz, trip, type);
 	else
 		handle_non_critical_trips(tz, trip, type);
+
+	handle_temp_notify(tz, trip);
 	/*
 	 * Alright, we handled this trip successfully.
 	 * So, start monitoring again.
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index a55d052..9182e36 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -84,6 +84,11 @@ enum thermal_trip_type {
 	THERMAL_TRIP_CRITICAL,
 };
 
+enum thermal_trip_state {
+	THERMAL_TRIP_NOT_TRIPPED = 0,
+	THERMAL_TRIP_TRIPPED
+};
+
 enum thermal_trend {
 	THERMAL_TREND_STABLE, /* temperature is stable */
 	THERMAL_TREND_RAISING, /* temperature is raising */
@@ -114,6 +119,11 @@ struct thermal_zone_device_ops {
 			  enum thermal_trend *);
 	int (*notify) (struct thermal_zone_device *, int,
 		       enum thermal_trip_type);
+	int (*get_trip_state)(struct thermal_zone_device *, int,
+			enum thermal_trip_state *);
+	int (*set_trip_state)(struct thermal_zone_device *, int,
+			enum thermal_trip_state);
+	bool (*enb_temp_notify)(struct thermal_zone_device *, int);
 };
 
 struct thermal_cooling_device_ops {
@@ -348,6 +358,7 @@ struct thermal_zone_of_device_ops {
  * @temperature: temperature value in miliCelsius
  * @hysteresis: relative hysteresis in miliCelsius
  * @type: trip point type
+ * @state: trip point state
  */
 
 struct thermal_trip {
@@ -355,6 +366,7 @@ struct thermal_trip {
 	unsigned long int temperature;
 	unsigned long int hysteresis;
 	enum thermal_trip_type type;
+	enum thermal_trip_state state;
 };
 
 /* Function declarations */
-- 
2.1.4

Re: [PATCH v3 2/4] thermal: send notifications for tz temperature

[Matt Longnecker]

On 03/29/2016 05:41 PM, Srikar Srimath Tirumala wrote:
> * Update the 'temp' attr of a thermal_zone sysfs node, when the
>    temperature goes above\below a trip point. Use sysfs_notify to send
>    the notifications.
Rather than notifying temp, would it make sense to notify 
trip_pointX_temp when temperature crosses the trip point going up and 
trip_pointX_hyst when temperature crosses the trip point going down? 
That would allow userspace to watch for only a subset of events by 
polling/selecting on the relevant nodes.

>   
>   struct thermal_cooling_device_ops {
> @@ -348,6 +358,7 @@ struct thermal_zone_of_device_ops {
>    * @temperature: temperature value in miliCelsius
>    * @hysteresis: relative hysteresis in miliCelsius
>    * @type: trip point type
> + * @state: trip point state
>    */
>   
>   struct thermal_trip {
> @@ -355,6 +366,7 @@ struct thermal_trip {
>   	unsigned long int temperature;
>   	unsigned long int hysteresis;
>   	enum thermal_trip_type type;
> +	enum thermal_trip_state state;
This looks vestigial. Remove it?

Re: [PATCH v3 2/4] thermal: send notifications for tz temperature

[Matt Longnecker]

On 03/30/2016 09:43 AM, Matt Longnecker wrote:
>
>
> On 03/29/2016 05:41 PM, Srikar Srimath Tirumala wrote:
>>     struct thermal_trip {
>> @@ -355,6 +366,7 @@ struct thermal_trip {
>>       unsigned long int temperature;
>>       unsigned long int hysteresis;
>>       enum thermal_trip_type type;
>> +    enum thermal_trip_state state;
> This looks vestigial. Remove it?
>
>
nevermind. I see that it's used in the next patch. And it really does 
belong here, not in the next patch.

[PATCH v3 1/4] thermal: send notifications for cdev state change

[Srikar Srimath Tirumala]

Send notifications using sysfs_notify to indicate that a cooling
device cur state has changed. This can alert the listeners that
hardware clocks are about to be throttled.

A listening user space application can use this notification to
perform workload throttling.

Change-Id: I71cd6d11bff87f181fa3b7db36bf6c953d1d77eb
Signed-off-by: Srikar Srimath Tirumala <srikars-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/thermal/thermal_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index d4b5465..b09fff1 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1642,6 +1642,7 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
 	cdev->updated = true;
 	trace_cdev_update(cdev, target);
 	dev_dbg(&cdev->device, "set to state %lu\n", target);
+	sysfs_notify(&cdev->device.kobj, NULL, "cur_state");
 }
 EXPORT_SYMBOL(thermal_cdev_update);
 
-- 
2.1.4

Re: [PATCH v3 1/4] thermal: send notifications for cdev state change

[Matt Longnecker]

Looks good to me.

The only platforms I've ever seen with substantial numbers of cooling 
devices whose states change relatively frequently are NVIDIA's. So I 
don't think this will be a performance concern.

-Matt Longnecker

On 03/29/2016 05:41 PM, Srikar Srimath Tirumala wrote:
> Send notifications using sysfs_notify to indicate that a cooling
> device cur state has changed. This can alert the listeners that
> hardware clocks are about to be throttled.
>
> A listening user space application can use this notification to
> perform workload throttling.
>
> Change-Id: I71cd6d11bff87f181fa3b7db36bf6c953d1d77eb
> Signed-off-by: Srikar Srimath Tirumala <srikars-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>   drivers/thermal/thermal_core.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index d4b5465..b09fff1 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1642,6 +1642,7 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
>   	cdev->updated = true;
>   	trace_cdev_update(cdev, target);
>   	dev_dbg(&cdev->device, "set to state %lu\n", target);
> +	sysfs_notify(&cdev->device.kobj, NULL, "cur_state");
>   }
>   EXPORT_SYMBOL(thermal_cdev_update);
>   

[PATCH v3 3/4] thermal: of: enable temperature notifications

[Srikar Srimath Tirumala]

* Add a state variable to track if trip point is triggered.
* Enable trip events only when the state of the tirp point changes.
* Implement the get\set callbacks for trip state.

Change-Id: I1bd6a7b0e5e520d8ee678b83111d23cada7a580c
Signed-off-by: Srikar Srimath Tirumala <srikars-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/thermal/of-thermal.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 49ac23d..f6d9d3d 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -379,6 +379,43 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
 	return -EINVAL;
 }
 
+static bool of_thermal_enb_temp_notify(struct thermal_zone_device *tz, int trip)
+{
+	bool ret = true;
+	struct __thermal_zone *data = tz->devdata;
+
+	if (trip >= data->ntrips || trip < 0)
+		ret = false;
+
+	return ret;
+}
+
+static int of_thermal_get_trip_state(struct thermal_zone_device *tz, int trip,
+				     enum thermal_trip_state *state)
+{
+	struct __thermal_zone *data = tz->devdata;
+
+	if (trip >= data->ntrips || trip < 0)
+		return -EDOM;
+
+	*state = data->trips[trip].state;
+
+	return 0;
+}
+
+static int of_thermal_set_trip_state(struct thermal_zone_device *tz, int trip,
+				     enum thermal_trip_state state)
+{
+	struct __thermal_zone *data = tz->devdata;
+
+	if (trip >= data->ntrips || trip < 0)
+		return -EDOM;
+
+	data->trips[trip].state = state;
+
+	return 0;
+}
+
 static struct thermal_zone_device_ops of_thermal_ops = {
 	.get_mode = of_thermal_get_mode,
 	.set_mode = of_thermal_set_mode,
@@ -392,6 +429,10 @@ static struct thermal_zone_device_ops of_thermal_ops = {
 
 	.bind = of_thermal_bind,
 	.unbind = of_thermal_unbind,
+
+	.get_trip_state = of_thermal_get_trip_state,
+	.set_trip_state = of_thermal_set_trip_state,
+	.enb_temp_notify = of_thermal_enb_temp_notify,
 };
 
 /***   sensor API   ***/
@@ -782,6 +823,8 @@ static int thermal_of_populate_trip(struct device_node *np,
 		return ret;
 	}
 
+	trip->state = THERMAL_TRIP_NOT_TRIPPED;
+
 	/* Required for cooling map matching */
 	trip->np = np;
 	of_node_get(np);
-- 
2.1.4

Re: [PATCH v3 3/4] thermal: of: enable temperature notifications

[Matt Longnecker]

Looks good to me.

On 03/29/2016 05:41 PM, Srikar Srimath Tirumala wrote:
> * Add a state variable to track if trip point is triggered.
> * Enable trip events only when the state of the tirp point changes.
> * Implement the get\set callbacks for trip state.
>
> Change-Id: I1bd6a7b0e5e520d8ee678b83111d23cada7a580c
> Signed-off-by: Srikar Srimath Tirumala <srikars@nvidia.com>
> ---
>   drivers/thermal/of-thermal.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 43 insertions(+)
>
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index 49ac23d..f6d9d3d 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -379,6 +379,43 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
>   	return -EINVAL;
>   }
>   
> +static bool of_thermal_enb_temp_notify(struct thermal_zone_device *tz, int trip)
> +{
> +	bool ret = true;
> +	struct __thermal_zone *data = tz->devdata;
> +
> +	if (trip >= data->ntrips || trip < 0)
> +		ret = false;
> +
> +	return ret;
> +}
> +
> +static int of_thermal_get_trip_state(struct thermal_zone_device *tz, int trip,
> +				     enum thermal_trip_state *state)
> +{
> +	struct __thermal_zone *data = tz->devdata;
> +
> +	if (trip >= data->ntrips || trip < 0)
> +		return -EDOM;
> +
> +	*state = data->trips[trip].state;
> +
> +	return 0;
> +}
> +
> +static int of_thermal_set_trip_state(struct thermal_zone_device *tz, int trip,
> +				     enum thermal_trip_state state)
> +{
> +	struct __thermal_zone *data = tz->devdata;
> +
> +	if (trip >= data->ntrips || trip < 0)
> +		return -EDOM;
> +
> +	data->trips[trip].state = state;
> +
> +	return 0;
> +}
> +
>   static struct thermal_zone_device_ops of_thermal_ops = {
>   	.get_mode = of_thermal_get_mode,
>   	.set_mode = of_thermal_set_mode,
> @@ -392,6 +429,10 @@ static struct thermal_zone_device_ops of_thermal_ops = {
>   
>   	.bind = of_thermal_bind,
>   	.unbind = of_thermal_unbind,
> +
> +	.get_trip_state = of_thermal_get_trip_state,
> +	.set_trip_state = of_thermal_set_trip_state,
> +	.enb_temp_notify = of_thermal_enb_temp_notify,
>   };
>   
>   /***   sensor API   ***/
> @@ -782,6 +823,8 @@ static int thermal_of_populate_trip(struct device_node *np,
>   		return ret;
>   	}
>   
> +	trip->state = THERMAL_TRIP_NOT_TRIPPED;
> +
>   	/* Required for cooling map matching */
>   	trip->np = np;
>   	of_node_get(np);

Re: [PATCH v3 3/4] thermal: of: enable temperature notifications

[Eduardo Valentin]

On Tue, Mar 29, 2016 at 05:41:36PM -0700, Srikar Srimath Tirumala wrote:
> * Add a state variable to track if trip point is triggered.
> * Enable trip events only when the state of the tirp point changes.
> * Implement the get\set callbacks for trip state.
> 
> Change-Id: I1bd6a7b0e5e520d8ee678b83111d23cada7a580c
> Signed-off-by: Srikar Srimath Tirumala <srikars-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/thermal/of-thermal.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index 49ac23d..f6d9d3d 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -379,6 +379,43 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
>  	return -EINVAL;
>  }
>  
> +static bool of_thermal_enb_temp_notify(struct thermal_zone_device *tz, int trip)
> +{
> +	bool ret = true;
> +	struct __thermal_zone *data = tz->devdata;
> +
> +	if (trip >= data->ntrips || trip < 0)
> +		ret = false;
> +
> +	return ret;
> +}
> +
> +static int of_thermal_get_trip_state(struct thermal_zone_device *tz, int trip,
> +				     enum thermal_trip_state *state)
> +{
> +	struct __thermal_zone *data = tz->devdata;
> +
> +	if (trip >= data->ntrips || trip < 0)
> +		return -EDOM;
> +
> +	*state = data->trips[trip].state;

For the sake of the current implementation, I believe tracking state is probably
more of a thermal core duty. Not sure this has anything to do with OF
thermal.


Could you please clarify why you think this has to be here?

> +
> +	return 0;
> +}
> +
> +static int of_thermal_set_trip_state(struct thermal_zone_device *tz, int trip,
> +				     enum thermal_trip_state state)
> +{
> +	struct __thermal_zone *data = tz->devdata;
> +
> +	if (trip >= data->ntrips || trip < 0)
> +		return -EDOM;
> +
> +	data->trips[trip].state = state;
> +
> +	return 0;
> +}
> +
>  static struct thermal_zone_device_ops of_thermal_ops = {
>  	.get_mode = of_thermal_get_mode,
>  	.set_mode = of_thermal_set_mode,
> @@ -392,6 +429,10 @@ static struct thermal_zone_device_ops of_thermal_ops = {
>  
>  	.bind = of_thermal_bind,
>  	.unbind = of_thermal_unbind,
> +
> +	.get_trip_state = of_thermal_get_trip_state,
> +	.set_trip_state = of_thermal_set_trip_state,
> +	.enb_temp_notify = of_thermal_enb_temp_notify,
>  };
>  
>  /***   sensor API   ***/
> @@ -782,6 +823,8 @@ static int thermal_of_populate_trip(struct device_node *np,
>  		return ret;
>  	}
>  
> +	trip->state = THERMAL_TRIP_NOT_TRIPPED;
> +
>  	/* Required for cooling map matching */
>  	trip->np = np;
>  	of_node_get(np);
> -- 
> 2.1.4
> 

[PATCH v3 4/4] thermal: update documentation for new device ops

[Srikar Srimath Tirumala]

Add documentation for two new thermal_zone_device_ops.

Change-Id: Id1c5cc2ae4051a398dfe81c7ff0b27c1eab02579
Signed-off-by: Srikar Srimath Tirumala <srikars-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 Documentation/thermal/sysfs-api.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index ed419d6..13344167 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -59,6 +59,11 @@ temperature) and throttle appropriate devices.
 			will be fired.
 	.set_emul_temp: set the emulation temperature which helps in debugging
 			different threshold temperature points.
+	.get_trip_state: get the current state (TRIPPED or NOT_TRIPPED) of a
+		certain trip point.
+	.set_trip_state: set the state of a certain trip point.
+	.enb_temp_notify: check if temperature notifications are enabled for a
+		certain trip point.
     tzp: thermal zone platform parameters.
     passive_delay: number of milliseconds to wait between polls when
 	performing passive cooling.
-- 
2.1.4

Re: [PATCH v3 4/4] thermal: update documentation for new device ops

[Matt Longnecker]

Looks good to me.

On 03/29/2016 05:41 PM, Srikar Srimath Tirumala wrote:
> different threshold temperature points.
> +	.get_trip_state: get the current state (TRIPPED or NOT_TRIPPED) of a
> +		certain trip point.
> +	.set_trip_state: set the state of a certain trip point.
> +	.enb_temp_notify: check if temperature notifications are enabled for a
> +		certain trip point.
>       tzp: thermal zone platform parameters.
>       passive_delay: number of milliseconds to wait between polls when
>   	performing passive cooling.

Re: [PATCH v3 4/4] thermal: update documentation for new device ops

[Eduardo Valentin]

On Tue, Mar 29, 2016 at 05:41:37PM -0700, Srikar Srimath Tirumala wrote:
> Add documentation for two new thermal_zone_device_ops.
> 
> Change-Id: Id1c5cc2ae4051a398dfe81c7ff0b27c1eab02579
> Signed-off-by: Srikar Srimath Tirumala <srikars@nvidia.com>
> ---
>  Documentation/thermal/sysfs-api.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index ed419d6..13344167 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -59,6 +59,11 @@ temperature) and throttle appropriate devices.
>  			will be fired.
>  	.set_emul_temp: set the emulation temperature which helps in debugging
>  			different threshold temperature points.
> +	.get_trip_state: get the current state (TRIPPED or NOT_TRIPPED) of a
> +		certain trip point.
> +	.set_trip_state: set the state of a certain trip point.
> +	.enb_temp_notify: check if temperature notifications are enabled for a
> +		certain trip point.

I would prefer we merge this to the patch which is adding them.

>      tzp: thermal zone platform parameters.
>      passive_delay: number of milliseconds to wait between polls when
>  	performing passive cooling.
> -- 
> 2.1.4
>