Re: [PATCH] drm/i915: Round-up GTT allocations for unfenced surfaces to the next tile row

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Round-up GTT allocations for unfenced surfaces to the next tile row
Date: Sat, 26 Mar 2011 09:43:03 +0000	[thread overview]
Message-ID: <849307$c62598@azsmga001.ch.intel.com> (raw)
In-Reply-To: <20110326092021.GA3485@viiv.ffwll.ch>

On Sat, 26 Mar 2011 10:20:21 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Sat, Mar 26, 2011 at 08:52:31AM +0000, Chris Wilson wrote:
> > However, I'm not sure if this truly prevents the corruption on i8xx with
> > 2.14.0. Can somebody break out an old machine and test?
> 
> It won't (assuming I correctly diagnosed the problem): Corruptions happen
> because i8xx uses copies of the scratch page in the lower-left corner.

Yeah, I couldn't explain it any other way, but I had to give the simple
fix a shot. Oh well. (And I still can't explain how it is *using* the
results of the out-of-bounds sampling unless our assumptions about tiling
on i8xx are fundamentally flawed. Later specs make it clear that sampling
may occur outside of the texture, but the results are automatically
culled.)

> Only extending the size of the allocation prevents that. What this patch
> does though is preventing corruptions in the immediately following bo.
> Dunno whether that's worth to fix given that people rather quickly
> complain about visual corruptions, but seem to somewhat accept crashy i8xx
> systems as a fact of life.

Eventually yes, we shouldn't let userspace easily foul up the hardware or
access information belonging to others. (I know we currently have lots of
shortcomings in just how far we can go here...) If we can provide more
information to the kernel, we can be even smarter about GTT allocations
versus physical page allocations. That's a pipe dream.

So, remaining options:

1) Convince everyone that it really is the *new* userspace code for 2.6.38
that is broken and they should just upgrade the buggy packages.

2) Disable RELAXED_FENCING for gen2 and let them continue to suffer. After
all, nobody has seemed to notice the performance regressions on i8xx...

3) Pad the allocation in set_tiling?

4) Introduce a new ioctl to provide more details about the surface tiling
to the kernel that can sanity check the allocation for the intended usage
and disable RELAXED_FENCING for the old set_tiling ioctl?

5) Something completely different.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre