[Intel-gfx] [PATCH 08/24] drm/i915: Shuffle wait_for_vblank out of primary_enable/disable funcs

[Paulo Zanoni]

2014-03-07 13:32 GMT-03:00  <ville.syrjala at linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> 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 at linux.intel.com>
> ---
>  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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni