[Mesa-dev] [PATCH 2/2] i965: Allow blorp to make decisions about the formats that it supports where it can

Chris Forbes chrisf at ijw.co.nz
Thu Nov 14 17:18:19 PST 2013


Why does this affect that piglit test?

-- Chris

On Fri, Nov 15, 2013 at 2:01 PM, Mark Mueller <markkmueller at gmail.com> wrote:
> fixed subject line
>
>
> On Thu, Nov 14, 2013 at 4:15 PM, Mark Mueller <markkmueller at gmail.com>
> wrote:
>>
>> This patch consolidates the decision about formats that blorp_blt does and
>> does not support to blorp instead of leaving that decision to callers.
>> This
>> opens the door to adding more functionality to blorp, including support
>> for
>> using GPU acceleration of processing and loading textures.
>>
>> This patch fixes piglit generated_tests/spec/glsl-1.50/compiler/-
>> built-in-functions/op-div-mat4x3-float.geom.
>>
>>
>> Signed-off-by: Mark Mueller <MarkKMueller at gmail.com>
>> ---
>>  src/mesa/drivers/dri/i965/brw_blorp.h             |   8 +-
>>  src/mesa/drivers/dri/i965/brw_blorp_blit.cpp      | 149
>> ++++++++++++++--------
>>  src/mesa/drivers/dri/i965/brw_state.h             |   5 +-
>>  src/mesa/drivers/dri/i965/brw_surface_formats.c   |  26 ++--
>>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  |  12 +-
>>  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c |   4 +-
>>  6 files changed, 131 insertions(+), 73 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h
>> b/src/mesa/drivers/dri/i965/brw_blorp.h
>> index 85bf099..5344851 100644
>> --- a/src/mesa/drivers/dri/i965/brw_blorp.h
>> +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
>> @@ -34,8 +34,7 @@ struct brw_context;
>>  extern "C" {
>>  #endif
>>
>> -void
>> -brw_blorp_blit_miptrees(struct brw_context *brw,
>> +bool brw_blorp_blit_miptrees(struct brw_context *brw,
>>                          struct intel_mipmap_tree *src_mt,
>>                          unsigned src_level, unsigned src_layer,
>>                          struct intel_mipmap_tree *dst_mt,
>> @@ -356,8 +355,13 @@ public:
>>     virtual uint32_t get_wm_prog(struct brw_context *brw,
>>                                  brw_blorp_prog_data **prog_data) const;
>>
>> +   bool is_valid() {
>> +       return valid_parameters;
>> +   }
>> +
>>  private:
>>     brw_blorp_blit_prog_key wm_prog_key;
>> +   bool valid_parameters;
>>  };
>>
>>  /**
>> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
>> b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
>> index d54b926..00461a4 100644
>> --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
>> @@ -123,7 +123,7 @@ find_miptree(GLbitfield buffer_bit, struct
>> intel_renderbuffer *irb)
>>     return mt;
>>  }
>>
>> -void
>> +bool
>>  brw_blorp_blit_miptrees(struct brw_context *brw,
>>                          struct intel_mipmap_tree *src_mt,
>>                          unsigned src_level, unsigned src_layer,
>> @@ -135,16 +135,6 @@ brw_blorp_blit_miptrees(struct brw_context *brw,
>>                          float dst_x1, float dst_y1,
>>                          GLenum filter, bool mirror_x, bool mirror_y)
>>  {
>> -   /* Get ready to blit.  This includes depth resolving the src and dst
>> -    * buffers if necessary.  Note: it's not necessary to do a color
>> resolve on
>> -    * the destination buffer because we use the standard render path to
>> render
>> -    * to destination color buffers, and the standard render path is
>> -    * fast-color-aware.
>> -    */
>> -   intel_miptree_resolve_color(brw, src_mt);
>> -   intel_miptree_slice_resolve_depth(brw, src_mt, src_level, src_layer);
>> -   intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level, dst_layer);
>> -
>>     DBG("%s from %s mt %p %d %d (%f,%f) (%f,%f)"
>>         "to %s mt %p %d %d (%f,%f) (%f,%f) (flip %d,%d)\n",
>>         __FUNCTION__,
>> @@ -162,12 +152,28 @@ brw_blorp_blit_miptrees(struct brw_context *brw,
>>                                  dst_x0, dst_y0,
>>                                  dst_x1, dst_y1,
>>                                  filter, mirror_x, mirror_y);
>> +
>> +   if (!params.is_valid()) {
>> +       return false;
>> +   }
>> +
>> +   /* Get ready to blit.  This includes depth resolving the src and dst
>> +    * buffers if necessary.  Note: it's not necessary to do a color
>> resolve on
>> +    * the destination buffer because we use the standard render path to
>> render
>> +    * to destination color buffers, and the standard render path is
>> +    * fast-color-aware.
>> +    */
>> +   intel_miptree_resolve_color(brw, src_mt);
>> +   intel_miptree_slice_resolve_depth(brw, src_mt, src_level, src_layer);
>> +   intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level, dst_layer);
>> +
>>     brw_blorp_exec(brw, &params);
>>
>>     intel_miptree_slice_set_needs_hiz_resolve(dst_mt, dst_level,
>> dst_layer);
>> +   return true;
>>  }
>>
>> -static void
>> +static bool
>>  do_blorp_blit(struct brw_context *brw, GLbitfield buffer_bit,
>>                struct intel_renderbuffer *src_irb,
>>                struct intel_renderbuffer *dst_irb,
>> @@ -180,14 +186,17 @@ do_blorp_blit(struct brw_context *brw, GLbitfield
>> buffer_bit,
>>     struct intel_mipmap_tree *dst_mt = find_miptree(buffer_bit, dst_irb);
>>
>>     /* Do the blit */
>> -   brw_blorp_blit_miptrees(brw,
>> -                           src_mt, src_irb->mt_level, src_irb->mt_layer,
>> -                           dst_mt, dst_irb->mt_level, dst_irb->mt_layer,
>> -                           srcX0, srcY0, srcX1, srcY1,
>> -                           dstX0, dstY0, dstX1, dstY1,
>> -                           filter, mirror_x, mirror_y);
>> +   if (!brw_blorp_blit_miptrees(brw,
>> +                                src_mt, src_irb->mt_level,
>> src_irb->mt_layer,
>> +                                dst_mt, dst_irb->mt_level,
>> dst_irb->mt_layer,
>> +                                srcX0, srcY0, srcX1, srcY1,
>> +                                dstX0, dstY0, dstX1, dstY1,
>> +                                filter, mirror_x, mirror_y)) {
>> +      return false;
>> +   }
>>
>>     intel_renderbuffer_set_needs_downsample(dst_irb);
>> +   return true;
>>  }
>>
>>  static bool
>> @@ -293,10 +302,13 @@ try_blorp_blit(struct brw_context *brw,
>>        }
>>        for (unsigned i = 0; i < ctx->DrawBuffer->_NumColorDrawBuffers;
>> ++i) {
>>           dst_irb =
>> intel_renderbuffer(ctx->DrawBuffer->_ColorDrawBuffers[i]);
>> -        if (dst_irb)
>> -            do_blorp_blit(brw, buffer_bit, src_irb, dst_irb, srcX0,
>> srcY0,
>> -                          srcX1, srcY1, dstX0, dstY0, dstX1, dstY1,
>> -                          filter, mirror_x, mirror_y);
>> +         if (dst_irb) {
>> +            if (!do_blorp_blit(brw, buffer_bit, src_irb, dst_irb, srcX0,
>> srcY0,
>> +                               srcX1, srcY1, dstX0, dstY0, dstX1, dstY1,
>> +                               filter, mirror_x, mirror_y)) {
>> +               return false;
>> +            }
>> +         }
>>        }
>>        break;
>>     case GL_DEPTH_BUFFER_BIT:
>> @@ -306,9 +318,11 @@ try_blorp_blit(struct brw_context *brw,
>>
>> intel_renderbuffer(draw_fb->Attachment[BUFFER_DEPTH].Renderbuffer);
>>        if (!formats_match(buffer_bit, src_irb, dst_irb))
>>           return false;
>> -      do_blorp_blit(brw, buffer_bit, src_irb, dst_irb, srcX0, srcY0,
>> -                    srcX1, srcY1, dstX0, dstY0, dstX1, dstY1,
>> -                    filter, mirror_x, mirror_y);
>> +      if (!do_blorp_blit(brw, buffer_bit, src_irb, dst_irb, srcX0, srcY0,
>> +                         srcX1, srcY1, dstX0, dstY0, dstX1, dstY1,
>> +                         filter, mirror_x, mirror_y)) {
>> +         return false;
>> +      }
>>        break;
>>     case GL_STENCIL_BUFFER_BIT:
>>        src_irb =
>> @@ -317,9 +331,11 @@ try_blorp_blit(struct brw_context *brw,
>>
>> intel_renderbuffer(draw_fb->Attachment[BUFFER_STENCIL].Renderbuffer);
>>        if (!formats_match(buffer_bit, src_irb, dst_irb))
>>           return false;
>> -      do_blorp_blit(brw, buffer_bit, src_irb, dst_irb, srcX0, srcY0,
>> -                    srcX1, srcY1, dstX0, dstY0, dstX1, dstY1,
>> -                    filter, mirror_x, mirror_y);
>> +      if (!do_blorp_blit(brw, buffer_bit, src_irb, dst_irb, srcX0, srcY0,
>> +                         srcX1, srcY1, dstX0, dstY0, dstX1, dstY1,
>> +                         filter, mirror_x, mirror_y)) {
>> +         return false;
>> +      }
>>        break;
>>     default:
>>        assert(false);
>> @@ -349,27 +365,11 @@ brw_blorp_copytexsubimage(struct brw_context *brw,
>>     struct intel_mipmap_tree *src_mt = src_irb->mt;
>>     struct intel_mipmap_tree *dst_mt = intel_image->mt;
>>
>> -   /* BLORP is not supported before Gen6. */
>> -   if (brw->gen < 6)
>> -      return false;
>> -
>>     if (_mesa_get_format_base_format(src_mt->format) !=
>>         _mesa_get_format_base_format(dst_mt->format)) {
>>        return false;
>>     }
>>
>> -   /* We can't handle format conversions between Z24 and other formats
>> since
>> -    * we have to lie about the surface format.  See the comments in
>> -    * brw_blorp_surface_info::set().
>> -    */
>> -   if ((src_mt->format == MESA_FORMAT_X8_Z24) !=
>> -       (dst_mt->format == MESA_FORMAT_X8_Z24)) {
>> -      return false;
>> -   }
>> -
>> -   if (!brw->format_supported_as_render_target[dst_mt->format])
>> -      return false;
>> -
>>     /* Source clipping shouldn't be necessary, since copytexsubimage (in
>>      * src/mesa/main/teximage.c) calls _mesa_clip_copytexsubimage() which
>>      * takes care of it.
>> @@ -396,12 +396,14 @@ brw_blorp_copytexsubimage(struct brw_context *brw,
>>        mirror_y = true;
>>     }
>>
>> -   brw_blorp_blit_miptrees(brw,
>> -                           src_mt, src_irb->mt_level, src_irb->mt_layer,
>> -                           dst_mt, dst_image->Level, dst_image->Face +
>> slice,
>> -                           srcX0, srcY0, srcX1, srcY1,
>> -                           dstX0, dstY0, dstX1, dstY1,
>> -                           GL_NEAREST, false, mirror_y);
>> +   if (!brw_blorp_blit_miptrees(brw,
>> +                                src_mt, src_irb->mt_level,
>> src_irb->mt_layer,
>> +                                dst_mt, dst_image->Level, dst_image->Face
>> + slice,
>> +                                srcX0, srcY0, srcX1, srcY1,
>> +                                dstX0, dstY0, dstX1, dstY1,
>> +                                GL_NEAREST, false, mirror_y)) {
>> +      return false;
>> +   }
>>
>>     /* If we're copying to a packed depth stencil texture and the source
>>      * framebuffer has separate stencil, we need to also copy the stencil
>> data
>> @@ -2069,10 +2071,52 @@
>> brw_blorp_blit_params::brw_blorp_blit_params(struct brw_context *brw,
>>                                               GLfloat dst_x1, GLfloat
>> dst_y1,
>>                                               GLenum filter,
>>                                               bool mirror_x, bool
>> mirror_y)
>> +    : valid_parameters(false)
>>  {
>>     struct gl_context *ctx = &brw->ctx;
>> -   const struct gl_framebuffer *read_fb = ctx->ReadBuffer;
>>
>> +   /* BLORP is not supported before Gen6. */
>> +   if (brw->gen < 6)
>> +      return;
>> +
>> +   if (MESA_FORMAT_NONE == src_mt->format || MESA_FORMAT_COUNT <=
>> src_mt->format) {
>> +      perf_debug("%s: Blorp doesn't yet support provided source
>> format.\n", __FUNCTION__);
>> +      return;
>> +   }
>> +
>> +   /* We can't handle format conversions between Z24 and other formats
>> since
>> +    * we have to lie about the surface format.  See the comments in
>> +    * brw_blorp_surface_info::set().
>> +    */
>> +   if ((src_mt->format == MESA_FORMAT_X8_Z24) !=
>> +       (dst_mt->format == MESA_FORMAT_X8_Z24)) {
>> +      return;
>> +   }
>> +
>> +   /* blorp lies about some formats to placate the GPU. */
>> +   const GLint srcFormatTranslated = translate_tex_format(brw,
>> src_mt->format, 0, true);
>> +   const GLint dstFormatTranslated = translate_tex_format(brw,
>> dst_mt->format, 0, true);
>> +   if (0 == srcFormatTranslated || 0 == dstFormatTranslated) {
>> +      perf_debug("%s: compatible brw_surface format not found for source
>> or target "
>> +                  "texture mip tree. Source: %s Target: %s\n",
>> +                  __FUNCTION__, _mesa_get_format_name(src_mt->format),
>> +                  _mesa_get_format_name(dst_mt->format));
>> +      return;
>> +   }
>> +
>> +   if (!brw->format_supported_as_render_target[dst_mt->format]) {
>> +      perf_debug("%s: The translated target miptree format is not
>> supported as a render target: %s.\n",
>> +                  __FUNCTION__, _mesa_get_format_name(dst_mt->format));
>> +      return;
>> +   }
>> +
>> +   if (!brw_format_for_sampling(brw, srcFormatTranslated)) {
>> +      perf_debug("%s: The translated source miptree format is not
>> supported for sampling: %s.\n",
>> +                  __FUNCTION__, _mesa_get_format_name(src_mt->format));
>> +      return;
>> +   }
>> +
>> +   const struct gl_framebuffer *read_fb = ctx->ReadBuffer;
>>     src.set(brw, src_mt, src_level, src_layer, false);
>>     dst.set(brw, dst_mt, dst_level, dst_layer, true);
>>
>> @@ -2123,7 +2167,7 @@ brw_blorp_blit_params::brw_blorp_blit_params(struct
>> brw_context *brw,
>>        wm_prog_key.texture_data_type = BRW_REGISTER_TYPE_D;
>>        break;
>>     default:
>> -      assert(!"Unrecognized blorp format");
>> +      _mesa_debug(&brw->ctx, "Unrecognized blorp format");
>>        break;
>>     }
>>
>> @@ -2329,6 +2373,7 @@ brw_blorp_blit_params::brw_blorp_blit_params(struct
>> brw_context *brw,
>>        src.x_offset *= 2;
>>        src.y_offset /= 2;
>>     }
>> +   valid_parameters = true;
>>  }
>>
>>  uint32_t
>> diff --git a/src/mesa/drivers/dri/i965/brw_state.h
>> b/src/mesa/drivers/dri/i965/brw_state.h
>> index 471f1da..49b02b6 100644
>> --- a/src/mesa/drivers/dri/i965/brw_state.h
>> +++ b/src/mesa/drivers/dri/i965/brw_state.h
>> @@ -188,13 +188,14 @@ uint32_t brw_get_surface_tiling_bits(uint32_t
>> tiling);
>>  uint32_t brw_get_surface_num_multisamples(unsigned num_samples);
>>
>>  uint32_t brw_format_for_mesa_format(gl_format mesa_format);
>> +bool brw_format_for_sampling(struct brw_context *brw, const unsigned
>> brw_surface_sel);
>>
>>  GLuint translate_tex_target(GLenum target);
>>
>>  GLuint translate_tex_format(struct brw_context *brw,
>>                              gl_format mesa_format,
>> -                           GLenum depth_mode,
>> -                           GLenum srgb_decode);
>> +                            GLenum srgb_decode,
>> +                            bool for_render);
>>
>>  int brw_get_texture_swizzle(const struct gl_context *ctx,
>>                              const struct gl_texture_object *t);
>> diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c
>> b/src/mesa/drivers/dri/i965/brw_surface_formats.c
>> index 7afca79..c9715a7 100644
>> --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
>> +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
>> @@ -306,6 +306,13 @@ const struct surface_format_info surface_formats[] =
>> {
>>     SF( x,  x,  x,  x,  x,  x,  x,  x,  x, BRW_SURFACEFORMAT_R8G8B8_UINT)
>>     SF( x,  x,  x,  x,  x,  x,  x,  x,  x, BRW_SURFACEFORMAT_R8G8B8_SINT)
>>  };
>> +
>> +bool
>> +brw_format_for_sampling(struct brw_context *brw, const unsigned
>> brw_surface_sel)
>> +{
>> +   return (brw_surface_sel && surface_formats[brw_surface_sel].sampling
>> <= brw->gen * 10);
>> +}
>> +
>>  #undef x
>>  #undef Y
>>
>> @@ -686,24 +693,26 @@ brw_render_target_supported(struct brw_context *brw,
>>  GLuint
>>  translate_tex_format(struct brw_context *brw,
>>                       gl_format mesa_format,
>> -                    GLenum depth_mode,
>> -                    GLenum srgb_decode)
>> +                     GLenum srgb_decode,
>> +                     bool for_render)
>>  {
>> -   struct gl_context *ctx = &brw->ctx;
>>     if (srgb_decode == GL_SKIP_DECODE_EXT)
>>        mesa_format = _mesa_get_srgb_format_linear(mesa_format);
>>
>>     switch( mesa_format ) {
>>
>> +   case MESA_FORMAT_S8:
>> +      return BRW_SURFACEFORMAT_R8_UNORM;
>> +
>>     case MESA_FORMAT_Z16:
>> -      return BRW_SURFACEFORMAT_I16_UNORM;
>> +      return for_render ? BRW_SURFACEFORMAT_R16_UNORM :
>> BRW_SURFACEFORMAT_I16_UNORM;
>>
>>     case MESA_FORMAT_S8_Z24:
>>     case MESA_FORMAT_X8_Z24:
>> -      return BRW_SURFACEFORMAT_I24X8_UNORM;
>> +      return for_render ? BRW_SURFACEFORMAT_B8G8R8A8_UNORM :
>> BRW_SURFACEFORMAT_I24X8_UNORM;
>>
>>     case MESA_FORMAT_Z32_FLOAT:
>> -      return BRW_SURFACEFORMAT_I32_FLOAT;
>> +      return for_render ? BRW_SURFACEFORMAT_R32_FLOAT :
>> BRW_SURFACEFORMAT_I32_FLOAT;
>>
>>     case MESA_FORMAT_Z32_FLOAT_X24S8:
>>        return BRW_SURFACEFORMAT_R32G32_FLOAT;
>> @@ -712,7 +721,7 @@ translate_tex_format(struct brw_context *brw,
>>        /* The value of this BRW_SURFACEFORMAT is 0, which tricks the
>>         * assertion below.
>>         */
>> -      return BRW_SURFACEFORMAT_R32G32B32A32_FLOAT;
>> +      return for_render ? BRW_SURFACEFORMAT_B8G8R8A8_UNORM :
>> BRW_SURFACEFORMAT_R32G32B32A32_FLOAT;
>>
>>     case MESA_FORMAT_SRGB_DXT1:
>>        if (brw->gen == 4 && !brw->is_g4x) {
>> @@ -720,13 +729,12 @@ translate_tex_format(struct brw_context *brw,
>>            * skipping SRGB decode.  It's not worth not supporting sRGB in
>>            * general to prevent this.
>>            */
>> +         struct gl_context *ctx = &brw->ctx;
>>           WARN_ONCE(true, "Demoting sRGB DXT1 texture to non-sRGB\n");
>>           mesa_format = MESA_FORMAT_RGB_DXT1;
>>        }
>> -      return brw_format_for_mesa_format(mesa_format);
>>
>>     default:
>> -      assert(brw_format_for_mesa_format(mesa_format) != 0);
>>        return brw_format_for_mesa_format(mesa_format);
>>     }
>>  }
>> 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 662c975..4873353 100644
>> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> @@ -285,13 +285,13 @@ brw_update_texture_surface(struct gl_context *ctx,
>>     (void) for_gather;   /* no w/a to apply for this gen */
>>
>>     surf[0] = (translate_tex_target(tObj->Target) <<
>> BRW_SURFACE_TYPE_SHIFT |
>> -             BRW_SURFACE_MIPMAPLAYOUT_BELOW <<
>> BRW_SURFACE_MIPLAYOUT_SHIFT |
>> -             BRW_SURFACE_CUBEFACE_ENABLES |
>> -             (translate_tex_format(brw,
>> +              BRW_SURFACE_MIPMAPLAYOUT_BELOW <<
>> BRW_SURFACE_MIPLAYOUT_SHIFT |
>> +              BRW_SURFACE_CUBEFACE_ENABLES |
>> +              (translate_tex_format(brw,
>>                                      mt->format,
>> -                                   tObj->DepthMode,
>> -                                   sampler->sRGBDecode) <<
>> -              BRW_SURFACE_FORMAT_SHIFT));
>> +                                    sampler->sRGBDecode,
>> +                                    false) <<
>> +               BRW_SURFACE_FORMAT_SHIFT));
>>
>>     surf[1] = intelObj->mt->region->bo->offset + intelObj->mt->offset; /*
>> reloc */
>>
>> 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 3f4817d..13065ec 100644
>> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
>> @@ -291,8 +291,8 @@ gen7_update_texture_surface(struct gl_context *ctx,
>>
>>     uint32_t tex_format = translate_tex_format(brw,
>>                                                mt->format,
>> -                                              tObj->DepthMode,
>> -                                              sampler->sRGBDecode);
>> +                                              sampler->sRGBDecode,
>> +                                              false);
>>
>>     if (for_gather && tex_format == BRW_SURFACEFORMAT_R32G32_FLOAT)
>>        tex_format = BRW_SURFACEFORMAT_R32G32_FLOAT_LD;
>> --
>> 1.8.3.1
>>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>


More information about the mesa-dev mailing list