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@xxxxxxxxx>
Cc: Matthew Brost <matthew.brost@xxxxxxxxx>
Cc: John Harrison <john.c.harrison@xxxxxxxxx>
---
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(>->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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
