<div class="gmail_quote">On 21 May 2012 18:00, Ian Romanick <span dir="ltr">&lt;<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On 05/11/2012 11:03 AM, Paul Berry wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This patch modifies the function brw_blorp_blit_program::texel_<u></u>fetch()<br>
to emit the SI (sample index) argument to the SAMPLE_LD message when<br>
reading from a sample index other than zero.<br>
<br>
Previously we were using the ugly hack of configuring multisampled<br>
source surfaces as single-sampled, and accessing sample indices other<br>
than zero by swizzling the texture coordinates in the WM program.<br>
---<br>
  src/mesa/drivers/dri/i965/brw_<u></u>blorp.cpp      |    7 +---<br>
  src/mesa/drivers/dri/i965/brw_<u></u>blorp_blit.cpp |   45 ++++++++++++++++---------<br>
  2 files changed, 31 insertions(+), 21 deletions(-)<br>
<br>
diff --git a/src/mesa/drivers/dri/i965/<u></u>brw_blorp.cpp b/src/mesa/drivers/dri/i965/<u></u>brw_blorp.cpp<br>
index 8e22511..f6aff44 100644<br>
--- a/src/mesa/drivers/dri/i965/<u></u>brw_blorp.cpp<br>
+++ b/src/mesa/drivers/dri/i965/<u></u>brw_blorp.cpp<br>
@@ -57,19 +57,16 @@ brw_blorp_surface_info::set(<u></u>struct intel_mipmap_tree *mt,<br>
                              unsigned int level, unsigned int layer)<br>
  {<br>
     brw_blorp_mip_info::set(mt, level, layer);<br>
+   this-&gt;num_samples = mt-&gt;num_samples;<br>
<br>
     if (mt-&gt;format == MESA_FORMAT_S8) {<br>
        /* The miptree is a W-tiled stencil buffer.  Surface states can&#39;t be set<br>
         * up for W tiling, so we&#39;ll need to use Y tiling and have the WM<br>
-       * program swizzle the coordinates.  Furthermore, we need to set up the<br>
-       * surface state as single-sampled, because the memory layout of related<br>
-       * samples doesn&#39;t match between W and Y tiling.<br>
+       * program swizzle the coordinates.<br>
         */<br>
        this-&gt;map_stencil_as_y_tiled = true;<br>
-      this-&gt;num_samples = 0;<br>
     } else {<br>
        this-&gt;map_stencil_as_y_tiled = false;<br>
-      this-&gt;num_samples = mt-&gt;num_samples;<br>
     }<br>
  }<br>
<br>
diff --git a/src/mesa/drivers/dri/i965/<u></u>brw_blorp_blit.cpp b/src/mesa/drivers/dri/i965/<u></u>brw_blorp_blit.cpp<br>
index d3ab0ba..e985fad 100644<br>
--- a/src/mesa/drivers/dri/i965/<u></u>brw_blorp_blit.cpp<br>
+++ b/src/mesa/drivers/dri/i965/<u></u>brw_blorp_blit.cpp<br>
@@ -220,6 +220,8 @@ enum sampler_message_arg<br>
     SAMPLER_MESSAGE_ARG_V_FLOAT,<br>
     SAMPLER_MESSAGE_ARG_U_INT,<br>
     SAMPLER_MESSAGE_ARG_V_INT,<br>
+   SAMPLER_MESSAGE_ARG_SI_INT,<br>
+   SAMPLER_MESSAGE_ARG_ZERO_INT,<br>
  };<br>
<br>
  /**<br>
@@ -435,14 +437,6 @@ brw_blorp_blit_program::<u></u>compile(struct brw_context *brw,<br>
                                  GLuint *program_size)<br>
  {<br>
     /* Sanity checks */<br>
-   if (key-&gt;src_tiled_w) {<br>
-      /* If the source image is W tiled, then tex_samples must be 0.<br>
-       * Otherwise, after conversion between W and Y tiling, there&#39;s no<br>
-       * guarantee that the sample index will be 0.<br>
-       */<br>
-      assert(key-&gt;tex_samples == 0);<br>
-   }<br>
-<br>
     if (key-&gt;dst_tiled_w) {<br>
        /* If the destination image is W tiled, then dst_samples must be 0.<br>
         * Otherwise, after conversion between W and Y tiling, there&#39;s no<br>
@@ -920,13 +914,15 @@ brw_blorp_blit_program::<u></u>sample()<br>
  void<br>
  brw_blorp_blit_program::texel_<u></u>fetch()<br>
  {<br>
-   static const sampler_message_arg args[2] = {<br>
+   static const sampler_message_arg args[5] = {<br>
        SAMPLER_MESSAGE_ARG_U_INT,<br>
-      SAMPLER_MESSAGE_ARG_V_INT<br>
+      SAMPLER_MESSAGE_ARG_V_INT,<br>
+      SAMPLER_MESSAGE_ARG_ZERO_INT, /* R */<br>
+      SAMPLER_MESSAGE_ARG_ZERO_INT, /* LOD */<br>
+      SAMPLER_MESSAGE_ARG_SI_INT<br>
     };<br>
<br>
-   assert(s_is_zero);<br>
-   texture_lookup(GEN5_SAMPLER_<u></u>MESSAGE_SAMPLE_LD, args, ARRAY_SIZE(args));<br>
+   texture_lookup(GEN5_SAMPLER_<u></u>MESSAGE_SAMPLE_LD, args, s_is_zero ? 2 : 5);<br>
  }<br>
<br>
  void<br>
@@ -960,6 +956,15 @@ brw_blorp_blit_program::<u></u>texture_lookup(GLuint msg_type,<br>
        case SAMPLER_MESSAGE_ARG_V_INT:<br>
           expand_to_32_bits(Y, mrf);<br>
           break;<br>
+      case SAMPLER_MESSAGE_ARG_SI_INT:<br>
+         if (s_is_zero)<br>
+            brw_MOV(&amp;func, mrf, brw_imm_ud(0));<br>
+         else<br>
+            expand_to_32_bits(S, mrf);<br>
+         break;<br>
</blockquote>
<br></div></div>
It seems like up above you&#39;ve guaranteed that you won&#39;t see SAMPLER_MESSAGE_ARG_SI_INT when s_is_zero.  Is this extra check necessary?<br></blockquote><div><br></div><div>You&#39;re certainly right for Gen6.  On Gen7 it&#39;s necessary, though, because on Gen7, when downsampling, the fragment shader has to grab all N samples (using N texel fetches) and blend them manually.  When it&#39;s grabbing sample 0, we will see SAMPLER_MESSAGE_ARG_SI_INT with s_is_zero=true.</div>
<div><br></div><div>I&#39;ll add a comment above the &quot;if (s_is_zero)&quot; line to explain why it&#39;s necessary.</div></div>