On Thu, Apr 24, 2014 at 11:55:40PM +0200, Daniel Vetter wrote:
> Mostly this patch is one big excersize in deleting code and asserts
> which are no longer needed. Note that we still abuse the shared dpll
> framework a bit since we call the enable/disable functions from the
> crtc mode_set and off hooks. But changing the actual hardware sequence
> will be done in the next step.
>
> Note that besides the massive amount of changes in this patch the
> places and order in which the low-level WRPLL code is called is
> absolutely unchanged.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Reviewed-by: Damien Lespiau <damien.lespiau@xxxxxxxxx>
--
Damien
> ---
> drivers/gpu/drm/i915/i915_drv.h | 6 --
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_ddi.c | 142 ++++-------------------------------
> drivers/gpu/drm/i915/intel_display.c | 14 ++--
> drivers/gpu/drm/i915/intel_drv.h | 9 ++-
> 5 files changed, 27 insertions(+), 145 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bebc507f776b..73371161777b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -225,11 +225,6 @@ void intel_link_compute_m_n(int bpp, int nlanes,
> int pixel_clock, int link_clock,
> struct intel_link_m_n *m_n);
>
> -struct intel_ddi_plls {
> - int wrpll1_refcount;
> - int wrpll2_refcount;
> -};
> -
> /* Interface history:
> *
> * 1.1: Original.
> @@ -1399,7 +1394,6 @@ struct drm_i915_private {
>
> int num_shared_dpll;
> struct intel_shared_dpll shared_dplls[I915_NUM_PLLS];
> - struct intel_ddi_plls ddi_plls;
> int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
>
> /* Reclocking support */
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 99051e6348b8..fcb1ca6eadb5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5529,6 +5529,7 @@ enum punit_power_well {
> #define PORT_CLK_SEL_LCPLL_1350 (1<<29)
> #define PORT_CLK_SEL_LCPLL_810 (2<<29)
> #define PORT_CLK_SEL_SPLL (3<<29)
> +#define PORT_CLK_SEL_WRPLL(pll) (((pll)+4)<<29)
> #define PORT_CLK_SEL_WRPLL1 (4<<29)
> #define PORT_CLK_SEL_WRPLL2 (5<<29)
> #define PORT_CLK_SEL_NONE (7<<29)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 21e451ea0c69..7386a1212e71 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -257,31 +257,12 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc)
>
> void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
> {
> - struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> - struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>
> - struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(intel_crtc);
> -
> - switch (intel_crtc->config.ddi_pll_sel) {
> - case PORT_CLK_SEL_WRPLL1:
> - plls->wrpll1_refcount--;
> - if (plls->wrpll1_refcount == 0) {
> - pll->disable(dev_priv, pll);
> - }
> - intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
> - break;
> - case PORT_CLK_SEL_WRPLL2:
> - plls->wrpll2_refcount--;
> - if (plls->wrpll2_refcount == 0) {
> - pll->disable(dev_priv, pll);
> - }
> - intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
> - break;
> - }
> + if (intel_crtc_to_shared_dpll(intel_crtc))
> + intel_disable_shared_dpll(intel_crtc);
>
> - WARN(plls->wrpll1_refcount < 0, "Invalid WRPLL1 refcount\n");
> - WARN(plls->wrpll2_refcount < 0, "Invalid WRPLL2 refcount\n");
> + intel_put_shared_dpll(intel_crtc);
> }
>
> #define LC_FREQ 2700
> @@ -601,17 +582,14 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
> {
> struct drm_crtc *crtc = &intel_crtc->base;
> struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
> - struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> - struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
> int type = intel_encoder->type;
> - enum pipe pipe = intel_crtc->pipe;
> int clock = intel_crtc->config.port_clock;
>
> intel_ddi_put_crtc_pll(crtc);
>
> if (type == INTEL_OUTPUT_HDMI) {
> struct intel_shared_dpll *pll;
> - uint32_t reg, val;
> + uint32_t val;
> unsigned p, n2, r2;
>
> intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
> @@ -620,79 +598,21 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
> WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
> WRPLL_DIVIDER_POST(p);
>
> - if (val == I915_READ(WRPLL_CTL1)) {
> - DRM_DEBUG_KMS("Reusing WRPLL 1 on pipe %c\n",
> - pipe_name(pipe));
> - reg = WRPLL_CTL1;
> - } else if (val == I915_READ(WRPLL_CTL2)) {
> - DRM_DEBUG_KMS("Reusing WRPLL 2 on pipe %c\n",
> - pipe_name(pipe));
> - reg = WRPLL_CTL2;
> - } else if (plls->wrpll1_refcount == 0) {
> - DRM_DEBUG_KMS("Using WRPLL 1 on pipe %c\n",
> - pipe_name(pipe));
> - reg = WRPLL_CTL1;
> - } else if (plls->wrpll2_refcount == 0) {
> - DRM_DEBUG_KMS("Using WRPLL 2 on pipe %c\n",
> - pipe_name(pipe));
> - reg = WRPLL_CTL2;
> - } else {
> - DRM_ERROR("No WRPLLs available!\n");
> - return false;
> - }
> + intel_crtc->config.dpll_hw_state.wrpll = val;
>
> - DRM_DEBUG_KMS("WRPLL: %dKHz refresh rate with p=%d, n2=%d r2=%d\n",
> - clock, p, n2, r2);
> -
> - if (reg == WRPLL_CTL1) {
> - plls->wrpll1_refcount++;
> - intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL1;
> - intel_crtc->config.shared_dpll = DPLL_ID_WRPLL1;
> - } else {
> - plls->wrpll2_refcount++;
> - intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
> - intel_crtc->config.shared_dpll = DPLL_ID_WRPLL2;
> + pll = intel_get_shared_dpll(intel_crtc);
> + if (pll == NULL) {
> + DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
> + pipe_name(intel_crtc->pipe));
> + return false;
> }
>
> - intel_crtc->config.dpll_hw_state.wrpll = val;
> -
> - pll = &dev_priv->shared_dplls[intel_crtc->config.shared_dpll];
> - pll->hw_state.wrpll = val;
> + intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
> }
>
> return true;
> }
>
> -/*
> - * To be called after intel_ddi_pll_select(). That one selects the PLL to be
> - * used, this one actually enables the PLL.
> - */
> -void intel_ddi_pll_enable(struct intel_crtc *crtc)
> -{
> - struct drm_device *dev = crtc->base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
> - int refcount;
> - struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
> -
> - switch (crtc->config.ddi_pll_sel) {
> - case PORT_CLK_SEL_WRPLL1:
> - case PORT_CLK_SEL_WRPLL2:
> - if (crtc->config.ddi_pll_sel == PORT_CLK_SEL_WRPLL1) {
> - refcount = plls->wrpll1_refcount;
> - } else {
> - refcount = plls->wrpll2_refcount;
> - }
> - break;
> - default:
> - return;
> - }
> -
> - if (refcount == 1) {
> - pll->enable(dev_priv, pll);
> - }
> -}
> -
> void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
> {
> struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> @@ -929,35 +849,6 @@ static bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> return intel_ddi_get_port_state(encoder, pipe, port);
> }
>
> -void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - enum pipe pipe;
> - struct intel_crtc *intel_crtc;
> -
> - dev_priv->ddi_plls.wrpll1_refcount = 0;
> - dev_priv->ddi_plls.wrpll2_refcount = 0;
> -
> - for_each_pipe(pipe) {
> - intel_crtc =
> - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> -
> - if (!intel_crtc->active) {
> - intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
> - continue;
> - }
> -
> - switch (intel_crtc->config.ddi_pll_sel) {
> - case PORT_CLK_SEL_WRPLL1:
> - dev_priv->ddi_plls.wrpll1_refcount++;
> - break;
> - case PORT_CLK_SEL_WRPLL2:
> - dev_priv->ddi_plls.wrpll2_refcount++;
> - break;
> - }
> - }
> -}
> -
> void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
> {
> struct drm_crtc *crtc = &intel_crtc->base;
> @@ -1157,10 +1048,6 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
> static void hsw_ddi_pll_enable(struct drm_i915_private *dev_priv,
> struct intel_shared_dpll *pll)
> {
> - uint32_t cur_val;
> -
> - cur_val = I915_READ(WRPLL_CTL(pll->id));
> - WARN(cur_val & WRPLL_PLL_ENABLE, "%s already enabled\n", pll->name);
> I915_WRITE(WRPLL_CTL(pll->id), pll->hw_state.wrpll);
> POSTING_READ(WRPLL_CTL(pll->id));
> udelay(20);
> @@ -1172,7 +1059,6 @@ static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
> uint32_t val;
>
> val = I915_READ(WRPLL_CTL(pll->id));
> - WARN_ON(!(val & WRPLL_PLL_ENABLE));
> I915_WRITE(WRPLL_CTL(pll->id), val & ~WRPLL_PLL_ENABLE);
> POSTING_READ(WRPLL_CTL(pll->id));
> }
> @@ -1200,11 +1086,9 @@ void intel_ddi_pll_init(struct drm_device *dev)
> uint32_t val = I915_READ(LCPLL_CTL);
> int i;
>
> - /* Dummy setup until everything is moved over to avoid upsetting the hw
> - * state cross checker. */
> - dev_priv->num_shared_dpll = 0;
> + dev_priv->num_shared_dpll = 2;
>
> - for (i = 0; i < 2; i++) {
> + for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> dev_priv->shared_dplls[i].id = i;
> dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
> dev_priv->shared_dplls[i].disable = hsw_ddi_pll_disable;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b890c97f1312..2fd77eba57f3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1599,7 +1599,7 @@ static void intel_enable_shared_dpll(struct intel_crtc *crtc)
> pll->on = true;
> }
>
> -static void intel_disable_shared_dpll(struct intel_crtc *crtc)
> +void intel_disable_shared_dpll(struct intel_crtc *crtc)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3232,7 +3232,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
> ironlake_enable_pch_transcoder(dev_priv, pipe);
> }
>
> -static void intel_put_shared_dpll(struct intel_crtc *crtc)
> +void intel_put_shared_dpll(struct intel_crtc *crtc)
> {
> struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
>
> @@ -3252,7 +3252,7 @@ static void intel_put_shared_dpll(struct intel_crtc *crtc)
> crtc->config.shared_dpll = DPLL_ID_PRIVATE;
> }
>
> -static struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
> +struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
> {
> struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
> @@ -7001,7 +7001,9 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>
> if (!intel_ddi_pll_select(intel_crtc))
> return -EINVAL;
> - intel_ddi_pll_enable(intel_crtc);
> +
> + if (intel_crtc_to_shared_dpll(intel_crtc))
> + intel_enable_shared_dpll(intel_crtc);
>
> intel_crtc->lowfreq_avail = false;
>
> @@ -11531,10 +11533,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> crtc->active ? "enabled" : "disabled");
> }
>
> - /* FIXME: Smash this into the new shared dpll infrastructure. */
> - if (HAS_DDI(dev))
> - intel_ddi_setup_hw_pll_state(dev);
> -
> for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e1d079fe47ea..acd32e8e5e13 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -682,9 +682,7 @@ void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
> enum transcoder cpu_transcoder);
> void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc);
> void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc);
> -void intel_ddi_setup_hw_pll_state(struct drm_device *dev);
> bool intel_ddi_pll_select(struct intel_crtc *crtc);
> -void intel_ddi_pll_enable(struct intel_crtc *crtc);
> void intel_ddi_put_crtc_pll(struct drm_crtc *crtc);
> void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
> void intel_ddi_prepare_link_retrain(struct drm_encoder *encoder);
> @@ -740,12 +738,19 @@ __intel_framebuffer_create(struct drm_device *dev,
> void intel_prepare_page_flip(struct drm_device *dev, int plane);
> void intel_finish_page_flip(struct drm_device *dev, int pipe);
> void intel_finish_page_flip_plane(struct drm_device *dev, int plane);
> +
> +/* shared dpll functions */
> struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
> void assert_shared_dpll(struct drm_i915_private *dev_priv,
> struct intel_shared_dpll *pll,
> bool state);
> #define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true)
> #define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
> +void intel_disable_shared_dpll(struct intel_crtc *crtc);
> +struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc);
> +void intel_put_shared_dpll(struct intel_crtc *crtc);
> +
> +/* modesetting asserts */
> void assert_pll(struct drm_i915_private *dev_priv,
> enum pipe pipe, bool state);
> #define assert_pll_enabled(d, p) assert_pll(d, p, true)
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
