On Tue, Nov 25, 2014 at 9:43 AM, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > On Mon, Nov 24, 2014 at 08:46:22PM +0100, Daniel Vetter wrote: >> On Mon, Nov 24, 2014 at 5:56 PM, Egbert Eich <eich@xxxxxxx> wrote: >> > Before testing if the panel VDD is enabled on eDP cancel any pending >> > disable worker. This makes sure the worker doesn't fire when we expect >> > VDD to be enabled. >> > >> > https://bugs.freedesktop.org/show_bug.cgi?id=86201 >> > >> > Signed-off-by: Egbert Eich <eich@xxxxxxx> >> >> This shouldn't be needed at all: >> - The vdd off rechecks ->want_panel_vdd under the pps lock. >> - The off function sets that and also reschedules the work (to make >> sure it doesn't kill vdd to early) again all under the same lock. > > It uses schedule_delayed_work() which does nothing if the work is > already pending. Hence the delay will be counted from the first vdd_off, > not the last one. > > But doing the cancel up front instead of using mod_delayed_work() has > the extra benefit of making it a bit less likely vdd will get turned off > if the transfer of a single AUX message takes longer than the vdd off > delay. > > However even with the cancel it'ss still possible the vdd off work already > started executing (but is blocked by pps_mutex) by the time we call > edp_panel_vdd_on(), in which case vdd would still be turned off as soon > as pps_mutex is released. We could track the last time vdd was used > to avoid that, but I'm not sure it's really worth the extra effort. Hm right. So at least the commit message needs a bit more clarity since the check in the worker will ensure that we don't disable vdd when we need it. The real problem is that the hysteresis isn't fully obeyed. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
