Re: [PATCH 1/2] drm/i915: clock readout support for DDI v2

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Jan 20, 2014 at 02:18:03PM -0800, Jesse Barnes wrote:
> Read out and calculate the port and pixel clocks on DDI configs as well.
> This means we have to grab the DP divider values and look at the port
> mapping to figure out which clock select reg to read out.
> 
> v2: do the work from ddi_get_config (Ville)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  6 ++++
>  drivers/gpu/drm/i915/intel_ddi.c     | 61 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c |  2 ++
>  3 files changed, 69 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a699efd..644e4f9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5318,8 +5318,13 @@
>  #define  WRPLL_PLL_SELECT_LCPLL_2700	(0x03<<28)
>  /* WRPLL divider programming */
>  #define  WRPLL_DIVIDER_REFERENCE(x)	((x)<<0)
> +#define  WRPLL_DIVIDER_REF_MASK		(0xff)
>  #define  WRPLL_DIVIDER_POST(x)		((x)<<8)
> +#define  WRPLL_DIVIDER_POST_MASK	(0x3f<<8)
> +#define  WRPLL_DIVIDER_POST_SHIFT	8
>  #define  WRPLL_DIVIDER_FEEDBACK(x)	((x)<<16)
> +#define  WRPLL_DIVIDER_FB_SHIFT		16
> +#define  WRPLL_DIVIDER_FB_MASK		(0xff<<16)
>  
>  /* Port clock selection */
>  #define PORT_CLK_SEL_A			0x46100
> @@ -5332,6 +5337,7 @@
>  #define  PORT_CLK_SEL_WRPLL1		(4<<29)
>  #define  PORT_CLK_SEL_WRPLL2		(5<<29)
>  #define  PORT_CLK_SEL_NONE		(7<<29)
> +#define  PORT_CLK_SEL_MASK		(7<<29)
>  
>  /* Transcoder clock selection */
>  #define TRANS_CLK_SEL_A			0x46140
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 1488b28..d836c16 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -633,6 +633,65 @@ static void wrpll_update_rnp(uint64_t freq2k, unsigned budget,
>  	/* Otherwise a < c && b >= d, do nothing */
>  }
>  
> +static int intel_ddi_calc_wrpll_link(u32 wrpll)
> +{
> +	int n, p, r;
> +
> +	r = wrpll & WRPLL_DIVIDER_REF_MASK;
> +	p = (wrpll & WRPLL_DIVIDER_POST_MASK) >> WRPLL_DIVIDER_POST_SHIFT;
> +	n = (wrpll & WRPLL_DIVIDER_FB_MASK) >> WRPLL_DIVIDER_FB_SHIFT;
> +
> +	return (LC_FREQ * n) / (p * r);

This is assuming the WRPLL will use the LCPLL as reference. Ideally we
should read out the ref clock settings too.

> +}
> +
> +static void haswell_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;
> +	u32 val, pll;
> +
> +	val = I915_READ(PORT_CLK_SEL(port));
> +	switch (val & PORT_CLK_SEL_MASK) {
> +	case PORT_CLK_SEL_LCPLL_810:
> +		link_clock = 81000;
> +		break;
> +	case PORT_CLK_SEL_LCPLL_1350:
> +		link_clock = 135000;
> +		break;
> +	case PORT_CLK_SEL_LCPLL_2700:
> +		link_clock = 270000;
> +		break;
> +	case PORT_CLK_SEL_WRPLL1:
> +		pll = I915_READ(WRPLL_CTL1);
> +		link_clock = intel_ddi_calc_wrpll_link(pll);
> +		break;
> +	case PORT_CLK_SEL_WRPLL2:
> +		pll = I915_READ(WRPLL_CTL2);
> +		link_clock = intel_ddi_calc_wrpll_link(pll);
> +		break;
> +	case PORT_CLK_SEL_SPLL:
> +		link_clock = 135000;

SPLL could also output 810 MHz.

> +		break;
> +	default:
> +		WARN(1, "bad port clock sel\n");
> +		return;
> +	}

Could do the port_clock = link_clock * 2; here, and then pass port clock
to intel_dotclock_calculate() and avoid having to multiply crtc_clock by
2 afterwards.

As a side note, I must say it's a bit annoying that the DDI PLL code is
different to the rest of our PLL code. Hz vs. kHz etc. Makes it a bit
harder to figure out what it's doing.

> +
> +	if (pipe_config->has_pch_encoder)
> +		pipe_config->adjusted_mode.crtc_clock =
> +			intel_dotclock_calculate(link_clock,
> +						 &pipe_config->fdi_m_n);
> +	else

else if (has_dp_encoder)

> +		pipe_config->adjusted_mode.crtc_clock =
> +			intel_dotclock_calculate(link_clock,
> +						 &pipe_config->dp_m_n);

else
	.crtc_clock = link_clock; // or port_clock if you take my suggestion above

> +
> +	pipe_config->port_clock = link_clock * 2;
> +	pipe_config->adjusted_mode.crtc_clock *= 2;
> +}
> +
>  static void
>  intel_ddi_calculate_wrpll(int clock /* in Hz */,
>  			  unsigned *r2_out, unsigned *n2_out, unsigned *p_out)
> @@ -1504,6 +1563,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  			      pipe_config->pipe_bpp, dev_priv->vbt.edp_bpp);
>  		dev_priv->vbt.edp_bpp = pipe_config->pipe_bpp;
>  	}
> +
> +	haswell_ddi_clock_get(encoder, pipe_config);
>  }
>  
>  static void intel_ddi_destroy(struct drm_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 74137d5..40e5252 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6998,6 +6998,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  		ironlake_get_fdi_m_n_config(crtc, pipe_config);
>  	}
>  
> +	intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder,
> +				     &pipe_config->dp_m_n);

Not needed. We already do intel_dp_get_m_n() in intel_ddi_get_config().

>  	intel_get_pipe_timings(crtc, pipe_config);
>  
>  	pfit_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC