[Intel-gfx] [RFC 0/6] Start separating GuC and execlists submission

[Matthew Brost]

On Fri, Jan 24, 2020 at 04:55:31PM -0800, Daniele Ceraolo Spurio wrote:
>Note: this applies on top of my series to commit early to GuC [1].
>
>Picking up from the feedback from my RFC series about splitting
>up intel_lrc.c [2], this series introduces GuC submission versions
>of most of the engine and context functions. As a starting point,
>the functions are still very similar to the execlists ones, but
>they will progressively diverge while we add the new submission
>logic. Some of the functions have been simplified by removing
>support for pre-gen11 cases as we only aim to enable GuC submission
>for gen11+; I've left comments and BUG_ONs to mark the reduces
>support spots to easily find them in case we ever want to enable
>GuC submission for older gens.
>
>Going slightly against the feedback from the previous series, I have
>kept the basic context and ring object management shared between
>execlists and GuC submission functions. The rationale is that those
>objects are HW-related and therefore their creation and their main
>attributes (e.g. context size) are not dependent on the submission
>method in any way. Handling of more SW constructs, like the timeline,
>has been duplicated.
>
>Still in theme of sharing, the flush and bb_start functions have also
>been re-used on the GuC side, but I'm not sure if keeping them in
>intel_lrc.c is the best solution. My medium-term plan is still the same
>as [2], i.e. split execlists_submission.c out of intel_lrc.c, with the
>latter holding the common code, but it might be worth moving the
>command emission to a dedicated file or to a header as inlines, like
>we already do in some cases.
>

I can't say I'm thrilled about the amount of code duplicated in this series.
i.e. 95% of the breadcrumb code is exactly the same, request_alloc is a copy and
paste, same with resume, and same with the context operations. Surely there is a
way to share the code common between the GuC execlists? I like the idea of
putting all of this common code in a shared file and then
intel_execlist_submission.c & intel_guc_submission.c call into the common
functions while retaining there own set of unique function pointers.

Matt

>The code is still a bit rough and the series has been compile-tested
>only. I wanted to get some early comments about the direction before
>rebasing the rest of the GuC code on top of it and start testing.
>
>[1] https://patchwork.freedesktop.org/series/72031/
>[2] https://patchwork.freedesktop.org/series/70787/
>
>Cc: Chris Wilson <chris at chris-wilson.co.uk>
>Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>Cc: John Harrison <John.C.Harrison at Intel.com>
>Cc: Matthew Brost <matthew.brost at intel.com>
>
>Daniele Ceraolo Spurio (6):
>  drm/i915/guc: Add guc-specific breadcrumb functions
>  drm/i915/guc: Add request_alloc for guc_submission
>  drm/i915/guc: Add engine->resume for GuC submission
>  drm/i915/guc: Re-use lrc flush functions
>  drm/i915/guc: Add initial context ops for GuC submission
>  drm/i915/guc: Stop inheriting from execlists_set_default_submission
>
> drivers/gpu/drm/i915/gt/intel_lrc.c           | 217 +++++-----
> drivers/gpu/drm/i915/gt/intel_lrc.h           |  17 +-
> drivers/gpu/drm/i915/gt/selftest_lrc.c        |  12 +-
> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 378 +++++++++++++++++-
> 4 files changed, 496 insertions(+), 128 deletions(-)
>
>-- 
>2.24.1
>