[Mesa-dev] [PATCH 5/7] util/blitter (and friends): generate appropriate SVIEW decls

Marek Olšák maraeo at gmail.com
Fri Jun 12 14:23:24 PDT 2015


Sounds good to me.

Marek

On Fri, Jun 12, 2015 at 10:38 PM, Rob Clark <robdclark at gmail.com> wrote:
> On Fri, Jun 12, 2015 at 9:47 AM, Rob Clark <robdclark at gmail.com> wrote:
>> On Thu, Jun 11, 2015 at 8:36 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>>> Am 12.06.2015 um 02:02 schrieb Rob Clark:
>>>> On Thu, Jun 11, 2015 at 5:47 PM, Brian Paul <brianp at vmware.com> wrote:
>>>>> On 06/11/2015 02:38 PM, Rob Clark wrote:
>>>>>>
>>>>>> From: Rob Clark <robclark at freedesktop.org>
>>>>>>
>>>>>> Some hardware needs to know the sampler type.  Update the blit related
>>>>>> shaders to include SVIEW decl.
>>>>>>
>>>>>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>>>>>> ---
>>>>>>   src/gallium/auxiliary/util/u_blit.c           | 35 +++++++++---
>>>>>>   src/gallium/auxiliary/util/u_blitter.c        | 53 +++++++++++++-----
>>>>>>   src/gallium/auxiliary/util/u_simple_shaders.c | 78
>>>>>> ++++++++++++++++++++-------
>>>>>>   src/gallium/auxiliary/util/u_simple_shaders.h | 16 +++---
>>>>>>   src/gallium/auxiliary/util/u_tests.c          |  3 +-
>>>>>>   src/gallium/tests/trivial/quad-tex.c          |  2 +-
>>>>>>   6 files changed, 140 insertions(+), 47 deletions(-)
>>>>>>
>>>>>> diff --git a/src/gallium/auxiliary/util/u_blit.c
>>>>>> b/src/gallium/auxiliary/util/u_blit.c
>>>>>> index 3f3b5fe..e3f3055 100644
>>>>>> --- a/src/gallium/auxiliary/util/u_blit.c
>>>>>> +++ b/src/gallium/auxiliary/util/u_blit.c
>>>>>> @@ -65,7 +65,7 @@ struct blit_state
>>>>>>      struct pipe_vertex_element velem[2];
>>>>>>
>>>>>>      void *vs;
>>>>>> -   void *fs[PIPE_MAX_TEXTURE_TYPES][TGSI_WRITEMASK_XYZW + 1];
>>>>>> +   void *fs[PIPE_MAX_TEXTURE_TYPES][TGSI_WRITEMASK_XYZW + 1][3];
>>>>>>
>>>>>>      struct pipe_resource *vbuf;  /**< quad vertices */
>>>>>>      unsigned vbuf_slot;
>>>>>> @@ -135,15 +135,17 @@ void
>>>>>>   util_destroy_blit(struct blit_state *ctx)
>>>>>>   {
>>>>>>      struct pipe_context *pipe = ctx->pipe;
>>>>>> -   unsigned i, j;
>>>>>> +   unsigned i, j, k;
>>>>>>
>>>>>>      if (ctx->vs)
>>>>>>         pipe->delete_vs_state(pipe, ctx->vs);
>>>>>>
>>>>>>      for (i = 0; i < Elements(ctx->fs); i++) {
>>>>>>         for (j = 0; j < Elements(ctx->fs[i]); j++) {
>>>>>> -         if (ctx->fs[i][j])
>>>>>> -            pipe->delete_fs_state(pipe, ctx->fs[i][j]);
>>>>>> +         for (k = 0; k < Elements(ctx->fs[i][j]); k++) {
>>>>>> +            if (ctx->fs[i][j][k])
>>>>>> +               pipe->delete_fs_state(pipe, ctx->fs[i][j][k]);
>>>>>> +         }
>>>>>>         }
>>>>>>      }
>>>>>>
>>>>>> @@ -158,18 +160,34 @@ util_destroy_blit(struct blit_state *ctx)
>>>>>>    */
>>>>>>   static INLINE void
>>>>>>   set_fragment_shader(struct blit_state *ctx, uint writemask,
>>>>>> +                    enum pipe_format format,
>>>>>>                       enum pipe_texture_target pipe_tex)
>>>>>>   {
>>>>>> -   if (!ctx->fs[pipe_tex][writemask]) {
>>>>>> +   enum tgsi_return_type stype;
>>>>>> +   unsigned idx;
>>>>>> +
>>>>>> +   if (util_format_is_pure_uint(format)) {
>>>>>> +      stype = TGSI_RETURN_TYPE_UINT;
>>>>>> +      idx = 0;
>>>>>> +   } else if (util_format_is_pure_sint(format)) {
>>>>>> +      stype = TGSI_RETURN_TYPE_SINT;
>>>>>> +      idx = 1;
>>>>>> +   } else {
>>>>>> +      stype = TGSI_RETURN_TYPE_FLOAT;
>>>>>> +      idx = 2;
>>>>>> +   }
>>>>>> +
>>>>>> +   if (!ctx->fs[pipe_tex][writemask][idx]) {
>>>>>>         unsigned tgsi_tex = util_pipe_tex_to_tgsi_tex(pipe_tex, 0);
>>>>>>
>>>>>> -      ctx->fs[pipe_tex][writemask] =
>>>>>> +      ctx->fs[pipe_tex][writemask][idx] =
>>>>>>            util_make_fragment_tex_shader_writemask(ctx->pipe, tgsi_tex,
>>>>>>
>>>>>> TGSI_INTERPOLATE_LINEAR,
>>>>>> -                                                 writemask);
>>>>>> +                                                 writemask,
>>>>>> +                                                 stype);
>>>>>>      }
>>>>>>
>>>>>> -   cso_set_fragment_shader_handle(ctx->cso,
>>>>>> ctx->fs[pipe_tex][writemask]);
>>>>>> +   cso_set_fragment_shader_handle(ctx->cso,
>>>>>> ctx->fs[pipe_tex][writemask][idx]);
>>>>>>   }
>>>>>>
>>>>>>
>>>>>> @@ -571,6 +589,7 @@ util_blit_pixels_tex(struct blit_state *ctx,
>>>>>>
>>>>>>      /* shaders */
>>>>>>      set_fragment_shader(ctx, TGSI_WRITEMASK_XYZW,
>>>>>> +                       src_sampler_view->format,
>>>>>>                          src_sampler_view->texture->target);
>>>>>>      set_vertex_shader(ctx);
>>>>>>      cso_set_tessctrl_shader_handle(ctx->cso, NULL);
>>>>>> diff --git a/src/gallium/auxiliary/util/u_blitter.c
>>>>>> b/src/gallium/auxiliary/util/u_blitter.c
>>>>>> index 16bf90f..5dfe2c7 100644
>>>>>> --- a/src/gallium/auxiliary/util/u_blitter.c
>>>>>> +++ b/src/gallium/auxiliary/util/u_blitter.c
>>>>>> @@ -81,6 +81,8 @@ struct blitter_context_priv
>>>>>>      /* FS which outputs a color from a texture,
>>>>>>         where the index is PIPE_TEXTURE_* to be sampled. */
>>>>>>      void *fs_texfetch_col[PIPE_MAX_TEXTURE_TYPES];
>>>>>> +   void *fs_texfetch_col_uint[PIPE_MAX_TEXTURE_TYPES];
>>>>>> +   void *fs_texfetch_col_sint[PIPE_MAX_TEXTURE_TYPES];
>>>>>>
>>>>>>      /* FS which outputs a depth from a texture,
>>>>>>         where the index is PIPE_TEXTURE_* to be sampled. */
>>>>>> @@ -90,6 +92,8 @@ struct blitter_context_priv
>>>>>>
>>>>>>      /* FS which outputs one sample from a multisample texture. */
>>>>>>      void *fs_texfetch_col_msaa[PIPE_MAX_TEXTURE_TYPES];
>>>>>> +   void *fs_texfetch_col_msaa_uint[PIPE_MAX_TEXTURE_TYPES];
>>>>>> +   void *fs_texfetch_col_msaa_sint[PIPE_MAX_TEXTURE_TYPES];
>>>>>>      void *fs_texfetch_depth_msaa[PIPE_MAX_TEXTURE_TYPES];
>>>>>>      void *fs_texfetch_depthstencil_msaa[PIPE_MAX_TEXTURE_TYPES];
>>>>>>      void *fs_texfetch_stencil_msaa[PIPE_MAX_TEXTURE_TYPES];
>>>>>> @@ -438,6 +442,10 @@ void util_blitter_destroy(struct blitter_context
>>>>>> *blitter)
>>>>>>      for (i = 0; i < PIPE_MAX_TEXTURE_TYPES; i++) {
>>>>>>         if (ctx->fs_texfetch_col[i])
>>>>>>            ctx->delete_fs_state(pipe, ctx->fs_texfetch_col[i]);
>>>>>> +      if (ctx->fs_texfetch_col_sint[i])
>>>>>> +         ctx->delete_fs_state(pipe, ctx->fs_texfetch_col_sint[i]);
>>>>>> +      if (ctx->fs_texfetch_col_uint[i])
>>>>>> +         ctx->delete_fs_state(pipe, ctx->fs_texfetch_col_uint[i]);
>>>>>>         if (ctx->fs_texfetch_depth[i])
>>>>>>            ctx->delete_fs_state(pipe, ctx->fs_texfetch_depth[i]);
>>>>>>         if (ctx->fs_texfetch_depthstencil[i])
>>>>>> @@ -447,6 +455,10 @@ void util_blitter_destroy(struct blitter_context
>>>>>> *blitter)
>>>>>>
>>>>>>         if (ctx->fs_texfetch_col_msaa[i])
>>>>>>            ctx->delete_fs_state(pipe, ctx->fs_texfetch_col_msaa[i]);
>>>>>> +      if (ctx->fs_texfetch_col_msaa_sint[i])
>>>>>> +         ctx->delete_fs_state(pipe, ctx->fs_texfetch_col_msaa_sint[i]);
>>>>>> +      if (ctx->fs_texfetch_col_msaa_uint[i])
>>>>>> +         ctx->delete_fs_state(pipe, ctx->fs_texfetch_col_msaa_uint[i]);
>>>>>>         if (ctx->fs_texfetch_depth_msaa[i])
>>>>>>            ctx->delete_fs_state(pipe, ctx->fs_texfetch_depth_msaa[i]);
>>>>>>         if (ctx->fs_texfetch_depthstencil_msaa[i])
>>>>>> @@ -844,25 +856,29 @@ static void *blitter_get_fs_texfetch_col(struct
>>>>>> blitter_context_priv *ctx,
>>>>>>   {
>>>>>>      struct pipe_context *pipe = ctx->base.pipe;
>>>>>>      unsigned tgsi_tex = util_pipe_tex_to_tgsi_tex(target,
>>>>>> src_nr_samples);
>>>>>> +   enum tgsi_return_type stype;
>>>>>>
>>>>>>      assert(target < PIPE_MAX_TEXTURE_TYPES);
>>>>>>
>>>>>> +       if (util_format_is_pure_uint(format))
>>>>>> +               stype = TGSI_RETURN_TYPE_UINT;
>>>>>> +       else if (util_format_is_pure_sint(format))
>>>>>> +               stype = TGSI_RETURN_TYPE_SINT;
>>>>>> +       else
>>>>>> +               stype = TGSI_RETURN_TYPE_FLOAT;
>>>>>> +
>>>>>
>>>>>
>>>>> Need to indent with spaces, not tabs.
>>>>
>>>> oh, whoops.. I apparently shouldn't be trying to carry on a
>>>> conversation while preparing patches.. I'll fix this up locally but I
>>>> don't think there is any point to resend unless other review
>>>> comments..
>>>>
>>>> that said, I've been trying to find a good way to test the draw module
>>>> changes (w/ ST_DEBUG=tgsi to verify resulting shader) with llvmpipe
>>>> (since the hw drivers I have access to don't use draw).. not sure if
>>>> ST_DEBUG=tgsi is only dumping out the shader before draw modifies it?
>>>> Or if I'm just not running the right piglit tests..  suggestions
>>>> welcome
>>>
>>> ST_DEBUG=tgsi may indeed be printing out unmodified shaders. You can use
>>> GALLIVM_DEBUG=tgsi which should print out what is actually compiled (at
>>> least for llvmpipe). Not sure how you could print them out for softpipe
>>> - it seems the stages which modify the tokens have their own debug code
>>> for calling tgsi_dump but it's ifdefed out...
>>> But there's probably very few tests actually hitting these paths.
>>
>> Ok, thanks.. the draw stuff seems to be working properly.  I did find
>> one minor problem with the glsl_to_tgsi, with fix squashed in locally:
>>
>> ---------
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index ce8f2ea..db2e4c8 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -3474,11 +3474,12 @@ count_resources(glsl_to_tgsi_visitor *v,
>> gl_program *prog)
>>     foreach_in_list(glsl_to_tgsi_instruction, inst, &v->instructions) {
>>        if (is_tex_instruction(inst->op)) {
>>           for (int i = 0; i < inst->sampler_array_size; i++) {
>> -            v->samplers_used |= 1 << (inst->sampler.index + i);
>> +            unsigned idx = inst->sampler.index + i;
>> +            v->samplers_used |= 1 << idx;
>>
>> -            debug_assert(i < (int)ARRAY_SIZE(v->sampler_types));
>> -            v->sampler_types[i] = inst->tex_type;
>> -            v->sampler_targets[i] =
>> +            debug_assert(idx < (int)ARRAY_SIZE(v->sampler_types));
>> +            v->sampler_types[idx] = inst->tex_type;
>> +            v->sampler_targets[idx] =
>>                 st_translate_texture_target(inst->tex_target, inst->tex_shadow);
>>
>>              if (inst->tex_shadow) {
>> ---------
>>
>> I can repost the resulting patchset if anyone wants.  In the mean time
>> I'm doing some piglit runs w/ gl3 force override (to get integer
>> textures) with freedreno, and once that all looks good, if no one
>> objects, I'd like to push these over the weekend
>
> so, in testing, I realized there are some paths (like DrawPixels) that
> insert texture sample instructions into the glsl ir..  the most
> straightforward solution seems to be to squash this into glsl_to_tgsi:
>
> ---------
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index db2e4c8..6146a50 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -581,6 +581,10 @@ glsl_to_tgsi_visitor::emit_asm(ir_instruction
> *ir, unsigned op,
>     inst->src[3] = src3;
>     inst->ir = ir;
>     inst->dead_mask = 0;
> +   /* default to float, for paths where this is not initialized
> +    * (since 0==UINT which is likely wrong):
> +    */
> +   inst->tex_type = GLSL_TYPE_FLOAT;
>
>     inst->function = NULL;
> ---------
>
> which then would get overridden to correct value in the normal cases
> in glsl_to_tgsi_visitor::visit(ir_texture *ir)
>
> I guess other option is to fish out all the other places where sample
> instructions are inserted.  I think all those cases are "old gl" type
> stuff, where you wouldn't have int/unsigned samplers.
>
> Thoughts?
>
> BR,
> -R
>
>> BR,
>> -R
>>
>>
>>> Roland
>>>
>>>
>>>> BR,
>>>> -R
>>>>
>>>>> -Brian
>>>>>
>>>> _______________________________________________
>>>> 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