On Thu, 28 Mar 2013 10:42:02 +0100
Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> Clock computations and handling are highly encoder specific, both in
> the optimal clock selection and also in which clocks to use and when
> sharing of clocks is possible.
>
> So the best place to do this is somewhere in the encoders, with a
> generic fallback for those encoders without special needs. To facility
> this, add a pipe_config->clocks_set boolean.
>
> This patch here is only prep work, it simply sets the computed clock
> values in pipe_config->dpll, and uses that data in the hw clock
> setting functions.
>
> Haswell code isn't touched, simply because Haswell clocks work much
> different and need their own infrastructure (with probably a
> Haswell-specific config->ddi_clock substruct).
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 155 +++++++++++++++++++----------------
> drivers/gpu/drm/i915/intel_drv.h | 12 +++
> 2 files changed, 95 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c9e873e..5319133 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4118,37 +4118,38 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
> return refclk;
> }
>
> -static void i9xx_adjust_sdvo_tv_clock(struct drm_display_mode *adjusted_mode,
> - intel_clock_t *clock)
> +static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc *crtc)
> {
> + unsigned dotclock = crtc->config.adjusted_mode.clock;
> + struct dpll *clock = &crtc->config.dpll;
> +
> /* SDVO TV has fixed PLL values depend on its clock range,
> this mirrors vbios setting. */
> - if (adjusted_mode->clock >= 100000
> - && adjusted_mode->clock < 140500) {
> + if (dotclock >= 100000 && dotclock < 140500) {
> clock->p1 = 2;
> clock->p2 = 10;
> clock->n = 3;
> clock->m1 = 16;
> clock->m2 = 8;
> - } else if (adjusted_mode->clock >= 140500
> - && adjusted_mode->clock <= 200000) {
> + } else if (dotclock >= 140500 && dotclock <= 200000) {
> clock->p1 = 1;
> clock->p2 = 10;
> clock->n = 6;
> clock->m1 = 12;
> clock->m2 = 8;
> }
> +
> + crtc->config.clock_set = true;
> }
>
> -static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
> - intel_clock_t *clock,
> +static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
> intel_clock_t *reduced_clock)
> {
> - struct drm_device *dev = crtc->dev;
> + struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - int pipe = intel_crtc->pipe;
> + int pipe = crtc->pipe;
> u32 fp, fp2 = 0;
> + struct dpll *clock = &crtc->config.dpll;
>
> if (IS_PINEVIEW(dev)) {
> fp = (1 << clock->n) << 16 | clock->m1 << 8 | clock->m2;
> @@ -4164,11 +4165,11 @@ static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
>
> I915_WRITE(FP0(pipe), fp);
>
> - intel_crtc->lowfreq_avail = false;
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> + crtc->lowfreq_avail = false;
> + if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
> reduced_clock && i915_powersave) {
> I915_WRITE(FP1(pipe), fp2);
> - intel_crtc->lowfreq_avail = true;
> + crtc->lowfreq_avail = true;
> } else {
> I915_WRITE(FP1(pipe), fp);
> }
> @@ -4182,14 +4183,11 @@ static void intel_dp_set_m_n(struct intel_crtc *crtc)
> intel_cpu_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
> }
>
> -static void vlv_update_pll(struct drm_crtc *crtc,
> - intel_clock_t *clock, intel_clock_t *reduced_clock,
> - int num_connectors)
> +static void vlv_update_pll(struct intel_crtc *crtc)
> {
> - struct drm_device *dev = crtc->dev;
> + struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - int pipe = intel_crtc->pipe;
> + int pipe = crtc->pipe;
> u32 dpll, mdiv, pdiv;
> u32 bestn, bestm1, bestm2, bestp1, bestp2;
> bool is_sdvo;
> @@ -4197,8 +4195,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>
> mutex_lock(&dev_priv->dpio_lock);
>
> - is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> - intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> + is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
> + intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
>
> dpll = DPLL_VGA_MODE_DIS;
> dpll |= DPLL_EXT_BUFFER_ENABLE_VLV;
> @@ -4208,11 +4206,11 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> I915_WRITE(DPLL(pipe), dpll);
> POSTING_READ(DPLL(pipe));
>
> - bestn = clock->n;
> - bestm1 = clock->m1;
> - bestm2 = clock->m2;
> - bestp1 = clock->p1;
> - bestp2 = clock->p2;
> + bestn = crtc->config.dpll.n;
> + bestm1 = crtc->config.dpll.m1;
> + bestm2 = crtc->config.dpll.m2;
> + bestp1 = crtc->config.dpll.p1;
> + bestp2 = crtc->config.dpll.p2;
>
> /*
> * In Valleyview PLL and program lane counter registers are exposed
> @@ -4244,8 +4242,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>
> intel_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x620);
>
> - if (intel_crtc->config.has_dp_encoder)
> - intel_dp_set_m_n(intel_crtc);
> + if (crtc->config.has_dp_encoder)
> + intel_dp_set_m_n(crtc);
>
> I915_WRITE(DPLL(pipe), dpll);
>
> @@ -4256,8 +4254,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> temp = 0;
> if (is_sdvo) {
> temp = 0;
> - if (intel_crtc->config.pixel_multiplier > 1) {
> - temp = (intel_crtc->config.pixel_multiplier - 1)
> + if (crtc->config.pixel_multiplier > 1) {
> + temp = (crtc->config.pixel_multiplier - 1)
> << DPLL_MD_UDI_MULTIPLIER_SHIFT;
> }
> }
> @@ -4265,16 +4263,15 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> POSTING_READ(DPLL_MD(pipe));
>
> /* Now program lane control registers */
> - if(intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)
> - || intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> - {
> + if(intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)
> + || intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI)) {
> temp = 0x1000C4;
> if(pipe == 1)
> temp |= (1 << 21);
> intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL1, temp);
> }
> - if(intel_pipe_has_type(crtc,INTEL_OUTPUT_EDP))
> - {
> +
> + if(intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP)) {
> temp = 0x1000C4;
> if(pipe == 1)
> temp |= (1 << 21);
> @@ -4284,39 +4281,39 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> mutex_unlock(&dev_priv->dpio_lock);
> }
>
> -static void i9xx_update_pll(struct drm_crtc *crtc,
> - intel_clock_t *clock, intel_clock_t *reduced_clock,
> +static void i9xx_update_pll(struct intel_crtc *crtc,
> + intel_clock_t *reduced_clock,
> int num_connectors)
> {
> - struct drm_device *dev = crtc->dev;
> + struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_encoder *encoder;
> - int pipe = intel_crtc->pipe;
> + int pipe = crtc->pipe;
> u32 dpll;
> bool is_sdvo;
> + struct dpll *clock = &crtc->config.dpll;
>
> - i9xx_update_pll_dividers(crtc, clock, reduced_clock);
> + i9xx_update_pll_dividers(crtc, reduced_clock);
>
> - is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> - intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> + is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
> + intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
>
> dpll = DPLL_VGA_MODE_DIS;
>
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
> + if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS))
> dpll |= DPLLB_MODE_LVDS;
> else
> dpll |= DPLLB_MODE_DAC_SERIAL;
>
> if (is_sdvo) {
> - if ((intel_crtc->config.pixel_multiplier > 1) &&
> + if ((crtc->config.pixel_multiplier > 1) &&
> (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))) {
> - dpll |= (intel_crtc->config.pixel_multiplier - 1)
> + dpll |= (crtc->config.pixel_multiplier - 1)
> << SDVO_MULTIPLIER_SHIFT_HIRES;
> }
> dpll |= DPLL_DVO_HIGH_SPEED;
> }
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
> + if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT))
> dpll |= DPLL_DVO_HIGH_SPEED;
>
> /* compute bitmask from p1 value */
> @@ -4344,13 +4341,13 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
> if (INTEL_INFO(dev)->gen >= 4)
> dpll |= (6 << PLL_LOAD_PULSE_PHASE_SHIFT);
>
> - if (is_sdvo && intel_pipe_has_type(crtc, INTEL_OUTPUT_TVOUT))
> + if (is_sdvo && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
> dpll |= PLL_REF_INPUT_TVCLKINBC;
> - else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_TVOUT))
> + else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
> /* XXX: just matching BIOS for now */
> /* dpll |= PLL_REF_INPUT_TVCLKINBC; */
> dpll |= 3;
> - else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> + else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
> intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
> else
> @@ -4361,12 +4358,12 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
> POSTING_READ(DPLL(pipe));
> udelay(150);
>
> - for_each_encoder_on_crtc(dev, crtc, encoder)
> + for_each_encoder_on_crtc(dev, &crtc->base, encoder)
> if (encoder->pre_pll_enable)
> encoder->pre_pll_enable(encoder);
>
> - if (intel_crtc->config.has_dp_encoder)
> - intel_dp_set_m_n(intel_crtc);
> + if (crtc->config.has_dp_encoder)
> + intel_dp_set_m_n(crtc);
>
> I915_WRITE(DPLL(pipe), dpll);
>
> @@ -4378,8 +4375,8 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
> u32 temp = 0;
> if (is_sdvo) {
> temp = 0;
> - if (intel_crtc->config.pixel_multiplier > 1) {
> - temp = (intel_crtc->config.pixel_multiplier - 1)
> + if (crtc->config.pixel_multiplier > 1) {
> + temp = (crtc->config.pixel_multiplier - 1)
> << DPLL_MD_UDI_MULTIPLIER_SHIFT;
> }
> }
> @@ -4394,23 +4391,23 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
> }
> }
>
> -static void i8xx_update_pll(struct drm_crtc *crtc,
> +static void i8xx_update_pll(struct intel_crtc *crtc,
> struct drm_display_mode *adjusted_mode,
> - intel_clock_t *clock, intel_clock_t *reduced_clock,
> + intel_clock_t *reduced_clock,
> int num_connectors)
> {
> - struct drm_device *dev = crtc->dev;
> + struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_encoder *encoder;
> - int pipe = intel_crtc->pipe;
> + int pipe = crtc->pipe;
> u32 dpll;
> + struct dpll *clock = &crtc->config.dpll;
>
> - i9xx_update_pll_dividers(crtc, clock, reduced_clock);
> + i9xx_update_pll_dividers(crtc, reduced_clock);
>
> dpll = DPLL_VGA_MODE_DIS;
>
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> + if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS)) {
> dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
> } else {
> if (clock->p1 == 2)
> @@ -4421,7 +4418,7 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
> dpll |= PLL_P2_DIVIDE_BY_4;
> }
>
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> + if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
> intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
> else
> @@ -4432,7 +4429,7 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
> POSTING_READ(DPLL(pipe));
> udelay(150);
>
> - for_each_encoder_on_crtc(dev, crtc, encoder)
> + for_each_encoder_on_crtc(dev, &crtc->base, encoder)
> if (encoder->pre_pll_enable)
> encoder->pre_pll_enable(encoder);
>
> @@ -4579,20 +4576,26 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> &clock,
> &reduced_clock);
> }
> + /* Compat-code for transition, will disappear. */
> + if (!intel_crtc->config.clock_set) {
> + intel_crtc->config.dpll.n = clock.n;
> + intel_crtc->config.dpll.m1 = clock.m1;
> + intel_crtc->config.dpll.m2 = clock.m2;
> + intel_crtc->config.dpll.p1 = clock.p1;
> + intel_crtc->config.dpll.p2 = clock.p2;
> + }
>
> if (is_sdvo && is_tv)
> - i9xx_adjust_sdvo_tv_clock(adjusted_mode, &clock);
> + i9xx_adjust_sdvo_tv_clock(intel_crtc);
>
> if (IS_GEN2(dev))
> - i8xx_update_pll(crtc, adjusted_mode, &clock,
> + i8xx_update_pll(intel_crtc, adjusted_mode,
> has_reduced_clock ? &reduced_clock : NULL,
> num_connectors);
> else if (IS_VALLEYVIEW(dev))
> - vlv_update_pll(crtc, &clock,
> - has_reduced_clock ? &reduced_clock : NULL,
> - num_connectors);
> + vlv_update_pll(intel_crtc);
> else
> - i9xx_update_pll(crtc, &clock,
> + i9xx_update_pll(intel_crtc,
> has_reduced_clock ? &reduced_clock : NULL,
> num_connectors);
>
> @@ -5221,7 +5224,7 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
> }
>
> if (is_sdvo && is_tv)
> - i9xx_adjust_sdvo_tv_clock(adjusted_mode, clock);
> + i9xx_adjust_sdvo_tv_clock(to_intel_crtc(crtc));
>
> return true;
> }
> @@ -5525,6 +5528,14 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> DRM_ERROR("Couldn't find PLL settings for mode!\n");
> return -EINVAL;
> }
> + /* Compat-code for transition, will disappear. */
> + if (!intel_crtc->config.clock_set) {
> + intel_crtc->config.dpll.n = clock.n;
> + intel_crtc->config.dpll.m1 = clock.m1;
> + intel_crtc->config.dpll.m2 = clock.m2;
> + intel_crtc->config.dpll.p1 = clock.p1;
> + intel_crtc->config.dpll.p2 = clock.p2;
> + }
>
> /* Ensure that the cursor is valid for the new mode before changing... */
> intel_crtc_update_cursor(crtc, true);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2a86a12..d27885a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -196,6 +196,18 @@ struct intel_crtc_config {
> * accordingly. */
> bool has_dp_encoder;
> bool dither;
> +
> + /* Controls for the clock computation, to override various stages. */
> + bool clock_set;
> +
> + /* Settings for the intel dpll used on pretty much everything but
> + * haswell. */
> + struct dpll {
> + unsigned n;
> + unsigned m1, m2;
> + unsigned p1, p2;
> + } dpll;
> +
> int pipe_bpp;
> struct intel_link_m_n dp_m_n;
> /**
This one's hard to review since you mixed in a drm_crtc->intel_crtc
function arg change.
I'd rather have that split out, but it looks ok.
Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
