On 17/05/2018 08:40, Chris Wilson wrote:
As we are splitting processing the CSB events from submitting the ELSP,
we also need to duplicate the check that we hold a device wakeref for our
hardware access to the disjoint locations.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/intel_lrc.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4928e9ad7826..6d3b03299b0c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -449,6 +449,16 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
struct execlist_port *port = execlists->port;
unsigned int n;
+ /*
+ * We can skip acquiring intel_runtime_pm_get() here as it was taken
+ * on our behalf by the request (see i915_gem_mark_busy()) and it will
+ * not be relinquished until the device is idle (see
+ * i915_gem_idle_work_handler()). As a precaution, we make sure
+ * that all ELSP are drained i.e. we have processed the CSB,
+ * before allowing ourselves to idle and calling intel_runtime_pm_put().
+ */
+ GEM_BUG_ON(!engine->i915->gt.awake);
Hmm.. I think it would be better to leave it in the tasklet and just add
another instance to process_csb.
Having it only deep in execlists_submit_ports could miss confusions when
upper layers want to submit and state say it is not possible yet.
+
/*
* ELSQ note: the submit queue is not cleared after being submitted
* to the HW so we need to make sure we always clean it up. This is
@@ -959,6 +969,12 @@ static void process_csb(struct intel_engine_cs *engine)
struct drm_i915_private *i915 = engine->i915;
bool fw = false;
+ /*
+ * We must never release our device wakeref until after we have
+ * finished processing all potential interrupts from the hardware.
+ */
+ GEM_BUG_ON(!engine->i915->gt.awake);
+
do {
/* The HWSP contains a (cacheable) mirror of the CSB */
const u32 *buf =
@@ -1139,16 +1155,6 @@ static void execlists_submission_tasklet(unsigned long data)
engine->execlists.active,
test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted));
- /*
- * We can skip acquiring intel_runtime_pm_get() here as it was taken
- * on our behalf by the request (see i915_gem_mark_busy()) and it will
- * not be relinquished until the device is idle (see
- * i915_gem_idle_work_handler()). As a precaution, we make sure
- * that all ELSP are drained i.e. we have processed the CSB,
- * before allowing ourselves to idle and calling intel_runtime_pm_put().
- */
- GEM_BUG_ON(!engine->i915->gt.awake);
-
/*
* Prefer doing test_and_clear_bit() as a two stage operation to avoid
* imposing the cost of a locked atomic transaction when submitting a
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
