On Tue, Jan 11, 2022 at 8:35 PM Limonciello, Mario <mario.limonciello@xxxxxxx> wrote: > > >> > >> That's why I thought my patch series made sense. > >> > >> I guess another (antisocial) response would be to to return false when > >> the suspend callback for amdgpu happens and dev_err mentioning that > >> firmware support is needed for suspend. > > > > If you want to avoid exposing s2idle at all in some cases, you need to > > change the sysfs I/F in /sys/power/ so that it doesn't expose "s2idle" > > in mem_sleep or "freeze" in state at all. > > Right now they make it so that s2idle isn't exposed in "mem_sleep", but > "freeze" is still exposed in "state". > > I'd be worried about breaking userspace with taking "freeze" out of state. > > > That's much more intrusive > > than just failing every s2idle attempt (which is what the patches do > > AFAICS) > As they stand right now when the BIOS is set to S3: > # cat /sys/power/mem_sleep > [deep] > > # echo freeze > /sys/power/state > Returns a failure though. > > And then when it's set to Modern Standby : > > # cat /sys/power/mem_sleep > [s2idle] > > # echo freeze > /sys/power/state > Works > > > > > Do you want to do that for platforms supporting S3, or for platforms > > that don't support any kind of suspend at all? > > You know since this series went out > 6dc8265f9803ccb7e5da804e01601f0c14f270e0 was merged. This will probably > have cleaned up the problem state that Bjoern found in the first place. > > So this is really a philosophical discussion now if it's worth offering > s2idle in /sys/power/mem_sleep on AMD systems. Well, if "freeze" is present in "state", then "s2idle" needs to be there in "mem_sleep", because they are clearly documented as alternative paths to the same functionality. > Admittedly this has been an APU notebook oriented discussion. Platforms > that don't support any kind of suspend (like servers) will get caught up > in this and could no longer do s2idle either. > > With the assumption that 6dc8265f9803ccb7e5da804e01601f0c14f270e0 helps > the problem state I'm leaning back on we should add some warnings to let > people know how to help themselves for power consumption if they did this. > > We can probably put those in amdgpu though. Sounds like a better direction to me, honestly.
