Re: [PATCH 06/12] drm/i915/bxt: DSI enable for BXT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 22 May 2015, Uma Shankar <uma.shankar@xxxxxxxxx> wrote:
> From: Shashank Sharma <shashank.sharma@xxxxxxxxx>
>
> 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.sharma@xxxxxxxxx>
> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux