Re: [PATCH] drm/i915: Avoid spurious WARNs about the wrong pipe in the PPS code

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Feb 09, 2017 at 12:29:04PM +0200, Imre Deak wrote:
> On Wed, Feb 08, 2017 at 07:52:54PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Until recently vlv_steal_power_sequencer() wasn't being called for
> > normal DP ports, and hence it could assert that it should only be
> > called for pipe A and B (since pipe C doesn't support eDP). However
> > that changed when we started to consider normal DP ports as well when
> > choosing a PPS. So we will now get spurious warnings when
> > vlv_steal_power_sequencer() does get called for pipe C. Avoid this by
> > moving the WARN down into vlv_detach_power_sequencer() where this
> > assertion should still hold.
> > 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: stable@vger.kernel.org
> > Fixes: 9f2bdb006a7e ("drm/i915: Prevent PPS stealing from a normal DP port on VLV/CHV")
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=95287
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 0f14e97e519b..2395046e19ce 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2905,6 +2905,9 @@ static void vlv_detach_power_sequencer(struct intel_dp *intel_dp)
> >  
> >  	WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
> >  
> > +	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> > +		return;
> > +
> 
> Yep, missed this:
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> So the above is called only for eDP, checking for that too would make
> things clearer imo.

We should have some is_edp() warns in the even lower level stuff,
I think. Not sure if we should have moved the pipe check even lower as
well. Oh well, at least this should shut up the spurious warns.

Pushed to dinq, thanks for the review.

> 
> >  	edp_panel_vdd_off_sync(intel_dp);
> >  
> >  	/*
> > @@ -2932,9 +2935,6 @@ static void vlv_steal_power_sequencer(struct drm_device *dev,
> >  
> >  	lockdep_assert_held(&dev_priv->pps_mutex);
> >  
> > -	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> > -		return;
> > -
> >  	for_each_intel_encoder(dev, encoder) {
> >  		struct intel_dp *intel_dp;
> >  		enum port port;
> > -- 
> > 2.10.2
> > 

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