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

V56 (v560tu/v540tu) Dasharo-Heads coreboot fork chosen commit needs to be par with Dasharo-UEFI #1249

Closed
tlaurion opened this issue Feb 20, 2025 · 5 comments

Comments

@tlaurion
Copy link

tlaurion commented Feb 20, 2025

I think I've found the issue. The NVidia release had some additional fixes for power and stability (VR loadline, psys), iGPU releases don't have this yet. HWINFO suggests that these settings aren't programmed correctly causing excessive chipset / ring thermal throttling, so I'm applying the same fix to iGPU, and the performance appears better already. Will publish some results today

Originally posted by @mkopec in #1216

For next firmware release of dasharo-heads, which will be less performant, by design, as opposed to nvidia dasharo-UEFI variant, which contained the fixes.

Related to linuxboot/heads#1913 (comment)


Please update release notes known issues pointing to business decision. I do not want Heads users to blame Heads for observed perf differences when comparing to UEFI variant, which will eb linked to coreboot fork unfixed issues.

@macpijan
Copy link
Contributor

There is no fix release for UEFI releases either.

UEFI releases always come first and are used as a base for following heads releases. Once we investigate what the problem (and then solution) is, we will issue UEFI release first, then heads release with these improvements.

@BeataZdunczyk
Copy link
Member

We have already updated the release notes and added the performance issue #1216 to the known issues section of the v0.9.0 V560TU Dasharo (coreboot + Heads) release.

We can edit the issue title on GitHub and in the release notes to make it clear that it is a coreboot issue. Does that sound good @tlaurion ?

@tlaurion
Copy link
Author

tlaurion commented Feb 21, 2025

We have already updated the release notes and added the performance issue #1216 to the known issues section of the v0.9.0 V560TU Dasharo (coreboot + Heads) release.

We can edit the issue title on GitHub and in the release notes to make it clear that it is a coreboot issue. Does that sound good @tlaurion ?

Absolutely, and sorry if this came out rude.
Its a testing outcome, where prior issue for uefi was closed, and where my local tests comparing nv41 performances vs v540tu/v560tu came a bit disappointing.

I'm glad the search for culprit showed that Nvidia branch vs non-Nvidia variant showed tweak commits that were not applied to the non-Nvidia branch. Heads relies on non-Nvidia branch, which consequently didn't have the fix.

This issue aims to make sure that the Heads release follows as closely as possible the uefi one, so that Heads variant does not seem slower than the uefi one from it's users.

I didn't intent to say that uefi variant contained fixes that heads didn't, as if it was intentional. I'm stating that between nvidia/non-Nvidia variants for same platform, an important performance issue was discovered, where my tests between nv41(older) and v56 (more recent) perf loss was linked to a lot of manual work to prove that it was not just a feeling but a reality, which at the end resulted in new IO testing framework from @marmarek under qubesos, to be able to diagnose those issues earlier next time and have data points outside of feelings to refer to to not have to do those manual tasks and compare platforms and repro, and compare between platforms as well (intr/extra group comparison to pinpoint drive issues vs coreboot forks, vs platform variants, if you will).

I understand that this is a lot of work and I'm grateful for all the work that you are doing, which is amazing, and needed, and hard. And recognized.

I do not intend to be a moral downer. Keep up the amazing work you already do. Once more, the urgency was to put this in release notes as close as possible from blog posts, social media hype, massive new users.... Blaming Heads because stock firmware being more performant than Heads firmware for same HCL.

I hope we are on same page and sorry if I was a moral downer.


Traces/cross references

Which led to

@tlaurion tlaurion changed the title V56 (v560tu/v540tu) Daaharo-Heads coreboot fork chosen commit needs to be par with Dasharo-UEFI V56 (v560tu/v540tu) Dasharo-Heads coreboot fork chosen commit needs to be par with Dasharo-UEFI Feb 21, 2025
@BeataZdunczyk
Copy link
Member

@tlaurion Thank you for explaining, I appreciate it.

We have updated the release notes: Dasharo/docs#1007. Please let us know if this addresses your issue for now.

I understand that this is a lot of work and I'm grateful for all the work that you are doing, which is amazing, and needed, and hard. And recognized.
I do not intend to be a moral downer. Keep up the amazing work you already do. Once more, the urgency was to put this in release notes as close as possible from blog posts, social media hype, massive new users.... Blaming Heads because stock firmware being more performant than Heads firmware for same HCL.
I hope we are on same page and sorry if I was a moral downer.

Our goal is to ensure a great Dasharo experience across all supported platforms, though not every issue can be fixed immediately for every platform. A fix for this platform will be included in the next release.

Thank you again!

@BeataZdunczyk
Copy link
Member

Closing the issue based on your reaction @tlaurion

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

No branches or pull requests

3 participants