drm/i915/fbc: rewrite the multiple_pipes_ok() code for locking

Older FBC platforms have this restriction where FBC can't be enabled
if multiple pipes are enabled. In the current code, we disable FBC
before the second pipe becomes visible.

One of the problems with this code is that the current
multiple_pipes_ok() implementation just iterates through all CRTCs
looking at their states, but it doesn't make sure that the state
locks are grabbed. It also can't just grab the locks for every CRTC
since this would kill one of the biggest advantages of atomic
modesetting.

After the recent FBC changes, we now have the appropriate locks for
the given CRTC, so we can just try to maintain the state of each CRTC
and update it once intel_fbc_pre_update is called.

As a last note, I don't have gen 2/3 machines to test this code. My
current plan is to enable FBC on just the newer platforms, so this
patch is just an attempt to get the gen 2/3 code at least looking
sane, so if one day someone decide to fix FBC on these platforms, they
may have less work to do.

Not-tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (only on HSW+)
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-16-git-send-email-paulo.r.zanoni@intel.com

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0df991371001d2dd40e1a2eb19de5fc56399deec..e4da97c6ff9ce8623676a57de712793fcef536a2 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -907,6 +907,7 @@ struct intel_fbc {
        unsigned threshold;
        unsigned int possible_framebuffer_bits;
        unsigned int busy_bits;
+       unsigned int visible_pipes_mask;
        struct intel_crtc *crtc;
 
        struct drm_mm_node compressed_fb;

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 60108e69982e832cf575c1ecde434fd53af9cfa3..136668a4e6843efd2c7a0e8458f7916254a50b72 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15912,6 +15912,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
                        modeset_put_power_domains(dev_priv, put_domains);
        }
        intel_display_set_init_power(dev_priv, false);
+
+       intel_fbc_init_pipe_state(dev_priv);
 }
 
 void intel_display_resume(struct drm_device *dev)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 06b04acba3b3b280fe95268bb0f004aa5d6668dc..00a835990bdb9f63f3572f788f0797ee902ad696 100644 (file)
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1333,6 +1333,7 @@ bool intel_fbc_is_active(struct drm_i915_private *dev_priv);
 void intel_fbc_pre_update(struct intel_crtc *crtc);
 void intel_fbc_post_update(struct intel_crtc *crtc);
 void intel_fbc_init(struct drm_i915_private *dev_priv);
+void intel_fbc_init_pipe_state(struct drm_i915_private *dev_priv);
 void intel_fbc_enable(struct intel_crtc *crtc);
 void intel_fbc_disable(struct intel_crtc *crtc);
 void intel_fbc_global_disable(struct drm_i915_private *dev_priv);

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 35d265092bc16041332341d332f90c65809aacd5..c2ef400a959962c9a900a9948410a069ed24b902 100644 (file)
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -56,6 +56,11 @@ static inline bool fbc_on_plane_a_only(struct drm_i915_private *dev_priv)
        return INTEL_INFO(dev_priv)->gen < 4;
 }
 
+static inline bool no_fbc_on_multiple_pipes(struct drm_i915_private *dev_priv)
+{
+       return INTEL_INFO(dev_priv)->gen <= 3;
+}
+
 /*
  * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the
  * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's
@@ -481,25 +486,25 @@ static bool crtc_can_fbc(struct intel_crtc *crtc)
        return true;
 }
 
-static bool multiple_pipes_ok(struct drm_i915_private *dev_priv)
+static bool multiple_pipes_ok(struct intel_crtc *crtc)
 {
-       enum pipe pipe;
-       int n_pipes = 0;
-       struct drm_crtc *crtc;
+       struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+       struct drm_plane *primary = crtc->base.primary;
+       struct intel_fbc *fbc = &dev_priv->fbc;
+       enum pipe pipe = crtc->pipe;
 
-       if (INTEL_INFO(dev_priv)->gen > 4)
+       /* Don't even bother tracking anything we don't need. */
+       if (!no_fbc_on_multiple_pipes(dev_priv))
                return true;
 
-       /* FIXME: we don't have the appropriate state locks to do this here. */
-       for_each_pipe(dev_priv, pipe) {
-               crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+       WARN_ON(!drm_modeset_is_locked(&primary->mutex));
 
-               if (intel_crtc_active(crtc) &&
-                   to_intel_plane_state(crtc->primary->state)->visible)
-                       n_pipes++;
-       }
+       if (to_intel_plane_state(primary->state)->visible)
+               fbc->visible_pipes_mask |= (1 << pipe);
+       else
+               fbc->visible_pipes_mask &= ~(1 << pipe);
 
-       return (n_pipes < 2);
+       return (fbc->visible_pipes_mask & ~(1 << pipe)) != 0;
 }
 
 static int find_compression_threshold(struct drm_i915_private *dev_priv,
@@ -891,7 +896,7 @@ void intel_fbc_pre_update(struct intel_crtc *crtc)
 
        mutex_lock(&fbc->lock);
 
-       if (!multiple_pipes_ok(dev_priv)) {
+       if (!multiple_pipes_ok(crtc)) {
                set_no_fbc_reason(dev_priv, "more than one pipe active");
                goto deactivate;
        }
@@ -1123,6 +1128,28 @@ void intel_fbc_global_disable(struct drm_i915_private *dev_priv)
        cancel_work_sync(&fbc->work.work);
 }
 
+/**
+ * intel_fbc_init_pipe_state - initialize FBC's CRTC visibility tracking
+ * @dev_priv: i915 device instance
+ *
+ * The FBC code needs to track CRTC visibility since the older platforms can't
+ * have FBC enabled while multiple pipes are used. This function does the
+ * initial setup at driver load to make sure FBC is matching the real hardware.
+ */
+void intel_fbc_init_pipe_state(struct drm_i915_private *dev_priv)
+{
+       struct intel_crtc *crtc;
+
+       /* Don't even bother tracking anything if we don't need. */
+       if (!no_fbc_on_multiple_pipes(dev_priv))
+               return;
+
+       for_each_intel_crtc(dev_priv->dev, crtc)
+               if (intel_crtc_active(&crtc->base) &&
+                   to_intel_plane_state(crtc->base.primary->state)->visible)
+                       dev_priv->fbc.visible_pipes_mask |= (1 << crtc->pipe);
+}
+
 /**
  * intel_fbc_init - Initialize FBC
  * @dev_priv: the i915 device