[RFC PATCH 0/12] generic thermal layer enhancement

[RFC PATCH 0/12] generic thermal layer enhancement

[Zhang Rui]

Hi, all,

this patch set is made to enhance the current generic thermal layer.
It fixes several gaps discussed in
http://marc.info/?l=linux-acpi&m=133836783425764&w=2
and introduces a simple arbitrator to fix the "one cooling devices
referenced by multiple trip points in multiple thermal zones" problem.
 
The whole idea is
1) make sure we have multiple cooling states for both active cooling and
   passive cooling devices. (patch 1,2,3)
2) remove passive specific requirement, aka, tc1/tc2, and
   introduce .get_trend() instead, for both active and passive cooling
   algorithm. (patch 4,5)
3) introduce new function thermal_zone_trip_update(), this contains the
   code for the general cooling algorithm. (patch 6)
4) rename some thermal structures. Use thermal_instance instead of
   thermal_cooling_device_instance, as this structure is used to
   describe the behavior of a certain cooling device for a certain trip
   point in a certain thermal zone.
   thermal_zone_device has a list of all the thermal instances in this
   thermal zone so that it can update them when the temperature changes.
   thermal_cooling_device has a list of all the thermal instances for
   this cooling device so that it can make decision which cooling state
   should be in when the temperature changes. (patch 7,8,9,10)
5) introduce a simple arbitrator. (patch 11)
   When temperature changes, we update a thermal zone in two stages,
   a) thermal_zone_trip_point() is the general cooling algorithm to
      update the thermal instances in the thermal zone device
   b) thermal_zone_do_update() is the arbitrator for the cooling device
      to choose the deepest cooling state required to enter.
6) unify the code for both passive and active cooling.

TODO, add locking mechanism. I know this is important but as this patch
set changes the thermal layer a lot, I just want to make sure I'm in the
right direction before going on.

BTW, in theory, they should make no change to the behavior of the
current generic thermal layer users. But I have just done the build
test.

Any comments are welcome.

thanks,
rui

--------------------------------------------------------------------
 drivers/acpi/thermal.c                   |   76 +++++++--
 drivers/platform/x86/acerhdf.c           |    4 +-
 drivers/platform/x86/intel_mid_thermal.c |    2 +-
 drivers/thermal/spear_thermal.c          |    2 +-
 drivers/thermal/thermal_sys.c            |  293
++++++++++++++++++------------
 include/linux/thermal.h                  |   22 ++-
 7 files changed, 271 insertions(+), 148 deletions(-)

Re: [RFC PATCH 0/12] generic thermal layer enhancement

[Eduardo Valentin]

Hello Rui,

On Mon, Jun 11, 2012 at 11:19:11AM +0800, Zhang Rui wrote:
> Hi, all,
> 
> this patch set is made to enhance the current generic thermal layer.
> It fixes several gaps discussed in
> http://marc.info/?l=linux-acpi&m=133836783425764&w=2
> and introduces a simple arbitrator to fix the "one cooling devices
> referenced by multiple trip points in multiple thermal zones" problem.

Thanks for taking this further. With code it is much better to progress.

BTW, are you keeping this series somewhere in a branch?

>  
> The whole idea is
> 1) make sure we have multiple cooling states for both active cooling and
>    passive cooling devices. (patch 1,2,3)

Nice!

> 2) remove passive specific requirement, aka, tc1/tc2, and
>    introduce .get_trend() instead, for both active and passive cooling
>    algorithm. (patch 4,5)

Cool. The .get_trend() might be also helpful in case there are sensors
that provide trending computation, or at least, some history buffer.

So, I definitely agree with this approach.

> 3) introduce new function thermal_zone_trip_update(), this contains the
>    code for the general cooling algorithm. (patch 6)

Ok.

> 4) rename some thermal structures. Use thermal_instance instead of
>    thermal_cooling_device_instance, as this structure is used to
>    describe the behavior of a certain cooling device for a certain trip
>    point in a certain thermal zone.
>    thermal_zone_device has a list of all the thermal instances in this
>    thermal zone so that it can update them when the temperature changes.
>    thermal_cooling_device has a list of all the thermal instances for
>    this cooling device so that it can make decision which cooling state
>    should be in when the temperature changes. (patch 7,8,9,10)

Ok.

> 5) introduce a simple arbitrator. (patch 11)
>    When temperature changes, we update a thermal zone in two stages,
>    a) thermal_zone_trip_point() is the general cooling algorithm to
>       update the thermal instances in the thermal zone device
>    b) thermal_zone_do_update() is the arbitrator for the cooling device
>       to choose the deepest cooling state required to enter.

The arbitrator is good starting point. I will have a deeper look on it.

But we may be careful to solve the constraint issue only at thermal code
level. There might be conflicting constraints coming from PM QoS layer,
for instance.

> 6) unify the code for both passive and active cooling.

This is good.

> 
> TODO, add locking mechanism. I know this is important but as this patch
> set changes the thermal layer a(lot, I just want to make sure I'm in the
> right direction before going on.

Right. I second you on the incremental approach.

> 
> BTW, in theory, they should make no change to the behavior of the
> current generic thermal layer users. But I have just done the build
> test.

OK. I will fetch them and give them a trial on my side, then send better review.

> 
> Any comments are welcome.
> 
> thanks,
> rui
> 
> --------------------------------------------------------------------
>  drivers/acpi/thermal.c                   |   76 +++++++--
>  drivers/platform/x86/acerhdf.c           |    4 +-
>  drivers/platform/x86/intel_mid_thermal.c |    2 +-
>  driverk/thermal/spear_thermal.c          |    2 +-
>  drivers/thermal/thermal_sys.c            |  293
> ++++++++++++++++++------------
>  include/linux/thermal.h                  |   22 ++-
>  7 files changed, 271 insertions(+), 148 deletions(-)
> 
> 

--
Eduardo Valentin

Re: [RFC PATCH 0/12] generic thermal layer enhancement

[Zhang Rui]

On 一, 2012-06-11 at 12:49 +0300, Eduardo Valentin wrote:
> Hello Rui,
> 
> On Mon, Jun 11, 2012 at 11:19:11AM +0800, Zhang Rui wrote:
> > Hi, all,
> > 
> > this patch set is made to enhance the current generic thermal layer.
> > It fixes several gaps discussed in
> > http://marc.info/?l=linux-acpi&m=133836783425764&w=2
> > and introduces a simple arbitrator to fix the "one cooling devices
> > referenced by multiple trip points in multiple thermal zones" problem.
> 
> Thanks for taking this further. With code it is much better to progress.
> 
> BTW, are you keeping this series somewhere in a branch?
> 
Not yet.
But I'm thinking of create one. :)

> >  
> > The whole idea is
> > 1) make sure we have multiple cooling states for both active cooling and
> >    passive cooling devices. (patch 1,2,3)
> 
> Nice!
> 
> > 2) remove passive specific requirement, aka, tc1/tc2, and
> >    introduce .get_trend() instead, for both active and passive cooling
> >    algorithm. (patch 4,5)
> 
> Cool. The .get_trend() might be also helpful in case there are sensors
> that provide trending computation, or at least, some history buffer.
> 
> So, I definitely agree with this approach.
> 
> > 3) introduce new function thermal_zone_trip_update(), this contains the
> >    code for the general cooling algorithm. (patch 6)
> 
> Ok.
> 
> > 4) rename some thermal structures. Use thermal_instance instead of
> >    thermal_cooling_device_instance, as this structure is used to
> >    describe the behavior of a certain cooling device for a certain trip
> >    point in a certain thermal zone.
> >    thermal_zone_device has a list of all the thermal instances in this
> >    thermal zone so that it can update them when the temperature changes.
> >    thermal_cooling_device has a list of all the thermal instances for
> >    this cooling device so that it can make decision which cooling state
> >    should be in when the temperature changes. (patch 7,8,9,10)
> 
> Ok.
> 
> > 5) introduce a simple arbitrator. (patch 11)
> >    When temperature changes, we update a thermal zone in two stages,
> >    a) thermal_zone_trip_point() is the general cooling algorithm to
> >       update the thermal instances in the thermal zone device
> >    b) thermal_zone_do_update() is the arbitrator for the cooling device
> >       to choose the deepest cooling state required to enter.
> 
> The arbitrator is good starting point. I will have a deeper look on it.
> 
> But we may be careful to solve the constraint issue only at thermal code
> level. There might be conflicting constraints coming from PM QoS layer,
> for instance.
> 
hmmm, do you have a link for the discussions/patches mentioned?

I'll take a look at this. :)

> > 6) unify the code for both passive and active cooling.
> 
> This is good.
> 
> > 
> > TODO, add locking mechanism. I know this is important but as this patch
> > set changes the thermal layer a(lot, I just want to make sure I'm in the
> > right direction before going on.
> 
> Right. I second you on the incremental approach.
> 
> > 
> > BTW, in theory, they should make no change to the behavior of the
> > current generic thermal layer users. But I have just done the build
> > test.
> 
> OK. I will fetch them and give them a trial on my side, then send better review.
> 
Great. That would be really helpful. Thanks for your interest on this,
Eduardo! :)

-rui

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC PATCH 0/12] generic thermal layer enhancement

[R, Durgadoss]

Hi All,

> The whole idea is
> 1) make sure we have multiple cooling states for both active cooling and
>    passive cooling devices. (patch 1,2,3)
> 2) remove passive specific requirement, aka, tc1/tc2, and
>    introduce .get_trend() instead, for both active and passive cooling
>    algorithm. (patch 4,5)
> 3) introduce new function thermal_zone_trip_update(), this contains the
>    code for the general cooling algorithm. (patch 6)
> 4) rename some thermal structures. Use thermal_instance instead of
>    thermal_cooling_device_instance, as this structure is used to
>    describe the behavior of a certain cooling device for a certain trip
>    point in a certain thermal zone.
>    thermal_zone_device has a list of all the thermal instances in this
>    thermal zone so that it can update them when the temperature changes.
>    thermal_cooling_device has a list of all the thermal instances for
>    this cooling device so that it can make decision which cooling state
>    should be in when the temperature changes. (patch 7,8,9,10)
> 5) introduce a simple arbitrator. (patch 11)
>    When temperature changes, we update a thermal zone in two stages,
>    a) thermal_zone_trip_point() is the general cooling algorithm to
>       update the thermal instances in the thermal zone device
>    b) thermal_zone_do_update() is the arbitrator for the cooling device
>       to choose the deepest cooling state required to enter.
> 6) unify the code for both passive and active cooling.
> 
> TODO, add locking mechanism. I know this is important but as this patch
> set changes the thermal layer a lot, I just want to make sure I'm in the
> right direction before going on.
> 
> BTW, in theory, they should make no change to the behavior of the
> current generic thermal layer users. But I have just done the build
> test.

I talked to Rui about this patch set and my other patch sets.
Rui suggested me to rebase my patches on top of his changes.

So, I hope Rui will be able to clean up these 12 patches and submit them
(in a week's time :-) 
I will rebase my patches on top of them.

Also, Rui promised me that he can host a tree for us to easily pick up changes, 
Let us see :-)

Thanks,
Durga