Re: [PATCH] drm/i915: Wait for vblank after enabling the primary plane on BDW

Rodrigo Vivi
2014-06-26T00:21:56Z

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

I'm sure this might affect Wayne, so, cc'ing him here.

from my point of view this is right so:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

On Tue, Jun 24, 2014 at 3:59 AM, <ville.syrjala@linux.intel.com> wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> BDW signals the flip done interrupt immediately after the DSPSURF write
> when the plane is disabled. This is true even if we've already armed
> DSPCNTR to enable the plane at the next vblank. This causes major
> problems for our page flip code which relies on the flip done interrupts
> happening at vblank time.
>
> So what happens is that we enable the plane, and immediately allow
> userspace to submit a page flip. If the plane is still in the process
> of being enabled when the page flip is issued, the flip done gets
> signalled immediately. Our DSPSURFLIVE check catches this to prevent
> premature flip completion, but it also means that we don't get a flip
> done interrupt when the plane actually gets enabled, and so the page
> flip is never completed.
>
> Work around this by re-introducing blocking vblank waits on BDW
> whenever we enable the primary plane.
>
> I removed some of the vblank waits here:
> commit 6304cd91e7f05f8802ea6f91287cac09741d9c46
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date: Fri Apr 25 13:30:12 2014 +0300
>
> drm/i915: Drop the excessive vblank waits from modeset codepaths
>
> To avoid these blocking vblank waits we should start using the vblank
> interrupt instead of the flip done interrupt to complete page flips.
> But that's material for another patch.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79354
> Tested-by: Guo Jinxian <jinxianx.guo@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
> drivers/gpu/drm/i915/intel_sprite.c | 8 ++++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 9188fed..f92efc6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2087,6 +2087,7 @@ void intel_flush_primary_plane(struct
> drm_i915_private *dev_priv,
> static void intel_enable_primary_hw_plane(struct drm_i915_private
> *dev_priv,
> enum plane plane, enum pipe pipe)
> {
> + struct drm_device *dev = dev_priv->dev;
> struct intel_crtc *intel_crtc =
> to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> int reg;
> @@ -2106,6 +2107,14 @@ static void intel_enable_primary_hw_plane(struct
> drm_i915_private *dev_priv,
>
> I915_WRITE(reg, val | DISPLAY_PLANE_ENABLE);
> intel_flush_primary_plane(dev_priv, plane);
> +
> + /*
> + * BDW signals flip done immediately if the plane
> + * is disabled, even if the plane enable is already
> + * armed to occur at the next vblank :(
> + */
> + if (IS_BROADWELL(dev))
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 1b66ddc..9a17b4e 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -691,6 +691,14 @@ intel_post_enable_primary(struct drm_crtc *crtc)
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>
> /*
> + * BDW signals flip done immediately if the plane
> + * is disabled, even if the plane enable is already
> + * armed to occur at the next vblank :(
> + */
> + if (IS_BROADWELL(dev))
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
> + /*
> * FIXME IPS should be fine as long as one plane is
> * enabled, but in practice it seems to have problems
> * when going from primary only to sprite only and vice
> --
> 1.8.5.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[-- Attachment #1.2: Type: text/html, Size: 5678 bytes --]

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