All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch net-next RFC v1] mlxsw: core: Add the hottest thermal zone detection
@ 2019-05-29 13:52 Vadim Pasternak
  2022-06-15 20:31 ` Daniel Lezcano
  0 siblings, 1 reply; 5+ messages in thread
From: Vadim Pasternak @ 2019-05-29 13:52 UTC (permalink / raw)
  To: davem
  Cc: netdev, linux, rui.zhang, edubezval, daniel.lezcano, jiri,
	idosch, Vadim Pasternak

When multiple sensors are mapped to the same cooling device, the
cooling device should be set according the worst sensor from the
sensors associated with this cooling device.

Provide the hottest thermal zone detection and enforce cooling device to
follow the temperature trends the hottest zone only.
Prevent competition for the cooling device control from others zones, by
"stable trend" indication. A cooling device will not perform any
actions associated with a zone with "stable trend".

When other thermal zone is detected as a hottest, a cooling device is to
be switched to following temperature trends of new hottest zone.

Thermal zone score is represented by 32 bits unsigned integer and
calculated according to the next formula:
For T < TZ<t><i>, where t from {normal trip = 0, high trip = 1, hot
trip = 2, critical = 3}:
TZ<i> score = (T + (TZ<t><i> - T) / 2) / (TZ<t><i> - T) * 256 ** j;
Highest thermal zone score s is set as MAX(TZ<i>score);
Following this formula, if TZ<i> is in trip point higher than TZ<k>,
the higher score is to be always assigned to TZ<i>.

For two thermal zones located at the same kind of trip point, the higher
score will be assigned to the zone, which closer to the next trip point.
Thus, the highest score will always be assigned objectively to the hottest
thermal zone.

Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core_thermal.c | 64 +++++++++++++++++++---
 1 file changed, 57 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
index cfab0e330a47..2491ff62b783 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -23,6 +23,7 @@
 #define MLXSW_THERMAL_HYSTERESIS_TEMP	5000	/* 5C */
 #define MLXSW_THERMAL_MODULE_TEMP_SHIFT	(MLXSW_THERMAL_HYSTERESIS_TEMP * 2)
 #define MLXSW_THERMAL_ZONE_MAX_NAME	16
+#define MLXSW_THERMAL_TEMP_SCORE_MAX	0xffffffff
 #define MLXSW_THERMAL_MAX_STATE	10
 #define MLXSW_THERMAL_MAX_DUTY	255
 /* Minimum and maximum fan allowed speed in percent: from 20% to 100%. Values
@@ -111,6 +112,8 @@ struct mlxsw_thermal {
 	struct mlxsw_thermal_trip trips[MLXSW_THERMAL_NUM_TRIPS];
 	enum thermal_device_mode mode;
 	struct mlxsw_thermal_module *tz_module_arr;
+	unsigned int tz_highest_score;
+	struct thermal_zone_device *tz_highest_dev;
 };
 
 static inline u8 mlxsw_state_to_duty(int state)
@@ -195,6 +198,34 @@ mlxsw_thermal_module_trips_update(struct device *dev, struct mlxsw_core *core,
 	return 0;
 }
 
+static void mlxsw_thermal_tz_score_update(struct mlxsw_thermal *thermal,
+					  struct thermal_zone_device *tzdev,
+					  struct mlxsw_thermal_trip *trips,
+					  int temp)
+{
+	struct mlxsw_thermal_trip *trip = trips;
+	unsigned int score, delta, i, shift = 1;
+
+	/* Calculate thermal zone score, if temperature is above the critical
+	 * threshold score is set to MLXSW_THERMAL_TEMP_SCORE_MAX.
+	 */
+	score = MLXSW_THERMAL_TEMP_SCORE_MAX;
+	for (i = MLXSW_THERMAL_TEMP_TRIP_NORM; i < MLXSW_THERMAL_NUM_TRIPS;
+	     i++, trip++) {
+		if (temp < trip->temp) {
+			delta = DIV_ROUND_CLOSEST(temp, trip->temp - temp);
+			score = delta * shift;
+			break;
+		}
+		shift *= 256;
+	}
+
+	if (score > thermal->tz_highest_score) {
+		thermal->tz_highest_score = score;
+		thermal->tz_highest_dev = tzdev;
+	}
+}
+
 static int mlxsw_thermal_bind(struct thermal_zone_device *tzdev,
 			      struct thermal_cooling_device *cdev)
 {
@@ -290,6 +321,9 @@ static int mlxsw_thermal_get_temp(struct thermal_zone_device *tzdev,
 		return err;
 	}
 	mlxsw_reg_mtmp_unpack(mtmp_pl, &temp, NULL, NULL);
+	if (temp)
+		mlxsw_thermal_tz_score_update(thermal, tzdev, thermal->trips,
+					      temp);
 
 	*p_temp = (int) temp;
 	return 0;
@@ -351,6 +385,22 @@ static int mlxsw_thermal_set_trip_hyst(struct thermal_zone_device *tzdev,
 	return 0;
 }
 
+static int mlxsw_thermal_trend_get(struct thermal_zone_device *tzdev,
+				   int trip, enum thermal_trend *trend)
+{
+	struct mlxsw_thermal_module *tz = tzdev->devdata;
+	struct mlxsw_thermal *thermal = tz->parent;
+
+	if (trip < 0 || trip >= MLXSW_THERMAL_NUM_TRIPS)
+		return -EINVAL;
+
+	if (tzdev == thermal->tz_highest_dev)
+		return 1;
+
+	*trend = THERMAL_TREND_STABLE;
+	return 0;
+}
+
 static struct thermal_zone_device_ops mlxsw_thermal_ops = {
 	.bind = mlxsw_thermal_bind,
 	.unbind = mlxsw_thermal_unbind,
@@ -362,6 +412,7 @@ static struct thermal_zone_device_ops mlxsw_thermal_ops = {
 	.set_trip_temp	= mlxsw_thermal_set_trip_temp,
 	.get_trip_hyst	= mlxsw_thermal_get_trip_hyst,
 	.set_trip_hyst	= mlxsw_thermal_set_trip_hyst,
+	.get_trend	= mlxsw_thermal_trend_get,
 };
 
 static int mlxsw_thermal_module_bind(struct thermal_zone_device *tzdev,
@@ -472,7 +523,9 @@ static int mlxsw_thermal_module_temp_get(struct thermal_zone_device *tzdev,
 		return 0;
 
 	/* Update trip points. */
-	mlxsw_thermal_module_trips_update(dev, thermal->core, tz);
+	err = mlxsw_thermal_module_trips_update(dev, thermal->core, tz);
+	if (!err)
+		mlxsw_thermal_tz_score_update(thermal, tzdev, tz->trips, temp);
 
 	return 0;
 }
@@ -537,10 +590,6 @@ mlxsw_thermal_module_trip_hyst_set(struct thermal_zone_device *tzdev, int trip,
 	return 0;
 }
 
-static struct thermal_zone_params mlxsw_thermal_module_params = {
-	.governor_name = "user_space",
-};
-
 static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
 	.bind		= mlxsw_thermal_module_bind,
 	.unbind		= mlxsw_thermal_module_unbind,
@@ -552,6 +601,7 @@ static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
 	.set_trip_temp	= mlxsw_thermal_module_trip_temp_set,
 	.get_trip_hyst	= mlxsw_thermal_module_trip_hyst_get,
 	.set_trip_hyst	= mlxsw_thermal_module_trip_hyst_set,
+	.get_trend	= mlxsw_thermal_trend_get,
 };
 
 static int mlxsw_thermal_get_max_state(struct thermal_cooling_device *cdev,
@@ -667,13 +717,13 @@ mlxsw_thermal_module_tz_init(struct mlxsw_thermal_module *module_tz)
 							MLXSW_THERMAL_TRIP_MASK,
 							module_tz,
 							&mlxsw_thermal_module_ops,
-							&mlxsw_thermal_module_params,
-							0, 0);
+							NULL, 0, 0);
 	if (IS_ERR(module_tz->tzdev)) {
 		err = PTR_ERR(module_tz->tzdev);
 		return err;
 	}
 
+	module_tz->mode = THERMAL_DEVICE_ENABLED;
 	return 0;
 }
 
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [patch net-next RFC v1] mlxsw: core: Add the hottest thermal zone detection
  2019-05-29 13:52 [patch net-next RFC v1] mlxsw: core: Add the hottest thermal zone detection Vadim Pasternak
@ 2022-06-15 20:31 ` Daniel Lezcano
  2022-06-15 22:06   ` Vadim Pasternak
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Lezcano @ 2022-06-15 20:31 UTC (permalink / raw)
  To: Vadim Pasternak, davem; +Cc: netdev, linux, rui.zhang, edubezval, jiri, idosch


Hi Vadim,

On 29/05/2019 15:52, Vadim Pasternak wrote:
> When multiple sensors are mapped to the same cooling device, the
> cooling device should be set according the worst sensor from the
> sensors associated with this cooling device.
> 
> Provide the hottest thermal zone detection and enforce cooling device to
> follow the temperature trends the hottest zone only.
> Prevent competition for the cooling device control from others zones, by
> "stable trend" indication. A cooling device will not perform any
> actions associated with a zone with "stable trend".
> 
> When other thermal zone is detected as a hottest, a cooling device is to
> be switched to following temperature trends of new hottest zone.
> 
> Thermal zone score is represented by 32 bits unsigned integer and
> calculated according to the next formula:
> For T < TZ<t><i>, where t from {normal trip = 0, high trip = 1, hot
> trip = 2, critical = 3}:
> TZ<i> score = (T + (TZ<t><i> - T) / 2) / (TZ<t><i> - T) * 256 ** j;
> Highest thermal zone score s is set as MAX(TZ<i>score);
> Following this formula, if TZ<i> is in trip point higher than TZ<k>,
> the higher score is to be always assigned to TZ<i>.
> 
> For two thermal zones located at the same kind of trip point, the higher
> score will be assigned to the zone, which closer to the next trip point.
> Thus, the highest score will always be assigned objectively to the hottest
> thermal zone.

While reading the code I noticed this change and I was wondering why it 
was needed.

The thermal framework does already aggregates the mitigation decisions, 
taking the highest cooling state [1].

That allows for instance a spanning fan on a dual socket. Two thermal 
zones for one cooling device.

AFAICS, the code hijacked the get_trend function just for the sake of 
returning 1 for the hotter thermal zone leading to a computation of the 
trend in the thermal core code.

I would like to get rid of the get_trend ops in the thermal framework 
and the changes in this patch sounds like pointless as the aggregation 
of the cooling action is already handled in the thermal framework.

Given the above, it would make sense to revert commit 6f73862fabd93 and 
2dc2f760052da ?

Thanks

   -- Daniel

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drivers/thermal/thermal_helpers.c#n190


[ ... ]


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [patch net-next RFC v1] mlxsw: core: Add the hottest thermal zone detection
  2022-06-15 20:31 ` Daniel Lezcano
@ 2022-06-15 22:06   ` Vadim Pasternak
  2022-06-15 22:27     ` Daniel Lezcano
  0 siblings, 1 reply; 5+ messages in thread
From: Vadim Pasternak @ 2022-06-15 22:06 UTC (permalink / raw)
  To: Daniel Lezcano, davem
  Cc: netdev, linux, rui.zhang, edubezval, jiri, Ido Schimmel

Hi Daniel,

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: Wednesday, June 15, 2022 11:32 PM
> To: Vadim Pasternak <vadimp@nvidia.com>; davem@davemloft.net
> Cc: netdev@vger.kernel.org; linux@roeck-us.net; rui.zhang@intel.com;
> edubezval@gmail.com; jiri@resnulli.us; Ido Schimmel <idosch@nvidia.com>
> Subject: Re: [patch net-next RFC v1] mlxsw: core: Add the hottest thermal
> zone detection
> 
> 
> Hi Vadim,
> 
> On 29/05/2019 15:52, Vadim Pasternak wrote:
> > When multiple sensors are mapped to the same cooling device, the
> > cooling device should be set according the worst sensor from the
> > sensors associated with this cooling device.
> >
> > Provide the hottest thermal zone detection and enforce cooling device
> > to follow the temperature trends the hottest zone only.
> > Prevent competition for the cooling device control from others zones,
> > by "stable trend" indication. A cooling device will not perform any
> > actions associated with a zone with "stable trend".
> >
> > When other thermal zone is detected as a hottest, a cooling device is
> > to be switched to following temperature trends of new hottest zone.
> >
> > Thermal zone score is represented by 32 bits unsigned integer and
> > calculated according to the next formula:
> > For T < TZ<t><i>, where t from {normal trip = 0, high trip = 1, hot
> > trip = 2, critical = 3}:
> > TZ<i> score = (T + (TZ<t><i> - T) / 2) / (TZ<t><i> - T) * 256 ** j;
> > Highest thermal zone score s is set as MAX(TZ<i>score); Following this
> > formula, if TZ<i> is in trip point higher than TZ<k>, the higher score
> > is to be always assigned to TZ<i>.
> >
> > For two thermal zones located at the same kind of trip point, the
> > higher score will be assigned to the zone, which closer to the next trip
> point.
> > Thus, the highest score will always be assigned objectively to the
> > hottest thermal zone.
> 
> While reading the code I noticed this change and I was wondering why it was
> needed.
> 
> The thermal framework does already aggregates the mitigation decisions,
> taking the highest cooling state [1].
> 
> That allows for instance a spanning fan on a dual socket. Two thermal zones
> for one cooling device.

Here the hottest thermal zone is calculated for different thermal zone_devices, for example, each
optical transceiver or gearbox is separated 'tzdev', while all of them share the same cooling device.
It could up to 128 transceivers.

It was also intention to avoid a competition between thermal zones when some of them
can be in trend up state and some  in trend down.

Are you saying that the below code will work for such case?

	/* Make sure cdev enters the deepest cooling state */
	list_for_each_entry(instance, &cdev->thermal_instances, cdev_node) {
		dev_dbg(&cdev->device, "zone%d->target=%lu\n",
			instance->tz->id, instance->target);
		if (instance->target == THERMAL_NO_TARGET)
			continue;
		if (instance->target > target)
			target = instance->target;
	}

> 
> AFAICS, the code hijacked the get_trend function just for the sake of
> returning 1 for the hotter thermal zone leading to a computation of the trend
> in the thermal core code.

Yes, get_trend() returns one just to indicate that cooling device should not be
touched for a thermal zone, which is not hottest.

> 
> I would like to get rid of the get_trend ops in the thermal framework and the
> changes in this patch sounds like pointless as the aggregation of the cooling
> action is already handled in the thermal framework.
> 
> Given the above, it would make sense to revert commit 6f73862fabd93 and
> 2dc2f760052da ?

I believe we should run thermal emulation to validate we are OK.

Thanks,
Vadim.

> 
> Thanks
> 
>    -- Daniel
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drive
> rs/thermal/thermal_helpers.c#n190
> 
> 
> [ ... ]
> 
> 
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch net-next RFC v1] mlxsw: core: Add the hottest thermal zone detection
  2022-06-15 22:06   ` Vadim Pasternak
@ 2022-06-15 22:27     ` Daniel Lezcano
  2022-06-16  3:41       ` [EXTERNAL][patch " Eduardo Valentin
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Lezcano @ 2022-06-15 22:27 UTC (permalink / raw)
  To: Vadim Pasternak, davem
  Cc: netdev, linux, rui.zhang, edubezval, jiri, Ido Schimmel


Hi Vadim,

On 16/06/2022 00:06, Vadim Pasternak wrote:
> Hi Daniel,
> 
>> -----Original Message-----
>> From: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Sent: Wednesday, June 15, 2022 11:32 PM
>> To: Vadim Pasternak <vadimp@nvidia.com>; davem@davemloft.net
>> Cc: netdev@vger.kernel.org; linux@roeck-us.net; rui.zhang@intel.com;
>> edubezval@gmail.com; jiri@resnulli.us; Ido Schimmel <idosch@nvidia.com>
>> Subject: Re: [patch net-next RFC v1] mlxsw: core: Add the hottest thermal
>> zone detection
>>
>>
>> Hi Vadim,
>>
>> On 29/05/2019 15:52, Vadim Pasternak wrote:
>>> When multiple sensors are mapped to the same cooling device, the
>>> cooling device should be set according the worst sensor from the
>>> sensors associated with this cooling device.
>>>
>>> Provide the hottest thermal zone detection and enforce cooling device
>>> to follow the temperature trends the hottest zone only.
>>> Prevent competition for the cooling device control from others zones,
>>> by "stable trend" indication. A cooling device will not perform any
>>> actions associated with a zone with "stable trend".
>>>
>>> When other thermal zone is detected as a hottest, a cooling device is
>>> to be switched to following temperature trends of new hottest zone.
>>>
>>> Thermal zone score is represented by 32 bits unsigned integer and
>>> calculated according to the next formula:
>>> For T < TZ<t><i>, where t from {normal trip = 0, high trip = 1, hot
>>> trip = 2, critical = 3}:
>>> TZ<i> score = (T + (TZ<t><i> - T) / 2) / (TZ<t><i> - T) * 256 ** j;
>>> Highest thermal zone score s is set as MAX(TZ<i>score); Following this
>>> formula, if TZ<i> is in trip point higher than TZ<k>, the higher score
>>> is to be always assigned to TZ<i>.
>>>
>>> For two thermal zones located at the same kind of trip point, the
>>> higher score will be assigned to the zone, which closer to the next trip
>> point.
>>> Thus, the highest score will always be assigned objectively to the
>>> hottest thermal zone.
>>
>> While reading the code I noticed this change and I was wondering why it was
>> needed.
>>
>> The thermal framework does already aggregates the mitigation decisions,
>> taking the highest cooling state [1].
>>
>> That allows for instance a spanning fan on a dual socket. Two thermal zones
>> for one cooling device.
> 
> Here the hottest thermal zone is calculated for different thermal zone_devices, for example, each
> optical transceiver or gearbox is separated 'tzdev', while all of them share the same cooling device.
> It could up to 128 transceivers.
> 
> It was also intention to avoid a competition between thermal zones when some of them
> can be in trend up state and some  in trend down.
> 
> Are you saying that the below code will work for such case?
> 
> 	/* Make sure cdev enters the deepest cooling state */
> 	list_for_each_entry(instance, &cdev->thermal_instances, cdev_node) {
> 		dev_dbg(&cdev->device, "zone%d->target=%lu\n",
> 			instance->tz->id, instance->target);
> 		if (instance->target == THERMAL_NO_TARGET)
> 			continue;
> 		if (instance->target > target)
> 			target = instance->target;
> 	}

Yes, that is my understanding.

For the thermal zones which are under the temperature limit, their 
instance->target will be THERMAL_NO_TARGET and will be discarded by this 
loop.

For the ones being mitigated, the highest cooling state will be used.

The purpose of this loop is exactly for your use case. If it happens it 
does not work as expected, then it is something in the core code to be 
fixed.


>> AFAICS, the code hijacked the get_trend function just for the sake of
>> returning 1 for the hotter thermal zone leading to a computation of the trend
>> in the thermal core code.
> 
> Yes, get_trend() returns one just to indicate that cooling device should not be
> touched for a thermal zone, which is not hottest.
> 
>>
>> I would like to get rid of the get_trend ops in the thermal framework and the
>> changes in this patch sounds like pointless as the aggregation of the cooling
>> action is already handled in the thermal framework.
>>
>> Given the above, it would make sense to revert commit 6f73862fabd93 and
>> 2dc2f760052da ?
> 
> I believe we should run thermal emulation to validate we are OK.

Sure, let me know

Thanks

   -- Daniel

> 
> Thanks,
> Vadim.
> 
>>
>> Thanks
>>
>>     -- Daniel
>>
>> [1]
>> https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drive
>> rs/thermal/thermal_helpers.c#n190
>>
>>
>> [ ... ]
>>
>>
>> --
>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>> <http://twitter.com/#!/linaroorg> Twitter |
>> <http://www.linaro.org/linaro-blog/> Blog


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [EXTERNAL][patch net-next RFC v1] mlxsw: core: Add the hottest thermal zone detection
  2022-06-15 22:27     ` Daniel Lezcano
@ 2022-06-16  3:41       ` Eduardo Valentin
  0 siblings, 0 replies; 5+ messages in thread
From: Eduardo Valentin @ 2022-06-16  3:41 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Vadim Pasternak, davem, netdev, linux, rui.zhang, edubezval,
	jiri, Ido Schimmel

Hey guys,

On Thu, Jun 16, 2022 at 12:27:39AM +0200, Daniel Lezcano wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> Hi Vadim,
> 
> On 16/06/2022 00:06, Vadim Pasternak wrote:
> > Hi Daniel,
> > 
> > > -----Original Message-----
> > > From: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > Sent: Wednesday, June 15, 2022 11:32 PM
> > > To: Vadim Pasternak <vadimp@nvidia.com>; davem@davemloft.net
> > > Cc: netdev@vger.kernel.org; linux@roeck-us.net; rui.zhang@intel.com;
> > > edubezval@gmail.com; jiri@resnulli.us; Ido Schimmel <idosch@nvidia.com>
> > > Subject: Re: [patch net-next RFC v1] mlxsw: core: Add the hottest thermal
> > > zone detection
> > > 
> > > 
> > > Hi Vadim,
> > > 
> > > On 29/05/2019 15:52, Vadim Pasternak wrote:
> > > > When multiple sensors are mapped to the same cooling device, the
> > > > cooling device should be set according the worst sensor from the
> > > > sensors associated with this cooling device.
> > > > 
> > > > Provide the hottest thermal zone detection and enforce cooling device
> > > > to follow the temperature trends the hottest zone only.
> > > > Prevent competition for the cooling device control from others zones,
> > > > by "stable trend" indication. A cooling device will not perform any
> > > > actions associated with a zone with "stable trend".
> > > > 
> > > > When other thermal zone is detected as a hottest, a cooling device is
> > > > to be switched to following temperature trends of new hottest zone.
> > > > 
> > > > Thermal zone score is represented by 32 bits unsigned integer and
> > > > calculated according to the next formula:
> > > > For T < TZ<t><i>, where t from {normal trip = 0, high trip = 1, hot
> > > > trip = 2, critical = 3}:
> > > > TZ<i> score = (T + (TZ<t><i> - T) / 2) / (TZ<t><i> - T) * 256 ** j;
> > > > Highest thermal zone score s is set as MAX(TZ<i>score); Following this
> > > > formula, if TZ<i> is in trip point higher than TZ<k>, the higher score
> > > > is to be always assigned to TZ<i>.
> > > > 
> > > > For two thermal zones located at the same kind of trip point, the
> > > > higher score will be assigned to the zone, which closer to the next trip
> > > point.
> > > > Thus, the highest score will always be assigned objectively to the
> > > > hottest thermal zone.
> > > 
> > > While reading the code I noticed this change and I was wondering why it was
> > > needed.
> > > 
> > > The thermal framework does already aggregates the mitigation decisions,
> > > taking the highest cooling state [1].
> > > 
> > > That allows for instance a spanning fan on a dual socket. Two thermal zones
> > > for one cooling device.
> > 
> > Here the hottest thermal zone is calculated for different thermal zone_devices, for example, each
> > optical transceiver or gearbox is separated 'tzdev', while all of them share the same cooling device.
> > It could up to 128 transceivers.
> > 
> > It was also intention to avoid a competition between thermal zones when some of them
> > can be in trend up state and some  in trend down.
> > 
> > Are you saying that the below code will work for such case?
> > 
> >       /* Make sure cdev enters the deepest cooling state */
> >       list_for_each_entry(instance, &cdev->thermal_instances, cdev_node) {
> >               dev_dbg(&cdev->device, "zone%d->target=%lu\n",
> >                       instance->tz->id, instance->target);
> >               if (instance->target == THERMAL_NO_TARGET)
> >                       continue;
> >               if (instance->target > target)
> >                       target = instance->target;
> >       }
> 
> Yes, that is my understanding.
> 
> For the thermal zones which are under the temperature limit, their
> instance->target will be THERMAL_NO_TARGET and will be discarded by this
> loop.
> 
> For the ones being mitigated, the highest cooling state will be used.
> 
> The purpose of this loop is exactly for your use case. If it happens it
> does not work as expected, then it is something in the core code to be
> fixed.
> 
> 
> > > AFAICS, the code hijacked the get_trend function just for the sake of
> > > returning 1 for the hotter thermal zone leading to a computation of the trend
> > > in the thermal core code.
> > 
> > Yes, get_trend() returns one just to indicate that cooling device should not be
> > touched for a thermal zone, which is not hottest.
> > 
> > > 
> > > I would like to get rid of the get_trend ops in the thermal framework and the
> > > changes in this patch sounds like pointless as the aggregation of the cooling
> > > action is already handled in the thermal framework.
> > > 
> > > Given the above, it would make sense to revert commit 6f73862fabd93 and
> > > 2dc2f760052da ?
> > 
> > I believe we should run thermal emulation to validate we are OK.

I agree with the suggestion that these commit shall be considered for revert. 

The thermal core will do aggregation while handling cooling devices
to set the cooling state to whatever is currently the one requiring more impact
on temperature.

And, yes, please check with thermal emulation.

Side note, please use my evalenti@kernel.org address if you need my attention.
I will keep on monitoring messages sent to that address.

I will be updating my contacts at MAINTAINERS soon.
> 
> Sure, let me know
> 
> Thanks
> 
>   -- Daniel
> 
> > 
> > Thanks,
> > Vadim.
> > 
> > > 
> > > Thanks
> > > 
> > >     -- Daniel
> > > 
> > > [1]
> > > https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drive
> > > rs/thermal/thermal_helpers.c#n190
> > > 
> > > 
> > > [ ... ]
> > > 
> > > 
> > > --
> > > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> > > 
> > > Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> > > <http://twitter.com/#!/linaroorg> Twitter |
> > > <http://www.linaro.org/linaro-blog/> Blog
> 
> 
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog

-- 
All the best,
Eduardo Valentin

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-06-16  3:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29 13:52 [patch net-next RFC v1] mlxsw: core: Add the hottest thermal zone detection Vadim Pasternak
2022-06-15 20:31 ` Daniel Lezcano
2022-06-15 22:06   ` Vadim Pasternak
2022-06-15 22:27     ` Daniel Lezcano
2022-06-16  3:41       ` [EXTERNAL][patch " Eduardo Valentin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.