On Thu, Jan 28, 2016 at 11:45:24AM +0000, Chris Wilson wrote: > On Thu, Jan 28, 2016 at 10:55:16AM +0000, Daniele Ceraolo Spurio wrote: > > > > > > On 27/01/16 16:39, Ville Syrjälä wrote: > > >On Wed, Jan 27, 2016 at 03:43:49PM +0000,daniele.ceraolospurio@xxxxxxxxx wrote: > > >>From: Daniele Ceraolo Spurio<daniele.ceraolospurio@xxxxxxxxx> > > >> > > >>While running some tests on the scheduler patches with rpm enabled I > > >>came across a corruption in the ringbuffer, which was root-caused to > > >>the GPU being suspended while commands were being emitted to the > > >>ringbuffer. The access to memory was failing because the GPU needs to > > >>be awake when accessing stolen memory (where my ringbuffer was located). > > >>Since we have this constraint it looks like a sensible idea to check > > >>that we hold a refcount when we access the rungbuffer. > > >> > > >>v2: move the check from ring_begin to ringbuffer iomap time (Chris) > > >>v3: update comment (Chris) > > >> > > >>Cc: John Harrison<John.C.Harrison@xxxxxxxxx> > > >>Cc: Chris Wilson<chris@xxxxxxxxxxxxxxxxxx> > > >>Signed-off-by: Daniele Ceraolo Spurio<daniele.ceraolospurio@xxxxxxxxx> > > >>--- > > >> drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++ > > >> 1 file changed, 3 insertions(+) > > >> > > >>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > > >>index 6f5b511..133321a 100644 > > >>--- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > >>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > >>@@ -2119,6 +2119,9 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev, > > >> return ret; > > >> } > > >>+ /* Access through the GTT requires the device to be awake. */ > > >>+ assert_rpm_wakelock_held(dev_priv); > > >>+ > > >Hmm. This function doesn't actually acces the ring buffer, so it's a bit > > >odd to see this here. > > > > I had it inring_begin initially, but Chris suggested moving it here > > because we pin the ringbuffer before accessing it. Do you have a > > different place in mind for where this should be added or would you > > be happy with a simple comment update? > > This function we call in order to acquire access to the ring iomap for > the request. At the beginning of the request, we should be pinning > everything we need to build the request. If writing through the GTT we > should be ensuring that the device is also awake. The oddity is that > this is not yet explicit and the asymmetry still exists between > legacy/execlists. Yeah, with ringbuffer mode this gets executed exactly once, so more or less useless at the moment. With execlists I suppose it might catch something on CHV/BXT. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
