On Tue, Mar 04, 2014 at 10:01:56PM +0000, Chris Wilson wrote: > 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@xxxxxxxxxxxxxxxxxx> > > > > > 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@xxxxxxxxxxxxxxxxxx> > > > > > > > > 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@xxxxxxxxxxxx> > > > > > > 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? Actually. Look at it, replacing the single if() with a copy_page_from_bo(dev_priv, src, i) is not the beautification you seek. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
