PowerD: Keep track of frontlight state change by interactive callers for suspend/resume purposes (#12283)

While #12256 papered over the tracking of a *single* suspend -> (resume->suspend) series of events, things still go out of sync if you tack on *more* suspend/resume events after that.

The upside is that our *actual* tracking of suspend/resume is solid, so at least we're actually doing the right thing as far as PM is concerned.

The downside is that on Kobo, the frontlight handling code is full of delays, and the ramping down/up itself also takes some time, so things can quickly overrun into the wrong event.

This means a couple of things:

* We need to cancel any scheduled frontlight toggles and ramps on Kobo, to ensure that only the one from the *last* event received goes through, in an attempt to limit the amount of potential crossover.
* Tracking fl_was_on *cannot* reliably be done from the suspend/resume handlers (especially on Kobo), as the ramps themselves may cross over the event barriers (and we potentially cancel the task anyway), so this was moved to the very few interactive callers that will actually change the frontlight state.
* On Kobo, crossover is still *somewhat* possible because the ramps take time. It's mostly harmless for the ramp down, we just need to tweak the ramp down to start from the actual intensity to avoid a weird jump on the initial step if there's an inconsistency. For the ramp up, we potentially need to manually kill the light first, because off -> on assumes it *really* starts from off ;).

Followup to #12256
Fix #12246 (again)
This commit is contained in:
NiLuJe
2024-08-06 01:13:14 +02:00
committed by GitHub
parent 9e6f3dac65
commit 7e8cdbcf65
4 changed files with 39 additions and 25 deletions

View File

@@ -116,6 +116,7 @@ if Device:hasFrontlight() then
else
powerd:setIntensity(new_intensity)
end
powerd:updateResumeFrontlightState()
return true
end
@@ -183,6 +184,7 @@ if Device:hasFrontlight() then
if not powerd:toggleFrontlight(notif_cb) then
Notification:notify(_("Frontlight unchanged."), notif_source)
end
powerd:updateResumeFrontlightState()
end
function DeviceListener:onShowFlDialog()

View File

@@ -18,6 +18,7 @@ local BasePowerD = {
last_aux_capacity_pull_time = time.s(-61), -- timestamp of last pull
is_fl_on = false, -- whether the frontlight is on
fl_was_on = nil, -- whether the frontlight *was* on before suspend
}
function BasePowerD:new(o)
@@ -29,6 +30,7 @@ function BasePowerD:new(o)
if o.device and o.device:hasFrontlight() then
o.fl_intensity = o:frontlightIntensityHW()
o:_decideFrontlightState()
o:updateResumeFrontlightState()
end
--- @note: Post-init, as the min/max values may be computed at runtime on some platforms
assert(o.fl_warmth_min < o.fl_warmth_max)
@@ -112,6 +114,11 @@ function BasePowerD:_decideFrontlightState()
self.is_fl_on = self:isFrontlightOnHW()
end
-- Separate from _decideFrontlightState, as this is only called by *interactive* codepaths
function BasePowerD:updateResumeFrontlightState()
self.fl_was_on = self:isFrontlightOn()
end
function BasePowerD:isFrontlightOff()
return not self:isFrontlightOn()
end

View File

@@ -3,6 +3,7 @@ local Math = require("optmath")
local NickelConf = require("device/kobo/nickel_conf")
local SysfsLight = require ("device/sysfs_light")
local UIManager
local logger = require("logger")
local RTC = require("ffi/rtc")
-- Here, we only deal with the real hw intensity.
@@ -17,7 +18,6 @@ local KoboPowerD = BasePowerD:new{
battery_sysfs = nil,
aux_battery_sysfs = nil,
fl_warmth_min = 0, fl_warmth_max = 100,
fl_was_on = nil,
}
--- @todo Remove G_defaults:readSetting("KOBO_LIGHT_ON_START")
@@ -84,7 +84,7 @@ function KoboPowerD:_syncKoboLightOnStart()
self.fl_warmth = new_warmth
end
-- In any case frontlight is off, ensure intensity is non-zero so untoggle works
-- In case frontlight is off, ensure hw_intensity is non-zero so toggle on works
if self.initial_is_fl_on == false and self.hw_intensity == 0 then
self.hw_intensity = 1
end
@@ -244,8 +244,8 @@ end
function KoboPowerD:isFrontlightOnHW()
if self.initial_is_fl_on ~= nil then -- happens only once after init()
-- give initial state to BasePowerD, which will
-- reset our self.hw_intensity to 0 if self.initial_is_fl_on is false
-- Pass our initial state to BasePowerD,
-- which will reset our self.hw_intensity to 0 if self.initial_is_fl_on is false
local ret = self.initial_is_fl_on
self.initial_is_fl_on = nil
return ret
@@ -263,9 +263,9 @@ function KoboPowerD:_setIntensityHW(intensity)
self.fl:setNaturalBrightness(intensity, self.fl_warmth)
end
self.hw_intensity = intensity
-- Now that we have set intensity, we need to let BasePowerD
-- know about possibly changed frontlight state (if we came
-- from light toggled off to some intensity > 0).
-- Now that we have set the intensity,
-- we need to let BasePowerD know about the possibly new frontlight state
-- (if we came from light toggled off to some intensity > 0).
self:_decideFrontlightState()
end
@@ -314,8 +314,11 @@ function KoboPowerD:isChargedHW()
return false
end
-- NOTE: When ramping down, we start from the *actual* intensity (hw_intensity),
-- instead of the expected one (fl_intensity),
-- in case a previously incomplete ramp was canceled and left us in an inconsistent state.
function KoboPowerD:_startRampDown(done_callback)
self:turnOffFrontlightRamp(self.fl_intensity, self.fl_min, done_callback)
self:turnOffFrontlightRamp(self.hw_intensity, self.fl_min, done_callback)
self.fl_ramp_down_running = true
end
@@ -372,7 +375,7 @@ function KoboPowerD:turnOffFrontlightHW(done_callback)
-- otherwise you just see a single delayed step (1%) or two stuttery ones (2%) ;).
-- FWIW, modern devices with a different PWM controller (i.e., with no controller-specific ramp_off_delay workarounds)
-- deal with our 2% ramp without stuttering.
if self.device.frontlight_settings.ramp_off_delay > 0.0 and self.fl_intensity <= 2 then
if self.device.frontlight_settings.ramp_off_delay > 0.0 and self.hw_intensity <= 2 then
UIManager:scheduleIn(self.device.frontlight_settings.ramp_delay, self._endRampDown, self, self.fl_min, done_callback)
else
-- NOTE: Similarly, some controllers *really* don't like to be interleaved with screen refreshes,
@@ -380,7 +383,7 @@ function KoboPowerD:turnOffFrontlightHW(done_callback)
if self.device.frontlight_settings.delay_ramp_start then
UIManager:nextTick(self._startRampDown, self, done_callback)
else
self:turnOffFrontlightRamp(self.fl_intensity, self.fl_min, done_callback)
self:turnOffFrontlightRamp(self.hw_intensity, self.fl_min, done_callback)
self.fl_ramp_down_running = true
end
end
@@ -462,15 +465,7 @@ function KoboPowerD:turnOnFrontlightHW(done_callback)
return true
end
-- NOTE: We delay those *slightly*, so tracking the fl_was_on state needs to be delayed with it,
-- or stuff gets wonky if you trip a resume/suspend in quick succession...
-- c.f., https://github.com/koreader/koreader/issues/12246#issuecomment-2261334603
function KoboPowerD:_suspendFrontlight()
-- Remember the current frontlight state
-- NOTE: self.is_fl_on flips to false as soon as we engage the ramp down, so, ideally,
-- we'd delay setting self.fl_was_on to the pre-ramp value at the end of the ramp (via the ramp's done_callback),
-- except for the fact that if the frontlight is off, turnOffFrontlight will abort early, so we can't ;).
self.fl_was_on = self.is_fl_on
self:turnOffFrontlight()
end
@@ -482,12 +477,10 @@ function KoboPowerD:beforeSuspend()
-- Handle the frontlight last,
-- to prevent as many things as we can from interfering with the smoothness of the ramp
if self.fl then
-- NOTE: We *cannot* cancel any pending frontlight tasks,
-- because it risks breaking self.fl_was_on tracking...
--[[
-- We only want the *last* scheduled suspend/resume frontlight task to run to avoid ramps running amok...
UIManager:unschedule(self._suspendFrontlight)
UIManager:unschedule(self._resumeFrontlight)
self:_stopFrontlightRamp()
--]]
-- Turn off the frontlight
-- NOTE: Funky delay mainly to yield to the EPDC's refresh on UP systems.
@@ -498,7 +491,19 @@ end
function KoboPowerD:_resumeFrontlight()
-- Don't bother if the light was already off on suspend
-- NOTE: Things gan go sideways quick when you mix the userland ramp,
-- delays all over the place, and quick successions of suspend/resume requests (e.g., jittery sleepcovers),
-- so trust fl_was_on over the actual state on beforeSuspend,
-- as said state might no longer actually represent the pre-suspend reality...
-- c.f., #12246
-- Note that fl_was_on is updated by *interactive* callers via `BasePowerD:updateResumeFrontlightState`
if self.fl_was_on then
-- If the frontlight is currently on because of madness resulting from multiple concurrent suspend/resume requests,
-- but at the wrong intensity, turn it straight off first so that turnOnFrontlight doesn't abort early...
if self.is_fl_on and self.hw_intensity ~= self.fl_intensity then
logger.warn("KoboPowerD:_resumeFrontlight: frontlight intensity is at", self.hw_intensity, "instead of the expected", self.fl_intensity)
self:setIntensityHW(self.fl_min)
end
-- Turn the frontlight back on
self:turnOnFrontlight()
end
@@ -517,11 +522,10 @@ function KoboPowerD:afterResume()
-- There's a whole bunch of stuff happening before us in Generic:onPowerEvent,
-- so we'll delay this ever so slightly so as to appear as smooth as possible...
if self.fl then
-- NOTE: Same as above, can't cancel any pending fl tasks.
--[[
-- Same reasoning as on suspend
UIManager:unschedule(self._suspendFrontlight)
UIManager:unschedule(self._resumeFrontlight)
self:_stopFrontlightRamp()
--]]
-- Turn the frontlight back on
-- NOTE: There's quite likely *more* resource contention than on suspend here :/.

View File

@@ -512,6 +512,7 @@ function FrontLightWidget:setFrontLightIntensity(intensity)
else
self.powerd:setIntensity(self.fl.cur)
end
self.powerd:updateResumeFrontlightState()
-- Retrieve the real level set by PowerD (will be different from `intensity` on toggle)
self.fl.cur = self.powerd:frontlightIntensity()