On Wed, Mar 15, 2017 at 08:14:04AM +0000, Tvrtko Ursulin wrote: > > 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: > >>>} > >>> > >>>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. At the moment, setup_common() has no dependencies. Having the callback for engine->set_default_submission inside init_common() doesn't break the pattern of all vfuncs being in setup_common() - if you no longer regard the two vfuncs set by the callback as being part of that set. In init_common() we already utilize the state set on the engine, the callback seems consistent there. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
