Martin L Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80260?usp=email )
Change subject: drivers/intel/gma: Add missing parentheses to brightness ACPI ......................................................................
drivers/intel/gma: Add missing parentheses to brightness ACPI
Commit d25277666829 ("tree: Replace And(a,b) with ASL 2.0 syntax") replaced two instances of `And(var, mask) == 0` with `var & mask == 0`. This expression needs parentheses - `(var & mask) == 0`.
Without parentheses, it is always false, since the masks are nonzero (`var & (mask == 0)`; `var & 0`; `0`).
This caused brightness changes on Intel GMA to take longer than normal since the status was never checked. The brightness would change immediately, but another brightness change could not occur until the first change timed out.
This was most noticeable in KDE, which waits for the brightness change to complete before accepting another brightness up/down keypress. Tapping brightness up/down repeatedly would take much longer to reach max/min brightness due to many presses being ignored.
It is noticeable in GNOME as well but less obvious. Tapping brightness up/down repeatedly would handle all keypresses, but the display's actual brightness would lag behind and skip some intermediate steps.
I tested both Librem 13v2 and Librem 14, as far as I know this would apply to all systems configuring brightness with Intel GMA.
Test: Verify brightness keys respond quickly again on Librem 13v2 / 14.
Change-Id: I57895e8c654c83368b452d7adfe1856c0a0341fb Signed-off-by: Jonathon Hall jonathon.hall@puri.sm Reviewed-on: https://review.coreboot.org/c/coreboot/+/80260 Reviewed-by: Arthur Heymans arthur@aheymans.xyz Reviewed-by: Patrick Rudolph patrick.rudolph@9elements.com Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@mailbox.org --- M src/drivers/intel/gma/acpi/configure_brightness_levels.asl 1 file changed, 2 insertions(+), 2 deletions(-)
Approvals: Patrick Rudolph: Looks good to me, approved build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve
diff --git a/src/drivers/intel/gma/acpi/configure_brightness_levels.asl b/src/drivers/intel/gma/acpi/configure_brightness_levels.asl index d6e417c..3a097e3 100644 --- a/src/drivers/intel/gma/acpi/configure_brightness_levels.asl +++ b/src/drivers/intel/gma/acpi/configure_brightness_levels.asl @@ -46,7 +46,7 @@ { Return (Ones) } - If (MBOX & 4 == 0) + If ((MBOX & 4) == 0) { Return (Ones) } @@ -74,7 +74,7 @@ While (Local0 > 0) { Sleep (1) - If (ASLC & 2 == 0) { + If ((ASLC & 2) == 0) { /* Request has been processed, check status: */ Local1 = (ASLC >> 12) & 3 If (Local1 == 0) {