On Mon, Jan 23, 2017 at 01:33:32PM +0200, Joonas Lahtinen wrote:
> On la, 2017-01-21 at 09:28 +0000, Chris Wilson wrote:
> > If the CSB head/tail pointers are unchanged, we can skip the update of
> > the CSB register afterwards.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>
> <SNIP>
>
> > @@ -577,7 +577,7 @@ static void intel_lrc_irq_handler(unsigned long data)
> >
> > intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
> >
> > - if (!execlists_elsp_idle(engine)) {
> > + if (!execlists_elsp_idle(engine)) do {
>
> Oh dear, not like this. I hope this was a leftover from debugging.
>
> > @@ -587,9 +587,12 @@ static void intel_lrc_irq_handler(unsigned long data)
> > csb = readl(csb_mmio);
> > head = GEN8_CSB_READ_PTR(csb);
> > tail = GEN8_CSB_WRITE_PTR(csb);
> > + if (head == tail)
> > + break;
> > +
> > if (tail < head)
> > tail += GEN8_CSB_ENTRIES;
> > - while (head < tail) {
> > + do {
> > unsigned int idx = ++head % GEN8_CSB_ENTRIES;
> > unsigned int status = readl(buf + 2 * idx);
>
> Theoretically it should not matter, I wonder if this is covered well
> enough in CI that we're confident to skip such writes?
We see head == tail reasonably frequently (just the effect of timing
between interrupts and mmio reads). The single register write is not of
huge consequence, it just looked easy to remove by adjusting the
loop. The dominant issue here are the mmio reads of the buffer. The
documentation talks about mapping these registers as cacheable and so
doing a single cacheline read to grab the entire set. That keeps
frustrating me.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
