> -----Original Message----- > From: Volkin, Bradley D > Sent: Friday, June 20, 2014 10:18 PM > To: Mateo Lozano, Oscar > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH 27/53] drm/i915/bdw: GEN-specific logical > ring emit request > > On Fri, Jun 13, 2014 at 08:37:45AM -0700, oscar.mateo@xxxxxxxxx wrote: > > From: Oscar Mateo <oscar.mateo@xxxxxxxxx> > > > > Very similar to the legacy add_request, only modified to account for > > logical ringbuffer. > > > > Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > drivers/gpu/drm/i915/intel_lrc.c | 61 > +++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++ > > 3 files changed, 64 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h index 9c8692a..63ec3ea 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -267,6 +267,7 @@ > > #define MI_FORCE_RESTORE (1<<1) > > #define MI_RESTORE_INHIBIT (1<<0) > > #define MI_STORE_DWORD_IMM MI_INSTR(0x20, 1) > > +#define MI_STORE_DWORD_IMM_GEN8 MI_INSTR(0x20, 2) > > #define MI_MEM_VIRTUAL (1 << 22) /* 965+ only */ > > #define MI_STORE_DWORD_INDEX MI_INSTR(0x21, 1) > > #define MI_STORE_DWORD_INDEX_SHIFT 2 > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > b/drivers/gpu/drm/i915/intel_lrc.c > > index 89aed7a..3debe8b 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -359,6 +359,62 @@ static void gen8_submit_ctx(struct > intel_engine_cs *ring, > > DRM_ERROR("Execlists still not ready!\n"); } > > > > +static int gen8_emit_request(struct intel_engine_cs *ring, > > + struct intel_context *ctx) > > +{ > > + struct intel_ringbuffer *ringbuf = logical_ringbuf_get(ring, ctx); > > + u32 cmd; > > + int ret; > > + > > + ret = intel_logical_ring_begin(ring, ctx, 6); > > + if (ret) > > + return ret; > > + > > + cmd = MI_FLUSH_DW + 1; > > + cmd |= MI_INVALIDATE_TLB; > > Is the TLB invalidation truely required here? Otherwise it seems like we could > use the same function for all rings, like on gen6+. Hmmmmm... this is inherited from back when we only had the simulator, and its true meaning has been lost in the multiple rewrites: drm/i915/bdw: Use MI_FLUSH_DW for requests The primary reason for doing this is MI_STORE_DWORD_IDX doesn't work in simulation. The simulator doesn't complain, it's just the seqno never gets pushed to memory. The theory is (and AFAICT this may be broken on existing platforms) we must issue an MI_FLUSH_DW after we emit the seqno, if we want to be able to read it back coherently. I´ll rewrite it to use the same MI_STORE_DATA_IMM for every ring and then test it on read hardware. Thanks for the heads up! > > + cmd |= MI_FLUSH_DW_OP_STOREDW; > > + > > + intel_logical_ring_emit(ringbuf, cmd); > > + intel_logical_ring_emit(ringbuf, > > + (ring->status_page.gfx_addr + > > + (I915_GEM_HWS_INDEX << > MI_STORE_DWORD_INDEX_SHIFT)) | > > + MI_FLUSH_DW_USE_GTT); > > + intel_logical_ring_emit(ringbuf, 0); > > + intel_logical_ring_emit(ringbuf, ring->outstanding_lazy_seqno); > > + intel_logical_ring_emit(ringbuf, MI_USER_INTERRUPT); > > + intel_logical_ring_emit(ringbuf, MI_NOOP); > > + intel_logical_ring_advance_and_submit(ring, ctx); > > + > > + return 0; > > +} > > + > > +static int gen8_emit_request_render(struct intel_engine_cs *ring, > > + struct intel_context *ctx) > > +{ > > + struct intel_ringbuffer *ringbuf = logical_ringbuf_get(ring, ctx); > > + u32 cmd; > > + int ret; > > + > > + ret = intel_logical_ring_begin(ring, ctx, 6); > > + if (ret) > > + return ret; > > + > > + cmd = MI_STORE_DWORD_IMM_GEN8; > > + cmd |= (1 << 22); /* use global GTT */ > > We could use MI_MEM_VIRTUAL or MI_GLOBAL_GTT instead. Will do, thanks. -- Oscar _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx