Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1213868 > unrolled thread

[PATCH v4 0/5] Let the power allocator thermal governor run on any thermal zone

Started byJavi Merino <javi.merino@arm.com>
First post2015-08-26 15:30 +0200
Last post2015-09-02 17:20 +0200
Articles 8 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 0/5] Let the power allocator thermal governor run on any thermal zone Javi Merino <javi.merino@arm.com> - 2015-08-26 15:30 +0200
    [PATCH v4 5/5] thermal: power_allocator: exit early if there are no cooling devices Javi Merino <javi.merino@arm.com> - 2015-08-26 15:30 +0200
    [PATCH v4 4/5] thermal: power_allocator: don't require tzp to be present for the thermal zone Javi Merino <javi.merino@arm.com> - 2015-08-26 15:30 +0200
      Re: [PATCH v4 4/5] thermal: power_allocator: don't require tzp to be  present for the thermal zone Daniel Kurtz <djkurtz@chromium.org> - 2015-08-28 04:20 +0200
        Re: [PATCH v4 4/5] thermal: power_allocator: don't require tzp to be  present for the thermal zone Javi Merino <javi.merino@arm.com> - 2015-08-28 18:30 +0200
          Re: [PATCH v4 4/5] thermal: power_allocator: don't require tzp to be  present for the thermal zone Daniel Kurtz <djkurtz@chromium.org> - 2015-08-31 15:20 +0200
    [PATCH v4 1/5] thermal: Add a function to get the minimum power Javi Merino <javi.merino@arm.com> - 2015-08-26 15:30 +0200
    Re: [PATCH v4 0/5] Let the power allocator thermal governor run on  any thermal zone Javi Merino <javi.merino@arm.com> - 2015-09-02 17:20 +0200

#1213868 — [PATCH v4 0/5] Let the power allocator thermal governor run on any thermal zone

FromJavi Merino <javi.merino@arm.com>
Date2015-08-26 15:30 +0200
Subject[PATCH v4 0/5] Let the power allocator thermal governor run on any thermal zone
Message-ID<q1IW5-6Yn-9@gated-at.bofh.it>
Relax the thermal governor requirements of sustainable_power and at
least two trip points so that it can be bound to any thermal zone.
Its behavior won't be optimal, it would be the best it can with the
data provided.

Changes since v3:
   - Don't hardcode a value for sustainable power and re-estimate
     the PID controllers every time if no sustainable power is given
     as suggested by Eduardo Valentin.
   - power_actor_get_min_power() moved to a patch of its own.

Changes since v2:
  - Typos suggested by Daniel Kurtz

Changes since v1:
  - Let the power allocator governor operate if the thermal zone
    doesn't have tzp as suggested by Chung-yih Wang

Javi Merino (5):
  thermal: Add a function to get the minimum power
  thermal: power_allocator: relax the requirement of a sustainable_power
    in tzp
  thermal: power_allocator: relax the requirement of two passive trip   
     points
  thermal: power_allocator: don't require tzp to be present for the
    thermal zone
  thermal: power_allocator: exit early if there are no cooling devices

 Documentation/thermal/power_allocator.txt |   2 +-
 drivers/thermal/power_allocator.c         | 241 ++++++++++++++++++++++--------
 drivers/thermal/thermal_core.c            |  28 ++++
 include/linux/thermal.h                   |   6 +
 4 files changed, 212 insertions(+), 65 deletions(-)

-- 
1.9.1

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

[toc] | [next] | [standalone]

#1213869 — [PATCH v4 5/5] thermal: power_allocator: exit early if there are no cooling devices

FromJavi Merino <javi.merino@arm.com>
Date2015-08-26 15:30 +0200
Subject[PATCH v4 5/5] thermal: power_allocator: exit early if there are no cooling devices
Message-ID<q1IW6-6Yn-27@gated-at.bofh.it>
In reply to#1213868
Don't waste cycles in the power allocator governor's throttle function
if there are no cooling devices and exit early.

This commit doesn't change any functionality, but should provide better
performance for the odd case of a thermal zone with trip points but
without cooling devices.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
 drivers/thermal/power_allocator.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/thermal/power_allocator.c b/drivers/thermal/power_allocator.c
index 85ce0aac9a41..e6fdcf24ba88 100644
--- a/drivers/thermal/power_allocator.c
+++ b/drivers/thermal/power_allocator.c
@@ -348,6 +348,11 @@ static int allocate_power(struct thermal_zone_device *tz,
 		}
 	}
 
+	if (!num_actors) {
+		ret = -ENODEV;
+		goto unlock;
+	}
+
 	/*
 	 * We need to allocate five arrays of the same size:
 	 * req_power, max_power, granted_power, extra_actor_power and
-- 
1.9.1

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

[toc] | [prev] | [next] | [standalone]

#1213872 — [PATCH v4 4/5] thermal: power_allocator: don't require tzp to be present for the thermal zone

FromJavi Merino <javi.merino@arm.com>
Date2015-08-26 15:30 +0200
Subject[PATCH v4 4/5] thermal: power_allocator: don't require tzp to be present for the thermal zone
Message-ID<q1IW6-6Yn-29@gated-at.bofh.it>
In reply to#1213868
Thermal zones created using thermal_zone_device_create() may not have
tzp.  As the governor gets its parameters from there, allocate it while
the governor is bound to the thermal zone so that it can operate in it.
In this case, tzp is freed when the thermal zone switches to another
governor.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---

While this would be easier to do by just ignoring the thermal zone if
there was no tzp, I think the approach in this patch provides a better
behavior.

 drivers/thermal/power_allocator.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/power_allocator.c b/drivers/thermal/power_allocator.c
index 2dfb8ade4d1b..85ce0aac9a41 100644
--- a/drivers/thermal/power_allocator.c
+++ b/drivers/thermal/power_allocator.c
@@ -58,6 +58,8 @@ static inline s64 div_frac(s64 x, s64 y)
 
 /**
  * struct power_allocator_params - parameters for the power allocator governor
+ * @allocated_tzp:	whether we have allocated tzp for this thermal zone and
+ *			it needs to be freed on unbind
  * @err_integral:	accumulated error in the PID controller.
  * @prev_err:	error in the previous iteration of the PID controller.
  *		Used to calculate the derivative term.
@@ -70,6 +72,7 @@ static inline s64 div_frac(s64 x, s64 y)
  *					controlling for.
  */
 struct power_allocator_params {
+	bool allocated_tzp;
 	s64 err_integral;
 	s32 prev_err;
 	int trip_switch_on;
@@ -530,8 +533,7 @@ static void allow_maximum_power(struct thermal_zone_device *tz)
  * Initialize the PID controller parameters and bind it to the thermal
  * zone.
  *
- * Return: 0 on success, -EINVAL if the thermal zone doesn't have tzp or -ENOMEM
- * if we ran out of memory.
+ * Return: 0 on success, or -ENOMEM if we ran out of memory.
  */
 static int power_allocator_bind(struct thermal_zone_device *tz)
 {
@@ -539,13 +541,20 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
 	struct power_allocator_params *params;
 	unsigned long control_temp;
 
-	if (!tz->tzp)
-		return -EINVAL;
-
 	params = kzalloc(sizeof(*params), GFP_KERNEL);
 	if (!params)
 		return -ENOMEM;
 
+	if (!tz->tzp) {
+		tz->tzp = kzalloc(sizeof(*tz->tzp), GFP_KERNEL);
+		if (!tz->tzp) {
+			ret = -ENOMEM;
+			goto free_params;
+		}
+
+		params->allocated_tzp = true;
+	}
+
 	if (!tz->tzp->sustainable_power)
 		dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");
 
@@ -562,11 +571,24 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
 	tz->governor_data = params;
 
 	return 0;
+
+free_params:
+	kfree(params);
+
+	return ret;
 }
 
 static void power_allocator_unbind(struct thermal_zone_device *tz)
 {
+	struct power_allocator_params *params = tz->governor_data;
+
 	dev_dbg(&tz->device, "Unbinding from thermal zone %d\n", tz->id);
+
+	if (params->allocated_tzp) {
+		kfree(tz->tzp);
+		tz->tzp = NULL;
+	}
+
 	kfree(tz->governor_data);
 	tz->governor_data = NULL;
 }
-- 
1.9.1

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

[toc] | [prev] | [next] | [standalone]

#1215062 — Re: [PATCH v4 4/5] thermal: power_allocator: don't require tzp to be present for the thermal zone

FromDaniel Kurtz <djkurtz@chromium.org>
Date2015-08-28 04:20 +0200
SubjectRe: [PATCH v4 4/5] thermal: power_allocator: don't require tzp to be present for the thermal zone
Message-ID<q2hqN-6lt-9@gated-at.bofh.it>
In reply to#1213872
On Wed, Aug 26, 2015 at 9:26 PM, Javi Merino <javi.merino@arm.com> wrote:
> Thermal zones created using thermal_zone_device_create() may not have
> tzp.  As the governor gets its parameters from there, allocate it while
> the governor is bound to the thermal zone so that it can operate in it.
> In this case, tzp is freed when the thermal zone switches to another
> governor.
>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Signed-off-by: Javi Merino <javi.merino@arm.com>
> ---
>
> While this would be easier to do by just ignoring the thermal zone if
> there was no tzp, I think the approach in this patch provides a better
> behavior.

Why?
Just ignoring the thermal zone seems reasonable and simpler.

>
>  drivers/thermal/power_allocator.c | 32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/thermal/power_allocator.c b/drivers/thermal/power_allocator.c
> index 2dfb8ade4d1b..85ce0aac9a41 100644
> --- a/drivers/thermal/power_allocator.c
> +++ b/drivers/thermal/power_allocator.c
> @@ -58,6 +58,8 @@ static inline s64 div_frac(s64 x, s64 y)
>
>  /**
>   * struct power_allocator_params - parameters for the power allocator governor
> + * @allocated_tzp:     whether we have allocated tzp for this thermal zone and
> + *                     it needs to be freed on unbind
>   * @err_integral:      accumulated error in the PID controller.
>   * @prev_err:  error in the previous iteration of the PID controller.
>   *             Used to calculate the derivative term.
> @@ -70,6 +72,7 @@ static inline s64 div_frac(s64 x, s64 y)
>   *                                     controlling for.
>   */
>  struct power_allocator_params {
> +       bool allocated_tzp;
>         s64 err_integral;
>         s32 prev_err;
>         int trip_switch_on;
> @@ -530,8 +533,7 @@ static void allow_maximum_power(struct thermal_zone_device *tz)
>   * Initialize the PID controller parameters and bind it to the thermal
>   * zone.
>   *
> - * Return: 0 on success, -EINVAL if the thermal zone doesn't have tzp or -ENOMEM
> - * if we ran out of memory.
> + * Return: 0 on success, or -ENOMEM if we ran out of memory.
>   */
>  static int power_allocator_bind(struct thermal_zone_device *tz)
>  {
> @@ -539,13 +541,20 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
>         struct power_allocator_params *params;
>         unsigned long control_temp;
>
> -       if (!tz->tzp)
> -               return -EINVAL;
> -
>         params = kzalloc(sizeof(*params), GFP_KERNEL);
>         if (!params)
>                 return -ENOMEM;
>
> +       if (!tz->tzp) {
> +               tz->tzp = kzalloc(sizeof(*tz->tzp), GFP_KERNEL);

Why bother to allocate this dummy struct?
Can't we just leave tz->tzp as NULL, and do a NULL check where needed?

> +               if (!tz->tzp) {
> +                       ret = -ENOMEM;
> +                       goto free_params;
> +               }
> +
> +               params->allocated_tzp = true;
> +       }
> +
>         if (!tz->tzp->sustainable_power)
>                 dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");
>
> @@ -562,11 +571,24 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
>         tz->governor_data = params;
>
>         return 0;
> +
> +free_params:
> +       kfree(params);
> +
> +       return ret;
>  }
>
>  static void power_allocator_unbind(struct thermal_zone_device *tz)
>  {
> +       struct power_allocator_params *params = tz->governor_data;
> +
>         dev_dbg(&tz->device, "Unbinding from thermal zone %d\n", tz->id);
> +
> +       if (params->allocated_tzp) {
> +               kfree(tz->tzp);
> +               tz->tzp = NULL;
> +       }
> +
>         kfree(tz->governor_data);
>         tz->governor_data = NULL;
>  }
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1215433 — Re: [PATCH v4 4/5] thermal: power_allocator: don't require tzp to be present for the thermal zone

FromJavi Merino <javi.merino@arm.com>
Date2015-08-28 18:30 +0200
SubjectRe: [PATCH v4 4/5] thermal: power_allocator: don't require tzp to be present for the thermal zone
Message-ID<q2uHo-7g-5@gated-at.bofh.it>
In reply to#1215062
On Fri, Aug 28, 2015 at 03:18:20AM +0100, Daniel Kurtz wrote:
> On Wed, Aug 26, 2015 at 9:26 PM, Javi Merino <javi.merino@arm.com> wrote:
> > Thermal zones created using thermal_zone_device_create() may not have
> > tzp.  As the governor gets its parameters from there, allocate it while
> > the governor is bound to the thermal zone so that it can operate in it.
> > In this case, tzp is freed when the thermal zone switches to another
> > governor.
> >
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Eduardo Valentin <edubezval@gmail.com>
> > Signed-off-by: Javi Merino <javi.merino@arm.com>
> > ---
> >
> > While this would be easier to do by just ignoring the thermal zone if
> > there was no tzp, I think the approach in this patch provides a better
> > behavior.
> 
> Why?
> Just ignoring the thermal zone seems reasonable and simpler.

From the developer point of view, I agree that it's simpler.  What I
want to avoid is the system integrator getting different behaviors
based on the presence of tzp when the thermal zone was created.  If
the integrator was to configure this from userspace, they would only
be able to do so if the thermal zone was created with tzp.  I don't
like this distinction, I prefer the consistency from the user point of
view that this patch gives.

Cheers,
Javi

> >  drivers/thermal/power_allocator.c | 32 +++++++++++++++++++++++++++-----
> >  1 file changed, 27 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/thermal/power_allocator.c b/drivers/thermal/power_allocator.c
> > index 2dfb8ade4d1b..85ce0aac9a41 100644
> > --- a/drivers/thermal/power_allocator.c
> > +++ b/drivers/thermal/power_allocator.c
> > @@ -58,6 +58,8 @@ static inline s64 div_frac(s64 x, s64 y)
> >
> >  /**
> >   * struct power_allocator_params - parameters for the power allocator governor
> > + * @allocated_tzp:     whether we have allocated tzp for this thermal zone and
> > + *                     it needs to be freed on unbind
> >   * @err_integral:      accumulated error in the PID controller.
> >   * @prev_err:  error in the previous iteration of the PID controller.
> >   *             Used to calculate the derivative term.
> > @@ -70,6 +72,7 @@ static inline s64 div_frac(s64 x, s64 y)
> >   *                                     controlling for.
> >   */
> >  struct power_allocator_params {
> > +       bool allocated_tzp;
> >         s64 err_integral;
> >         s32 prev_err;
> >         int trip_switch_on;
> > @@ -530,8 +533,7 @@ static void allow_maximum_power(struct thermal_zone_device *tz)
> >   * Initialize the PID controller parameters and bind it to the thermal
> >   * zone.
> >   *
> > - * Return: 0 on success, -EINVAL if the thermal zone doesn't have tzp or -ENOMEM
> > - * if we ran out of memory.
> > + * Return: 0 on success, or -ENOMEM if we ran out of memory.
> >   */
> >  static int power_allocator_bind(struct thermal_zone_device *tz)
> >  {
> > @@ -539,13 +541,20 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
> >         struct power_allocator_params *params;
> >         unsigned long control_temp;
> >
> > -       if (!tz->tzp)
> > -               return -EINVAL;
> > -
> >         params = kzalloc(sizeof(*params), GFP_KERNEL);
> >         if (!params)
> >                 return -ENOMEM;
> >
> > +       if (!tz->tzp) {
> > +               tz->tzp = kzalloc(sizeof(*tz->tzp), GFP_KERNEL);
> 
> Why bother to allocate this dummy struct?
> Can't we just leave tz->tzp as NULL, and do a NULL check where needed?
> 
> > +               if (!tz->tzp) {
> > +                       ret = -ENOMEM;
> > +                       goto free_params;
> > +               }
> > +
> > +               params->allocated_tzp = true;
> > +       }
> > +
> >         if (!tz->tzp->sustainable_power)
> >                 dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");
> >
> > @@ -562,11 +571,24 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
> >         tz->governor_data = params;
> >
> >         return 0;
> > +
> > +free_params:
> > +       kfree(params);
> > +
> > +       return ret;
> >  }
> >
> >  static void power_allocator_unbind(struct thermal_zone_device *tz)
> >  {
> > +       struct power_allocator_params *params = tz->governor_data;
> > +
> >         dev_dbg(&tz->device, "Unbinding from thermal zone %d\n", tz->id);
> > +
> > +       if (params->allocated_tzp) {
> > +               kfree(tz->tzp);
> > +               tz->tzp = NULL;
> > +       }
> > +
> >         kfree(tz->governor_data);
> >         tz->governor_data = NULL;
> >  }
> > --
> > 1.9.1
> >
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216171 — Re: [PATCH v4 4/5] thermal: power_allocator: don't require tzp to be present for the thermal zone

FromDaniel Kurtz <djkurtz@chromium.org>
Date2015-08-31 15:20 +0200
SubjectRe: [PATCH v4 4/5] thermal: power_allocator: don't require tzp to be present for the thermal zone
Message-ID<q3xa9-jz-7@gated-at.bofh.it>
In reply to#1215433
Hi Javi,

On Sat, Aug 29, 2015 at 12:28 AM, Javi Merino <javi.merino@arm.com> wrote:
> On Fri, Aug 28, 2015 at 03:18:20AM +0100, Daniel Kurtz wrote:
>> On Wed, Aug 26, 2015 at 9:26 PM, Javi Merino <javi.merino@arm.com> wrote:
>> > Thermal zones created using thermal_zone_device_create() may not have
>> > tzp.  As the governor gets its parameters from there, allocate it while
>> > the governor is bound to the thermal zone so that it can operate in it.
>> > In this case, tzp is freed when the thermal zone switches to another
>> > governor.
>> >
>> > Cc: Zhang Rui <rui.zhang@intel.com>
>> > Cc: Eduardo Valentin <edubezval@gmail.com>
>> > Signed-off-by: Javi Merino <javi.merino@arm.com>
>> > ---
>> >
>> > While this would be easier to do by just ignoring the thermal zone if
>> > there was no tzp, I think the approach in this patch provides a better
>> > behavior.
>>
>> Why?
>> Just ignoring the thermal zone seems reasonable and simpler.
>
> From the developer point of view, I agree that it's simpler.  What I
> want to avoid is the system integrator getting different behaviors
> based on the presence of tzp when the thermal zone was created.  If
> the integrator was to configure this from userspace, they would only
> be able to do so if the thermal zone was created with tzp.  I don't
> like this distinction, I prefer the consistency from the user point of
> view that this patch gives.

Ok, thanks for the answer.

Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>

Thanks!
-Dan

>
> Cheers,
> Javi
>
>> >  drivers/thermal/power_allocator.c | 32 +++++++++++++++++++++++++++-----
>> >  1 file changed, 27 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/thermal/power_allocator.c b/drivers/thermal/power_allocator.c
>> > index 2dfb8ade4d1b..85ce0aac9a41 100644
>> > --- a/drivers/thermal/power_allocator.c
>> > +++ b/drivers/thermal/power_allocator.c
>> > @@ -58,6 +58,8 @@ static inline s64 div_frac(s64 x, s64 y)
>> >
>> >  /**
>> >   * struct power_allocator_params - parameters for the power allocator governor
>> > + * @allocated_tzp:     whether we have allocated tzp for this thermal zone and
>> > + *                     it needs to be freed on unbind
>> >   * @err_integral:      accumulated error in the PID controller.
>> >   * @prev_err:  error in the previous iteration of the PID controller.
>> >   *             Used to calculate the derivative term.
>> > @@ -70,6 +72,7 @@ static inline s64 div_frac(s64 x, s64 y)
>> >   *                                     controlling for.
>> >   */
>> >  struct power_allocator_params {
>> > +       bool allocated_tzp;
>> >         s64 err_integral;
>> >         s32 prev_err;
>> >         int trip_switch_on;
>> > @@ -530,8 +533,7 @@ static void allow_maximum_power(struct thermal_zone_device *tz)
>> >   * Initialize the PID controller parameters and bind it to the thermal
>> >   * zone.
>> >   *
>> > - * Return: 0 on success, -EINVAL if the thermal zone doesn't have tzp or -ENOMEM
>> > - * if we ran out of memory.
>> > + * Return: 0 on success, or -ENOMEM if we ran out of memory.
>> >   */
>> >  static int power_allocator_bind(struct thermal_zone_device *tz)
>> >  {
>> > @@ -539,13 +541,20 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
>> >         struct power_allocator_params *params;
>> >         unsigned long control_temp;
>> >
>> > -       if (!tz->tzp)
>> > -               return -EINVAL;
>> > -
>> >         params = kzalloc(sizeof(*params), GFP_KERNEL);
>> >         if (!params)
>> >                 return -ENOMEM;
>> >
>> > +       if (!tz->tzp) {
>> > +               tz->tzp = kzalloc(sizeof(*tz->tzp), GFP_KERNEL);
>>
>> Why bother to allocate this dummy struct?
>> Can't we just leave tz->tzp as NULL, and do a NULL check where needed?
>>
>> > +               if (!tz->tzp) {
>> > +                       ret = -ENOMEM;
>> > +                       goto free_params;
>> > +               }
>> > +
>> > +               params->allocated_tzp = true;
>> > +       }
>> > +
>> >         if (!tz->tzp->sustainable_power)
>> >                 dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");
>> >
>> > @@ -562,11 +571,24 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
>> >         tz->governor_data = params;
>> >
>> >         return 0;
>> > +
>> > +free_params:
>> > +       kfree(params);
>> > +
>> > +       return ret;
>> >  }
>> >
>> >  static void power_allocator_unbind(struct thermal_zone_device *tz)
>> >  {
>> > +       struct power_allocator_params *params = tz->governor_data;
>> > +
>> >         dev_dbg(&tz->device, "Unbinding from thermal zone %d\n", tz->id);
>> > +
>> > +       if (params->allocated_tzp) {
>> > +               kfree(tz->tzp);
>> > +               tz->tzp = NULL;
>> > +       }
>> > +
>> >         kfree(tz->governor_data);
>> >         tz->governor_data = NULL;
>> >  }
>> > --
>> > 1.9.1
>> >
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1213873 — [PATCH v4 1/5] thermal: Add a function to get the minimum power

FromJavi Merino <javi.merino@arm.com>
Date2015-08-26 15:30 +0200
Subject[PATCH v4 1/5] thermal: Add a function to get the minimum power
Message-ID<q1IW6-6Yn-31@gated-at.bofh.it>
In reply to#1213868
The thermal core already has a function to get the maximum power of a
cooling device: power_actor_get_max_power().  Add a function to get the
minimum power of a cooling device.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
 drivers/thermal/thermal_core.c | 28 ++++++++++++++++++++++++++++
 include/linux/thermal.h        |  6 ++++++
 2 files changed, 34 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 4ca211be4c0f..760204f0b63c 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -997,6 +997,34 @@ int power_actor_get_max_power(struct thermal_cooling_device *cdev,
 }
 
 /**
+ * power_actor_get_min_power() - get the mainimum power that a cdev can consume
+ * @cdev:	pointer to &thermal_cooling_device
+ * @tz:		a valid thermal zone device pointer
+ * @min_power:	pointer in which to store the minimum power
+ *
+ * Calculate the minimum power consumption in milliwatts that the
+ * cooling device can currently consume and store it in @min_power.
+ *
+ * Return: 0 on success, -EINVAL if @cdev doesn't support the
+ * power_actor API or -E* on other error.
+ */
+int power_actor_get_min_power(struct thermal_cooling_device *cdev,
+			      struct thermal_zone_device *tz, u32 *min_power)
+{
+	unsigned long max_state;
+	int ret;
+
+	if (!cdev_is_power_actor(cdev))
+		return -EINVAL;
+
+	ret = cdev->ops->get_max_state(cdev, &max_state);
+	if (ret)
+		return ret;
+
+	return cdev->ops->state2power(cdev, tz, max_state, min_power);
+}
+
+/**
  * power_actor_set_power() - limit the maximum power that a cooling device can consume
  * @cdev:	pointer to &thermal_cooling_device
  * @instance:	thermal instance to update
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 037e9df2f610..f99d934d373a 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -384,6 +384,8 @@ static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev)
 
 int power_actor_get_max_power(struct thermal_cooling_device *,
 			      struct thermal_zone_device *tz, u32 *max_power);
+int power_actor_get_min_power(struct thermal_cooling_device *,
+			      struct thermal_zone_device *tz, u32 *min_power);
 int power_actor_set_power(struct thermal_cooling_device *,
 			  struct thermal_instance *, u32);
 struct thermal_zone_device *thermal_zone_device_register(const char *, int, int,
@@ -419,6 +421,10 @@ static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev)
 static inline int power_actor_get_max_power(struct thermal_cooling_device *cdev,
 			      struct thermal_zone_device *tz, u32 *max_power)
 { return 0; }
+static inline int power_actor_get_min_power(struct thermal_cooling_device *cdev,
+					    struct thermal_zone_device *tz,
+					    u32 *min_power)
+{ return -ENODEV; }
 static inline int power_actor_set_power(struct thermal_cooling_device *cdev,
 			  struct thermal_instance *tz, u32 power)
 { return 0; }
-- 
1.9.1

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

[toc] | [prev] | [next] | [standalone]

#1217694 — Re: [PATCH v4 0/5] Let the power allocator thermal governor run on any thermal zone

FromJavi Merino <javi.merino@arm.com>
Date2015-09-02 17:20 +0200
SubjectRe: [PATCH v4 0/5] Let the power allocator thermal governor run on any thermal zone
Message-ID<q4hZo-n0-17@gated-at.bofh.it>
In reply to#1213868
On Wed, Aug 26, 2015 at 02:26:39PM +0100, Javi Merino wrote:
> Relax the thermal governor requirements of sustainable_power and at
> least two trip points so that it can be bound to any thermal zone.
> Its behavior won't be optimal, it would be the best it can with the
> data provided.
> 
> Changes since v3:
>    - Don't hardcode a value for sustainable power and re-estimate
>      the PID controllers every time if no sustainable power is given
>      as suggested by Eduardo Valentin.
>    - power_actor_get_min_power() moved to a patch of its own.
> 
> Changes since v2:
>   - Typos suggested by Daniel Kurtz
> 
> Changes since v1:
>   - Let the power allocator governor operate if the thermal zone
>     doesn't have tzp as suggested by Chung-yih Wang
> 
> Javi Merino (5):
>   thermal: Add a function to get the minimum power
>   thermal: power_allocator: relax the requirement of a sustainable_power
>     in tzp
>   thermal: power_allocator: relax the requirement of two passive trip   
>      points
>   thermal: power_allocator: don't require tzp to be present for the
>     thermal zone
>   thermal: power_allocator: exit early if there are no cooling devices
> 
>  Documentation/thermal/power_allocator.txt |   2 +-
>  drivers/thermal/power_allocator.c         | 241 ++++++++++++++++++++++--------
>  drivers/thermal/thermal_core.c            |  28 ++++
>  include/linux/thermal.h                   |   6 +
>  4 files changed, 212 insertions(+), 65 deletions(-)

Gentle ping.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web