On 14/03/2017 21:33, Chris Wilson wrote:
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@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
---
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.
Sounds good.
}
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?
Why do you think it is hairy for intel_engine_setup_common? It keeps the
setup/init split for lrc, where all vfuncs are initialized in the setup
phase which was the intention.
Legacy is a bit more uncleanly split now that I look at it, but putting
it at intel_engine_init_common would be just more of the same.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
