On Tue, Oct 28, 2014 at 09:22:12AM +0100, Daniel Vetter wrote:
> On Thu, Oct 16, 2014 at 09:29:45PM +0300, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > 
> > The power seqeuencer kick procedure requires the DPLL to be running
> > in order to complete succesfully. In case the DPLL isn't currently
> > running when we need to kick the power seqeuncer enable it
> > temporarily. This can happen eg. during ->detect() when the pipe is
> > not already active.
> > 
> > To avoid needlessly duplicating the DPLL programming re-use the already
> > existing functions by passing a temporary pipe config to them instead
> > of having them consult the current pipe config at crtc->config.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> 
> [snip]
> 
> > +   /*
> > +    * The DPLL for the pipe must be enabled for this to work.
> > +    * So enable temporarily it if it's not already enabled.
> > +    */
> > +   if (!pll_enabled) {
> > +           struct intel_crtc *crtc =
> > +                   to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> > +           struct intel_crtc_config pipe_config = {
> > +                   .pixel_multiplier = 1,
> > +                   .dpll = IS_CHERRYVIEW(dev) ?
> > +                           chv_dpll[0].dpll : vlv_dpll[0].dpll,
> > +                   .port_clock = 162000,
> > +                   .clock_set = true,
> > +                   .has_dp_encoder = true,
> > +           };
> > +
> > +           if (IS_CHERRYVIEW(dev)) {
> > +                   chv_update_pll(crtc, &pipe_config);
> > +                   chv_prepare_pll(crtc, &pipe_config);
> > +                   chv_enable_pll(crtc, &pipe_config);
> > +           } else {
> > +                   vlv_update_pll(crtc, &pipe_config);
> > +                   vlv_prepare_pll(crtc, &pipe_config);
> > +                   vlv_enable_pll(crtc, &pipe_config);
> 
> I'm not terribly happy with the massive list of non-static platform
> functions this exposes. And it's also fairly leaky since the minimal pipe
> config depends upon what exactly the vlv/chv pll functions need.
> 
> Could we instead have a wrapper function for both the enable and disable
> which sits right next to the vlv/chv dpll code in intel_display.c? E.g.
> valleyview_force_pll_on/off(dev, pipe) or something like that.

Sure. I'll respin this with that idea.

> -Daniel
> 
> > +           }
> > +   }
> > +
> >     /*
> >      * Similar magic as in intel_dp_enable_port().
> >      * We _must_ do this port enable + disable trick
> > @@ -365,6 +395,13 @@ vlv_power_sequencer_kick(struct intel_dp *intel_dp)
> >  
> >     I915_WRITE(intel_dp->output_reg, DP & ~DP_PORT_EN);
> >     POSTING_READ(intel_dp->output_reg);
> > +
> > +   if (!pll_enabled) {
> > +           if (IS_CHERRYVIEW(dev))
> > +                   chv_disable_pll(dev_priv, pipe);
> > +           else
> > +                   vlv_disable_pll(dev_priv, pipe);
> > +   }
> >  }
> >  
> >  static enum pipe
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index f7ba1fc..69c8c5f 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -911,6 +911,21 @@ void assert_shared_dpll(struct drm_i915_private 
> > *dev_priv,
> >  struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc);
> >  void intel_put_shared_dpll(struct intel_crtc *crtc);
> >  
> > +void vlv_update_pll(struct intel_crtc *crtc,
> > +               struct intel_crtc_config *pipe_config);
> > +void chv_update_pll(struct intel_crtc *crtc,
> > +               struct intel_crtc_config *pipe_config);
> > +void vlv_prepare_pll(struct intel_crtc *crtc,
> > +                const struct intel_crtc_config *pipe_config);
> > +void chv_prepare_pll(struct intel_crtc *crtc,
> > +                const struct intel_crtc_config *pipe_config);
> > +void vlv_enable_pll(struct intel_crtc *crtc,
> > +               const struct intel_crtc_config *pipe_config);
> > +void chv_enable_pll(struct intel_crtc *crtc,
> > +               const struct intel_crtc_config *pipe_config);
> > +void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe);
> > +void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe);
> > +
> >  /* modesetting asserts */
> >  void assert_panel_unlocked(struct drm_i915_private *dev_priv,
> >                        enum pipe pipe);
> > -- 
> > 2.0.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

Reply via email to