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