On Fri, 2014-06-27 at 11:20 +0800, Aaron Lu wrote: > On 06/25/2014 07:08 PM, Jani Nikula wrote: > > On Tue, 24 Jun 2014, Aaron Lu <aaron.lu@xxxxxxxxx> wrote: > >> 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, Hi, > Are you using legacy boot or UEFI boot? I'm using UEFI. > Possible to test the other case? yes, I'll test it with legacy in 10 mins. > > > > > One comment inline, otherwise > > Will add that in next revision. > > > > > Acked-by: Jani Nikula <jani.nikula@xxxxxxxxx> > > Thanks for the review! > > -Aaron > > > > > for merging through the ACPI tree, as the change is more likely to > > conflict there. > > > >> 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 <i.gnatenko.brain@xxxxxxxxx> > >> Reported-and-tested-by: Anton Gubarkov <anton.gubarkov@xxxxxxxxx> > >> Signed-off-by: Aaron Lu <aaron.lu@xxxxxxxxx> > >> --- > >> 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 > >> 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) > >> > >> 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! > > > >> + > >> 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, > >> { > >> return -ENODEV; > >> } > >> +static bool acpi_video_verify_backlight_support() { return false; } > >> #endif > >> > >> #endif > >> -- > >> 1.9.3 > >> > > > -- -Igor Gnatenko
Attachment:
signature.asc
Description: This is a digitally signed message part
