On 29/04/16 11:00, Chris Wilson wrote:
On Fri, Apr 29, 2016 at 10:50:02AM +0100, Tvrtko Ursulin wrote:
On 29/04/16 10:39, Chris Wilson wrote:
On Fri, Apr 29, 2016 at 10:25:41AM +0100, Tvrtko Ursulin wrote:
On 29/04/16 10:15, Chris Wilson wrote:
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2e0eaa9fa240..2c94072ab085 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2016,14 +2016,17 @@ logical_ring_setup(struct drm_device *dev, enum intel_engine_id id)
struct intel_engine_cs *engine = &dev_priv->engine[id];
enum forcewake_domains fw_domains;
- engine->dev = dev;
-
engine->id = id;
engine->name = info->name;
engine->exec_id = info->exec_id;
engine->guc_id = info->guc_id;
engine->mmio_base = info->mmio_base;
+ /* disable interrupts to this engine before we install ourselves*/
+ I915_WRITE_IMR(engine, ~0);
+
+ engine->dev = dev;
+
/* Intentionally left blank. */
engine->buffer = NULL;
Make sense?
Not the most elegant because all the hw access we have so far is in
engine->init_hw. Why can't we just make intel_engine_initialized
return false until the very last thing in engine constructors?
In my defence sanitizing the hw before we are ready is common practice
across the driver. The unfun part is that irq install is before gem_init
(because modeset init wants irq enabled for GMBUS/dp-aux). gem init
itself could be split up and moved around so that the setup and init_hw
phases are separate (which would be next on the init ordering hitlist I
hope).
I want engine->dev/engine->i915 set early so we can use it during setup
and init-hw and so that for_each_engine() works as expected in that
time.
Why wouldn't an explicit engine->initialized flag solve that? You
could keep setting engine->dev early (as it should be) and then set
engine->initialized at the end of per-engine constructors.
Because it becomes irrelevant very shortly. The only interesting
question remaining is whether or not we should be sanitizing the IMR
first. It has been suggested elsewhere (in Ville's review of the GT
interrupt handling) that doing the sanitization would be useful.
How come it becomes irrelevant? Will there not be
intel_engine_initialized? Because as long as there is, imho it makes
sense not to use engine->dev for it.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
