Re: [PATCH] drm/i915: dsi: fix pipe-off timeout due to port vs. pipe disable ordering

["Kumar, Shobhit" <shobhit.kumar@intel.com>]

On 5/27/2014 9:30 PM, Imre Deak wrote:
> If we disable first the port (by disabling DPI) and only then the
> display pipe the pipe-off flag will never be set, possibly leading to a
> hanged pipe state at the next modeset-enable.
>
> Note that according to the VLV2 display cluster HAS, we should disable
> the port before the pipe. This doesn't seem to match reality based on
> the above and it's also asymmetric with the enabling sequence, where we
> first enable the port and then the pipe.
>
> v2:
> - send the panel shutdown command before stopping the pipe, since this
>    is the recommended sequence (Shobhit)
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Shobhit Kumar <shobhit.kumar@intel.com>

Regards
Shobhit

> ---
>   drivers/gpu/drm/i915/intel_dsi.c | 25 ++++++++++++++++++-------
>   1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 3ea3839..b397465 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -207,14 +207,9 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
>   	 */
>   }
>
> -static void intel_dsi_disable(struct intel_encoder *encoder)
> +static void intel_dsi_pre_disable(struct intel_encoder *encoder)
>   {
> -	struct drm_device *dev = encoder->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>   	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> -	int pipe = intel_crtc->pipe;
> -	u32 temp;
>
>   	DRM_DEBUG_KMS("\n");
>
> @@ -222,7 +217,21 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
>   		/* Send Shutdown command to the panel in LP mode */
>   		dpi_send_cmd(intel_dsi, SHUTDOWN, DPI_LP_MODE_EN);
>   		msleep(10);
> +	}
> +}
>
> +static void intel_dsi_disable(struct intel_encoder *encoder)
> +{
> +	struct drm_device *dev = encoder->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	int pipe = intel_crtc->pipe;
> +	u32 temp;
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	if (is_vid_mode(intel_dsi)) {
>   		/* de-assert ip_tg_enable signal */
>   		temp = I915_READ(MIPI_PORT_CTRL(pipe));
>   		I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
> @@ -294,6 +303,8 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
>
>   	DRM_DEBUG_KMS("\n");
>
> +	intel_dsi_disable(encoder);
> +
>   	intel_dsi_clear_device_ready(encoder);
>
>   	val = I915_READ(DSPCLK_GATE_D);
> @@ -692,7 +703,7 @@ bool intel_dsi_init(struct drm_device *dev)
>   	intel_encoder->pre_pll_enable = intel_dsi_pre_pll_enable;
>   	intel_encoder->pre_enable = intel_dsi_pre_enable;
>   	intel_encoder->enable = intel_dsi_enable_nop;
> -	intel_encoder->disable = intel_dsi_disable;
> +	intel_encoder->disable = intel_dsi_pre_disable;
>   	intel_encoder->post_disable = intel_dsi_post_disable;
>   	intel_encoder->get_hw_state = intel_dsi_get_hw_state;
>   	intel_encoder->get_config = intel_dsi_get_config;
>