On Tue, Oct 28, 2014 at 09:22:12AM +0100, Daniel Vetter wrote:
> On Thu, Oct 16, 2014 at 09:29:45PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > 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.syrjala@xxxxxxxxxxxxxxx>
>
> [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@xxxxxxxxxxxxxxxxxxxxx
> > 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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
