Quoting Tvrtko Ursulin (2019-04-18 16:08:58) > > On 18/04/2019 15:59, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2019-04-18 15:34:09) > >> > >> On 18/04/2019 14:27, Chris Wilson wrote: > >>> We store the engine->imr mask and set up the RING_IMR register on > >>> restarting the engine. We do not then want to overwrite it with > >>> an incomplete mask later as we may then lose interrupts! > >>> > >>> Reported-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > >>> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > >>> --- > >>> drivers/gpu/drm/i915/intel_ringbuffer.c | 3 --- > >>> 1 file changed, 3 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > >>> index 029fd8ec1857..00bd9eeb053d 100644 > >>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > >>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > >>> @@ -873,9 +873,6 @@ static int init_render_ring(struct intel_engine_cs *engine) > >>> if (IS_GEN_RANGE(dev_priv, 6, 7)) > >>> I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING)); > >>> > >>> - if (INTEL_GEN(dev_priv) >= 6) > >>> - ENGINE_WRITE(engine, RING_IMR, ~engine->irq_keep_mask); > >>> - > >>> return 0; > >>> } > >>> > >>> > >> > >> Is it perhaps a way to enable L3_DPF on IVB and HSW when breadcrumbs are > >> off (no one listening)? Mind you, the same should be done in this case > >> under execlists for ctx switch irq. I just can't find the code there > >> which unmasks it unless a single engine->irq_enable/irq_disable cycle is > >> guaranteed on load indirectly. Is there a request_wait or wait_for_idle > >> hidden somewhere during init? Or I missed the ball completely? > > > > See intel_engine_reset_breadcrumbs(), we always re-emit the breadcrumb > > setup (IMR) for either the disabled or enabled state. That should make > > sure engine->irq_keep_mask is applied. > > Ah.. thanks! Although it is a bit conceptually wrong that breadcrumbs > have to do the basic/unrelated engine setup. As such the legacy > submission indeed looks more correct, but still wrong that engine reset > can turn off user interrupts. Blah.. another pedestrian task for a rainy > day. I kind of agree, but it's quite tangled up -- how dare they put extra state in my breadcrumb register :) Take the view that breadcrumbs is a badly named RING_IMR wrapper. Maybe you can find an angle there. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
