[Intel-gfx] [PATCH] drm/i915/eDP: When enabling panel VDD cancel pending disable worker

[Daniel Vetter]

On Tue, Nov 25, 2014 at 9:43 AM, Ville Syrjälä
<ville.syrjala at linux.intel.com> 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 at suse.de> 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 at suse.de>
>>
>> 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