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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 15/03/2017 09:41, Chris Wilson wrote:
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.

Okay, you convinced me. :)

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux