Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

display: Add wait option to handle monitor disconnection #183

Closed
wants to merge 1 commit into from

Conversation

VinDuv
Copy link
Contributor

@VinDuv VinDuv commented Mar 27, 2022

Add an explicit wait for monitor reset in display.c, triggered by the display=wait (or display=wait,<mode…>) boot parameter.

TODO:

  • Check that it works on monitors other than mine
  • Check that it works with payloads
  • Maybe improve parameter parsing so that the order of parameters doesn’t matter
  • Currently the wait for the monitor to disconnect/reconnect is done between dcp_ib_swap_begin and dcp_ib_swap_set_layer. The wait is now done before the swap, relying on stage1 m1n1 to wake up the monitor. However, this will fail to initialize the monitor if too much time passes between stage1 m1n1 and stage2 m1n1, allowing the monitor to fully go back to sleep between the two. That is probably not a huge concern.

@VinDuv
Copy link
Contributor Author

VinDuv commented Mar 28, 2022

I modified the patch so it only waits for disconnection before setting the mode and performing the swap, relying on the fact that the monitor is already waking up from stage1 m1n1. This works and allow the monitor to display something during the wait when a chainload is performed.

I tried to add u-boot payloads but for some reason that caused the monitor initialization to fail. Will investigate this evening.

@marcan
Copy link
Member

marcan commented Mar 28, 2022 via email

@VinDuv
Copy link
Contributor Author

VinDuv commented Mar 28, 2022

Thanks for the heads-up! That was indeed the problem. It seems to work fine with payloads now.
I modified the patch to add a configurable wait delay.
Now to see if it works with other monitors…

Some monitors disconnect when getting out of sleep mode; they rely on the OS
hotplug support to work properly.

Since stage1 m1n1 initializes the display, the monitor is already getting out
of sleep mode when stage2 m1n1 starts execution. For the display to work,
stage2 m1n1 has to wait for the monitor to disconnect (which unfortunately
can take close to 10 seconds), wait for reconnection, and then proceed
as normal.

Since this process delays boot for 10 seconds on monitors that don’t
disconnect, it is not enabled by default.

To enable it, use the following boot parameter:
display=wait
To specify the wait delay (in seconds):
display=wait:5
To specify the resolution (can also include an explicit wait delay):
display=wait,1980x1080
(The parameters cannot be swapped)

Should fix AsahiLinux#159.

Signed-off-by: Vincent Duvert <vincent@duvert.net>
@possiblemeatball
Copy link

possiblemeatball commented Mar 29, 2022

Hello, good morning from EDT! Wanted to chime in with my personal experience with this patch.

I noticed a similar display bug you are describing, with a Samsung C27JG5X monitor. asahi-installer would successfully complete, but no display upon reboot. This is the monitor I will be describing in this comment! EDIT: wanted to mention this is all connected over HDMI, presumably, to a 2020 M1 mini

To install this patch, I have cloned the asahi-installer and edited the source to add the m1n1 var

# (osinstall.py, asahi-installer)
m1n1_vars.append(f"display=wait,2560x1440")

I will comment that, the base asahi-installer does not make this a rather easy process. I do not have much background with this project, admittedly--the recent Alpha release is what finally caught my eye, however I notice the installer does not allow the end-user (or expert-user for that matter) to modify m1n1 boot arguments. At minimum this patch would be more easily accessible with an accompanying asahi-installer patch, allowing the end (or expert!) user to add boot variables.

After rebooting my Mac mini with this patch applied, I can now see Arch Linux ARM with linux-asahi at full resolution, with the entire Asahi Linux Desktop package installed and functioning as well as it would otherwise.

Wonderful experience overall, & i am hoping this will be upstreamed until GPU support makes it in!

@VinDuv VinDuv marked this pull request as ready for review April 1, 2022 18:58
@VinDuv
Copy link
Contributor Author

VinDuv commented Apr 1, 2022

Okay, it looks like it works. The parser still requires “wait[:x],” to come before the specified resolution (if any), but I don’t think it’s worth making it more complicated to handle this.

@VinDuv VinDuv changed the title WIP: display: Add wait option to handle monitor disconnection display: Add wait option to handle monitor disconnection May 24, 2022

if (!config || !strcmp(config, "auto"))
if (!config)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep the mode parsing code as is. I think it makes sense to keep it as first parameters for the display= variable. Have a look how I added support for comma separated parameters in #212 for retina

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, will do


if (!config || !strcmp(config, "auto"))
if (!config)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep the mode parsing code as is. I think it makes sense to keep it as first parameters for the display= variable. Have a look how I added support for comma separated parameters in #212

int max_retries = wait_delay * 1000 / DISPLAY_STATUS_DELAY;

for (int retries = 0; retries < max_retries; retries += 1) {
hpd = dcp_ib_get_hpd(iboot, &timing_cnt, &color_cnt);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dcp_ib_get_hpd can be called with NULL for timing_cnt and color_cnt, would be simpler here since we are just waiting for hpd == 0

int max_retries = wait_delay * 1000 / DISPLAY_STATUS_DELAY;

for (int retries = 0; retries < max_retries; retries += 1) {
hpd = dcp_ib_get_hpd(iboot, &timing_cnt, &color_cnt);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dcp_ib_get_hpd can be called with NULL for timing_cnt and color_cnt

@marcan
Copy link
Member

marcan commented Aug 30, 2022

I think this is obsolete with the DCP shutdown hack.

@marcan marcan closed this Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants