Re: [BXT MIPI PATCH v2 06/13] drm/i915/bxt: DSI enable for BXT

[Jani Nikula <jani.nikula@linux.intel.com>]

On Sun, 26 Jul 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Shashank Sharma <shashank.sharma@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
>
> v2: Fixed Jani's review comments. Removed the changes in VLV/CHV
>     code. Fixed the macros to get proper port offsets.
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |    9 +++
>  drivers/gpu/drm/i915/intel_dsi.c |  156 ++++++++++++++++++++++++++------------
>  2 files changed, 115 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8796b25..7559062 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7487,6 +7487,15 @@ 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)	_MIPI_PORT(tc, _BXT_MIPIA_PORT_CTRL, \
> +						_BXT_MIPIC_PORT_CTRL)
> +#define GET_DSI_PORT_CTRL(dev) (IS_BROXTON(dev) ? BXT_MIPI_PORT_CTRL(port) : \
> +						MIPI_PORT_CTRL(port))

I think you should just leave that IS_BROXTON check in the code, and
drop the GET_DSI_PORT_CTRL macro. We haven't really done this sort of
thing, and part of the reason (AFAICS) is that it's more obvious to read
the code when you know which register will be used.

> +
>  #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 0b20534..c5889c4 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -281,6 +281,85 @@ 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_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->sb_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->sb_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);
> +
> +		I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_EXIT);
> +		usleep_range(2500, 3000);
> +
> +		I915_WRITE(MIPI_DEVICE_READY(port), DEVICE_READY);
> +		usleep_range(2500, 3000);
> +	}
> +}

I'd prefer you put these functions where the intel_dsi_device_ready
function is now instead of separating these from it. You'll also get a
nice diff where you see that things are not changed for vlv.

> +
> +
>  static void intel_dsi_port_enable(struct intel_encoder *encoder)
>  {
>  	struct drm_device *dev = encoder->base.dev;
> @@ -289,6 +368,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);
> @@ -299,7 +379,9 @@ 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 = GET_DSI_PORT_CTRL(dev);
> +		temp = I915_READ(port_ctrl);
> +
>  		temp &= ~LANE_CONFIGURATION_MASK;
>  		temp &= ~DUAL_LINK_MODE_MASK;
>  
> @@ -311,8 +393,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);
>  	}
>  }
>  
> @@ -334,41 +416,12 @@ 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->sb_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->sb_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);
> -
> -		I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_EXIT);
> -		usleep_range(2500, 3000);
> +	struct drm_device *dev = encoder->base.dev;
>  
> -		I915_WRITE(MIPI_DEVICE_READY(port), DEVICE_READY);
> -		usleep_range(2500, 3000);
> -	}
> +	if (IS_VALLEYVIEW(dev))
> +		vlv_dsi_device_ready(encoder);
> +	else if (IS_BROXTON(dev))
> +		bxt_dsi_device_ready(encoder);
>  }
>  
>  static void intel_dsi_enable(struct intel_encoder *encoder)
> @@ -410,19 +463,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_REF_CLK_ENABLE_VLV;
> -	I915_WRITE(DPLL(pipe), tmp);
> -
> -	/* update the hw state for DPLL */
> -	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
> -		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> -
> -	tmp = I915_READ(DSPCLK_GATE_D);
> -	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> -	I915_WRITE(DSPCLK_GATE_D, 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_REF_CLK_ENABLE_VLV;
> +		I915_WRITE(DPLL(pipe), tmp);
> +
> +		/* update the hw state for DPLL */
> +		intel_crtc->config->dpll_hw_state.dpll =
> +			DPLL_INTEGRATED_REF_CLK_VLV |
> +			DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> +
> +		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);
> -- 
> 1.7.9.5
>

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