Comment 159 for bug 541511

> --- Comment #132 from Indan Zupancic <email address hidden> 2010-04-19 15:26:36 PDT ---
> What I don't understand is why your patch slows things down so much for me,
> it seems to do only a few thousand flushes anyway.

Well, worst-case a flush can take 1 ms.

> I guess copying around is what the old drivers did?

Nope. But for various reasons it changed mappings _much_ less. So much
less likely to crash.

> Some random ideas:
>
> - Increase I830_MCH_WRITE_BUFFER_SIZE?

Tried. Given up at 64 kb.

> - Instead of writing zeroes, actually change the content of the flush page.
> Flushing caches doesn't seem to do much if the new content is the same as
> the old one?

Patch does that atm for all writes. Furthermore I've never seen hw that
clever (it's a total worthless optimization, usually).

> - The text you quoted in one of your commit messages said that the memory
> content isn't coherent, but it didn't say anything about the mapping itself.
> Can't you update the gtt mapping to effectively flush it? I mean, if you
> move pages out of the gtt and back in, shouldn't that flush the old content?
> Maybe move it to a different index, e.g. insert new mapping to the start
> instead of the end, in case the hw caches it by address+index. Similar to
> Chris Wilson's gtt disabling thing, but instead of disabling, altering it
> in a smart, flush causing way.

Well, that's exactly where the shit usually hits the fan. Furthermore, at
least on i845 there are chipset errata that says (no joke) if you change a
mapping shortly before the gpu reads stuff from it, it may read adjacent
pages. Chris is trying to battle that one. Oh, and no, rewriting the gtt
entries doesn't flush data (only tlb, but not everywhere, see above).

> If the problem is that the flush is needed to avoid the hardware from writing
> stale data to old gtt mapped physical memory:
>
> - If an entry is added, there should be no need for a flush, because the all
> memory is still valid. If an entry is removed, the gpu can continue to write
> to those pages. What about copying the content to a new physical page and
> keeping the original page for a while until the gpu is done with it?

Something similar is already done. Look for scratch_page in intel-gtt.c

> > Ok, that's bad. Can you change the following define in
> > include/drm/intel-gtt.h and see whether you still get failed chipset
> > flushes?
> >
> > -#define I830_CC_CANARY_FLOCK_GTT_PAGES 8
> > +#define I830_CC_CANARY_FLOCK_GTT_PAGES 16
> >
> > The whole stuff make somewhat more sense this way around, anyway.
>
> I will try this later, first I'm going to try without your latest commit
> ("fix i85x gtt chipset flush") to see how it behaves without that stuff,
> both performance and amount of failed flushes.

If your X40 is anything like mine, you're in for a bad surprise :(

> > Oh, and add some details about your box, please (brand&model + cpu,
> > mostly, the rest is all in the dmesg, anyway).
>
> See my first post: Thinkpad X40, 855GM (rev 02), Pentium M (family 6, model 13,
> stepping 6: It has clflush).

Thanks, I'm regularly losing my overview with all the different testers on
this bug ;)