Re: dell-laptop and separate AC timeouts on some Dell systems

["Pali Rohár" <pali.rohar@gmail.com>]

[-- Attachment #1: Type: Text/Plain, Size: 2184 bytes --]

Hi!

On Friday 07 April 2017 21:33:53 Mario.Limonciello@dell.com wrote:
> Pali,
> 
> For some time there have been folks reporting some issues where
> keyboard backlight setting is failing on various Dell systems
> (https://github.com/dell/libsmbios/issues/3).  I've been circling
> around internally to find out what's going on. This affects a number
> of systems from the last year or so.

Great!

> What is happening is that some platforms support an alternate
> keyboard timeout while on AC.  The old "timeout" value is treated as
> just a battery timeout and the separate value is the AC timeout.
> Unfortunately if the AC timeout is /not/ set in the timeout
> setting/update request, this will fail.

So in case notebook is running on AC and we want to change timeout for 
keyboard backlight, we need to tell smbios two timeouts, one for battery 
and one for AC?

> As a result I prototyped a few changes for this at the libsmbios
> branch here:
> https://github.com/dell/libsmbios/commits/fix-g8-keyboard-backlight

Can you update also documentation which is at the end of file? It would 
help for kernel implementation.

> It's not yet requested for merging because I still don't know why
> the request to supported features returns "Always On" still (that
> shouldn't be supported).

It looks like that "Always On" mode is reported by smbios by supported, 
but setting it will cause failure. Any idea where is a problem? Probably 
Dell firmware bug? Or needs to be that mode handled specially?

> As far as I can tell this doesn't really map well to how the keyboard
> backlight driver in the kernel works today, so I wanted to raise
> this to you to discuss what the best way to handle it is. One of
> these systems can be detected by the presence of the token 0x451. 
> When that is found, the additional timeout unit and value should be
> sent with the requests.
> 
> Can you let me know your thoughts?

I think that extending struct kbd_state should work.

There are already functions dell_send_intensity() and 
dell_get_intensity() which handle state based on battery/AC mode.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]