[Mesa-dev] [PATCH] i965: Remove redundant gen check

[Ben Widawsky]

On Fri, Apr 10, 2015 at 02:14:09AM -0700, Kenneth Graunke wrote:
> On Thursday, April 09, 2015 07:46:34 PM Ben Widawsky wrote:
> > The blit ring doesn't exist until Gen6, therefore we don't need to actually
> > check it. Keep an assert to make sure old code doesn't do stupid stuff.
> > 
> > (Only compile tested)
> > 
> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > ---
> >  src/mesa/drivers/dri/i965/intel_batchbuffer.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> > index e522e4e..ada5573 100644
> > --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> > +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> > @@ -702,7 +702,8 @@ intel_emit_post_sync_nonzero_flush(struct brw_context *brw)
> >  void
> >  intel_batchbuffer_emit_mi_flush(struct brw_context *brw)
> >  {
> > -   if (brw->batch.ring == BLT_RING && brw->gen >= 6) {
> > +   if (brw->batch.ring == BLT_RING) {
> > +      assert(brw->gen >= 6);
> >        BEGIN_BATCH_BLT(4);
> >        OUT_BATCH(MI_FLUSH_DW);
> >        OUT_BATCH(0);
> > 
> 
> NAK.  It doesn't work that way.
> 
> BEGIN_BATCH() sets brw->batch.ring to RENDER_RING.
> BEGIN_BATCH_BLT() sets brw->batch.ring to BLT_RING.
> 
> This happens on all generations.  When doing execbuf, we ignore the ring
> type on Gen4-5, and always use I915_EXEC_RENDER.  On Gen6+, we use both.
> 
> On Gen6+, we flush the batch when changing ring types.  On Gen4-5, we
> don't care - it just reflects the type of commands emitted last.
> 
> I suppose we could change BEGIN_BATCH_BLT to use RENDER_RING for
> everything on Gen4-5, so the ring enum matches up better.  That would
> be fine.
> 
> As is, this will assert fail on Gen4-5.

Yeah, we had a discussion on IRC last night and I realized it's very likely this
is the case. I have no desire to change it, I think it's fine to keep it as is -
only slightly confusing, but not bad. (Indeed I did submit it to jenkins last
night after sending the patch, and it fails gen4-5).

Interestingly I also realized last night that ILK PRM suggests there is a BCS...
weird.

Thanks for looking at it - sorry to waste your time.