| Message ID | 1408389369-22898-15-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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;
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;