On Thu, Apr 24, 2014 at 11:55:33PM +0200, Daniel Vetter wrote:
> To make things a bit more manageable extract a new function for
> reading out common ddi port state. This means a bit of duplication
> between encoders and the core since both look at the same registers,
> but doesn't seem worth to make a fuzz about.
>
> We can also remove the state readout code in intel_ddi_setup_hw_pll_state.
> That code is only called from the hardware take over and not the cross
> check code, and only after the crtc state is reconstructed. So we can
> rely on an accurate value of crtc->config.ddi_pll_sel already.
>
> Compared to the old code also trust the hw state more and don't
> special-case port A - we want to cross-check the actual-state, not
> bake in our own assumptions about how this is supposed to all be
> linked up.
>
> v2: Make use of the read-out ddi_pll_sel in intel_ddi_clock_get.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Moving things out of intel_ddi.c to intel_display.c seems to be the
opposite of what we want. I don't see why we couldn't leave the DDI
readout function inside intel_ddi.c?
Reviewed-by: Damien Lespiau <damien.lespiau@xxxxxxxxx>
--
Damien
> ---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_ddi.c | 40 +-----------------------------
> drivers/gpu/drm/i915/intel_display.c | 48 ++++++++++++++++++++++++------------
> 3 files changed, 34 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 64d40f22e708..4c1cefb5f3eb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5366,6 +5366,7 @@ enum punit_power_well {
> #define TRANS_DDI_FUNC_ENABLE (1<<31)
> /* Those bits are ignored by pipe EDP since it can only connect to DDI A */
> #define TRANS_DDI_PORT_MASK (7<<28)
> +#define TRANS_DDI_PORT_SHIFT 28
> #define TRANS_DDI_SELECT_PORT(x) ((x)<<28)
> #define TRANS_DDI_PORT_NONE (0<<28)
> #define TRANS_DDI_MODE_SELECT_MASK (7<<24)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 2adcc917806e..571cfe431558 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -481,11 +481,10 @@ static void intel_ddi_clock_get(struct intel_encoder *encoder,
> struct intel_crtc_config *pipe_config)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> - enum port port = intel_ddi_get_encoder_port(encoder);
> int link_clock = 0;
> u32 val, pll;
>
> - val = I915_READ(PORT_CLK_SEL(port));
> + val = pipe_config->ddi_pll_sel;
> switch (val & PORT_CLK_SEL_MASK) {
> case PORT_CLK_SEL_LCPLL_810:
> link_clock = 81000;
> @@ -987,40 +986,6 @@ static bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> return intel_ddi_get_port_state(encoder, pipe, port);
> }
>
> -static uint32_t intel_ddi_get_crtc_pll(struct drm_i915_private *dev_priv,
> - enum pipe pipe)
> -{
> - uint32_t temp, ret;
> - enum port port = I915_MAX_PORTS;
> - enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
> - pipe);
> - int i;
> -
> - if (cpu_transcoder == TRANSCODER_EDP) {
> - port = PORT_A;
> - } else {
> - temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> - temp &= TRANS_DDI_PORT_MASK;
> -
> - for (i = PORT_B; i <= PORT_E; i++)
> - if (temp == TRANS_DDI_SELECT_PORT(i))
> - port = i;
> - }
> -
> - if (port == I915_MAX_PORTS) {
> - WARN(1, "Pipe %c enabled on an unknown port\n",
> - pipe_name(pipe));
> - ret = PORT_CLK_SEL_NONE;
> - } else {
> - ret = I915_READ(PORT_CLK_SEL(port));
> - DRM_DEBUG_KMS("Pipe %c connected to port %c using clock "
> - "0x%08x\n", pipe_name(pipe), port_name(port),
> - ret);
> - }
> -
> - return ret;
> -}
> -
> void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1039,9 +1004,6 @@ void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
> continue;
> }
>
> - intel_crtc->config.ddi_pll_sel = intel_ddi_get_crtc_pll(dev_priv,
> - pipe);
> -
> switch (intel_crtc->config.ddi_pll_sel) {
> case PORT_CLK_SEL_WRPLL1:
> dev_priv->ddi_plls.wrpll1_refcount++;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 728b5a25cb80..1601da1b57a1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7006,6 +7006,35 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> return 0;
> }
>
> +static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
> + struct intel_crtc_config *pipe_config)
> +{
> + struct drm_device *dev = crtc->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + enum port port;
> + uint32_t tmp;
> +
> + tmp = I915_READ(TRANS_DDI_FUNC_CTL(pipe_config->cpu_transcoder));
> +
> + port = (tmp & TRANS_DDI_PORT_MASK) >> TRANS_DDI_PORT_SHIFT;
> +
> + pipe_config->ddi_pll_sel = I915_READ(PORT_CLK_SEL(port));
> + /*
> + * Haswell has only FDI/PCH transcoder A. It is which is connected to
> + * DDI E. So just check whether this pipe is wired to DDI E and whether
> + * the PCH transcoder is on.
> + */
> + if ((port == PORT_E) && I915_READ(LPT_TRANSCONF) & TRANS_ENABLE) {
> + pipe_config->has_pch_encoder = true;
> +
> + tmp = I915_READ(FDI_RX_CTL(PIPE_A));
> + pipe_config->fdi_lanes = ((FDI_DP_PORT_WIDTH_MASK & tmp) >>
> + FDI_DP_PORT_WIDTH_SHIFT) + 1;
> +
> + ironlake_get_fdi_m_n_config(crtc, pipe_config);
> + }
> +}
> +
> static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> struct intel_crtc_config *pipe_config)
> {
> @@ -7051,22 +7080,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> if (!(tmp & PIPECONF_ENABLE))
> return false;
>
> - /*
> - * Haswell has only FDI/PCH transcoder A. It is which is connected to
> - * DDI E. So just check whether this pipe is wired to DDI E and whether
> - * the PCH transcoder is on.
> - */
> - tmp = I915_READ(TRANS_DDI_FUNC_CTL(pipe_config->cpu_transcoder));
> - if ((tmp & TRANS_DDI_PORT_MASK) == TRANS_DDI_SELECT_PORT(PORT_E) &&
> - I915_READ(LPT_TRANSCONF) & TRANS_ENABLE) {
> - pipe_config->has_pch_encoder = true;
> -
> - tmp = I915_READ(FDI_RX_CTL(PIPE_A));
> - pipe_config->fdi_lanes = ((FDI_DP_PORT_WIDTH_MASK & tmp) >>
> - FDI_DP_PORT_WIDTH_SHIFT) + 1;
> -
> - ironlake_get_fdi_m_n_config(crtc, pipe_config);
> - }
> + haswell_get_ddi_port_state(crtc, pipe_config);
>
> intel_get_pipe_timings(crtc, pipe_config);
>
> @@ -9521,6 +9535,8 @@ intel_pipe_config_compare(struct drm_device *dev,
>
> PIPE_CONF_CHECK_I(double_wide);
>
> + PIPE_CONF_CHECK_X(ddi_pll_sel);
> +
> PIPE_CONF_CHECK_I(shared_dpll);
> PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
> PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
> --
> 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
