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

[Egbert Eich <eich@suse.com>]

Daniel Vetter writes:
 > On Mon, Nov 24, 2014 at 5:56 PM, Egbert Eich <eich@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@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.
 
No. edp_panel_vdd_off() calls edp_panel_vdd_schedule_off() when not called
with sync == true. edp_panel_vdd_schedule_off() calls schedule_delayed_work()
which doesn't reschedule pending work.

 > 
 > 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, I came across this when I was looking for the problem in fdo#86201.
And I agree, it is not strictly needed, however if you follow
fdo#86201 you will see a list of calls to edp_panel_vdd_off_sync()
soon to be followed by calls to edp_panel_vdd_on(). Many of them are 
unnecessary and can be gotten rid of the uneeded ones by canelling the work 
queue. (When you follow fdo#86201 you will see why there was an abnormal
situation and what caused it). 
Of course due to the locking we already have serialization and canelling 
pending workers did not resolve the issue but it at least got rid of some
unneeded overhead.

Cheers,
	Egbert.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx