2014-03-07 13:32 GMT-03:00 <ville.syrjala@xxxxxxxxxxxxxxx>: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Rather than have a wait_for_vblank() in the primary plane enable/disable > funcs, move the wait_for_vblank() to happen after enabling/disabling all > planes. Why exactly? What is improved? Are we solving a bug? What are the risks? What's the problem with the current code? Did you check the modeset sequence documentation of every single platform (since you changed them all) to make sure this is safe? Please update the commit message with the answers. Also, we should probably update the first comment of hsw_enable_ips. It seems things have changed since it was written. > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_display.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 2815351..4986887 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1899,7 +1899,6 @@ static void intel_enable_primary_plane(struct drm_i915_private *dev_priv, > > I915_WRITE(reg, val | DISPLAY_PLANE_ENABLE); > intel_flush_primary_plane(dev_priv, plane); > - intel_wait_for_vblank(dev_priv->dev, pipe); > } > > /** > @@ -1927,7 +1926,6 @@ static void intel_disable_primary_plane(struct drm_i915_private *dev_priv, > > I915_WRITE(reg, val & ~DISPLAY_PLANE_ENABLE); > intel_flush_primary_plane(dev_priv, plane); > - intel_wait_for_vblank(dev_priv->dev, pipe); > } > > static bool need_vtd_wa(struct drm_device *dev) > @@ -3550,6 +3548,7 @@ static void ilk_crtc_enable_planes(struct drm_crtc *crtc) > intel_enable_primary_plane(dev_priv, plane, pipe); > intel_enable_planes(crtc); > intel_crtc_update_cursor(crtc, true); > + intel_wait_for_vblank(dev, pipe); > > hsw_enable_ips(intel_crtc); > > @@ -3579,6 +3578,7 @@ static void ilk_crtc_disable_planes(struct drm_crtc *crtc) > intel_crtc_update_cursor(crtc, false); > intel_disable_planes(crtc); > intel_disable_primary_plane(dev_priv, plane, pipe); > + intel_wait_for_vblank(dev, pipe); > } > > static void ironlake_crtc_enable(struct drm_crtc *crtc) > @@ -4211,6 +4211,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) > intel_enable_primary_plane(dev_priv, plane, pipe); > intel_enable_planes(crtc); > intel_crtc_update_cursor(crtc, true); > + intel_wait_for_vblank(dev, pipe); > > intel_update_fbc(dev); > > @@ -4258,6 +4259,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) > > /* Give the overlay scaler a chance to enable if it's on this pipe */ > intel_crtc_dpms_overlay(intel_crtc, true); > + intel_wait_for_vblank(dev, pipe); > > intel_update_fbc(dev); > > @@ -4308,6 +4310,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) > intel_crtc_update_cursor(crtc, false); > intel_disable_planes(crtc); > intel_disable_primary_plane(dev_priv, plane, pipe); > + intel_wait_for_vblank(dev, pipe); > > intel_set_cpu_fifo_underrun_reporting(dev, pipe, false); > intel_disable_pipe(dev_priv, pipe); > -- > 1.8.3.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
