Re: [PATCH 2/2] platform/x86: panasonic-laptop: allow to use all hotkeys

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

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

Hi,

On 6/20/22 17:21, Kenneth Chan wrote:
> It took quite a while to do a full compile, just to be safe.

<snip backlight stuff, which looks as expected>

> The mute, volume up/down keys are still duplicated by atkbd after
> applying 0005-platform-x86-panasonic-laptop-filter-out-duplicate-v.patch.

Hmm, can you add a couple of:

 pr_info("data 0x%02x\n", data);

at the top of the new panasonic_i8042_filter() function
and then check in dmesg what is output for the volume keys.

The patch should filter out those duplicate keys, unless
I got the codes wrong somehow.

Also can you please try the attached 2 patches on top of my
last series, this should hide the broken panasonic backlight
device and otherwise it should make no difference (but maybe
double check the duplicate brightness keys are not back.

Regards,

Hans

[-- Attachment #2: 0001-platform-x86-panasonic-laptop-Use-acpi_video_get_bac.patch --]
[-- Type: text/x-patch, Size: 1938 bytes --]

From f9afd65564e1f4e8e41b0cdbc754b11fc0e2edf6 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Tue, 21 Jun 2022 11:20:42 +0200
Subject: [PATCH 1/2] platform/x86: panasonic-laptop: Use
 acpi_video_get_backlight_type()

Use acpi_video_get_backlight_type() to determine if we should register
the panasonic specific backlight interface. To avoid registering this
on systems where the ACPI or GPU native backlight control methods
should be used instead.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/panasonic-laptop.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index a4c82b3a81cf..0fa7695089e2 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -998,15 +998,19 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
 		pr_err("Couldn't retrieve BIOS data\n");
 		goto out_input;
 	}
-	/* initialize backlight */
-	memset(&props, 0, sizeof(struct backlight_properties));
-	props.type = BACKLIGHT_PLATFORM;
-	props.max_brightness = pcc->sinf[SINF_AC_MAX_BRIGHT];
-	pcc->backlight = backlight_device_register("panasonic", NULL, pcc,
-						   &pcc_backlight_ops, &props);
-	if (IS_ERR(pcc->backlight)) {
-		result = PTR_ERR(pcc->backlight);
-		goto out_input;
+
+	if (acpi_video_get_backlight_type() == acpi_backlight_vendor) {
+		/* initialize backlight */
+		memset(&props, 0, sizeof(struct backlight_properties));
+		props.type = BACKLIGHT_PLATFORM;
+		props.max_brightness = pcc->sinf[SINF_AC_MAX_BRIGHT];
+		
+		pcc->backlight = backlight_device_register("panasonic", NULL, pcc,
+							   &pcc_backlight_ops, &props);
+		if (IS_ERR(pcc->backlight)) {
+			result = PTR_ERR(pcc->backlight);
+			goto out_input;
+		}
 	}
 
 	/* read the initial brightness setting from the hardware */
-- 
2.36.0


[-- Attachment #3: 0002-ACPI-video-Change-how-we-determine-if-brightness-key.patch --]
[-- Type: text/x-patch, Size: 2465 bytes --]

From 70d07e9157bb2813726986d068fb0c99e1a619d9 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Tue, 21 Jun 2022 11:12:51 +0200
Subject: [PATCH 2/2] ACPI: video: Change how we determine if brightness
 key-presses are handled

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.

Reported-by: Stefan Seyfried <seife+kernel@b1-systems.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 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 01c3f62295c3..be7f4d1912de 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