On 05/02/2012 01:52 PM, Paul Berry wrote:
> This patch splits up the gen6_blorp_exec and gen7_blorp_exec
> functions, which were very long, into simple component functions.
> With a few exceptions, there is one function per state packet.
>
> This will allow blit functionality to be added without significantly
> complicating the code.
> ---
> src/mesa/drivers/dri/i965/brw_blorp.h | 23 +-
> src/mesa/drivers/dri/i965/gen6_blorp.cpp | 596
> +++++++++++++++++-------------
> src/mesa/drivers/dri/i965/gen7_blorp.cpp | 448 ++++++++++++-----------
> 3 files changed, 595 insertions(+), 472 deletions(-)
[snip]
> +/**
> + * Disable thread dispatch (dw5.19) and enable the HiZ op.
> + */
> +static void
> +gen6_blorp_emit_wm_disable(struct brw_context *brw,
> + const brw_blorp_params *params)
/begin bikeshedding
I think the name of this function is misleading. In the other
gen6_blorp_emit_UNIT_disable functions, we *really* disable the unit
and don't care much about the contents of the 3DSTATE_UNIT packet,
because the packet doesn't instruct the gpu to do anything.
However, that's not the case for 3DSTATE_WM. The content of this packet
is paramount; it tells the gpu which hiz op to execute. Technically,
this function *does disable* the WM. But, on the other hand, it does
much more than that.
Ditto for gen7.
But, I see that I'm bikeshedding now. I need to continue on and do actual,
*useful* review now :)
/end bikeshedding
[snip]
> /**
> - * \param out_offset is relative to
> - * CMD_STATE_BASE_ADDRESS.DynamicStateBaseAddress.
> + * \brief Execute a blit or render pass operation.
> + *
> + * To execute the operation, this function manually constructs and emits a
> + * batch to draw a rectangle primitive. The batchbuffer is flushed before
> + * constructing and after emitting the batch.
> + *
> + * This function alters no GL state.
> */
> void
> -gen6_blorp_emit_depth_stencil_state(struct brw_context *brw,
> - const brw_blorp_params *params,
> - uint32_t *out_offset)
> +gen6_blorp_exec(struct intel_context *intel,
> + const brw_blorp_params *params)
> {
> - struct gen6_depth_stencil_state *state;
> - state = (struct gen6_depth_stencil_state *)
> - brw_state_batch(brw, AUB_TRACE_DEPTH_STENCIL_STATE,
> - sizeof(*state), 64,
> - out_offset);
> - memset(state, 0, sizeof(*state));
> + struct gl_context *ctx = &intel->ctx;
> + struct brw_context *brw = brw_context(ctx);
> + uint32_t depthstencil_offset;
>
> - /* See the following sections of the Sandy Bridge PRM, Volume 1, Part2:
> - * - 7.5.3.1 Depth Buffer Clear
> - * - 7.5.3.2 Depth Buffer Resolve
> - * - 7.5.3.3 Hierarchical Depth Buffer Resolve
> + gen6_blorp_emit_batch_head(brw, params);
> + gen6_blorp_emit_vertices(brw, params);
> + gen6_blorp_emit_urb_config(brw, params);
> + depthstencil_offset = gen6_blorp_emit_depth_stencil_state(brw, params);
> + gen6_blorp_emit_cc_state_pointers(brw, params, depthstencil_offset);
> + gen6_blorp_emit_vs_disable(brw, params);
> + gen6_blorp_emit_gs_disable(brw, params);
> + gen6_blorp_emit_clip_disable(brw, params);
> + gen6_blorp_emit_sf_config(brw, params);
> + gen6_blorp_emit_wm_disable(brw, params);
> +
> + gen6_blorp_emit_depth_stencil_config(brw, params);
> + gen6_blorp_emit_clear_params(brw, params);
> + gen6_blorp_emit_drawing_rectangle(brw, params);
> + gen6_blorp_emit_primitive(brw, params);
> +
> + /* See comments above at first invocation of intel_flush() in
> + * gen6_blorp_emit_batch_head().
> */
> - state->ds2.depth_write_enable = 1;
> - if (params->hiz_op == GEN6_HIZ_OP_DEPTH_RESOLVE) {
> - state->ds2.depth_test_enable = 1;
> - state->ds2.depth_test_func = COMPAREFUNC_NEVER;
> - }
> + intel_flush(ctx);
> +
> + /* Be safe. */
> + brw->state.dirty.brw = ~0;
> + brw->state.dirty.cache = ~0;
> }
I like the new form of gen6_blorp_exec. I look forward to discover how,
in later patches, you integrate msaa into it.
Many of the functions introduced by this patch contain a cast
struct intel_context *intel = &brw->intel;
but do not use the 'intel' variable. After cleaning up the
unused variables, this patch is
Reviewed-by: Chad Versace <chad.vers...@linux.intel.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
