drm/i915/opregion: ignore firmware requests for backlight change
Subsystems (4)
PATCH 1/1 drm/i915/opregion: ignore firmware requests for backlight change
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.
--- 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(-)
drivers/acpi/video.c +2 -1
@@ -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)
drivers/gpu/drm/i915/intel_opregion.c +7 -0
@@ -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 ; +if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;
include/acpi/video.h +2 -0
@@ -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
Signature · 1 line
1.9.3
Quoted 6 lines · Aaron Lu
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.
One comment inline, otherwise
for merging through the ACPI tree, as the change is more likely to conflict there.
Quoted 36 lines
Reported-and-tested-by Igor Gnatenko <[email protected]>Reported-and-tested-by Anton Gubarkov <[email protected]>Signed-off-by Aaron Lu <[email protected]>--- 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(-)drivers/acpi/video.c +2
-1@@ -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)drivers/gpu/drm/i915/intel_opregion.c +0
-0
drivers/gpu/drm/i915/intel_opregion.c static u32 asle_set_backlight(struct drm_device *dev, u32 bclp) · line 411
Quoted 9 lines
drivers/gpu/drm/i915/intel_opregion.c +6
-0@@ -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!
Quoted 27 lines
drivers/gpu/drm/i915/intel_opregion.c +1
-0+if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;include/acpi/video.h +2
-0@@ -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 Signature · 1 line
1.9.3
Signature · 1 line
Jani Nikula, Intel Open Source Technology Center
Quoted 9 lines · Jani Nikula
Quoted 6 lines · Aaron Lu
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 2 lines
One comment inline, otherwise
Will add that in next revision.
Quoted 2 lines
Acked-by Jani Nikula <[email protected]>
Thanks for the review!
-Aaron
Quoted 81 lines
for merging through the ACPI tree, as the change is more likely to conflict there.
Quoted 36 lines
Reported-and-tested-by Igor Gnatenko <[email protected]>Reported-and-tested-by Anton Gubarkov <[email protected]>Signed-off-by Aaron Lu <[email protected]>--- 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(-)drivers/acpi/video.c +2
-1@@ -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)drivers/gpu/drm/i915/intel_opregion.c +0
-0drivers/gpu/drm/i915/intel_opregion.c static u32 asle_set_backlight(struct drm_device *dev, u32 bclp) · line 411
Quoted 9 lines
drivers/gpu/drm/i915/intel_opregion.c +6
-0@@ -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_KMShere about what happened. We're bound to wonder about that staring at some dmesg later on!Quoted 27 lines
drivers/gpu/drm/i915/intel_opregion.c +1
-0+if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;include/acpi/video.h +2
-0@@ -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 Signature · 1 line
1.9.3
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 <[email protected]>:
Quoted 87 lines · Jani Nikula
Quoted 9 lines · Aaron Lu
Quoted 6 lines
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 2 lines
One comment inline, otherwise
Will add that in next revision.
Quoted 2 lines
Acked-by Jani Nikula <[email protected]>Thanks for the review!
-Aaron
Quoted 33 lines
for merging through the ACPI tree, as the change is more likely to conflict there.
Quoted 29 lines
Reported-and-tested-by Igor Gnatenko <[email protected]>Reported-and-tested-by Anton Gubarkov <[email protected]>Signed-off-by Aaron Lu <[email protected]>--- 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(-)drivers/acpi/video.c +2
-1@@ -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);Quoted 3 lines
Review
Quoted 3 lines
drivers/acpi/video.c +0
-0/* backlight device sysfs support */ static int acpi_video_get_brightness (struct backlight_device *bd)(quoted diff) +0
-0
b/drivers/gpu/drm/i915/intel_opregion.cQuoted 4 lines
drivers/gpu/drm/i915/intel_opregion.c static u32 asle_set_backlight(struct drm_device
Quoted 4 lines
index
2e2c71fcc9ed..02943d93e88e100644drivers/gpu/drm/i915/intel_opregion.c +0
-0@@ -403,6 +403,13 @@ static u32 asle_set_backlight(struct drm_device*dev, u32 bclp)
Quoted 17 lines
drivers/gpu/drm/i915/intel_opregion.c static u32 asle_set_backlight(struct drm_device *dev, u32 bclp) · line 411
Quoted 9 lines
drivers/gpu/drm/i915/intel_opregion.c +6
-0DRM_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_KMShere about what happened. We're bound to wonder about that staring at some dmesg later on!Quoted 4 lines
drivers/gpu/drm/i915/intel_opregion.c +1
-0+if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;
Quoted 27 lines
Quoted 5 lines
Quoted 5 lines
include/acpi/video.h +0
-0@@ -22,6 +22,7 @@ extern void acpi_video_unregister(void);Quoted 8 lines
include/acpi/video.h static inline int acpi_video_get_edid(struct
Quoted 8 lines
include/acpi/video.h +1
-0extern 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 11 lines
Quoted 10 lines
include/acpi/video.h +1
-1{return -ENODEV ; } +static bool acpi_video_verify_backlight_support () {return false; }#endif #endif --1.9.3
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 9 lines · Aaron Lu
Quoted 8 lines · Jani Nikula
Quoted 6 lines · Aaron Lu
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.
Quoted 3 lines
Quoted 1 line
I'm curious whether this happens with EFI boot, or only with legacy.
Igor, Anton,
Hi,
Quoted 1 line
Are you using legacy boot or UEFI boot?
I'm using UEFI.
Quoted 1 line
Possible to test the other case?
yes, I'll test it with legacy in 10 mins.
Quoted 95 lines
Quoted 3 lines
One comment inline, otherwise
Will add that in next revision.
Quoted 2 lines
Acked-by Jani Nikula <[email protected]>Thanks for the review!
-Aaron
Quoted 81 lines
for merging through the ACPI tree, as the change is more likely to conflict there.
Quoted 36 lines
Reported-and-tested-by Igor Gnatenko <[email protected]>Reported-and-tested-by Anton Gubarkov <[email protected]>Signed-off-by Aaron Lu <[email protected]>--- 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(-)drivers/acpi/video.c +2
-1@@ -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)drivers/gpu/drm/i915/intel_opregion.c +0
-0drivers/gpu/drm/i915/intel_opregion.c static u32 asle_set_backlight(struct drm_device *dev, u32 bclp) · line 411
Quoted 9 lines
drivers/gpu/drm/i915/intel_opregion.c +6
-0@@ -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_KMShere about what happened. We're bound to wonder about that staring at some dmesg later on!Quoted 27 lines
drivers/gpu/drm/i915/intel_opregion.c +1
-0+if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;include/acpi/video.h +2
-0@@ -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 Signature · 1 line
1.9.3
Signature · 1 line
-Igor Gnatenko
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
I've tested legacy boot. I have this bug.
Quoted 110 lines · Aaron Lu
Quoted 9 lines · Jani Nikula
Quoted 6 lines · Aaron Lu
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 2 lines
One comment inline, otherwise
Will add that in next revision.
Quoted 2 lines
Acked-by Jani Nikula <[email protected]>Thanks for the review!
-Aaron
Quoted 81 lines
for merging through the ACPI tree, as the change is more likely to conflict there.
Quoted 36 lines
Reported-and-tested-by Igor Gnatenko <[email protected]>Reported-and-tested-by Anton Gubarkov <[email protected]>Signed-off-by Aaron Lu <[email protected]>--- 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(-)drivers/acpi/video.c +2
-1@@ -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)drivers/gpu/drm/i915/intel_opregion.c +0
-0drivers/gpu/drm/i915/intel_opregion.c static u32 asle_set_backlight(struct drm_device *dev, u32 bclp) · line 411
Quoted 9 lines
drivers/gpu/drm/i915/intel_opregion.c +6
-0@@ -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_KMShere about what happened. We're bound to wonder about that staring at some dmesg later on!Quoted 27 lines
drivers/gpu/drm/i915/intel_opregion.c +1
-0+if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;include/acpi/video.h +2
-1@@ -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
Signature · 1 line
-Igor Gnatenko
PATCH v2 1/1 drm/i915/opregion: ignore firmware requests for backlight change
In reply to Jani NikulaSome 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.
---
v2: add a debug message when ignoring opregion request as suggested by
Jani Nikula.
drivers/acpi/video.c | 3 ++- drivers/gpu/drm/i915/intel_opregion.c | 9 +++++++++ include/acpi/video.h | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-)
drivers/acpi/video.c +2 -1
@@ -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)
drivers/gpu/drm/i915/intel_opregion.c +9 -0
@@ -403,6 +403,15 @@ 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 ()) { +DRM_DEBUG_KMS ("opregion backlight request ignored\n" ); +return 0 ; + } +if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;
include/acpi/video.h +2 -0
@@ -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
Signature · 1 line
1.9.3
Quoted 7 lines · Aaron Lu
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.
Quoted 6 lines
Reported-and-tested-by Igor Gnatenko <[email protected]>Reported-and-tested-by Anton Gubarkov <[email protected]>Signed-off-by Aaron Lu <[email protected]>Acked-by Jani Nikula <[email protected]>
I think we need this in 3.16, right?
Is it also necessary in any -stable and if so, which one?
Rafael
Quoted 70 lines
---
v2: add a debug message when ignoring opregion request as suggested by
Jani Nikula.drivers/acpi/video.c | 3 ++- drivers/gpu/drm/i915/intel_opregion.c | 9 +++++++++ include/acpi/video.h | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-)drivers/acpi/video.c +2
-1@@ -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)drivers/gpu/drm/i915/intel_opregion.c +9
-0@@ -403,6 +403,15 @@ 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 ()) { +DRM_DEBUG_KMS ("opregion backlight request ignored\n" ); +return 0 ; + } +if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;include/acpi/video.h +2
-0@@ -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
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
Quoted 8 lines · Rafael J. Wysocki
Quoted 7 lines · Aaron Lu
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.
Quoted 8 lines
Quoted 6 lines
Reported-and-tested-by Igor Gnatenko <[email protected]>Reported-and-tested-by Anton Gubarkov <[email protected]>Signed-off-by Aaron Lu <[email protected]>Acked-by Jani Nikula <[email protected]>I think we need this in 3.16, right?
Yes, this patch is for v3.16.
Quoted 2 lines
Is it also necessary in any -stable and if so, which one?
For one of the affected system 'ThinkPad X1 Carbon', it makes sense to back port the patch to v3.14 and v3.15 since it is in video module's use_native_backlight DMI table. For the other one, since it is not in the DMI table, it doesn't make a difference. I'll leave this to you to decide.
Thanks,
Aaron
Quoted 75 lines
Rafael
Quoted 70 lines
---
v2: add a debug message when ignoring opregion request as suggested by
Jani Nikula.drivers/acpi/video.c | 3 ++- drivers/gpu/drm/i915/intel_opregion.c | 9 +++++++++ include/acpi/video.h | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-)drivers/acpi/video.c +2
-1@@ -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)drivers/gpu/drm/i915/intel_opregion.c +9
-0@@ -403,6 +403,15 @@ 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 ()) { +DRM_DEBUG_KMS ("opregion backlight request ignored\n" ); +return 0 ; + } +if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;include/acpi/video.h +2
-0@@ -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
Quoted 29 lines · Aaron Lu
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.
Reported-and-tested-by Igor Gnatenko <[email protected]>Reported-and-tested-by Anton Gubarkov <[email protected]>Signed-off-by Aaron Lu <[email protected]>Acked-by Jani Nikula <[email protected]>---
v2: add a debug message when ignoring opregion request as suggested by
Jani Nikula.drivers/acpi/video.c | 3 ++- drivers/gpu/drm/i915/intel_opregion.c | 9 +++++++++ include/acpi/video.h | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-)drivers/acpi/video.c +0
-0@@ -241,13 +241,14 @@ static bool acpi_video_use_native_backlight(void)return use_native_backlight_dmi; }
drivers/acpi/video.c static bool acpi_video_use_native_backlight(void) · line 251
Quoted 10 lines
drivers/acpi/video.c +2
-1-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);
Is there any reason for this not to be EXPORT_SYMBOL_GPL()?
Quoted 35 lines
drivers/acpi/video.c +0
-0/* backlight device sysfs support */ static int acpi_video_get_brightness (struct backlight_device *bd)drivers/gpu/drm/i915/intel_opregion.c +9
-0@@ -403,6 +403,15 @@ 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 ()) { +DRM_DEBUG_KMS ("opregion backlight request ignored\n" ); +return 0 ; + } +if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;include/acpi/video.h +1
-0@@ -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 ; }
include/acpi/video.h static inline int acpi_video_get_edid(struct acpi_device *device, int type, · line 35
Quoted 4 lines
include/acpi/video.h +1
-0@@ -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; }
And for this not to be static inline?
include/acpi/video.h static inline int acpi_video_get_edid(struct acpi_device *device, int type, · line 38
Quoted 4 lines
include/acpi/video.h +0
-0#endif #endif
Rafael
Quoted 31 lines · Rafael J. Wysocki
Quoted 30 lines · Aaron Lu
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.
Reported-and-tested-by Igor Gnatenko <[email protected]>Reported-and-tested-by Anton Gubarkov <[email protected]>Signed-off-by Aaron Lu <[email protected]>Acked-by Jani Nikula <[email protected]>---
v2: add a debug message when ignoring opregion request as suggested by
Jani Nikula.drivers/acpi/video.c | 3 ++- drivers/gpu/drm/i915/intel_opregion.c | 9 +++++++++ include/acpi/video.h | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-)drivers/acpi/video.c +0
-0@@ -241,13 +241,14 @@ static bool acpi_video_use_native_backlight(void)return use_native_backlight_dmi; }
Quoted 11 lines
drivers/acpi/video.c static bool acpi_video_use_native_backlight(void) · line 251
Quoted 9 lines
drivers/acpi/video.c +2
-1-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);Is there any reason for this not to be
EXPORT_SYMBOL_GPL()?
No particular reason, just follow what is used for other exported functions in this file. Will change this to EXPORT_SYMBOL_GPL.
Quoted 34 lines
Quoted 34 lines
drivers/acpi/video.c +0
-0/* backlight device sysfs support */ static int acpi_video_get_brightness (struct backlight_device *bd)drivers/gpu/drm/i915/intel_opregion.c +9
-0@@ -403,6 +403,15 @@ 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 ()) { +DRM_DEBUG_KMS ("opregion backlight request ignored\n" ); +return 0 ; + } +if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;include/acpi/video.h +1
-0@@ -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 ; }
Quoted 8 lines
include/acpi/video.h static inline int acpi_video_get_edid(struct acpi_device *device, int type, · line 35
Quoted 6 lines
include/acpi/video.h +1
-0static 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; }And for this not to be static inline?
Will add inline.
Thanks for the review.
-Aaron
Quoted 8 lines
include/acpi/video.h static inline int acpi_video_get_edid(struct acpi_device *device, int type, · line 38
Quoted 5 lines
include/acpi/video.h +0
-0#endif #endif Rafael
PATCH v3 1/1 drm/i915/opregion: ignore firmware requests for backlight change
In reply to Aaron LuSome 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.
---
v3:
- use EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL;
- add inline to stub
as suggested by Rafael.
drivers/acpi/video.c | 3 ++- drivers/gpu/drm/i915/intel_opregion.c | 9 +++++++++ include/acpi/video.h | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-)
drivers/acpi/video.c +2 -1
@@ -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_GPL (acpi_video_verify_backlight_support);/* backlight device sysfs support */ static int acpi_video_get_brightness (struct backlight_device *bd)
drivers/gpu/drm/i915/intel_opregion.c +9 -0
@@ -403,6 +403,15 @@ 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 ()) { +DRM_DEBUG_KMS ("opregion backlight request ignored\n" ); +return 0 ; + } +if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;
include/acpi/video.h +2 -0
@@ -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 inline bool acpi_video_verify_backlight_support () {return false; }#endif #endif
Signature · 1 line
1.9.3
Quoted 13 lines · Aaron Lu
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.
Reported-and-tested-by Igor Gnatenko <[email protected]>Reported-and-tested-by Anton Gubarkov <[email protected]>Signed-off-by Aaron Lu <[email protected]>Acked-by Jani Nikula <[email protected]>
Quoted 5 lines
---
v3:
- useEXPORT_SYMBOL_GPLinstead ofEXPORT_SYMBOL;
- add inline to stub
as suggested by Rafael.
I've made these two changes already in the commit in my tree.
Quoted 58 lines
drivers/acpi/video.c | 3 ++- drivers/gpu/drm/i915/intel_opregion.c | 9 +++++++++ include/acpi/video.h | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-)drivers/acpi/video.c +2
-1@@ -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_GPL (acpi_video_verify_backlight_support);/* backlight device sysfs support */ static int acpi_video_get_brightness (struct backlight_device *bd)drivers/gpu/drm/i915/intel_opregion.c +9
-0@@ -403,6 +403,15 @@ 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 ()) { +DRM_DEBUG_KMS ("opregion backlight request ignored\n" ); +return 0 ; + } +if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;include/acpi/video.h +1
-0@@ -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 ; }
include/acpi/video.h static inline int acpi_video_get_edid(struct acpi_device *device, int type, · line 35
Quoted 4 lines
include/acpi/video.h +1
-0@@ -31,6 +32,7 @@ static inline int acpi_video_get_edid(struct acpi_device *device, int type, {return -ENODEV ; } +static inline bool acpi_video_verify_backlight_support () {return false; }
And here you need acpi_video_verify_backlight_support(void) (fixed up too).
Quoted 4 lines
include/acpi/video.h +0
-0#endif #endif
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
Quoted 20 lines · Rafael J. Wysocki
Quoted 13 lines · Aaron Lu
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.
Reported-and-tested-by Igor Gnatenko <[email protected]>Reported-and-tested-by Anton Gubarkov <[email protected]>Signed-off-by Aaron Lu <[email protected]>Acked-by Jani Nikula <[email protected]>Quoted 5 lines
---
v3:
- useEXPORT_SYMBOL_GPLinstead ofEXPORT_SYMBOL;
- add inline to stub
as suggested by Rafael.
Quoted 1 line
I've made these two changes already in the commit in my tree.
Good, thanks!
-Aaron
Quoted 71 lines
Quoted 59 lines
drivers/acpi/video.c | 3 ++- drivers/gpu/drm/i915/intel_opregion.c | 9 +++++++++ include/acpi/video.h | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-)drivers/acpi/video.c +2
-1@@ -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_GPL (acpi_video_verify_backlight_support);/* backlight device sysfs support */ static int acpi_video_get_brightness (struct backlight_device *bd)drivers/gpu/drm/i915/intel_opregion.c +9
-0@@ -403,6 +403,15 @@ 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 ()) { +DRM_DEBUG_KMS ("opregion backlight request ignored\n" ); +return 0 ; + } +if (!(bclp &ASLE_BCLP_VALID ))return ASLC_BACKLIGHT_FAILED ;include/acpi/video.h +1
-0@@ -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 ; }include/acpi/video.h static inline int acpi_video_get_edid(struct acpi_device *device, int type, · line 35
Quoted 4 lines
include/acpi/video.h +1
-0@@ -31,6 +32,7 @@ static inline int acpi_video_get_edid(struct acpi_device *device, int type, {return -ENODEV ; } +static inline bool acpi_video_verify_backlight_support () {return false; }And here you need
acpi_video_verify_backlight_support(void)(fixed up too).Quoted 4 lines
include/acpi/video.h +0
-0#endif #endif