Re: [PATCH] drm/i915/opregion: ignore firmware requests for backlight change

Hi, I would like to inform that I use UEFI boot. I don't have a spare
machine/disk to test legacy unfortunately.

regards
Anton.


2014-06-27 7:20 GMT+04:00 Aaron Lu [off-list ref]:

On 06/25/2014 07:08 PM, Jani Nikula wrote:

quoted

On Tue, 24 Jun 2014, Aaron Lu [off-list ref] wrote:

quoted

Some Thinkpad laptops' firmware will initiate a backlight level change
request through operation region on the events of AC plug/unplug, but
since we are not using firmware's interface to do the backlight setting
on these affected laptops, we do not want the firmware to use some
arbitrary value from its ASL variable to set the backlight level on
AC plug/unplug either.

I'm curious whether this happens with EFI boot, or only with legacy.

Igor, Anton,

Are you using legacy boot or UEFI boot?
Possible to test the other case?

quoted

One comment inline, otherwise

Will add that in next revision.

quoted

Acked-by: Jani Nikula <redacted>

Thanks for the review!

-Aaron

quoted

for merging through the ACPI tree, as the change is more likely to
conflict there.

quoted

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=76491
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=77091
Reported-and-tested-by: Igor Gnatenko <redacted>
Reported-and-tested-by: Anton Gubarkov <redacted>
Signed-off-by: Aaron Lu <redacted>
---
 drivers/acpi/video.c                  | 3 ++-
 drivers/gpu/drm/i915/intel_opregion.c | 7 +++++++
 include/acpi/video.h                  | 2 ++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index fb9ffe9adc64..cf99d6d2d491 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c

@@ -241,13 +241,14 @@ static bool acpi_video_use_native_backlight(void)
             return use_native_backlight_dmi;
 }

-static bool acpi_video_verify_backlight_support(void)
+bool acpi_video_verify_backlight_support(void)
 {
     if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
         backlight_device_registered(BACKLIGHT_RAW))
             return false;
     return acpi_video_backlight_support();
 }
+EXPORT_SYMBOL(acpi_video_verify_backlight_support);

 /* backlight device sysfs support */
 static int acpi_video_get_brightness(struct backlight_device *bd)

diff --git a/drivers/gpu/drm/i915/intel_opregion.c

b/drivers/gpu/drm/i915/intel_opregion.c

quoted

quoted

index 2e2c71fcc9ed..02943d93e88e 100644

--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c

@@ -403,6 +403,13 @@ static u32 asle_set_backlight(struct drm_device

*dev, u32 bclp)

quoted

quoted

     DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);

+    /*
+     * If the acpi_video interface is not supposed to be used, don't
+     * bother processing backlight level change requests from firmware.
+     */
+    if (!acpi_video_verify_backlight_support())
+            return 0;

I'd appreciate a DRM_DEBUG_KMS here about what happened. We're bound to
wonder about that staring at some dmesg later on!

quoted

+
     if (!(bclp & ASLE_BCLP_VALID))
             return ASLC_BACKLIGHT_FAILED;

diff --git a/include/acpi/video.h b/include/acpi/video.h
index ea4c7bbded4d..92f8c4bffefb 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h

@@ -22,6 +22,7 @@ extern void acpi_video_unregister(void);
 extern void acpi_video_unregister_backlight(void);
 extern int acpi_video_get_edid(struct acpi_device *device, int type,
                            int device_id, void **edid);
+extern bool acpi_video_verify_backlight_support(void);
 #else
 static inline int acpi_video_register(void) { return 0; }
 static inline void acpi_video_unregister(void) { return; }

@@ -31,6 +32,7 @@ static inline int acpi_video_get_edid(struct

acpi_device *device, int type,

quoted

quoted

 {
     return -ENODEV;
 }
+static bool acpi_video_verify_backlight_support() { return false; }
 #endif

 #endif
--
1.9.3