[Intel-gfx] [PATCH] drm/i915: Wait for vblank after enabling the primary plane on BDW
Ville Syrjälä
ville.syrjala at linux.intel.com
Tue Aug 12 15:02:17 CEST 2014
On Tue, Aug 12, 2014 at 11:24:11AM +0300, Ville Syrjälä wrote:
> On Mon, Aug 11, 2014 at 04:44:23PM -0300, Paulo Zanoni wrote:
> > 2014-06-30 7:10 GMT-03:00 Jani Nikula <jani.nikula at linux.intel.com>:
> > > On Thu, 26 Jun 2014, Rodrigo Vivi <rodrigo.vivi at gmail.com> wrote:
> > >> 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 at intel.com>
> > >
> > > Pushed to -fixes, thanks for the patch and review.
> > >
> > > BR,
> > > Jani.
> > >
> > >
> > >>
> > >>
> > >> On Tue, Jun 24, 2014 at 3:59 AM, <ville.syrjala at linux.intel.com> wrote:
> > >>
> > >>> From: Ville Syrjälä <ville.syrjala at 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 at 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 at intel.com>
> > >>> Signed-off-by: Ville Syrjälä <ville.syrjala at 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);
> >
> > This chunk triggers "WARN(ret == 0)" from drm_wait_one_vblank when
> > using HDMI on BDW.
>
> Are we still calling drm_vblank_off() too soon or something?
Oh it was enable. That could mean we still haven't yet called
drm_vblank_on(). But at least the order in
intel_crtc_enable_planes() seems correct, and also
intel_primary_plane_setplane() should never try enable the
primary plane when the crtc is not active ('visible'
should be false in that case).
>
> >
> >
> > >>> }
> > >>>
> > >>> /**
> > >>> 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 at lists.freedesktop.org
> > >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >>>
> > >>
> > >>
> > >>
> > >> --
> > >> Rodrigo Vivi
> > >> Blog: http://blog.vivi.eng.br
> > >> _______________________________________________
> > >> Intel-gfx mailing list
> > >> Intel-gfx at lists.freedesktop.org
> > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
> > > --
> > > Jani Nikula, Intel Open Source Technology Center
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
> >
> > --
> > Paulo Zanoni
>
> --
> Ville Syrjälä
> Intel OTC
--
Ville Syrjälä
Intel OTC
More information about the Intel-gfx
mailing list