[v7,04/20] drm/i915: Skip reset request if there is one already

Submitted by Michel Thierry on April 27, 2017, 11:12 p.m.

Details

Message ID 20170427231300.32841-5-michel.thierry@intel.com
State New
Headers show
Series "Gen8+ engine-reset" ( rev: 10 9 8 7 6 5 4 ) in Intel GFX

Browsing this patch as part of:
"Gen8+ engine-reset" rev 7 in Intel GFX
<< prev patch [2/20] next patch >>

Commit Message

Michel Thierry April 27, 2017, 11:12 p.m.
From: Mika Kuoppala <mika.kuoppala@linux.intel.com>

To perform engine reset we first disable engine to capture its state. This
is done by issuing a reset request. Because we are reusing existing
infrastructure, again when we actually reset an engine, reset function
checks engine mask and issues reset request again which is unnecessary. To
avoid this we check if the engine is already prepared, if so we just exit
from that point.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 3ebba6b2dd74..120fb440bb8b 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1686,10 +1686,15 @@  int intel_wait_for_register(struct drm_i915_private *dev_priv,
 static int gen8_reset_engine_start(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
+	const i915_reg_t reset_ctrl = RING_RESET_CTL(engine->mmio_base);
+	const u32 ready = RESET_CTL_REQUEST_RESET | RESET_CTL_READY_TO_RESET;
 	int ret;
 
-	I915_WRITE_FW(RING_RESET_CTL(engine->mmio_base),
-		      _MASKED_BIT_ENABLE(RESET_CTL_REQUEST_RESET));
+	/* If engine has been already prepared, we can shortcut here */
+	if ((I915_READ_FW(reset_ctrl) & ready) == ready)
+		return 0;
+
+	I915_WRITE_FW(reset_ctrl, _MASKED_BIT_ENABLE(RESET_CTL_REQUEST_RESET));
 
 	ret = intel_wait_for_register_fw(dev_priv,
 					 RING_RESET_CTL(engine->mmio_base),

Comments

On Thu, Apr 27, 2017 at 04:12:44PM -0700, Michel Thierry wrote:
> From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> 
> To perform engine reset we first disable engine to capture its state. This
> is done by issuing a reset request. Because we are reusing existing
> infrastructure, again when we actually reset an engine, reset function
> checks engine mask and issues reset request again which is unnecessary. To
> avoid this we check if the engine is already prepared, if so we just exit
> from that point.

Do we still need this? I am a bit dubious because it implies we have no
idea what we are doing, recursively calling resets.
-Chris
On 29/04/17 07:21, Chris Wilson wrote:
> On Thu, Apr 27, 2017 at 04:12:44PM -0700, Michel Thierry wrote:
>> From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>>
>> To perform engine reset we first disable engine to capture its state. This
>> is done by issuing a reset request. Because we are reusing existing
>> infrastructure, again when we actually reset an engine, reset function
>> checks engine mask and issues reset request again which is unnecessary. To
>> avoid this we check if the engine is already prepared, if so we just exit
>> from that point.
>
> Do we still need this? I am a bit dubious because it implies we have no
> idea what we are doing, recursively calling resets.
> -Chris
>

I can drop this one. It isn't really needed (the 'shortcut' it refers is 
because we already set the bit in intel_reset_engine_start).

btw here it's only setting/querying "Ready-ness for Reset", and I've 
heard rumours that the register may not clear itself sometimes (but I 
haven't seen that behaviour myself).

-Michel