[Intel-gfx] [PATCH 7/9] drm/i915: don't alloc/free fbc_work at every update

Paulo Zanoni przanoni at gmail.com
Tue Dec 23 04:35:43 PST 2014


From: Paulo Zanoni <paulo.r.zanoni at intel.com>

Because there's no need for it. Just use a static structure with a
bool field to tell if it's in use or not. The big advantage here is
not saving kzalloc/kfree calls, it's cutting the ugly "failed to
allocate FBC work structure" code path: in this path we call
enable_fbc() directly but we don't update fbc.crtc, fbc.fb_id and
fbc.y - they are updated in intel_fbc_work_fn(), which we're not
calling. And since testing out-of-memory cases like this is really
hard, getting rid of the code path is a major relief.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  3 ++-
 drivers/gpu/drm/i915/intel_fbc.c | 41 +++++++++++++++-------------------------
 2 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 18fcce4..40bc864 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -751,10 +751,11 @@ struct i915_fbc {
 	bool enabled;
 
 	struct intel_fbc_work {
+		bool scheduled;
 		struct delayed_work work;
 		struct drm_crtc *crtc;
 		struct drm_framebuffer *fb;
-	} *fbc_work;
+	} work;
 
 	enum no_fbc_reason {
 		FBC_OK, /* FBC is enabled */
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 6611266..80bdbde 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -336,7 +336,7 @@ static void intel_fbc_work_fn(struct work_struct *__work)
 
 	mutex_lock(&dev->struct_mutex);
 	mutex_lock(&dev_priv->fbc.lock);
-	if (work == dev_priv->fbc.fbc_work) {
+	if (dev_priv->fbc.work.scheduled) {
 		/* Double check that we haven't switched fb without cancelling
 		 * the prior work.
 		 */
@@ -348,42 +348,37 @@ static void intel_fbc_work_fn(struct work_struct *__work)
 			dev_priv->fbc.y = work->crtc->y;
 		}
 
-		dev_priv->fbc.fbc_work = NULL;
+		dev_priv->fbc.work.scheduled = false;
 	}
 	mutex_unlock(&dev_priv->fbc.lock);
 	mutex_unlock(&dev->struct_mutex);
-
-	kfree(work);
 }
 
 static void intel_fbc_cancel_work(struct drm_i915_private *dev_priv)
 {
 	lockdep_assert_held(&dev_priv->fbc.lock);
 
-	if (dev_priv->fbc.fbc_work == NULL)
+	if (!dev_priv->fbc.work.scheduled)
 		return;
 
 	DRM_DEBUG_KMS("cancelling pending FBC enable\n");
 
-	/* Synchronisation is provided by struct_mutex and checking of
-	 * dev_priv->fbc.fbc_work, so we can perform the cancellation
+	/* Synchronisation is provided by fbc.lock and checking of
+	 * dev_priv->fbc.work.scheduled, so we can perform the cancellation
 	 * entirely asynchronously.
 	 */
-	if (cancel_delayed_work(&dev_priv->fbc.fbc_work->work))
-		/* tasklet was killed before being run, clean up */
-		kfree(dev_priv->fbc.fbc_work);
+	cancel_delayed_work(&dev_priv->fbc.work.work);
 
 	/* Mark the work as no longer wanted so that if it does
 	 * wake-up (because the work was already running and waiting
 	 * for our mutex), it will discover that is no longer
 	 * necessary to run.
 	 */
-	dev_priv->fbc.fbc_work = NULL;
+	dev_priv->fbc.work.scheduled = false;
 }
 
 static void intel_fbc_enable(struct drm_crtc *crtc)
 {
-	struct intel_fbc_work *work;
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
@@ -394,18 +389,10 @@ static void intel_fbc_enable(struct drm_crtc *crtc)
 
 	intel_fbc_cancel_work(dev_priv);
 
-	work = kzalloc(sizeof(*work), GFP_KERNEL);
-	if (work == NULL) {
-		DRM_ERROR("Failed to allocate FBC work structure\n");
-		dev_priv->display.enable_fbc(crtc);
-		return;
-	}
+	dev_priv->fbc.work.crtc = crtc;
+	dev_priv->fbc.work.fb = crtc->primary->fb;
 
-	work->crtc = crtc;
-	work->fb = crtc->primary->fb;
-	INIT_DELAYED_WORK(&work->work, intel_fbc_work_fn);
-
-	dev_priv->fbc.fbc_work = work;
+	dev_priv->fbc.work.scheduled = true;
 
 	/* Delay the actual enabling to let pageflipping cease and the
 	 * display to settle before starting the compression. Note that
@@ -420,7 +407,7 @@ static void intel_fbc_enable(struct drm_crtc *crtc)
 	 *
 	 * WaFbcWaitForVBlankBeforeEnable:ilk,snb
 	 */
-	schedule_delayed_work(&work->work, msecs_to_jiffies(50));
+	schedule_delayed_work(&dev_priv->fbc.work.work, msecs_to_jiffies(50));
 }
 
 static void __intel_fbc_disable(struct drm_device *dev)
@@ -702,9 +689,9 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
 
 	if (dev_priv->fbc.enabled)
 		fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe);
-	else if (dev_priv->fbc.fbc_work)
+	else if (dev_priv->fbc.work.scheduled)
 		fbc_bits = INTEL_FRONTBUFFER_PRIMARY(
-			to_intel_crtc(dev_priv->fbc.fbc_work->crtc)->pipe);
+			to_intel_crtc(dev_priv->fbc.work.crtc)->pipe);
 	else
 		fbc_bits = dev_priv->fbc.possible_framebuffer_bits;
 
@@ -773,6 +760,8 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
 		return;
 	}
 
+	INIT_DELAYED_WORK(&dev_priv->fbc.work.work, intel_fbc_work_fn);
+
 	for_each_pipe(dev_priv, pipe) {
 		dev_priv->fbc.possible_framebuffer_bits |=
 				INTEL_FRONTBUFFER_PRIMARY(pipe);
-- 
2.1.3



More information about the Intel-gfx mailing list