mbox series

[0/7] ACPI: video / platform/x86: Fix Panasonic laptop missing keypresses

Message ID 20220624112340.10130-1-hdegoede@redhat.com
Headers show
Series ACPI: video / platform/x86: Fix Panasonic laptop missing keypresses | expand

Message

Hans de Goede June 24, 2022, 11:23 a.m. UTC
Hi All,

Here is a series fixing the missing keypresses on some Panasonic Toughbook
models. These missing keypresses are caused by
commit ed83c9171829 ("platform/x86: panasonic-laptop: Resolve hotkey double
trigger bug"), which made the panasonic-laptop driver unconditionally drop
most WMI reported hotkeys.

This series reverts that commit and then adds some more selective filtering
to still avoid the double key-presses on some models, while avoiding
completely missing keypresses on others.

Rafael, these fixes rely on patch 1/7, which is a tweak to
the acpi_video_handles_brightness_key_presses() helper. Without this
tweak this series will cause a regression, so I would like to merge
the entire series through the pdx86 tree, may I have your ack for this?

Regards,

Hans


Hans de Goede (6):
  ACPI: video: Change how we determine if brightness key-presses are
    handled
  platform/x86: panasonic-laptop: sort includes alphabetically
  platform/x86: panasonic-laptop: revert "Resolve hotkey double trigger
    bug"
  platform/x86: panasonic-laptop: don't report duplicate brightness
    key-presses
  platform/x86: panasonic-laptop: filter out duplicate volume
    up/down/mute keypresses
  platform/x86: panasonic-laptop: Use acpi_video_get_backlight_type()

Stefan Seyfried (1):
  platform/x86: panasonic-laptop: de-obfuscate button codes

 drivers/acpi/acpi_video.c               |  13 ++-
 drivers/platform/x86/Kconfig            |   2 +
 drivers/platform/x86/panasonic-laptop.c | 112 ++++++++++++++++++------
 3 files changed, 91 insertions(+), 36 deletions(-)

Comments

Stefan Seyfried June 24, 2022, 7:49 p.m. UTC | #1
On 24.06.22 13:23, Hans de Goede wrote:
> Hi All,
> 
> Here is a series fixing the missing keypresses on some Panasonic Toughbook
> models. These missing keypresses are caused by
> commit ed83c9171829 ("platform/x86: panasonic-laptop: Resolve hotkey double
> trigger bug"), which made the panasonic-laptop driver unconditionally drop
> most WMI reported hotkeys.
> 
> This series reverts that commit and then adds some more selective filtering
> to still avoid the double key-presses on some models, while avoiding
> completely missing keypresses on others.
> 
> Rafael, these fixes rely on patch 1/7, which is a tweak to
> the acpi_video_handles_brightness_key_presses() helper. Without this
> tweak this series will cause a regression, so I would like to merge
> the entire series through the pdx86 tree, may I have your ack for this?
> 
> Regards,
> 
> Hans
> 
> 
> Hans de Goede (6):
>    ACPI: video: Change how we determine if brightness key-presses are
>      handled
>    platform/x86: panasonic-laptop: sort includes alphabetically
>    platform/x86: panasonic-laptop: revert "Resolve hotkey double trigger
>      bug"
>    platform/x86: panasonic-laptop: don't report duplicate brightness
>      key-presses
>    platform/x86: panasonic-laptop: filter out duplicate volume
>      up/down/mute keypresses
>    platform/x86: panasonic-laptop: Use acpi_video_get_backlight_type()
> 
> Stefan Seyfried (1):
>    platform/x86: panasonic-laptop: de-obfuscate button codes
> 
>   drivers/acpi/acpi_video.c               |  13 ++-
>   drivers/platform/x86/Kconfig            |   2 +
>   drivers/platform/x86/panasonic-laptop.c | 112 ++++++++++++++++++------
>   3 files changed, 91 insertions(+), 36 deletions(-)

The whole series works without any manual setup on my Toughbook CF-51:

Tested-by: Stefan Seyfried <seife+kernel@b1-systems.com>

Thanks again!

Stefan
Rafael J. Wysocki June 25, 2022, 1:43 p.m. UTC | #2
On Fri, Jun 24, 2022 at 1:23 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Some systems have an ACPI video bus but not ACPI video devices with
> backlight capability. On these devices brightness key-presses are
> (logically) not reported through the ACPI video bus.
>
> Change how acpi_video_handles_brightness_key_presses() determines if
> brightness key-presses are handled by the ACPI video driver to avoid
> vendor specific drivers/platform/x86 drivers filtering out their
> brightness key-presses even though they are the only ones reporting
> these presses.
>
> Fixes: ed83c9171829 ("platform/x86: panasonic-laptop: Resolve hotkey double trigger bug")
> Reported-and-tested-by: Stefan Seyfried <seife+kernel@b1-systems.com>
> Reported-and-tested-by: Kenneth Chan <kenneth.t.chan@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

and I'm assuming that you'll take this one along with the rest of the series.

> ---
>  drivers/acpi/acpi_video.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index b6ee27cb32f3..dc3c037d6313 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -79,6 +79,7 @@ module_param(device_id_scheme, bool, 0444);
>  static int only_lcd = -1;
>  module_param(only_lcd, int, 0444);
>
> +static bool has_backlight;
>  static int register_count;
>  static DEFINE_MUTEX(register_count_mutex);
>  static DEFINE_MUTEX(video_list_lock);
> @@ -1230,6 +1231,9 @@ acpi_video_bus_get_one_device(struct acpi_device *device,
>         acpi_video_device_bind(video, data);
>         acpi_video_device_find_cap(data);
>
> +       if (data->cap._BCM && data->cap._BCL)
> +               has_backlight = true;
> +
>         mutex_lock(&video->device_list_lock);
>         list_add_tail(&data->entry, &video->video_device_list);
>         mutex_unlock(&video->device_list_lock);
> @@ -2276,6 +2280,7 @@ void acpi_video_unregister(void)
>                 cancel_delayed_work_sync(&video_bus_register_backlight_work);
>                 acpi_bus_unregister_driver(&acpi_video_bus);
>                 register_count = 0;
> +               has_backlight = false;
>         }
>         mutex_unlock(&register_count_mutex);
>  }
> @@ -2294,13 +2299,7 @@ EXPORT_SYMBOL(acpi_video_register_backlight);
>
>  bool acpi_video_handles_brightness_key_presses(void)
>  {
> -       bool have_video_busses;
> -
> -       mutex_lock(&video_list_lock);
> -       have_video_busses = !list_empty(&video_bus_head);
> -       mutex_unlock(&video_list_lock);
> -
> -       return have_video_busses &&
> +       return has_backlight &&
>                (report_key_events & REPORT_BRIGHTNESS_KEY_EVENTS);
>  }
>  EXPORT_SYMBOL(acpi_video_handles_brightness_key_presses);
> --
> 2.36.0
>
Kenneth Chan June 26, 2022, 6:58 a.m. UTC | #3
Hi Hans and Stefan,

On Tue, 21 Jun 2022 at 02:10, Stefan Seyfried
<stefan.seyfried@googlemail.com> wrote:
>
> Well, I looked into the acpi_video.c module and that one is to blame.
> By default, it assumes that both "OUTPUT_KEY_EVENTS" and
> "BRIGHTNESS_KEY_EVENTS" should be handled by this module.
> But on the CF-51, this does not happen. "Video Bus" does not generate
> any key events (I'm not sure about output, but plugging in a VGA monitor
> and enabling/disabling it with xrandr or tapping the "display" fn-f3
> hotkey does not get anything from "Video Bus" input device.
>

The "display" Fn-F3 hotkey doesn't generate any key events on mine
either. I have no external VGA monitors to test it anyway.

Apart from that, the patches work perfectly on my Let's Note CF-W5.
Cheers, Hans!

Tested-by: Kenneth Chan <kenneth.t.chan@gmail.com>


On Sat, 25 Jun 2022 at 03:49, Stefan Seyfried
<stefan.seyfried@googlemail.com> wrote:
>
> On 24.06.22 13:23, Hans de Goede wrote:
> > Hi All,
> >
> > Here is a series fixing the missing keypresses on some Panasonic Toughbook
> > models. These missing keypresses are caused by
> > commit ed83c9171829 ("platform/x86: panasonic-laptop: Resolve hotkey double
> > trigger bug"), which made the panasonic-laptop driver unconditionally drop
> > most WMI reported hotkeys.
> >
> > This series reverts that commit and then adds some more selective filtering
> > to still avoid the double key-presses on some models, while avoiding
> > completely missing keypresses on others.
> >
> > Rafael, these fixes rely on patch 1/7, which is a tweak to
> > the acpi_video_handles_brightness_key_presses() helper. Without this
> > tweak this series will cause a regression, so I would like to merge
> > the entire series through the pdx86 tree, may I have your ack for this?
> >
> > Regards,
> >
> > Hans
> >
> >
> > Hans de Goede (6):
> >    ACPI: video: Change how we determine if brightness key-presses are
> >      handled
> >    platform/x86: panasonic-laptop: sort includes alphabetically
> >    platform/x86: panasonic-laptop: revert "Resolve hotkey double trigger
> >      bug"
> >    platform/x86: panasonic-laptop: don't report duplicate brightness
> >      key-presses
> >    platform/x86: panasonic-laptop: filter out duplicate volume
> >      up/down/mute keypresses
> >    platform/x86: panasonic-laptop: Use acpi_video_get_backlight_type()
> >
> > Stefan Seyfried (1):
> >    platform/x86: panasonic-laptop: de-obfuscate button codes
> >
> >   drivers/acpi/acpi_video.c               |  13 ++-
> >   drivers/platform/x86/Kconfig            |   2 +
> >   drivers/platform/x86/panasonic-laptop.c | 112 ++++++++++++++++++------
> >   3 files changed, 91 insertions(+), 36 deletions(-)
>
> The whole series works without any manual setup on my Toughbook CF-51:
>
> Tested-by: Stefan Seyfried <seife+kernel@b1-systems.com>
>
> Thanks again!
>
> Stefan
> --
> Stefan Seyfried
>
> "For a successful technology, reality must take precedence over
>   public relations, for nature cannot be fooled." -- Richard Feynman
Andy Shevchenko June 28, 2022, 10:35 a.m. UTC | #4
On Fri, Jun 24, 2022 at 01:23:33PM +0200, Hans de Goede wrote:
> Hi All,
> 
> Here is a series fixing the missing keypresses on some Panasonic Toughbook
> models. These missing keypresses are caused by
> commit ed83c9171829 ("platform/x86: panasonic-laptop: Resolve hotkey double
> trigger bug"), which made the panasonic-laptop driver unconditionally drop
> most WMI reported hotkeys.
> 
> This series reverts that commit and then adds some more selective filtering
> to still avoid the double key-presses on some models, while avoiding
> completely missing keypresses on others.
> 
> Rafael, these fixes rely on patch 1/7, which is a tweak to
> the acpi_video_handles_brightness_key_presses() helper. Without this
> tweak this series will cause a regression, so I would like to merge
> the entire series through the pdx86 tree, may I have your ack for this?


I followed this series on Bugzilla,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

with or without my comments addressed (they are optional)

> Regards,
> 
> Hans
> 
> 
> Hans de Goede (6):
>   ACPI: video: Change how we determine if brightness key-presses are
>     handled
>   platform/x86: panasonic-laptop: sort includes alphabetically
>   platform/x86: panasonic-laptop: revert "Resolve hotkey double trigger
>     bug"
>   platform/x86: panasonic-laptop: don't report duplicate brightness
>     key-presses
>   platform/x86: panasonic-laptop: filter out duplicate volume
>     up/down/mute keypresses
>   platform/x86: panasonic-laptop: Use acpi_video_get_backlight_type()
> 
> Stefan Seyfried (1):
>   platform/x86: panasonic-laptop: de-obfuscate button codes
> 
>  drivers/acpi/acpi_video.c               |  13 ++-
>  drivers/platform/x86/Kconfig            |   2 +
>  drivers/platform/x86/panasonic-laptop.c | 112 ++++++++++++++++++------
>  3 files changed, 91 insertions(+), 36 deletions(-)
> 
> -- 
> 2.36.0
>
Hans de Goede June 28, 2022, 8:02 p.m. UTC | #5
Hi All,

On 6/24/22 13:23, Hans de Goede wrote:
> Hi All,
> 
> Here is a series fixing the missing keypresses on some Panasonic Toughbook
> models. These missing keypresses are caused by
> commit ed83c9171829 ("platform/x86: panasonic-laptop: Resolve hotkey double
> trigger bug"), which made the panasonic-laptop driver unconditionally drop
> most WMI reported hotkeys.
> 
> This series reverts that commit and then adds some more selective filtering
> to still avoid the double key-presses on some models, while avoiding
> completely missing keypresses on others.
> 
> Rafael, these fixes rely on patch 1/7, which is a tweak to
> the acpi_video_handles_brightness_key_presses() helper. Without this
> tweak this series will cause a regression, so I would like to merge
> the entire series through the pdx86 tree, may I have your ack for this?

I've added this series to my review-hans (soon to be for-next) branch now.
Patches 1-6 have also been added to the fixes branch.

Regards,

Hans

> 
> Regards,
> 
> Hans
> 
> 
> Hans de Goede (6):
>   ACPI: video: Change how we determine if brightness key-presses are
>     handled
>   platform/x86: panasonic-laptop: sort includes alphabetically
>   platform/x86: panasonic-laptop: revert "Resolve hotkey double trigger
>     bug"
>   platform/x86: panasonic-laptop: don't report duplicate brightness
>     key-presses
>   platform/x86: panasonic-laptop: filter out duplicate volume
>     up/down/mute keypresses
>   platform/x86: panasonic-laptop: Use acpi_video_get_backlight_type()
> 
> Stefan Seyfried (1):
>   platform/x86: panasonic-laptop: de-obfuscate button codes
> 
>  drivers/acpi/acpi_video.c               |  13 ++-
>  drivers/platform/x86/Kconfig            |   2 +
>  drivers/platform/x86/panasonic-laptop.c | 112 ++++++++++++++++++------
>  3 files changed, 91 insertions(+), 36 deletions(-)
>
Hans de Goede June 28, 2022, 8:09 p.m. UTC | #6
Hi,

On 6/26/22 08:58, Kenneth Chan wrote:
> Hi Hans and Stefan,
> 
> On Tue, 21 Jun 2022 at 02:10, Stefan Seyfried
> <stefan.seyfried@googlemail.com> wrote:
>>
>> Well, I looked into the acpi_video.c module and that one is to blame.
>> By default, it assumes that both "OUTPUT_KEY_EVENTS" and
>> "BRIGHTNESS_KEY_EVENTS" should be handled by this module.
>> But on the CF-51, this does not happen. "Video Bus" does not generate
>> any key events (I'm not sure about output, but plugging in a VGA monitor
>> and enabling/disabling it with xrandr or tapping the "display" fn-f3
>> hotkey does not get anything from "Video Bus" input device.
>>
> 
> The "display" Fn-F3 hotkey doesn't generate any key events on mine
> either.

Hmm, well that is unrelated to the double / missing other hotkeys,
but still worth investigating.

Normally that key just sends Windows-key (Super/Meta) + P, did you
check the atkbd for this being outputted?

You could install acpid and then run acpi_listen and see if any
events are reported for the key-press.

You could also try adding:

wmi.debug_event=Y to your kernel commandline and see if:

dmesg -w 

Shows any new output when the key is pressed ?


> I have no external VGA monitors to test it anyway.

Typically the key-press is handled independently from there
actually being an external monitor.

> Apart from that, the patches work perfectly on my Let's Note CF-W5.
> Cheers, Hans!
> 
> Tested-by: Kenneth Chan <kenneth.t.chan@gmail.com>

Thanks (to both of you).

Regards,

Hans


> 
> 
> On Sat, 25 Jun 2022 at 03:49, Stefan Seyfried
> <stefan.seyfried@googlemail.com> wrote:
>>
>> On 24.06.22 13:23, Hans de Goede wrote:
>>> Hi All,
>>>
>>> Here is a series fixing the missing keypresses on some Panasonic Toughbook
>>> models. These missing keypresses are caused by
>>> commit ed83c9171829 ("platform/x86: panasonic-laptop: Resolve hotkey double
>>> trigger bug"), which made the panasonic-laptop driver unconditionally drop
>>> most WMI reported hotkeys.
>>>
>>> This series reverts that commit and then adds some more selective filtering
>>> to still avoid the double key-presses on some models, while avoiding
>>> completely missing keypresses on others.
>>>
>>> Rafael, these fixes rely on patch 1/7, which is a tweak to
>>> the acpi_video_handles_brightness_key_presses() helper. Without this
>>> tweak this series will cause a regression, so I would like to merge
>>> the entire series through the pdx86 tree, may I have your ack for this?
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>>
>>> Hans de Goede (6):
>>>    ACPI: video: Change how we determine if brightness key-presses are
>>>      handled
>>>    platform/x86: panasonic-laptop: sort includes alphabetically
>>>    platform/x86: panasonic-laptop: revert "Resolve hotkey double trigger
>>>      bug"
>>>    platform/x86: panasonic-laptop: don't report duplicate brightness
>>>      key-presses
>>>    platform/x86: panasonic-laptop: filter out duplicate volume
>>>      up/down/mute keypresses
>>>    platform/x86: panasonic-laptop: Use acpi_video_get_backlight_type()
>>>
>>> Stefan Seyfried (1):
>>>    platform/x86: panasonic-laptop: de-obfuscate button codes
>>>
>>>   drivers/acpi/acpi_video.c               |  13 ++-
>>>   drivers/platform/x86/Kconfig            |   2 +
>>>   drivers/platform/x86/panasonic-laptop.c | 112 ++++++++++++++++++------
>>>   3 files changed, 91 insertions(+), 36 deletions(-)
>>
>> The whole series works without any manual setup on my Toughbook CF-51:
>>
>> Tested-by: Stefan Seyfried <seife+kernel@b1-systems.com>
>>
>> Thanks again!
>>
>> Stefan
>> --
>> Stefan Seyfried
>>
>> "For a successful technology, reality must take precedence over
>>   public relations, for nature cannot be fooled." -- Richard Feynman
>