From: Paulo Zanoni <paulo.r.zan...@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.zan...@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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to