[Mesa-dev] [PATCH 03/15] i965/blorp: Emit sample index in SAMPLE_LD message when necessary

[Paul Berry]

On 21 May 2012 18:00, Ian Romanick <idr at freedesktop.org> wrote:

> On 05/11/2012 11:03 AM, Paul Berry wrote:
>
>> This patch modifies the function brw_blorp_blit_program::texel_**fetch()
>> to emit the SI (sample index) argument to the SAMPLE_LD message when
>> reading from a sample index other than zero.
>>
>> Previously we were using the ugly hack of configuring multisampled
>> source surfaces as single-sampled, and accessing sample indices other
>> than zero by swizzling the texture coordinates in the WM program.
>> ---
>>  src/mesa/drivers/dri/i965/brw_**blorp.cpp      |    7 +---
>>  src/mesa/drivers/dri/i965/brw_**blorp_blit.cpp |   45
>> ++++++++++++++++---------
>>  2 files changed, 31 insertions(+), 21 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/**brw_blorp.cpp
>> b/src/mesa/drivers/dri/i965/**brw_blorp.cpp
>> index 8e22511..f6aff44 100644
>> --- a/src/mesa/drivers/dri/i965/**brw_blorp.cpp
>> +++ b/src/mesa/drivers/dri/i965/**brw_blorp.cpp
>> @@ -57,19 +57,16 @@ brw_blorp_surface_info::set(**struct
>> intel_mipmap_tree *mt,
>>                              unsigned int level, unsigned int layer)
>>  {
>>     brw_blorp_mip_info::set(mt, level, layer);
>> +   this->num_samples = mt->num_samples;
>>
>>     if (mt->format == MESA_FORMAT_S8) {
>>        /* The miptree is a W-tiled stencil buffer.  Surface states can't
>> be set
>>         * up for W tiling, so we'll need to use Y tiling and have the WM
>> -       * program swizzle the coordinates.  Furthermore, we need to set
>> up the
>> -       * surface state as single-sampled, because the memory layout of
>> related
>> -       * samples doesn't match between W and Y tiling.
>> +       * program swizzle the coordinates.
>>         */
>>        this->map_stencil_as_y_tiled = true;
>> -      this->num_samples = 0;
>>     } else {
>>        this->map_stencil_as_y_tiled = false;
>> -      this->num_samples = mt->num_samples;
>>     }
>>  }
>>
>> diff --git a/src/mesa/drivers/dri/i965/**brw_blorp_blit.cpp
>> b/src/mesa/drivers/dri/i965/**brw_blorp_blit.cpp
>> index d3ab0ba..e985fad 100644
>> --- a/src/mesa/drivers/dri/i965/**brw_blorp_blit.cpp
>> +++ b/src/mesa/drivers/dri/i965/**brw_blorp_blit.cpp
>> @@ -220,6 +220,8 @@ enum sampler_message_arg
>>     SAMPLER_MESSAGE_ARG_V_FLOAT,
>>     SAMPLER_MESSAGE_ARG_U_INT,
>>     SAMPLER_MESSAGE_ARG_V_INT,
>> +   SAMPLER_MESSAGE_ARG_SI_INT,
>> +   SAMPLER_MESSAGE_ARG_ZERO_INT,
>>  };
>>
>>  /**
>> @@ -435,14 +437,6 @@ brw_blorp_blit_program::**compile(struct
>> brw_context *brw,
>>                                  GLuint *program_size)
>>  {
>>     /* Sanity checks */
>> -   if (key->src_tiled_w) {
>> -      /* If the source image is W tiled, then tex_samples must be 0.
>> -       * Otherwise, after conversion between W and Y tiling, there's no
>> -       * guarantee that the sample index will be 0.
>> -       */
>> -      assert(key->tex_samples == 0);
>> -   }
>> -
>>     if (key->dst_tiled_w) {
>>        /* If the destination image is W tiled, then dst_samples must be 0.
>>         * Otherwise, after conversion between W and Y tiling, there's no
>> @@ -920,13 +914,15 @@ brw_blorp_blit_program::**sample()
>>  void
>>  brw_blorp_blit_program::texel_**fetch()
>>  {
>> -   static const sampler_message_arg args[2] = {
>> +   static const sampler_message_arg args[5] = {
>>        SAMPLER_MESSAGE_ARG_U_INT,
>> -      SAMPLER_MESSAGE_ARG_V_INT
>> +      SAMPLER_MESSAGE_ARG_V_INT,
>> +      SAMPLER_MESSAGE_ARG_ZERO_INT, /* R */
>> +      SAMPLER_MESSAGE_ARG_ZERO_INT, /* LOD */
>> +      SAMPLER_MESSAGE_ARG_SI_INT
>>     };
>>
>> -   assert(s_is_zero);
>> -   texture_lookup(GEN5_SAMPLER_**MESSAGE_SAMPLE_LD, args,
>> ARRAY_SIZE(args));
>> +   texture_lookup(GEN5_SAMPLER_**MESSAGE_SAMPLE_LD, args, s_is_zero ? 2
>> : 5);
>>  }
>>
>>  void
>> @@ -960,6 +956,15 @@ brw_blorp_blit_program::**texture_lookup(GLuint
>> msg_type,
>>        case SAMPLER_MESSAGE_ARG_V_INT:
>>           expand_to_32_bits(Y, mrf);
>>           break;
>> +      case SAMPLER_MESSAGE_ARG_SI_INT:
>> +         if (s_is_zero)
>> +            brw_MOV(&func, mrf, brw_imm_ud(0));
>> +         else
>> +            expand_to_32_bits(S, mrf);
>> +         break;
>>
>
> It seems like up above you've guaranteed that you won't see
> SAMPLER_MESSAGE_ARG_SI_INT when s_is_zero.  Is this extra check necessary?
>

You're certainly right for Gen6.  On Gen7 it'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's grabbing sample
0, we will see SAMPLER_MESSAGE_ARG_SI_INT with s_is_zero=true.

I'll add a comment above the "if (s_is_zero)" line to explain why it's
necessary.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120521/ff3874af/attachment.htm>