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. > > So no one can sneak in and the work racing with us isn't an issue. Or > shouldn't be at least. So if this helps we need to dig a bit deeper. > -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 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx