On Fri, 22 May 2015, Uma Shankar <uma.shan...@intel.com> wrote:
> From: Shashank Sharma <shashank.sha...@intel.com>
>
> This patch contains following changes:
> 1. MIPI device ready changes to support dsi_pre_enable. Changes
>    are specific to BXT device ready sequence. Added check for
>    ULPS mode(No effects on VLV).
> 2. Changes in dsi_enable to pick BXT port control register.
> 3. Changes in dsi_pre_enable to restrict DPIO programming for VLV
>
> Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> Signed-off-by: Uma Shankar <uma.shan...@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |    7 ++
>  drivers/gpu/drm/i915/intel_dsi.c |  185 
> ++++++++++++++++++++++++++++----------
>  2 files changed, 144 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 57c3a48..5eeb2b7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7408,6 +7408,13 @@ enum skl_disp_power_wells {
>  #define _MIPIA_PORT_CTRL                     (VLV_DISPLAY_BASE + 0x61190)
>  #define _MIPIC_PORT_CTRL                     (VLV_DISPLAY_BASE + 0x61700)
>  #define MIPI_PORT_CTRL(port) _MIPI_PORT(port, _MIPIA_PORT_CTRL, 
> _MIPIC_PORT_CTRL)
> +
> + /* BXT port control */
> +#define _BXT_MIPIA_PORT_CTRL                           0x6B0C0
> +#define _BXT_MIPIC_PORT_CTRL                           0x6B8C0
> +#define BXT_MIPI_PORT_CTRL(tc)         _TRANSCODER(tc, _BXT_MIPIA_PORT_CTRL, 
> \
> +                                                     _BXT_MIPIC_PORT_CTRL)

Shouldn't this use _MIPI_PORT, not _TRANSCODER?

> +
>  #define  DPI_ENABLE                                  (1 << 31) /* A + C */
>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT           27
>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK            (0xf << 27)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c 
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 892b518..3a1bb04 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -286,6 +286,101 @@ static bool intel_dsi_compute_config(struct 
> intel_encoder *encoder,
>       return true;
>  }
>  
> +static void bxt_dsi_device_ready(struct intel_encoder *encoder)
> +{
> +     struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> +     struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +     enum port port;
> +     u32 val;
> +
> +     DRM_DEBUG_KMS("\n");
> +
> +     /* Exit Low power state in 4 steps*/
> +     for_each_dsi_port(port, intel_dsi->ports) {
> +
> +             /* 1. Enable MIPI PHY transparent latch */
> +             val = I915_READ(BXT_MIPI_PORT_CTRL(port));
> +             I915_WRITE(BXT_MIPI_PORT_CTRL(port), val | LP_OUTPUT_HOLD);
> +             usleep_range(2000, 2500);
> +
> +             /* 2. Enter ULPS */
> +             val = I915_READ(MIPI_DEVICE_READY(port));
> +             val &= ~ULPS_STATE_MASK;
> +             val |= (ULPS_STATE_ENTER | DEVICE_READY);
> +             I915_WRITE(MIPI_DEVICE_READY(port), val);
> +             usleep_range(2, 3);
> +
> +             /* 3. Exit ULPS */
> +             val = I915_READ(MIPI_DEVICE_READY(port));
> +             val &= ~ULPS_STATE_MASK;
> +             val |= (ULPS_STATE_EXIT | DEVICE_READY);
> +             I915_WRITE(MIPI_DEVICE_READY(port), val);
> +             usleep_range(1000, 1500);
> +
> +             /* Clear ULPS and set device ready */
> +             val = I915_READ(MIPI_DEVICE_READY(port));
> +             val &= ~ULPS_STATE_MASK;
> +             val |= DEVICE_READY;
> +             I915_WRITE(MIPI_DEVICE_READY(port), val);
> +     }
> +}
> +
> +static void vlv_dsi_device_ready(struct intel_encoder *encoder)
> +{
> +     struct drm_device *dev = encoder->base.dev;
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +     struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +     struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> +     int pipe = intel_crtc->pipe;

enum pipe.

> +     u32 val;
> +     int count = 1;
> +     u32 reg = 0;
> +     bool afe_latchout = true;
> +
> +     DRM_DEBUG_KMS("\n");
> +
> +     pipe = intel_crtc->pipe;
> +     reg = MIPI_PORT_CTRL(pipe);

Take care to not use pipe for port. Passing PIPE_B as port will *not*
use PORT_C.

> +
> +     /* Ceck LP state */
> +     val = I915_READ(reg);
> +     afe_latchout = (val & AFE_LATCHOUT);
> +
> +     /* Enter low power state, if not already in */
> +     if (afe_latchout) {
> +             /* Enter ULPS, wait for 2.5 ms */
> +             reg = MIPI_DEVICE_READY(pipe);
> +             I915_WRITE(reg, val | ULPS_STATE_ENTER);
> +             usleep_range(2000, 2500);
> +     }
> +
> +     /* Enable transparent latch */
> +     I915_WRITE(reg, val | LP_OUTPUT_HOLD);
> +
> +     if (intel_dsi->dual_link) {
> +             count = 2;
> +             pipe = PIPE_A;
> +     }
> +
> +     do {
> +             I915_WRITE(reg, val | ULPS_STATE_EXIT);
> +             usleep_range(2000, 2500);
> +             /* Clear ULPS state */
> +             val = I915_READ(MIPI_DEVICE_READY(pipe)) & ~ULPS_STATE_MASK;
> +             I915_WRITE(MIPI_DEVICE_READY(pipe), val);
> +             usleep_range(2000, 2500);
> +
> +             /* Set device ready */
> +             val |= DEVICE_READY;
> +             I915_WRITE(MIPI_DEVICE_READY(pipe), val);
> +             usleep_range(2000, 2500);
> +
> +             /* For Port C for dual link */
> +             if (intel_dsi->dual_link)
> +                     pipe = PIPE_B;
> +     } while (--count > 0);

You're making functional changes to the BYT/BSW DSI code. These *must*
be separated to independent patches.

Additionally the loop is hard to follow. If you abstract the block into
a separate function, you can just call it once for single link, and
twice for dual link, with appropriate parameters.

> +}
> +
>  static void intel_dsi_port_enable(struct intel_encoder *encoder)
>  {
>       struct drm_device *dev = encoder->base.dev;
> @@ -294,6 +389,7 @@ static void intel_dsi_port_enable(struct intel_encoder 
> *encoder)
>       struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>       enum port port;
>       u32 temp;
> +     u32 port_ctrl;
>  
>       if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
>               temp = I915_READ(VLV_CHICKEN_3);
> @@ -304,7 +400,10 @@ static void intel_dsi_port_enable(struct intel_encoder 
> *encoder)
>       }
>  
>       for_each_dsi_port(port, intel_dsi->ports) {
> -             temp = I915_READ(MIPI_PORT_CTRL(port));
> +             port_ctrl = IS_BROXTON(dev) ? BXT_MIPI_PORT_CTRL(port) :
> +                                     MIPI_PORT_CTRL(port);
> +             temp = I915_READ(port_ctrl);
> +
>               temp &= ~LANE_CONFIGURATION_MASK;
>               temp &= ~DUAL_LINK_MODE_MASK;
>  
> @@ -316,8 +415,8 @@ static void intel_dsi_port_enable(struct intel_encoder 
> *encoder)
>                                       LANE_CONFIGURATION_DUAL_LINK_A;
>               }
>               /* assert ip_tg_enable signal */
> -             I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
> -             POSTING_READ(MIPI_PORT_CTRL(port));
> +             I915_WRITE(port_ctrl, temp | DPI_ENABLE);
> +             POSTING_READ(port_ctrl);
>       }
>  }
>  
> @@ -339,41 +438,15 @@ static void intel_dsi_port_disable(struct intel_encoder 
> *encoder)
>  
>  static void intel_dsi_device_ready(struct intel_encoder *encoder)
>  {
> -     struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> -     struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> -     enum port port;
> -     u32 val;
> -
> -     DRM_DEBUG_KMS("\n");
> -
> -     mutex_lock(&dev_priv->dpio_lock);
> -     /* program rcomp for compliance, reduce from 50 ohms to 45 ohms
> -      * needed everytime after power gate */
> -     vlv_flisdsi_write(dev_priv, 0x04, 0x0004);
> -     mutex_unlock(&dev_priv->dpio_lock);
> -
> -     /* bandgap reset is needed after everytime we do power gate */
> -     band_gap_reset(dev_priv);
> -
> -     for_each_dsi_port(port, intel_dsi->ports) {
> -
> -             I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_ENTER);
> -             usleep_range(2500, 3000);
> -
> -             /* Enable MIPI PHY transparent latch
> -              * Common bit for both MIPI Port A & MIPI Port C
> -              * No similar bit in MIPI Port C reg
> -              */
> -             val = I915_READ(MIPI_PORT_CTRL(PORT_A));
> -             I915_WRITE(MIPI_PORT_CTRL(PORT_A), val | LP_OUTPUT_HOLD);
> -             usleep_range(1000, 1500);
> +     struct drm_device *dev = encoder->base.dev;
>  
> -             I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_EXIT);
> -             usleep_range(2500, 3000);
> +     if (IS_VALLEYVIEW(dev))
> +             vlv_dsi_device_ready(encoder);
> +     else if (IS_BROXTON(dev))
> +             bxt_dsi_device_ready(encoder);
> +     else
> +             DRM_ERROR("Invalid DSI device to device ready\n");

Please remove all such else branches.

>  
> -             I915_WRITE(MIPI_DEVICE_READY(port), DEVICE_READY);
> -             usleep_range(2500, 3000);
> -     }
>  }
>  
>  static void intel_dsi_enable(struct intel_encoder *encoder)
> @@ -415,19 +488,22 @@ static void intel_dsi_pre_enable(struct intel_encoder 
> *encoder)
>  
>       DRM_DEBUG_KMS("\n");
>  
> -     /* Disable DPOunit clock gating, can stall pipe
> -      * and we need DPLL REFA always enabled */
> -     tmp = I915_READ(DPLL(pipe));
> -     tmp |= DPLL_REFA_CLK_ENABLE_VLV;
> -     I915_WRITE(DPLL(pipe), tmp);
> +     if (IS_VALLEYVIEW(dev)) {
> +             /* Disable DPOunit clock gating, can stall pipe
> +              * and we need DPLL REFA always enabled
> +              */
> +             tmp = I915_READ(DPLL(pipe));
> +             tmp |= DPLL_REFA_CLK_ENABLE_VLV;
> +             I915_WRITE(DPLL(pipe), tmp);
>  
> -     /* update the hw state for DPLL */
> -     intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
> -             DPLL_REFA_CLK_ENABLE_VLV;
> +             /* update the hw state for DPLL */
> +             intel_crtc->config->dpll_hw_state.dpll =
> +                     DPLL_INTEGRATED_CLOCK_VLV | DPLL_REFA_CLK_ENABLE_VLV;
>  
> -     tmp = I915_READ(DSPCLK_GATE_D);
> -     tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> -     I915_WRITE(DSPCLK_GATE_D, tmp);
> +             tmp = I915_READ(DSPCLK_GATE_D);
> +             tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> +             I915_WRITE(DSPCLK_GATE_D, tmp);
> +     }
>  
>       /* put device in ready state */
>       intel_dsi_device_ready(encoder);
> @@ -959,11 +1035,24 @@ static void intel_dsi_prepare(struct intel_encoder 
> *intel_encoder)
>  
>  static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
>  {
> +     struct drm_device *dev = encoder->base.dev;
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +
>       DRM_DEBUG_KMS("\n");
>  
> +     if (IS_VALLEYVIEW(dev)) {
> +             mutex_lock(&dev_priv->dpio_lock);
> +             /* program rcomp for compliance, reduce from 50 ohms to 45 ohms
> +              * needed everytime after power gate */
> +             vlv_flisdsi_write(dev_priv, 0x04, 0x0004);
> +             mutex_unlock(&dev_priv->dpio_lock);
> +
> +             /* bandgap reset is needed after everytime we do power gate */
> +             band_gap_reset(dev_priv);
> +     }
> +

You're making functional changes to BYT/BSW DSI code. These *must* be
separated to independent patches.

BR,
Jani.

>       intel_dsi_prepare(encoder);
>       intel_enable_dsi_pll(encoder);
> -
>  }
>  
>  static enum drm_connector_status
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to