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

init: Do not disable 'ondemand' governor on shift #261

Merged
merged 1 commit into from
Dec 2, 2024

Conversation

roliver-rpi
Copy link

No description provided.

@roliver-rpi
Copy link
Author

@XECDesign and @spl237 , I would appreciate any input you might have.

Cheers,
Richard

@spl237
Copy link

spl237 commented Nov 8, 2024

Not an area I know anything about, so fine with me unless Serge has any concerns?

@XECDesign
Copy link
Member

Nope, I'm glad to see this go.

Doesn't etc/default/cpu_governor also need to be in maintscript?

Also, wouldn't it mean people will be stuck with powersave unless they upgrade the kernel to a version that's not in apt yet? If that's the case, then I don't think we can merge it until that kernel has been out for a while.

@roliver-rpi
Copy link
Author

cpu_governor is less critical as it won't cause any ill-effects if it's left on-disk; I'm happy to actively remove it though if we think that's for the best? It might be possible to get that one with the 'remove-on-upgrade' flag in conffiles.

In terms of being stuck on powersave, would get get away with marking this package as 'Breaks' or 'Conflicts' with older kernel versions such that the new version of this package doesn't get selected until the kernel version moves on?

@roliver-rpi roliver-rpi marked this pull request as draft November 8, 2024 16:31
@XECDesign
Copy link
Member

In terms of being stuck on powersave, would get get away with marking this package as 'Breaks' or 'Conflicts' with older kernel versions such that the new version of this package doesn't get selected until the kernel version moves on?

I think some set of users will see that there's a package update being held back and try to install it anyway, which will result in the kernel getting deleted. I think it's best to just wait until the new kernel has been out for a few months and then make this change.

@timg236
Copy link

timg236 commented Nov 11, 2024

Is it still worth changing it so that it's no-longer and init service and dropping the SHIFT key behavior? There still needs to be something that reads /etc/default and picks the on-demand parameters but there's no need for this to be interactive

@XECDesign
Copy link
Member

Is it still worth changing it so that it's no-longer and init service and dropping the SHIFT key behavior? There still needs to be something that reads /etc/default and picks the on-demand parameters but there's no need for this to be interactive

Yeah, good point.

@roliver-rpi
Copy link
Author

Is it still worth changing it so that it's no-longer and init service and dropping the SHIFT key behavior? There still needs to be something that reads /etc/default and picks the on-demand parameters but there's no need for this to be interactive

I'll put a change together that implements this.

@XECDesign
Copy link
Member

XECDesign commented Nov 22, 2024

A quick and easy step might be to just remove the use of thd and timeout and just default to the behaviour that happens when shift is not pressed.

@roliver-rpi
Copy link
Author

I've forced-pushed my no_shift branch; this removes the need for a separate service all together as a combination of udev/tmpfiles rules are used to select/configure the ondemand governor.

This change removes the raspi-config SysVinit script that configures the
cpu governor during boot based on 'shift' key status. The
raspberrypi-sys-mods package now takes resposibility for the
selection/configuration of the ondemand cpu governor. The
/etc/default/cpu_governor configuration file is also removed by this
change.
@XECDesign XECDesign merged commit 2ff934f into RPi-Distro:bookworm Dec 2, 2024
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