[Intel-gfx] [PATCH v3 1/2] drm/i915: Move engine->submit_request selection to a vfunc

[Chris Wilson]

On Tue, Mar 14, 2017 at 04:31:58PM +0000, Tvrtko Ursulin wrote:
> 
> On 14/03/2017 09:34, Chris Wilson wrote:
> >It turns out that we may want to restore the original
> >engine->submit_request (and engine->schedule) callbacks from more than
> >just the guc <-> execlists transition. Move this to a vfunc so we can
> >have a common interface.
> >
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> >---
> > drivers/gpu/drm/i915/i915_guc_submission.c |  2 +-
> > drivers/gpu/drm/i915/intel_engine_cs.c     | 10 ++++++++++
> > drivers/gpu/drm/i915/intel_lrc.c           | 15 +++++----------
> > drivers/gpu/drm/i915/intel_lrc.h           |  1 -
> > drivers/gpu/drm/i915/intel_ringbuffer.c    | 15 +++++++++++++--
> > drivers/gpu/drm/i915/intel_ringbuffer.h    |  4 ++++
> > 6 files changed, 33 insertions(+), 14 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> >index b4d24cd7639a..119b5c073833 100644
> >--- a/drivers/gpu/drm/i915/i915_guc_submission.c
> >+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> >@@ -1051,7 +1051,7 @@ void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
> > 		return;
> >
> > 	/* Revert back to manual ELSP submission */
> >-	intel_execlists_enable_submission(dev_priv);
> >+	intel_engines_enable_submission(dev_priv);
> 
> intel_engines_default_submission came to my mind but that will also
> be misleading once the guc switch is toggled. But I think less
> misleading than enable_submission. And vfunc name maybe as
> assign_default_submission?

intel_engines_reset_default_submission
engine->set_default_submission

Not overly enamoured, but the above seems like the best compromise so
far.

> 
> > }
> >
> > void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
> >diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> >index 73fe718516a5..5663ebab851f 100644
> >--- a/drivers/gpu/drm/i915/intel_engine_cs.c
> >+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> >@@ -191,6 +191,7 @@ int intel_engines_init(struct drm_i915_private *dev_priv)
> > 			goto cleanup;
> > 		}
> >
> >+		engine->enable_submission(engine);
> 
> Could be moved to intel_engines_setup_common if the
> logical_ring_setup was re-arranged a bit so that the default_vfuncs
> are assigned before it. Legacy looks like it would be alright with
> that approach already.
> 
> My thinking here is not to expose this vfunc so prominently in the
> code since it is a bit of a low level internal implementation thing.

The concern is reasonable, but equally moving it to
intel_engine_setup_common() is hairy. Otoh, I think it is suitable for
intel_engine_init_common(). Happy?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre