On Mon, May 09, 2016 at 09:58:20AM +0200, Daniel Vetter wrote: > On Mon, May 09, 2016 at 08:45:16AM +0100, Chris Wilson wrote: > > This is sanitize. We do enable it in engine->init_hw(), but the point > > raised by Ville earlier in his review of GT irq handling is that nobody > > currently disables the ring IMR before use. Here we have a > > chicken-and-egg problem, do we duplicate knowledge of available engines > > (and their mmio_base) in irq preinstall/sanitize or do we do the engine > > specific mmio in engine initialisation? The problem Turslin was raising > > was that on future enabling, somebody had enabled the engine IRQ before > > the engines were initialised (i.e. had completely disregarded the > > current init_hw sequence). Plonking it in i915_irq.c is not foolproof > > either! > > Hm, couldn't we put it into init_hw? i915_irq.c sets up the top-level > interrupts, but for GT stuff all masked. In init_hw we could clear that > then, and before init_hw no one should call ring->get_irq to enable it and > potentially cause havoc. Or still too fragile in your opinion? The race is if we get an interrupt inside init_engine, after we set engine->dev but before we setup the state for the irq handler. (Note the race isn't strictly just dev, everything we touch inside the irq handler gives arise to a potential ordering issue.) > Indeed putting it into i915_irq.c seems to not be great since it splits > the gt per-engine mask reg handling too far. Yeah. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
