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

feat: Add HPET configuration from Atomic Studio #1726

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

EPOCHvoyager
Copy link
Contributor

I have to preface this by stating that I do not understand the underlying mechanisms adequately myself - but after speaking to the Atomic Studio dev last month, I'm told this tweak might help those facing audio issues, particularly on constrained handhelds - and that it should have no ill effect otherwise.

Atomic Studio currently implements it here.

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. enhancement New feature or request labels Oct 5, 2024
@EPOCHvoyager
Copy link
Contributor Author

Do note that this is my first technical PR, and that as such this might not be the proper or preferable way of implementing this.

@HikariKnight
Copy link
Member

@tulilirockz you might be able to give more insight in this for us as to how this works and help.

@@ -0,0 +1 @@
dev.hpet.max-user-freq=3072
Copy link
Member

@HikariKnight HikariKnight Oct 8, 2024

Choose a reason for hiding this comment

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

This is not needed when you add the file in the desktop image, as the deck image is built from the desktop image and get the file through that.
If we needed the number to be different for the deck image then this file would be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, good to know! Sending in another commit to fix this.

@RayBa82
Copy link

RayBa82 commented Oct 10, 2024

Cool I could approve that, shouldn't I be a maintainer or something? :-D
But fixing audio issues is always nice :-)

@HikariKnight
Copy link
Member

HikariKnight commented Oct 17, 2024

After having talked to @tulilirockz about the details of this, i don't think i will be approving this as it would increase power consumption (and the desktop image is also used on laptops) and battery drain for little to no gain for gaming audio.

They also mentioned that if this did get merged, make sure it does not get included in the -deck images.

I think this instead would make more sense as a ujust command, that way it is optional and we can also throw in a warning that it will increase power consumption.
The difference in location then is just that you write the file to /etc/sysctl.d/ instead using bash in the justfile.

@EPOCHvoyager
Copy link
Contributor Author

Alright; with that in mind, I think it might be even better to implement as part of a ujust setup-realtime command - which could automatically layer the setup-realtime package and start the appropriate systemd services, with this little tweak as a bonus. Though with RT being merged for kernel 6.12, it could be better to wait out a bit.

@HikariKnight
Copy link
Member

HikariKnight commented Oct 17, 2024

Alright; with that in mind, I think it might be even better to implement as part of a ujust setup-realtime command - which could automatically layer the setup-realtime package and start the appropriate systemd services, with this little tweak as a bonus. Though with RT being merged for kernel 6.12, it could be better to wait out a bit.

Does setup-realtime add any kernel modules? if so it cant be done on the users end.
also i think setup-realtime-audio would be more descriptive even though it would have a description.

@EPOCHvoyager
Copy link
Contributor Author

Does setup-realtime add any kernel modules?

No - but the realtime-setup package adds a service(of two, the other is a workaround for network interrupts) that specifically supports a RHEL-RT kernel, which might become regular upstream after 6.12. It's mainly just an /etc/limits.d/ configuration file and some user group management under the hood, as far as I can tell.

also i think setup-realtime-audio would be more descriptive even though it would have a description.

It would be the main use most likely, but there are others. I think mostly sharing the name with the package could leave less guesswork to the user?

@HikariKnight
Copy link
Member

Does setup-realtime add any kernel modules?

No - but the realtime-setup package adds a service(of two, the other is a workaround for network interrupts) that specifically supports a RHEL-RT kernel, which might become regular upstream after 6.12. It's mainly just an /etc/limits.d/ configuration file and some user group management under the hood, as far as I can tell.

also i think setup-realtime-audio would be more descriptive even though it would have a description.

It would be the main use most likely, but there are others. I think mostly sharing the name with the package could leave less guesswork to the user?

if it does more than audio then setup-realtime works better
i would say maybe wait with this until bazzite is on the 6.12 kernel?
can still prep the PR but add the note to not merge until bazzite uses the 6.12 kernel

@EPOCHvoyager
Copy link
Contributor Author

I'll convert this PR to a draft for now.

@EPOCHvoyager EPOCHvoyager marked this pull request as draft October 17, 2024 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants