[Intel-gfx] [PATCH 02/17] drm/i915: Warn if stealing power sequencer from an active eDP port

[Daniel Vetter]

On Tue, Oct 28, 2014 at 11:07:27AM +0200, Ville Syrjälä wrote:
> On Tue, Oct 28, 2014 at 09:34:40AM +0100, Daniel Vetter wrote:
> > On Tue, Oct 28, 2014 at 10:14:54AM +0200, Ville Syrjälä wrote:
> > > On Tue, Oct 28, 2014 at 09:10:13AM +0100, Daniel Vetter wrote:
> > > > On Thu, Oct 16, 2014 at 09:27:28PM +0300, ville.syrjala at linux.intel.com wrote:
> > > > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > 
> > > > > eDP ports need the power seqeuncer whenever the port is active. Warn if
> > > > > we accidentally steal the power sequener from an active eDP port. This
> > > > > should not happen unless there's a bug somewhere else, but it's best to
> > > > > scream loudly if it happens to help with debugging.
> > > > > 
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_dp.c | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index de919e9..7ac5c47 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -2606,6 +2606,10 @@ static void vlv_steal_power_sequencer(struct drm_device *dev,
> > > > >  		DRM_DEBUG_KMS("stealing pipe %c power sequencer from port %c\n",
> > > > >  			      pipe_name(pipe), port_name(port));
> > > > >  
> > > > > +		WARN(encoder->connectors_active,
> > > > 
> > > > This only checks for the pipe actually running, i.e. dpms on. But I guess
> > > > we actually want to check for logically enabled (i.e. no matter the dpms
> > > > state) to make sure we can always transition from dpms off to on again.
> > > > That should check for encoder->base.crtc instead.
> > > 
> > > We can allow stealing the power sequencer whenever the port is not
> > > running. When the port gets re-enabled the power seqeuencer gets
> > > stolen back.
> > 
> > Hm, but how does this work if we run out of power sequencers? Generally
> > dpms on isn't allowed to fail, so if there's any shared resources we need
> > to keep them reserved.
> > 
> > Or am I super-dense again?
> 
> Currently it shouldn't fail. We have two power sequencers and at most two
> eDP ports. If that should change we need to rethink this code.

Ok I'm convinced. Added a summary of our discussion here and merged your
patch.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch