Page MenuHomeFreeBSD

Support loader screen rotation for panels mounted at non-native orientations.
Needs ReviewPublic

Authored by crahman_gmail.com on Jul 29 2022, 6:32 AM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 20 2023, 4:50 AM
Unknown Object (File)
Dec 12 2023, 4:51 PM
Unknown Object (File)
Dec 12 2023, 2:29 PM
Unknown Object (File)
Dec 2 2023, 5:08 PM
Unknown Object (File)
Sep 23 2023, 8:47 AM
Unknown Object (File)
Aug 3 2023, 7:46 AM
Unknown Object (File)
Jun 21 2023, 3:55 PM
Unknown Object (File)
May 3 2023, 7:53 PM

Details

Reviewers
manu
imp
Summary

This is one of three patches adding support for non-native panel orientation.
The other two patches add similar support to the drm module, and to the vt framebuffer.

Each of these patches may be applied separately or not, but consistent
rotation through all phases of bootstrapping and running the system console
will require all three.

The screen orientation is set by adding

screen.rotate="angle"

to /boot/loader.conf. This value is passed by the loader to the kernel for use after
bootstrapping, but the loader needs the information before /boot/loader.conf is
available.

To solve this problem, during startup and shutdown the loader.conf file is read and
the appropriate rotation angle is stored in a UEFI variable by /etc/rc.d/uefivars.

Test Plan

Edit /boot/loader.conf to add the screen.rotate key/value, and reboot.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 46671
Build 43560: arc lint + arc unit

Event Timeline

imp requested changes to this revision.Jul 29 2022, 2:58 PM
imp added inline comments.
libexec/rc/rc.d/uefivars
47

why is /boot/loader.conf hard coded?

70

-z is likely a better test.

stand/common/gfx_fb.c
790

I think this is not indented with 8 character tabs, but 4 space characters.

794

angle == 0 is != 90 and != 180 and != 270, so you can drop that part of the test to simplify

799

Style(9) doesn't align continuations like this (which is an emacs default).

890

this should be in a header somewhere.

stand/efi/loader/framebuffer.c
173

why not just use the loader''s own variables instead of this indirection that requires extra effort to set in an UEFI variable at boot?

This revision now requires changes to proceed.Jul 29 2022, 2:58 PM
crahman_gmail.com added inline comments.
stand/efi/loader/framebuffer.c
173

It's necessary to set the framebuffer orientation before the framebuffer is initialized, else functional problems result. The framebuffer is initialized very early, in efi/loader/main.c cons_probe(). This is important, since if it is not, problems with mounting filesystems, etc. will not be reported. This means /boot/loader.conf is not available when we need the rotation information.

/boot/efi/EFI/FreeBSD/loader.env is not currently opened before cons_probe(), but it could be moved there. But this is not a satisfactory solution because:
It's awkward to ask users to edit it;
Opening the EFI filesystem is still a complex enough operation that errors during open should be reported, and failures here could affect booting;
Most importantly, it's not necessarily mounted and writable when the kernel is running. This is important, because there are three sources of panel orientation information:

The user, via /boot/loader.conf;
The ACPI tables, which are read by the drm-kms module when it is loaded;
The drm-kms quirk tables, also only available after this module has loaded.

I have not yet had a chance to write the code to fully use the drm-kms source; it will involve setting screen.rotate="AUTO". However, the code in the drm-kms module is well developed and simply needs to be interfaced.

So information exposed quite late in the boot process needs to be available when the loader is just starting, and it needs to be reliably written late in the kernel boot process and also during shutdown (in case a user edits /boot/loader.conf), and it needs to be easily retrieved without much other infrastructure, such as a filesystem.

The UEFI variable works very well for this.

That said, I am not entirely happy with one thing. The problem is that the uefivars script needs to parse /boot/loader.conf on its own. Of course it can read the screen.rotate variable quite reliably if it's in the /boot/loader.conf file, but the script is currently not going to search through all the various sub-files. It's actually easy to fix this by embedding /boot/lua/config.lua into an executable parser available to shell scripts, but that solution, while already written, involves introducing another complexity into the system. Perhaps it's worth it; it's potentially useful to be able to access /boot/loader.conf configuration from scripts. But for most purposes, kenv(1) provides this functionality. And that's another option, to use kenv(1), but that would mean that after editing /boot/loader.conf, a user would have to reboot twice to see the effect.

  • Modified to address review comments from D35982

Happy consumer of the loader and vt (D35983) patch, thanks a lot (pocket3 PC).
Hope this will make it into main sooner than later!

libexec/rc/rc.d/uefivars
72

debug "DELETE $var" would underline the intended versatility I guess,
likewise some lines below in ' debug "UPDATE $var $config_value"'