[Intel-gfx] [PATCH] drm/i915: Do not access stolen memory directly by the CPU, even for error capture

[Chris Wilson]

On Tue, Mar 04, 2014 at 01:27:05PM -0800, Ben Widawsky wrote:
> On Tue, Mar 04, 2014 at 03:45:56PM +0100, Daniel Vetter wrote:
> > On Tue, Feb 18, 2014 at 11:18:04AM -0800, Ben Widawsky wrote:
> > > On Wed, Feb 12, 2014 at 07:18:40PM +0000, Chris Wilson wrote:
> > > > For stolen pages, since it is verboten to access them directly on many
> > > > architectures, we have to read them through the GTT aperture. If they
> > > > are not accessible through the aperture, then we have to abort.
> > > > 
> > > > This was complicated by
> > > > 
> > > > commit 8b6124a633d8095b0c8364f585edff9c59568a96
> > > > Author: Chris Wilson <chris at chris-wilson.co.uk>
> > > > Date:   Thu Jan 30 14:38:16 2014 +0000
> > > > 
> > > >     drm/i915: Don't access snooped pages through the GTT (even for error capture)
> > > > 
> > > > and the desire to use stolen memory for ringbuffers, contexts and
> > > > batches in the future.
> > > > 
> > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > 
> > > I'd prefer separate functions for the different types of error objects
> > > (gtt vs CPU mapped). Or maybe just pass in the capture type as an
> > > argument and then create a helper to determine the right thing. It'd at
> > > least be a bit easier for review.
> > > 
> > > Anyway, having not actually looked at the code, the idea is solid:
> > > Acked-by: Ben Widawsky <ben at bwidawsk.net>
> > 
> > Can you please actually look at the code and upgrade this to a full
> > review?
> > 
> > Thanks, Daniel
> 
> I had been hoping for a bit of a rewrite to do the review. Chris are you
> opposed to my suggestions?

You can convert the loop into a function pointer, but is that going to
make the decision tree as to which function pointer to use any clearer -
since it stays the same?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre