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

From: Egbert Eich <eich@suse.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Egbert Eich <eich@suse.de>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/eDP: When enabling panel VDD cancel pending disable worker
Date: Mon, 24 Nov 2014 20:04:52 +0100	[thread overview]
Message-ID: <21619.33108.353895.367041@linux-qknr.fritz.box> (raw)
In-Reply-To: ville.syrjala@linux.intel.com wrote on Monday, 24 November 2014 at 19:44:11 +0200

Ville Syrjälä writes:
 > On Mon, Nov 24, 2014 at 07:32:49PM +0200, Ville Syrjälä wrote:
 > > On Mon, Nov 24, 2014 at 05:56:20PM +0100, Egbert Eich 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>
 > > > ---
 > > >  drivers/gpu/drm/i915/intel_dp.c | 1 +
 > > >  1 file changed, 1 insertion(+)
 > > > 
 > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 > > > index 70bb8d0b..81f959d 100644
 > > > --- a/drivers/gpu/drm/i915/intel_dp.c
 > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
 > > > @@ -1503,6 +1503,7 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
 > > >  	if (!is_edp(intel_dp))
 > > >  		return false;
 > > >  
 > > > +	cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
 > > 
 > > This can deadlock since we're already holding pps_mutex and
 > > edp_panel_vdd_work() also grabs it.
 > > 
 > > I'm thinking we may want something like mod_delayed_work() instead.
 > 
 > Or rather just cancel_delayed_work() is enough here I guess. W/o the
 > _sync we shouldn't deadlock. And if we always cancel here the timer
 > shoduln't be pending in edp_panel_vdd_schedule_off() anymore, so
 > mod_delayed_work() there would anyway just be the same as
 > schedule_delayed_work().

Right. I was thinking we need to make sure we are not in the midst of
the worker but the pps_mutexes indeed serialize this as well.

Will resend the patch.

Thanks!
Cheers,
	Egbert.
 > 
 > > 
 > > 
 > > >  	intel_dp->want_panel_vdd = true;
 > > >  
 > > >  	if (edp_have_panel_vdd(intel_dp))
 > > > -- 
 > > > 1.8.4.5
 > > 
 > > -- 
 > > Ville Syrjälä
 > > Intel OTC
 > 
 > -- 
 > Ville Syrjälä
 > Intel OTC

-- 
Any jackass can report a bug, but it takes a good engineer to fix one.
   Loosely base on 
   	 Sam Rayburn (48th, 50th and 52nd Speaker of the US House of Rep.)

Egbert Eich (Res. & Dev.)                SUSE LINUX Products GmbH
SUSE Labs KMS / DRM / X Window System Development
Tel: +49 911-740 53 0                       http://www.suse.de 
-----------------------------------------------------------------
SUSE LINUX GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 21284 (AG Nürnberg) 
Maxfeldstraße 5                         
90409 Nürnberg 
Germany 

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