[Intel-gfx] [PATCH 46/66] drm/i915: Move hsw_fdi_link_train into intel_crt.c
Daniel Vetter
daniel at ffwll.ch
Thu May 22 23:57:02 CEST 2014
On Thu, May 22, 2014 at 05:28:05PM -0300, Paulo Zanoni wrote:
> 2014-04-24 18:55 GMT-03:00 Daniel Vetter <daniel.vetter at ffwll.ch>:
> > The pch encoder case really isn't anything generic on hsw:
> > - It's for the vga port only and
> > - the vga port does only exist on some hsw platforms.
> >
> > Imo it helps the generic code flow a lot if we shovel all this into
> > hsw specific enable/disable hooks. A bonus is that some of our largest
> > files (intel_ddi.c and intel_display.c) will lose a pile of really big
> > functions.
> >
> > Step one is to move the fdi link training code.
>
> I don't think that helps much, since the other fdi link train code is
> still at intel_display.c, and even if the only thing that needs fdi
> link training on HSW is CRT, fdi link training is really _not_ a CRT
> thing. So IMHO we're breaking an abstraction here by putting fdi link
> training in CRT code. Also, the fact that HSW+ won't be using
> dev_priv->display.fdi_link_train will makes things even more confusing
> for people reading our code.
>
> I'd really prefer if we merge
> http://patchwork.freedesktop.org/patch/24019/ instead. Or something
> based on that.
Well my idea with all this was that even on hsw vga on the pch is a bit
the special case, and on bdw+ it's completely gone. So taking this out of
the common modeset code should help bdw+ a bit since now the modeset
sequence and our code really nicely align.
Also with ilk-ivb having the fdi code in the crtc functions made a lot of
sense since the big crossbar was in the pch. So doing everything up to the
pch crossbar, including all the fdi handling in crtc code, and everything
after that in encoder callbacks.
But on hsw+ we have the big crossbar on the cpu side, between the
transcoders and the ddi ports. So from a functional pov it makes much more
sense imo to have all the pipe/transcoder code in the crtc, and all the
ddi handling code in encoder callbacks. The pch and fdi link that hang off
ddi port E work more like a drm_bridge (but we don't need that since the
lpt pch will never be used outside of intel platforms).
So with this example, grouping all the fdi code together only groups
functions by their name. But moving the hsw fdi stuff into the crt encoder
groups the code by the behaviour of the hardware. And that's massively
different betweent ivb and hsw.
I hope this explains a bit my thinking here.
-Daniel
>
>
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > ---
> > drivers/gpu/drm/i915/intel_crt.c | 136 +++++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_ddi.c | 134 +---------------------------------
> > drivers/gpu/drm/i915/intel_display.c | 4 --
> > drivers/gpu/drm/i915/intel_drv.h | 3 +-
> > 4 files changed, 140 insertions(+), 137 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> > index f34d1df88918..2d8f4fe1b450 100644
> > --- a/drivers/gpu/drm/i915/intel_crt.c
> > +++ b/drivers/gpu/drm/i915/intel_crt.c
> > @@ -143,6 +143,141 @@ static bool hsw_crt_get_hw_state(struct intel_encoder *encoder,
> > return intel_ddi_get_port_state(encoder, pipe, PORT_E);
> > }
> >
> > +static const long hsw_ddi_buf_ctl_values[] = {
> > + DDI_BUF_EMP_400MV_0DB_HSW,
> > + DDI_BUF_EMP_400MV_3_5DB_HSW,
> > + DDI_BUF_EMP_400MV_6DB_HSW,
> > + DDI_BUF_EMP_400MV_9_5DB_HSW,
> > + DDI_BUF_EMP_600MV_0DB_HSW,
> > + DDI_BUF_EMP_600MV_3_5DB_HSW,
> > + DDI_BUF_EMP_600MV_6DB_HSW,
> > + DDI_BUF_EMP_800MV_0DB_HSW,
> > + DDI_BUF_EMP_800MV_3_5DB_HSW
> > +};
> > +
> > +static void hsw_fdi_link_train(struct drm_crtc *crtc)
> > +{
> > + struct drm_device *dev = crtc->dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > + u32 temp, i, rx_ctl_val;
> > +
> > + /* Set the FDI_RX_MISC pwrdn lanes and the 2 workarounds listed at the
> > + * mode set "sequence for CRT port" document:
> > + * - TP1 to TP2 time with the default value
> > + * - FDI delay to 90h
> > + *
> > + * WaFDIAutoLinkSetTimingOverrride:hsw
> > + */
> > + I915_WRITE(_FDI_RXA_MISC, FDI_RX_PWRDN_LANE1_VAL(2) |
> > + FDI_RX_PWRDN_LANE0_VAL(2) |
> > + FDI_RX_TP1_TO_TP2_48 | FDI_RX_FDI_DELAY_90);
> > +
> > + /* Enable the PCH Receiver FDI PLL */
> > + rx_ctl_val = dev_priv->fdi_rx_config | FDI_RX_ENHANCE_FRAME_ENABLE |
> > + FDI_RX_PLL_ENABLE |
> > + FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
> > + I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
> > + POSTING_READ(_FDI_RXA_CTL);
> > + udelay(220);
> > +
> > + /* Switch from Rawclk to PCDclk */
> > + rx_ctl_val |= FDI_PCDCLK;
> > + I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
> > +
> > + /* Configure Port Clock Select */
> > + I915_WRITE(PORT_CLK_SEL(PORT_E), intel_crtc->ddi_pll_sel);
> > +
> > + /* Start the training iterating through available voltages and emphasis,
> > + * testing each value twice. */
> > + for (i = 0; i < ARRAY_SIZE(hsw_ddi_buf_ctl_values) * 2; i++) {
> > + /* Configure DP_TP_CTL with auto-training */
> > + I915_WRITE(DP_TP_CTL(PORT_E),
> > + DP_TP_CTL_FDI_AUTOTRAIN |
> > + DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> > + DP_TP_CTL_LINK_TRAIN_PAT1 |
> > + DP_TP_CTL_ENABLE);
> > +
> > + /* Configure and enable DDI_BUF_CTL for DDI E with next voltage.
> > + * DDI E does not support port reversal, the functionality is
> > + * achieved on the PCH side in FDI_RX_CTL, so no need to set the
> > + * port reversal bit */
> > + I915_WRITE(DDI_BUF_CTL(PORT_E),
> > + DDI_BUF_CTL_ENABLE |
> > + ((intel_crtc->config.fdi_lanes - 1) << 1) |
> > + hsw_ddi_buf_ctl_values[i / 2]);
> > + POSTING_READ(DDI_BUF_CTL(PORT_E));
> > +
> > + udelay(600);
> > +
> > + /* Program PCH FDI Receiver TU */
> > + I915_WRITE(_FDI_RXA_TUSIZE1, TU_SIZE(64));
> > +
> > + /* Enable PCH FDI Receiver with auto-training */
> > + rx_ctl_val |= FDI_RX_ENABLE | FDI_LINK_TRAIN_AUTO;
> > + I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
> > + POSTING_READ(_FDI_RXA_CTL);
> > +
> > + /* Wait for FDI receiver lane calibration */
> > + udelay(30);
> > +
> > + /* Unset FDI_RX_MISC pwrdn lanes */
> > + temp = I915_READ(_FDI_RXA_MISC);
> > + temp &= ~(FDI_RX_PWRDN_LANE1_MASK | FDI_RX_PWRDN_LANE0_MASK);
> > + I915_WRITE(_FDI_RXA_MISC, temp);
> > + POSTING_READ(_FDI_RXA_MISC);
> > +
> > + /* Wait for FDI auto training time */
> > + udelay(5);
> > +
> > + temp = I915_READ(DP_TP_STATUS(PORT_E));
> > + if (temp & DP_TP_STATUS_AUTOTRAIN_DONE) {
> > + DRM_DEBUG_KMS("FDI link training done on step %d\n", i);
> > +
> > + /* Enable normal pixel sending for FDI */
> > + I915_WRITE(DP_TP_CTL(PORT_E),
> > + DP_TP_CTL_FDI_AUTOTRAIN |
> > + DP_TP_CTL_LINK_TRAIN_NORMAL |
> > + DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> > + DP_TP_CTL_ENABLE);
> > +
> > + return;
> > + }
> > +
> > + temp = I915_READ(DDI_BUF_CTL(PORT_E));
> > + temp &= ~DDI_BUF_CTL_ENABLE;
> > + I915_WRITE(DDI_BUF_CTL(PORT_E), temp);
> > + POSTING_READ(DDI_BUF_CTL(PORT_E));
> > +
> > + /* Disable DP_TP_CTL and FDI_RX_CTL and retry */
> > + temp = I915_READ(DP_TP_CTL(PORT_E));
> > + temp &= ~(DP_TP_CTL_ENABLE | DP_TP_CTL_LINK_TRAIN_MASK);
> > + temp |= DP_TP_CTL_LINK_TRAIN_PAT1;
> > + I915_WRITE(DP_TP_CTL(PORT_E), temp);
> > + POSTING_READ(DP_TP_CTL(PORT_E));
> > +
> > + intel_wait_ddi_buf_idle(dev_priv, PORT_E);
> > +
> > + rx_ctl_val &= ~FDI_RX_ENABLE;
> > + I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
> > + POSTING_READ(_FDI_RXA_CTL);
> > +
> > + /* Reset FDI_RX_MISC pwrdn lanes */
> > + temp = I915_READ(_FDI_RXA_MISC);
> > + temp &= ~(FDI_RX_PWRDN_LANE1_MASK | FDI_RX_PWRDN_LANE0_MASK);
> > + temp |= FDI_RX_PWRDN_LANE1_VAL(2) | FDI_RX_PWRDN_LANE0_VAL(2);
> > + I915_WRITE(_FDI_RXA_MISC, temp);
> > + POSTING_READ(_FDI_RXA_MISC);
> > + }
> > +
> > + DRM_ERROR("FDI link training failed!\n");
> > +}
> > +
> > +static void hsw_crt_pre_enable(struct intel_encoder *encoder)
> > +{
> > + hsw_fdi_link_train(encoder->base.crtc);
> > +}
> > +
> > /* Note: The caller is required to filter out dpms modes not supported by the
> > * platform. */
> > static void intel_crt_set_dpms(struct intel_encoder *encoder, int mode)
> > @@ -865,6 +1000,7 @@ void intel_crt_init(struct drm_device *dev)
> > if (HAS_DDI(dev)) {
> > crt->base.get_config = hsw_crt_get_config;
> > crt->base.get_hw_state = hsw_crt_get_hw_state;
> > + crt->base.pre_enable = hsw_crt_pre_enable;
> > } else {
> > crt->base.get_config = intel_crt_get_config;
> > crt->base.get_hw_state = intel_crt_get_hw_state;
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index afa1e87c54cc..13abde3e848f 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -211,20 +211,8 @@ void intel_prepare_ddi(struct drm_device *dev)
> > intel_prepare_ddi_buffers(dev, port);
> > }
> >
> > -static const long hsw_ddi_buf_ctl_values[] = {
> > - DDI_BUF_EMP_400MV_0DB_HSW,
> > - DDI_BUF_EMP_400MV_3_5DB_HSW,
> > - DDI_BUF_EMP_400MV_6DB_HSW,
> > - DDI_BUF_EMP_400MV_9_5DB_HSW,
> > - DDI_BUF_EMP_600MV_0DB_HSW,
> > - DDI_BUF_EMP_600MV_3_5DB_HSW,
> > - DDI_BUF_EMP_600MV_6DB_HSW,
> > - DDI_BUF_EMP_800MV_0DB_HSW,
> > - DDI_BUF_EMP_800MV_3_5DB_HSW
> > -};
> > -
> > -static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
> > - enum port port)
> > +void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
> > + enum port port)
> > {
> > uint32_t reg = DDI_BUF_CTL(port);
> > int i;
> > @@ -246,124 +234,6 @@ static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
> > * DDI A (which is used for eDP)
> > */
> >
> > -void hsw_fdi_link_train(struct drm_crtc *crtc)
> > -{
> > - struct drm_device *dev = crtc->dev;
> > - struct drm_i915_private *dev_priv = dev->dev_private;
> > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > - u32 temp, i, rx_ctl_val;
> > -
> > - /* Set the FDI_RX_MISC pwrdn lanes and the 2 workarounds listed at the
> > - * mode set "sequence for CRT port" document:
> > - * - TP1 to TP2 time with the default value
> > - * - FDI delay to 90h
> > - *
> > - * WaFDIAutoLinkSetTimingOverrride:hsw
> > - */
> > - I915_WRITE(_FDI_RXA_MISC, FDI_RX_PWRDN_LANE1_VAL(2) |
> > - FDI_RX_PWRDN_LANE0_VAL(2) |
> > - FDI_RX_TP1_TO_TP2_48 | FDI_RX_FDI_DELAY_90);
> > -
> > - /* Enable the PCH Receiver FDI PLL */
> > - rx_ctl_val = dev_priv->fdi_rx_config | FDI_RX_ENHANCE_FRAME_ENABLE |
> > - FDI_RX_PLL_ENABLE |
> > - FDI_DP_PORT_WIDTH(intel_crtc->config.fdi_lanes);
> > - I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
> > - POSTING_READ(_FDI_RXA_CTL);
> > - udelay(220);
> > -
> > - /* Switch from Rawclk to PCDclk */
> > - rx_ctl_val |= FDI_PCDCLK;
> > - I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
> > -
> > - /* Configure Port Clock Select */
> > - I915_WRITE(PORT_CLK_SEL(PORT_E), intel_crtc->ddi_pll_sel);
> > -
> > - /* Start the training iterating through available voltages and emphasis,
> > - * testing each value twice. */
> > - for (i = 0; i < ARRAY_SIZE(hsw_ddi_buf_ctl_values) * 2; i++) {
> > - /* Configure DP_TP_CTL with auto-training */
> > - I915_WRITE(DP_TP_CTL(PORT_E),
> > - DP_TP_CTL_FDI_AUTOTRAIN |
> > - DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> > - DP_TP_CTL_LINK_TRAIN_PAT1 |
> > - DP_TP_CTL_ENABLE);
> > -
> > - /* Configure and enable DDI_BUF_CTL for DDI E with next voltage.
> > - * DDI E does not support port reversal, the functionality is
> > - * achieved on the PCH side in FDI_RX_CTL, so no need to set the
> > - * port reversal bit */
> > - I915_WRITE(DDI_BUF_CTL(PORT_E),
> > - DDI_BUF_CTL_ENABLE |
> > - ((intel_crtc->config.fdi_lanes - 1) << 1) |
> > - hsw_ddi_buf_ctl_values[i / 2]);
> > - POSTING_READ(DDI_BUF_CTL(PORT_E));
> > -
> > - udelay(600);
> > -
> > - /* Program PCH FDI Receiver TU */
> > - I915_WRITE(_FDI_RXA_TUSIZE1, TU_SIZE(64));
> > -
> > - /* Enable PCH FDI Receiver with auto-training */
> > - rx_ctl_val |= FDI_RX_ENABLE | FDI_LINK_TRAIN_AUTO;
> > - I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
> > - POSTING_READ(_FDI_RXA_CTL);
> > -
> > - /* Wait for FDI receiver lane calibration */
> > - udelay(30);
> > -
> > - /* Unset FDI_RX_MISC pwrdn lanes */
> > - temp = I915_READ(_FDI_RXA_MISC);
> > - temp &= ~(FDI_RX_PWRDN_LANE1_MASK | FDI_RX_PWRDN_LANE0_MASK);
> > - I915_WRITE(_FDI_RXA_MISC, temp);
> > - POSTING_READ(_FDI_RXA_MISC);
> > -
> > - /* Wait for FDI auto training time */
> > - udelay(5);
> > -
> > - temp = I915_READ(DP_TP_STATUS(PORT_E));
> > - if (temp & DP_TP_STATUS_AUTOTRAIN_DONE) {
> > - DRM_DEBUG_KMS("FDI link training done on step %d\n", i);
> > -
> > - /* Enable normal pixel sending for FDI */
> > - I915_WRITE(DP_TP_CTL(PORT_E),
> > - DP_TP_CTL_FDI_AUTOTRAIN |
> > - DP_TP_CTL_LINK_TRAIN_NORMAL |
> > - DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> > - DP_TP_CTL_ENABLE);
> > -
> > - return;
> > - }
> > -
> > - temp = I915_READ(DDI_BUF_CTL(PORT_E));
> > - temp &= ~DDI_BUF_CTL_ENABLE;
> > - I915_WRITE(DDI_BUF_CTL(PORT_E), temp);
> > - POSTING_READ(DDI_BUF_CTL(PORT_E));
> > -
> > - /* Disable DP_TP_CTL and FDI_RX_CTL and retry */
> > - temp = I915_READ(DP_TP_CTL(PORT_E));
> > - temp &= ~(DP_TP_CTL_ENABLE | DP_TP_CTL_LINK_TRAIN_MASK);
> > - temp |= DP_TP_CTL_LINK_TRAIN_PAT1;
> > - I915_WRITE(DP_TP_CTL(PORT_E), temp);
> > - POSTING_READ(DP_TP_CTL(PORT_E));
> > -
> > - intel_wait_ddi_buf_idle(dev_priv, PORT_E);
> > -
> > - rx_ctl_val &= ~FDI_RX_ENABLE;
> > - I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
> > - POSTING_READ(_FDI_RXA_CTL);
> > -
> > - /* Reset FDI_RX_MISC pwrdn lanes */
> > - temp = I915_READ(_FDI_RXA_MISC);
> > - temp &= ~(FDI_RX_PWRDN_LANE1_MASK | FDI_RX_PWRDN_LANE0_MASK);
> > - temp |= FDI_RX_PWRDN_LANE1_VAL(2) | FDI_RX_PWRDN_LANE0_VAL(2);
> > - I915_WRITE(_FDI_RXA_MISC, temp);
> > - POSTING_READ(_FDI_RXA_MISC);
> > - }
> > -
> > - DRM_ERROR("FDI link training failed!\n");
> > -}
> > -
> > static struct intel_encoder *
> > intel_ddi_get_crtc_encoder(struct drm_crtc *crtc)
> > {
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 82ad84eefc8d..80b34ac31d0a 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3858,9 +3858,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> > if (intel_crtc->config.has_pch_encoder)
> > intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, true);
> >
> > - if (intel_crtc->config.has_pch_encoder)
> > - dev_priv->display.fdi_link_train(crtc);
> > -
> > for_each_encoder_on_crtc(dev, crtc, encoder)
> > if (encoder->pre_enable)
> > encoder->pre_enable(encoder);
> > @@ -11120,7 +11117,6 @@ static void intel_init_display(struct drm_device *dev)
> > dev_priv->display.modeset_global_resources =
> > ivb_modeset_global_resources;
> > } else if (IS_HASWELL(dev) || IS_GEN8(dev)) {
> > - dev_priv->display.fdi_link_train = hsw_fdi_link_train;
> > dev_priv->display.write_eld = haswell_write_eld;
> > dev_priv->display.modeset_global_resources =
> > haswell_modeset_global_resources;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 1c093e71db7d..7f1d7f675953 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -668,11 +668,12 @@ void intel_crt_init(struct drm_device *dev);
> >
> > /* intel_ddi.c */
> > void intel_prepare_ddi(struct drm_device *dev);
> > -void hsw_fdi_link_train(struct drm_crtc *crtc);
> > void intel_ddi_init(struct drm_device *dev, enum port port);
> > enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder);
> > bool intel_ddi_get_port_state(struct intel_encoder *encoder, enum pipe *pipe,
> > enum port port);
> > +void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
> > + enum port port);
> > int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv);
> > void intel_ddi_pll_init(struct drm_device *dev);
> > void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc);
> > --
> > 1.8.1.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list