<div dir="ltr">On 15 September 2013 00:19, Francisco Jerez <span dir="ltr"><<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">---<br>
 src/mesa/drivers/dri/i965/brw_eu_emit.c           | 25 ++++-------------------<br>
 src/mesa/drivers/dri/i965/brw_state.h             |  2 --<br>
 src/mesa/drivers/dri/i965/brw_vs_surface_state.c  |  4 +++-<br>
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c  |  4 +++-<br>
 src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 17 ---------------<br>
 5 files changed, 10 insertions(+), 42 deletions(-)<br></blockquote><div><br></div><div>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:<br>
<br></div><div>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br><br></div><div>However, there are two things which I expected to see in this patch series but I didn't:<br>
<br></div><div>- Do atomic_uints need similar handling to ir_sampler_replacement_visitor in opt_function_inlining?<br><br>- 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.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c<br>
index 7484649..a6cc92a 100644<br>
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c<br>
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c<br>
@@ -2619,25 +2619,8 @@ void brw_shader_time_add(struct brw_compile *p,<br>
                                       BRW_ARF_NULL, 0));<br>
    brw_set_src0(p, send, brw_vec1_reg(payload.file,<br>
                                       <a href="http://payload.nr" target="_blank">payload.nr</a>, 0));<br>
-<br>
-   uint32_t sfid, msg_type;<br>
-   if (brw->is_haswell) {<br>
-      sfid = HSW_SFID_DATAPORT_DATA_CACHE_1;<br>
-      msg_type = HSW_DATAPORT_DC_PORT1_UNTYPED_ATOMIC_OP;<br>
-   } else {<br>
-      sfid = GEN7_SFID_DATAPORT_DATA_CACHE;<br>
-      msg_type = GEN7_DATAPORT_DC_UNTYPED_ATOMIC_OP;<br>
-   }<br>
-<br>
-   bool header_present = false;<br>
-   bool eot = false;<br>
-   uint32_t mlen = 2; /* offset, value */<br>
-   uint32_t rlen = 0;<br>
-   brw_set_message_descriptor(p, send, sfid, mlen, rlen, header_present, eot);<br>
-<br>
-   send->bits3.ud |= msg_type << 14;<br>
-   send->bits3.ud |= 0 << 13; /* no return data */<br>
-   send->bits3.ud |= 1 << 12; /* SIMD8 mode */<br>
-   send->bits3.ud |= BRW_AOP_ADD << 8;<br>
-   send->bits3.ud |= surf_index << 0;<br>
+   brw_set_dp_untyped_atomic_message(p, send, BRW_AOP_ADD, surf_index,<br>
+                                     2 /* message length */,<br>
+                                     0 /* response length */,<br>
+                                     false /* header present */);<br>
 }<br>
diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h<br>
index 1d01406..6101aae 100644<br>
--- a/src/mesa/drivers/dri/i965/brw_state.h<br>
+++ b/src/mesa/drivers/dri/i965/brw_state.h<br>
@@ -209,8 +209,6 @@ void gen7_set_surface_mcs_info(struct brw_context *brw,<br>
                                bool is_render_target);<br>
 void gen7_check_surface_setup(uint32_t *surf, bool is_render_target);<br>
 void gen7_init_vtable_surface_functions(struct brw_context *brw);<br>
-void gen7_create_shader_time_surface(struct brw_context *brw,<br>
-                                     uint32_t *out_offset);<br>
<br>
 /* gen7_sol_state.c */<br>
 void gen7_upload_3dstate_so_decl_list(struct brw_context *brw,<br>
diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c<br>
index 9f8c0f5..17fa30d 100644<br>
--- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c<br>
+++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c<br>
@@ -179,7 +179,9 @@ brw_vec4_upload_binding_table(struct brw_context *brw,<br>
    int i;<br>
<br>
    if (INTEL_DEBUG & DEBUG_SHADER_TIME) {<br>
-      gen7_create_shader_time_surface(brw, &stage_state->surf_offset[SURF_INDEX_VEC4_SHADER_TIME]);<br>
+      brw->vtbl.create_raw_surface(<br>
+         brw, brw-><a href="http://shader_time.bo" target="_blank">shader_time.bo</a>, 0, brw->shader_time.bo->size,<br>
+         &stage_state->surf_offset[SURF_INDEX_VEC4_SHADER_TIME], true);<br>
    }<br>
<br>
    /* Skip making a binding table if we don't use textures or pull<br>
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c<br>
index 48f351f..da7dac1 100644<br>
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c<br>
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c<br>
@@ -886,7 +886,9 @@ brw_upload_wm_binding_table(struct brw_context *brw)<br>
    int i;<br>
<br>
    if (INTEL_DEBUG & DEBUG_SHADER_TIME) {<br>
-      gen7_create_shader_time_surface(brw, &brw->wm.base.surf_offset[SURF_INDEX_WM_SHADER_TIME]);<br>
+      brw->vtbl.create_raw_surface(<br>
+         brw, brw-><a href="http://shader_time.bo" target="_blank">shader_time.bo</a>, 0, brw->shader_time.bo->size,<br>
+         &brw->wm.base.surf_offset[SURF_INDEX_WM_SHADER_TIME], true);<br>
    }<br>
<br>
    /* CACHE_NEW_WM_PROG */<br>
diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c<br>
index 8b86387..6f5e670 100644<br>
--- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c<br>
+++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c<br>
@@ -455,23 +455,6 @@ gen7_create_raw_surface(struct brw_context *brw, drm_intel_bo *bo,<br>
                                   true /* rw */);<br>
 }<br>
<br>
-/**<br>
- * Create a surface for shader time.<br>
- */<br>
-void<br>
-gen7_create_shader_time_surface(struct brw_context *brw, uint32_t *out_offset)<br>
-{<br>
-   gen7_emit_buffer_surface_state(brw,<br>
-                                  out_offset,<br>
-                                  brw-><a href="http://shader_time.bo" target="_blank">shader_time.bo</a>,<br>
-                                  0,<br>
-                                  BRW_SURFACEFORMAT_RAW,<br>
-                                  brw->shader_time.bo->size - 1,<br>
-                                  1,<br>
-                                  0 /* mocs */,<br>
-                                  true /* rw */);<br>
-}<br>
-<br>
 static void<br>
 gen7_update_null_renderbuffer_surface(struct brw_context *brw, unsigned unit)<br>
 {<br>
<span class=""><font color="#888888">--<br>
1.8.3.4<br>
<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</font></span></blockquote></div><br></div></div>