<div dir="ltr">On 6 February 2013 23:26, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</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">This is basically a copy and paste of gen7_create_constant_surface, but<br>
with the parameters filled in to offer a simpler interface.<br>
<br>
It will diverge shortly.<br>
<br>
I didn't bother adding it to the vtable for now since shader time is<br>
only exposed on Gen7+.<br>
<br>
NOTE: This is a candidate for the 9.1 branch.<br>
Cc: Eric Anholt <<a href="mailto:eric@anholt.net">eric@anholt.net</a>><br>
Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>><br>
---<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 | 35 +++++++++++++++++++++++<br>
 4 files changed, 39 insertions(+), 6 deletions(-)<br>
<br>
diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h<br>
index adc64e3..7e4fecc 100644<br>
--- a/src/mesa/drivers/dri/i965/brw_state.h<br>
+++ b/src/mesa/drivers/dri/i965/brw_state.h<br>
@@ -215,6 +215,8 @@ 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>
 /* brw_wm_sampler_state.c */<br>
 uint32_t translate_wrap_mode(GLenum wrap, bool using_nearest);<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 4da7eaa..9aa775f 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>
@@ -142,9 +142,7 @@ brw_vs_upload_binding_table(struct brw_context *brw)<br>
    int i;<br>
<br>
    if (INTEL_DEBUG & DEBUG_SHADER_TIME) {<br>
-      intel->vtbl.create_constant_surface(brw, brw-><a href="http://shader_time.bo" target="_blank">shader_time.bo</a>, 0,<br>
-                                          brw->shader_time.bo->size,<br>
-                                          &brw->vs.surf_offset[SURF_INDEX_VS_SHADER_TIME]);<br>
+      gen7_create_shader_time_surface(brw, &brw->vs.surf_offset[SURF_INDEX_VS_SHADER_TIME]);<br>
<br>
       assert(brw->vs.prog_data->num_surfaces <= SURF_INDEX_VS_SHADER_TIME);<br>
       brw->vs.prog_data->num_surfaces = SURF_INDEX_VS_SHADER_TIME;<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 77b7ab3..f0cc9b6 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>
@@ -1470,9 +1470,7 @@ brw_upload_wm_binding_table(struct brw_context *brw)<br>
    int i;<br>
<br>
    if (INTEL_DEBUG & DEBUG_SHADER_TIME) {<br>
-      intel->vtbl.create_constant_surface(brw, brw-><a href="http://shader_time.bo" target="_blank">shader_time.bo</a>, 0,<br>
-                                          brw->shader_time.bo->size,<br>
-                                          &brw->wm.surf_offset[SURF_INDEX_WM_SHADER_TIME]);<br>
+      gen7_create_shader_time_surface(brw, &brw->wm.surf_offset[SURF_INDEX_WM_SHADER_TIME]);<br>
    }<br>
<br>
    /* Might want to calculate nr_surfaces first, to avoid taking up so much<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 179024a..6a4c009 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>
@@ -422,6 +422,41 @@ gen7_create_constant_surface(struct brw_context *brw,<br>
    gen7_check_surface_setup(surf, false /* is_render_target */);<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>
+   struct intel_context *intel = &brw->intel;<br>
+   const int w = brw->shader_time.bo->size - 1;<br>
+<br>
+   uint32_t *surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,<br>
+                                    8 * 4, 32, out_offset);<br></blockquote><div><br>gen7_create_constant_surface has a "memset(surf, 0, 8 * 4);" here.  I think we need to do that in this function too, or we risk setting some MBZ fields to garbage.<br>
 </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+   surf[0] = BRW_SURFACE_BUFFER << BRW_SURFACE_TYPE_SHIFT |<br>
+             BRW_SURFACEFORMAT_R32G32B32A32_FLOAT << BRW_SURFACE_FORMAT_SHIFT |<br>
+             BRW_SURFACE_RC_READ_WRITE;<br>
+<br>
+   surf[1] = brw->shader_time.bo->offset; /* reloc */<br>
+<br>
+   surf[2] = SET_FIELD(w & 0x7f, GEN7_SURFACE_WIDTH) |<br>
+             SET_FIELD((w >> 7) & 0x1fff, GEN7_SURFACE_HEIGHT);<br>
+   surf[3] = SET_FIELD((w >> 20) & 0x7f, BRW_SURFACE_DEPTH) |<br>
+             (16 - 1); /* stride between samples */<br></blockquote><div><br></div><div>You might want to include a brief comment here explaining why you're not setting surf[7] here on Haswell.  (We talked about it in person and I think you're right that it's not necessary, but since the HW docs aren't spectacularly clear on this point it would be nice to have some explanatory text).<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>
+   /* Emit relocation to surface contents.  Section 5.1.1 of the gen4<br>
+    * bspec ("Data Cache") says that the data cache does not exist as<br>
+    * a separate cache and is just the sampler cache.<br>
+    */<br>
+   drm_intel_bo_emit_reloc(intel-><a href="http://batch.bo" target="_blank">batch.bo</a>,<br>
+                          *out_offset + 4,<br>
+                          brw-><a href="http://shader_time.bo" target="_blank">shader_time.bo</a>, 0,<br>
+                          I915_GEM_DOMAIN_SAMPLER, 0);<br>
+<br>
+   gen7_check_surface_setup(surf, false /* is_render_target */);<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.1.1<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><br></div><div class="gmail_extra">With that fixed, this patch is:<br><br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br></div>
</div>