Disallow setting max < min and min > max because the kernel now does too

This commit is contained in:
NGnius (Graham) 2023-01-16 18:46:40 -05:00
parent 12e75cd2f3
commit 8332649e45
5 changed files with 194 additions and 121 deletions

View file

@ -159,8 +159,18 @@ pub fn set_clock_limits(
if let Some(&Primitive::F64(index)) = params_in.get(0) {
if let Some(&Primitive::F64(min)) = params_in.get(1) {
if let Some(&Primitive::F64(max)) = params_in.get(2) {
setter(index as usize, MinMax {min: min as u64, max: max as u64});
vec![min.into(), max.into()]
let safe_max = if max < min {
min
} else {
max
};
let safe_min = if min > max {
max
} else {
min
};
setter(index as usize, MinMax {min: safe_min as u64, max: safe_max as u64});
vec![safe_min.into(), safe_max.into()]
} else {
vec!["set_clock_limits missing parameter 2".into()]
}

View file

@ -78,11 +78,21 @@ pub fn set_clock_limits(
move |params_in: super::ApiParameterType| {
if let Some(&Primitive::F64(min)) = params_in.get(0) {
if let Some(&Primitive::F64(max)) = params_in.get(1) {
let safe_max = if max < min {
min
} else {
max
};
let safe_min = if min > max {
max
} else {
min
};
setter(MinMax {
min: min as _,
max: max as _,
min: safe_min as _,
max: safe_max as _,
});
vec![(min as u64).into(), (max as u64).into()]
vec![(safe_min as u64).into(), (safe_max as u64).into()]
} else {
vec!["set_clock_limits missing parameter 1".into()]
}

View file

@ -32,7 +32,8 @@ pub enum BatteryMessage {
}
impl BatteryMessage {
fn process(self, settings: &mut dyn TBattery) {
fn process(self, settings: &mut dyn TBattery) -> bool {
let dirty = self.is_modify();
match self {
Self::SetChargeRate(rate) => settings.charge_rate(rate),
Self::GetChargeRate(cb) => cb(settings.get_charge_rate()),
@ -43,6 +44,12 @@ impl BatteryMessage {
Self::ReadChargeDesign(cb) => cb(settings.read_charge_design()),
Self::ReadCurrentNow(cb) => cb(settings.read_current_now()),
}
dirty
}
/// Message instructs the driver to modify settings
fn is_modify(&self) -> bool {
matches!(self, Self::SetChargeRate(_) | Self::SetChargeMode(_))
}
}
@ -59,7 +66,8 @@ pub enum CpuMessage {
}
impl CpuMessage {
fn process(self, settings: &mut dyn TCpus) {
fn process(self, settings: &mut dyn TCpus) -> bool {
let dirty = self.is_modify();
// NOTE: "cpu" refers to the Linux kernel definition of a CPU, which is actually a hardware thread
// not to be confused with a CPU chip, which usually has multiple hardware threads (cpu cores/threads) in the chip
match self {
@ -129,6 +137,18 @@ impl CpuMessage {
cb(result);
}
}
dirty
}
/// Message instructs the driver to modify settings
fn is_modify(&self) -> bool {
matches!(self,
Self::SetCpuOnline(_, _)
| Self::SetCpusOnline(_)
| Self::SetClockLimits(_, _)
| Self::SetCpuGovernor(_, _)
| Self::SetCpusGovernor(_)
)
}
}
@ -142,7 +162,8 @@ pub enum GpuMessage {
}
impl GpuMessage {
fn process(self, settings: &mut dyn TGpu) {
fn process(self, settings: &mut dyn TGpu) -> bool {
let dirty = self.is_modify();
match self {
Self::SetPpt(fast, slow) => settings.ppt(fast, slow),
Self::GetPpt(cb) => cb(settings.get_ppt()),
@ -151,6 +172,15 @@ impl GpuMessage {
Self::SetSlowMemory(val) => *settings.slow_memory() = val,
Self::GetSlowMemory(cb) => cb(*settings.slow_memory()),
}
dirty
}
fn is_modify(&self) -> bool {
matches!(self,
Self::SetPpt(_, _)
| Self::SetClockLimits(_)
| Self::SetSlowMemory(_)
)
}
}
@ -161,12 +191,18 @@ pub enum GeneralMessage {
}
impl GeneralMessage {
fn process(self, settings: &mut dyn TGeneral) {
fn process(self, settings: &mut dyn TGeneral) -> bool {
let dirty = self.is_modify();
match self {
Self::SetPersistent(val) => *settings.persistent() = val,
Self::GetPersistent(cb) => cb(*settings.persistent()),
Self::GetCurrentProfileName(cb) => cb(settings.get_name().to_owned()),
}
dirty
}
fn is_modify(&self) -> bool {
matches!(self, Self::SetPersistent(_))
}
}
@ -178,10 +214,11 @@ pub struct ApiMessageHandler {
impl ApiMessageHandler {
pub fn process_forever(&mut self, settings: &mut Settings) {
while let Ok(msg) = self.intake.recv() {
self.process(settings, msg);
let mut dirty = self.process(settings, msg);
while let Ok(msg) = self.intake.try_recv() {
self.process(settings, msg);
dirty |= self.process(settings, msg);
}
if dirty {
// run on_set
if let Err(e) = settings.on_set() {
log::error!("Settings on_set() err: {}", e);
@ -211,10 +248,13 @@ impl ApiMessageHandler {
log::debug!("Ignored save request for non-persistent settings");
}
}
} else {
log::debug!("Skipping callbacks for non-modify handled message(s)");
}
}
}
pub fn process(&mut self, settings: &mut Settings, message: ApiMessage) {
pub fn process(&mut self, settings: &mut Settings, message: ApiMessage) -> bool {
match message {
ApiMessage::Battery(x) => x.process(settings.battery.as_mut()),
ApiMessage::Cpu(x) => x.process(settings.cpus.as_mut()),
@ -224,13 +264,18 @@ impl ApiMessageHandler {
if let Err(e) = settings.on_resume() {
log::error!("Settings on_resume() err: {}", e);
}
false
}
ApiMessage::WaitForEmptyQueue(callback) => self.on_empty.push(callback),
ApiMessage::WaitForEmptyQueue(callback) => {
self.on_empty.push(callback);
false
},
ApiMessage::LoadSettings(path, name) => {
match settings.load_file(path.into(), name, false) {
Ok(success) => log::info!("Loaded settings file? {}", success),
Err(e) => log::warn!("Load file err: {}", e),
}
true
}
ApiMessage::LoadMainSettings => {
match settings.load_file(
@ -241,9 +286,11 @@ impl ApiMessageHandler {
Ok(success) => log::info!("Loaded main settings file? {}", success),
Err(e) => log::warn!("Load file err: {}", e),
}
true
}
ApiMessage::LoadSystemSettings => {
settings.load_system_default();
true
},
ApiMessage::GetLimits(cb) => {
cb(super::SettingsLimits {
@ -252,6 +299,7 @@ impl ApiMessageHandler {
gpu: settings.gpu.limits(),
general: settings.general.limits(),
});
false
},
ApiMessage::GetProvider(name, cb) => {
cb(match &name as &str {
@ -259,7 +307,8 @@ impl ApiMessageHandler {
"cpu" | "cpus" => settings.cpus.provider(),
"gpu" => settings.gpu.provider(),
_ => settings.general.provider(),
})
});
false
}
}
}

View file

@ -172,7 +172,8 @@ export class Cpus extends Component<{}, CpuState> {
onChange={(freq: number) => {
backend.log(backend.LogLevel.Debug, "Min freq slider is now " + freq.toString());
const freqNow = get_value(CLOCK_MIN_CPU);
if (freq != freqNow) {
const maxNow = get_value(CLOCK_MAX_CPU);
if (freq != freqNow && ((maxNow != null && freq > maxNow) || maxNow == null)) {
set_value(CLOCK_MIN_CPU, freq);
for (let i = 0; i < total_cpus; i++) {
backend.resolve(backend.setCpuClockLimits(i, freq, get_value(CLOCK_MAX_CPU)),
@ -202,7 +203,8 @@ export class Cpus extends Component<{}, CpuState> {
onChange={(freq: number) => {
backend.log(backend.LogLevel.Debug, "Max freq slider is now " + freq.toString());
const freqNow = get_value(CLOCK_MAX_CPU);
if (freq != freqNow) {
const minNow = get_value(CLOCK_MIN_CPU);
if (freq != freqNow && ((minNow != null && freq > minNow) || minNow == null)) {
set_value(CLOCK_MAX_CPU, freq);
for (let i = 0; i < total_cpus; i++) {
backend.resolve(backend.setCpuClockLimits(i, get_value(CLOCK_MIN_CPU), freq),
@ -301,7 +303,7 @@ export class Cpus extends Component<{}, CpuState> {
onChange={(freq: number) => {
backend.log(backend.LogLevel.Debug, "Min freq slider for " + advancedCpu.toString() + " is now " + freq.toString());
const freqNow = get_value(CLOCK_MIN_MAX_CPU)[advancedCpuIndex] as MinMax;
if (freq != freqNow.min) {
if (freq != freqNow.min && ((freqNow.max != null && freqNow.max > freq) || freqNow.max == null)) {
backend.resolve(backend.setCpuClockLimits(advancedCpuIndex, freq, freqNow.max!),
(limits: number[]) => {
const clocks = get_value(CLOCK_MIN_MAX_CPU) as MinMax[];
@ -326,7 +328,7 @@ export class Cpus extends Component<{}, CpuState> {
onChange={(freq: number) => {
backend.log(backend.LogLevel.Debug, "Max freq slider for " + advancedCpu.toString() + " is now " + freq.toString());
const freqNow = get_value(CLOCK_MIN_MAX_CPU)[advancedCpuIndex] as MinMax;
if (freq != freqNow.max) {
if (freq != freqNow.max && ((freqNow.min != null && freq > freqNow.min) || freqNow.min == null)) {
backend.resolve(backend.setCpuClockLimits(advancedCpuIndex, freqNow.min!, freq),
(limits: number[]) => {
const clocks = get_value(CLOCK_MIN_MAX_CPU) as MinMax[];

View file

@ -144,7 +144,8 @@ export class Gpu extends Component<{}> {
onChange={(val: number) => {
backend.log(backend.LogLevel.Debug, "GPU Clock Min is now " + val.toString());
const valNow = get_value(CLOCK_MIN_GPU);
if (val != valNow) {
const maxNow = get_value(CLOCK_MAX_GPU);
if (val != valNow && ((maxNow != null && val < maxNow) || maxNow == null)) {
backend.resolve(backend.setGpuClockLimits(val, get_value(CLOCK_MAX_GPU)),
(limits: number[]) => {
set_value(CLOCK_MIN_GPU, limits[0]);
@ -167,7 +168,8 @@ export class Gpu extends Component<{}> {
onChange={(val: number) => {
backend.log(backend.LogLevel.Debug, "GPU Clock Max is now " + val.toString());
const valNow = get_value(CLOCK_MAX_GPU);
if (val != valNow) {
const minNow = get_value(CLOCK_MIN_GPU);
if (val != valNow && ((minNow != null && val > minNow) || minNow == null)) {
backend.resolve(backend.setGpuClockLimits(get_value(CLOCK_MIN_GPU), val),
(limits: number[]) => {
set_value(CLOCK_MIN_GPU, limits[0]);