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

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Egbert Eich <eich@suse.de>, intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915/eDP: When enabling panel VDD cancel pending disable worker
Date: Tue, 25 Nov 2014 10:43:17 +0200	[thread overview]
Message-ID: <20141125084317.GM10649@intel.com> (raw)
In-Reply-To: <CAKMK7uGhBcdmfTYouY3=91y8cHu8MgEp-D-oewSic3v2PeLUuA@mail.gmail.com>

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@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.

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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx