<div dir="ltr">On 28 August 2013 16:49, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">These functions have almost identical code; the only difference is that a few of the bits moved around. Adding a few trivial conditionals allows the same function to work on all generations, and the resulting code is still quite readable. Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> </blockquote><div> </div><div>Thanks for doing this! One minor comment below. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> --- src/mesa/drivers/dri/i965/brw_draw.c | 80 ++++++++---------------------------- 1 file changed, 17 insertions(+), 63 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c index c7164ac..df9b750 100644 --- a/src/mesa/drivers/dri/i965/brw_draw.c +++ b/src/mesa/drivers/dri/i965/brw_draw.c @@ -171,11 +171,15 @@ static void brw_emit_prim(struct brw_context *brw, start_vertex_location = prim->start; base_vertex_location = prim->basevertex; if (prim->indexed) { - vertex_access_type = GEN4_3DPRIM_VERTEXBUFFER_ACCESS_RANDOM; + vertex_access_type = brw->gen >= 7 ? + GEN7_3DPRIM_VERTEXBUFFER_ACCESS_RANDOM : + GEN4_3DPRIM_VERTEXBUFFER_ACCESS_RANDOM; start_vertex_location += brw->ib.start_vertex_offset; base_vertex_location += brw->vb.start_vertex_bias; } else { - vertex_access_type = GEN4_3DPRIM_VERTEXBUFFER_ACCESS_SEQUENTIAL; + vertex_access_type = brw->gen >= 7 ? + GEN7_3DPRIM_VERTEXBUFFER_ACCESS_SEQUENTIAL : + GEN4_3DPRIM_VERTEXBUFFER_ACCESS_SEQUENTIAL; start_vertex_location += brw->vb.start_vertex_bias; } @@ -198,65 +202,16 @@ static void brw_emit_prim(struct brw_context *brw, intel_batchbuffer_emit_mi_flush(brw); } - BEGIN_BATCH(6); - OUT_BATCH(CMD_3D_PRIM << 16 | (6 - 2) | - hw_prim << GEN4_3DPRIM_TOPOLOGY_TYPE_SHIFT | - vertex_access_type); - OUT_BATCH(verts_per_instance); - OUT_BATCH(start_vertex_location); - OUT_BATCH(prim->num_instances); - OUT_BATCH(prim->base_instance); - OUT_BATCH(base_vertex_location); - ADVANCE_BATCH(); - - brw->batch.need_workaround_flush = true; - - if (brw->always_flush_cache) { - intel_batchbuffer_emit_mi_flush(brw); - } -} - -static void gen7_emit_prim(struct brw_context *brw, - const struct _mesa_prim *prim, - uint32_t hw_prim) -{ - int verts_per_instance; - int vertex_access_type; - int start_vertex_location; - int base_vertex_location; - - DBG("PRIM: %s %d %d\n", _mesa_lookup_enum_by_nr(prim->mode), - prim->start, prim->count); - - start_vertex_location = prim->start; - base_vertex_location = prim->basevertex; - if (prim->indexed) { - vertex_access_type = GEN7_3DPRIM_VERTEXBUFFER_ACCESS_RANDOM; - start_vertex_location += brw->ib.start_vertex_offset; - base_vertex_location += brw->vb.start_vertex_bias; + if (brw->gen >= 7) { + BEGIN_BATCH(7); + OUT_BATCH(CMD_3D_PRIM << 16 | (7 - 2)); + OUT_BATCH(hw_prim | vertex_access_type); } else { - vertex_access_type = GEN7_3DPRIM_VERTEXBUFFER_ACCESS_SEQUENTIAL; - start_vertex_location += brw->vb.start_vertex_bias; + BEGIN_BATCH(6); + OUT_BATCH(CMD_3D_PRIM << 16 | (6 - 2) | + hw_prim << GEN4_3DPRIM_TOPOLOGY_TYPE_SHIFT | + vertex_access_type); } - - verts_per_instance = prim->count; - - /* If nothing to emit, just return. */ - if (verts_per_instance == 0) - return; - - /* If we're set to always flush, do it before and after the primitive emit. - * We want to catch both missed flushes that hurt instruction/state cache - * and missed flushes of the render cache as it heads to other parts of - * the besides the draw code. - */ - if (brw->always_flush_cache) { - intel_batchbuffer_emit_mi_flush(brw); - } - - BEGIN_BATCH(7); - OUT_BATCH(CMD_3D_PRIM << 16 | (7 - 2)); - OUT_BATCH(hw_prim | vertex_access_type); OUT_BATCH(verts_per_instance); OUT_BATCH(start_vertex_location); OUT_BATCH(prim->num_instances); @@ -264,6 +219,8 @@ static void gen7_emit_prim(struct brw_context *brw, OUT_BATCH(base_vertex_location); ADVANCE_BATCH(); + brw->batch.need_workaround_flush = true; + </blockquote><div> </div><div>It took a while for to me to figure out why this wasn't inside a conditional (prior to this patch it only existed in brw_emit_prim). </div><div>It might be nice to add a comment either here or in the commit message explaining that it is safe to set brw->batch.need_workaround_flush unconditionally, since the boolean is only used on Gen6. </div><div>In any case, this patch is: Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> if (brw->always_flush_cache) { intel_batchbuffer_emit_mi_flush(brw); } @@ -453,10 +410,7 @@ retry: brw_upload_state(brw); } - if (brw->gen >= 7) - gen7_emit_prim(brw, &prim[i], brw->primitive); - else - brw_emit_prim(brw, &prim[i], brw->primitive); + brw_emit_prim(brw, &prim[i], brw->primitive); brw->no_batch_wrap = false; -- 1.8.3.4 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>