Differential D48732
acpi: Abstract over ACPI Sx sleep states with `enum sleep_type` Authored by obiwac on Jan 30 2025, 6:26 PM. Tags None Referenced Files
Details This will probably be superseded by the D52036, D52043, D52044 stack. Borrow the idea to abstract over ACPI Sx sleep types with enum sleep_type from Ben's s2idle patch, D17675, but use this everywhere it makes sense to. This is in preparation for an s2idle patch, which will add a new non-ACPI sleep state (STYPE_SUSPEND_TO_IDLE). All still works on the Framework 13 AMD Ryzen 7040 series, though I can not make sure that S3 sleep still works.
Diff Detail
Event TimelineComment Actions This looks reasonable but I'm a bit worried about potential future issues from the way we conflate sleep_type with ACPI constants. in particular with < and > comparisons. Perhaps we deal with that if/when we encounter it though. Comment Actions I agree, perhaps we should break from the ACPI values completely. I do want to move enum sleep_type out of dev/acpica/acpivar.h and have it be generic sleep states as described in D51591, so this would make sense. Currently there is the following in sys/sys/power.h: /* Sleep state */ #define POWER_SLEEP_STATE_STANDBY 0x00 #define POWER_SLEEP_STATE_SUSPEND 0x01 #define POWER_SLEEP_STATE_HIBERNATE 0x02 But maybe we want to keep these constants separate to enum sleep_type, since they're used at a higher level than enum sleep_type (e.g. when POWER_SLEEP_STATE_SUSPEND is passed to acpi_pm_func, it chooses whether to do S3 or s2idle depending on the hw.acpi.suspend_state sysctl). But then we might want to pull the mechanism for choosing which specific sleep type we do depending on the requested sleep state out from ACPI and make hw.acpi.suspend_state generic. What are your opinions on this? And @jkim ? Comment Actions
I don't think I have enough background to have a strong opinion, but the shape of that proposal sounds good. My biggest concern is that this would result in some surprising or hard to debug failure in the future, but since this is something that is being actively considered (e.g. WIP in D51591) I think it would be fine to conflate them for now and then iterate on it from there. Comment Actions Since other things transport this info deep into the storage stack, i like the notion of abstracting them so we don't churn that too much... Comment Actions I concur with @imp comment that I like the notion of abstracting them so we don't churn that too much... |