[Mesa-dev] [PATCH 24/24] i965: Simplify the shader time code by using atomic counter helpers.
Paul Berry
stereotype441 at gmail.com
Thu Sep 26 14:31:10 PDT 2013
On 15 September 2013 00:19, Francisco Jerez <currojerez at riseup.net> wrote:
> ---
> src/mesa/drivers/dri/i965/brw_eu_emit.c | 25
> ++++-------------------
> src/mesa/drivers/dri/i965/brw_state.h | 2 --
> src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 4 +++-
> src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 4 +++-
> src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 17 ---------------
> 5 files changed, 10 insertions(+), 42 deletions(-)
>
I've sent comments on patches 3, 4, 6, 8, 9, 10, 12, 13, 14, 15, 16, 18,
19, 21, and 22. The rest (1, 2, 5, 7, 11, 17, 20, 23, and 24) are:
Reviewed-by: Paul Berry <stereotype441 at gmail.com>
However, there are two things which I expected to see in this patch series
but I didn't:
- Do atomic_uints need similar handling to ir_sampler_replacement_visitor
in opt_function_inlining?
- Do we need to update schedule_node::set_latency_gen7() with an estimate
of how long it takes to execute an atomic operation? Currently it looks
like it's assuming 14 cycles, which seems almost certainly wrong.
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index 7484649..a6cc92a 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -2619,25 +2619,8 @@ void brw_shader_time_add(struct brw_compile *p,
> BRW_ARF_NULL, 0));
> brw_set_src0(p, send, brw_vec1_reg(payload.file,
> payload.nr, 0));
> -
> - uint32_t sfid, msg_type;
> - if (brw->is_haswell) {
> - sfid = HSW_SFID_DATAPORT_DATA_CACHE_1;
> - msg_type = HSW_DATAPORT_DC_PORT1_UNTYPED_ATOMIC_OP;
> - } else {
> - sfid = GEN7_SFID_DATAPORT_DATA_CACHE;
> - msg_type = GEN7_DATAPORT_DC_UNTYPED_ATOMIC_OP;
> - }
> -
> - bool header_present = false;
> - bool eot = false;
> - uint32_t mlen = 2; /* offset, value */
> - uint32_t rlen = 0;
> - brw_set_message_descriptor(p, send, sfid, mlen, rlen, header_present,
> eot);
> -
> - send->bits3.ud |= msg_type << 14;
> - send->bits3.ud |= 0 << 13; /* no return data */
> - send->bits3.ud |= 1 << 12; /* SIMD8 mode */
> - send->bits3.ud |= BRW_AOP_ADD << 8;
> - send->bits3.ud |= surf_index << 0;
> + brw_set_dp_untyped_atomic_message(p, send, BRW_AOP_ADD, surf_index,
> + 2 /* message length */,
> + 0 /* response length */,
> + false /* header present */);
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h
> b/src/mesa/drivers/dri/i965/brw_state.h
> index 1d01406..6101aae 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -209,8 +209,6 @@ void gen7_set_surface_mcs_info(struct brw_context *brw,
> bool is_render_target);
> void gen7_check_surface_setup(uint32_t *surf, bool is_render_target);
> void gen7_init_vtable_surface_functions(struct brw_context *brw);
> -void gen7_create_shader_time_surface(struct brw_context *brw,
> - uint32_t *out_offset);
>
> /* gen7_sol_state.c */
> void gen7_upload_3dstate_so_decl_list(struct brw_context *brw,
> diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> index 9f8c0f5..17fa30d 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> @@ -179,7 +179,9 @@ brw_vec4_upload_binding_table(struct brw_context *brw,
> int i;
>
> if (INTEL_DEBUG & DEBUG_SHADER_TIME) {
> - gen7_create_shader_time_surface(brw,
> &stage_state->surf_offset[SURF_INDEX_VEC4_SHADER_TIME]);
> + brw->vtbl.create_raw_surface(
> + brw, brw->shader_time.bo, 0, brw->shader_time.bo->size,
> + &stage_state->surf_offset[SURF_INDEX_VEC4_SHADER_TIME], true);
> }
>
> /* Skip making a binding table if we don't use textures or pull
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 48f351f..da7dac1 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -886,7 +886,9 @@ brw_upload_wm_binding_table(struct brw_context *brw)
> int i;
>
> if (INTEL_DEBUG & DEBUG_SHADER_TIME) {
> - gen7_create_shader_time_surface(brw,
> &brw->wm.base.surf_offset[SURF_INDEX_WM_SHADER_TIME]);
> + brw->vtbl.create_raw_surface(
> + brw, brw->shader_time.bo, 0, brw->shader_time.bo->size,
> + &brw->wm.base.surf_offset[SURF_INDEX_WM_SHADER_TIME], true);
> }
>
> /* CACHE_NEW_WM_PROG */
> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> index 8b86387..6f5e670 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> @@ -455,23 +455,6 @@ gen7_create_raw_surface(struct brw_context *brw,
> drm_intel_bo *bo,
> true /* rw */);
> }
>
> -/**
> - * Create a surface for shader time.
> - */
> -void
> -gen7_create_shader_time_surface(struct brw_context *brw, uint32_t
> *out_offset)
> -{
> - gen7_emit_buffer_surface_state(brw,
> - out_offset,
> - brw->shader_time.bo,
> - 0,
> - BRW_SURFACEFORMAT_RAW,
> - brw->shader_time.bo->size - 1,
> - 1,
> - 0 /* mocs */,
> - true /* rw */);
> -}
> -
> static void
> gen7_update_null_renderbuffer_surface(struct brw_context *brw, unsigned
> unit)
> {
> --
> 1.8.3.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130926/aaaf3356/attachment-0001.html>
More information about the mesa-dev
mailing list