[Intel-gfx] [PATCH 0/5] Split GuC submission from execlists submission

[Intel-gfx] [PATCH 0/5] Split GuC submission from execlists submission

[Daniele Ceraolo Spurio]

Now that we have a common set of function for general lrc management,
the only remaining dependency the guc submission code has towards the
execlists submission is the engine setup. This series gets rid of that
by copying the required execlists setup function in the GuC submission
file; the copied functions have been further simplified by removing all
the parts that are specific to the execlists submission back-end.

To make the work easier, a bunch of GuC code that is not applicable to
the latest GuC submission flow (which should be posted soon-ish) is
removed as part of the series.

While doing this split I've noticed that it is not very clear which
settings belong in the engine setup and which ones in the
set_default_submission() call. I believe most of it can be moved from
the latter to the former, but, given that this impacts the execlists
backend as well, I plan to tackle it separately.

Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>

Daniele Ceraolo Spurio (4):
  drm/i915/guc: do not dump execlists state with GuC submission
  drm/i915/guc: init engine directly in GuC submission mode
  drm/i915/guc: stop calling execlists_set_default_submission
  drm/i915/guc: enable only the user interrupt when using GuC submission

Matthew Brost (1):
  drm/i915/guc: Delete GuC code unused in future patches

 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   9 +-
 .../drm/i915/gt/intel_execlists_submission.c  |   6 +-
 .../drm/i915/gt/intel_execlists_submission.h  |   2 -
 drivers/gpu/drm/i915/gt/intel_gt_irq.c        |  18 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc.c        |  16 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc.h        |   7 -
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 477 ++++++++++--------
 .../gpu/drm/i915/gt/uc/intel_guc_submission.h |   1 +
 8 files changed, 291 insertions(+), 245 deletions(-)

-- 
2.29.2

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

[Intel-gfx] [PATCH 1/5] drm/i915/guc: Delete GuC code unused in future patches

[Daniele Ceraolo Spurio]

From: Matthew Brost <matthew.brost@intel.com>

Delete GuC code unused in future patches that rewrite the GuC interface
to work with the new firmware. Most of the code deleted relates to
workqueues or execlist port.

The defines + structs for the process descriptor and workqueue remain.
Although the new GuC interface does not require either of these for the
normal submission path multi-lrc submission does. The usage of the
process descriptor and workqueue for multi-lrc will be quite different
from the code that is deleted in this patch. A future patch will
implement multi-lrc submission.

Signed-off-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: John Harrison <john.c.harrison@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.c        |  16 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc.h        |   7 -
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 170 +-----------------
 3 files changed, 3 insertions(+), 190 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 2a343a977987..4545e90e3bf1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -579,20 +579,8 @@ int intel_guc_reset_engine(struct intel_guc *guc,
  */
 int intel_guc_resume(struct intel_guc *guc)
 {
-	u32 action[] = {
-		INTEL_GUC_ACTION_EXIT_S_STATE,
-		GUC_POWER_D0,
-	};
-
-	/*
-	 * If GuC communication is enabled but submission is not supported,
-	 * we do not need to resume the GuC but we do need to enable the
-	 * GuC communication on resume (above).
-	 */
-	if (!intel_guc_submission_is_used(guc) || !intel_guc_is_ready(guc))
-		return 0;
-
-	return intel_guc_send(guc, action, ARRAY_SIZE(action));
+	/* XXX: to be implemented with submission interface rework */
+	return 0;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index e84ab67b317d..bc2ba7d0626c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -47,13 +47,6 @@ struct intel_guc {
 	struct i915_vma *stage_desc_pool;
 	void *stage_desc_pool_vaddr;
 
-	struct i915_vma *workqueue;
-	void *workqueue_vaddr;
-	spinlock_t wq_lock;
-
-	struct i915_vma *proc_desc;
-	void *proc_desc_vaddr;
-
 	/* Control params for fw initialization */
 	u32 params[GUC_CTL_MAX_DWORDS];
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 694ee424b4ee..d4f88d2379e9 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -67,58 +67,6 @@ static struct guc_stage_desc *__get_stage_desc(struct intel_guc *guc, u32 id)
 	return &base[id];
 }
 
-static int guc_workqueue_create(struct intel_guc *guc)
-{
-	return intel_guc_allocate_and_map_vma(guc, GUC_WQ_SIZE, &guc->workqueue,
-					      &guc->workqueue_vaddr);
-}
-
-static void guc_workqueue_destroy(struct intel_guc *guc)
-{
-	i915_vma_unpin_and_release(&guc->workqueue, I915_VMA_RELEASE_MAP);
-}
-
-/*
- * Initialise the process descriptor shared with the GuC firmware.
- */
-static int guc_proc_desc_create(struct intel_guc *guc)
-{
-	const u32 size = PAGE_ALIGN(sizeof(struct guc_process_desc));
-
-	return intel_guc_allocate_and_map_vma(guc, size, &guc->proc_desc,
-					      &guc->proc_desc_vaddr);
-}
-
-static void guc_proc_desc_destroy(struct intel_guc *guc)
-{
-	i915_vma_unpin_and_release(&guc->proc_desc, I915_VMA_RELEASE_MAP);
-}
-
-static void guc_proc_desc_init(struct intel_guc *guc)
-{
-	struct guc_process_desc *desc;
-
-	desc = memset(guc->proc_desc_vaddr, 0, sizeof(*desc));
-
-	/*
-	 * XXX: pDoorbell and WQVBaseAddress are pointers in process address
-	 * space for ring3 clients (set them as in mmap_ioctl) or kernel
-	 * space for kernel clients (map on demand instead? May make debug
-	 * easier to have it mapped).
-	 */
-	desc->wq_base_addr = 0;
-	desc->db_base_addr = 0;
-
-	desc->wq_size_bytes = GUC_WQ_SIZE;
-	desc->wq_status = WQ_STATUS_ACTIVE;
-	desc->priority = GUC_CLIENT_PRIORITY_KMD_NORMAL;
-}
-
-static void guc_proc_desc_fini(struct intel_guc *guc)
-{
-	memset(guc->proc_desc_vaddr, 0, sizeof(struct guc_process_desc));
-}
-
 static int guc_stage_desc_pool_create(struct intel_guc *guc)
 {
 	u32 size = PAGE_ALIGN(sizeof(struct guc_stage_desc) *
@@ -154,8 +102,6 @@ static void guc_stage_desc_init(struct intel_guc *guc)
 	desc->stage_id = 0;
 	desc->priority = GUC_CLIENT_PRIORITY_KMD_NORMAL;
 
-	desc->process_desc = intel_guc_ggtt_offset(guc, guc->proc_desc);
-	desc->wq_addr = intel_guc_ggtt_offset(guc, guc->workqueue);
 	desc->wq_size = GUC_WQ_SIZE;
 }
 
@@ -167,62 +113,9 @@ static void guc_stage_desc_fini(struct intel_guc *guc)
 	memset(desc, 0, sizeof(*desc));
 }
 
-/* Construct a Work Item and append it to the GuC's Work Queue */
-static void guc_wq_item_append(struct intel_guc *guc,
-			       u32 target_engine, u32 context_desc,
-			       u32 ring_tail, u32 fence_id)
-{
-	/* wqi_len is in DWords, and does not include the one-word header */
-	const size_t wqi_size = sizeof(struct guc_wq_item);
-	const u32 wqi_len = wqi_size / sizeof(u32) - 1;
-	struct guc_process_desc *desc = guc->proc_desc_vaddr;
-	struct guc_wq_item *wqi;
-	u32 wq_off;
-
-	lockdep_assert_held(&guc->wq_lock);
-
-	/* For now workqueue item is 4 DWs; workqueue buffer is 2 pages. So we
-	 * should not have the case where structure wqi is across page, neither
-	 * wrapped to the beginning. This simplifies the implementation below.
-	 *
-	 * XXX: if not the case, we need save data to a temp wqi and copy it to
-	 * workqueue buffer dw by dw.
-	 */
-	BUILD_BUG_ON(wqi_size != 16);
-
-	/* We expect the WQ to be active if we're appending items to it */
-	GEM_BUG_ON(desc->wq_status != WQ_STATUS_ACTIVE);
-
-	/* Free space is guaranteed. */
-	wq_off = READ_ONCE(desc->tail);
-	GEM_BUG_ON(CIRC_SPACE(wq_off, READ_ONCE(desc->head),
-			      GUC_WQ_SIZE) < wqi_size);
-	GEM_BUG_ON(wq_off & (wqi_size - 1));
-
-	wqi = guc->workqueue_vaddr + wq_off;
-
-	/* Now fill in the 4-word work queue item */
-	wqi->header = WQ_TYPE_INORDER |
-		      (wqi_len << WQ_LEN_SHIFT) |
-		      (target_engine << WQ_TARGET_SHIFT) |
-		      WQ_NO_WCFLUSH_WAIT;
-	wqi->context_desc = context_desc;
-	wqi->submit_element_info = ring_tail << WQ_RING_TAIL_SHIFT;
-	GEM_BUG_ON(ring_tail > WQ_RING_TAIL_MAX);
-	wqi->fence_id = fence_id;
-
-	/* Make the update visible to GuC */
-	WRITE_ONCE(desc->tail, (wq_off + wqi_size) & (GUC_WQ_SIZE - 1));
-}
-
 static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
 {
-	struct intel_engine_cs *engine = rq->engine;
-	u32 ctx_desc = rq->context->lrc.ccid;
-	u32 ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
-
-	guc_wq_item_append(guc, engine->guc_id, ctx_desc,
-			   ring_tail, rq->fence.seqno);
+	/* Leaving stub as this function will be used in future patches */
 }
 
 /*
@@ -245,16 +138,12 @@ static void guc_submit(struct intel_engine_cs *engine,
 {
 	struct intel_guc *guc = &engine->gt->uc.guc;
 
-	spin_lock(&guc->wq_lock);
-
 	do {
 		struct i915_request *rq = *out++;
 
 		flush_ggtt_writes(rq->ring->vma);
 		guc_add_request(guc, rq);
 	} while (out != end);
-
-	spin_unlock(&guc->wq_lock);
 }
 
 static inline int rq_prio(const struct i915_request *rq)
@@ -389,19 +278,6 @@ static void guc_reset_prepare(struct intel_engine_cs *engine)
 	__tasklet_disable_sync_once(&execlists->tasklet);
 }
 
-static void
-cancel_port_requests(struct intel_engine_execlists * const execlists)
-{
-	struct i915_request * const *port, *rq;
-
-	/* Note we are only using the inflight and not the pending queue */
-
-	for (port = execlists->active; (rq = *port); port++)
-		schedule_out(rq);
-	execlists->active =
-		memset(execlists->inflight, 0, sizeof(execlists->inflight));
-}
-
 static void guc_reset_state(struct intel_context *ce,
 			    struct intel_engine_cs *engine,
 			    u32 head,
@@ -432,8 +308,6 @@ static void guc_reset_rewind(struct intel_engine_cs *engine, bool stalled)
 
 	spin_lock_irqsave(&engine->active.lock, flags);
 
-	cancel_port_requests(execlists);
-
 	/* Push back any incomplete requests for replay after the reset. */
 	rq = execlists_unwind_incomplete_requests(execlists);
 	if (!rq)
@@ -474,9 +348,6 @@ static void guc_reset_cancel(struct intel_engine_cs *engine)
 	 */
 	spin_lock_irqsave(&engine->active.lock, flags);
 
-	/* Cancel the requests on the HW and clear the ELSP tracker. */
-	cancel_port_requests(execlists);
-
 	/* Mark all executing requests as skipped. */
 	list_for_each_entry(rq, &engine->active.requests, sched.link) {
 		i915_request_set_error_once(rq, -EIO);
@@ -519,12 +390,6 @@ static void guc_reset_finish(struct intel_engine_cs *engine)
 		     atomic_read(&execlists->tasklet.count));
 }
 
-/*
- * Everything below here is concerned with setup & teardown, and is
- * therefore not part of the somewhat time-critical batch-submission
- * path of guc_submit() above.
- */
-
 /*
  * Set up the memory resources to be shared with the GuC (via the GGTT)
  * at firmware loading time.
@@ -545,30 +410,12 @@ int intel_guc_submission_init(struct intel_guc *guc)
 	 */
 	GEM_BUG_ON(!guc->stage_desc_pool);
 
-	ret = guc_workqueue_create(guc);
-	if (ret)
-		goto err_pool;
-
-	ret = guc_proc_desc_create(guc);
-	if (ret)
-		goto err_workqueue;
-
-	spin_lock_init(&guc->wq_lock);
-
 	return 0;
-
-err_workqueue:
-	guc_workqueue_destroy(guc);
-err_pool:
-	guc_stage_desc_pool_destroy(guc);
-	return ret;
 }
 
 void intel_guc_submission_fini(struct intel_guc *guc)
 {
 	if (guc->stage_desc_pool) {
-		guc_proc_desc_destroy(guc);
-		guc_workqueue_destroy(guc);
 		guc_stage_desc_pool_destroy(guc);
 	}
 }
@@ -642,20 +489,6 @@ void intel_guc_submission_enable(struct intel_guc *guc)
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 
-	/*
-	 * We're using GuC work items for submitting work through GuC. Since
-	 * we're coalescing multiple requests from a single context into a
-	 * single work item prior to assigning it to execlist_port, we can
-	 * never have more work items than the total number of ports (for all
-	 * engines). The GuC firmware is controlling the HEAD of work queue,
-	 * and it is guaranteed that it will remove the work item from the
-	 * queue before our request is completed.
-	 */
-	BUILD_BUG_ON(ARRAY_SIZE(engine->execlists.inflight) *
-		     sizeof(struct guc_wq_item) *
-		     I915_NUM_ENGINES > GUC_WQ_SIZE);
-
-	guc_proc_desc_init(guc);
 	guc_stage_desc_init(guc);
 
 	/* Take over from manual control of ELSP (execlists) */
@@ -678,7 +511,6 @@ void intel_guc_submission_disable(struct intel_guc *guc)
 	guc_interrupts_release(gt);
 
 	guc_stage_desc_fini(guc);
-	guc_proc_desc_fini(guc);
 }
 
 static bool __guc_submission_selected(struct intel_guc *guc)
-- 
2.29.2

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

[Intel-gfx] [PATCH 2/5] drm/i915/guc: do not dump execlists state with GuC submission

[Daniele Ceraolo Spurio]

GuC owns the execlists state and the context IDs used for submission, so
the status of the ports and the CSB entries are not something we control
or can decode from the i915 side, therefore we can avoid dumping it. A
follow-up patch will also stop setting the csb pointers when using GuC
submission.

GuC dumps all the required events in the GuC logs when verbosity is set
high enough.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 1847d3c2ea99..f62303bf80b8 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1470,7 +1470,9 @@ static void intel_engine_print_registers(struct intel_engine_cs *engine,
 		drm_printf(m, "\tIPEHR: 0x%08x\n", ENGINE_READ(engine, IPEHR));
 	}
 
-	if (HAS_EXECLISTS(dev_priv)) {
+	if (intel_engine_in_guc_submission_mode(engine)) {
+		/* nothing to print yet */
+	} else if (HAS_EXECLISTS(dev_priv)) {
 		struct i915_request * const *port, *rq;
 		const u32 *hws =
 			&engine->status_page.addr[I915_HWS_CSB_BUF0_INDEX];
-- 
2.29.2

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

[Intel-gfx] [PATCH 3/5] drm/i915/guc: init engine directly in GuC submission mode

[Daniele Ceraolo Spurio]

Instead of starting the engine in execlists submission mode and then
switching to GuC, start directly in GuC submission mode. The initial
setup functions have been copied over from the execlists code
and simplified by removing the execlists submission-specific parts.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: John Harrison <john.c.harrison@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   5 +-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 249 +++++++++++++++++-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.h |   1 +
 3 files changed, 244 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index f62303bf80b8..6b4483b72c3f 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -40,6 +40,7 @@
 #include "intel_lrc_reg.h"
 #include "intel_reset.h"
 #include "intel_ring.h"
+#include "uc/intel_guc_submission.h"
 
 /* Haswell does have the CXT_SIZE register however it does not appear to be
  * valid. Now, docs explain in dwords what is in the context object. The full
@@ -907,7 +908,9 @@ int intel_engines_init(struct intel_gt *gt)
 	enum intel_engine_id id;
 	int err;
 
-	if (HAS_EXECLISTS(gt->i915))
+	if (intel_uc_uses_guc_submission(&gt->uc))
+		setup = intel_guc_submission_setup;
+	else if (HAS_EXECLISTS(gt->i915))
 		setup = intel_execlists_submission_setup;
 	else
 		setup = intel_ring_submission_setup;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index d4f88d2379e9..2faaa14e6e42 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -6,12 +6,15 @@
 #include <linux/circ_buf.h>
 
 #include "gem/i915_gem_context.h"
+#include "gt/gen8_engine_cs.h"
+#include "gt/intel_breadcrumbs.h"
 #include "gt/intel_context.h"
 #include "gt/intel_engine_pm.h"
 #include "gt/intel_execlists_submission.h" /* XXX */
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_pm.h"
 #include "gt/intel_lrc.h"
+#include "gt/intel_mocs.h"
 #include "gt/intel_ring.h"
 
 #include "intel_guc_submission.h"
@@ -55,6 +58,8 @@
  *
  */
 
+#define GUC_REQUEST_SIZE 64 /* bytes */
+
 static inline struct i915_priolist *to_priolist(struct rb_node *rb)
 {
 	return rb_entry(rb, struct i915_priolist, node);
@@ -446,6 +451,153 @@ static void guc_interrupts_release(struct intel_gt *gt)
 	intel_uncore_rmw(uncore, GEN11_VCS_VECS_INTR_ENABLE, 0, dmask);
 }
 
+static int guc_context_alloc(struct intel_context *ce)
+{
+	return lrc_alloc(ce, ce->engine);
+}
+
+static int guc_context_pre_pin(struct intel_context *ce,
+				     struct i915_gem_ww_ctx *ww,
+				     void **vaddr)
+{
+	return lrc_pre_pin(ce, ce->engine, ww, vaddr);
+}
+
+static int guc_context_pin(struct intel_context *ce, void *vaddr)
+{
+	return lrc_pin(ce, ce->engine, vaddr);
+}
+
+static const struct intel_context_ops guc_context_ops = {
+	.alloc = guc_context_alloc,
+
+	.pre_pin = guc_context_pre_pin,
+	.pin = guc_context_pin,
+	.unpin = lrc_unpin,
+	.post_unpin = lrc_post_unpin,
+
+	.enter = intel_context_enter_engine,
+	.exit = intel_context_exit_engine,
+
+	.reset = lrc_reset,
+	.destroy = lrc_destroy,
+};
+
+static int guc_request_alloc(struct i915_request *request)
+{
+	int ret;
+
+	GEM_BUG_ON(!intel_context_is_pinned(request->context));
+
+	/*
+	 * Flush enough space to reduce the likelihood of waiting after
+	 * we start building the request - in which case we will just
+	 * have to repeat work.
+	 */
+	request->reserved_space += GUC_REQUEST_SIZE;
+
+	/*
+	 * Note that after this point, we have committed to using
+	 * this request as it is being used to both track the
+	 * state of engine initialisation and liveness of the
+	 * golden renderstate above. Think twice before you try
+	 * to cancel/unwind this request now.
+	 */
+
+	/* Unconditionally invalidate GPU caches and TLBs. */
+	ret = request->engine->emit_flush(request, EMIT_INVALIDATE);
+	if (ret)
+		return ret;
+
+	request->reserved_space -= GUC_REQUEST_SIZE;
+	return 0;
+}
+
+static void sanitize_hwsp(struct intel_engine_cs *engine)
+{
+	struct intel_timeline *tl;
+
+	list_for_each_entry(tl, &engine->status_page.timelines, engine_link)
+		intel_timeline_reset_seqno(tl);
+}
+
+static void guc_sanitize(struct intel_engine_cs *engine)
+{
+	/*
+	 * Poison residual state on resume, in case the suspend didn't!
+	 *
+	 * We have to assume that across suspend/resume (or other loss
+	 * of control) that the contents of our pinned buffers has been
+	 * lost, replaced by garbage. Since this doesn't always happen,
+	 * let's poison such state so that we more quickly spot when
+	 * we falsely assume it has been preserved.
+	 */
+	if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
+		memset(engine->status_page.addr, POISON_INUSE, PAGE_SIZE);
+
+	/*
+	 * The kernel_context HWSP is stored in the status_page. As above,
+	 * that may be lost on resume/initialisation, and so we need to
+	 * reset the value in the HWSP.
+	 */
+	sanitize_hwsp(engine);
+
+	/* And scrub the dirty cachelines for the HWSP */
+	clflush_cache_range(engine->status_page.addr, PAGE_SIZE);
+}
+
+static void setup_hwsp(struct intel_engine_cs *engine)
+{
+	intel_engine_set_hwsp_writemask(engine, ~0u); /* HWSTAM */
+
+	ENGINE_WRITE_FW(engine,
+			RING_HWS_PGA,
+			i915_ggtt_offset(engine->status_page.vma));
+}
+
+static void start_engine(struct intel_engine_cs *engine)
+{
+	ENGINE_WRITE_FW(engine,
+			RING_MODE_GEN7,
+			_MASKED_BIT_ENABLE(GEN11_GFX_DISABLE_LEGACY_MODE));
+
+	ENGINE_WRITE_FW(engine, RING_MI_MODE, _MASKED_BIT_DISABLE(STOP_RING));
+	ENGINE_POSTING_READ(engine, RING_MI_MODE);
+}
+
+static bool unexpected_starting_state(struct intel_engine_cs *engine)
+{
+	bool unexpected = false;
+
+	if (ENGINE_READ_FW(engine, RING_MI_MODE) & STOP_RING) {
+		drm_dbg(&engine->i915->drm,
+			"STOP_RING still set in RING_MI_MODE\n");
+		unexpected = true;
+	}
+
+	return unexpected;
+}
+
+static int guc_resume(struct intel_engine_cs *engine)
+{
+	assert_forcewakes_active(engine->uncore, FORCEWAKE_ALL);
+
+	intel_mocs_init_engine(engine);
+
+	intel_breadcrumbs_reset(engine->breadcrumbs);
+
+	if (GEM_SHOW_DEBUG() && unexpected_starting_state(engine)) {
+		struct drm_printer p = drm_debug_printer(__func__);
+
+		intel_engine_dump(engine, &p, NULL);
+	}
+
+	setup_hwsp(engine);
+	start_engine(engine);
+
+	return 0;
+}
+
 static void guc_set_default_submission(struct intel_engine_cs *engine)
 {
 	/*
@@ -483,23 +635,100 @@ static void guc_set_default_submission(struct intel_engine_cs *engine)
 	GEM_BUG_ON(engine->irq_enable || engine->irq_disable);
 }
 
-void intel_guc_submission_enable(struct intel_guc *guc)
+static void guc_release(struct intel_engine_cs *engine)
 {
-	struct intel_gt *gt = guc_to_gt(guc);
-	struct intel_engine_cs *engine;
-	enum intel_engine_id id;
+	engine->sanitize = NULL; /* no longer in control, nothing to sanitize */
 
-	guc_stage_desc_init(guc);
+	tasklet_kill(&engine->execlists.tasklet);
 
-	/* Take over from manual control of ELSP (execlists) */
-	guc_interrupts_capture(gt);
+	intel_engine_cleanup_common(engine);
+	lrc_fini_wa_ctx(engine);
+}
+
+static void guc_default_vfuncs(struct intel_engine_cs *engine)
+{
+	/* Default vfuncs which can be overriden by each engine. */
+
+	engine->resume = guc_resume;
+
+	engine->cops = &guc_context_ops;
+	engine->request_alloc = guc_request_alloc;
+
+	engine->emit_flush = gen8_emit_flush_xcs;
+	engine->emit_init_breadcrumb = gen8_emit_init_breadcrumb;
+	engine->emit_fini_breadcrumb = gen8_emit_fini_breadcrumb_xcs;
+	if (INTEL_GEN(engine->i915) >= 12) {
+		engine->emit_fini_breadcrumb = gen12_emit_fini_breadcrumb_xcs;
+		engine->emit_flush = gen12_emit_flush_xcs;
+	}
+	engine->set_default_submission = guc_set_default_submission;
+}
 
-	for_each_engine(engine, gt, id) {
-		engine->set_default_submission = guc_set_default_submission;
-		engine->set_default_submission(engine);
+static void rcs_submission_override(struct intel_engine_cs *engine)
+{
+	switch (INTEL_GEN(engine->i915)) {
+	case 12:
+		engine->emit_flush = gen12_emit_flush_rcs;
+		engine->emit_fini_breadcrumb = gen12_emit_fini_breadcrumb_rcs;
+		break;
+	case 11:
+		engine->emit_flush = gen11_emit_flush_rcs;
+		engine->emit_fini_breadcrumb = gen11_emit_fini_breadcrumb_rcs;
+		break;
+	default:
+		engine->emit_flush = gen8_emit_flush_rcs;
+		engine->emit_fini_breadcrumb = gen8_emit_fini_breadcrumb_rcs;
+		break;
 	}
 }
 
+static inline void guc_default_irqs(struct intel_engine_cs *engine)
+{
+	engine->irq_keep_mask = GT_RENDER_USER_INTERRUPT;
+}
+
+int intel_guc_submission_setup(struct intel_engine_cs *engine)
+{
+	struct drm_i915_private *i915 = engine->i915;
+
+	/*
+	 * The setup relies on several assumptions (e.g. irqs always enabled)
+	 * that are only valid on gen11+
+	 */
+	GEM_BUG_ON(INTEL_GEN(i915) < 11);
+
+	tasklet_init(&engine->execlists.tasklet,
+		     guc_submission_tasklet, (unsigned long)engine);
+
+	guc_default_vfuncs(engine);
+	guc_default_irqs(engine);
+
+	if (engine->class == RENDER_CLASS)
+		rcs_submission_override(engine);
+
+	if (lrc_init_wa_ctx(engine))
+		/*
+		 * We continue even if we fail to initialize WA batch
+		 * because we only expect rare glitches but nothing
+		 * critical to prevent us from using GPU
+		 */
+		drm_err(&i915->drm, "WA batch buffer initialization failed\n");
+
+	/* Finally, take ownership and responsibility for cleanup! */
+	engine->sanitize = guc_sanitize;
+	engine->release = guc_release;
+
+	return 0;
+}
+
+void intel_guc_submission_enable(struct intel_guc *guc)
+{
+	guc_stage_desc_init(guc);
+
+	/* Take over from manual control of ELSP (execlists) */
+	guc_interrupts_capture(guc_to_gt(guc));
+}
+
 void intel_guc_submission_disable(struct intel_guc *guc)
 {
 	struct intel_gt *gt = guc_to_gt(guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
index 4cf9d3e50263..5f7b9e6347d0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
@@ -19,6 +19,7 @@ void intel_guc_submission_disable(struct intel_guc *guc);
 void intel_guc_submission_fini(struct intel_guc *guc);
 int intel_guc_preempt_work_create(struct intel_guc *guc);
 void intel_guc_preempt_work_destroy(struct intel_guc *guc);
+int intel_guc_submission_setup(struct intel_engine_cs *engine);
 bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine);
 
 static inline bool intel_guc_submission_is_supported(struct intel_guc *guc)
-- 
2.29.2

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

[Intel-gfx] [PATCH 4/5] drm/i915/guc: stop calling execlists_set_default_submission

[Daniele Ceraolo Spurio]

Initialize all required entries from guc_set_default_submission, instead
of calling the execlists function. The previously inherited setup has
been copied over from the execlist code and simplified by removing the
execlists submission-specific parts.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: John Harrison <john.c.harrison@intel.com>
---
 .../drm/i915/gt/intel_execlists_submission.c  |  6 +-
 .../drm/i915/gt/intel_execlists_submission.h  |  2 -
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 63 +++++++++++++------
 3 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index a5b442683c18..3b2821516b93 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -3082,7 +3082,7 @@ static void execlists_park(struct intel_engine_cs *engine)
 	cancel_timer(&engine->execlists.preempt);
 }
 
-void intel_execlists_set_default_submission(struct intel_engine_cs *engine)
+static void execlists_set_default_submission(struct intel_engine_cs *engine)
 {
 	engine->submit_request = execlists_submit_request;
 	engine->schedule = i915_schedule;
@@ -3150,7 +3150,7 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine)
 		engine->emit_fini_breadcrumb = gen12_emit_fini_breadcrumb_xcs;
 		engine->emit_flush = gen12_emit_flush_xcs;
 	}
-	engine->set_default_submission = intel_execlists_set_default_submission;
+	engine->set_default_submission = execlists_set_default_submission;
 
 	if (INTEL_GEN(engine->i915) < 11) {
 		engine->irq_enable = gen8_logical_ring_enable_irq;
@@ -3915,7 +3915,7 @@ bool
 intel_engine_in_execlists_submission_mode(const struct intel_engine_cs *engine)
 {
 	return engine->set_default_submission ==
-	       intel_execlists_set_default_submission;
+	       execlists_set_default_submission;
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.h b/drivers/gpu/drm/i915/gt/intel_execlists_submission.h
index 0c675bbff351..a8fd7adefd82 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.h
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.h
@@ -22,8 +22,6 @@ enum {
 
 int intel_execlists_submission_setup(struct intel_engine_cs *engine);
 
-void intel_execlists_set_default_submission(struct intel_engine_cs *engine);
-
 void intel_execlists_show_requests(struct intel_engine_cs *engine,
 				   struct drm_printer *m,
 				   void (*show_request)(struct drm_printer *m,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 2faaa14e6e42..3993f1d75e87 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -10,7 +10,6 @@
 #include "gt/intel_breadcrumbs.h"
 #include "gt/intel_context.h"
 #include "gt/intel_engine_pm.h"
-#include "gt/intel_execlists_submission.h" /* XXX */
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_pm.h"
 #include "gt/intel_lrc.h"
@@ -513,6 +512,34 @@ static int guc_request_alloc(struct i915_request *request)
 	return 0;
 }
 
+static inline void queue_request(struct intel_engine_cs *engine,
+				 struct i915_request *rq,
+				 int prio)
+{
+	GEM_BUG_ON(!list_empty(&rq->sched.link));
+	list_add_tail(&rq->sched.link,
+		      i915_sched_lookup_priolist(engine, prio));
+	set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
+}
+
+static void guc_submit_request(struct i915_request *rq)
+{
+	struct intel_engine_cs *engine = rq->engine;
+	unsigned long flags;
+
+	/* Will be called from irq-context when using foreign fences. */
+	spin_lock_irqsave(&engine->active.lock, flags);
+
+	queue_request(engine, rq, rq_prio(rq));
+
+	GEM_BUG_ON(RB_EMPTY_ROOT(&engine->execlists.queue.rb_root));
+	GEM_BUG_ON(list_empty(&rq->sched.link));
+
+	tasklet_hi_schedule(&engine->execlists.tasklet);
+
+	spin_unlock_irqrestore(&engine->active.lock, flags);
+}
+
 static void sanitize_hwsp(struct intel_engine_cs *engine)
 {
 	struct intel_timeline *tl;
@@ -600,31 +627,31 @@ static int guc_resume(struct intel_engine_cs *engine)
 
 static void guc_set_default_submission(struct intel_engine_cs *engine)
 {
-	/*
-	 * We inherit a bunch of functions from execlists that we'd like
-	 * to keep using:
-	 *
-	 *    engine->submit_request = execlists_submit_request;
-	 *    engine->cancel_requests = execlists_cancel_requests;
-	 *    engine->schedule = execlists_schedule;
-	 *
-	 * But we need to override the actual submission backend in order
-	 * to talk to the GuC.
-	 */
-	intel_execlists_set_default_submission(engine);
-
+	engine->submit_request = guc_submit_request;
+	engine->schedule = i915_schedule;
 	engine->execlists.tasklet.func = guc_submission_tasklet;
 
-	/* do not use execlists park/unpark */
-	engine->park = engine->unpark = NULL;
-
 	engine->reset.prepare = guc_reset_prepare;
 	engine->reset.rewind = guc_reset_rewind;
 	engine->reset.cancel = guc_reset_cancel;
 	engine->reset.finish = guc_reset_finish;
 
-	engine->flags &= ~I915_ENGINE_SUPPORTS_STATS;
 	engine->flags |= I915_ENGINE_NEEDS_BREADCRUMB_TASKLET;
+	engine->flags |= I915_ENGINE_HAS_PREEMPTION;
+
+	/*
+	 * TODO: GuC supports timeslicing and semaphores as well, but they're
+	 * handled by the firmware so some minor tweaks are required before
+	 * enabling.
+	 *
+	 * engine->flags |= I915_ENGINE_HAS_TIMESLICES;
+	 * engine->flags |= I915_ENGINE_HAS_SEMAPHORES;
+	 */
+
+	if (INTEL_GEN(engine->i915) >= 12)
+		engine->flags |= I915_ENGINE_HAS_RELATIVE_MMIO;
+
+	engine->emit_bb_start = gen8_emit_bb_start;
 
 	/*
 	 * For the breadcrumb irq to work we need the interrupts to stay
-- 
2.29.2

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

[Intel-gfx] [PATCH 5/5] drm/i915/guc: enable only the user interrupt when using GuC submission

[Daniele Ceraolo Spurio]

In GuC submission mode the CS is owned by the GuC FW, so all CS status
interrupts are handled by it. We only need the user interrupt as that
signals request completion.

Since we're now starting the engines directly in GuC submission mode
when selected, we can stop switching back and forth between the
execlists and the GuC programming and select directly the correct
interrupt mask.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: John Harrison <john.c.harrison@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_irq.c        | 18 ++++++-----
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 31 -------------------
 2 files changed, 11 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
index 9830342aa6f4..7b2b8cb2d2be 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -237,14 +237,18 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
 
 void gen11_gt_irq_postinstall(struct intel_gt *gt)
 {
-	const u32 irqs =
-		GT_CS_MASTER_ERROR_INTERRUPT |
-		GT_RENDER_USER_INTERRUPT |
-		GT_CONTEXT_SWITCH_INTERRUPT |
-		GT_WAIT_SEMAPHORE_INTERRUPT;
 	struct intel_uncore *uncore = gt->uncore;
-	const u32 dmask = irqs << 16 | irqs;
-	const u32 smask = irqs << 16;
+	u32 irqs = GT_RENDER_USER_INTERRUPT;
+	u32 dmask;
+	u32 smask;
+
+	if (!intel_uc_wants_guc_submission(&gt->uc))
+		irqs |= GT_CS_MASTER_ERROR_INTERRUPT |
+			GT_CONTEXT_SWITCH_INTERRUPT |
+			GT_WAIT_SEMAPHORE_INTERRUPT;
+
+	dmask = irqs << 16 | irqs;
+	smask = irqs << 16;
 
 	BUILD_BUG_ON(irqs & 0xffff0000);
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 3993f1d75e87..84ec118f270d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -424,32 +424,6 @@ void intel_guc_submission_fini(struct intel_guc *guc)
 	}
 }
 
-static void guc_interrupts_capture(struct intel_gt *gt)
-{
-	struct intel_uncore *uncore = gt->uncore;
-	u32 irqs = GT_CONTEXT_SWITCH_INTERRUPT;
-	u32 dmask = irqs << 16 | irqs;
-
-	GEM_BUG_ON(INTEL_GEN(gt->i915) < 11);
-
-	/* Don't handle the ctx switch interrupt in GuC submission mode */
-	intel_uncore_rmw(uncore, GEN11_RENDER_COPY_INTR_ENABLE, dmask, 0);
-	intel_uncore_rmw(uncore, GEN11_VCS_VECS_INTR_ENABLE, dmask, 0);
-}
-
-static void guc_interrupts_release(struct intel_gt *gt)
-{
-	struct intel_uncore *uncore = gt->uncore;
-	u32 irqs = GT_CONTEXT_SWITCH_INTERRUPT;
-	u32 dmask = irqs << 16 | irqs;
-
-	GEM_BUG_ON(INTEL_GEN(gt->i915) < 11);
-
-	/* Handle ctx switch interrupts again */
-	intel_uncore_rmw(uncore, GEN11_RENDER_COPY_INTR_ENABLE, 0, dmask);
-	intel_uncore_rmw(uncore, GEN11_VCS_VECS_INTR_ENABLE, 0, dmask);
-}
-
 static int guc_context_alloc(struct intel_context *ce)
 {
 	return lrc_alloc(ce, ce->engine);
@@ -751,9 +725,6 @@ int intel_guc_submission_setup(struct intel_engine_cs *engine)
 void intel_guc_submission_enable(struct intel_guc *guc)
 {
 	guc_stage_desc_init(guc);
-
-	/* Take over from manual control of ELSP (execlists) */
-	guc_interrupts_capture(guc_to_gt(guc));
 }
 
 void intel_guc_submission_disable(struct intel_guc *guc)
@@ -764,8 +735,6 @@ void intel_guc_submission_disable(struct intel_guc *guc)
 
 	/* Note: By the time we're here, GuC may have already been reset */
 
-	guc_interrupts_release(gt);
-
 	guc_stage_desc_fini(guc);
 }
 
-- 
2.29.2

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

Re: [Intel-gfx] [PATCH 3/5] drm/i915/guc: init engine directly in GuC submission mode

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:45)
> Instead of starting the engine in execlists submission mode and then
> switching to GuC, start directly in GuC submission mode. The initial
> setup functions have been copied over from the execlists code
> and simplified by removing the execlists submission-specific parts.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: John Harrison <john.c.harrison@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   5 +-
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 249 +++++++++++++++++-
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.h |   1 +
>  3 files changed, 244 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index f62303bf80b8..6b4483b72c3f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -40,6 +40,7 @@
>  #include "intel_lrc_reg.h"
>  #include "intel_reset.h"
>  #include "intel_ring.h"
> +#include "uc/intel_guc_submission.h"
>  
>  /* Haswell does have the CXT_SIZE register however it does not appear to be
>   * valid. Now, docs explain in dwords what is in the context object. The full
> @@ -907,7 +908,9 @@ int intel_engines_init(struct intel_gt *gt)
>         enum intel_engine_id id;
>         int err;
>  
> -       if (HAS_EXECLISTS(gt->i915))
> +       if (intel_uc_uses_guc_submission(&gt->uc))

When do we determine intel_uc_uses_guc_submission?

I'm a bit nervous since I've lost track of needs/wants/uses. Is there a
telltale we can place here to assert that we've processed the relevant
init functions (also acting as an aide memoire)?

> +               setup = intel_guc_submission_setup;
> +       else if (HAS_EXECLISTS(gt->i915))
>                 setup = intel_execlists_submission_setup;
>         else
>                 setup = intel_ring_submission_setup;

> +static bool unexpected_starting_state(struct intel_engine_cs *engine)
> +{
> +       bool unexpected = false;
> +
> +       if (ENGINE_READ_FW(engine, RING_MI_MODE) & STOP_RING) {
> +               drm_dbg(&engine->i915->drm,
> +                       "STOP_RING still set in RING_MI_MODE\n");
> +               unexpected = true;
> +       }

Do we care? Is this something we can assume the guc will handle?
(It originated in debugging reset failures.)

> +       return unexpected;
> +}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/5] drm/i915/guc: enable only the user interrupt when using GuC submission

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:47)
> In GuC submission mode the CS is owned by the GuC FW, so all CS status
> interrupts are handled by it. We only need the user interrupt as that
> signals request completion.
> 
> Since we're now starting the engines directly in GuC submission mode
> when selected, we can stop switching back and forth between the
> execlists and the GuC programming and select directly the correct
> interrupt mask.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: John Harrison <john.c.harrison@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_irq.c        | 18 ++++++-----
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 31 -------------------
>  2 files changed, 11 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> index 9830342aa6f4..7b2b8cb2d2be 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> @@ -237,14 +237,18 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
>  
>  void gen11_gt_irq_postinstall(struct intel_gt *gt)
>  {
> -       const u32 irqs =
> -               GT_CS_MASTER_ERROR_INTERRUPT |
> -               GT_RENDER_USER_INTERRUPT |
> -               GT_CONTEXT_SWITCH_INTERRUPT |
> -               GT_WAIT_SEMAPHORE_INTERRUPT;
>         struct intel_uncore *uncore = gt->uncore;
> -       const u32 dmask = irqs << 16 | irqs;
> -       const u32 smask = irqs << 16;
> +       u32 irqs = GT_RENDER_USER_INTERRUPT;
> +       u32 dmask;
> +       u32 smask;
> +
> +       if (!intel_uc_wants_guc_submission(&gt->uc))
> +               irqs |= GT_CS_MASTER_ERROR_INTERRUPT |
> +                       GT_CONTEXT_SWITCH_INTERRUPT |
> +                       GT_WAIT_SEMAPHORE_INTERRUPT;

Hmm, we should stop performing this by default then, and make the
execlists setup request the interrupt vector it desires.

That's certainly a bit more fiddly to untangle the packed iir across
multiple gen. :|
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/5] drm/i915/guc: init engine directly in GuC submission mode

[Daniele Ceraolo Spurio]

On 1/5/2021 3:33 PM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:45)
>> Instead of starting the engine in execlists submission mode and then
>> switching to GuC, start directly in GuC submission mode. The initial
>> setup functions have been copied over from the execlists code
>> and simplified by removing the execlists submission-specific parts.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: John Harrison <john.c.harrison@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   5 +-
>>   .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 249 +++++++++++++++++-
>>   .../gpu/drm/i915/gt/uc/intel_guc_submission.h |   1 +
>>   3 files changed, 244 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> index f62303bf80b8..6b4483b72c3f 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> @@ -40,6 +40,7 @@
>>   #include "intel_lrc_reg.h"
>>   #include "intel_reset.h"
>>   #include "intel_ring.h"
>> +#include "uc/intel_guc_submission.h"
>>   
>>   /* Haswell does have the CXT_SIZE register however it does not appear to be
>>    * valid. Now, docs explain in dwords what is in the context object. The full
>> @@ -907,7 +908,9 @@ int intel_engines_init(struct intel_gt *gt)
>>          enum intel_engine_id id;
>>          int err;
>>   
>> -       if (HAS_EXECLISTS(gt->i915))
>> +       if (intel_uc_uses_guc_submission(&gt->uc))
> When do we determine intel_uc_uses_guc_submission?

at firmware fetch time.

>
> I'm a bit nervous since I've lost track of needs/wants/uses. Is there a
> telltale we can place here to assert that we've processed the relevant
> init functions (also acting as an aide memoire)?

There is already a GEM_BUG_ON for this inside the function, it'll 
trigger if we call it too early.
For the submission side of things, there isn't much difference at the 
moment between "wants" and "uses" since we do wedge if GuC submission is 
selected and the FW is not found. I still prefer to use "uses" where 
possible in case we want to change this in the future.

>
>> +               setup = intel_guc_submission_setup;
>> +       else if (HAS_EXECLISTS(gt->i915))
>>                  setup = intel_execlists_submission_setup;
>>          else
>>                  setup = intel_ring_submission_setup;
>> +static bool unexpected_starting_state(struct intel_engine_cs *engine)
>> +{
>> +       bool unexpected = false;
>> +
>> +       if (ENGINE_READ_FW(engine, RING_MI_MODE) & STOP_RING) {
>> +               drm_dbg(&engine->i915->drm,
>> +                       "STOP_RING still set in RING_MI_MODE\n");
>> +               unexpected = true;
>> +       }
> Do we care? Is this something we can assume the guc will handle?
> (It originated in debugging reset failures.)

GuC handles it post engine reset, but not on init and resume. If you 
think this only makes sense for reset debug then I'll get rid of it.

Thanks,
Daniele

>
>> +       return unexpected;
>> +}

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

Re: [Intel-gfx] [PATCH 5/5] drm/i915/guc: enable only the user interrupt when using GuC submission

[Daniele Ceraolo Spurio]

On 1/5/2021 3:38 PM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:47)
>> In GuC submission mode the CS is owned by the GuC FW, so all CS status
>> interrupts are handled by it. We only need the user interrupt as that
>> signals request completion.
>>
>> Since we're now starting the engines directly in GuC submission mode
>> when selected, we can stop switching back and forth between the
>> execlists and the GuC programming and select directly the correct
>> interrupt mask.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: John Harrison <john.c.harrison@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_gt_irq.c        | 18 ++++++-----
>>   .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 31 -------------------
>>   2 files changed, 11 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>> index 9830342aa6f4..7b2b8cb2d2be 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>> @@ -237,14 +237,18 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
>>   
>>   void gen11_gt_irq_postinstall(struct intel_gt *gt)
>>   {
>> -       const u32 irqs =
>> -               GT_CS_MASTER_ERROR_INTERRUPT |
>> -               GT_RENDER_USER_INTERRUPT |
>> -               GT_CONTEXT_SWITCH_INTERRUPT |
>> -               GT_WAIT_SEMAPHORE_INTERRUPT;
>>          struct intel_uncore *uncore = gt->uncore;
>> -       const u32 dmask = irqs << 16 | irqs;
>> -       const u32 smask = irqs << 16;
>> +       u32 irqs = GT_RENDER_USER_INTERRUPT;
>> +       u32 dmask;
>> +       u32 smask;
>> +
>> +       if (!intel_uc_wants_guc_submission(&gt->uc))
>> +               irqs |= GT_CS_MASTER_ERROR_INTERRUPT |
>> +                       GT_CONTEXT_SWITCH_INTERRUPT |
>> +                       GT_WAIT_SEMAPHORE_INTERRUPT;
> Hmm, we should stop performing this by default then, and make the
> execlists setup request the interrupt vector it desires.
>
> That's certainly a bit more fiddly to untangle the packed iir across
> multiple gen. :|

I had considered that, but this is a gt-level setup while the execlists 
submission code is currently all engine-based, so I couldn't find a good 
place to move this to and I didn't want to add a new function just for 
it. Any preference?

Daniele

> -Chris

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

Re: [Intel-gfx] [PATCH 3/5] drm/i915/guc: init engine directly in GuC submission mode

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2021-01-05 23:51:43)
> 
> 
> On 1/5/2021 3:33 PM, Chris Wilson wrote:
> > Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:45)
> >> Instead of starting the engine in execlists submission mode and then
> >> switching to GuC, start directly in GuC submission mode. The initial
> >> setup functions have been copied over from the execlists code
> >> and simplified by removing the execlists submission-specific parts.
> >>
> >> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >> Cc: Matthew Brost <matthew.brost@intel.com>
> >> Cc: John Harrison <john.c.harrison@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   5 +-
> >>   .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 249 +++++++++++++++++-
> >>   .../gpu/drm/i915/gt/uc/intel_guc_submission.h |   1 +
> >>   3 files changed, 244 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >> index f62303bf80b8..6b4483b72c3f 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >> @@ -40,6 +40,7 @@
> >>   #include "intel_lrc_reg.h"
> >>   #include "intel_reset.h"
> >>   #include "intel_ring.h"
> >> +#include "uc/intel_guc_submission.h"
> >>   
> >>   /* Haswell does have the CXT_SIZE register however it does not appear to be
> >>    * valid. Now, docs explain in dwords what is in the context object. The full
> >> @@ -907,7 +908,9 @@ int intel_engines_init(struct intel_gt *gt)
> >>          enum intel_engine_id id;
> >>          int err;
> >>   
> >> -       if (HAS_EXECLISTS(gt->i915))
> >> +       if (intel_uc_uses_guc_submission(&gt->uc))
> > When do we determine intel_uc_uses_guc_submission?
> 
> at firmware fetch time.
> 
> >
> > I'm a bit nervous since I've lost track of needs/wants/uses. Is there a
> > telltale we can place here to assert that we've processed the relevant
> > init functions (also acting as an aide memoire)?
> 
> There is already a GEM_BUG_ON for this inside the function, it'll 
> trigger if we call it too early.
> For the submission side of things, there isn't much difference at the 
> moment between "wants" and "uses" since we do wedge if GuC submission is 
> selected and the FW is not found. I still prefer to use "uses" where 
> possible in case we want to change this in the future.

Ok. If there's a bug on to catch us reordering init incorrectly, that's
all I'm concerned about.

> >> +               setup = intel_guc_submission_setup;
> >> +       else if (HAS_EXECLISTS(gt->i915))
> >>                  setup = intel_execlists_submission_setup;
> >>          else
> >>                  setup = intel_ring_submission_setup;
> >> +static bool unexpected_starting_state(struct intel_engine_cs *engine)
> >> +{
> >> +       bool unexpected = false;
> >> +
> >> +       if (ENGINE_READ_FW(engine, RING_MI_MODE) & STOP_RING) {
> >> +               drm_dbg(&engine->i915->drm,
> >> +                       "STOP_RING still set in RING_MI_MODE\n");
> >> +               unexpected = true;
> >> +       }
> > Do we care? Is this something we can assume the guc will handle?
> > (It originated in debugging reset failures.)
> 
> GuC handles it post engine reset, but not on init and resume. If you 
> think this only makes sense for reset debug then I'll get rid of it.

Yes. I think this can be left as execlists debug code.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/5] drm/i915/guc: enable only the user interrupt when using GuC submission

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2021-01-05 23:56:52)
> 
> 
> On 1/5/2021 3:38 PM, Chris Wilson wrote:
> > Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:47)
> >> In GuC submission mode the CS is owned by the GuC FW, so all CS status
> >> interrupts are handled by it. We only need the user interrupt as that
> >> signals request completion.
> >>
> >> Since we're now starting the engines directly in GuC submission mode
> >> when selected, we can stop switching back and forth between the
> >> execlists and the GuC programming and select directly the correct
> >> interrupt mask.
> >>
> >> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >> Cc: Matthew Brost <matthew.brost@intel.com>
> >> Cc: John Harrison <john.c.harrison@intel.com>
> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/gt/intel_gt_irq.c        | 18 ++++++-----
> >>   .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 31 -------------------
> >>   2 files changed, 11 insertions(+), 38 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> >> index 9830342aa6f4..7b2b8cb2d2be 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> >> @@ -237,14 +237,18 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
> >>   
> >>   void gen11_gt_irq_postinstall(struct intel_gt *gt)
> >>   {
> >> -       const u32 irqs =
> >> -               GT_CS_MASTER_ERROR_INTERRUPT |
> >> -               GT_RENDER_USER_INTERRUPT |
> >> -               GT_CONTEXT_SWITCH_INTERRUPT |
> >> -               GT_WAIT_SEMAPHORE_INTERRUPT;
> >>          struct intel_uncore *uncore = gt->uncore;
> >> -       const u32 dmask = irqs << 16 | irqs;
> >> -       const u32 smask = irqs << 16;
> >> +       u32 irqs = GT_RENDER_USER_INTERRUPT;
> >> +       u32 dmask;
> >> +       u32 smask;
> >> +
> >> +       if (!intel_uc_wants_guc_submission(&gt->uc))
> >> +               irqs |= GT_CS_MASTER_ERROR_INTERRUPT |
> >> +                       GT_CONTEXT_SWITCH_INTERRUPT |
> >> +                       GT_WAIT_SEMAPHORE_INTERRUPT;
> > Hmm, we should stop performing this by default then, and make the
> > execlists setup request the interrupt vector it desires.
> >
> > That's certainly a bit more fiddly to untangle the packed iir across
> > multiple gen. :|
> 
> I had considered that, but this is a gt-level setup while the execlists 
> submission code is currently all engine-based, so I couldn't find a good 
> place to move this to and I didn't want to add a new function just for 
> it. Any preference?

I think we should definitely strive to avoid having too many backend-
conditional paths in the common routines, and we can certainly adjust
the interrupts as we enable each engine (it certainly would help when
debugging by removing engines by removing spurious iir).

I guess we may end up with something like
gen8_gt_unmask_engine_irq(struct intel_gt *gt,
			  struct intel_engine_cs *engine,
			  u16 iir)
{
	if (INTEL_GEN(gt->i915) >= 11)
		__gen11...
	else
		__gen8...
}
[Not sure if gen8_gt or intel_gt; I chose gen8_gt for the iir definition]

Then that would fit reasonably into logical_ring_default_irqs().
Hindsight says that if we had done that earlier, we could have avoided
maintaining the same engine-iir in two different sites. So maybe worth
a bit of effort.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Split GuC submission from execlists submission

[Patchwork]

== Series Details ==

Series: Split GuC submission from execlists submission
URL   : https://patchwork.freedesktop.org/series/85528/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
b91fa7500d8f drm/i915/guc: Delete GuC code unused in future patches
45b23606fe0e drm/i915/guc: do not dump execlists state with GuC submission
19efc6761a69 drm/i915/guc: init engine directly in GuC submission mode
-:77: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#77: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:460:
+static int guc_context_pre_pin(struct intel_context *ce,
+				     struct i915_gem_ww_ctx *ww,

-:244: WARNING:TYPO_SPELLING: 'overriden' may be misspelled - perhaps 'overridden'?
#244: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:650:
+	/* Default vfuncs which can be overriden by each engine. */
 	                               ^^^^^^^^^

total: 0 errors, 1 warnings, 1 checks, 310 lines checked
c8a929723bd2 drm/i915/guc: stop calling execlists_set_default_submission
95e1c72faff4 drm/i915/guc: enable only the user interrupt when using GuC submission


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

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Split GuC submission from execlists submission

[Patchwork]

== Series Details ==

Series: Split GuC submission from execlists submission
URL   : https://patchwork.freedesktop.org/series/85528/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/gt/intel_reset.c:1329:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/gt/selftest_reset.c:101:20:    expected void *in
+drivers/gpu/drm/i915/gt/selftest_reset.c:101:20:    got void [noderef] __iomem *[assigned] s
+drivers/gpu/drm/i915/gt/selftest_reset.c:101:20: warning: incorrect type in assignment (different address spaces)
+drivers/gpu/drm/i915/gt/selftest_reset.c:102:46:    expected void const *src
+drivers/gpu/drm/i915/gt/selftest_reset.c:102:46:    got void [noderef] __iomem *[assigned] s
+drivers/gpu/drm/i915/gt/selftest_reset.c:102:46: warning: incorrect type in argument 2 (different address spaces)
+drivers/gpu/drm/i915/gt/selftest_reset.c:137:20:    expected void *in
+drivers/gpu/drm/i915/gt/selftest_reset.c:137:20:    got void [noderef] __iomem *[assigned] s
+drivers/gpu/drm/i915/gt/selftest_reset.c:137:20: warning: incorrect type in assignment (different address spaces)
+drivers/gpu/drm/i915/gt/selftest_reset.c:138:46:    expected void const *src
+drivers/gpu/drm/i915/gt/selftest_reset.c:138:46:    got void [noderef] __iomem *[assigned] s
+drivers/gpu/drm/i915/gt/selftest_reset.c:138:46: warning: incorrect type in argument 2 (different address spaces)
+drivers/gpu/drm/i915/gt/selftest_reset.c:99:34:    expected unsigned int [usertype] *s
+drivers/gpu/drm/i915/gt/selftest_reset.c:99:34:    got void [noderef] __iomem *[assigned] s
+drivers/gpu/drm/i915/gt/selftest_reset.c:99:34: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gvt/mmio.c:295:23: warning: memcpy with byte count of 279040
+drivers/gpu/drm/i915/i915_perf.c:1450:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1504:15: warning: memset with byte count of 16777216
+./include/linux/seqlock.h:843:24: warning: trying to copy expression type 31
+./include/linux/seqlock.h:843:24: warning: trying to copy expression type 31
+./include/linux/seqlock.h:869:16: warning: trying to copy expression type 31
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block


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

Re: [Intel-gfx] [PATCH 1/5] drm/i915/guc: Delete GuC code unused in future patches

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:43)
> From: Matthew Brost <matthew.brost@intel.com>
> 
> Delete GuC code unused in future patches that rewrite the GuC interface
> to work with the new firmware. Most of the code deleted relates to
> workqueues or execlist port.

Should we not be complaining if i915.enable_guc=1 is set during this
time? We may already be, if so please mention it so that we know that
nobody can accidentally execute this code.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for Split GuC submission from execlists submission

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 4540 bytes --]

== Series Details ==

Series: Split GuC submission from execlists submission
URL   : https://patchwork.freedesktop.org/series/85528/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9547 -> Patchwork_19262
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/index.html

Known issues
------------

  Here are the changes found in Patchwork_19262 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-byt-j1900:       NOTRUN -> [SKIP][1] ([fdo#109271]) +17 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/fi-byt-j1900/igt@amdgpu/amd_basic@userptr.html

  * igt@debugfs_test@read_all_entries:
    - fi-tgl-y:           [PASS][2] -> [DMESG-WARN][3] ([i915#402]) +2 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/fi-tgl-y/igt@debugfs_test@read_all_entries.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/fi-tgl-y/igt@debugfs_test@read_all_entries.html

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-icl-u2:          [PASS][4] -> [DMESG-WARN][5] ([i915#2203] / [i915#2868])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/fi-icl-u2/igt@kms_chamelium@hdmi-crc-fast.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/fi-icl-u2/igt@kms_chamelium@hdmi-crc-fast.html

  
#### Possible fixes ####

  * igt@gem_exec_parallel@engines@contexts:
    - fi-tgl-y:           [FAIL][6] ([i915#2780]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/fi-tgl-y/igt@gem_exec_parallel@engines@contexts.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/fi-tgl-y/igt@gem_exec_parallel@engines@contexts.html

  * igt@gem_sync@basic-all:
    - fi-tgl-y:           [DMESG-WARN][8] ([i915#402]) -> [PASS][9] +1 similar issue
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/fi-tgl-y/igt@gem_sync@basic-all.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/fi-tgl-y/igt@gem_sync@basic-all.html

  * igt@i915_pm_rpm@module-reload:
    - fi-byt-j1900:       [INCOMPLETE][10] ([i915#142] / [i915#2405]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/fi-byt-j1900/igt@i915_pm_rpm@module-reload.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/fi-byt-j1900/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-kbl-soraka:      [DMESG-FAIL][12] ([i915#2291] / [i915#541]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#142]: https://gitlab.freedesktop.org/drm/intel/issues/142
  [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
  [i915#2291]: https://gitlab.freedesktop.org/drm/intel/issues/2291
  [i915#2405]: https://gitlab.freedesktop.org/drm/intel/issues/2405
  [i915#2780]: https://gitlab.freedesktop.org/drm/intel/issues/2780
  [i915#2868]: https://gitlab.freedesktop.org/drm/intel/issues/2868
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541


Participating hosts (42 -> 37)
------------------------------

  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_9547 -> Patchwork_19262

  CI-20190529: 20190529
  CI_DRM_9547: 4f31e1ebafa4d59ce9692a7ef7f76e63e51f58d6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5942: e14e76a87c44c684ec958b391b030bb549254f88 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19262: 95e1c72faff4fcd33f689e4379ea66a3e91fa5ba @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

95e1c72faff4 drm/i915/guc: enable only the user interrupt when using GuC submission
c8a929723bd2 drm/i915/guc: stop calling execlists_set_default_submission
19efc6761a69 drm/i915/guc: init engine directly in GuC submission mode
45b23606fe0e drm/i915/guc: do not dump execlists state with GuC submission
b91fa7500d8f drm/i915/guc: Delete GuC code unused in future patches

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/index.html

[-- Attachment #1.2: Type: text/html, Size: 5396 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [Intel-gfx] [PATCH 2/5] drm/i915/guc: do not dump execlists state with GuC submission

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:44)
> GuC owns the execlists state and the context IDs used for submission, so
> the status of the ports and the CSB entries are not something we control
> or can decode from the i915 side, therefore we can avoid dumping it. A
> follow-up patch will also stop setting the csb pointers when using GuC
> submission.
> 
> GuC dumps all the required events in the GuC logs when verbosity is set
> high enough.

Would not be worth including, or is it not very helpful for debugging
curious engine stalls?

> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/5] drm/i915/guc: stop calling execlists_set_default_submission

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:46)
> Initialize all required entries from guc_set_default_submission, instead
> of calling the execlists function. The previously inherited setup has
> been copied over from the execlist code and simplified by removing the
> execlists submission-specific parts.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: John Harrison <john.c.harrison@intel.com>
> ---
> +       if (INTEL_GEN(engine->i915) >= 12)
> +               engine->flags |= I915_ENGINE_HAS_RELATIVE_MMIO;

We should probably lift this to intel_engine_setup().

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/5] drm/i915/guc: enable only the user interrupt when using GuC submission

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:47)
> In GuC submission mode the CS is owned by the GuC FW, so all CS status
> interrupts are handled by it. We only need the user interrupt as that
> signals request completion.
> 
> Since we're now starting the engines directly in GuC submission mode
> when selected, we can stop switching back and forth between the
> execlists and the GuC programming and select directly the correct
> interrupt mask.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: John Harrison <john.c.harrison@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_irq.c        | 18 ++++++-----
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 31 -------------------
>  2 files changed, 11 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> index 9830342aa6f4..7b2b8cb2d2be 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> @@ -237,14 +237,18 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
>  
>  void gen11_gt_irq_postinstall(struct intel_gt *gt)
>  {
> -       const u32 irqs =
> -               GT_CS_MASTER_ERROR_INTERRUPT |
> -               GT_RENDER_USER_INTERRUPT |
> -               GT_CONTEXT_SWITCH_INTERRUPT |
> -               GT_WAIT_SEMAPHORE_INTERRUPT;
>         struct intel_uncore *uncore = gt->uncore;
> -       const u32 dmask = irqs << 16 | irqs;
> -       const u32 smask = irqs << 16;
> +       u32 irqs = GT_RENDER_USER_INTERRUPT;
> +       u32 dmask;
> +       u32 smask;
> +
> +       if (!intel_uc_wants_guc_submission(&gt->uc))
> +               irqs |= GT_CS_MASTER_ERROR_INTERRUPT |
> +                       GT_CONTEXT_SWITCH_INTERRUPT |
> +                       GT_WAIT_SEMAPHORE_INTERRUPT;

Since these are all inside the engine mask as well, we can set the GT
iir with minimal concern. Is there any advantage in having a patch with
	if (!intel_uc_wants_guc_submission(&gt->uc))
here at this point in time?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/5] drm/i915/guc: Delete GuC code unused in future patches

[Daniele Ceraolo Spurio]

On 1/5/2021 4:55 PM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:43)
>> From: Matthew Brost <matthew.brost@intel.com>
>>
>> Delete GuC code unused in future patches that rewrite the GuC interface
>> to work with the new firmware. Most of the code deleted relates to
>> workqueues or execlist port.
> Should we not be complaining if i915.enable_guc=1 is set during this
> time? We may already be, if so please mention it so that we know that
> nobody can accidentally execute this code.
> -Chris

We already do, I'll add a note in the commit message.

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

Re: [Intel-gfx] [PATCH 2/5] drm/i915/guc: do not dump execlists state with GuC submission

[Daniele Ceraolo Spurio]

On 1/5/2021 4:58 PM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:44)
>> GuC owns the execlists state and the context IDs used for submission, so
>> the status of the ports and the CSB entries are not something we control
>> or can decode from the i915 side, therefore we can avoid dumping it. A
>> follow-up patch will also stop setting the csb pointers when using GuC
>> submission.
>>
>> GuC dumps all the required events in the GuC logs when verbosity is set
>> high enough.
> Would not be worth including, or is it not very helpful for debugging
> curious engine stalls?

GuC is going to reset the engine if it stalls, so we should get the GuC 
logs and the engine state included in the error state.

Daniele

>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: John Harrison <John.C.Harrison@Intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

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

Re: [Intel-gfx] [PATCH 4/5] drm/i915/guc: stop calling execlists_set_default_submission

[Daniele Ceraolo Spurio]

On 1/5/2021 5:02 PM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:46)
>> Initialize all required entries from guc_set_default_submission, instead
>> of calling the execlists function. The previously inherited setup has
>> been copied over from the execlist code and simplified by removing the
>> execlists submission-specific parts.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: John Harrison <john.c.harrison@intel.com>
>> ---
>> +       if (INTEL_GEN(engine->i915) >= 12)
>> +               engine->flags |= I915_ENGINE_HAS_RELATIVE_MMIO;
> We should probably lift this to intel_engine_setup().

GuC requires a more extensive usage of the relative mmio stuff, given 
that it picks which engine to submit to when using virtual engines, so 
I'm not sure if the support is going to look exactly the same for both 
back-ends. There is an old patch from John H to rework the relative mmio 
(https://patchwork.freedesktop.org/patch/332558/), which will have to 
land in some form as part of the GuC submission re-enabling. I'd prefer 
to wait for that to land before moving this flag.

Daniele

>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

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

Re: [Intel-gfx] [PATCH 5/5] drm/i915/guc: enable only the user interrupt when using GuC submission

[Daniele Ceraolo Spurio]

On 1/5/2021 4:15 PM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2021-01-05 23:56:52)
>>
>> On 1/5/2021 3:38 PM, Chris Wilson wrote:
>>> Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:47)
>>>> In GuC submission mode the CS is owned by the GuC FW, so all CS status
>>>> interrupts are handled by it. We only need the user interrupt as that
>>>> signals request completion.
>>>>
>>>> Since we're now starting the engines directly in GuC submission mode
>>>> when selected, we can stop switching back and forth between the
>>>> execlists and the GuC programming and select directly the correct
>>>> interrupt mask.
>>>>
>>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>>> Cc: Matthew Brost <matthew.brost@intel.com>
>>>> Cc: John Harrison <john.c.harrison@intel.com>
>>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/gt/intel_gt_irq.c        | 18 ++++++-----
>>>>    .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 31 -------------------
>>>>    2 files changed, 11 insertions(+), 38 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>>>> index 9830342aa6f4..7b2b8cb2d2be 100644
>>>> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>>>> @@ -237,14 +237,18 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
>>>>    
>>>>    void gen11_gt_irq_postinstall(struct intel_gt *gt)
>>>>    {
>>>> -       const u32 irqs =
>>>> -               GT_CS_MASTER_ERROR_INTERRUPT |
>>>> -               GT_RENDER_USER_INTERRUPT |
>>>> -               GT_CONTEXT_SWITCH_INTERRUPT |
>>>> -               GT_WAIT_SEMAPHORE_INTERRUPT;
>>>>           struct intel_uncore *uncore = gt->uncore;
>>>> -       const u32 dmask = irqs << 16 | irqs;
>>>> -       const u32 smask = irqs << 16;
>>>> +       u32 irqs = GT_RENDER_USER_INTERRUPT;
>>>> +       u32 dmask;
>>>> +       u32 smask;
>>>> +
>>>> +       if (!intel_uc_wants_guc_submission(&gt->uc))
>>>> +               irqs |= GT_CS_MASTER_ERROR_INTERRUPT |
>>>> +                       GT_CONTEXT_SWITCH_INTERRUPT |
>>>> +                       GT_WAIT_SEMAPHORE_INTERRUPT;
>>> Hmm, we should stop performing this by default then, and make the
>>> execlists setup request the interrupt vector it desires.
>>>
>>> That's certainly a bit more fiddly to untangle the packed iir across
>>> multiple gen. :|
>> I had considered that, but this is a gt-level setup while the execlists
>> submission code is currently all engine-based, so I couldn't find a good
>> place to move this to and I didn't want to add a new function just for
>> it. Any preference?
> I think we should definitely strive to avoid having too many backend-
> conditional paths in the common routines, and we can certainly adjust
> the interrupts as we enable each engine (it certainly would help when
> debugging by removing engines by removing spurious iir).
>
> I guess we may end up with something like
> gen8_gt_unmask_engine_irq(struct intel_gt *gt,
> 			  struct intel_engine_cs *engine,
> 			  u16 iir)
> {
> 	if (INTEL_GEN(gt->i915) >= 11)
> 		__gen11...
> 	else
> 		__gen8...
> }
> [Not sure if gen8_gt or intel_gt; I chose gen8_gt for the iir definition]
>
> Then that would fit reasonably into logical_ring_default_irqs().
> Hindsight says that if we had done that earlier, we could have avoided
> maintaining the same engine-iir in two different sites. So maybe worth
> a bit of effort.
> -Chris

Looks reasonable, I'll give it a try.

Daniele

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

Re: [Intel-gfx] [PATCH 2/5] drm/i915/guc: do not dump execlists state with GuC submission

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2021-01-06 02:32:28)
> 
> 
> On 1/5/2021 4:58 PM, Chris Wilson wrote:
> > Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:44)
> >> GuC owns the execlists state and the context IDs used for submission, so
> >> the status of the ports and the CSB entries are not something we control
> >> or can decode from the i915 side, therefore we can avoid dumping it. A
> >> follow-up patch will also stop setting the csb pointers when using GuC
> >> submission.
> >>
> >> GuC dumps all the required events in the GuC logs when verbosity is set
> >> high enough.
> > Would not be worth including, or is it not very helpful for debugging
> > curious engine stalls?
> 
> GuC is going to reset the engine if it stalls, so we should get the GuC 
> logs and the engine state included in the error state.

Here we would be focusing on "why hasn't a request been submitted/executed".
A bad request is usually self-evident, but a missing one is tricky.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/5] drm/i915/guc: stop calling execlists_set_default_submission

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2021-01-06 02:38:09)
> 
> 
> On 1/5/2021 5:02 PM, Chris Wilson wrote:
> > Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:46)
> >> Initialize all required entries from guc_set_default_submission, instead
> >> of calling the execlists function. The previously inherited setup has
> >> been copied over from the execlist code and simplified by removing the
> >> execlists submission-specific parts.
> >>
> >> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >> Cc: Matthew Brost <matthew.brost@intel.com>
> >> Cc: John Harrison <john.c.harrison@intel.com>
> >> ---
> >> +       if (INTEL_GEN(engine->i915) >= 12)
> >> +               engine->flags |= I915_ENGINE_HAS_RELATIVE_MMIO;
> > We should probably lift this to intel_engine_setup().
> 
> GuC requires a more extensive usage of the relative mmio stuff, given 
> that it picks which engine to submit to when using virtual engines, so 
> I'm not sure if the support is going to look exactly the same for both 
> back-ends. There is an old patch from John H to rework the relative mmio 
> (https://patchwork.freedesktop.org/patch/332558/), which will have to 
> land in some form as part of the GuC submission re-enabling. I'd prefer 
> to wait for that to land before moving this flag.

Whether or not LR* take the flag is independent of the submission
backend. As to when to use the flag, I think that patch needs a lot more
refinement.

I915_ENGINE_HAS_RELATIVE_MMIO is the odd-one-out in the flags atm as it
reflects HW capability.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/5] drm/i915/guc: Delete GuC code unused in future patches

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2021-01-06 02:28:46)
> 
> 
> On 1/5/2021 4:55 PM, Chris Wilson wrote:
> > Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:43)
> >> From: Matthew Brost <matthew.brost@intel.com>
> >>
> >> Delete GuC code unused in future patches that rewrite the GuC interface
> >> to work with the new firmware. Most of the code deleted relates to
> >> workqueues or execlist port.
> > Should we not be complaining if i915.enable_guc=1 is set during this
> > time? We may already be, if so please mention it so that we know that
> > nobody can accidentally execute this code.
> > -Chris
> 
> We already do, I'll add a note in the commit message.

With a quick explanation as to how it can do no harm,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/5] drm/i915/guc: init engine directly in GuC submission mode

[Chris Wilson]

Quoting Chris Wilson (2021-01-06 00:02:17)
> Quoting Daniele Ceraolo Spurio (2021-01-05 23:51:43)
> > 
> > 
> > On 1/5/2021 3:33 PM, Chris Wilson wrote:
> > > Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:45)
> > >> Instead of starting the engine in execlists submission mode and then
> > >> switching to GuC, start directly in GuC submission mode. The initial
> > >> setup functions have been copied over from the execlists code
> > >> and simplified by removing the execlists submission-specific parts.
> > >>
> > >> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > >> Cc: Matthew Brost <matthew.brost@intel.com>
> > >> Cc: John Harrison <john.c.harrison@intel.com>
> > >> ---
> > >>   drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   5 +-
> > >>   .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 249 +++++++++++++++++-
> > >>   .../gpu/drm/i915/gt/uc/intel_guc_submission.h |   1 +
> > >>   3 files changed, 244 insertions(+), 11 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > >> index f62303bf80b8..6b4483b72c3f 100644
> > >> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > >> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > >> @@ -40,6 +40,7 @@
> > >>   #include "intel_lrc_reg.h"
> > >>   #include "intel_reset.h"
> > >>   #include "intel_ring.h"
> > >> +#include "uc/intel_guc_submission.h"
> > >>   
> > >>   /* Haswell does have the CXT_SIZE register however it does not appear to be
> > >>    * valid. Now, docs explain in dwords what is in the context object. The full
> > >> @@ -907,7 +908,9 @@ int intel_engines_init(struct intel_gt *gt)
> > >>          enum intel_engine_id id;
> > >>          int err;
> > >>   
> > >> -       if (HAS_EXECLISTS(gt->i915))
> > >> +       if (intel_uc_uses_guc_submission(&gt->uc))
> > > When do we determine intel_uc_uses_guc_submission?
> > 
> > at firmware fetch time.
> > 
> > >
> > > I'm a bit nervous since I've lost track of needs/wants/uses. Is there a
> > > telltale we can place here to assert that we've processed the relevant
> > > init functions (also acting as an aide memoire)?
> > 
> > There is already a GEM_BUG_ON for this inside the function, it'll 
> > trigger if we call it too early.
> > For the submission side of things, there isn't much difference at the 
> > moment between "wants" and "uses" since we do wedge if GuC submission is 
> > selected and the FW is not found. I still prefer to use "uses" where 
> > possible in case we want to change this in the future.
> 
> Ok. If there's a bug on to catch us reordering init incorrectly, that's
> all I'm concerned about.
> 
> > >> +               setup = intel_guc_submission_setup;
> > >> +       else if (HAS_EXECLISTS(gt->i915))
> > >>                  setup = intel_execlists_submission_setup;
> > >>          else
> > >>                  setup = intel_ring_submission_setup;
> > >> +static bool unexpected_starting_state(struct intel_engine_cs *engine)
> > >> +{
> > >> +       bool unexpected = false;
> > >> +
> > >> +       if (ENGINE_READ_FW(engine, RING_MI_MODE) & STOP_RING) {
> > >> +               drm_dbg(&engine->i915->drm,
> > >> +                       "STOP_RING still set in RING_MI_MODE\n");
> > >> +               unexpected = true;
> > >> +       }
> > > Do we care? Is this something we can assume the guc will handle?
> > > (It originated in debugging reset failures.)
> > 
> > GuC handles it post engine reset, but not on init and resume. If you 
> > think this only makes sense for reset debug then I'll get rid of it.
> 
> Yes. I think this can be left as execlists debug code.

Otherwise it looks straightforward,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.IGT: success for Split GuC submission from execlists submission

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 26570 bytes --]

== Series Details ==

Series: Split GuC submission from execlists submission
URL   : https://patchwork.freedesktop.org/series/85528/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9547_full -> Patchwork_19262_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_19262_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_persistence@legacy-engines-persistence:
    - shard-hsw:          NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#1099]) +2 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-hsw4/igt@gem_ctx_persistence@legacy-engines-persistence.html

  * igt@gem_ctx_persistence@replace@rcs0:
    - shard-skl:          [PASS][2] -> [FAIL][3] ([i915#2410])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-skl2/igt@gem_ctx_persistence@replace@rcs0.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl1/igt@gem_ctx_persistence@replace@rcs0.html

  * igt@gem_ctx_sseu@mmap-args:
    - shard-tglb:         NOTRUN -> [SKIP][4] ([i915#280])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-tglb5/igt@gem_ctx_sseu@mmap-args.html

  * igt@gem_exec_params@no-bsd:
    - shard-iclb:         NOTRUN -> [SKIP][5] ([fdo#109283])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@gem_exec_params@no-bsd.html

  * igt@gem_exec_params@rsvd2-dirt:
    - shard-tglb:         NOTRUN -> [SKIP][6] ([fdo#109283])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-tglb5/igt@gem_exec_params@rsvd2-dirt.html

  * igt@gem_exec_reloc@basic-wide-active@bcs0:
    - shard-hsw:          NOTRUN -> [FAIL][7] ([i915#2389]) +3 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-hsw7/igt@gem_exec_reloc@basic-wide-active@bcs0.html

  * igt@gem_exec_reloc@basic-wide-active@vcs1:
    - shard-iclb:         NOTRUN -> [FAIL][8] ([i915#2389])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@gem_exec_reloc@basic-wide-active@vcs1.html

  * igt@gem_exec_whisper@basic-fds-priority:
    - shard-glk:          [PASS][9] -> [DMESG-WARN][10] ([i915#118] / [i915#95])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-glk2/igt@gem_exec_whisper@basic-fds-priority.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-glk3/igt@gem_exec_whisper@basic-fds-priority.html

  * igt@gem_huc_copy@huc-copy:
    - shard-iclb:         NOTRUN -> [SKIP][11] ([i915#2190])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb8/igt@gem_huc_copy@huc-copy.html

  * igt@gem_pread@exhaustion:
    - shard-iclb:         NOTRUN -> [WARN][12] ([i915#2658])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb8/igt@gem_pread@exhaustion.html

  * igt@gem_softpin@evict-snoop-interruptible:
    - shard-iclb:         NOTRUN -> [SKIP][13] ([fdo#109312])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb8/igt@gem_softpin@evict-snoop-interruptible.html

  * igt@gem_userptr_blits@readonly-mmap-unsync@wb:
    - shard-iclb:         NOTRUN -> [SKIP][14] ([i915#1704]) +3 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb8/igt@gem_userptr_blits@readonly-mmap-unsync@wb.html

  * igt@gen9_exec_parse@allowed-all:
    - shard-tglb:         NOTRUN -> [SKIP][15] ([fdo#112306])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-tglb5/igt@gen9_exec_parse@allowed-all.html

  * igt@gen9_exec_parse@secure-batches:
    - shard-iclb:         NOTRUN -> [SKIP][16] ([fdo#112306])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@gen9_exec_parse@secure-batches.html

  * igt@kms_async_flips@alternate-sync-async-flip:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([i915#2521])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-skl7/igt@kms_async_flips@alternate-sync-async-flip.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl9/igt@kms_async_flips@alternate-sync-async-flip.html

  * igt@kms_atomic_transition@plane-all-modeset-transition:
    - shard-iclb:         NOTRUN -> [SKIP][19] ([i915#1769])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb8/igt@kms_atomic_transition@plane-all-modeset-transition.html

  * igt@kms_big_fb@linear-8bpp-rotate-90:
    - shard-iclb:         NOTRUN -> [SKIP][20] ([fdo#110725] / [fdo#111614])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@kms_big_fb@linear-8bpp-rotate-90.html

  * igt@kms_big_fb@x-tiled-16bpp-rotate-270:
    - shard-tglb:         NOTRUN -> [SKIP][21] ([fdo#111614])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-tglb5/igt@kms_big_fb@x-tiled-16bpp-rotate-270.html

  * igt@kms_big_fb@yf-tiled-64bpp-rotate-270:
    - shard-iclb:         NOTRUN -> [SKIP][22] ([fdo#110723])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb8/igt@kms_big_fb@yf-tiled-64bpp-rotate-270.html

  * igt@kms_chamelium@hdmi-cmp-planar-formats:
    - shard-kbl:          NOTRUN -> [SKIP][23] ([fdo#109271] / [fdo#111827]) +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-kbl1/igt@kms_chamelium@hdmi-cmp-planar-formats.html

  * igt@kms_chamelium@vga-edid-read:
    - shard-iclb:         NOTRUN -> [SKIP][24] ([fdo#109284] / [fdo#111827]) +3 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@kms_chamelium@vga-edid-read.html

  * igt@kms_color@pipe-a-ctm-0-75:
    - shard-iclb:         NOTRUN -> [FAIL][25] ([i915#1149] / [i915#315])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@kms_color@pipe-a-ctm-0-75.html

  * igt@kms_color_chamelium@pipe-a-ctm-0-5:
    - shard-tglb:         NOTRUN -> [SKIP][26] ([fdo#109284] / [fdo#111827]) +1 similar issue
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-tglb5/igt@kms_color_chamelium@pipe-a-ctm-0-5.html

  * igt@kms_color_chamelium@pipe-d-ctm-red-to-blue:
    - shard-skl:          NOTRUN -> [SKIP][27] ([fdo#109271] / [fdo#111827]) +4 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl2/igt@kms_color_chamelium@pipe-d-ctm-red-to-blue.html

  * igt@kms_color_chamelium@pipe-d-gamma:
    - shard-hsw:          NOTRUN -> [SKIP][28] ([fdo#109271] / [fdo#111827]) +21 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-hsw4/igt@kms_color_chamelium@pipe-d-gamma.html

  * igt@kms_cursor_crc@pipe-a-cursor-512x512-offscreen:
    - shard-iclb:         NOTRUN -> [SKIP][29] ([fdo#109278] / [fdo#109279]) +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb8/igt@kms_cursor_crc@pipe-a-cursor-512x512-offscreen.html

  * igt@kms_cursor_crc@pipe-a-cursor-512x512-sliding:
    - shard-tglb:         NOTRUN -> [SKIP][30] ([fdo#109279])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-tglb5/igt@kms_cursor_crc@pipe-a-cursor-512x512-sliding.html

  * igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding:
    - shard-skl:          NOTRUN -> [FAIL][31] ([i915#54])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl6/igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-skl:          [PASS][32] -> [FAIL][33] ([i915#54]) +7 similar issues
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-skl9/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
    - shard-tglb:         NOTRUN -> [SKIP][34] ([fdo#111825]) +7 similar issues
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-tglb5/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html

  * igt@kms_cursor_legacy@cursorb-vs-flipa-atomic-transitions:
    - shard-iclb:         NOTRUN -> [SKIP][35] ([fdo#109274] / [fdo#109278])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@kms_cursor_legacy@cursorb-vs-flipa-atomic-transitions.html

  * igt@kms_cursor_legacy@pipe-d-single-move:
    - shard-iclb:         NOTRUN -> [SKIP][36] ([fdo#109278]) +6 similar issues
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@kms_cursor_legacy@pipe-d-single-move.html

  * igt@kms_cursor_legacy@pipe-d-torture-move:
    - shard-skl:          NOTRUN -> [SKIP][37] ([fdo#109271]) +66 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl6/igt@kms_cursor_legacy@pipe-d-torture-move.html

  * igt@kms_dp_tiled_display@basic-test-pattern:
    - shard-tglb:         NOTRUN -> [SKIP][38] ([i915#426])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-tglb5/igt@kms_dp_tiled_display@basic-test-pattern.html

  * igt@kms_flip@2x-flip-vs-rmfb-interruptible:
    - shard-iclb:         NOTRUN -> [SKIP][39] ([fdo#109274])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@kms_flip@2x-flip-vs-rmfb-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-draw-pwrite:
    - shard-iclb:         NOTRUN -> [SKIP][40] ([fdo#109280]) +12 similar issues
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-kbl:          NOTRUN -> [SKIP][41] ([fdo#109271]) +11 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-kbl1/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc.html

  * igt@kms_hdr@static-toggle-dpms:
    - shard-iclb:         NOTRUN -> [SKIP][42] ([i915#1187])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb8/igt@kms_hdr@static-toggle-dpms.html

  * igt@kms_pipe_b_c_ivb@from-pipe-c-to-b-with-3-lanes:
    - shard-iclb:         NOTRUN -> [SKIP][43] ([fdo#109289]) +1 similar issue
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@kms_pipe_b_c_ivb@from-pipe-c-to-b-with-3-lanes.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][44] -> [FAIL][45] ([fdo#108145] / [i915#265]) +1 similar issue
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl5/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-kbl:          NOTRUN -> [FAIL][46] ([fdo#108145] / [i915#265])
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-kbl1/igt@kms_plane_alpha_blend@pipe-c-alpha-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-yf:
    - shard-tglb:         NOTRUN -> [SKIP][47] ([fdo#111615])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-tglb5/igt@kms_plane_lowres@pipe-a-tiling-yf.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [PASS][48] -> [SKIP][49] ([fdo#109441]) +1 similar issue
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb7/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_psr@psr2_suspend:
    - shard-iclb:         NOTRUN -> [SKIP][50] ([fdo#109441]) +1 similar issue
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@kms_psr@psr2_suspend.html

  * igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
    - shard-skl:          [PASS][51] -> [INCOMPLETE][52] ([i915#198])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-skl5/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl10/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html

  * igt@nouveau_crc@pipe-a-ctx-flip-detection:
    - shard-iclb:         NOTRUN -> [SKIP][53] ([i915#2530])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@nouveau_crc@pipe-a-ctx-flip-detection.html

  * igt@nouveau_crc@pipe-d-source-rg:
    - shard-iclb:         NOTRUN -> [SKIP][54] ([fdo#109278] / [i915#2530])
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@nouveau_crc@pipe-d-source-rg.html

  * igt@prime_nv_api@i915_self_import:
    - shard-tglb:         NOTRUN -> [SKIP][55] ([fdo#109291])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-tglb5/igt@prime_nv_api@i915_self_import.html

  * igt@prime_nv_api@i915_self_import_to_different_fd:
    - shard-iclb:         NOTRUN -> [SKIP][56] ([fdo#109291])
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb8/igt@prime_nv_api@i915_self_import_to_different_fd.html

  * igt@prime_vgem@sync@rcs0:
    - shard-iclb:         [PASS][57] -> [INCOMPLETE][58] ([i915#409])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-iclb5/igt@prime_vgem@sync@rcs0.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb1/igt@prime_vgem@sync@rcs0.html

  * igt@sysfs_heartbeat_interval@precise@vcs0:
    - shard-hsw:          NOTRUN -> [SKIP][59] ([fdo#109271]) +239 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-hsw4/igt@sysfs_heartbeat_interval@precise@vcs0.html

  
#### Possible fixes ####

  * {igt@gem_exec_balancer@fairslice}:
    - shard-tglb:         [FAIL][60] ([i915#2802]) -> [PASS][61]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-tglb2/igt@gem_exec_balancer@fairslice.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-tglb1/igt@gem_exec_balancer@fairslice.html

  * {igt@gem_exec_fair@basic-none-share@rcs0}:
    - shard-apl:          [SKIP][62] ([fdo#109271]) -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-apl1/igt@gem_exec_fair@basic-none-share@rcs0.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-apl4/igt@gem_exec_fair@basic-none-share@rcs0.html

  * {igt@gem_exec_fair@basic-pace@rcs0}:
    - shard-iclb:         [FAIL][64] ([i915#2842]) -> [PASS][65] +1 similar issue
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-iclb3/igt@gem_exec_fair@basic-pace@rcs0.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb3/igt@gem_exec_fair@basic-pace@rcs0.html

  * {igt@gem_exec_fair@basic-pace@vecs0}:
    - shard-glk:          [FAIL][66] ([i915#2842]) -> [PASS][67] +1 similar issue
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-glk8/igt@gem_exec_fair@basic-pace@vecs0.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-glk7/igt@gem_exec_fair@basic-pace@vecs0.html

  * {igt@gem_exec_schedule@u-fairslice@rcs0}:
    - shard-kbl:          [DMESG-WARN][68] ([i915#1610] / [i915#2803]) -> [PASS][69]
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-kbl4/igt@gem_exec_schedule@u-fairslice@rcs0.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-kbl1/igt@gem_exec_schedule@u-fairslice@rcs0.html

  * igt@gem_mmap_gtt@big-bo:
    - shard-skl:          [DMESG-WARN][70] ([i915#1982]) -> [PASS][71] +1 similar issue
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-skl1/igt@gem_mmap_gtt@big-bo.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl5/igt@gem_mmap_gtt@big-bo.html

  * {igt@gem_vm_create@destroy-race}:
    - shard-tglb:         [INCOMPLETE][72] -> [PASS][73]
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-tglb5/igt@gem_vm_create@destroy-race.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-tglb5/igt@gem_vm_create@destroy-race.html

  * igt@i915_selftest@live@execlists:
    - shard-iclb:         [INCOMPLETE][74] ([i915#1037] / [i915#2276]) -> [PASS][75]
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-iclb2/igt@i915_selftest@live@execlists.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb8/igt@i915_selftest@live@execlists.html

  * igt@i915_selftest@live@gt_heartbeat:
    - shard-glk:          [DMESG-FAIL][76] ([i915#2291]) -> [PASS][77]
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-glk6/igt@i915_selftest@live@gt_heartbeat.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-glk4/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding:
    - shard-skl:          [FAIL][78] ([i915#54]) -> [PASS][79] +4 similar issues
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-skl6/igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl6/igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [FAIL][80] ([i915#1188]) -> [PASS][81] +1 similar issue
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-skl1/igt@kms_hdr@bpc-switch-dpms.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl9/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][82] ([fdo#108145] / [i915#265]) -> [PASS][83]
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [SKIP][84] ([fdo#109642] / [fdo#111068]) -> [PASS][85]
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-iclb8/igt@kms_psr2_su@page_flip.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb2/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_sprite_blt:
    - shard-iclb:         [SKIP][86] ([fdo#109441]) -> [PASS][87] +1 similar issue
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-iclb8/igt@kms_psr@psr2_sprite_blt.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb2/igt@kms_psr@psr2_sprite_blt.html

  * igt@kms_vblank@pipe-b-ts-continuation-modeset-hang:
    - shard-kbl:          [DMESG-WARN][88] ([i915#165] / [i915#180]) -> [PASS][89]
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-kbl2/igt@kms_vblank@pipe-b-ts-continuation-modeset-hang.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-kbl4/igt@kms_vblank@pipe-b-ts-continuation-modeset-hang.html

  * igt@prime_vgem@sync@rcs0:
    - shard-skl:          [INCOMPLETE][90] ([i915#409]) -> [PASS][91]
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-skl7/igt@prime_vgem@sync@rcs0.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl2/igt@prime_vgem@sync@rcs0.html

  * igt@syncobj_timeline@multi-wait-for-submit-unsubmitted-signaled:
    - shard-iclb:         [INCOMPLETE][92] -> [PASS][93]
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-iclb4/igt@syncobj_timeline@multi-wait-for-submit-unsubmitted-signaled.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb4/igt@syncobj_timeline@multi-wait-for-submit-unsubmitted-signaled.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc6-psr:
    - shard-skl:          [INCOMPLETE][94] ([i915#198]) -> [FAIL][95] ([i915#454])
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-skl1/igt@i915_pm_dc@dc6-psr.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-skl6/igt@i915_pm_dc@dc6-psr.html

  * igt@runner@aborted:
    - shard-kbl:          ([FAIL][96], [FAIL][97]) ([i915#2295] / [i915#2426] / [i915#2505] / [i915#483]) -> [FAIL][98] ([i915#2295] / [i915#483])
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-kbl4/igt@runner@aborted.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-kbl6/igt@runner@aborted.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-kbl4/igt@runner@aborted.html
    - shard-iclb:         [FAIL][99] ([i915#2295] / [i915#2724] / [i915#483]) -> ([FAIL][100], [FAIL][101]) ([i915#2295] / [i915#2426] / [i915#2724] / [i915#409] / [i915#483])
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-iclb5/igt@runner@aborted.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb1/igt@runner@aborted.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-iclb1/igt@runner@aborted.html
    - shard-apl:          [FAIL][102] ([i915#2295]) -> ([FAIL][103], [FAIL][104]) ([i915#1610] / [i915#2295] / [i915#2426])
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9547/shard-apl7/igt@runner@aborted.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-apl7/igt@runner@aborted.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/shard-apl1/igt@runner@aborted.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109312]: https://bugs.freedesktop.org/show_bug.cgi?id=109312
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#110725]: https://bugs.freedesktop.org/show_bug.cgi?id=110725
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112306]: https://bugs.freedesktop.org/show_bug.cgi?id=112306
  [i915#1037]: https://gitlab.freedesktop.org/drm/intel/issues/1037
  [i915#1099]: https://gitlab.freedesktop.org/drm/intel/issues/1099
  [i915#1149]: https://gitlab.freedesktop.org/drm/intel/issues/1149
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1187]: https://gitlab.freedesktop.org/drm/intel/issues/1187
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1610]: https://gitlab.freedesktop.org/drm/intel/issues/1610
  [i915#165]: https://gitlab.freedesktop.org/drm/intel/issues/165
  [i915#1704]: https://gitlab.freedesktop.org/drm/intel/issues/1704
  [i915#1769]: https://gitlab.freedesktop.org/drm/intel/issues/1769
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2276]: https://gitlab.freedesktop.org/drm/intel/issues/2276
  [i915#2291]: https://gitlab.freedesktop.org/drm/intel/issues/2291
  [i915#2295]: https://gitlab.freedesktop.org/drm/intel/issues/2295
  [i915#2389]: https://gitlab.freedesktop.org/drm/intel/issues/2389
  [i915#2410]: https://gitlab.freedesktop.org/drm/intel/issues/2410
  [i915#2426]: https://gitlab.freedesktop.org/drm/intel/issues/2426
  [i915#2505]: https://gitlab.freedesktop.org/drm/intel/issues/2505
  [i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
  [i915#2530]: https://gitlab.freedesktop.org/drm/intel/issues/2530
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
  [i915#2724]: https://gitlab.freedesktop.org/drm/intel/issues/2724
  [i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
  [i915#2802]: https://gitlab.freedesktop.org/drm/intel/issues/2802
  [i915#2803]: https://gitlab.freedesktop.org/drm/intel/issues/2803
  [i915#2804]: https://gitlab.freedesktop.org/drm/intel/issues/2804
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2849]: https://gitlab.freedesktop.org/drm/intel/issues/2849
  [i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
  [i915#409]: https://gitlab.freedesktop.org/drm/intel/issues/409
  [i915#426]: https://gitlab.freedesktop.org/drm/intel/issues/426
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#483]: https://gitlab.freedesktop.org/drm/intel/issues/483
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_9547 -> Patchwork_19262

  CI-20190529: 20190529
  CI_DRM_9547: 4f31e1ebafa4d59ce9692a7ef7f76e63e51f58d6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5942: e14e76a87c44c684ec958b391b030bb549254f88 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19262: 95e1c72faff4fcd33f689e4379ea66a3e91fa5ba @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19262/index.html

[-- Attachment #1.2: Type: text/html, Size: 31896 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [Intel-gfx] [PATCH 2/5] drm/i915/guc: do not dump execlists state with GuC submission

[Daniele Ceraolo Spurio]

On 1/5/2021 6:55 PM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2021-01-06 02:32:28)
>>
>> On 1/5/2021 4:58 PM, Chris Wilson wrote:
>>> Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:44)
>>>> GuC owns the execlists state and the context IDs used for submission, so
>>>> the status of the ports and the CSB entries are not something we control
>>>> or can decode from the i915 side, therefore we can avoid dumping it. A
>>>> follow-up patch will also stop setting the csb pointers when using GuC
>>>> submission.
>>>>
>>>> GuC dumps all the required events in the GuC logs when verbosity is set
>>>> high enough.
>>> Would not be worth including, or is it not very helpful for debugging
>>> curious engine stalls?
>> GuC is going to reset the engine if it stalls, so we should get the GuC
>> logs and the engine state included in the error state.
> Here we would be focusing on "why hasn't a request been submitted/executed".
> A bad request is usually self-evident, but a missing one is tricky.

Agreed, but I still don't think we could use the CSB info even if we 
dumped it. We currently can't map CSB events in GuC submission mode to 
specific contexts, because the ctx IDs used by the GuC do not map to the 
ones used by i915. We've asked the GuC team to expose a way to do such a 
mapping, but that's still under discussion. In the meantime we plan to 
add a few traces to make sure the requests reach the GuC and use the GuC 
logs for what goes on inside the FW (GuC logs include the context IDs it 
uses for submission and all CSB events on high verbosity).

Daniele

> -Chris

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

Re: [Intel-gfx] [PATCH 4/5] drm/i915/guc: stop calling execlists_set_default_submission

[Daniele Ceraolo Spurio]

On 1/5/2021 7:09 PM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2021-01-06 02:38:09)
>>
>> On 1/5/2021 5:02 PM, Chris Wilson wrote:
>>> Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:46)
>>>> Initialize all required entries from guc_set_default_submission, instead
>>>> of calling the execlists function. The previously inherited setup has
>>>> been copied over from the execlist code and simplified by removing the
>>>> execlists submission-specific parts.
>>>>
>>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>>> Cc: Matthew Brost <matthew.brost@intel.com>
>>>> Cc: John Harrison <john.c.harrison@intel.com>
>>>> ---
>>>> +       if (INTEL_GEN(engine->i915) >= 12)
>>>> +               engine->flags |= I915_ENGINE_HAS_RELATIVE_MMIO;
>>> We should probably lift this to intel_engine_setup().
>> GuC requires a more extensive usage of the relative mmio stuff, given
>> that it picks which engine to submit to when using virtual engines, so
>> I'm not sure if the support is going to look exactly the same for both
>> back-ends. There is an old patch from John H to rework the relative mmio
>> (https://patchwork.freedesktop.org/patch/332558/), which will have to
>> land in some form as part of the GuC submission re-enabling. I'd prefer
>> to wait for that to land before moving this flag.
> Whether or not LR* take the flag is independent of the submission
> backend. As to when to use the flag, I think that patch needs a lot more
> refinement.

it definitely does. I'll lift this up for now, we can reconsider later 
if needed.

Daniele

> I915_ENGINE_HAS_RELATIVE_MMIO is the odd-one-out in the flags atm as it
> reflects HW capability.
> -Chris

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

Re: [Intel-gfx] [PATCH 2/5] drm/i915/guc: do not dump execlists state with GuC submission

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2021-01-06 17:21:16)
> 
> 
> On 1/5/2021 6:55 PM, Chris Wilson wrote:
> > Quoting Daniele Ceraolo Spurio (2021-01-06 02:32:28)
> >>
> >> On 1/5/2021 4:58 PM, Chris Wilson wrote:
> >>> Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:44)
> >>>> GuC owns the execlists state and the context IDs used for submission, so
> >>>> the status of the ports and the CSB entries are not something we control
> >>>> or can decode from the i915 side, therefore we can avoid dumping it. A
> >>>> follow-up patch will also stop setting the csb pointers when using GuC
> >>>> submission.
> >>>>
> >>>> GuC dumps all the required events in the GuC logs when verbosity is set
> >>>> high enough.
> >>> Would not be worth including, or is it not very helpful for debugging
> >>> curious engine stalls?
> >> GuC is going to reset the engine if it stalls, so we should get the GuC
> >> logs and the engine state included in the error state.
> > Here we would be focusing on "why hasn't a request been submitted/executed".
> > A bad request is usually self-evident, but a missing one is tricky.
> 
> Agreed, but I still don't think we could use the CSB info even if we 
> dumped it. We currently can't map CSB events in GuC submission mode to 
> specific contexts, because the ctx IDs used by the GuC do not map to the 
> ones used by i915. We've asked the GuC team to expose a way to do such a 
> mapping, but that's still under discussion. In the meantime we plan to 
> add a few traces to make sure the requests reach the GuC and use the GuC 
> logs for what goes on inside the FW (GuC logs include the context IDs it 
> uses for submission and all CSB events on high verbosity).

I was more reflecting on what could be here instead. Details of the ctb?
It would be great to have a snapshot of some relevant guc state, merely
wonder if we could extract anything from the log automatically. As well
as the usual what have we submitted to the guc.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/5] drm/i915/guc: do not dump execlists state with GuC submission

[Daniele Ceraolo Spurio]

On 1/6/2021 11:43 AM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2021-01-06 17:21:16)
>>
>> On 1/5/2021 6:55 PM, Chris Wilson wrote:
>>> Quoting Daniele Ceraolo Spurio (2021-01-06 02:32:28)
>>>> On 1/5/2021 4:58 PM, Chris Wilson wrote:
>>>>> Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:44)
>>>>>> GuC owns the execlists state and the context IDs used for submission, so
>>>>>> the status of the ports and the CSB entries are not something we control
>>>>>> or can decode from the i915 side, therefore we can avoid dumping it. A
>>>>>> follow-up patch will also stop setting the csb pointers when using GuC
>>>>>> submission.
>>>>>>
>>>>>> GuC dumps all the required events in the GuC logs when verbosity is set
>>>>>> high enough.
>>>>> Would not be worth including, or is it not very helpful for debugging
>>>>> curious engine stalls?
>>>> GuC is going to reset the engine if it stalls, so we should get the GuC
>>>> logs and the engine state included in the error state.
>>> Here we would be focusing on "why hasn't a request been submitted/executed".
>>> A bad request is usually self-evident, but a missing one is tricky.
>> Agreed, but I still don't think we could use the CSB info even if we
>> dumped it. We currently can't map CSB events in GuC submission mode to
>> specific contexts, because the ctx IDs used by the GuC do not map to the
>> ones used by i915. We've asked the GuC team to expose a way to do such a
>> mapping, but that's still under discussion. In the meantime we plan to
>> add a few traces to make sure the requests reach the GuC and use the GuC
>> logs for what goes on inside the FW (GuC logs include the context IDs it
>> uses for submission and all CSB events on high verbosity).
> I was more reflecting on what could be here instead. Details of the ctb?
> It would be great to have a snapshot of some relevant guc state, merely
> wonder if we could extract anything from the log automatically. As well
> as the usual what have we submitted to the guc.
> -Chris

Just realized I hadn't replied to this. We're still discussing with the 
GuC team about what type of GuC status we can extract and/or ask GuC to 
provide.
Request list aside, most of the i915-side of the GuC info is going to be 
global (single ctb channel, single submission queue into GuC), so it'll 
likely end up in a different printer function.

Daniele

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