[Mesa-dev] [PATCH 09/10] i965: Clean up misleading defines for DWORD 2 of URB_WRITE header.
Kenneth Graunke
kenneth at whitecape.org
Tue Dec 6 23:31:31 PST 2011
On 12/05/2011 09:40 AM, Paul Berry wrote:
> R02_PRIM_END and R02_PRIM_START don't actually refer to bits in DWORD
> 2 of R0 (as the name, and comments in the code, would seem to
> indicate). Actually they refer to bits in DWORD 2 of the header for
> URB_WRITE messages.
>
> This patch renames the defines to reflect what they actually mean. It
> also addes a define URB_WRITE_M02_PRIM_TYPE_SHIFT, which previously
> was just hardcoded in .c files.
I'm all for renaming these. But I might drop the "_M02_" altogether,
and simply go with:
URB_WRITE_PRIM_START
URB_WRITE_PRIM_END
URB_WRITE_PRIM_TYPE_SHIFT
Those names seem clear enough, and are a bit more concise. We don't
usually put the DWord number in most of our #defines. Also, I
personally find both "R02" and "M02" confusing...usually "R<n>" means
GRF n and "M<n>" means MRF n. Yes, the leading 0 suggests it probably
means something else...but will people figure out what? :)
I might instead expand the comment in brw_defines.h slightly to mention
that these bits go in DW2 of the URB_WRITE message header.
> ---
> src/mesa/drivers/dri/i965/brw_clip_line.c | 8 +++-
> src/mesa/drivers/dri/i965/brw_clip_tri.c | 11 ++++--
> src/mesa/drivers/dri/i965/brw_clip_unfilled.c | 13 ++++++--
> src/mesa/drivers/dri/i965/brw_defines.h | 8 ++--
> src/mesa/drivers/dri/i965/brw_gs_emit.c | 44 ++++++++++++++++++-------
> 5 files changed, 60 insertions(+), 24 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_line.c b/src/mesa/drivers/dri/i965/brw_clip_line.c
> index 75c64c0..0b11bcb 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_line.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_line.c
> @@ -254,8 +254,12 @@ static void clip_and_emit_line( struct brw_clip_compile *c )
> brw_clip_interp_vertex(c, newvtx0, vtx0, vtx1, c->reg.t0, false);
> brw_clip_interp_vertex(c, newvtx1, vtx1, vtx0, c->reg.t1, false);
>
> - brw_clip_emit_vue(c, newvtx0, 1, 0, (_3DPRIM_LINESTRIP << 2) | R02_PRIM_START);
> - brw_clip_emit_vue(c, newvtx1, 0, 1, (_3DPRIM_LINESTRIP << 2) | R02_PRIM_END);
> + brw_clip_emit_vue(c, newvtx0, 1, 0,
> + (_3DPRIM_LINESTRIP << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_START);
> + brw_clip_emit_vue(c, newvtx1, 0, 1,
> + (_3DPRIM_LINESTRIP << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_END);
> }
> brw_ENDIF(p);
> brw_clip_kill_thread(c);
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_tri.c b/src/mesa/drivers/dri/i965/brw_clip_tri.c
> index ffbfe94..bc7f542 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_tri.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_tri.c
> @@ -422,14 +422,17 @@ void brw_clip_tri_emit_polygon(struct brw_clip_compile *c)
> brw_MOV(p, get_addr_reg(vptr), brw_address(c->reg.inlist));
> brw_MOV(p, get_addr_reg(v0), deref_1uw(vptr, 0));
>
> - brw_clip_emit_vue(c, v0, 1, 0, ((_3DPRIM_TRIFAN << 2) | R02_PRIM_START));
> + brw_clip_emit_vue(c, v0, 1, 0,
> + ((_3DPRIM_TRIFAN << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_START));
>
> brw_ADD(p, get_addr_reg(vptr), get_addr_reg(vptr), brw_imm_uw(2));
> brw_MOV(p, get_addr_reg(v0), deref_1uw(vptr, 0));
>
> loop = brw_DO(p, BRW_EXECUTE_1);
> {
> - brw_clip_emit_vue(c, v0, 1, 0, (_3DPRIM_TRIFAN << 2));
> + brw_clip_emit_vue(c, v0, 1, 0,
> + (_3DPRIM_TRIFAN << URB_WRITE_M02_PRIM_TYPE_SHIFT));
>
> brw_ADD(p, get_addr_reg(vptr), get_addr_reg(vptr), brw_imm_uw(2));
> brw_MOV(p, get_addr_reg(v0), deref_1uw(vptr, 0));
> @@ -439,7 +442,9 @@ void brw_clip_tri_emit_polygon(struct brw_clip_compile *c)
> }
> brw_WHILE(p, loop);
>
> - brw_clip_emit_vue(c, v0, 0, 1, ((_3DPRIM_TRIFAN << 2) | R02_PRIM_END));
> + brw_clip_emit_vue(c, v0, 0, 1,
> + ((_3DPRIM_TRIFAN << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_END));
> }
> brw_ENDIF(p);
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_unfilled.c b/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
> index ae84e19..0191b4a 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_unfilled.c
> @@ -321,8 +321,12 @@ static void emit_lines(struct brw_clip_compile *c,
> brw_imm_f(0));
> brw_IF(p, BRW_EXECUTE_1);
> {
> - brw_clip_emit_vue(c, v0, 1, 0, (_3DPRIM_LINESTRIP << 2) | R02_PRIM_START);
> - brw_clip_emit_vue(c, v1, 1, 0, (_3DPRIM_LINESTRIP << 2) | R02_PRIM_END);
> + brw_clip_emit_vue(c, v0, 1, 0,
> + (_3DPRIM_LINESTRIP << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_START);
> + brw_clip_emit_vue(c, v1, 1, 0,
> + (_3DPRIM_LINESTRIP << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_END);
> }
> brw_ENDIF(p);
>
> @@ -363,7 +367,10 @@ static void emit_points(struct brw_clip_compile *c,
> if (do_offset)
> apply_one_offset(c, v0);
>
> - brw_clip_emit_vue(c, v0, 1, 0, (_3DPRIM_POINTLIST << 2) | R02_PRIM_START | R02_PRIM_END);
> + brw_clip_emit_vue(c, v0, 1, 0,
> + (_3DPRIM_POINTLIST << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_START
> + | URB_WRITE_M02_PRIM_END);
> }
> brw_ENDIF(p);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
> index bb79bfb..2990c90 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -1413,10 +1413,10 @@ enum brw_wm_barycentric_interp_mode {
> #define CMD_MI_FLUSH 0x0200
>
>
> -/* Various values from the R0 vertex header:
> - */
> -#define R02_PRIM_END 0x1
> -#define R02_PRIM_START 0x2
> +/* Bitfields for the URB_WRITE message: */
> +#define URB_WRITE_M02_PRIM_END 0x1
> +#define URB_WRITE_M02_PRIM_START 0x2
> +#define URB_WRITE_M02_PRIM_TYPE_SHIFT 2
>
> #include "intel_chipset.h"
>
> diff --git a/src/mesa/drivers/dri/i965/brw_gs_emit.c b/src/mesa/drivers/dri/i965/brw_gs_emit.c
> index 935e663..3d332c4 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs_emit.c
> @@ -188,21 +188,29 @@ void brw_gs_quads( struct brw_gs_compile *c, struct brw_gs_prog_key *key )
> */
> if (intel->needs_ff_sync)
> brw_gs_ff_sync(c, 1);
> - brw_gs_overwrite_header_dw2(c, (_3DPRIM_POLYGON << 2) | R02_PRIM_START);
> + brw_gs_overwrite_header_dw2(
> + c, ((_3DPRIM_POLYGON << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_START));
> if (key->pv_first) {
> brw_gs_emit_vue(c, c->reg.vertex[0], 0);
> - brw_gs_overwrite_header_dw2(c, _3DPRIM_POLYGON << 2);
> + brw_gs_overwrite_header_dw2(
> + c, _3DPRIM_POLYGON << URB_WRITE_M02_PRIM_TYPE_SHIFT);
> brw_gs_emit_vue(c, c->reg.vertex[1], 0);
> brw_gs_emit_vue(c, c->reg.vertex[2], 0);
> - brw_gs_overwrite_header_dw2(c, (_3DPRIM_POLYGON << 2) | R02_PRIM_END);
> + brw_gs_overwrite_header_dw2(
> + c, ((_3DPRIM_POLYGON << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_END));
> brw_gs_emit_vue(c, c->reg.vertex[3], 1);
> }
> else {
> brw_gs_emit_vue(c, c->reg.vertex[3], 0);
> - brw_gs_overwrite_header_dw2(c, _3DPRIM_POLYGON << 2);
> + brw_gs_overwrite_header_dw2(
> + c, _3DPRIM_POLYGON << URB_WRITE_M02_PRIM_TYPE_SHIFT);
> brw_gs_emit_vue(c, c->reg.vertex[0], 0);
> brw_gs_emit_vue(c, c->reg.vertex[1], 0);
> - brw_gs_overwrite_header_dw2(c, (_3DPRIM_POLYGON << 2) | R02_PRIM_END);
> + brw_gs_overwrite_header_dw2(
> + c, ((_3DPRIM_POLYGON << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_END));
> brw_gs_emit_vue(c, c->reg.vertex[2], 1);
> }
> }
> @@ -216,21 +224,29 @@ void brw_gs_quad_strip( struct brw_gs_compile *c, struct brw_gs_prog_key *key )
>
> if (intel->needs_ff_sync)
> brw_gs_ff_sync(c, 1);
> - brw_gs_overwrite_header_dw2(c, (_3DPRIM_POLYGON << 2) | R02_PRIM_START);
> + brw_gs_overwrite_header_dw2(
> + c, ((_3DPRIM_POLYGON << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_START));
> if (key->pv_first) {
> brw_gs_emit_vue(c, c->reg.vertex[0], 0);
> - brw_gs_overwrite_header_dw2(c, _3DPRIM_POLYGON << 2);
> + brw_gs_overwrite_header_dw2(
> + c, _3DPRIM_POLYGON << URB_WRITE_M02_PRIM_TYPE_SHIFT);
> brw_gs_emit_vue(c, c->reg.vertex[1], 0);
> brw_gs_emit_vue(c, c->reg.vertex[2], 0);
> - brw_gs_overwrite_header_dw2(c, (_3DPRIM_POLYGON << 2) | R02_PRIM_END);
> + brw_gs_overwrite_header_dw2(
> + c, ((_3DPRIM_POLYGON << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_END));
> brw_gs_emit_vue(c, c->reg.vertex[3], 1);
> }
> else {
> brw_gs_emit_vue(c, c->reg.vertex[2], 0);
> - brw_gs_overwrite_header_dw2(c, _3DPRIM_POLYGON << 2);
> + brw_gs_overwrite_header_dw2(
> + c, _3DPRIM_POLYGON << URB_WRITE_M02_PRIM_TYPE_SHIFT);
> brw_gs_emit_vue(c, c->reg.vertex[3], 0);
> brw_gs_emit_vue(c, c->reg.vertex[0], 0);
> - brw_gs_overwrite_header_dw2(c, (_3DPRIM_POLYGON << 2) | R02_PRIM_END);
> + brw_gs_overwrite_header_dw2(
> + c, ((_3DPRIM_POLYGON << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_END));
> brw_gs_emit_vue(c, c->reg.vertex[1], 1);
> }
> }
> @@ -244,8 +260,12 @@ void brw_gs_lines( struct brw_gs_compile *c )
>
> if (intel->needs_ff_sync)
> brw_gs_ff_sync(c, 1);
> - brw_gs_overwrite_header_dw2(c, (_3DPRIM_LINESTRIP << 2) | R02_PRIM_START);
> + brw_gs_overwrite_header_dw2(
> + c, ((_3DPRIM_LINESTRIP << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_START));
> brw_gs_emit_vue(c, c->reg.vertex[0], 0);
> - brw_gs_overwrite_header_dw2(c, (_3DPRIM_LINESTRIP << 2) | R02_PRIM_END);
> + brw_gs_overwrite_header_dw2(
> + c, ((_3DPRIM_LINESTRIP << URB_WRITE_M02_PRIM_TYPE_SHIFT)
> + | URB_WRITE_M02_PRIM_END));
> brw_gs_emit_vue(c, c->reg.vertex[1], 1);
> }
More information about the mesa-dev
mailing list