Re: [PATCH 14/14] drm/i915: Move DP port disable to post_disable for pch platforms

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Aug 18, 2014 at 10:16:09PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We need to turn the DP port off after the pipe, otherwise the pipe won't
> turn off properly on certain pch platforms at least (happens on my ILK for
> example).  This also matches the BSpec modeset sequence better. We still
> don't match the spec exactly though (eg. audio disable should happen
> much earlier), but at last this eliminates the nasty
> wait_for_pipe_off() timeouts.
> 
> We already did the port disable after the pipe for VLV/CHV and for CPU
> eDP.
> 
> For g4x leave the port disable where it is since that matches the
> modeset sequence in the documentation and I don't have a suitable
> machine to test if the other order would work.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

We have a pile of bug reports on this topic:

https://bugs.freedesktop.org/show_bug.cgi?id=67462
https://bugs.freedesktop.org/show_bug.cgi?id=54687
https://bugzilla.kernel.org/show_bug.cgi?id=52061
https://bugzilla.kernel.org/show_bug.cgi?id=52061

Can you please run them by these reports and collect tested-bys? If it
checks out it's imo good to go.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 12925be..915d4ec 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2194,7 +2194,6 @@ void intel_edp_psr_init(struct drm_device *dev)
>  static void intel_disable_dp(struct intel_encoder *encoder)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> -	enum port port = dp_to_dig_port(intel_dp)->port;
>  	struct drm_device *dev = encoder->base.dev;
>  
>  	/* Make sure the panel is off before trying to change the mode. But also
> @@ -2204,21 +2203,19 @@ static void intel_disable_dp(struct intel_encoder *encoder)
>  	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>  	intel_edp_panel_off(intel_dp);
>  
> -	/* cpu edp my only be disable _after_ the cpu pipe/plane is disabled. */
> -	if (!(port == PORT_A || IS_VALLEYVIEW(dev)))
> +	/* disable the port before the pipe on g4x */
> +	if (INTEL_INFO(dev)->gen < 5)
>  		intel_dp_link_down(intel_dp);
>  }
>  
> -static void g4x_post_disable_dp(struct intel_encoder *encoder)
> +static void ilk_post_disable_dp(struct intel_encoder *encoder)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>  	enum port port = dp_to_dig_port(intel_dp)->port;
>  
> -	if (port != PORT_A)
> -		return;
> -
>  	intel_dp_link_down(intel_dp);
> -	ironlake_edp_pll_off(intel_dp);
> +	if (port == PORT_A)
> +		ironlake_edp_pll_off(intel_dp);
>  }
>  
>  static void vlv_post_disable_dp(struct intel_encoder *encoder)
> @@ -5044,7 +5041,8 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  	} else {
>  		intel_encoder->pre_enable = g4x_pre_enable_dp;
>  		intel_encoder->enable = g4x_enable_dp;
> -		intel_encoder->post_disable = g4x_post_disable_dp;
> +		if (INTEL_INFO(dev)->gen >= 5)
> +			intel_encoder->post_disable = ilk_post_disable_dp;
>  	}
>  
>  	intel_dig_port->port = port;
> -- 
> 1.8.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch