On Fri, Jun 12, 2015 at 9:47 AM, Rob Clark <robdcl...@gmail.com> wrote:
> On Thu, Jun 11, 2015 at 8:36 PM, Roland Scheidegger <srol...@vmware.com>
> wrote:
>> Am 12.06.2015 um 02:02 schrieb Rob Clark:
>>> On Thu, Jun 11, 2015 at 5:47 PM, Brian Paul <bri...@vmware.com> wrote:
>>>> On 06/11/2015 02:38 PM, Rob Clark wrote:
>>>>>
>>>>> From: Rob Clark <robcl...@freedesktop.org>
>>>>>
>>>>> Some hardware needs to know the sampler type. Update the blit related
>>>>> shaders to include SVIEW decl.
>>>>>
>>>>> Signed-off-by: Rob Clark <robcl...@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@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
