Please test coreboot using laptops with kernel 6.1-rc# for a possible backlight regression
Hi All,
I have landed a large(ish) refactor of the ACPI/x86 backlight detection code in the kernel for 6.1. It has come to my attention that this may cause backlight control to stop working on laptops where the original BIOS has been replaced with coreboot, see this patch, which is the root cause of the possible regression + its replies: https://lore.kernel.org/platform-driver-x86/20220825143726.269890-3-hdegoede...
On ACPI/x86 laptops there may be multiple methods available to control the backlight (native GPU control, ACPI video bus, vendor specific) and even though a method is available it does not always work, basically the only method guaranteed to work is the one preferred by the Windows version with which the laptop shipped.
The kernel so far has delegated part of this problem by registering multiple backlight devices under /sys/class/backlight and then letting userspace figure it out. One goal of the refactor is to only register 1 backlight device per panel.
On many (somewhat older) laptops "ls /sys/class/backlight" would show 2 backlight devices: "acpi_video0" and "intel_backlight" and the various (per desktop) userspace backlight helpers would prefer the "acpi_video0". The goal of the linked kernel patch is to simply not register "intel_backlight" in this case, just like the kernel was already not registering "acpi_video0" on the models where "intel_backlight" should be used.
But this may cause problems / a change in behavior if:
1. You have an unusual / or customized userspace which prefers intel_backlight over acpi_video0 when both are present
2. You always only had intel_backlight and that now no longer gets registered, leaving you completely without backlight support.
I would like to try and find and resolve any such problems before 6.1 gets released. As such if you are using a laptop with coreboot please run the following tests:
1. Boot kernel 6.0 or older, and collect the output of:
ls /sys/class/backlight
2. Boot kernel 6.1, note for ChromeBooks please used Torvalds latest master and not rc2 since a fix for ChromeBooks has just landed, and collect the output of:
ls /sys/class/backlight
3. Check if everything still works as expected with kernel 6.1
Report the results of 1-3 either on the list or in a direct email to me (your choice).
Please include the model of your laptop and please report this even if everything still works fine, I need to know how things work on coreboot laptops which have not regressed too, to avoid accidentally regressing those if I need to do fixes for some other coreboot capable models.
4. Run:
sudo dmidecode > dmidecode.txt sudo acpidump -o acpidump.txt
And then send me a *private* off-list email with the dmidecode.txt and acpidump.txt files attached. Please don't forget to mention the model of your laptop in this email.
Sorry for any breakage my backlight refactor may have caused and thank you in advance for taking the time to test this.
Regards,
Hans
Hi,
On 10/26/22 14:03, Hans de Goede wrote:
Hi All,
I have landed a large(ish) refactor of the ACPI/x86 backlight detection code in the kernel for 6.1. It has come to my attention that this may cause backlight control to stop working on laptops where the original BIOS has been replaced with coreboot, see this patch, which is the root cause of the possible regression + its replies: https://lore.kernel.org/platform-driver-x86/20220825143726.269890-3-hdegoede...
Note I have written a blogpost to ask for more general testing of old/weird laptops for this. This includes an easy way to identify if your laptop might be affected (without needing to build a 6.1-rc# kernel). This easy way should also apply to coreboot using laptops:
https://hansdegoede.dreamwidth.org/26548.html
Please give this a try if possible.
Regards,
Hans
On ACPI/x86 laptops there may be multiple methods available to control the backlight (native GPU control, ACPI video bus, vendor specific) and even though a method is available it does not always work, basically the only method guaranteed to work is the one preferred by the Windows version with which the laptop shipped.
The kernel so far has delegated part of this problem by registering multiple backlight devices under /sys/class/backlight and then letting userspace figure it out. One goal of the refactor is to only register 1 backlight device per panel.
On many (somewhat older) laptops "ls /sys/class/backlight" would show 2 backlight devices: "acpi_video0" and "intel_backlight" and the various (per desktop) userspace backlight helpers would prefer the "acpi_video0". The goal of the linked kernel patch is to simply not register "intel_backlight" in this case, just like the kernel was already not registering "acpi_video0" on the models where "intel_backlight" should be used.
But this may cause problems / a change in behavior if:
- You have an unusual / or customized userspace which prefers
intel_backlight over acpi_video0 when both are present
- You always only had intel_backlight and that now no longer
gets registered, leaving you completely without backlight support.
I would like to try and find and resolve any such problems before 6.1 gets released. As such if you are using a laptop with coreboot please run the following tests:
- Boot kernel 6.0 or older, and collect the output of:
ls /sys/class/backlight
- Boot kernel 6.1, note for ChromeBooks please used Torvalds latest master and not rc2 since a fix for ChromeBooks has just landed, and collect the output of:
ls /sys/class/backlight
- Check if everything still works as expected with kernel 6.1
Report the results of 1-3 either on the list or in a direct email to me (your choice).
Please include the model of your laptop and please report this even if everything still works fine, I need to know how things work on coreboot laptops which have not regressed too, to avoid accidentally regressing those if I need to do fixes for some other coreboot capable models.
- Run:
sudo dmidecode > dmidecode.txt sudo acpidump -o acpidump.txt
And then send me a *private* off-list email with the dmidecode.txt and acpidump.txt files attached. Please don't forget to mention the model of your laptop in this email.
Sorry for any breakage my backlight refactor may have caused and thank you in advance for taking the time to test this.
Regards,
Hans
Hi Hans,
I hope you are well.
Hans de Goede wrote:
I have landed a large(ish) refactor of the ACPI/x86 backlight detection code in the kernel for 6.1. It has come to my attention that this may cause backlight control to stop working on laptops
..
please run the following tests:
To me, this looks like Red Hat wanting to externalize QA cost, which I find really unreasonable. (I admit to some bias against Red Hat since my experience with them (via Hans) in another project.)
Sorry for any breakage my backlight refactor may have caused and thank you in advance for taking the time to test this.
I don't know your mandate but if your employer values the coreboot community (retrofit but maybe especially native use cases; I guess that not only retrofit is affected, even if early data suggests so) and wants to ensure that your work doesn't introduce regressions then I expect you to push for inhouse QA resources and reject begging others to do that job, for optics as well as reliability.
Kind regards
//Peter
I have tested backlight controls on Pop!_OS 22.04 with Ubuntu mainline kernel 6.1-rc3 (as well as Windows 10) on the following:
- system76/oryp6 (CML-H) - system76/lemp10 (TGL-U) - system76/lemp11 (ADL-U) - system76/oryp9 (ADL-P)
ADL requires hooking up the GMA controls [1], and the mainboards need to specify the gfx in devicetree. With these changes, `acpi_video0` is present and backlight controls work.
Everything else is working as expected. I don't have anything older than CML-H anymore to test on.
[1]: https://review.coreboot.org/c/coreboot/+/69076
On Tue, Nov 01, 2022 at 12:30:57PM -0600, Tim Crawford via coreboot wrote:
I have tested backlight controls on Pop!_OS 22.04 with Ubuntu mainline kernel 6.1-rc3 (as well as Windows 10) on the following:
- system76/oryp6 (CML-H)
- system76/lemp10 (TGL-U)
- system76/lemp11 (ADL-U)
- system76/oryp9 (ADL-P)
ADL requires hooking up the GMA controls [1], and the mainboards need to specify the gfx in devicetree. With these changes, `acpi_video0` is present and backlight controls work.
Everything else is working as expected. I don't have anything older than CML-H anymore to test on.
And the test is described at https://hansdegoede.livejournal.com/26427.html
Groeten Geert Stappers
[1]: https://review.coreboot.org/c/coreboot/+/69076
Hi Tim,
On 11/1/22 19:30, Tim Crawford via coreboot wrote:
I have tested backlight controls on Pop!_OS 22.04 with Ubuntu mainline kernel 6.1-rc3 (as well as Windows 10) on the following:
- system76/oryp6 (CML-H)
- system76/lemp10 (TGL-U)
- system76/lemp11 (ADL-U)
- system76/oryp9 (ADL-P)
ADL requires hooking up the GMA controls [1], and the mainboards need to specify the gfx in devicetree. With these changes, `acpi_video0` is present and backlight controls work.
Everything else is working as expected. I don't have anything older than CML-H anymore to test on.
Great, thank you for testing!
About the change on the ADL system, I guess that it did work with 6.0 and older Linux kernels, with a /sys/class/backlight/intel_backlight being present then ?
Just wondering. I'm happy to see coreboot adjusted to make the kernels heuristics work. OTOH though coreboot BIOS users really should not need to upgrade their BIOS to stop things breaking with a new kernel.
I'm not entirely sure what the right balance is.
Note that I plan to submit a revert for the patch which hides: /sys/class/backlight/intel_backlight when the acpi_video_get_backlight_type() heuristics say that another backlight interface should be used, since it seems that this will break quite a few (mostly older, beginning of Intel x86_64 era) laptops.
So with the final 6.1 /sys/class/backlight/intel_backlight should stay around, but I do want to try again later since I really want to get rid of the having multiple /sys/class/backlight/ entries for a single panel and then let userspace choose shenanigans.
I've some ideas on how to change the heuristics which will hopefully fix the beginning of Intel x86_64 era laptops, but I don't think those will help on the coreboot using System76 laptops case...
Regards,
Hans
About the change on the ADL system, I guess that it did work with 6.0 and older Linux kernels, with a /sys/class/backlight/intel_backlight being present then ?
Just wondering. I'm happy to see coreboot adjusted to make the kernels heuristics work. OTOH though coreboot BIOS users really should not need to upgrade their BIOS to stop things breaking with a new kernel.
Yes, on 6.0 it works due to `intel_backlight` being available. But it works on Linux only. Backlight controls on Windows 10 do not work.
There are other issues with our ADL boards that I expect are related to the missing gfx control, like external displays being used as the default device instead of the eDP (maybe due to `_DOD` not being generated).
I've some ideas on how to change the heuristics which will hopefully fix the beginning of Intel x86_64 era laptops, but I don't think those will help on the coreboot using System76 laptops case...
I already have several fixes for problems on ADL that need to go out to users. The gfx change will be included. We will not need any kernel quirks for it.
Hi,
On 11/1/22 21:33, Tim Crawford wrote:
About the change on the ADL system, I guess that it did work with 6.0 and older Linux kernels, with a /sys/class/backlight/intel_backlight being present then ?
Just wondering. I'm happy to see coreboot adjusted to make the kernels heuristics work. OTOH though coreboot BIOS users really should not need to upgrade their BIOS to stop things breaking with a new kernel.
Yes, on 6.0 it works due to `intel_backlight` being available. But it works on Linux only. Backlight controls on Windows 10 do not work.
There are other issues with our ADL boards that I expect are related to the missing gfx control, like external displays being used as the default device instead of the eDP (maybe due to `_DOD` not being generated).
I've some ideas on how to change the heuristics which will hopefully fix the beginning of Intel x86_64 era laptops, but I don't think those will help on the coreboot using System76 laptops case...
I already have several fixes for problems on ADL that need to go out to users. The gfx change will be included. We will not need any kernel quirks for it.
Ok, thank you.
Regards,
Hans
Hi Hans,
Thank you for warning about the potential regressions that could happen due to the backlight changes in the Linux kernel.
Sorry if this reply sounds a bit scatterbrained, it is very late.
On Tue, Nov 1, 2022 at 7:11 PM Hans de Goede hdegoede@redhat.com wrote:
Just wondering. I'm happy to see coreboot adjusted to make the kernels heuristics work. OTOH though coreboot BIOS users really should not need to upgrade their BIOS to stop things breaking with a new kernel.
I'm not entirely sure what the right balance is.
It is true that firmware updates are risky, and that having to deal with things breaking after an OS update is unpleasant. However, one of the good things about coreboot is that firmware bugs can be fixed (at least most of them). Maximizing compatibility is important, but so is getting firmware bugs fixed. Maybe the right balance is to work around coreboot bugs in the kernel (if necessary) in a way that allows them to be fixed in coreboot, and of course communicate with coreboot (this email is a good example). One of the worst things that could happen is for coreboot to have to work around OSes' workarounds for coreboot because coreboot was broken and the OSes' workarounds are too inflexible for coreboot to start doing the right things: effectively, it would be a deadlock.
Note that I plan to submit a revert for the patch which hides: /sys/class/backlight/intel_backlight when the acpi_video_get_backlight_type() heuristics say that another backlight interface should be used, since it seems that this will break quite a few (mostly older, beginning of Intel x86_64 era) laptops.
Do you have any examples? coreboot supports several older Intel x86_64 platforms, including laptops, that could be affected too.
So with the final 6.1 /sys/class/backlight/intel_backlight should stay around, but I do want to try again later since I really want to get rid of the having multiple /sys/class/backlight/ entries for a single panel and then let userspace choose shenanigans.
Agreed. Having multiple backlight entries is confusing, especially when using the same OS install on a bunch of different machines.
I've some ideas on how to change the heuristics which will hopefully fix the beginning of Intel x86_64 era laptops, but I don't think those will help on the coreboot using System76 laptops case...
Well, there's a firmware bug anyway, because brightness controls don't work on Alder Lake when using Windows. And it doesn't look like Windows will accept pull requests anytime soon...
An idea (which may be impractical to implement) would be to not expose intel_backlight if ACPI backlight control is available.
Regards,
Hans
Best regards,
Angel
Hi Angel,
On 11/2/22 04:07, Angel Pons wrote:
Hi Hans,
Thank you for warning about the potential regressions that could happen due to the backlight changes in the Linux kernel.
Sorry if this reply sounds a bit scatterbrained, it is very late.
On Tue, Nov 1, 2022 at 7:11 PM Hans de Goede hdegoede@redhat.com wrote:
Just wondering. I'm happy to see coreboot adjusted to make the kernels heuristics work. OTOH though coreboot BIOS users really should not need to upgrade their BIOS to stop things breaking with a new kernel.
I'm not entirely sure what the right balance is.
It is true that firmware updates are risky, and that having to deal with things breaking after an OS update is unpleasant. However, one of the good things about coreboot is that firmware bugs can be fixed (at least most of them). Maximizing compatibility is important, but so is getting firmware bugs fixed. Maybe the right balance is to work around coreboot bugs in the kernel (if necessary) in a way that allows them to be fixed in coreboot, and of course communicate with coreboot (this email is a good example). One of the worst things that could happen is for coreboot to have to work around OSes' workarounds for coreboot because coreboot was broken and the OSes' workarounds are too inflexible for coreboot to start doing the right things: effectively, it would be a deadlock.
That is a good point. So where possible I will try to avoid adding DMI based quirks for coreboot using laptops, because those will hardcode the backlight control-method (to either ACPI video or the "native" GPU register poking method).
Although hardcoding to native should not really be a problem on devices where native actually works, coreboot updates should not impact the native control method there.
Note that I plan to submit a revert for the patch which hides: /sys/class/backlight/intel_backlight when the acpi_video_get_backlight_type() heuristics say that another backlight interface should be used, since it seems that this will break quite a few (mostly older, beginning of Intel x86_64 era) laptops.
Do you have any examples? coreboot supports several older Intel x86_64 platforms, including laptops, that could be affected too.
ATM the list is:
Acer Aspire 1640 Apple macbook-4.1 Dell N1410 IBM ThinkPad X40 System76 Starling Star1 (and I still have a lot of emails / test-reports to process)
These only have a native intel_backlight interface and the heuristics from acpi_video_get_backlight_type() return acpi_backlight_vendor there causing commit b1d36e73cc1c ("drm/i915: Don't register backlight when another backlight should be used (v2)") to result in an empty /sys/class/backlight breaking users ability to control there laptop panel's brightness.
I will submit a revert for commit b1d36e73cc1c to avoid these regressing in 6.1 (resulting in some duplicate /sys/class/backlight entries again).
So with the final 6.1 /sys/class/backlight/intel_backlight should stay around, but I do want to try again later since I really want to get rid of the having multiple /sys/class/backlight/ entries for a single panel and then let userspace choose shenanigans.
Agreed. Having multiple backlight entries is confusing, especially when using the same OS install on a bunch of different machines.
I do plan to try to re-introduce that change again later. First I need to change the heuristics to still native on more models so that on models where the native intel_backlight is the only (working) entry they will return native.
I've some ideas on how to change the heuristics which will hopefully fix the beginning of Intel x86_64 era laptops, but I don't think those will help on the coreboot using System76 laptops case...
Well, there's a firmware bug anyway, because brightness controls don't work on Alder Lake when using Windows. And it doesn't look like Windows will accept pull requests anytime soon...
An idea (which may be impractical to implement) would be to not expose intel_backlight if ACPI backlight control is available.
The problematic code path in the heuristics actually is the one where ACPI backlight control is *not* available, arm the heuristics just return "vendor" (e.g. dell-laptop, asus-wmi, etc. backlight devs) there.
I'm starting to think that the heuristics should prefer native (when available which the heuristics know) over vendor. But that might break (really) old laptops which rely on vendor control now, but which happen to also have (non functional) native backlight control.
Regards,
Hans
-
Angel Pons -
Geert Stappers -
Hans de Goede -
Peter Stuge -
Tim Crawford