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

Imre Deak
2014-09-03T11:21:06Z

[-- Attachment #1.1: Type: text/plain, Size: 3008 bytes --]

On Mon, 2014-08-18 at 22:16 +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>

Reviewed-by: Imre Deak <imre.deak@intel.com>

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

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]