drm/i915: Split out parking from the idle worker for reuse

We will want to park GEM before disengaging the drive^W^W^W unwedging.
Since we already do the work for idling, expose the guts as a new
function that we can then reuse.

v2: Just skip if already parked; makes it more forgiving to use by
future callers.
v3: Extract mark_busy, rename it to i915_gem_unpark and place it next to
i915_gem_park so that we can evaluate it for symmetry more easily.
Calling GEM from inside i915_request looks to be a bit of a layering
violation, for the moment I am imaging them as being notify_cb.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> #v1
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180406155144.27791-1-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9650a7b10c5f8fe418ebb81ef97d1add84c05d50..a69dc19a0bdba760e936ae5fd292054197480034 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -136,6 +136,100 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
        return 0;
 }
 
+static u32 __i915_gem_park(struct drm_i915_private *i915)
+{
+       lockdep_assert_held(&i915->drm.struct_mutex);
+       GEM_BUG_ON(i915->gt.active_requests);
+
+       if (!i915->gt.awake)
+               return I915_EPOCH_INVALID;
+
+       GEM_BUG_ON(i915->gt.epoch == I915_EPOCH_INVALID);
+
+       /*
+        * Be paranoid and flush a concurrent interrupt to make sure
+        * we don't reactivate any irq tasklets after parking.
+        *
+        * FIXME: Note that even though we have waited for execlists to be idle,
+        * there may still be an in-flight interrupt even though the CSB
+        * is now empty. synchronize_irq() makes sure that a residual interrupt
+        * is completed before we continue, but it doesn't prevent the HW from
+        * raising a spurious interrupt later. To complete the shield we should
+        * coordinate disabling the CS irq with flushing the interrupts.
+        */
+       synchronize_irq(i915->drm.irq);
+
+       intel_engines_park(i915);
+       i915_gem_timelines_park(i915);
+
+       i915_pmu_gt_parked(i915);
+
+       i915->gt.awake = false;
+
+       if (INTEL_GEN(i915) >= 6)
+               gen6_rps_idle(i915);
+
+       intel_display_power_put(i915, POWER_DOMAIN_GT_IRQ);
+
+       intel_runtime_pm_put(i915);
+
+       return i915->gt.epoch;
+}
+
+void i915_gem_park(struct drm_i915_private *i915)
+{
+       lockdep_assert_held(&i915->drm.struct_mutex);
+       GEM_BUG_ON(i915->gt.active_requests);
+
+       if (!i915->gt.awake)
+               return;
+
+       /* Defer the actual call to __i915_gem_park() to prevent ping-pongs */
+       mod_delayed_work(i915->wq, &i915->gt.idle_work, msecs_to_jiffies(100));
+}
+
+void i915_gem_unpark(struct drm_i915_private *i915)
+{
+       lockdep_assert_held(&i915->drm.struct_mutex);
+       GEM_BUG_ON(!i915->gt.active_requests);
+
+       if (i915->gt.awake)
+               return;
+
+       intel_runtime_pm_get_noresume(i915);
+
+       /*
+        * It seems that the DMC likes to transition between the DC states a lot
+        * when there are no connected displays (no active power domains) during
+        * command submission.
+        *
+        * This activity has negative impact on the performance of the chip with
+        * huge latencies observed in the interrupt handler and elsewhere.
+        *
+        * Work around it by grabbing a GT IRQ power domain whilst there is any
+        * GT activity, preventing any DC state transitions.
+        */
+       intel_display_power_get(i915, POWER_DOMAIN_GT_IRQ);
+
+       i915->gt.awake = true;
+       if (unlikely(++i915->gt.epoch == 0)) /* keep 0 as invalid */
+               i915->gt.epoch = 1;
+
+       intel_enable_gt_powersave(i915);
+       i915_update_gfx_val(i915);
+       if (INTEL_GEN(i915) >= 6)
+               gen6_rps_busy(i915);
+       i915_pmu_gt_unparked(i915);
+
+       intel_engines_unpark(i915);
+
+       i915_queue_hangcheck(i915);
+
+       queue_delayed_work(i915->wq,
+                          &i915->gt.retire_work,
+                          round_jiffies_up_relative(HZ));
+}
+
 int
 i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
                            struct drm_file *file)
@@ -3496,36 +3590,9 @@ i915_gem_idle_work_handler(struct work_struct *work)
        if (new_requests_since_last_retire(dev_priv))
                goto out_unlock;
 
-       /*
-        * Be paranoid and flush a concurrent interrupt to make sure
-        * we don't reactivate any irq tasklets after parking.
-        *
-        * FIXME: Note that even though we have waited for execlists to be idle,
-        * there may still be an in-flight interrupt even though the CSB
-        * is now empty. synchronize_irq() makes sure that a residual interrupt
-        * is completed before we continue, but it doesn't prevent the HW from
-        * raising a spurious interrupt later. To complete the shield we should
-        * coordinate disabling the CS irq with flushing the interrupts.
-        */
-       synchronize_irq(dev_priv->drm.irq);
-
-       intel_engines_park(dev_priv);
-       i915_gem_timelines_park(dev_priv);
-
-       i915_pmu_gt_parked(dev_priv);
+       epoch = __i915_gem_park(dev_priv);
 
-       GEM_BUG_ON(!dev_priv->gt.awake);
-       dev_priv->gt.awake = false;
-       epoch = dev_priv->gt.epoch;
-       GEM_BUG_ON(epoch == I915_EPOCH_INVALID);
        rearm_hangcheck = false;
-
-       if (INTEL_GEN(dev_priv) >= 6)
-               gen6_rps_idle(dev_priv);
-
-       intel_display_power_put(dev_priv, POWER_DOMAIN_GT_IRQ);
-
-       intel_runtime_pm_put(dev_priv);
 out_unlock:
        mutex_unlock(&dev_priv->drm.struct_mutex);
 

diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 8922344fc21bcde3480b2e729a29ca73d7642922..deaf78d2ae8be8cefc091436167b7229b70e4ddd 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -27,6 +27,8 @@
 
 #include <linux/bug.h>
 
+struct drm_i915_private;
+
 #ifdef CONFIG_DRM_I915_DEBUG_GEM
 #define GEM_BUG_ON(condition) do { if (unlikely((condition))) {        \
                pr_err("%s:%d GEM_BUG_ON(%s)\n", \
@@ -61,4 +63,7 @@
 
 #define I915_NUM_ENGINES 8
 
+void i915_gem_park(struct drm_i915_private *i915);
+void i915_gem_unpark(struct drm_i915_private *i915);
+
 #endif /* __I915_GEM_H__ */

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 585242831974d02665a6a7797cdb91ba07aed3ba..a9d0bde16443f7d7884ef538e20fb864d63c0f7a 100644 (file)
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -255,47 +255,6 @@ int i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno)
        return reset_all_global_seqno(i915, seqno - 1);
 }
 
-static void mark_busy(struct drm_i915_private *i915)
-{
-       if (i915->gt.awake)
-               return;
-
-       GEM_BUG_ON(!i915->gt.active_requests);
-
-       intel_runtime_pm_get_noresume(i915);
-
-       /*
-        * It seems that the DMC likes to transition between the DC states a lot
-        * when there are no connected displays (no active power domains) during
-        * command submission.
-        *
-        * This activity has negative impact on the performance of the chip with
-        * huge latencies observed in the interrupt handler and elsewhere.
-        *
-        * Work around it by grabbing a GT IRQ power domain whilst there is any
-        * GT activity, preventing any DC state transitions.
-        */
-       intel_display_power_get(i915, POWER_DOMAIN_GT_IRQ);
-
-       i915->gt.awake = true;
-       if (unlikely(++i915->gt.epoch == 0)) /* keep 0 as invalid */
-               i915->gt.epoch = 1;
-
-       intel_enable_gt_powersave(i915);
-       i915_update_gfx_val(i915);
-       if (INTEL_GEN(i915) >= 6)
-               gen6_rps_busy(i915);
-       i915_pmu_gt_unparked(i915);
-
-       intel_engines_unpark(i915);
-
-       i915_queue_hangcheck(i915);
-
-       queue_delayed_work(i915->wq,
-                          &i915->gt.retire_work,
-                          round_jiffies_up_relative(HZ));
-}
-
 static int reserve_engine(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *i915 = engine->i915;
@@ -313,7 +272,7 @@ static int reserve_engine(struct intel_engine_cs *engine)
        }
 
        if (!i915->gt.active_requests++)
-               mark_busy(i915);
+               i915_gem_unpark(i915);
 
        return 0;
 }
@@ -322,13 +281,8 @@ static void unreserve_engine(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *i915 = engine->i915;
 
-       if (!--i915->gt.active_requests) {
-               /* Cancel the mark_busy() from our reserve_engine() */
-               GEM_BUG_ON(!i915->gt.awake);
-               mod_delayed_work(i915->wq,
-                                &i915->gt.idle_work,
-                                msecs_to_jiffies(100));
-       }
+       if (!--i915->gt.active_requests)
+               i915_gem_park(i915);
 
        GEM_BUG_ON(!engine->timeline->inflight_seqnos);
        engine->timeline->inflight_seqnos--;