[2/6] drm/i915: fix the FBC work allocation failure path

Commit Message

Paulo Zanoni July 8, 2015, 8:58 p.m. UTC

From: Paulo Zanoni <[e-mail]>

We were forgetting to set fbc.crtc, fbc.fb_id and fbc.y when the
kzalloc call failed. Luckily, this is not a very common case, I hope.

Signed-off-by: Paulo Zanoni <[e-mail]>
---
 drivers/gpu/drm/i915/intel_fbc.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

Comments

Chris Wilson July 8, 2015, 10:05 p.m. UTC | #1

On Wed, Jul 08, 2015 at 05:58:55PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <[e-mail]>
> 
> We were forgetting to set fbc.crtc, fbc.fb_id and fbc.y when the
> kzalloc call failed. Luckily, this is not a very common case, I hope.

That did not prepare me for the patch that lies ahead.

"Always update the currrent crtc, fb and vertical offset after calling
enable_fbc. We were forgetting to do so along the failure paths when
enabling fbc synchronously. Fix this with a new helper to enable_fbc()
and update the state simultaneously."
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 1448746..64f317a 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -308,6 +308,18 @@  bool intel_fbc_enabled(struct drm_i915_private *dev_priv)
 	return dev_priv->fbc.enabled;
 }
 
+static void intel_fbc_enable(struct intel_crtc *crtc,
+			     struct drm_framebuffer *fb)
+{
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+
+	dev_priv->fbc.enable_fbc(crtc);
+
+	dev_priv->fbc.crtc = crtc;
+	dev_priv->fbc.fb_id = fb->base.id;
+	dev_priv->fbc.y = crtc->base.y;
+}
+
 static void intel_fbc_work_fn(struct work_struct *__work)
 {
 	struct intel_fbc_work *work =
@@ -321,13 +333,8 @@  static void intel_fbc_work_fn(struct work_struct *__work)
 		/* Double check that we haven't switched fb without cancelling
 		 * the prior work.
 		 */
-		if (crtc_fb == work->fb) {
-			dev_priv->fbc.enable_fbc(work->crtc);
-
-			dev_priv->fbc.crtc = work->crtc;
-			dev_priv->fbc.fb_id = crtc_fb->base.id;
-			dev_priv->fbc.y = work->crtc->base.y;
-		}
+		if (crtc_fb == work->fb)
+			intel_fbc_enable(work->crtc, work->fb);
 
 		dev_priv->fbc.fbc_work = NULL;
 	}
@@ -361,7 +368,7 @@  static void intel_fbc_cancel_work(struct drm_i915_private *dev_priv)
 	dev_priv->fbc.fbc_work = NULL;
 }
 
-static void intel_fbc_enable(struct intel_crtc *crtc)
+static void intel_fbc_schedule_enable(struct intel_crtc *crtc)
 {
 	struct intel_fbc_work *work;
 	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
@@ -373,7 +380,7 @@  static void intel_fbc_enable(struct intel_crtc *crtc)
 	work = kzalloc(sizeof(*work), GFP_KERNEL);
 	if (work == NULL) {
 		DRM_ERROR("Failed to allocate FBC work structure\n");
-		dev_priv->fbc.enable_fbc(crtc);
+		intel_fbc_enable(crtc, crtc->base.primary->fb);
 		return;
 	}
 
@@ -826,7 +833,7 @@  static void __intel_fbc_update(struct drm_i915_private *dev_priv)
 		__intel_fbc_disable(dev_priv);
 	}
 
-	intel_fbc_enable(intel_crtc);
+	intel_fbc_schedule_enable(intel_crtc);
 	dev_priv->fbc.no_fbc_reason = FBC_OK;
 	return;