OS Reboot with i915 Enabled Disables Internal Display - x230 fhd_edp (a.gain)

[lethedata]

Basic Details

A. Provide Hardware Details

1. What board are you using (see list of boards here)?
x230-maximized-fhd_edp

Mod Kit: a.gain X220_X230 FHD RevA1.0

2. Does your computer have a dGPU or is it iGPU-only?

• iGPU-only

3. Who installed Heads on this computer?

• Self-installed

4. What PGP key is being used?

• Yubikey

5. Are you using the PGP key to provide HOTP verification?

• No

B. Identify how the board was flashed

1. Is this problem related to updating heads or flashing it for the first time?

• Updating heads

2. If the problem is related to an update, how did you attempt to apply the update?

• Using the Heads GUI

3. How was Heads initially flashed

• External flashing

4. Was the board flashed with a maximized or non-maximized/legacy rom?

• Maximized

5. If Heads was externally flashed, was IFD unlocked?

• Yes

C. Identify the rom related to this bug report

1. Did you download or build the rom at issue in this bug report?

• I downloaded it

2. If you downloaded your rom, where did you get it from?

• Heads CircleCi

Please provide the release number or otherwise identify the rom downloaded
heads-x230-maximized-fhd_edp-v0.2.0-2022-g5bff519

Problem

Describe the bug
After updating from an older version (early Feb 2023 I think) of Heads rebooting from OS doesn't seem to work. All the systems lights turn off (USB still blinks if plugged in) but the system hangs in that state appearing to be powered off. Holding the power button leads to the "pop" sound as the system is forced shutdown. I experienced this in both the latest Fedora and QubesOS.

To Reproduce
Steps to reproduce the behavior:

1. Boot OS
2. Login
3. Reboot device

Expected behavior
Successful reboot of system

Additional context
Rebooting from heads itself works without issue. Non-fhd_edp x230 appears to works without issue.
heads-x230-maximized-fhd_edp-v0.2.0-1978-gc9e067c the oldest version I could download also has the issue

Additional Tests

• Alternative heads versions
  • c4b964c : Functional
  • 5c7148f : Functional
  • 6300dd1 : Functional
  • 1f39d16 : Failed reboot
  • heads-x230-maximized-fhd_edp-v0.2.0-1978-gc9e067c : Failed reboot
  • heads-x230-maximized-fhd_edp-v0.2.0-1978-gc9e067c with stock vbt : Failed reboot
  • non-fhd board
• Test Coreboot with SeaBIOS payload

[lethedata]

Not really sure of any other troubleshooting method except building out some of the older versions till one works. Any ideas?

[tlaurion]

The edp patch changed last time at 6300dd1 so about 1 year ago.

This coreboot borrowed patch was never merged upstream as referred per commit reference to upstream unmerged https://review.coreboot.org/c/coreboot/+/28950

First time this issue poweroff preventing issue is reported downstream/upstream.

Per #692 edp known reported board owners/willingntesters: x230-fhd/edp variant: @n4ru @computer-user123 (nitro caster board) @Tonux599 @househead @pcm720 (eDP 4.0 board and 1440p display)

Can you replicate this issue and collaborate under coreboot to fix it?

Some historic:

• Patchset introduced under coreboot in 2018.
• Testers asked to test and report upstream since then.
• Not enough tests reported in time before a commit release nor commented upstream to resolve issues since then prior of coreboot release since then. Tried myself but not having the board with mod doesn't qualify myself as someone that can do this. Board owners need to participate there.
• Tested by users under Heads since coreboot 4.13 where it was first introduced under 2b05a6b, and then ported to 4.19 but still not merged upstream today.

Consequently, if no collaboration happens under coreboot by current board owners, which some are developers who could collaborate upstream to make this work for all downstream projects not just Heads, and for Heads to simply build for edp/fhd variant without having to maintain downstream the patch (this edp board being one of the reasons we don't bump boards to 4.22), edp variant will most probably be dropped under Heads soon

I never thought that by introducing a board under Heads I would be responsible to bridge communications upstream or maintain a downstream patch forever with coreboot releases every 3 months with coreboot still needing reports from users to maintain and merge the patchset which otherwise will never be merged. I won't maintain this patchset myself.

So edp/fhd board owner variants: please participate under https://review.coreboot.org/c/coreboot/+/28950 and report here when done. Upstream policy is: unless all comments "closed" (reviewed) a patchset is not merged. Each time a new comment is made, it needs to be addressed and closed, normally with a subsequent patch addressing past unresolved comments. When consensus is met, patchset is merged.

[pcm720]

Cannot replicate on the latest Linux kernel currently available in Arch Linux (Linux 6.7.3-arch1-1) and heads-x230-maximized-fhd_edp-v0.2.0-2022-g5bff519. Reboot works as expected for me across several back-to-back reboots with and without battery inserted.

[tlaurion]

owners/willing testers: x230-fhd/edp variant: @n4ru @computer-user123 (nitro caster board) @Tonux599 @househead @pcm720 (eDP 4.0 board and 1440p display)

As one can read upstream, some mods doesn't require patched vbt or edp to be enabled to drive Fhd? Basically, each board owner here should state clearly there what is the mod they installed (soldering involved or not?) and report upstream about functional/not functional state and only then upstream final patchset should state what kits are covered to be working with modified vbt and board variant. As stated there, using edp board variant might not work depending of the kit used and this is most probably why this patchset was never merged usptream yet.

[pcm720]

@tlaurion, like I mentioned in some previous issue or MR, I don't know what kind of participation is expected from us to get this patch merged. It has always worked perfectly for me.

What this patch actually does is disabling the internal LCD and setting external display as internal so that OS only sees one internal display, it has nothing to do with the actual brightness control or driving external display since it's connected to DisplayPort on the docking connector. All mods work fine without the patch, but without it the OS sees two displays: one internal (that's no longer connected) and external (the one actually installed).

In the comments that are supposed to be resolved before this patch can be accepted on coreboot's Gerrit, I don't see anything actually blocking and requiring any rewrites, it looks like someone just has to accept those as "resolved". As far as I'm aware, all variations of this mod are working fine with the patch if they were installed as intended by their creators.

In my opinion, everything else (like controlling VCC3P as suggested by some X220 patches) should be considered unsupported and YMMV unless someone decides to implement it in another patchset.

I left a comment under src/mainboard/lenovo/x230/Kconfig.name with details about the mod I have in my X230 if that's of any help.

[lethedata]

I'd agree with @pcm720 on the outstanding comments preventing getting it merged upstream. The VCC3P should be a separate future patch but shouldn't prevent the current patch from being merged.

I'm currently in the process of building c4b964c the parent patch to 6300dd1 tho my hunch is that this issue is unrelated to the edp specifically as heads has no issues. The next commit I will try is 5c7148f which I think is the version I had before updating.

In the meantime trying a few different things as it builds. I installed the latest Arch Linux and the system is now acting slightly differently leaving the mute lights. The battery indicator has remained on no matter what OS is installed which I just noticed. -- Didn't end up getting anywhere with this.

Just had a bad flash by my own doing and don't have access to tools to externally flash atm so have to put further investigations on hold.

[tlaurion]

I'd agree with @pcm720 on the outstanding comments preventing getting it merged upstream. The VCC3P should be a separate future patch but shouldn't prevent the current patch from being merged.

I'm currently in the process of building c4b964c the parent patch to 6300dd1 tho my hunch is that this issue is unrelated to the edp specifically as heads has no issues. The next commit I will try is 5c7148f which I think is the version I had before updating.

In the meantime trying a few different things as it builds. I installed the latest Arch Linux and the system is now acting slightly differently leaving the mute lights. The battery indicator has remained on no matter what OS is installed which I just noticed. -- Didn't end up getting anywhere with this.

Just had a bad flash by my own doing and don't have access to tools to externally flash atm so have to put further investigations on hold.

@lethedata if you keep Linux logs in your os, you can grep journalctl logs for Heads coreboot/linux and find past version used.

What I meant prior with my commit traces was to show that the only changes there were bumping coreboot 4.13 to 4.19 with edp patch having changed a little between those. One would have to change that patch against 4.19 or 4.22 (master) to be accepted upstream, pushing the idea that any other mod requiring soldering or anything not covered in current patchset will need to be done subsequently. My understanding of the patchset not getting merged is because of the scope bing too large, and the kits covered not being enough specific in the commit messages.

As @pcm720 said prior, he cannot reproduce the reboot issue. I am not sure why OS does what it needs to do to reboot while the power lines are kept alive in your machine. To be honest I didn't understand your comment @lethedata stating that the mute button stays in some undesired state either; all of which behaviors are driven by EC firmware+OS kernel, not coreboot nor the patch and where Heads job stops when the OS is kexec'ed so I have no insight here but pushing back to: what EC version and what is the Edp kit used to obtain a different and non-reproducible behavior just between you too. As you noted, Heads can poweroff/reboot (which uses sysrq magic but at the end, all the same).

That is why the patchset is not yet merged upstream IMO. At least that is my understanding and seems to be the understanding of coreboot devs being reviewers not having edp mod, and just like me, being a bit too confused about this since 2018 to make things go forward where past iterations went nowhere near a merge.

I poked Felix off channel to point here for at least advices on how to move this forward.

[lethedata]

@lethedata if you keep Linux logs in your os, you can grep journalctl logs for Heads coreboot/linux and find past version used.

I wiped the drive to install something new before I realized there was an issue unfortunately. Noted for next time tho.

What I meant prior with my commit traces was to show that the only changes there were bumping coreboot 4.13 to 4.19 with edp patch having changed a little between those. One would have to change that patch against 4.19 or 4.22 (master) to be accepted upstream, pushing the idea that any other mod requiring soldering or anything not covered in current patchset will need to be done subsequently. My understanding of the patchset not getting merged is because of the scope bing too large, and the kits covered not being enough specific in the commit messages.

That makes sense, to many boards floating around there's not enough info to tell what works and what doesn't. I'd say there is also a knowledge gap with the people running the mods. Get it running and just leave it making keeping things up to date and tested much harder.

As @pcm720 said prior, he cannot reproduce the reboot issue. I am not sure why OS does what it needs to do to reboot while the power lines are kept alive in your machine. To be honest I didn't understand your comment @lethedata stating that the mute button stays in some undesired state either; all of which behaviors are driven by EC firmware+OS kernel, not coreboot nor the patch and where Heads job stops when the OS is kexec'ed so I have no insight here but pushing back to: what EC version and what is the Edp kit used to obtain a different and non-reproducible behavior just between you too. As you noted, Heads can poweroff/reboot (which uses sysrq magic but at the end, all the same).

The comment on the mute buttons and such are unrelated, its just what I'm seeing that shows that the system is keeping power for whatever reason.

Hopefully I'll have time later this week to tear the device open, re-flash externally, and get more information about what Edp kit is installed. It was already modded when I received it so didn't really look to deeply into things.

[tlaurion]

@lethedata

It was already modded when I received it so didn't really look to deeply into things.

I think this is exactly why the patch is not merged: the fear of causing more harm then good. If you flash this board config with a different kit, you get a brick. Coreboot won't merge this until this is clarified. I hope we have an understanding that this is upstream work needing to happen where Heads cannot replace coreboot discussions here. Heads is a mere user of coreboot patchsets, which this one was picked at a time where it was thought to be merged in coreboot 4.14 which never happened. I reworked and interacted with coreboot before 4.20 release, hoping this would be the last time this patchset was going to be maintained by me for Heads integration. And then I attempted again before 4.22 release and gave up.

This is a lot of time and as said in other tickets, if this patchset doesn't get merged into coreboot, I will bump coreboot version soon to 4.22 for all other coreboot 4.19 depending boards and edp variant will go unsupported at the same tjme under Heads until situation is fixed upstream.

Heads building for 5 different coreboot buikdstack, that Circleci has to build and cache doesn't scale at all. Boards currently depending on 4.19, while other forks depend on 4.20, 4.11 etc is really costly for QA and build time and CI resources. That, added with multiple kernel versions is also time consuming for CI and maintaining best practices across supported boards. Heads need unification here, and to do so, it was concluded in the past that patches maintained under Heads are there only waiting to be merged upstream. Ideally, patches/coreboot* directories don't contain anything other then reproducible patches or heads related patches. The goal here is to keep them there as a reminder of what is supposed to be merged upstream. Heads, and it's users/collaborators are supposed to contribute upstream to minimize what is kept downstream. The goal here is to have Heads benefit the ecosystem it uses, not maintaining what should be maintained, and fixed for all, upstream. I'm not blaming coreboot here, but it's the same for all modules/* configurations here and patches/* under heads tree: they should be as minimal as possible and heads should focus on strengthening security policies and usability for all supported boards, and that is what is under initrd/* files, nothing else.

[pcm720]

@tlaurion

I think this is exactly why the patch is not merged: the fear of causing more harm then good. If you flash this board config with a different kit, you get a brick.

I still don't believe that this patch can cause a brick.
Like I mentioned in my previous comment, it doesn't do anything except disabling internal display and changing the external port on dock connector as internally connected.

Coreboot won't merge this until this is clarified. I hope we have an understanding that this is upstream work needing to happen where Heads cannot replace coreboot discussions here.

Let's work on upstreaming it together then.

While I'm interested in getting this patch merged, I don't have any eDP mods other than eDP 4.0 and I'm not willing to get another X230 and spend ~120 USD to mod it.
How long is the merge window before 4.22 is released? Since this mod is pretty niche, I doubt we'll be able to collect enough information from other users, so we'll have to work with what we already have.

Apart from eDP 4.0 mod, we know at least two users with nitrocaster's mod that reportedly have no issues running Heads on their X230s. It doesn't support anything higher than 1920x1080 due to how works (it connects only two DisplayPort lanes instead of four).
All of these reports are pretty old though...

We also know that a.gain mod works with the patch as well since Xue Yao (XY Tech) has been selling pre-modded X230 and recommended this patch to customers.

So that's at least three mods working with the patch we can mention in the merge request.
If we need any more details on mods, I guess we'll need to ping anyone else we know that has this mod installed and ask them to chip in.

[tlaurion]

How long is the merge window before 4.22 is released?

4.22 is released. Meaning if this patch, rebased on coreboot master/4.22 works then heads can use 4.22 under modules/coreboot which is endgoal here.

[pcm720]

4.22 is released. Meaning if this patch, rebased on coreboot master/4.22 works then heads can use 4.22 under modules/coreboot which is endgoal here.

Well, okay then, I guess I overlooked it.
I also see that you've already updated the patchset for 4.22 around three months ago. So how can we test if this patch works?
There was some activity back in November (#1524), but it got nowhere because CI build was failing for some reason.
Are there any updates on this?

I'd test it myself, but I can't build Heads on my Debian 12 VM, even the latest commit that CI has no issues building.
For some reason, building flashrom fails on musl-gcc being unable to resolve references to pci_* functions when linking.

[tlaurion]

I'll try rebuilding clean on top of master after updating that PR real quick.

[computer-user123]

@tlaurion - I will test anything we need to test, especially pertaining to x230-FHD-eDP (nitro caster) If it’s not too much trouble for you, please send the url to circle ci artifact, and I will download and test tonight ! Thanks man! Appreciate your hard work and this project a lot!

…

On Mon, Feb 5, 2024 at 10:51 AM tlaurion ***@***.***> wrote: I'll try rebuilding clean on top of master after updating that PR real quick. — Reply to this email directly, view it on GitHub <#1603 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ASIUQUEVHDZLOZ5666MELDDYSD5ZXAVCNFSM6AAAAABCXUZ3PGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRXGMYDOOBVHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>