[Intel-gfx] [PATCH v6 14/20] drm/i915/guc: Add support for reset engine using GuC commands

[Chris Wilson]

On Wed, Apr 19, 2017 at 04:22:43PM -0700, Michel Thierry wrote:
> On 19/04/17 03:27, Chris Wilson wrote:
> >On Tue, Apr 18, 2017 at 01:23:29PM -0700, Michel Thierry wrote:
> >>This patch adds per engine reset and recovery (TDR) support when GuC is
> >>used to submit workloads to GPU.
> >>
> >>In the case of i915 directly submission to ELSP, driver manages hang
> >>detection, recovery and resubmission. With GuC submission these tasks
> >>are shared between driver and GuC. i915 is still responsible for detecting
> >>a hang, and when it does it only requests GuC to reset that Engine. GuC
> >>internally manages acquiring forcewake and idling the engine before actually
> >>resetting it.
> >>
> >>Once the reset is successful, i915 takes over again and handles resubmission.
> >>The scheduler in i915 knows which requests are pending so after resetting
> >>a engine, pending workloads/requests are resubmitted again.
> >>
> >>v2: s/i915_guc_request_engine_reset/i915_guc_reset_engine/ to match the
> >>non-guc funtion names.
> >>
> >>Signed-off-by: Arun Siluvery <arun.siluvery at linux.intel.com>
> >>Signed-off-by: Jeff McGee <jeff.mcgee at intel.com>
> >>Signed-off-by: Michel Thierry <michel.thierry at intel.com>
> >>---
> >>diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >>index 7df278fe492e..6295760098a1 100644
> >>--- a/drivers/gpu/drm/i915/intel_lrc.c
> >>+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >>@@ -1176,14 +1176,15 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine)
> >>
> >> 	/* After a GPU reset, we may have requests to replay */
> >> 	clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
> >>-	if (!i915.enable_guc_submission && !execlists_elsp_idle(engine)) {
> >>+	if (!execlists_elsp_idle(engine)) {
> >> 		DRM_DEBUG_DRIVER("Restarting %s from requests [0x%x, 0x%x]\n",
> >> 				 engine->name,
> >> 				 port_seqno(&engine->execlist_port[0]),
> >> 				 port_seqno(&engine->execlist_port[1]));
> >> 		engine->execlist_port[0].count = 0;
> >> 		engine->execlist_port[1].count = 0;
> >>-		execlists_submit_ports(engine);
> >>+		if (!dev_priv->guc.execbuf_client)
> >>+			execlists_submit_ports(engine);
> >
> >Not sure what you were intending to do here as this only resets the
> >submission count -- which is not used by guc dequeue. Some merit in the
> >making the code look similar, certainly adds the dbg message but I think
> >it is unrelated to the rest of the patch.
> 
> Yes, it only keeps the same debug message (originally added to check
> it was taking the right path). I can remove if you think it doesn't
> provide anything useful.

Just a small patch by itself, it is only a distraction to the larger
patch.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre