[Mesa-dev] [PATCH 1/2] r600g: precalculate semantic indices for SPI setup

Alex Deucher alexdeucher at gmail.com
Fri Nov 4 15:34:46 PDT 2011


On Fri, Nov 4, 2011 at 5:53 PM, Marek Olšák <maraeo at gmail.com> wrote:
> FYI, this commit completely breaks RV670. glxgears is completely
> black, other apps are either black or rendered incorrectly (mostly
> one-colored, the clear color I guess).

I knew I should have tested 6xx before I pushed.  Reverted for now.

Alex

>
> Marek
>
> On Fri, Nov 4, 2011 at 6:24 PM, Vadim Girlin <vadimgirlin at gmail.com> wrote:
>> There is no need to duplicate semantic mapping which is done in hw, so get
>> rid of r600_find_vs_semantic_index.
>>
>> TGSI name/sid pair is mapped to the 8-bit semantic index for SPI.
>>
>> Signed-off-by: Vadim Girlin <vadimgirlin at gmail.com>
>> ---
>>
>> This patch and the next one were tested on the evergreen,
>> there are some changes in the test results:
>>
>> Following tests are passing due to slightly different handling of
>> TGSI_SEMANTIC_BCOLOR (it's still not completely correct).
>>        vertex-program-two-side enabled back back2      fail -> pass
>>        vertex-program-two-side enabled back front2     fail -> pass
>>        vertex-program-two-side enabled front back2     fail -> pass
>>
>> This test usually fails for me after these patches, but passes when
>> it's run independently - probably unrelated:
>>        time-elapsed    pass -> fail
>>
>>  src/gallium/drivers/r600/evergreen_state.c   |   19 ++++----
>>  src/gallium/drivers/r600/r600_shader.c       |   67 ++++++++++++++++----------
>>  src/gallium/drivers/r600/r600_shader.h       |    2 +-
>>  src/gallium/drivers/r600/r600_state.c        |   20 +++-----
>>  src/gallium/drivers/r600/r600_state_common.c |   13 ++---
>>  5 files changed, 64 insertions(+), 57 deletions(-)
>>
>> diff --git a/src/gallium/drivers/r600/evergreen_state.c b/src/gallium/drivers/r600/evergreen_state.c
>> index fd2e5da..904267d 100644
>> --- a/src/gallium/drivers/r600/evergreen_state.c
>> +++ b/src/gallium/drivers/r600/evergreen_state.c
>> @@ -2375,20 +2375,20 @@ void evergreen_pipe_shader_vs(struct pipe_context *ctx, struct r600_pipe_shader
>>        struct r600_pipe_context *rctx = (struct r600_pipe_context *)ctx;
>>        struct r600_pipe_state *rstate = &shader->rstate;
>>        struct r600_shader *rshader = &shader->shader;
>> -       unsigned spi_vs_out_id[10];
>> -       unsigned i, tmp, nparams;
>> +       unsigned spi_vs_out_id[10] = {};
>> +       unsigned i, tmp, nparams = 0;
>>
>>        /* clear previous register */
>>        rstate->nregs = 0;
>>
>> -       /* so far never got proper semantic id from tgsi */
>> -       for (i = 0; i < 10; i++) {
>> -               spi_vs_out_id[i] = 0;
>> -       }
>> -       for (i = 0; i < 32; i++) {
>> -               tmp = i << ((i & 3) * 8);
>> -               spi_vs_out_id[i / 4] |= tmp;
>> +       for (i = 0; i < rshader->noutput; i++) {
>> +               if (rshader->output[i].spi_sid) {
>> +                       tmp = rshader->output[i].spi_sid << ((nparams & 3) * 8);
>> +                       spi_vs_out_id[nparams / 4] |= tmp;
>> +                       nparams++;
>> +               }
>>        }
>> +
>>        for (i = 0; i < 10; i++) {
>>                r600_pipe_state_add_reg(rstate,
>>                                        R_02861C_SPI_VS_OUT_ID_0 + i * 4,
>> @@ -2399,7 +2399,6 @@ void evergreen_pipe_shader_vs(struct pipe_context *ctx, struct r600_pipe_shader
>>         * VS is required to export at least one param and r600_shader_from_tgsi()
>>         * takes care of adding a dummy export.
>>         */
>> -       nparams = rshader->noutput - rshader->npos;
>>        if (nparams < 1)
>>                nparams = 1;
>>
>> diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c
>> index 87164ce..448e03a 100644
>> --- a/src/gallium/drivers/r600/r600_shader.c
>> +++ b/src/gallium/drivers/r600/r600_shader.c
>> @@ -57,24 +57,6 @@ issued in the w slot as well.
>>  The compiler must issue the source argument to slots z, y, and x
>>  */
>>
>> -
>> -int r600_find_vs_semantic_index(struct r600_shader *vs,
>> -                               struct r600_shader *ps, int id)
>> -{
>> -       struct r600_shader_io *input = &ps->input[id];
>> -       int index = 0;
>> -
>> -       for (int i = 0; i < vs->noutput; i++) {
>> -               if (input->name == vs->output[i].name &&
>> -                               input->sid == vs->output[i].sid)
>> -                       return index;
>> -               else if (vs->output[i].name != TGSI_SEMANTIC_POSITION &&
>> -                                vs->output[i].name != TGSI_SEMANTIC_PSIZE)
>> -                       index++;
>> -       }
>> -       return 0;
>> -}
>> -
>>  static int r600_pipe_shader(struct pipe_context *ctx, struct r600_pipe_shader *shader)
>>  {
>>        struct r600_pipe_context *rctx = (struct r600_pipe_context *)ctx;
>> @@ -361,6 +343,44 @@ static int evergreen_interp_flat(struct r600_shader_ctx *ctx, int input)
>>  * DB_SOURCE_FORMAT - export control restrictions
>>  *
>>  */
>> +
>> +
>> +/* Map name/sid pair from tgsi to the 8-bit semantic index for SPI setup */
>> +static int r600_spi_sid(struct r600_shader_io * io)
>> +{
>> +       int index, name = io->name;
>> +
>> +       /* These params are handled differently, they don't need
>> +        * semantic indices, so we'll use 0 for them.
>> +        */
>> +       if (name == TGSI_SEMANTIC_POSITION ||
>> +               name == TGSI_SEMANTIC_PSIZE ||
>> +               name == TGSI_SEMANTIC_FACE)
>> +               index = 0;
>> +       else {
>> +               if (name == TGSI_SEMANTIC_GENERIC) {
>> +                       /* For generic params simply use sid from tgsi */
>> +                       index = io->sid;
>> +               } else {
>> +
>> +                       /* FIXME: two-side rendering is broken in r600g, this will
>> +                        * keep old functionality */
>> +                       if (name == TGSI_SEMANTIC_BCOLOR)
>> +                               name = TGSI_SEMANTIC_COLOR;
>> +
>> +                       /* For non-generic params - pack name and sid into 8 bits */
>> +                       index = 0x80 | (name<<3) | (io->sid);
>> +               }
>> +
>> +               /* Make sure that all really used indices have nonzero value, so
>> +                * we can just compare it to 0 later instead of comparing the name
>> +                * with different values to detect special cases. */
>> +               index++;
>> +       }
>> +
>> +       return index;
>> +};
>> +
>>  static int tgsi_declaration(struct r600_shader_ctx *ctx)
>>  {
>>        struct tgsi_full_declaration *d = &ctx->parse.FullToken.FullDeclaration;
>> @@ -372,13 +392,13 @@ static int tgsi_declaration(struct r600_shader_ctx *ctx)
>>                i = ctx->shader->ninput++;
>>                ctx->shader->input[i].name = d->Semantic.Name;
>>                ctx->shader->input[i].sid = d->Semantic.Index;
>> +               ctx->shader->input[i].spi_sid = r600_spi_sid(&ctx->shader->input[i]);
>>                ctx->shader->input[i].interpolate = d->Declaration.Interpolate;
>>                ctx->shader->input[i].centroid = d->Declaration.Centroid;
>>                ctx->shader->input[i].gpr = ctx->file_offset[TGSI_FILE_INPUT] + d->Range.First;
>>                if (ctx->type == TGSI_PROCESSOR_FRAGMENT && ctx->bc->chip_class >= EVERGREEN) {
>>                        /* turn input into interpolate on EG */
>> -                       if (ctx->shader->input[i].name != TGSI_SEMANTIC_POSITION &&
>> -                           ctx->shader->input[i].name != TGSI_SEMANTIC_FACE) {
>> +                       if (ctx->shader->input[i].spi_sid) {
>>                                ctx->shader->input[i].lds_pos = ctx->shader->nlds++;
>>                                if (ctx->shader->input[i].interpolate > 0) {
>>                                        evergreen_interp_alu(ctx, i);
>> @@ -392,14 +412,9 @@ static int tgsi_declaration(struct r600_shader_ctx *ctx)
>>                i = ctx->shader->noutput++;
>>                ctx->shader->output[i].name = d->Semantic.Name;
>>                ctx->shader->output[i].sid = d->Semantic.Index;
>> +               ctx->shader->output[i].spi_sid = r600_spi_sid(&ctx->shader->output[i]);
>>                ctx->shader->output[i].gpr = ctx->file_offset[TGSI_FILE_OUTPUT] + d->Range.First;
>>                ctx->shader->output[i].interpolate = d->Declaration.Interpolate;
>> -               if (ctx->type == TGSI_PROCESSOR_VERTEX) {
>> -                       /* these don't count as vertex param exports */
>> -                       if ((ctx->shader->output[i].name == TGSI_SEMANTIC_POSITION) ||
>> -                           (ctx->shader->output[i].name == TGSI_SEMANTIC_PSIZE))
>> -                               ctx->shader->npos++;
>> -               }
>>                break;
>>        case TGSI_FILE_CONSTANT:
>>        case TGSI_FILE_TEMPORARY:
>> diff --git a/src/gallium/drivers/r600/r600_shader.h b/src/gallium/drivers/r600/r600_shader.h
>> index ada369a..9990ba6 100644
>> --- a/src/gallium/drivers/r600/r600_shader.h
>> +++ b/src/gallium/drivers/r600/r600_shader.h
>> @@ -30,6 +30,7 @@ struct r600_shader_io {
>>        unsigned                gpr;
>>        unsigned                done;
>>        int                     sid;
>> +       int                     spi_sid;
>>        unsigned                interpolate;
>>        boolean                 centroid;
>>        unsigned                lds_pos; /* for evergreen */
>> @@ -40,7 +41,6 @@ struct r600_shader {
>>        struct r600_bytecode            bc;
>>        unsigned                ninput;
>>        unsigned                noutput;
>> -       unsigned                npos;
>>        unsigned                nlds;
>>        struct r600_shader_io   input[32];
>>        struct r600_shader_io   output[32];
>> diff --git a/src/gallium/drivers/r600/r600_state.c b/src/gallium/drivers/r600/r600_state.c
>> index bf3da79..ce65da6 100644
>> --- a/src/gallium/drivers/r600/r600_state.c
>> +++ b/src/gallium/drivers/r600/r600_state.c
>> @@ -2142,22 +2142,19 @@ void r600_pipe_shader_vs(struct pipe_context *ctx, struct r600_pipe_shader *shad
>>        struct r600_pipe_state *rstate = &shader->rstate;
>>        struct r600_shader *rshader = &shader->shader;
>>        unsigned spi_vs_out_id[10];
>> -       unsigned i, tmp, nparams;
>> +       unsigned i, tmp, nparams = 0;
>>
>>        /* clear previous register */
>>        rstate->nregs = 0;
>>
>> -       /* so far never got proper semantic id from tgsi */
>> -       /* FIXME better to move this in config things so they get emited
>> -        * only one time per cs
>> -        */
>> -       for (i = 0; i < 10; i++) {
>> -               spi_vs_out_id[i] = 0;
>> -       }
>> -       for (i = 0; i < 32; i++) {
>> -               tmp = i << ((i & 3) * 8);
>> -               spi_vs_out_id[i / 4] |= tmp;
>> +       for (i = 0; i < rshader->noutput; i++) {
>> +               if (rshader->output[i].spi_sid) {
>> +                       tmp = rshader->output[i].spi_sid << ((nparams & 3) * 8);
>> +                       spi_vs_out_id[nparams / 4] |= tmp;
>> +                       nparams++;
>> +               }
>>        }
>> +
>>        for (i = 0; i < 10; i++) {
>>                r600_pipe_state_add_reg(rstate,
>>                                        R_028614_SPI_VS_OUT_ID_0 + i * 4,
>> @@ -2168,7 +2165,6 @@ void r600_pipe_shader_vs(struct pipe_context *ctx, struct r600_pipe_shader *shad
>>         * VS is required to export at least one param and r600_shader_from_tgsi()
>>         * takes care of adding a dummy export.
>>         */
>> -       nparams = rshader->noutput - rshader->npos;
>>        if (nparams < 1)
>>                nparams = 1;
>>
>> diff --git a/src/gallium/drivers/r600/r600_state_common.c b/src/gallium/drivers/r600/r600_state_common.c
>> index b24bb54..3d0345a 100644
>> --- a/src/gallium/drivers/r600/r600_state_common.c
>> +++ b/src/gallium/drivers/r600/r600_state_common.c
>> @@ -366,14 +366,11 @@ static void r600_spi_update(struct r600_pipe_context *rctx)
>>
>>        rstate->nregs = 0;
>>        for (i = 0; i < rshader->ninput; i++) {
>> -               if (rshader->input[i].name == TGSI_SEMANTIC_POSITION ||
>> -                   rshader->input[i].name == TGSI_SEMANTIC_FACE)
>> -                       if (rctx->chip_class >= EVERGREEN)
>> -                               continue;
>> -                       else
>> -                               sid=0;
>> -               else
>> -                       sid=r600_find_vs_semantic_index(&rctx->vs_shader->shader, rshader, i);
>> +
>> +               sid = rshader->input[i].spi_sid;
>> +
>> +               if (!sid && (rctx->chip_class >= EVERGREEN))
>> +                       continue;
>>
>>                tmp = S_028644_SEMANTIC(sid);
>>
>> --
>> 1.7.7
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
> _______________________________________________
> 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