Fix disp_mode boot option on sunxi

[matthijskooijman]

Description

Setting the display mode in armbianEnv.txt no longer worked, probably because it used an old, sunxi-specific interface. See the commit message (also shown below) for details about this.

I'm marking this as draft, because:

• I'm not sure if we should reuse the disp_mode variable, or maybe use a different name (video_mode?) to make it clearer that the values it accepts are of a slightly different format (would also help to see that older documentation does not apply to this option).
• This fixes just sunxi, which I've been able to test only on a single board (Orange Pi PC). I'm not sure if similar changes are needed elsewhere (see last point in commit message).

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.

• Run original image for Orange Pi PC, set disp_mode=1280x720p60 and see that the monitor input stays at the default 1920x1080.
• Run the modified image for Orange Pi PC, set disp_mode=1280x720p60 and see that the monitor input changes to 1280x720 as expected.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation - seems disp_mode is not really documented?
• My changes generate no new warnings
• Any dependent changes have been merged and published in downstream modules N/A

Commit message:

The disp_mode variable can be set in armbianEnv.txt and boot-sunxi.cmd (boot.scr on the final image) takes care of putting its value in the disp.screen0_output_mode variable on the kernel commandline. However, it seems that this variable is no not actually used by the kernel, probably because it applied to the sunxi video driver from the sunxi-3.x legacy kernels.

On newer kernels, it seems that the standard way to force drm/kms to use a specific mode is through the "video=" kernel argument, as documented here:

https://wiki.archlinux.org/title/kernel_mode_setting#Forcing_modes

This commit adopts to this new way, by updating the boot script to pass the disp_mode variable to the kernel's video= parameter now.

Some additional notes:

• The format of the variable did slightly change, exchanging the "p" for progressive with an @ to indicate the refresh rate, and the video= parameter also allows some additional details (like specifying a specific video connect, bit depth and other info). This means that existing configs are not going to end up work as-is (tried, didn't work), but that seems ok.
• This comments out the forced 1920x1080 mode (and removes it from the boot script), leaving the default to just autodetect the resolution (which is already the current behavior, given the old option did not work).
• This also removes the hdmi.audio option from the commandline, which was probably also specific to the old video driver (without it HDMI audio still works normally on an Orange Pi PC).
• Note that the mode passed to video= is actually added to the list of supported modes (from EDID) as a userdef mode (shown in drm_info output), possibly replacing an existing mode, or adding a new mode if the display does not advertise support for it (no guarantee that the display can actually use the mode, of course).
• All current sunxi kernel branches, including legacy, seem to use mainline kernels now, so supporting the old way of setting the video mode should no longer be needed.
• The old "disp.screen0_output_mode" commandline is still used by nfs-boot.cmd.template and the armbian-install and nand-sata-install commands. Additionally, the screenx_output_mode variable is also still used by various (mostly sun50i) DTS files and FEX scripts. These might need a similar treatment, but I cannot oversee the consequences (and have no hardware to test).

[Kreyren]

@matthijskooijman

Is this stale? -> close

is there any expected action to be taken? -> Elaborate

Is this still in works? -> Elaborate what needs to be done

[matthijskooijman]

I'm waiting for a review, hopefully from someone a bit more knowledgable about (the armbian policies around) uboot scripts and/or sunxi.

In particular, the following points (repeated from the first point) are left to be addressed:

• I'm not sure if we should reuse the disp_mode variable, or maybe use a different name (video_mode?) to make it clearer that the values it accepts are of a slightly different format (would also help to see that older documentation does not apply to this option).
• This fixes just sunxi, which I've been able to test only on a single board (Orange Pi PC). I'm not sure if similar changes are needed elsewhere (see last point in commit message).

[Kreyren]

I'm not sure if we should reuse the disp_mode variable, or maybe use a different name (video_mode?) to make it clearer that the values it accepts are of a slightly different format (would also help to see that older documentation does not apply to this option).

Sounds like a question for @igorpecovnik to decide i support using video_mode as looking through the code it seems it's more descriptive and i don't see any rationale for disp_mode in the A64's docs

This fixes just sunxi, which I've been able to test only on a single board (Orange Pi PC). I'm not sure if similar changes are needed elsewhere (see last point in commit message).

Can you elaborate what the issue is? I am using armbian-next on OLIMEX Teres-A64 which sets:

./packages/blobs/sunxi/a64/teres-a64.dts: disp_mode = <0x0>;

And i am not aware of having this issue on the current kernel, the edge seems to boot in a blackscreen that i didn't yet diagnose.. is that consistent with the behavior you observed on your board?

[matthijskooijman]

Can you elaborate what the issue is? I am using armbian-next on OLIMEX Teres-A64 which sets:

It looks like you are talking about a different issue. You're referencing the disp_mode variable in the devicetree, my PR is about the disp_mode variable in armbianEnv.txt that is processed by the u-boot script and used in the disp.screen0_output_mode kernel commandline option (before this PR) or video kernel commandline option (after this PR).

The issue I was having was only that forcing a particular mode through armbianEnv.txt did not work, video output did otherwise work normally.

[ColorfulRhino]

🧹 PR Cleanup Quest 🧹

Ping @matthijskooijman

Can this be merged after changing the variable to video_mode like @Kreyren supports?

[Kreyren]

🧹 PR Cleanup Quest 🧹 -- @ColorfulRhino (#4399 (comment))

Oh cool, i was sent on a similar quest once! It lasted 7 min, made igor very angry and i was asked to never do that again~

Can this be merged after changing the variable to video_mode like @Kreyren supports? -- @ColorfulRhino (#4399 (comment))

I would think that just adjusting the DTS would be better option, but i am the kind of guy who hates fixes downstream and rather fixes upstream so it might be invalid here and would prefer no touchy my board with un-needed bloat ^=^

[Kreyren]

The issue I was having was only that forcing a particular mode through armbianEnv.txt did not work, video output did otherwise work normally. -- @matthijskooijman (#4399 (comment))

Actually looking through this again, it seems that it might be relevant to #6630, but i doubt it as that issue seems to be a regression in u-boot that i am currently looking through, though please correct me if i am wrong as it would save me a lot of pain and time

[ColorfulRhino]

🧹 PR Cleanup Quest 🧹 -- @ColorfulRhino (#4399 (comment))

Oh cool, i was sent on a similar quest once! It lasted 7 min, made igor very angry and i was asked to never do that again~

Oh no 😅 I'm just trying to help spending my free time to look through the PRs with the Backlog label and see if they are still relevant, need to be updated or can be merged. I'm not trying to annoy anyone :D But some of those PRs were even from 2022 without any recent comments.

Can this be merged after changing the variable to video_mode like @Kreyren supports? -- @ColorfulRhino (#4399 (comment))

I would think that just adjusting the DTS would be better option, but i am the kind of guy who hates fixes downstream and rather fixes upstream so it might be invalid here and would prefer no touchy my board with un-needed bloat ^=^

If this issue can be solved by updating something in the devicetree, I agree that it's probably a cleaner solution. It doesn't have to be fixed upstream though, converting this PR into a dts patch would also work (at least for Armbian).

[matthijskooijman]

This PR has nothing to do with the devicetree, it is about u-boot script parameters that add kernel comandline parameters. @Kreyren mentioned devicetrees, but I think because they misunderstood. See also #4399

I also think that #6630 is not related at all. That talks about a regression where the display does not work at all. This PR is about u-boot variables to force a particular mode (but without setting these variables, the kernel should just autodetect the best mode already).

As for moving this PR forward, I am fine with renaming the armbianEnv.txt (sunxi.txt in the source tree) variable disp_mode to video_mode if that would be better, but another question is also still open:

This fixes just sunxi, which I've been able to test only on a single board (Orange Pi PC). I'm not sure if similar changes are needed elsewhere.

The old "disp.screen0_output_mode" commandline is still used by nfs-boot.cmd.template and the armbian-install and nand-sata-install commands. Additionally, the screenx_output_mode variable is also still used by various (mostly sun50i) DTS files and FEX scripts. These might need a similar treatment, but I cannot oversee the consequences (and have no hardware to test).

As for the variable name, writing this I realize that it might be best to just name the variable video to match the kernel commandline option that it controls, which could make things more predictable (even though video is a slightly less clear and more generic name than something like video_mode).

[Kreyren]

This PR is about u-boot variables to force a particular mode (but without setting these variables, the kernel should just autodetect the best mode already). -- @matthijskooijman (#4399 (comment))

In general the kernel doesn't autodetect anything on arm as this isn't ACPI like x86_64, it's either in DeviceTree or the functionality is braindead.. (with some caviats from already defined values and pre-dts era) see e.g. https://www.kernel.org/doc/Documentation/devicetree/bindings/display/panel/panel-timing.yaml

But yea it can be set in a command line as well

[matthijskooijman]

This PR (or at least my usecase for it) is about overriding resolutions on an HDMI output, which are normally autodetected using EDID or something like that.

This is distinct from the usecase of e.g. LVDS or DSI panels, where (AFAIU) there is no autodetection and the resolution must be manually configured. But as you say, in that case the devicetree is probably the best place for this configuration (I'm not even sure if the video= option actually works as an alternative for devicetree config there). In any case, this is not the usecase of my PR and, I think, also not the usecase for supporting this kernel commandline option from armbianEnv.txt - that would be overriding HDMI (or similar) output modes.

[ColorfulRhino]

Tbh I can't provide meaningful insights to this discussion since I'm not deep into this stuff or sunxi in gerneral.

I just wanted to see if this PR can be moved forward, with either approach.

It looks to me as the only question remaining is the renaming of the variable. I think this can be done :)

[matthijskooijman]

It looks to me as the only question remaining is the renaming of the variable. I think this can be done :)

I think the question posed here, about making this change in other places as well, is actually more important and also still open: #4399 (comment)

[ColorfulRhino]

I think the question posed here, about making this change in other places as well, is actually more important and also still open: #4399 (comment)

I see your point 👍 Unfortunately I can't help with that since I don't have any of this hardware to test and am lacking knowledge about this topic in general.

[Kreyren]

This is distinct from the usecase of e.g. LVDS or DSI panels, where (AFAIU) there is no autodetection and the resolution must be manually configured. But as you say, in that case the devicetree is probably the best place for this configuration (I'm not even sure if the video= option actually works as an alternative for devicetree config there). In any case, this is not the usecase of my PR and, I think, also not the usecase for supporting this kernel commandline option from armbianEnv.txt - that would be overriding HDMI (or similar) output modes. -- @matthijskooijman (#4399 (comment))

That's an important context that should be better explained in the OP, it sounds like you are trying to hotfix something that sunxi developers spend 5 months fixing and rewriting in mainline, please test kernel 6.8+ to see if it works out of the box for you.

Also if you get a black screen then invoke reboot from booted linux and try this at least 2 times as u-boot on some devices lacks sufficient initialization sequence for some displays e.g. why #6630 is a thing (i plan on submitting an upstream bug in ~2 days to address that).

Additionally 1080p displays are not very common for sunxi so this change still seems like something that should be documented on the sunxi wiki AND armbian page with instructions how to adjust u-boot variables via SSH, telnet or serial console imho as user with non-1080p display probably won't get a functioning display.