Re: [PATCH 1/3] thinkpad_acpi: add support for force_discharge

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 9/29/21 7:47 AM, Thomas Koch wrote:
> Hi Hans,
> 
> On 27.09.21 17:12, Hans de Goede wrote:
>> Hi Nicolò,
>>
>> On 9/27/21 5:00 PM, Nicolò Piazzalunga wrote:
>>> Hi,
>>>
>>> On 9/27/21 3:59 PM, Mark Pearson wrote:
>>>> Do let me know if there are some important use cases for these so I can go back to the FW team and discuss supporting them properly.
>>>
>>> The important use cases are force discharge and inhibit charge.
>>> These at present are dealt with using tpacpi-bat, which relies on (out of tree) acpi_call.
>>> See also your previous reply.
>>
>> I can see how those can be useful in certain circumstances.
>>
>> I can also understand how Lenovo does not want these to be
>> available by default everywhere.
>>
>> I think a good compromise would be to add a bool module option
>> which defaults to false to enable these.
> 
> From the user perspective, I don't agree that this is a good
> compromise. Users simply want to recalibrate their battery.

But can't they already do this by just unplugging the AC and
then let the machine die ?

How is this different, does it somehow magically flicks on the
AC just before the battery deep-discharge protection kicks in
so no unsaved work is lost ?

I have never used this, but that seems unlikely.

Or is the advantage of this that the AC being present results in
no nagging low battery warnings as well as no auto-shutdown
near the end ?

If the advantage of this is indeed the no nagging + auto-shutdown
a better fix would be to do some userspace work to allow a
full, undisturbed, discharge while not on AC for calibration
purposes. That would also work on any vendor laptop, which would
be a big added bonus.

I do believe that Lenovo does want to support doing a calibration
cycle under Linux. This whole thread got resurfaced / revived by
Mark because of discussions with Lenovo about offering better
battery management options under Linux.

> Having to
> set the module option beforehand is an unnecessary hurdle imho.
> 
> Of course a module option again leads to support overhead in "user
> space". Then tlp-stat -b would have to inform the user that
> force_discharge is unfortunately not available, but he should try to set
> the module option. What would that be good for?
> 
> While I respect Mark's official opinion, I would like to counter with my
> experience from 10 years of TLP development and support:
> 
> The calls for force_discharge work unmodified since the 2012 ThinkPads
> (T420/X220) on all models that also support charge thresholds.
> 
> They also work reliably, otherwise the issue tracker at tpacpi-bat and
> TLP would be full of user issues.
> 
> inhibit_charge is probably used rather rarely, at least no TLP user has
> asked for it.

Ok, so I think we should look at the end goal here, which is doing
a battery calibration run, which AFAIK means discharging until
the machine turns off (by itself rather then an emergency shutdown
by the OS), followed by charging to 100%/full in one go.

So as mentioned before if we can disable the emergency shutdown
then we should be able to do this on all vendor laptops.

But I guess the real question here is, what do the Lenovo tools
under Windows do?

Mark, I assume that the Lenovo battery management tool under Windows
does allow doing a calibration, can you figure out how this works?

Maybe that does actually use force_disharge and things got mixed up
when you asked the firmware team about all 3 features in one email,
and the other 2 are only for testing purposes?

> @Mark: what is Lenovo's position on the calls for charge thresholds
> already included in thinkpad_acpi? Are they also internal?

I'm not Mark, but I believe that these are officially supported,
as mentioned before Lenovo does want to offer better battery management
options under Linux.

Regards,

Hans