linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* power_supply cooling interface
@ 2022-07-06 12:54 Caleb Connolly
  2023-05-12 16:48 ` Pavel Machek
  0 siblings, 1 reply; 5+ messages in thread
From: Caleb Connolly @ 2022-07-06 12:54 UTC (permalink / raw)
  To: Sebastian Reichel, Rafael J. Wysocki, Daniel Lezcano,
	Amit Kucheria, Zhang Rui, linux-pm, linux-kernel, linux-arm-msm

Hi,

I've been working on a driver for the charger found in most Snapdragon 845 
phones (the OnePlus 6, SHIFT6mq, PocoPhone F1, etc). I wanted to include support 
for the POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT property.

My understanding is that it exposes the current limit as a cooling device so 
that userspace (or frameworks like DTPM) can optimise for performance in a 
thermally constrained device by limiting the input current and thus reducing the 
heat generated by the charger circuitry, a similar idea was applied on the Pixel C:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a4496d52b3430cb3c4c16d03cdd5f4ee97ad1241

However, reading through the sysfs docs for cooling devices, and looking at the 
implementation in power_supply_core.c, it seems like the behavior here is wrong 
in a few ways:
  1. The values should scale from 0: no cooling to max_state: max cooling, but 
the power_supply docs and the only existing implementation (the smbb driver) 
just export the current_limit, such that increasing cur_state would increase the 
current limit, not decrease it.
  2. (unsure?)The scale is completely different to most other cooling devices, 
most cooling devices don't seem to have a max state much beyond the double 
digits, but CHARGE_CONTROL_LIMIT is on the scale of uA, so approaches like 
incrementing the cooling state by 1 don't really work.
  3. The value exposed is current, not power, making it tricky for something 
like the DTPM framework to make good use of it, and making it harder to 
correlate a particular "amount" of cooling with a change in thermal headroom.


I don't really know what the right approach is here, one idea might be to have 
the power_supply cooling device implementation try and be more intelligent. 
Scaling based on the power rather than current and exposing some smaller range 
so that at maximum cooling the device doesn't just stop charging entirely 
(unless we've hit some thermal trip?).

Maybe a way to determine the amount of thermal headroom you can expect by 
adjusting the current limit on a particular charger / device like a power 
efficiency curve might be useful too, although gathering that data might be 
difficult to do.

I'll include the POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT and 
POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX properties in my driver, as the 
maximum current allowed and whatever the maximum is right now.

-- 
Kind Regards,
Caleb (they/he)

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

* Re: power_supply cooling interface
  2022-07-06 12:54 power_supply cooling interface Caleb Connolly
@ 2023-05-12 16:48 ` Pavel Machek
  2023-05-12 17:22   ` Daniel Lezcano
  0 siblings, 1 reply; 5+ messages in thread
From: Pavel Machek @ 2023-05-12 16:48 UTC (permalink / raw)
  To: Caleb Connolly
  Cc: Sebastian Reichel, Rafael J. Wysocki, Daniel Lezcano,
	Amit Kucheria, Zhang Rui, linux-pm, linux-kernel, linux-arm-msm

Hi!

> I've been working on a driver for the charger found in most Snapdragon
> 845 phones (the OnePlus 6, SHIFT6mq, PocoPhone F1, etc). I wanted to
> include support for the POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT
> property.
> 
> My understanding is that it exposes the current limit as a cooling
> device so that userspace (or frameworks like DTPM) can optimise for
> performance in a thermally constrained device by limiting the input
> current and thus reducing the heat generated by the charger circuitry,
> a similar idea was applied on the Pixel C:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a4496d52b3430cb3c4c16d03cdd5f4ee97ad1241
> 
> However, reading through the sysfs docs for cooling devices, and
> looking at the implementation in power_supply_core.c, it seems like the
> behavior here is wrong in a few ways:
>  1. The values should scale from 0: no cooling to max_state: max
> cooling, but the power_supply docs and the only existing implementation
> (the smbb driver) just export the current_limit, such that increasing
> cur_state would increase the current limit, not decrease it.
>  2. (unsure?)The scale is completely different to most other cooling
> devices, most cooling devices don't seem to have a max state much
> beyond the double digits, but CHARGE_CONTROL_LIMIT is on the scale of
> uA, so approaches like incrementing the cooling state by 1 don't really
> work.

Did this get solved somehow?

Anyway, I am not sure mW will be useful here, as elsewhere it is mW
thermal and here it is mW from charger. Most of that energy should be
stored in battery, not converted to heat.

Best regards,
							Pavel

-- 

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

* Re: power_supply cooling interface
  2023-05-12 16:48 ` Pavel Machek
@ 2023-05-12 17:22   ` Daniel Lezcano
  2023-05-12 17:53     ` Pavel Machek
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Lezcano @ 2023-05-12 17:22 UTC (permalink / raw)
  To: Pavel Machek, Caleb Connolly
  Cc: Sebastian Reichel, Rafael J. Wysocki, Amit Kucheria, Zhang Rui,
	linux-pm, linux-kernel, linux-arm-msm

On 12/05/2023 18:48, Pavel Machek wrote:
> Hi!
> 
>> I've been working on a driver for the charger found in most Snapdragon
>> 845 phones (the OnePlus 6, SHIFT6mq, PocoPhone F1, etc). I wanted to
>> include support for the POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT
>> property.
>>
>> My understanding is that it exposes the current limit as a cooling
>> device so that userspace (or frameworks like DTPM) can optimise for
>> performance in a thermally constrained device by limiting the input
>> current and thus reducing the heat generated by the charger circuitry,
>> a similar idea was applied on the Pixel C:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a4496d52b3430cb3c4c16d03cdd5f4ee97ad1241
>>
>> However, reading through the sysfs docs for cooling devices, and
>> looking at the implementation in power_supply_core.c, it seems like the
>> behavior here is wrong in a few ways:
>>   1. The values should scale from 0: no cooling to max_state: max
>> cooling, but the power_supply docs and the only existing implementation
>> (the smbb driver) just export the current_limit, such that increasing
>> cur_state would increase the current limit, not decrease it.
>>   2. (unsure?)The scale is completely different to most other cooling
>> devices, most cooling devices don't seem to have a max state much
>> beyond the double digits, but CHARGE_CONTROL_LIMIT is on the scale of
>> uA, so approaches like incrementing the cooling state by 1 don't really
>> work.
> 
> Did this get solved somehow?

Thanks for resurrecting the discussion.

> Anyway, I am not sure mW will be useful here, as elsewhere it is mW
> thermal and here it is mW from charger. Most of that energy should be
> stored in battery, not converted to heat.

I'm not sure to understand the comment. The question is about decreasing 
the speed of the charge of the battery because the faster it charges the 
warmer it gets. Doing a fast charge is ok, if the phone is for instance 
on a table doing nothing. But if the environment is hot (a car, a 
pocket) or there are other sources of heat on the phone like a game, the 
temperature of the battery could be too high (or the skin temperature). 
In this case we have to balance the heat contribution of the different 
components by reducing their performances. The first knob to act on is 
to reduce the charge speed of the battery by reducing the delivered power.

For that we need a connection between the thermal framework which 
monitors the battery temperature and the power supply to reduce the 
charge speed when it is too hot. This connection is the cooling device.

The cooling devices have opaque values where the min and max cooling 
effect vary depending on the implementation (eg. a fan 0/1, a LCD light 
0/1023).

Here the power supply has yet another unit (uA) to act on and difficult 
to translate to a cooling device discrete numbers (that is my 
understanding).

My suggestion is adding to the power supply the power capping capability 
using the powercap framework and add it to the DTPM nomenclature.

With enough components in DTPM, it will be possible to create a generic 
power cooling device using the unified unit uW with the powercap API.



-- 
<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: power_supply cooling interface
  2023-05-12 17:22   ` Daniel Lezcano
@ 2023-05-12 17:53     ` Pavel Machek
  2023-05-13  2:07       ` Caleb Connolly
  0 siblings, 1 reply; 5+ messages in thread
From: Pavel Machek @ 2023-05-12 17:53 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Caleb Connolly, Sebastian Reichel, Rafael J. Wysocki,
	Amit Kucheria, Zhang Rui, linux-pm, linux-kernel, linux-arm-msm

[-- Attachment #1: Type: text/plain, Size: 3760 bytes --]

Hi!

> > > I've been working on a driver for the charger found in most Snapdragon
> > > 845 phones (the OnePlus 6, SHIFT6mq, PocoPhone F1, etc). I wanted to
> > > include support for the POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT
> > > property.
> > > 
> > > My understanding is that it exposes the current limit as a cooling
> > > device so that userspace (or frameworks like DTPM) can optimise for
> > > performance in a thermally constrained device by limiting the input
> > > current and thus reducing the heat generated by the charger circuitry,
> > > a similar idea was applied on the Pixel C:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a4496d52b3430cb3c4c16d03cdd5f4ee97ad1241
> > > 
> > > However, reading through the sysfs docs for cooling devices, and
> > > looking at the implementation in power_supply_core.c, it seems like the
> > > behavior here is wrong in a few ways:
> > >   1. The values should scale from 0: no cooling to max_state: max
> > > cooling, but the power_supply docs and the only existing implementation
> > > (the smbb driver) just export the current_limit, such that increasing
> > > cur_state would increase the current limit, not decrease it.
> > >   2. (unsure?)The scale is completely different to most other cooling
> > > devices, most cooling devices don't seem to have a max state much
> > > beyond the double digits, but CHARGE_CONTROL_LIMIT is on the scale of
> > > uA, so approaches like incrementing the cooling state by 1 don't really
> > > work.
> > 
> > Did this get solved somehow?
> 
> Thanks for resurrecting the discussion.
> 
> > Anyway, I am not sure mW will be useful here, as elsewhere it is mW
> > thermal and here it is mW from charger. Most of that energy should be
> > stored in battery, not converted to heat.
> 
> I'm not sure to understand the comment. The question is about decreasing the
> speed of the charge of the battery because the faster it charges the warmer
> it gets. Doing a fast charge is ok, if the phone is for instance on a table
> doing nothing. But if the environment is hot (a car, a pocket) or there are
> other sources of heat on the phone like a game, the temperature of the
> battery could be too high (or the skin temperature). In this case we have to
> balance the heat contribution of the different components by reducing their
> performances. The first knob to act on is to reduce the charge speed of the
> battery by reducing the delivered power.

Understood.

> For that we need a connection between the thermal framework which monitors
> the battery temperature and the power supply to reduce the charge speed when
> it is too hot. This connection is the cooling device.
> 
> The cooling devices have opaque values where the min and max cooling effect
> vary depending on the implementation (eg. a fan 0/1, a LCD light 0/1023).

Aha, ok.

> Here the power supply has yet another unit (uA) to act on and difficult to
> translate to a cooling device discrete numbers (that is my
> understanding).

Well, if you can accept 1000 steps like you do for LCD, all you really
need is maximum current and then stepping in 1/100 of that.

> With enough components in DTPM, it will be possible to create a generic
> power cooling device using the unified unit uW with the powercap API.

I was trying to point out trouble with uW: you don't know them in case
of battery charging.

You know phone is drawing 500mA @ close to 5V (-> 2.5W), but you don't
really know how much is stored in battery, and how much is turned into
heat.

But I guess you could approximate that somehow.

BR,								Pavel

-- People of Russia, stop Putin before his war on Ukraine
escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

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

* Re: power_supply cooling interface
  2023-05-12 17:53     ` Pavel Machek
@ 2023-05-13  2:07       ` Caleb Connolly
  0 siblings, 0 replies; 5+ messages in thread
From: Caleb Connolly @ 2023-05-13  2:07 UTC (permalink / raw)
  To: Pavel Machek, Daniel Lezcano
  Cc: Sebastian Reichel, Rafael J. Wysocki, Amit Kucheria, Zhang Rui,
	linux-pm, linux-kernel, linux-arm-msm



On 12/05/2023 17:53, Pavel Machek wrote:
> Hi!
> 
>>>> I've been working on a driver for the charger found in most Snapdragon
>>>> 845 phones (the OnePlus 6, SHIFT6mq, PocoPhone F1, etc). I wanted to
>>>> include support for the POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT
>>>> property.
>>>>
>>>> My understanding is that it exposes the current limit as a cooling
>>>> device so that userspace (or frameworks like DTPM) can optimise for
>>>> performance in a thermally constrained device by limiting the input
>>>> current and thus reducing the heat generated by the charger circuitry,
>>>> a similar idea was applied on the Pixel C:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a4496d52b3430cb3c4c16d03cdd5f4ee97ad1241
>>>>
>>>> However, reading through the sysfs docs for cooling devices, and
>>>> looking at the implementation in power_supply_core.c, it seems like the
>>>> behavior here is wrong in a few ways:
>>>>   1. The values should scale from 0: no cooling to max_state: max
>>>> cooling, but the power_supply docs and the only existing implementation
>>>> (the smbb driver) just export the current_limit, such that increasing
>>>> cur_state would increase the current limit, not decrease it.
>>>>   2. (unsure?)The scale is completely different to most other cooling
>>>> devices, most cooling devices don't seem to have a max state much
>>>> beyond the double digits, but CHARGE_CONTROL_LIMIT is on the scale of
>>>> uA, so approaches like incrementing the cooling state by 1 don't really
>>>> work.
>>>
>>> Did this get solved somehow?
>>
>> Thanks for resurrecting the discussion.
>>
>>> Anyway, I am not sure mW will be useful here, as elsewhere it is mW
>>> thermal and here it is mW from charger. Most of that energy should be
>>> stored in battery, not converted to heat.
>>
>> I'm not sure to understand the comment. The question is about decreasing the
>> speed of the charge of the battery because the faster it charges the warmer
>> it gets. Doing a fast charge is ok, if the phone is for instance on a table
>> doing nothing. But if the environment is hot (a car, a pocket) or there are
>> other sources of heat on the phone like a game, the temperature of the
>> battery could be too high (or the skin temperature). In this case we have to
>> balance the heat contribution of the different components by reducing their
>> performances. The first knob to act on is to reduce the charge speed of the
>> battery by reducing the delivered power.
> 
> Understood.
> 
>> For that we need a connection between the thermal framework which monitors
>> the battery temperature and the power supply to reduce the charge speed when
>> it is too hot. This connection is the cooling device.
>>
>> The cooling devices have opaque values where the min and max cooling effect
>> vary depending on the implementation (eg. a fan 0/1, a LCD light 0/1023).
> 
> Aha, ok.
> 
>> Here the power supply has yet another unit (uA) to act on and difficult to
>> translate to a cooling device discrete numbers (that is my
>> understanding).
> 
> Well, if you can accept 1000 steps like you do for LCD, all you really
> need is maximum current and then stepping in 1/100 of that.
> 
>> With enough components in DTPM, it will be possible to create a generic
>> power cooling device using the unified unit uW with the powercap API.
> 
> I was trying to point out trouble with uW: you don't know them in case
> of battery charging.
> 
> You know phone is drawing 500mA @ close to 5V (-> 2.5W), but you don't
> really know how much is stored in battery, and how much is turned into
> heat.

You can know the thermal effect if the charger IC has a temperature
probe, and use time constant estimates to figure out the power losses.
> 
> But I guess you could approximate that somehow.

Also knowing the rough efficiency of the switching converter under the
circumstances (current limited? proportion of input current going to the
battery? voltage? battery voltage?)

I can attest to charging losses being a pretty significant heat source,
at least on the phones I've owned.

It gets tricky with phones that use charge pumps (e.g. OnePlus/Oppo VOOC
charging) to offload the thermal losses to the wall charger. Some
devices also have 2 or more battery cells, enabling them to be charged
in parallel for greater efficiency from a PD charger at 9V. My OnePlus 9
gets noticeably hotter when charging at 5V@3A compared to 9V@2A.

Most SDM845 devices use "parallel charging" with two discrete charger
ICs (one in the PMIC, one external i2c), effectively allowing for much
higher input current and much greater efficiency.

Can these be modeled well via DTPM? As I don't think the existing PSY
subsystem has a nice way to deal with this, and I don't think it's
sensible to offload all of that to userspace.

> 
> BR,								Pavel
> 
> -- People of Russia, stop Putin before his war on Ukraine
> escalates.

-- 
Kind Regards,
Caleb (they/them)

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

end of thread, other threads:[~2023-05-13  2:07 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-06 12:54 power_supply cooling interface Caleb Connolly
2023-05-12 16:48 ` Pavel Machek
2023-05-12 17:22   ` Daniel Lezcano
2023-05-12 17:53     ` Pavel Machek
2023-05-13  2:07       ` Caleb Connolly

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).