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

[Kumar, Shobhit]

On 5/27/2014 6:45 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.
>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dsi.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index e73bec6..3756af1 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -206,6 +206,11 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
>   	 */
>   }
>
> +static void intel_dsi_disable_nop(struct intel_encoder *encoder)
> +{
> +	DRM_DEBUG_KMS("\n");
> +}
> +

Make this as *_pre_disable and the SHUTDOWN command should be sent in 
this function. That is the recommended sequence in disable path. First 
thing is to shutdown the panel. Remove that bit from the intel_dsi_disable.

>   static void intel_dsi_disable(struct intel_encoder *encoder)
>   {
>   	struct drm_device *dev = encoder->base.dev;
> @@ -293,6 +298,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);
> @@ -691,7 +698,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_disable_nop;

intel_encoder->disable = intel_dsi_pre_disable
as suggested above

>   	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;
>

Regards
Shobhit