[PATCH 00/22] Fallback to native backlight

[PATCH 00/22] Fallback to native backlight

[Akihiko Odaki]

Commit 2600bfa3df99 ("ACPI: video: Add acpi_video_backlight_use_native()
helper") and following commits made native backlight unavailable if
CONFIG_ACPI_VIDEO is set and the backlight feature of ACPI video is
unavailable, which broke the backlight functionality on Lenovo ThinkPad
C13 Yoga Chromebook. Allow to fall back to native backlight in such
cases.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Akihiko Odaki (22):
  drm/i915/opregion: Improve backlight request condition
  ACPI: video: Introduce acpi_video_get_backlight_types()
  LoongArch: Use acpi_video_get_backlight_types()
  platform/x86: acer-wmi: Use acpi_video_get_backlight_types()
  platform/x86: asus-laptop: Use acpi_video_get_backlight_types()
  platform/x86: asus-wmi: Use acpi_video_get_backlight_types()
  platform/x86: compal-laptop: Use acpi_video_get_backlight_types()
  platform/x86: eeepc-laptop: Use acpi_video_get_backlight_types()
  platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()
  platform/x86: ideapad-laptop: Use acpi_video_get_backlight_types()
  platform/x86: msi-laptop: Use acpi_video_get_backlight_types()
  platform/x86: msi-wmi: Use acpi_video_get_backlight_types()
  platform/x86: nvidia-wmi-ec-backlight: Use
    acpi_video_get_backlight_types()
  platform/x86: panasonic-laptop: Use acpi_video_get_backlight_types()
  platform/x86: samsung-laptop: Use acpi_video_get_backlight_types()
  platform/x86: sony-laptop: Use acpi_video_get_backlight_types()
  platform/x86: thinkpad_acpi: Use acpi_video_get_backlight_types()
  platform/x86: toshiba_acpi: Use acpi_video_get_backlight_types()
  platform/x86: dell-laptop: Use acpi_video_get_backlight_types()
  platform/x86: intel_oaktrail: Use acpi_video_get_backlight_types()
  ACPI: video: Remove acpi_video_get_backlight_type()
  ACPI: video: Fallback to native backlight

 Documentation/gpu/todo.rst                    |  8 +--
 drivers/acpi/acpi_video.c                     |  2 +-
 drivers/acpi/video_detect.c                   | 54 ++++++++++---------
 drivers/gpu/drm/i915/display/intel_opregion.c |  3 +-
 drivers/platform/loongarch/loongson-laptop.c  |  4 +-
 drivers/platform/x86/acer-wmi.c               |  2 +-
 drivers/platform/x86/asus-laptop.c            |  2 +-
 drivers/platform/x86/asus-wmi.c               |  4 +-
 drivers/platform/x86/compal-laptop.c          |  2 +-
 drivers/platform/x86/dell/dell-laptop.c       |  2 +-
 drivers/platform/x86/eeepc-laptop.c           |  2 +-
 drivers/platform/x86/fujitsu-laptop.c         |  4 +-
 drivers/platform/x86/ideapad-laptop.c         |  2 +-
 drivers/platform/x86/intel/oaktrail.c         |  2 +-
 drivers/platform/x86/msi-laptop.c             |  2 +-
 drivers/platform/x86/msi-wmi.c                |  2 +-
 .../platform/x86/nvidia-wmi-ec-backlight.c    |  2 +-
 drivers/platform/x86/panasonic-laptop.c       |  2 +-
 drivers/platform/x86/samsung-laptop.c         |  2 +-
 drivers/platform/x86/sony-laptop.c            |  2 +-
 drivers/platform/x86/thinkpad_acpi.c          |  4 +-
 drivers/platform/x86/toshiba_acpi.c           |  2 +-
 drivers/video/backlight/backlight.c           | 18 +++++++
 include/acpi/video.h                          | 21 ++++----
 include/linux/backlight.h                     |  1 +
 25 files changed, 85 insertions(+), 66 deletions(-)

-- 
2.37.3

[PATCH 01/22] drm/i915/opregion: Improve backlight request condition

[Akihiko Odaki]

asle_set_backlight() needs to accept backlight request only if the
firmware controls the backlight. It used the following expression for
this purpose:
acpi_video_get_backlight_type() == acpi_backlight_native

This expression works well in practice, but has two semantic problems.
One is that it actually determines if a backlight device which directly
modifies hardware registers ("native backlight") exists. It is possible
that a device which does not have backlight at all incorrectly triggers
asle_set_backlight(), and the expression does not cover such a case.

Another problem is that acpi_video_get_backlight_type() always return
acpi_backlight_vendor in reality if CONFIG_ACPI_VIDEO is unset. It
means even its ability to determine the existence of native backlight is
somewhat limited.

This change introduces a new function backlight_device_non_raw_exists(),
which returns if the firmware is controlling the backlight, and is
always available if backlight support is enabled.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/gpu/drm/i915/display/intel_opregion.c |  3 ++-
 drivers/video/backlight/backlight.c           | 18 ++++++++++++++++++
 include/linux/backlight.h                     |  1 +
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
index caa07ef34f21..82ea02ab94c2 100644
--- a/drivers/gpu/drm/i915/display/intel_opregion.c
+++ b/drivers/gpu/drm/i915/display/intel_opregion.c
@@ -26,6 +26,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/backlight.h>
 #include <linux/dmi.h>
 #include <linux/firmware.h>
 #include <acpi/video.h>
@@ -467,7 +468,7 @@ static u32 asle_set_backlight(struct drm_i915_private *dev_priv, u32 bclp)
 
 	drm_dbg(&dev_priv->drm, "bclp = 0x%08x\n", bclp);
 
-	if (acpi_video_get_backlight_type() == acpi_backlight_native) {
+	if (!backlight_device_non_raw_exists()) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "opregion backlight request ignored\n");
 		return 0;
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index b788ff3d0f45..4f0ce463e250 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -516,6 +516,24 @@ struct backlight_device *backlight_device_get_by_name(const char *name)
 }
 EXPORT_SYMBOL(backlight_device_get_by_name);
 
+bool backlight_device_non_raw_exists(void)
+{
+	bool found = false;
+	struct backlight_device *bd;
+
+	mutex_lock(&backlight_dev_list_mutex);
+	list_for_each_entry(bd, &backlight_dev_list, entry) {
+		if (bd->props.type != BACKLIGHT_RAW) {
+			found = true;
+			break;
+		}
+	}
+	mutex_unlock(&backlight_dev_list_mutex);
+
+	return found;
+}
+EXPORT_SYMBOL(backlight_device_non_raw_exists);
+
 /* deprecated - use devm_backlight_device_unregister() */
 void backlight_device_unregister(struct backlight_device *bd)
 {
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 614653e07e3a..364ef6f99a9e 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -444,6 +444,7 @@ int backlight_register_notifier(struct notifier_block *nb);
 int backlight_unregister_notifier(struct notifier_block *nb);
 struct backlight_device *backlight_device_get_by_name(const char *name);
 struct backlight_device *backlight_device_get_by_type(enum backlight_type type);
+bool backlight_device_non_raw_exists(void);
 int backlight_device_set_brightness(struct backlight_device *bd,
 				    unsigned long brightness);
 
-- 
2.37.3

[PATCH 02/22] ACPI: video: Introduce acpi_video_get_backlight_types()

[Akihiko Odaki]

This deprecates acpi_video_get_backlight_type().

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/acpi/acpi_video.c   |  2 +-
 drivers/acpi/video_detect.c |  6 +++---
 include/acpi/video.h        | 12 ++++++++++++
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 32953646caeb..63e41aac71bd 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -1823,7 +1823,7 @@ static int acpi_video_bus_register_backlight(struct acpi_video_bus *video)
 	if (video->backlight_registered)
 		return 0;
 
-	if (acpi_video_get_backlight_type() != acpi_backlight_video)
+	if (!(acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VIDEO))
 		return 0;
 
 	mutex_lock(&video->device_list_lock);
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 0d9064a9804c..9cb12e4f06f7 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -17,13 +17,13 @@
  * Otherwise vendor specific drivers like thinkpad_acpi, asus-laptop,
  * sony_acpi,... can take care about backlight brightness.
  *
- * Backlight drivers can use acpi_video_get_backlight_type() to determine which
+ * Backlight drivers can use acpi_video_get_backlight_types() to determine which
  * driver should handle the backlight. RAW/GPU-driver backlight drivers must
  * use the acpi_video_backlight_use_native() helper for this.
  *
  * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m)
- * this file will not be compiled and acpi_video_get_backlight_type() will
- * always return acpi_backlight_vendor.
+ * this file will not be compiled and acpi_video_get_backlight_types() will
+ * always return ACPI_BACKLIGHT_VENDOR.
  */
 
 #include <linux/export.h>
diff --git a/include/acpi/video.h b/include/acpi/video.h
index a275c35e5249..a565ba27fae0 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -2,6 +2,7 @@
 #ifndef __ACPI_VIDEO_H
 #define __ACPI_VIDEO_H
 
+#include <linux/bits.h> /* for BIT */
 #include <linux/errno.h> /* for ENODEV */
 #include <linux/types.h> /* for bool */
 
@@ -52,6 +53,12 @@ enum acpi_backlight_type {
 	acpi_backlight_apple_gmux,
 };
 
+#define ACPI_BACKLIGHT_VIDEO BIT(acpi_backlight_video)
+#define ACPI_BACKLIGHT_VENDOR BIT(acpi_backlight_vendor)
+#define ACPI_BACKLIGHT_NATIVE BIT(acpi_backlight_native)
+#define ACPI_BACKLIGHT_NVIDIA_WMI_EC BIT(acpi_backlight_nvidia_wmi_ec)
+#define ACPI_BACKLIGHT_APPLE_GMUX BIT(acpi_backlight_apple_gmux)
+
 #if IS_ENABLED(CONFIG_ACPI_VIDEO)
 extern int acpi_video_register(void);
 extern void acpi_video_unregister(void);
@@ -97,4 +104,9 @@ static inline int acpi_video_get_levels(struct acpi_device *device,
 }
 #endif
 
+static inline int acpi_video_get_backlight_types(void)
+{
+	return BIT(acpi_video_get_backlight_type());
+}
+
 #endif
-- 
2.37.3

[PATCH 03/22] LoongArch: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/loongarch/loongson-laptop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/loongarch/loongson-laptop.c b/drivers/platform/loongarch/loongson-laptop.c
index f0166ad5d2c2..3b54b4cefedb 100644
--- a/drivers/platform/loongarch/loongson-laptop.c
+++ b/drivers/platform/loongarch/loongson-laptop.c
@@ -454,9 +454,9 @@ static int __init event_init(struct generic_sub_driver *sub_driver)
 
 	/*
 	 * This hotkey driver handle backlight event when
-	 * acpi_video_get_backlight_type() gets acpi_backlight_vendor
+	 * acpi_video_get_backlight_types() gets ACPI_BACKLIGHT_VENDOR
 	 */
-	if (acpi_video_get_backlight_type() == acpi_backlight_vendor)
+	if ((acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR))
 		hotkey_backlight_set(true);
 	else
 		hotkey_backlight_set(false);
-- 
2.37.3

[PATCH 04/22] platform/x86: acer-wmi: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/acer-wmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 18224f9a5bc0..10d0819e48ff 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -2421,7 +2421,7 @@ static int __init acer_wmi_init(void)
 
 	set_quirks();
 
-	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
+	if (!(acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR))
 		interface->capability &= ~ACER_CAP_BRIGHTNESS;
 
 	if (wmi_has_guid(WMID_GUID3))
-- 
2.37.3

[PATCH 05/22] platform/x86: asus-laptop: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/asus-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index 47b2f8bb6fb5..fb7f9e8ca6c4 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -1854,7 +1854,7 @@ static int asus_acpi_add(struct acpi_device *device)
 	if (result)
 		goto fail_platform;
 
-	if (acpi_video_get_backlight_type() == acpi_backlight_vendor) {
+	if ((acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
 		result = asus_backlight_init(asus);
 		if (result)
 			goto fail_backlight;
-- 
2.37.3

[PATCH 06/22] platform/x86: asus-wmi: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/asus-wmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 6e8e093f96b3..55cca068f783 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -3259,7 +3259,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 		code = ASUS_WMI_BRN_DOWN;
 
 	if (code == ASUS_WMI_BRN_DOWN || code == ASUS_WMI_BRN_UP) {
-		if (acpi_video_get_backlight_type() == acpi_backlight_vendor) {
+		if ((acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
 			asus_wmi_backlight_notify(asus, orig_code);
 			return;
 		}
@@ -3802,7 +3802,7 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (asus->driver->quirks->xusb2pr)
 		asus_wmi_set_xusb2pr(asus);
 
-	if (acpi_video_get_backlight_type() == acpi_backlight_vendor) {
+	if ((acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
 		err = asus_wmi_backlight_init(asus);
 		if (err && err != -ENODEV)
 			goto fail_backlight;
-- 
2.37.3

[PATCH 07/22] platform/x86: compal-laptop: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/compal-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/compal-laptop.c b/drivers/platform/x86/compal-laptop.c
index e10d2f64dfad..16ced03fa1b7 100644
--- a/drivers/platform/x86/compal-laptop.c
+++ b/drivers/platform/x86/compal-laptop.c
@@ -1043,7 +1043,7 @@ static int __init compal_init(void)
 		return -ENODEV;
 	}
 
-	if (acpi_video_get_backlight_type() == acpi_backlight_vendor) {
+	if ((acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
 		struct backlight_properties props;
 		memset(&props, 0, sizeof(struct backlight_properties));
 		props.type = BACKLIGHT_PLATFORM;
-- 
2.37.3

[PATCH 08/22] platform/x86: eeepc-laptop: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/eeepc-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index a388a28b6f2a..91dea49f7c3d 100644
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -1400,7 +1400,7 @@ static int eeepc_acpi_add(struct acpi_device *device)
 	if (result)
 		goto fail_platform;
 
-	if (acpi_video_get_backlight_type() == acpi_backlight_vendor) {
+	if ((acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
 		result = eeepc_backlight_init(eeepc);
 		if (result)
 			goto fail_backlight;
-- 
2.37.3

[PATCH 09/22] platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/fujitsu-laptop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index b543d117b12c..e820de39cb68 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -387,7 +387,7 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
 	struct fujitsu_bl *priv;
 	int ret;
 
-	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
+	if (!(acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR))
 		return -ENODEV;
 
 	priv = devm_kzalloc(&device->dev, sizeof(*priv), GFP_KERNEL);
@@ -819,7 +819,7 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
 
 	/* Sync backlight power status */
 	if (fujitsu_bl && fujitsu_bl->bl_device &&
-	    acpi_video_get_backlight_type() == acpi_backlight_vendor) {
+	    (acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
 		if (call_fext_func(fext, FUNC_BACKLIGHT, 0x2,
 				   BACKLIGHT_PARAM_POWER, 0x0) == BACKLIGHT_OFF)
 			fujitsu_bl->bl_device->props.power = FB_BLANK_POWERDOWN;
-- 
2.37.3

Re: [PATCH 09/22] platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()

[Jonathan Woithe]

On Mon, Oct 24, 2022 at 08:35:00PM +0900, Akihiko Odaki wrote:
> acpi_video_get_backlight_type() is now deprecated.

The practical impact of this patch series on fujitsu-laptop is obviously
very minor assuming the new acpi_video_get_backlight_types() function
functions as advertised.  Accordingly, as maintainer of fujitsu-laptop I
will defer to the opinions of others who maintain the lower level
infrastructure which is more substantially affected by the bulk of the
changes in this series.

I note that Hans has naked the series and I'm happy to go along with that.

Regards
  jonathan

> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  drivers/platform/x86/fujitsu-laptop.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> index b543d117b12c..e820de39cb68 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -387,7 +387,7 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
>  	struct fujitsu_bl *priv;
>  	int ret;
>  
> -	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
> +	if (!(acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR))
>  		return -ENODEV;
>  
>  	priv = devm_kzalloc(&device->dev, sizeof(*priv), GFP_KERNEL);
> @@ -819,7 +819,7 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
>  
>  	/* Sync backlight power status */
>  	if (fujitsu_bl && fujitsu_bl->bl_device &&
> -	    acpi_video_get_backlight_type() == acpi_backlight_vendor) {
> +	    (acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
>  		if (call_fext_func(fext, FUNC_BACKLIGHT, 0x2,
>  				   BACKLIGHT_PARAM_POWER, 0x0) == BACKLIGHT_OFF)
>  			fujitsu_bl->bl_device->props.power = FB_BLANK_POWERDOWN;
> -- 
> 2.37.3

[PATCH 10/22] platform/x86: ideapad-laptop: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/ideapad-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index abd0c81d62c4..befd5dc24ed6 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1633,7 +1633,7 @@ static int ideapad_acpi_add(struct platform_device *pdev)
 			dev_info(&pdev->dev, "DYTC interface is not available\n");
 	}
 
-	if (acpi_video_get_backlight_type() == acpi_backlight_vendor) {
+	if ((acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
 		err = ideapad_backlight_init(priv);
 		if (err && err != -ENODEV)
 			goto backlight_failed;
-- 
2.37.3

[PATCH 11/22] platform/x86: msi-laptop: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/msi-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
index 6b18ec543ac3..19e2f164181f 100644
--- a/drivers/platform/x86/msi-laptop.c
+++ b/drivers/platform/x86/msi-laptop.c
@@ -1048,7 +1048,7 @@ static int __init msi_init(void)
 
 	/* Register backlight stuff */
 	if (quirks->old_ec_model &&
-	    acpi_video_get_backlight_type() == acpi_backlight_vendor) {
+	    (acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
 		struct backlight_properties props;
 		memset(&props, 0, sizeof(struct backlight_properties));
 		props.type = BACKLIGHT_PLATFORM;
-- 
2.37.3

[PATCH 12/22] platform/x86: msi-wmi: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/msi-wmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/msi-wmi.c b/drivers/platform/x86/msi-wmi.c
index fd318cdfe313..3a74f0135fae 100644
--- a/drivers/platform/x86/msi-wmi.c
+++ b/drivers/platform/x86/msi-wmi.c
@@ -309,7 +309,7 @@ static int __init msi_wmi_init(void)
 	}
 
 	if (wmi_has_guid(MSIWMI_BIOS_GUID) &&
-	    acpi_video_get_backlight_type() == acpi_backlight_vendor) {
+	    (acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
 		err = msi_wmi_backlight_setup();
 		if (err) {
 			pr_err("Unable to setup backlight device\n");
-- 
2.37.3

[PATCH 13/22] platform/x86: nvidia-wmi-ec-backlight: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/nvidia-wmi-ec-backlight.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/nvidia-wmi-ec-backlight.c b/drivers/platform/x86/nvidia-wmi-ec-backlight.c
index baccdf658538..a42e3914c73d 100644
--- a/drivers/platform/x86/nvidia-wmi-ec-backlight.c
+++ b/drivers/platform/x86/nvidia-wmi-ec-backlight.c
@@ -91,7 +91,7 @@ static int nvidia_wmi_ec_backlight_probe(struct wmi_device *wdev, const void *ct
 	int ret;
 
 	/* drivers/acpi/video_detect.c also checks that SOURCE == EC */
-	if (acpi_video_get_backlight_type() != acpi_backlight_nvidia_wmi_ec)
+	if (!(acpi_video_get_backlight_types() & ACPI_BACKLIGHT_NVIDIA_WMI_EC))
 		return -ENODEV;
 
 	/*
-- 
2.37.3

[PATCH 14/22] platform/x86: panasonic-laptop: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/panasonic-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index ad3083f9946d..eaf9469fabb3 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -999,7 +999,7 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
 		goto out_input;
 	}
 
-	if (acpi_video_get_backlight_type() == acpi_backlight_vendor) {
+	if ((acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
 		/* initialize backlight */
 		memset(&props, 0, sizeof(struct backlight_properties));
 		props.type = BACKLIGHT_PLATFORM;
-- 
2.37.3

[PATCH 15/22] platform/x86: samsung-laptop: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/samsung-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index b4aa8ba35d2d..8ba377df73e8 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -1574,7 +1574,7 @@ static int __init samsung_init(void)
 	samsung->handle_backlight = true;
 	samsung->quirks = quirks;
 
-	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
+	if (!(acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR))
 		samsung->handle_backlight = false;
 
 	ret = samsung_platform_init(samsung);
-- 
2.37.3

[PATCH 16/22] platform/x86: sony-laptop: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/sony-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index 765fcaba4d12..987942719ba6 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -3201,7 +3201,7 @@ static int sony_nc_add(struct acpi_device *device)
 			sony_nc_function_setup(device, sony_pf_device);
 	}
 
-	if (acpi_video_get_backlight_type() == acpi_backlight_vendor)
+	if ((acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR))
 		sony_nc_backlight_setup();
 
 	/* create sony_pf sysfs attributes related to the SNC device */
-- 
2.37.3

[PATCH 17/22] platform/x86: thinkpad_acpi: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 6a823b850a77..d756d2ce54d1 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -3547,7 +3547,7 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 	/* Do not issue duplicate brightness change events to
 	 * userspace. tpacpi_detect_brightness_capabilities() must have
 	 * been called before this point  */
-	if (acpi_video_get_backlight_type() != acpi_backlight_vendor) {
+	if (!(acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
 		pr_info("This ThinkPad has standard ACPI backlight brightness control, supported by the ACPI video driver\n");
 		pr_notice("Disabling thinkpad-acpi brightness events by default...\n");
 
@@ -6983,7 +6983,7 @@ static int __init brightness_init(struct ibm_init_struct *iibm)
 		return -ENODEV;
 	}
 
-	if (acpi_video_get_backlight_type() != acpi_backlight_vendor) {
+	if (!(acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
 		if (brightness_enable > 1) {
 			pr_info("Standard ACPI backlight interface available, not loading native one\n");
 			return -ENODEV;
-- 
2.37.3

[PATCH 18/22] platform/x86: toshiba_acpi: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/toshiba_acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 160abd3b3af8..3d747a901ad8 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -2978,7 +2978,7 @@ static int toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
 		return 0;
 	}
 
-	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
+	if (!(acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR))
 		return 0;
 
 	memset(&props, 0, sizeof(props));
-- 
2.37.3

[PATCH 19/22] platform/x86: dell-laptop: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/dell/dell-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c
index e92c3ad06d69..9cfffbe2bd95 100644
--- a/drivers/platform/x86/dell/dell-laptop.c
+++ b/drivers/platform/x86/dell/dell-laptop.c
@@ -2230,7 +2230,7 @@ static int __init dell_init(void)
 		micmute_led_registered = true;
 	}
 
-	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
+	if (!(acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR))
 		return 0;
 
 	token = dell_smbios_find_token(BRIGHTNESS_TOKEN);
-- 
2.37.3

[PATCH 20/22] platform/x86: intel_oaktrail: Use acpi_video_get_backlight_types()

[Akihiko Odaki]

acpi_video_get_backlight_type() is now deprecated.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/platform/x86/intel/oaktrail.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/oaktrail.c b/drivers/platform/x86/intel/oaktrail.c
index 7c5c623630c1..f44a51dad16a 100644
--- a/drivers/platform/x86/intel/oaktrail.c
+++ b/drivers/platform/x86/intel/oaktrail.c
@@ -330,7 +330,7 @@ static int __init oaktrail_init(void)
 		goto err_device_add;
 	}
 
-	if (acpi_video_get_backlight_type() == acpi_backlight_vendor) {
+	if ((acpi_video_get_backlight_types() & ACPI_BACKLIGHT_VENDOR)) {
 		ret = oaktrail_backlight_init();
 		if (ret)
 			goto err_backlight;
-- 
2.37.3

[PATCH 21/22] ACPI: video: Remove acpi_video_get_backlight_type()

[Akihiko Odaki]

acpi_video_get_backlight_type() is deprecated and now there is
no user of it.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 Documentation/gpu/todo.rst  |  8 +++----
 drivers/acpi/video_detect.c | 46 +++++++++++++++++++------------------
 include/acpi/video.h        | 31 +++++++------------------
 3 files changed, 36 insertions(+), 49 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 393d218e4a0c..39e2ff2beb29 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -687,7 +687,7 @@ On x86/ACPI devices there can be multiple backlight firmware interfaces:
 register programming by the KMS driver.
 
 To deal with this backlight drivers used on x86/ACPI call
-acpi_video_get_backlight_type() which has heuristics (+quirks) to select
+acpi_video_get_backlight_types() which has heuristics (+quirks) to select
 which backlight interface to use; and backlight drivers which do not match
 the returned type will not register themselves, so that only one backlight
 device gets registered (in a single GPU setup, see below).
@@ -696,7 +696,7 @@ At the moment this more or less assumes that there will only
 be 1 (internal) panel on a system.
 
 On systems with 2 panels this may be a problem, depending on
-what interface acpi_video_get_backlight_type() selects:
+what interface acpi_video_get_backlight_types() selects:
 
 1. native: in this case the KMS driver is expected to know which backlight
    device belongs to which output so everything should just work.
@@ -708,11 +708,11 @@ Things will break on systems with multiple panels where the 2 panels need
 a different type of control. E.g. one panel needs ACPI video backlight control,
 where as the other is using native backlight control. Currently in this case
 only one of the 2 required backlight devices will get registered, based on
-the acpi_video_get_backlight_type() return value.
+the acpi_video_get_backlight_types() return value.
 
 If this (theoretical) case ever shows up, then supporting this will need some
 work. A possible solution here would be to pass a device and connector-name
-to acpi_video_get_backlight_type() so that it can deal with this.
+to acpi_video_get_backlight_types() so that it can deal with this.
 
 Note in a way we already have a case where userspace sees 2 panels,
 in dual GPU laptop setups with a mux. On those systems we may see
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 9cb12e4f06f7..88462f2fb8cc 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -38,19 +38,21 @@
 #include <linux/workqueue.h>
 #include <acpi/video.h>
 
-static enum acpi_backlight_type acpi_backlight_cmdline = acpi_backlight_undef;
-static enum acpi_backlight_type acpi_backlight_dmi = acpi_backlight_undef;
+#define ACPI_BACKLIGHT_UNDEF (-1)
+
+static int acpi_backlight_cmdline = ACPI_BACKLIGHT_UNDEF;
+static int acpi_backlight_dmi = ACPI_BACKLIGHT_UNDEF;
 
 static void acpi_video_parse_cmdline(void)
 {
 	if (!strcmp("vendor", acpi_video_backlight_string))
-		acpi_backlight_cmdline = acpi_backlight_vendor;
+		acpi_backlight_cmdline = ACPI_BACKLIGHT_VENDOR;
 	if (!strcmp("video", acpi_video_backlight_string))
-		acpi_backlight_cmdline = acpi_backlight_video;
+		acpi_backlight_cmdline = ACPI_BACKLIGHT_VIDEO;
 	if (!strcmp("native", acpi_video_backlight_string))
-		acpi_backlight_cmdline = acpi_backlight_native;
+		acpi_backlight_cmdline = ACPI_BACKLIGHT_NATIVE;
 	if (!strcmp("none", acpi_video_backlight_string))
-		acpi_backlight_cmdline = acpi_backlight_none;
+		acpi_backlight_cmdline = 0;
 }
 
 static acpi_status
@@ -109,25 +111,25 @@ static bool nvidia_wmi_ec_supported(void)
  * buggy */
 static int video_detect_force_vendor(const struct dmi_system_id *d)
 {
-	acpi_backlight_dmi = acpi_backlight_vendor;
+	acpi_backlight_dmi = ACPI_BACKLIGHT_VENDOR;
 	return 0;
 }
 
 static int video_detect_force_video(const struct dmi_system_id *d)
 {
-	acpi_backlight_dmi = acpi_backlight_video;
+	acpi_backlight_dmi = ACPI_BACKLIGHT_VIDEO;
 	return 0;
 }
 
 static int video_detect_force_native(const struct dmi_system_id *d)
 {
-	acpi_backlight_dmi = acpi_backlight_native;
+	acpi_backlight_dmi = ACPI_BACKLIGHT_NATIVE;
 	return 0;
 }
 
 static int video_detect_force_none(const struct dmi_system_id *d)
 {
-	acpi_backlight_dmi = acpi_backlight_none;
+	acpi_backlight_dmi = 0;
 	return 0;
 }
 
@@ -672,7 +674,7 @@ static const struct dmi_system_id video_detect_dmi_table[] = {
  * Determine which type of backlight interface to use on this system,
  * First check cmdline, then dmi quirks, then do autodetect.
  */
-static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
+static int __acpi_video_get_backlight_types(bool native)
 {
 	static DEFINE_MUTEX(init_mutex);
 	static bool nvidia_wmi_ec_present;
@@ -699,19 +701,19 @@ static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
 	 * The below heuristics / detection steps are in order of descending
 	 * presedence. The commandline takes presedence over anything else.
 	 */
-	if (acpi_backlight_cmdline != acpi_backlight_undef)
+	if (acpi_backlight_cmdline != ACPI_BACKLIGHT_UNDEF)
 		return acpi_backlight_cmdline;
 
 	/* DMI quirks override any autodetection. */
-	if (acpi_backlight_dmi != acpi_backlight_undef)
+	if (acpi_backlight_dmi != ACPI_BACKLIGHT_UNDEF)
 		return acpi_backlight_dmi;
 
 	/* Special cases such as nvidia_wmi_ec and apple gmux. */
 	if (nvidia_wmi_ec_present)
-		return acpi_backlight_nvidia_wmi_ec;
+		return ACPI_BACKLIGHT_NVIDIA_WMI_EC;
 
 	if (apple_gmux_present())
-		return acpi_backlight_apple_gmux;
+		return ACPI_BACKLIGHT_APPLE_GMUX;
 
 	/* On systems with ACPI video use either native or ACPI video. */
 	if (video_caps & ACPI_VIDEO_BACKLIGHT) {
@@ -725,23 +727,23 @@ static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
 		 * is usually not the best choice.
 		 */
 		if (acpi_osi_is_win8() && native_available)
-			return acpi_backlight_native;
+			return ACPI_BACKLIGHT_NATIVE;
 		else
-			return acpi_backlight_video;
+			return ACPI_BACKLIGHT_VIDEO;
 	}
 
 	/* No ACPI video (old hw), use vendor specific fw methods. */
-	return acpi_backlight_vendor;
+	return ACPI_BACKLIGHT_VENDOR;
 }
 
-enum acpi_backlight_type acpi_video_get_backlight_type(void)
+int acpi_video_get_backlight_types(void)
 {
-	return __acpi_video_get_backlight_type(false);
+	return __acpi_video_get_backlight_types(false);
 }
-EXPORT_SYMBOL(acpi_video_get_backlight_type);
+EXPORT_SYMBOL(acpi_video_get_backlight_types);
 
 bool acpi_video_backlight_use_native(void)
 {
-	return __acpi_video_get_backlight_type(true) == acpi_backlight_native;
+	return !!(__acpi_video_get_backlight_types(true) & ACPI_BACKLIGHT_NATIVE);
 }
 EXPORT_SYMBOL(acpi_video_backlight_use_native);
diff --git a/include/acpi/video.h b/include/acpi/video.h
index a565ba27fae0..5b748fdb606e 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -43,21 +43,11 @@ struct acpi_device;
 #define ACPI_VIDEO_NOTIFY_ZERO_BRIGHTNESS	0x88
 #define ACPI_VIDEO_NOTIFY_DISPLAY_OFF		0x89
 
-enum acpi_backlight_type {
-	acpi_backlight_undef = -1,
-	acpi_backlight_none = 0,
-	acpi_backlight_video,
-	acpi_backlight_vendor,
-	acpi_backlight_native,
-	acpi_backlight_nvidia_wmi_ec,
-	acpi_backlight_apple_gmux,
-};
-
-#define ACPI_BACKLIGHT_VIDEO BIT(acpi_backlight_video)
-#define ACPI_BACKLIGHT_VENDOR BIT(acpi_backlight_vendor)
-#define ACPI_BACKLIGHT_NATIVE BIT(acpi_backlight_native)
-#define ACPI_BACKLIGHT_NVIDIA_WMI_EC BIT(acpi_backlight_nvidia_wmi_ec)
-#define ACPI_BACKLIGHT_APPLE_GMUX BIT(acpi_backlight_apple_gmux)
+#define ACPI_BACKLIGHT_VIDEO BIT(0)
+#define ACPI_BACKLIGHT_VENDOR BIT(1)
+#define ACPI_BACKLIGHT_NATIVE BIT(2)
+#define ACPI_BACKLIGHT_NVIDIA_WMI_EC BIT(3)
+#define ACPI_BACKLIGHT_APPLE_GMUX BIT(4)
 
 #if IS_ENABLED(CONFIG_ACPI_VIDEO)
 extern int acpi_video_register(void);
@@ -65,7 +55,7 @@ extern void acpi_video_unregister(void);
 extern void acpi_video_register_backlight(void);
 extern int acpi_video_get_edid(struct acpi_device *device, int type,
 			       int device_id, void **edid);
-extern enum acpi_backlight_type acpi_video_get_backlight_type(void);
+extern int acpi_video_get_backlight_types(void);
 extern bool acpi_video_backlight_use_native(void);
 /*
  * Note: The value returned by acpi_video_handles_brightness_key_presses()
@@ -84,9 +74,9 @@ static inline int acpi_video_get_edid(struct acpi_device *device, int type,
 {
 	return -ENODEV;
 }
-static inline enum acpi_backlight_type acpi_video_get_backlight_type(void)
+static inline int acpi_video_get_backlight_types(void)
 {
-	return acpi_backlight_vendor;
+	return ACPI_BACKLIGHT_VENDOR;
 }
 static inline bool acpi_video_backlight_use_native(void)
 {
@@ -104,9 +94,4 @@ static inline int acpi_video_get_levels(struct acpi_device *device,
 }
 #endif
 
-static inline int acpi_video_get_backlight_types(void)
-{
-	return BIT(acpi_video_get_backlight_type());
-}
-
 #endif
-- 
2.37.3

[PATCH 22/22] ACPI: video: Fallback to native backlight

[Akihiko Odaki]

Commit 2600bfa3df99 ("ACPI: video: Add acpi_video_backlight_use_native()
helper") and following commits made native backlight unavailable if
CONFIG_ACPI_VIDEO is set and the backlight feature of ACPI video is
unavailable, which broke the backlight functionality on Lenovo ThinkPad
C13 Yoga Chromebook. Allow to fall back to native backlight in such
cases.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 drivers/acpi/video_detect.c | 4 ++--
 include/acpi/video.h        | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 88462f2fb8cc..36354241c740 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -732,8 +732,8 @@ static int __acpi_video_get_backlight_types(bool native)
 			return ACPI_BACKLIGHT_VIDEO;
 	}
 
-	/* No ACPI video (old hw), use vendor specific fw methods. */
-	return ACPI_BACKLIGHT_VENDOR;
+	/* No ACPI video, use native or vendor specific fw methods. */
+	return ACPI_BACKLIGHT_VENDOR | ACPI_BACKLIGHT_NATIVE;
 }
 
 int acpi_video_get_backlight_types(void)
diff --git a/include/acpi/video.h b/include/acpi/video.h
index 5b748fdb606e..656b59acfd1f 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -76,7 +76,7 @@ static inline int acpi_video_get_edid(struct acpi_device *device, int type,
 }
 static inline int acpi_video_get_backlight_types(void)
 {
-	return ACPI_BACKLIGHT_VENDOR;
+	return ACPI_BACKLIGHT_VENDOR | ACPI_BACKLIGHT_NATIVE;
 }
 static inline bool acpi_video_backlight_use_native(void)
 {
-- 
2.37.3

Re: [PATCH 00/22] Fallback to native backlight

[Jani Nikula]

On Mon, 24 Oct 2022, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> Commit 2600bfa3df99 ("ACPI: video: Add acpi_video_backlight_use_native()
> helper") and following commits made native backlight unavailable if
> CONFIG_ACPI_VIDEO is set and the backlight feature of ACPI video is
> unavailable, which broke the backlight functionality on Lenovo ThinkPad
> C13 Yoga Chromebook. Allow to fall back to native backlight in such
> cases.

Where's the bug report with relevant logs, kconfigs, etc?

BR,
Jani.

>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>
> Akihiko Odaki (22):
>   drm/i915/opregion: Improve backlight request condition
>   ACPI: video: Introduce acpi_video_get_backlight_types()
>   LoongArch: Use acpi_video_get_backlight_types()
>   platform/x86: acer-wmi: Use acpi_video_get_backlight_types()
>   platform/x86: asus-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: asus-wmi: Use acpi_video_get_backlight_types()
>   platform/x86: compal-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: eeepc-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: ideapad-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: msi-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: msi-wmi: Use acpi_video_get_backlight_types()
>   platform/x86: nvidia-wmi-ec-backlight: Use
>     acpi_video_get_backlight_types()
>   platform/x86: panasonic-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: samsung-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: sony-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: thinkpad_acpi: Use acpi_video_get_backlight_types()
>   platform/x86: toshiba_acpi: Use acpi_video_get_backlight_types()
>   platform/x86: dell-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: intel_oaktrail: Use acpi_video_get_backlight_types()
>   ACPI: video: Remove acpi_video_get_backlight_type()
>   ACPI: video: Fallback to native backlight
>
>  Documentation/gpu/todo.rst                    |  8 +--
>  drivers/acpi/acpi_video.c                     |  2 +-
>  drivers/acpi/video_detect.c                   | 54 ++++++++++---------
>  drivers/gpu/drm/i915/display/intel_opregion.c |  3 +-
>  drivers/platform/loongarch/loongson-laptop.c  |  4 +-
>  drivers/platform/x86/acer-wmi.c               |  2 +-
>  drivers/platform/x86/asus-laptop.c            |  2 +-
>  drivers/platform/x86/asus-wmi.c               |  4 +-
>  drivers/platform/x86/compal-laptop.c          |  2 +-
>  drivers/platform/x86/dell/dell-laptop.c       |  2 +-
>  drivers/platform/x86/eeepc-laptop.c           |  2 +-
>  drivers/platform/x86/fujitsu-laptop.c         |  4 +-
>  drivers/platform/x86/ideapad-laptop.c         |  2 +-
>  drivers/platform/x86/intel/oaktrail.c         |  2 +-
>  drivers/platform/x86/msi-laptop.c             |  2 +-
>  drivers/platform/x86/msi-wmi.c                |  2 +-
>  .../platform/x86/nvidia-wmi-ec-backlight.c    |  2 +-
>  drivers/platform/x86/panasonic-laptop.c       |  2 +-
>  drivers/platform/x86/samsung-laptop.c         |  2 +-
>  drivers/platform/x86/sony-laptop.c            |  2 +-
>  drivers/platform/x86/thinkpad_acpi.c          |  4 +-
>  drivers/platform/x86/toshiba_acpi.c           |  2 +-
>  drivers/video/backlight/backlight.c           | 18 +++++++
>  include/acpi/video.h                          | 21 ++++----
>  include/linux/backlight.h                     |  1 +
>  25 files changed, 85 insertions(+), 66 deletions(-)

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH 00/22] Fallback to native backlight

[Akihiko Odaki]

On 2022/10/24 20:48, Jani Nikula wrote:
> On Mon, 24 Oct 2022, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>> Commit 2600bfa3df99 ("ACPI: video: Add acpi_video_backlight_use_native()
>> helper") and following commits made native backlight unavailable if
>> CONFIG_ACPI_VIDEO is set and the backlight feature of ACPI video is
>> unavailable, which broke the backlight functionality on Lenovo ThinkPad
>> C13 Yoga Chromebook. Allow to fall back to native backlight in such
>> cases.
> 
> Where's the bug report with relevant logs, kconfigs, etc?

I haven't filed one. Should I? Please tell me where to report and what 
information you would need (to bugzilla.kernel.org with things mentioned 
in Documentation/admin-guide/reporting-issues.rst?)

Regards,
Akihiko Odaki

> 
> BR,
> Jani.
> 
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>
>> Akihiko Odaki (22):
>>    drm/i915/opregion: Improve backlight request condition
>>    ACPI: video: Introduce acpi_video_get_backlight_types()
>>    LoongArch: Use acpi_video_get_backlight_types()
>>    platform/x86: acer-wmi: Use acpi_video_get_backlight_types()
>>    platform/x86: asus-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: asus-wmi: Use acpi_video_get_backlight_types()
>>    platform/x86: compal-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: eeepc-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: ideapad-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: msi-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: msi-wmi: Use acpi_video_get_backlight_types()
>>    platform/x86: nvidia-wmi-ec-backlight: Use
>>      acpi_video_get_backlight_types()
>>    platform/x86: panasonic-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: samsung-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: sony-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: thinkpad_acpi: Use acpi_video_get_backlight_types()
>>    platform/x86: toshiba_acpi: Use acpi_video_get_backlight_types()
>>    platform/x86: dell-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: intel_oaktrail: Use acpi_video_get_backlight_types()
>>    ACPI: video: Remove acpi_video_get_backlight_type()
>>    ACPI: video: Fallback to native backlight
>>
>>   Documentation/gpu/todo.rst                    |  8 +--
>>   drivers/acpi/acpi_video.c                     |  2 +-
>>   drivers/acpi/video_detect.c                   | 54 ++++++++++---------
>>   drivers/gpu/drm/i915/display/intel_opregion.c |  3 +-
>>   drivers/platform/loongarch/loongson-laptop.c  |  4 +-
>>   drivers/platform/x86/acer-wmi.c               |  2 +-
>>   drivers/platform/x86/asus-laptop.c            |  2 +-
>>   drivers/platform/x86/asus-wmi.c               |  4 +-
>>   drivers/platform/x86/compal-laptop.c          |  2 +-
>>   drivers/platform/x86/dell/dell-laptop.c       |  2 +-
>>   drivers/platform/x86/eeepc-laptop.c           |  2 +-
>>   drivers/platform/x86/fujitsu-laptop.c         |  4 +-
>>   drivers/platform/x86/ideapad-laptop.c         |  2 +-
>>   drivers/platform/x86/intel/oaktrail.c         |  2 +-
>>   drivers/platform/x86/msi-laptop.c             |  2 +-
>>   drivers/platform/x86/msi-wmi.c                |  2 +-
>>   .../platform/x86/nvidia-wmi-ec-backlight.c    |  2 +-
>>   drivers/platform/x86/panasonic-laptop.c       |  2 +-
>>   drivers/platform/x86/samsung-laptop.c         |  2 +-
>>   drivers/platform/x86/sony-laptop.c            |  2 +-
>>   drivers/platform/x86/thinkpad_acpi.c          |  4 +-
>>   drivers/platform/x86/toshiba_acpi.c           |  2 +-
>>   drivers/video/backlight/backlight.c           | 18 +++++++
>>   include/acpi/video.h                          | 21 ++++----
>>   include/linux/backlight.h                     |  1 +
>>   25 files changed, 85 insertions(+), 66 deletions(-)
> 

Re: [PATCH 00/22] Fallback to native backlight

[Hans de Goede]

Hi,

On 10/24/22 13:56, Akihiko Odaki wrote:
> On 2022/10/24 20:48, Jani Nikula wrote:
>> On Mon, 24 Oct 2022, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>> Commit 2600bfa3df99 ("ACPI: video: Add acpi_video_backlight_use_native()
>>> helper") and following commits made native backlight unavailable if
>>> CONFIG_ACPI_VIDEO is set and the backlight feature of ACPI video is
>>> unavailable, which broke the backlight functionality on Lenovo ThinkPad
>>> C13 Yoga Chromebook. Allow to fall back to native backlight in such
>>> cases.
>>
>> Where's the bug report with relevant logs, kconfigs, etc?
> 
> I haven't filed one. Should I? Please tell me where to report and what information you would need (to bugzilla.kernel.org with things mentioned in Documentation/admin-guide/reporting-issues.rst?)

As mentioned in my other email this is a known issue, and your effort
to fix this is appreciated very much, but I don't believe your solution
to be the right one.

See: https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/
for more details and possible solutions. Please try implementing one of
those solutions for your Chromebook. I unfortunately do not have hw to
test this myself.

Regards,

Hans



> 
> Regards,
> Akihiko Odaki
> 
>>
>> BR,
>> Jani.
>>
>>>
>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>>
>>> Akihiko Odaki (22):
>>>    drm/i915/opregion: Improve backlight request condition
>>>    ACPI: video: Introduce acpi_video_get_backlight_types()
>>>    LoongArch: Use acpi_video_get_backlight_types()
>>>    platform/x86: acer-wmi: Use acpi_video_get_backlight_types()
>>>    platform/x86: asus-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: asus-wmi: Use acpi_video_get_backlight_types()
>>>    platform/x86: compal-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: eeepc-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: ideapad-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: msi-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: msi-wmi: Use acpi_video_get_backlight_types()
>>>    platform/x86: nvidia-wmi-ec-backlight: Use
>>>      acpi_video_get_backlight_types()
>>>    platform/x86: panasonic-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: samsung-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: sony-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: thinkpad_acpi: Use acpi_video_get_backlight_types()
>>>    platform/x86: toshiba_acpi: Use acpi_video_get_backlight_types()
>>>    platform/x86: dell-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: intel_oaktrail: Use acpi_video_get_backlight_types()
>>>    ACPI: video: Remove acpi_video_get_backlight_type()
>>>    ACPI: video: Fallback to native backlight
>>>
>>>   Documentation/gpu/todo.rst                    |  8 +--
>>>   drivers/acpi/acpi_video.c                     |  2 +-
>>>   drivers/acpi/video_detect.c                   | 54 ++++++++++---------
>>>   drivers/gpu/drm/i915/display/intel_opregion.c |  3 +-
>>>   drivers/platform/loongarch/loongson-laptop.c  |  4 +-
>>>   drivers/platform/x86/acer-wmi.c               |  2 +-
>>>   drivers/platform/x86/asus-laptop.c            |  2 +-
>>>   drivers/platform/x86/asus-wmi.c               |  4 +-
>>>   drivers/platform/x86/compal-laptop.c          |  2 +-
>>>   drivers/platform/x86/dell/dell-laptop.c       |  2 +-
>>>   drivers/platform/x86/eeepc-laptop.c           |  2 +-
>>>   drivers/platform/x86/fujitsu-laptop.c         |  4 +-
>>>   drivers/platform/x86/ideapad-laptop.c         |  2 +-
>>>   drivers/platform/x86/intel/oaktrail.c         |  2 +-
>>>   drivers/platform/x86/msi-laptop.c             |  2 +-
>>>   drivers/platform/x86/msi-wmi.c                |  2 +-
>>>   .../platform/x86/nvidia-wmi-ec-backlight.c    |  2 +-
>>>   drivers/platform/x86/panasonic-laptop.c       |  2 +-
>>>   drivers/platform/x86/samsung-laptop.c         |  2 +-
>>>   drivers/platform/x86/sony-laptop.c            |  2 +-
>>>   drivers/platform/x86/thinkpad_acpi.c          |  4 +-
>>>   drivers/platform/x86/toshiba_acpi.c           |  2 +-
>>>   drivers/video/backlight/backlight.c           | 18 +++++++
>>>   include/acpi/video.h                          | 21 ++++----
>>>   include/linux/backlight.h                     |  1 +
>>>   25 files changed, 85 insertions(+), 66 deletions(-)
>>
> 

Re: [PATCH 00/22] Fallback to native backlight

[Hans de Goede]

Hi Akihiko,

On 10/24/22 13:34, Akihiko Odaki wrote:
> Commit 2600bfa3df99 ("ACPI: video: Add acpi_video_backlight_use_native()
> helper") and following commits made native backlight unavailable if
> CONFIG_ACPI_VIDEO is set and the backlight feature of ACPI video is
> unavailable, which broke the backlight functionality on Lenovo ThinkPad
> C13 Yoga Chromebook. Allow to fall back to native backlight in such
> cases.

I appreciate your work on this, but what this in essence does is
it allows 2 backlight drivers (vendor + native) to get registered
for the same panel again. While the whole goal of the backlight refactor
series landing in 6.1 was to make it so that there always is only
*1* backlight device registered instead of (possibly) registering
multiple and letting userspace figure it out. It is also important
to only always have 1 backlight device per panel for further
upcoming changes.

So nack for this solution, sorry.

I am aware that this breaks backlight control on some Chromebooks,
this was already reported and I wrote a long reply explaining why
things are done the way they are done now and also suggesting
2 possible (much simpler) fixes, see:
https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/

Unfortunately the reported has not followed-up on this and
I don't have the hardware to test this myself.

Can you please try implementing 1 of the fixes suggested there
and then submit that upstream ?

Regards,

Hans









> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> 
> Akihiko Odaki (22):
>   drm/i915/opregion: Improve backlight request condition
>   ACPI: video: Introduce acpi_video_get_backlight_types()
>   LoongArch: Use acpi_video_get_backlight_types()
>   platform/x86: acer-wmi: Use acpi_video_get_backlight_types()
>   platform/x86: asus-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: asus-wmi: Use acpi_video_get_backlight_types()
>   platform/x86: compal-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: eeepc-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: ideapad-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: msi-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: msi-wmi: Use acpi_video_get_backlight_types()
>   platform/x86: nvidia-wmi-ec-backlight: Use
>     acpi_video_get_backlight_types()
>   platform/x86: panasonic-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: samsung-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: sony-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: thinkpad_acpi: Use acpi_video_get_backlight_types()
>   platform/x86: toshiba_acpi: Use acpi_video_get_backlight_types()
>   platform/x86: dell-laptop: Use acpi_video_get_backlight_types()
>   platform/x86: intel_oaktrail: Use acpi_video_get_backlight_types()
>   ACPI: video: Remove acpi_video_get_backlight_type()
>   ACPI: video: Fallback to native backlight
> 
>  Documentation/gpu/todo.rst                    |  8 +--
>  drivers/acpi/acpi_video.c                     |  2 +-
>  drivers/acpi/video_detect.c                   | 54 ++++++++++---------
>  drivers/gpu/drm/i915/display/intel_opregion.c |  3 +-
>  drivers/platform/loongarch/loongson-laptop.c  |  4 +-
>  drivers/platform/x86/acer-wmi.c               |  2 +-
>  drivers/platform/x86/asus-laptop.c            |  2 +-
>  drivers/platform/x86/asus-wmi.c               |  4 +-
>  drivers/platform/x86/compal-laptop.c          |  2 +-
>  drivers/platform/x86/dell/dell-laptop.c       |  2 +-
>  drivers/platform/x86/eeepc-laptop.c           |  2 +-
>  drivers/platform/x86/fujitsu-laptop.c         |  4 +-
>  drivers/platform/x86/ideapad-laptop.c         |  2 +-
>  drivers/platform/x86/intel/oaktrail.c         |  2 +-
>  drivers/platform/x86/msi-laptop.c             |  2 +-
>  drivers/platform/x86/msi-wmi.c                |  2 +-
>  .../platform/x86/nvidia-wmi-ec-backlight.c    |  2 +-
>  drivers/platform/x86/panasonic-laptop.c       |  2 +-
>  drivers/platform/x86/samsung-laptop.c         |  2 +-
>  drivers/platform/x86/sony-laptop.c            |  2 +-
>  drivers/platform/x86/thinkpad_acpi.c          |  4 +-
>  drivers/platform/x86/toshiba_acpi.c           |  2 +-
>  drivers/video/backlight/backlight.c           | 18 +++++++
>  include/acpi/video.h                          | 21 ++++----
>  include/linux/backlight.h                     |  1 +
>  25 files changed, 85 insertions(+), 66 deletions(-)
> 

Re: [PATCH 00/22] Fallback to native backlight

[Akihiko Odaki]

On 2022/10/24 20:53, Hans de Goede wrote:
> Hi Akihiko,
> 
> On 10/24/22 13:34, Akihiko Odaki wrote:
>> Commit 2600bfa3df99 ("ACPI: video: Add acpi_video_backlight_use_native()
>> helper") and following commits made native backlight unavailable if
>> CONFIG_ACPI_VIDEO is set and the backlight feature of ACPI video is
>> unavailable, which broke the backlight functionality on Lenovo ThinkPad
>> C13 Yoga Chromebook. Allow to fall back to native backlight in such
>> cases.
> 
> I appreciate your work on this, but what this in essence does is
> it allows 2 backlight drivers (vendor + native) to get registered
> for the same panel again. While the whole goal of the backlight refactor
> series landing in 6.1 was to make it so that there always is only
> *1* backlight device registered instead of (possibly) registering
> multiple and letting userspace figure it out. It is also important
> to only always have 1 backlight device per panel for further
> upcoming changes.
> 
> So nack for this solution, sorry.
> 
> I am aware that this breaks backlight control on some Chromebooks,
> this was already reported and I wrote a long reply explaining why
> things are done the way they are done now and also suggesting
> 2 possible (much simpler) fixes, see:
> https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/
> 
> Unfortunately the reported has not followed-up on this and
> I don't have the hardware to test this myself.
> 
> Can you please try implementing 1 of the fixes suggested there
> and then submit that upstream ?
> 
> Regards,
> 
> Hans
> 

Hi Hans,

Thanks for reviewing and letting me know the prior attempt.

In this case, there is only a native backlight device and no vendor 
backlight device so the duplication of backlight devices does not 
happen. I think it is better to handle such a case without quirks.

I understand it is still questionable to cover the case by allowing 
duplication when both of a vendor backlight device and native one. To 
explain my understanding and reasoning for *not* trying to apply the 
de-duplication rule to the vendor/native combination, let me first 
describe that the de-duplication which happens in 
acpi_video_get_backlight_type() is a heuristics and limited.

As the background of acpi_video_get_backlight_type(), there is an 
assumption that it should be common that both of the firmware, 
implementing ACPI, and the kernel have code to drive backlight. In the 
case, the more reliable one should be picked instead of using both, and 
that is what the statements in `if (video_caps & ACPI_VIDEO_BACKLIGHT)` 
does.

However, the method has two limitations:
1. It does not cover the case where two backlight devices with the same 
type exist.
2. The underlying assumption does not apply to vendor/native combination.

Regarding the second limitation, I don't even understand the difference 
between vendor and native. My guess is that a vendor backlight device 
uses vendor-specific ACPI interface, and a native one directly uses 
hardware registers. If my guess is correct, the difference between 
vendor and native does not imply that both of them are likely to exist 
at the same time. As the conclusion, there is no more motivation to try 
to de-duplicate the vendor/native combination than to try to 
de-duplicate combination of devices with a single type.

Of course, it is better if we could also avoid registering two devices 
with one type for one physical device. Possibly we can do so by 
providing a parameter to indicate that it is for the same "internal" 
backlight to devm_backlight_device_register(), and let the function 
check for the duplication. However, this rule may be too restrict, or 
may have problems I missed.

Based on the discussion above, we can deduce three possibilities:
a. There is no reason to distinguish vendor and native in this case, and 
we can stick to my current proposal.
b. There is a valid reason to distinguish vendor and native, and we can 
adopt the same strategy that already adopted for ACPI video/vendor 
combination.
c. We can implement de-duplication in devm_backlight_device_register().
d. The other possible options are not worth, and we can just implement 
quirks specific to Chromebook/coreboot.

In case b, it should be noted that vendor and native backlight device do 
not require ACPI video, and CONFIG_ACPI_VIDEO may not be enabled. In the 
case, the de-duplication needs to be implemented in backlight class device.

Regards,
Akihiko Odaki

> 
> 
> 
> 
> 
> 
> 
> 
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>
>> Akihiko Odaki (22):
>>    drm/i915/opregion: Improve backlight request condition
>>    ACPI: video: Introduce acpi_video_get_backlight_types()
>>    LoongArch: Use acpi_video_get_backlight_types()
>>    platform/x86: acer-wmi: Use acpi_video_get_backlight_types()
>>    platform/x86: asus-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: asus-wmi: Use acpi_video_get_backlight_types()
>>    platform/x86: compal-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: eeepc-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: ideapad-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: msi-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: msi-wmi: Use acpi_video_get_backlight_types()
>>    platform/x86: nvidia-wmi-ec-backlight: Use
>>      acpi_video_get_backlight_types()
>>    platform/x86: panasonic-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: samsung-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: sony-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: thinkpad_acpi: Use acpi_video_get_backlight_types()
>>    platform/x86: toshiba_acpi: Use acpi_video_get_backlight_types()
>>    platform/x86: dell-laptop: Use acpi_video_get_backlight_types()
>>    platform/x86: intel_oaktrail: Use acpi_video_get_backlight_types()
>>    ACPI: video: Remove acpi_video_get_backlight_type()
>>    ACPI: video: Fallback to native backlight
>>
>>   Documentation/gpu/todo.rst                    |  8 +--
>>   drivers/acpi/acpi_video.c                     |  2 +-
>>   drivers/acpi/video_detect.c                   | 54 ++++++++++---------
>>   drivers/gpu/drm/i915/display/intel_opregion.c |  3 +-
>>   drivers/platform/loongarch/loongson-laptop.c  |  4 +-
>>   drivers/platform/x86/acer-wmi.c               |  2 +-
>>   drivers/platform/x86/asus-laptop.c            |  2 +-
>>   drivers/platform/x86/asus-wmi.c               |  4 +-
>>   drivers/platform/x86/compal-laptop.c          |  2 +-
>>   drivers/platform/x86/dell/dell-laptop.c       |  2 +-
>>   drivers/platform/x86/eeepc-laptop.c           |  2 +-
>>   drivers/platform/x86/fujitsu-laptop.c         |  4 +-
>>   drivers/platform/x86/ideapad-laptop.c         |  2 +-
>>   drivers/platform/x86/intel/oaktrail.c         |  2 +-
>>   drivers/platform/x86/msi-laptop.c             |  2 +-
>>   drivers/platform/x86/msi-wmi.c                |  2 +-
>>   .../platform/x86/nvidia-wmi-ec-backlight.c    |  2 +-
>>   drivers/platform/x86/panasonic-laptop.c       |  2 +-
>>   drivers/platform/x86/samsung-laptop.c         |  2 +-
>>   drivers/platform/x86/sony-laptop.c            |  2 +-
>>   drivers/platform/x86/thinkpad_acpi.c          |  4 +-
>>   drivers/platform/x86/toshiba_acpi.c           |  2 +-
>>   drivers/video/backlight/backlight.c           | 18 +++++++
>>   include/acpi/video.h                          | 21 ++++----
>>   include/linux/backlight.h                     |  1 +
>>   25 files changed, 85 insertions(+), 66 deletions(-)
>>
> 

Re: [PATCH 00/22] Fallback to native backlight

[Pali Rohár]

On Monday 24 October 2022 21:58:57 Akihiko Odaki wrote:
> Regarding the second limitation, I don't even understand the difference
> between vendor and native. My guess is that a vendor backlight device uses
> vendor-specific ACPI interface, and a native one directly uses hardware
> registers. If my guess is correct, the difference between vendor and native
> does not imply that both of them are likely to exist at the same time. As
> the conclusion, there is no more motivation to try to de-duplicate the
> vendor/native combination than to try to de-duplicate combination of devices
> with a single type.

Hello! I just want to point one thing. On some Dell laptops there are
3 different ways (= 3 different APIs) how to control display backlight.
There is ACPI driver (uses ACPI), GPU/DRM driver (i915.ko; uses directly
HW) and platform vendor driver (dell-laptop.ko; uses vendor BIOS or
firmware API). Just every driver has different pre-calculated scaling
values. So sometimes user wants to choose different driver just because
it allows to set backlight level with "better" granularity. Registering
all 3 device drivers is bad as user does not want to see 3 display
panels and forcing registration of specific one without runtime option
is also bad (some of those drivers do not have to be suitable or has
worse granularity as other).

Re: [PATCH 00/22] Fallback to native backlight

[Hans de Goede]

Hi,

On 10/24/22 15:14, Pali Rohár wrote:
> On Monday 24 October 2022 21:58:57 Akihiko Odaki wrote:
>> Regarding the second limitation, I don't even understand the difference
>> between vendor and native. My guess is that a vendor backlight device uses
>> vendor-specific ACPI interface, and a native one directly uses hardware
>> registers. If my guess is correct, the difference between vendor and native
>> does not imply that both of them are likely to exist at the same time. As
>> the conclusion, there is no more motivation to try to de-duplicate the
>> vendor/native combination than to try to de-duplicate combination of devices
>> with a single type.
> 
> Hello! I just want to point one thing. On some Dell laptops there are
> 3 different ways (= 3 different APIs) how to control display backlight.
> There is ACPI driver (uses ACPI), GPU/DRM driver (i915.ko; uses directly
> HW) and platform vendor driver (dell-laptop.ko; uses vendor BIOS or
> firmware API).

Right and that is just one example of laptops which can register both
vendor + native backlight devices, which is why this whole series is
a bad idea.

Regards,

Hans



> Just every driver has different pre-calculated scaling
> values. So sometimes user wants to choose different driver just because
> it allows to set backlight level with "better" granularity. Registering
> all 3 device drivers is bad as user does not want to see 3 display
> panels and forcing registration of specific one without runtime option
> is also bad (some of those drivers do not have to be suitable or has
> worse granularity as other).

Re: [PATCH 00/22] Fallback to native backlight

[Hans de Goede]

Hi,

On 10/24/22 14:58, Akihiko Odaki wrote:
> On 2022/10/24 20:53, Hans de Goede wrote:
>> Hi Akihiko,
>>
>> On 10/24/22 13:34, Akihiko Odaki wrote:
>>> Commit 2600bfa3df99 ("ACPI: video: Add acpi_video_backlight_use_native()
>>> helper") and following commits made native backlight unavailable if
>>> CONFIG_ACPI_VIDEO is set and the backlight feature of ACPI video is
>>> unavailable, which broke the backlight functionality on Lenovo ThinkPad
>>> C13 Yoga Chromebook. Allow to fall back to native backlight in such
>>> cases.
>>
>> I appreciate your work on this, but what this in essence does is
>> it allows 2 backlight drivers (vendor + native) to get registered
>> for the same panel again. While the whole goal of the backlight refactor
>> series landing in 6.1 was to make it so that there always is only
>> *1* backlight device registered instead of (possibly) registering
>> multiple and letting userspace figure it out. It is also important
>> to only always have 1 backlight device per panel for further
>> upcoming changes.
>>
>> So nack for this solution, sorry.
>>
>> I am aware that this breaks backlight control on some Chromebooks,
>> this was already reported and I wrote a long reply explaining why
>> things are done the way they are done now and also suggesting
>> 2 possible (much simpler) fixes, see:
>> https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/
>>
>> Unfortunately the reported has not followed-up on this and
>> I don't have the hardware to test this myself.
>>
>> Can you please try implementing 1 of the fixes suggested there
>> and then submit that upstream ?
>>
>> Regards,
>>
>> Hans
>>
> 
> Hi Hans,
> 
> Thanks for reviewing and letting me know the prior attempt.
> 
> In this case, there is only a native backlight device and no vendor backlight device so the duplication of backlight devices does not happen. I think it is better to handle such a case without quirks.

Adding a single heuristic for all chromebooks is something completely different
then adding per model quirks which indeed ideally should be avoided (but this
is not always possible).

> I understand it is still questionable to cover the case by allowing duplication when both of a vendor backlight device and native one. To explain my understanding and reasoning for *not* trying to apply the de-duplication rule to the vendor/native combination, let me first describe that the de-duplication which happens in acpi_video_get_backlight_type() is a heuristics and limited.
> 
> As the background of acpi_video_get_backlight_type(), there is an assumption that it should be common that both of the firmware, implementing ACPI, and the kernel have code to drive backlight. In the case, the more reliable one should be picked instead of using both, and that is what the statements in `if (video_caps & ACPI_VIDEO_BACKLIGHT)` does.
> 
> However, the method has two limitations:
> 1. It does not cover the case where two backlight devices with the same type exist.

This only happens when there are 2 panels; or 2 gpus driving a single panel
which are both special cases where we actually want 2 backlight devices.

> 2. The underlying assumption does not apply to vendor/native combination.
> 
> Regarding the second limitation, I don't even understand the difference between vendor and native. My guess is that a vendor backlight device uses vendor-specific ACPI interface, and a native one directly uses hardware registers. If my guess is correct, the difference between vendor and native does not imply that both of them are likely to exist at the same time. As the conclusion, there is no more motivation to try to de-duplicate the vendor/native combination than to try to de-duplicate combination of devices with a single type.
> 
> Of course, it is better if we could also avoid registering two devices with one type for one physical device. Possibly we can do so by providing a parameter to indicate that it is for the same "internal" backlight to devm_backlight_device_register(), and let the function check for the duplication. However, this rule may be too restrict, or may have problems I missed.
> 
> Based on the discussion above, we can deduce three possibilities:
> a. There is no reason to distinguish vendor and native in this case, and we can stick to my current proposal.
> b. There is a valid reason to distinguish vendor and native, and we can adopt the same strategy that already adopted for ACPI video/vendor combination.
> c. We can implement de-duplication in devm_backlight_device_register().
> d. The other possible options are not worth, and we can just implement quirks specific to Chromebook/coreboot.
> 
> In case b, it should be noted that vendor and native backlight device do not require ACPI video, and CONFIG_ACPI_VIDEO may not be enabled. In the case, the de-duplication needs to be implemented in backlight class device.

I have been working on the ACPI/x86 backlight detection code since 2015, please trust
me when I say that allowing both vendor + native backlight devices at the same time
is a bad idea.

I'm currently in direct contact with the ChromeOS team about fixing the Chromebook
backlight issue introduced in 6.1-rc1.

If you wan to help, please read:

https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/

And try implementing 1 if the 2 solutions suggested there.

Regards,

Hans



>>> Akihiko Odaki (22):
>>>    drm/i915/opregion: Improve backlight request condition
>>>    ACPI: video: Introduce acpi_video_get_backlight_types()
>>>    LoongArch: Use acpi_video_get_backlight_types()
>>>    platform/x86: acer-wmi: Use acpi_video_get_backlight_types()
>>>    platform/x86: asus-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: asus-wmi: Use acpi_video_get_backlight_types()
>>>    platform/x86: compal-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: eeepc-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: ideapad-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: msi-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: msi-wmi: Use acpi_video_get_backlight_types()
>>>    platform/x86: nvidia-wmi-ec-backlight: Use
>>>      acpi_video_get_backlight_types()
>>>    platform/x86: panasonic-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: samsung-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: sony-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: thinkpad_acpi: Use acpi_video_get_backlight_types()
>>>    platform/x86: toshiba_acpi: Use acpi_video_get_backlight_types()
>>>    platform/x86: dell-laptop: Use acpi_video_get_backlight_types()
>>>    platform/x86: intel_oaktrail: Use acpi_video_get_backlight_types()
>>>    ACPI: video: Remove acpi_video_get_backlight_type()
>>>    ACPI: video: Fallback to native backlight
>>>
>>>   Documentation/gpu/todo.rst                    |  8 +--
>>>   drivers/acpi/acpi_video.c                     |  2 +-
>>>   drivers/acpi/video_detect.c                   | 54 ++++++++++---------
>>>   drivers/gpu/drm/i915/display/intel_opregion.c |  3 +-
>>>   drivers/platform/loongarch/loongson-laptop.c  |  4 +-
>>>   drivers/platform/x86/acer-wmi.c               |  2 +-
>>>   drivers/platform/x86/asus-laptop.c            |  2 +-
>>>   drivers/platform/x86/asus-wmi.c               |  4 +-
>>>   drivers/platform/x86/compal-laptop.c          |  2 +-
>>>   drivers/platform/x86/dell/dell-laptop.c       |  2 +-
>>>   drivers/platform/x86/eeepc-laptop.c           |  2 +-
>>>   drivers/platform/x86/fujitsu-laptop.c         |  4 +-
>>>   drivers/platform/x86/ideapad-laptop.c         |  2 +-
>>>   drivers/platform/x86/intel/oaktrail.c         |  2 +-
>>>   drivers/platform/x86/msi-laptop.c             |  2 +-
>>>   drivers/platform/x86/msi-wmi.c                |  2 +-
>>>   .../platform/x86/nvidia-wmi-ec-backlight.c    |  2 +-
>>>   drivers/platform/x86/panasonic-laptop.c       |  2 +-
>>>   drivers/platform/x86/samsung-laptop.c         |  2 +-
>>>   drivers/platform/x86/sony-laptop.c            |  2 +-
>>>   drivers/platform/x86/thinkpad_acpi.c          |  4 +-
>>>   drivers/platform/x86/toshiba_acpi.c           |  2 +-
>>>   drivers/video/backlight/backlight.c           | 18 +++++++
>>>   include/acpi/video.h                          | 21 ++++----
>>>   include/linux/backlight.h                     |  1 +
>>>   25 files changed, 85 insertions(+), 66 deletions(-)
>>>
>>
> 

Re: [PATCH 00/22] Fallback to native backlight

[Akihiko Odaki]

On 2022/10/24 22:21, Hans de Goede wrote:
> Hi,
> 
> On 10/24/22 14:58, Akihiko Odaki wrote:
>> On 2022/10/24 20:53, Hans de Goede wrote:
>>> Hi Akihiko,
>>>
>>> On 10/24/22 13:34, Akihiko Odaki wrote:
>>>> Commit 2600bfa3df99 ("ACPI: video: Add acpi_video_backlight_use_native()
>>>> helper") and following commits made native backlight unavailable if
>>>> CONFIG_ACPI_VIDEO is set and the backlight feature of ACPI video is
>>>> unavailable, which broke the backlight functionality on Lenovo ThinkPad
>>>> C13 Yoga Chromebook. Allow to fall back to native backlight in such
>>>> cases.
>>>
>>> I appreciate your work on this, but what this in essence does is
>>> it allows 2 backlight drivers (vendor + native) to get registered
>>> for the same panel again. While the whole goal of the backlight refactor
>>> series landing in 6.1 was to make it so that there always is only
>>> *1* backlight device registered instead of (possibly) registering
>>> multiple and letting userspace figure it out. It is also important
>>> to only always have 1 backlight device per panel for further
>>> upcoming changes.
>>>
>>> So nack for this solution, sorry.
>>>
>>> I am aware that this breaks backlight control on some Chromebooks,
>>> this was already reported and I wrote a long reply explaining why
>>> things are done the way they are done now and also suggesting
>>> 2 possible (much simpler) fixes, see:
>>> https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/
>>>
>>> Unfortunately the reported has not followed-up on this and
>>> I don't have the hardware to test this myself.
>>>
>>> Can you please try implementing 1 of the fixes suggested there
>>> and then submit that upstream ?
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>
>> Hi Hans,
>>
>> Thanks for reviewing and letting me know the prior attempt.
>>
>> In this case, there is only a native backlight device and no vendor backlight device so the duplication of backlight devices does not happen. I think it is better to handle such a case without quirks.
> 
> Adding a single heuristic for all chromebooks is something completely different
> then adding per model quirks which indeed ideally should be avoided (but this
> is not always possible).
> 
>> I understand it is still questionable to cover the case by allowing duplication when both of a vendor backlight device and native one. To explain my understanding and reasoning for *not* trying to apply the de-duplication rule to the vendor/native combination, let me first describe that the de-duplication which happens in acpi_video_get_backlight_type() is a heuristics and limited.
>>
>> As the background of acpi_video_get_backlight_type(), there is an assumption that it should be common that both of the firmware, implementing ACPI, and the kernel have code to drive backlight. In the case, the more reliable one should be picked instead of using both, and that is what the statements in `if (video_caps & ACPI_VIDEO_BACKLIGHT)` does.
>>
>> However, the method has two limitations:
>> 1. It does not cover the case where two backlight devices with the same type exist.
> 
> This only happens when there are 2 panels; or 2 gpus driving a single panel
> which are both special cases where we actually want 2 backlight devices.
> 
>> 2. The underlying assumption does not apply to vendor/native combination.
>>
>> Regarding the second limitation, I don't even understand the difference between vendor and native. My guess is that a vendor backlight device uses vendor-specific ACPI interface, and a native one directly uses hardware registers. If my guess is correct, the difference between vendor and native does not imply that both of them are likely to exist at the same time. As the conclusion, there is no more motivation to try to de-duplicate the vendor/native combination than to try to de-duplicate combination of devices with a single type.
>>
>> Of course, it is better if we could also avoid registering two devices with one type for one physical device. Possibly we can do so by providing a parameter to indicate that it is for the same "internal" backlight to devm_backlight_device_register(), and let the function check for the duplication. However, this rule may be too restrict, or may have problems I missed.
>>
>> Based on the discussion above, we can deduce three possibilities:
>> a. There is no reason to distinguish vendor and native in this case, and we can stick to my current proposal.
>> b. There is a valid reason to distinguish vendor and native, and we can adopt the same strategy that already adopted for ACPI video/vendor combination.
>> c. We can implement de-duplication in devm_backlight_device_register().
>> d. The other possible options are not worth, and we can just implement quirks specific to Chromebook/coreboot.
>>
>> In case b, it should be noted that vendor and native backlight device do not require ACPI video, and CONFIG_ACPI_VIDEO may not be enabled. In the case, the de-duplication needs to be implemented in backlight class device.
> 
> I have been working on the ACPI/x86 backlight detection code since 2015, please trust
> me when I say that allowing both vendor + native backlight devices at the same time
> is a bad idea.
> 
> I'm currently in direct contact with the ChromeOS team about fixing the Chromebook
> backlight issue introduced in 6.1-rc1.
> 
> If you wan to help, please read:
> 
> https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/
> 
> And try implementing 1 if the 2 solutions suggested there.
> 
> Regards,
> 
> Hans

Hi,

I just wanted to confirm your intention that we should distinguish 
vendor and native. In the case I think it is better to modify 
__acpi_video_get_backlight_type() and add "native_available" check in 
case of no ACPI video as already done for the ACPI video/native combination.

Unfortunately this has one pitfall though: it does not work if 
CONFIG_ACPI_VIDEO is disabled. If it is, acpi_video_get_backlight_type() 
always return acpi_backlight_vendor, and 
acpi_video_backlight_use_native() always returns true. It is not a 
regression but the current behavior. Fixing it requires also refactoring 
touching both of ACPI video and backlight class driver so I guess I'm 
not an appropriate person to do that, and I should just add 
"native_available" check to __acpi_video_get_backlight_type().

Does that sound good?

Regards,
Akihiko Odaki

>  >
> 
>>>> Akihiko Odaki (22):
>>>>     drm/i915/opregion: Improve backlight request condition
>>>>     ACPI: video: Introduce acpi_video_get_backlight_types()
>>>>     LoongArch: Use acpi_video_get_backlight_types()
>>>>     platform/x86: acer-wmi: Use acpi_video_get_backlight_types()
>>>>     platform/x86: asus-laptop: Use acpi_video_get_backlight_types()
>>>>     platform/x86: asus-wmi: Use acpi_video_get_backlight_types()
>>>>     platform/x86: compal-laptop: Use acpi_video_get_backlight_types()
>>>>     platform/x86: eeepc-laptop: Use acpi_video_get_backlight_types()
>>>>     platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()
>>>>     platform/x86: ideapad-laptop: Use acpi_video_get_backlight_types()
>>>>     platform/x86: msi-laptop: Use acpi_video_get_backlight_types()
>>>>     platform/x86: msi-wmi: Use acpi_video_get_backlight_types()
>>>>     platform/x86: nvidia-wmi-ec-backlight: Use
>>>>       acpi_video_get_backlight_types()
>>>>     platform/x86: panasonic-laptop: Use acpi_video_get_backlight_types()
>>>>     platform/x86: samsung-laptop: Use acpi_video_get_backlight_types()
>>>>     platform/x86: sony-laptop: Use acpi_video_get_backlight_types()
>>>>     platform/x86: thinkpad_acpi: Use acpi_video_get_backlight_types()
>>>>     platform/x86: toshiba_acpi: Use acpi_video_get_backlight_types()
>>>>     platform/x86: dell-laptop: Use acpi_video_get_backlight_types()
>>>>     platform/x86: intel_oaktrail: Use acpi_video_get_backlight_types()
>>>>     ACPI: video: Remove acpi_video_get_backlight_type()
>>>>     ACPI: video: Fallback to native backlight
>>>>
>>>>    Documentation/gpu/todo.rst                    |  8 +--
>>>>    drivers/acpi/acpi_video.c                     |  2 +-
>>>>    drivers/acpi/video_detect.c                   | 54 ++++++++++---------
>>>>    drivers/gpu/drm/i915/display/intel_opregion.c |  3 +-
>>>>    drivers/platform/loongarch/loongson-laptop.c  |  4 +-
>>>>    drivers/platform/x86/acer-wmi.c               |  2 +-
>>>>    drivers/platform/x86/asus-laptop.c            |  2 +-
>>>>    drivers/platform/x86/asus-wmi.c               |  4 +-
>>>>    drivers/platform/x86/compal-laptop.c          |  2 +-
>>>>    drivers/platform/x86/dell/dell-laptop.c       |  2 +-
>>>>    drivers/platform/x86/eeepc-laptop.c           |  2 +-
>>>>    drivers/platform/x86/fujitsu-laptop.c         |  4 +-
>>>>    drivers/platform/x86/ideapad-laptop.c         |  2 +-
>>>>    drivers/platform/x86/intel/oaktrail.c         |  2 +-
>>>>    drivers/platform/x86/msi-laptop.c             |  2 +-
>>>>    drivers/platform/x86/msi-wmi.c                |  2 +-
>>>>    .../platform/x86/nvidia-wmi-ec-backlight.c    |  2 +-
>>>>    drivers/platform/x86/panasonic-laptop.c       |  2 +-
>>>>    drivers/platform/x86/samsung-laptop.c         |  2 +-
>>>>    drivers/platform/x86/sony-laptop.c            |  2 +-
>>>>    drivers/platform/x86/thinkpad_acpi.c          |  4 +-
>>>>    drivers/platform/x86/toshiba_acpi.c           |  2 +-
>>>>    drivers/video/backlight/backlight.c           | 18 +++++++
>>>>    include/acpi/video.h                          | 21 ++++----
>>>>    include/linux/backlight.h                     |  1 +
>>>>    25 files changed, 85 insertions(+), 66 deletions(-)
>>>>
>>>
>>
> 

Re: [PATCH 00/22] Fallback to native backlight

[Akihiko Odaki]

On 2022/10/24 23:06, Akihiko Odaki wrote:
> On 2022/10/24 22:21, Hans de Goede wrote:
>> Hi,
>>
>> On 10/24/22 14:58, Akihiko Odaki wrote:
>>> On 2022/10/24 20:53, Hans de Goede wrote:
>>>> Hi Akihiko,
>>>>
>>>> On 10/24/22 13:34, Akihiko Odaki wrote:
>>>>> Commit 2600bfa3df99 ("ACPI: video: Add 
>>>>> acpi_video_backlight_use_native()
>>>>> helper") and following commits made native backlight unavailable if
>>>>> CONFIG_ACPI_VIDEO is set and the backlight feature of ACPI video is
>>>>> unavailable, which broke the backlight functionality on Lenovo 
>>>>> ThinkPad
>>>>> C13 Yoga Chromebook. Allow to fall back to native backlight in such
>>>>> cases.
>>>>
>>>> I appreciate your work on this, but what this in essence does is
>>>> it allows 2 backlight drivers (vendor + native) to get registered
>>>> for the same panel again. While the whole goal of the backlight 
>>>> refactor
>>>> series landing in 6.1 was to make it so that there always is only
>>>> *1* backlight device registered instead of (possibly) registering
>>>> multiple and letting userspace figure it out. It is also important
>>>> to only always have 1 backlight device per panel for further
>>>> upcoming changes.
>>>>
>>>> So nack for this solution, sorry.
>>>>
>>>> I am aware that this breaks backlight control on some Chromebooks,
>>>> this was already reported and I wrote a long reply explaining why
>>>> things are done the way they are done now and also suggesting
>>>> 2 possible (much simpler) fixes, see:
>>>> https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/
>>>>
>>>> Unfortunately the reported has not followed-up on this and
>>>> I don't have the hardware to test this myself.
>>>>
>>>> Can you please try implementing 1 of the fixes suggested there
>>>> and then submit that upstream ?
>>>>
>>>> Regards,
>>>>
>>>> Hans
>>>>
>>>
>>> Hi Hans,
>>>
>>> Thanks for reviewing and letting me know the prior attempt.
>>>
>>> In this case, there is only a native backlight device and no vendor 
>>> backlight device so the duplication of backlight devices does not 
>>> happen. I think it is better to handle such a case without quirks.
>>
>> Adding a single heuristic for all chromebooks is something completely 
>> different
>> then adding per model quirks which indeed ideally should be avoided 
>> (but this
>> is not always possible).
>>
>>> I understand it is still questionable to cover the case by allowing 
>>> duplication when both of a vendor backlight device and native one. To 
>>> explain my understanding and reasoning for *not* trying to apply the 
>>> de-duplication rule to the vendor/native combination, let me first 
>>> describe that the de-duplication which happens in 
>>> acpi_video_get_backlight_type() is a heuristics and limited.
>>>
>>> As the background of acpi_video_get_backlight_type(), there is an 
>>> assumption that it should be common that both of the firmware, 
>>> implementing ACPI, and the kernel have code to drive backlight. In 
>>> the case, the more reliable one should be picked instead of using 
>>> both, and that is what the statements in `if (video_caps & 
>>> ACPI_VIDEO_BACKLIGHT)` does.
>>>
>>> However, the method has two limitations:
>>> 1. It does not cover the case where two backlight devices with the 
>>> same type exist.
>>
>> This only happens when there are 2 panels; or 2 gpus driving a single 
>> panel
>> which are both special cases where we actually want 2 backlight devices.
>>
>>> 2. The underlying assumption does not apply to vendor/native 
>>> combination.
>>>
>>> Regarding the second limitation, I don't even understand the 
>>> difference between vendor and native. My guess is that a vendor 
>>> backlight device uses vendor-specific ACPI interface, and a native 
>>> one directly uses hardware registers. If my guess is correct, the 
>>> difference between vendor and native does not imply that both of them 
>>> are likely to exist at the same time. As the conclusion, there is no 
>>> more motivation to try to de-duplicate the vendor/native combination 
>>> than to try to de-duplicate combination of devices with a single type.
>>>
>>> Of course, it is better if we could also avoid registering two 
>>> devices with one type for one physical device. Possibly we can do so 
>>> by providing a parameter to indicate that it is for the same 
>>> "internal" backlight to devm_backlight_device_register(), and let the 
>>> function check for the duplication. However, this rule may be too 
>>> restrict, or may have problems I missed.
>>>
>>> Based on the discussion above, we can deduce three possibilities:
>>> a. There is no reason to distinguish vendor and native in this case, 
>>> and we can stick to my current proposal.
>>> b. There is a valid reason to distinguish vendor and native, and we 
>>> can adopt the same strategy that already adopted for ACPI 
>>> video/vendor combination.
>>> c. We can implement de-duplication in devm_backlight_device_register().
>>> d. The other possible options are not worth, and we can just 
>>> implement quirks specific to Chromebook/coreboot.
>>>
>>> In case b, it should be noted that vendor and native backlight device 
>>> do not require ACPI video, and CONFIG_ACPI_VIDEO may not be enabled. 
>>> In the case, the de-duplication needs to be implemented in backlight 
>>> class device.
>>
>> I have been working on the ACPI/x86 backlight detection code since 
>> 2015, please trust
>> me when I say that allowing both vendor + native backlight devices at 
>> the same time
>> is a bad idea.
>>
>> I'm currently in direct contact with the ChromeOS team about fixing 
>> the Chromebook
>> backlight issue introduced in 6.1-rc1.
>>
>> If you wan to help, please read:
>>
>> https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/
>>
>> And try implementing 1 if the 2 solutions suggested there.
>>
>> Regards,
>>
>> Hans
> 
> Hi,
> 
> I just wanted to confirm your intention that we should distinguish 
> vendor and native. In the case I think it is better to modify 
> __acpi_video_get_backlight_type() and add "native_available" check in 
> case of no ACPI video as already done for the ACPI video/native 
> combination.
> 
> Unfortunately this has one pitfall though: it does not work if 
> CONFIG_ACPI_VIDEO is disabled. If it is, acpi_video_get_backlight_type() 
> always return acpi_backlight_vendor, and 
> acpi_video_backlight_use_native() always returns true. It is not a 
> regression but the current behavior. Fixing it requires also refactoring 
> touching both of ACPI video and backlight class driver so I guess I'm 
> not an appropriate person to do that, and I should just add 
> "native_available" check to __acpi_video_get_backlight_type().
> 
> Does that sound good?

Well, it would not be that easy since just adding native_available 
cannot handle the case where the vendor driver gets registered first. 
Checking with "native_available" was possible for ACPI video/vendor 
combination because ACPI video registers its backlight after some delay. 
I still think it does not overcomplicate things to modify 
__acpi_video_get_backlight_type() so that it can use both of vendor and 
native as fallback while preventing duplicate registration.

Regards,
Akihiko Odaki

> 
> Regards,
> Akihiko Odaki
> 
>>  >
>>
>>>>> Akihiko Odaki (22):
>>>>>     drm/i915/opregion: Improve backlight request condition
>>>>>     ACPI: video: Introduce acpi_video_get_backlight_types()
>>>>>     LoongArch: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: acer-wmi: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: asus-laptop: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: asus-wmi: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: compal-laptop: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: eeepc-laptop: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: ideapad-laptop: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: msi-laptop: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: msi-wmi: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: nvidia-wmi-ec-backlight: Use
>>>>>       acpi_video_get_backlight_types()
>>>>>     platform/x86: panasonic-laptop: Use 
>>>>> acpi_video_get_backlight_types()
>>>>>     platform/x86: samsung-laptop: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: sony-laptop: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: thinkpad_acpi: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: toshiba_acpi: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: dell-laptop: Use acpi_video_get_backlight_types()
>>>>>     platform/x86: intel_oaktrail: Use acpi_video_get_backlight_types()
>>>>>     ACPI: video: Remove acpi_video_get_backlight_type()
>>>>>     ACPI: video: Fallback to native backlight
>>>>>
>>>>>    Documentation/gpu/todo.rst                    |  8 +--
>>>>>    drivers/acpi/acpi_video.c                     |  2 +-
>>>>>    drivers/acpi/video_detect.c                   | 54 
>>>>> ++++++++++---------
>>>>>    drivers/gpu/drm/i915/display/intel_opregion.c |  3 +-
>>>>>    drivers/platform/loongarch/loongson-laptop.c  |  4 +-
>>>>>    drivers/platform/x86/acer-wmi.c               |  2 +-
>>>>>    drivers/platform/x86/asus-laptop.c            |  2 +-
>>>>>    drivers/platform/x86/asus-wmi.c               |  4 +-
>>>>>    drivers/platform/x86/compal-laptop.c          |  2 +-
>>>>>    drivers/platform/x86/dell/dell-laptop.c       |  2 +-
>>>>>    drivers/platform/x86/eeepc-laptop.c           |  2 +-
>>>>>    drivers/platform/x86/fujitsu-laptop.c         |  4 +-
>>>>>    drivers/platform/x86/ideapad-laptop.c         |  2 +-
>>>>>    drivers/platform/x86/intel/oaktrail.c         |  2 +-
>>>>>    drivers/platform/x86/msi-laptop.c             |  2 +-
>>>>>    drivers/platform/x86/msi-wmi.c                |  2 +-
>>>>>    .../platform/x86/nvidia-wmi-ec-backlight.c    |  2 +-
>>>>>    drivers/platform/x86/panasonic-laptop.c       |  2 +-
>>>>>    drivers/platform/x86/samsung-laptop.c         |  2 +-
>>>>>    drivers/platform/x86/sony-laptop.c            |  2 +-
>>>>>    drivers/platform/x86/thinkpad_acpi.c          |  4 +-
>>>>>    drivers/platform/x86/toshiba_acpi.c           |  2 +-
>>>>>    drivers/video/backlight/backlight.c           | 18 +++++++
>>>>>    include/acpi/video.h                          | 21 ++++----
>>>>>    include/linux/backlight.h                     |  1 +
>>>>>    25 files changed, 85 insertions(+), 66 deletions(-)
>>>>>
>>>>
>>>
>>

Re: [PATCH 00/22] Fallback to native backlight

[Hans de Goede]

Hi,

On 10/24/22 16:31, Akihiko Odaki wrote:
> On 2022/10/24 23:06, Akihiko Odaki wrote:
>> On 2022/10/24 22:21, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 10/24/22 14:58, Akihiko Odaki wrote:
>>>> On 2022/10/24 20:53, Hans de Goede wrote:
>>>>> Hi Akihiko,
>>>>>
>>>>> On 10/24/22 13:34, Akihiko Odaki wrote:
>>>>>> Commit 2600bfa3df99 ("ACPI: video: Add acpi_video_backlight_use_native()
>>>>>> helper") and following commits made native backlight unavailable if
>>>>>> CONFIG_ACPI_VIDEO is set and the backlight feature of ACPI video is
>>>>>> unavailable, which broke the backlight functionality on Lenovo ThinkPad
>>>>>> C13 Yoga Chromebook. Allow to fall back to native backlight in such
>>>>>> cases.
>>>>>
>>>>> I appreciate your work on this, but what this in essence does is
>>>>> it allows 2 backlight drivers (vendor + native) to get registered
>>>>> for the same panel again. While the whole goal of the backlight refactor
>>>>> series landing in 6.1 was to make it so that there always is only
>>>>> *1* backlight device registered instead of (possibly) registering
>>>>> multiple and letting userspace figure it out. It is also important
>>>>> to only always have 1 backlight device per panel for further
>>>>> upcoming changes.
>>>>>
>>>>> So nack for this solution, sorry.
>>>>>
>>>>> I am aware that this breaks backlight control on some Chromebooks,
>>>>> this was already reported and I wrote a long reply explaining why
>>>>> things are done the way they are done now and also suggesting
>>>>> 2 possible (much simpler) fixes, see:
>>>>> https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/
>>>>>
>>>>> Unfortunately the reported has not followed-up on this and
>>>>> I don't have the hardware to test this myself.
>>>>>
>>>>> Can you please try implementing 1 of the fixes suggested there
>>>>> and then submit that upstream ?
>>>>>
>>>>> Regards,
>>>>>
>>>>> Hans
>>>>>
>>>>
>>>> Hi Hans,
>>>>
>>>> Thanks for reviewing and letting me know the prior attempt.
>>>>
>>>> In this case, there is only a native backlight device and no vendor backlight device so the duplication of backlight devices does not happen. I think it is better to handle such a case without quirks.
>>>
>>> Adding a single heuristic for all chromebooks is something completely different
>>> then adding per model quirks which indeed ideally should be avoided (but this
>>> is not always possible).
>>>
>>>> I understand it is still questionable to cover the case by allowing duplication when both of a vendor backlight device and native one. To explain my understanding and reasoning for *not* trying to apply the de-duplication rule to the vendor/native combination, let me first describe that the de-duplication which happens in acpi_video_get_backlight_type() is a heuristics and limited.
>>>>
>>>> As the background of acpi_video_get_backlight_type(), there is an assumption that it should be common that both of the firmware, implementing ACPI, and the kernel have code to drive backlight. In the case, the more reliable one should be picked instead of using both, and that is what the statements in `if (video_caps & ACPI_VIDEO_BACKLIGHT)` does.
>>>>
>>>> However, the method has two limitations:
>>>> 1. It does not cover the case where two backlight devices with the same type exist.
>>>
>>> This only happens when there are 2 panels; or 2 gpus driving a single panel
>>> which are both special cases where we actually want 2 backlight devices.
>>>
>>>> 2. The underlying assumption does not apply to vendor/native combination.
>>>>
>>>> Regarding the second limitation, I don't even understand the difference between vendor and native. My guess is that a vendor backlight device uses vendor-specific ACPI interface, and a native one directly uses hardware registers. If my guess is correct, the difference between vendor and native does not imply that both of them are likely to exist at the same time. As the conclusion, there is no more motivation to try to de-duplicate the vendor/native combination than to try to de-duplicate combination of devices with a single type.
>>>>
>>>> Of course, it is better if we could also avoid registering two devices with one type for one physical device. Possibly we can do so by providing a parameter to indicate that it is for the same "internal" backlight to devm_backlight_device_register(), and let the function check for the duplication. However, this rule may be too restrict, or may have problems I missed.
>>>>
>>>> Based on the discussion above, we can deduce three possibilities:
>>>> a. There is no reason to distinguish vendor and native in this case, and we can stick to my current proposal.
>>>> b. There is a valid reason to distinguish vendor and native, and we can adopt the same strategy that already adopted for ACPI video/vendor combination.
>>>> c. We can implement de-duplication in devm_backlight_device_register().
>>>> d. The other possible options are not worth, and we can just implement quirks specific to Chromebook/coreboot.
>>>>
>>>> In case b, it should be noted that vendor and native backlight device do not require ACPI video, and CONFIG_ACPI_VIDEO may not be enabled. In the case, the de-duplication needs to be implemented in backlight class device.
>>>
>>> I have been working on the ACPI/x86 backlight detection code since 2015, please trust
>>> me when I say that allowing both vendor + native backlight devices at the same time
>>> is a bad idea.
>>>
>>> I'm currently in direct contact with the ChromeOS team about fixing the Chromebook
>>> backlight issue introduced in 6.1-rc1.
>>>
>>> If you wan to help, please read:
>>>
>>> https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/
>>>
>>> And try implementing 1 if the 2 solutions suggested there.
>>>
>>> Regards,
>>>
>>> Hans
>>
>> Hi,
>>
>> I just wanted to confirm your intention that we should distinguish vendor and native. In the case I think it is better to modify __acpi_video_get_backlight_type() and add "native_available" check in case of no ACPI video as already done for the ACPI video/native combination.
>>
>> Unfortunately this has one pitfall though: it does not work if CONFIG_ACPI_VIDEO is disabled. If it is, acpi_video_get_backlight_type() always return acpi_backlight_vendor, and acpi_video_backlight_use_native() always returns true. It is not a regression but the current behavior. Fixing it requires also refactoring touching both of ACPI video and backlight class driver so I guess I'm not an appropriate person to do that, and I should just add "native_available" check to __acpi_video_get_backlight_type().
>>
>> Does that sound good?
> 
> Well, it would not be that easy since just adding native_available cannot handle the case where the vendor driver gets registered first. Checking with "native_available" was possible for ACPI video/vendor combination because ACPI video registers its backlight after some delay. I still think it does not overcomplicate things to modify __acpi_video_get_backlight_type() so that it can use both of vendor and native as fallback while preventing duplicate registration.

In the mean time this has already been fixed by a single patch of just a few
lines, rather then requiring 22 patches, see:

https://lore.kernel.org/dri-devel/20221024141210.67784-1-dmitry.osipenko@collabora.com/

Regards,

Hans




>>>>>> Akihiko Odaki (22):
>>>>>>     drm/i915/opregion: Improve backlight request condition
>>>>>>     ACPI: video: Introduce acpi_video_get_backlight_types()
>>>>>>     LoongArch: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: acer-wmi: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: asus-laptop: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: asus-wmi: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: compal-laptop: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: eeepc-laptop: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: ideapad-laptop: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: msi-laptop: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: msi-wmi: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: nvidia-wmi-ec-backlight: Use
>>>>>>       acpi_video_get_backlight_types()
>>>>>>     platform/x86: panasonic-laptop: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: samsung-laptop: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: sony-laptop: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: thinkpad_acpi: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: toshiba_acpi: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: dell-laptop: Use acpi_video_get_backlight_types()
>>>>>>     platform/x86: intel_oaktrail: Use acpi_video_get_backlight_types()
>>>>>>     ACPI: video: Remove acpi_video_get_backlight_type()
>>>>>>     ACPI: video: Fallback to native backlight
>>>>>>
>>>>>>    Documentation/gpu/todo.rst                    |  8 +--
>>>>>>    drivers/acpi/acpi_video.c                     |  2 +-
>>>>>>    drivers/acpi/video_detect.c                   | 54 ++++++++++---------
>>>>>>    drivers/gpu/drm/i915/display/intel_opregion.c |  3 +-
>>>>>>    drivers/platform/loongarch/loongson-laptop.c  |  4 +-
>>>>>>    drivers/platform/x86/acer-wmi.c               |  2 +-
>>>>>>    drivers/platform/x86/asus-laptop.c            |  2 +-
>>>>>>    drivers/platform/x86/asus-wmi.c               |  4 +-
>>>>>>    drivers/platform/x86/compal-laptop.c          |  2 +-
>>>>>>    drivers/platform/x86/dell/dell-laptop.c       |  2 +-
>>>>>>    drivers/platform/x86/eeepc-laptop.c           |  2 +-
>>>>>>    drivers/platform/x86/fujitsu-laptop.c         |  4 +-
>>>>>>    drivers/platform/x86/ideapad-laptop.c         |  2 +-
>>>>>>    drivers/platform/x86/intel/oaktrail.c         |  2 +-
>>>>>>    drivers/platform/x86/msi-laptop.c             |  2 +-
>>>>>>    drivers/platform/x86/msi-wmi.c                |  2 +-
>>>>>>    .../platform/x86/nvidia-wmi-ec-backlight.c    |  2 +-
>>>>>>    drivers/platform/x86/panasonic-laptop.c       |  2 +-
>>>>>>    drivers/platform/x86/samsung-laptop.c         |  2 +-
>>>>>>    drivers/platform/x86/sony-laptop.c            |  2 +-
>>>>>>    drivers/platform/x86/thinkpad_acpi.c          |  4 +-
>>>>>>    drivers/platform/x86/toshiba_acpi.c           |  2 +-
>>>>>>    drivers/video/backlight/backlight.c           | 18 +++++++
>>>>>>    include/acpi/video.h                          | 21 ++++----
>>>>>>    include/linux/backlight.h                     |  1 +
>>>>>>    25 files changed, 85 insertions(+), 66 deletions(-)
>>>>>>
>>>>>
>>>>
>>>
> 

Re: [PATCH 00/22] Fallback to native backlight

[Akihiko Odaki]

On 2022/10/24 23:37, Hans de Goede wrote:
> Hi,
> 
> On 10/24/22 16:31, Akihiko Odaki wrote:
>> On 2022/10/24 23:06, Akihiko Odaki wrote:
>>> On 2022/10/24 22:21, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 10/24/22 14:58, Akihiko Odaki wrote:
>>>>> On 2022/10/24 20:53, Hans de Goede wrote:
>>>>>> Hi Akihiko,
>>>>>>
>>>>>> On 10/24/22 13:34, Akihiko Odaki wrote:
>>>>>>> Commit 2600bfa3df99 ("ACPI: video: Add acpi_video_backlight_use_native()
>>>>>>> helper") and following commits made native backlight unavailable if
>>>>>>> CONFIG_ACPI_VIDEO is set and the backlight feature of ACPI video is
>>>>>>> unavailable, which broke the backlight functionality on Lenovo ThinkPad
>>>>>>> C13 Yoga Chromebook. Allow to fall back to native backlight in such
>>>>>>> cases.
>>>>>>
>>>>>> I appreciate your work on this, but what this in essence does is
>>>>>> it allows 2 backlight drivers (vendor + native) to get registered
>>>>>> for the same panel again. While the whole goal of the backlight refactor
>>>>>> series landing in 6.1 was to make it so that there always is only
>>>>>> *1* backlight device registered instead of (possibly) registering
>>>>>> multiple and letting userspace figure it out. It is also important
>>>>>> to only always have 1 backlight device per panel for further
>>>>>> upcoming changes.
>>>>>>
>>>>>> So nack for this solution, sorry.
>>>>>>
>>>>>> I am aware that this breaks backlight control on some Chromebooks,
>>>>>> this was already reported and I wrote a long reply explaining why
>>>>>> things are done the way they are done now and also suggesting
>>>>>> 2 possible (much simpler) fixes, see:
>>>>>> https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/
>>>>>>
>>>>>> Unfortunately the reported has not followed-up on this and
>>>>>> I don't have the hardware to test this myself.
>>>>>>
>>>>>> Can you please try implementing 1 of the fixes suggested there
>>>>>> and then submit that upstream ?
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Hans
>>>>>>
>>>>>
>>>>> Hi Hans,
>>>>>
>>>>> Thanks for reviewing and letting me know the prior attempt.
>>>>>
>>>>> In this case, there is only a native backlight device and no vendor backlight device so the duplication of backlight devices does not happen. I think it is better to handle such a case without quirks.
>>>>
>>>> Adding a single heuristic for all chromebooks is something completely different
>>>> then adding per model quirks which indeed ideally should be avoided (but this
>>>> is not always possible).
>>>>
>>>>> I understand it is still questionable to cover the case by allowing duplication when both of a vendor backlight device and native one. To explain my understanding and reasoning for *not* trying to apply the de-duplication rule to the vendor/native combination, let me first describe that the de-duplication which happens in acpi_video_get_backlight_type() is a heuristics and limited.
>>>>>
>>>>> As the background of acpi_video_get_backlight_type(), there is an assumption that it should be common that both of the firmware, implementing ACPI, and the kernel have code to drive backlight. In the case, the more reliable one should be picked instead of using both, and that is what the statements in `if (video_caps & ACPI_VIDEO_BACKLIGHT)` does.
>>>>>
>>>>> However, the method has two limitations:
>>>>> 1. It does not cover the case where two backlight devices with the same type exist.
>>>>
>>>> This only happens when there are 2 panels; or 2 gpus driving a single panel
>>>> which are both special cases where we actually want 2 backlight devices.
>>>>
>>>>> 2. The underlying assumption does not apply to vendor/native combination.
>>>>>
>>>>> Regarding the second limitation, I don't even understand the difference between vendor and native. My guess is that a vendor backlight device uses vendor-specific ACPI interface, and a native one directly uses hardware registers. If my guess is correct, the difference between vendor and native does not imply that both of them are likely to exist at the same time. As the conclusion, there is no more motivation to try to de-duplicate the vendor/native combination than to try to de-duplicate combination of devices with a single type.
>>>>>
>>>>> Of course, it is better if we could also avoid registering two devices with one type for one physical device. Possibly we can do so by providing a parameter to indicate that it is for the same "internal" backlight to devm_backlight_device_register(), and let the function check for the duplication. However, this rule may be too restrict, or may have problems I missed.
>>>>>
>>>>> Based on the discussion above, we can deduce three possibilities:
>>>>> a. There is no reason to distinguish vendor and native in this case, and we can stick to my current proposal.
>>>>> b. There is a valid reason to distinguish vendor and native, and we can adopt the same strategy that already adopted for ACPI video/vendor combination.
>>>>> c. We can implement de-duplication in devm_backlight_device_register().
>>>>> d. The other possible options are not worth, and we can just implement quirks specific to Chromebook/coreboot.
>>>>>
>>>>> In case b, it should be noted that vendor and native backlight device do not require ACPI video, and CONFIG_ACPI_VIDEO may not be enabled. In the case, the de-duplication needs to be implemented in backlight class device.
>>>>
>>>> I have been working on the ACPI/x86 backlight detection code since 2015, please trust
>>>> me when I say that allowing both vendor + native backlight devices at the same time
>>>> is a bad idea.
>>>>
>>>> I'm currently in direct contact with the ChromeOS team about fixing the Chromebook
>>>> backlight issue introduced in 6.1-rc1.
>>>>
>>>> If you wan to help, please read:
>>>>
>>>> https://lore.kernel.org/linux-acpi/42a5f2c9-a1dc-8fc0-7334-fe6c390ecfbb@redhat.com/
>>>>
>>>> And try implementing 1 if the 2 solutions suggested there.
>>>>
>>>> Regards,
>>>>
>>>> Hans
>>>
>>> Hi,
>>>
>>> I just wanted to confirm your intention that we should distinguish vendor and native. In the case I think it is better to modify __acpi_video_get_backlight_type() and add "native_available" check in case of no ACPI video as already done for the ACPI video/native combination.
>>>
>>> Unfortunately this has one pitfall though: it does not work if CONFIG_ACPI_VIDEO is disabled. If it is, acpi_video_get_backlight_type() always return acpi_backlight_vendor, and acpi_video_backlight_use_native() always returns true. It is not a regression but the current behavior. Fixing it requires also refactoring touching both of ACPI video and backlight class driver so I guess I'm not an appropriate person to do that, and I should just add "native_available" check to __acpi_video_get_backlight_type().
>>>
>>> Does that sound good?
>>
>> Well, it would not be that easy since just adding native_available cannot handle the case where the vendor driver gets registered first. Checking with "native_available" was possible for ACPI video/vendor combination because ACPI video registers its backlight after some delay. I still think it does not overcomplicate things to modify __acpi_video_get_backlight_type() so that it can use both of vendor and native as fallback while preventing duplicate registration.
> 
> In the mean time this has already been fixed by a single patch of just a few
> lines, rather then requiring 22 patches, see:
> 
> https://lore.kernel.org/dri-devel/20221024141210.67784-1-dmitry.osipenko@collabora.com/
> 
> Regards,
> 
> Hans

It should be the smaller indeed. Modifying 
__acpi_video_get_backlight_type() so that it can fall back to either of 
vendor and native requires all of the vendor drivers to use something 
like acpi_video_backlight_use_native() but for vendor. It certainly 
requires 22 patches.

That aside, the first patch in this series can be applied without the 
later patches so you may have a look at it. It's fine if you don't merge 
it though since it does not fix really a pragmatic bug as its message says.

Regards,
Akihiko Odaki

> 
> 
> 
> 
>>>>>>> Akihiko Odaki (22):
>>>>>>>      drm/i915/opregion: Improve backlight request condition
>>>>>>>      ACPI: video: Introduce acpi_video_get_backlight_types()
>>>>>>>      LoongArch: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: acer-wmi: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: asus-laptop: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: asus-wmi: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: compal-laptop: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: eeepc-laptop: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: fujitsu-laptop: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: ideapad-laptop: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: msi-laptop: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: msi-wmi: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: nvidia-wmi-ec-backlight: Use
>>>>>>>        acpi_video_get_backlight_types()
>>>>>>>      platform/x86: panasonic-laptop: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: samsung-laptop: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: sony-laptop: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: thinkpad_acpi: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: toshiba_acpi: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: dell-laptop: Use acpi_video_get_backlight_types()
>>>>>>>      platform/x86: intel_oaktrail: Use acpi_video_get_backlight_types()
>>>>>>>      ACPI: video: Remove acpi_video_get_backlight_type()
>>>>>>>      ACPI: video: Fallback to native backlight
>>>>>>>
>>>>>>>     Documentation/gpu/todo.rst                    |  8 +--
>>>>>>>     drivers/acpi/acpi_video.c                     |  2 +-
>>>>>>>     drivers/acpi/video_detect.c                   | 54 ++++++++++---------
>>>>>>>     drivers/gpu/drm/i915/display/intel_opregion.c |  3 +-
>>>>>>>     drivers/platform/loongarch/loongson-laptop.c  |  4 +-
>>>>>>>     drivers/platform/x86/acer-wmi.c               |  2 +-
>>>>>>>     drivers/platform/x86/asus-laptop.c            |  2 +-
>>>>>>>     drivers/platform/x86/asus-wmi.c               |  4 +-
>>>>>>>     drivers/platform/x86/compal-laptop.c          |  2 +-
>>>>>>>     drivers/platform/x86/dell/dell-laptop.c       |  2 +-
>>>>>>>     drivers/platform/x86/eeepc-laptop.c           |  2 +-
>>>>>>>     drivers/platform/x86/fujitsu-laptop.c         |  4 +-
>>>>>>>     drivers/platform/x86/ideapad-laptop.c         |  2 +-
>>>>>>>     drivers/platform/x86/intel/oaktrail.c         |  2 +-
>>>>>>>     drivers/platform/x86/msi-laptop.c             |  2 +-
>>>>>>>     drivers/platform/x86/msi-wmi.c                |  2 +-
>>>>>>>     .../platform/x86/nvidia-wmi-ec-backlight.c    |  2 +-
>>>>>>>     drivers/platform/x86/panasonic-laptop.c       |  2 +-
>>>>>>>     drivers/platform/x86/samsung-laptop.c         |  2 +-
>>>>>>>     drivers/platform/x86/sony-laptop.c            |  2 +-
>>>>>>>     drivers/platform/x86/thinkpad_acpi.c          |  4 +-
>>>>>>>     drivers/platform/x86/toshiba_acpi.c           |  2 +-
>>>>>>>     drivers/video/backlight/backlight.c           | 18 +++++++
>>>>>>>     include/acpi/video.h                          | 21 ++++----
>>>>>>>     include/linux/backlight.h                     |  1 +
>>>>>>>     25 files changed, 85 insertions(+), 66 deletions(-)
>>>>>>>
>>>>>>
>>>>>
>>>>
>>
> 

Re: [PATCH 00/22] Fallback to native backlight

[Jani Nikula]

On Tue, 25 Oct 2022, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> That aside, the first patch in this series can be applied without the 
> later patches so you may have a look at it. It's fine if you don't merge 
> it though since it does not fix really a pragmatic bug as its message says.

I think it's problematic because it needlessly ties i915 backlight
operation to existence of backlight devices that may not be related to
Intel GPU at all. The direction should be multiple supported backlight
devices, across GPUs and connectors, but only one per display.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH 00/22] Fallback to native backlight

[Akihiko Odaki]

On 2022/10/25 3:11, Jani Nikula wrote:
> On Tue, 25 Oct 2022, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>> That aside, the first patch in this series can be applied without the
>> later patches so you may have a look at it. It's fine if you don't merge
>> it though since it does not fix really a pragmatic bug as its message says.
> 
> I think it's problematic because it needlessly ties i915 backlight
> operation to existence of backlight devices that may not be related to
> Intel GPU at all. The direction should be multiple supported backlight
> devices, across GPUs and connectors, but only one per display.
> 
> BR,
> Jani.
> 
> 

Unfortunately it is the current situation (even without this patch), and 
this patch is not meant to fix the particular issue.

This patch replaces the following expression:
acpi_video_get_backlight_type() == acpi_backlight_native

As you can see, acpi_video_get_backlight_type() doesn't take a parameter 
which represents the backlight currently being operated. The problem is 
known and documented in "Brightness handling on devices with multiple 
internal panels" section of Documentation/gpu/todo.rst.

The exiting solution is based on the assumption that no device with i915 
and multiple internal backlights.

Regards,
Akihiko Odaki