[Mesa-dev] [PATCH] tgsi/nouveau: Add support for tesselation ctrl and tesselation eval
Ilia Mirkin
imirkin at alum.mit.edu
Thu Aug 13 21:31:01 PDT 2015
On Fri, Aug 14, 2015 at 12:25 AM, Marcos Souza
<marcos.souza.org at gmail.com> wrote:
> Hi Ilia,
>
> 2015-08-14 1:02 GMT-03:00 Ilia Mirkin <imirkin at alum.mit.edu>:
>>
>> On Thu, Aug 13, 2015 at 11:55 PM, Marcos Souza
>> <marcos.souza.org at gmail.com> wrote:
>> > Hi Ilia,
>> >
>> > So i found the point here it addrs that double brackets, and the
>> > following
>> > patch solves it, but this is a right solution? If someone could guide me
>> > here, I could fix it :)
>> >
>> > diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> > b/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> > index 8ceb5b4..046471e 100644
>> > --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> > +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> > @@ -302,10 +302,14 @@ iter_declaration(
>> > TXT("[]");
>> > }
>> >
>> > - if (decl->Declaration.Dimension) {
>>
>> The issue is that the declaration is getting a dimension set by the
>> parser, which in turn is causing it to print funny. It shouldn't be
>> getting a dimension in the first place for those.
>
>
> The following patch fix the problem, is it the right place to put it?
I don't think so. Just glanced at the code, look at
parse_register_dcl
/* for geometry shader we don't really care about
* the first brackets it's always the size of the
* input primitive. so we want to declare just
* the index relevant to the semantics which is in
* the second bracket */
if (ctx->processor == TGSI_PROCESSOR_GEOMETRY && *file ==
TGSI_FILE_INPUT) {
brackets[0] = brackets[1];
*num_brackets = 1;
}
Basically you need to extend this logic to similarly exclude
(a) tess ctrl inputs and outputs
(b) tess eval inputs
Technically you need to exclude patch/tessinner/tessouter from that,
but in practice they won't have an extra set of brackets either.
>
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c
> b/src/gallium/auxiliary/tgsi/tgsi_text.c
> index 8647e4e..f734d58 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_text.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_text.c
> @@ -1185,7 +1185,10 @@ static boolean parse_declaration( struct
> translate_ctx *ctx )
> decl.Range.First = brackets[1].first;
> decl.Range.Last = brackets[1].last;
>
> - decl.Declaration.Dimension = 1;
> + if (!(ctx->processor == TGSI_PROCESSOR_TESS_CTRL ||
> + ctx->processor == TGSI_PROCESSOR_TESS_EVAL))
> + decl.Declaration.Dimension = 1;
> +
> decl.Dim.Index2D = brackets[0].first;
> }
>
>
>
>>
>>
>> > - CHR('[');
>> > - SID(decl->Dim.Index2D);
>> > - CHR(']');
>> > + /* FIXME: patched version could have tree dimensions?? */
>> > + if (patch && (iter->processor.Processor == TGSI_PROCESSOR_TESS_CTRL
>> > ||
>> > + iter->processor.Processor == TGSI_PROCESSOR_TESS_EVAL)) {
>> > + if (decl->Declaration.Dimension) {
>> > + CHR('[');
>> > + SID(decl->Dim.Index2D);
>> > + CHR(']');
>> > + }
>> > }
>> >
>> > After this patch, tess_eval output is the same before and after, but
>> > tess_ctrl is a little different:
>> > [marcos at x mesa]$ diff tess_ctrl tess_ctrl_new
>> > 29c29
>> > < 15: LRP OUT[ADDR[1].x][3], TEMP[1].xxxx, TEMP[3], TEMP[2]
>> > ---
>> >> 15: LRP OUT[0][3], TEMP[1].xxxx, TEMP[3], TEMP[2]
>> > 40c40
>> > < 26: MOV OUT[ADDR[1].x][2], TEMP[0]
>> > ---
>> >> 26: MOV OUT[0][2], TEMP[0]
>> >
>> > I'll try to investigate and send a new patch in the weekend.
>> >
>> > Thanks for all help Ilia and others!
>> >
>> > 2015-08-13 18:43 GMT-03:00 Ilia Mirkin <imirkin at alum.mit.edu>:
>> >>
>> >> [mesa-dev readded, please don't drop CC's]
>> >>
>> >> I found it by feeding the shader to nouveau_compiler with
>> >> NV50_PROG_DEBUG=1 set, which dumps the input tgsi. Those two should
>> >> match up.
>> >>
>> >> On Thu, Aug 13, 2015 at 5:39 PM, Marcos Paulo de souza
>> >> <marcos.souza.org at gmail.com> wrote:
>> >> > Hi Ilia,
>> >> >
>> >> > So, how can I test it? Do I need to especify some patameter to verify
>> >> > this
>> >> > type of problem?
>> >> >
>> >> > Thanks for the quick revision!
>> >> >
>> >> >
>> >> > Em 13-08-2015 16:03, Ilia Mirkin escreveu:
>> >> >>
>> >> >> Hi Macros,
>> >> >>
>> >> >> Looks like it's not parsed in exactly right. It will parse something
>> >> >> like
>> >> >>
>> >> >> TESS_EVAL
>> >> >> PROPERTY TES_PRIM_MODE 7
>> >> >> PROPERTY TES_SPACING 2
>> >> >> PROPERTY TES_VERTEX_ORDER_CW 0
>> >> >> PROPERTY TES_POINT_MODE 0
>> >> >> DCL IN[][0], GENERIC[0]
>> >> >> DCL IN[][1], GENERIC[1]
>> >> >>
>> >> >> as
>> >> >>
>> >> >> TESS_EVAL
>> >> >> PROPERTY TES_PRIM_MODE 7
>> >> >> PROPERTY TES_SPACING 2
>> >> >> PROPERTY TES_VERTEX_ORDER_CW 0
>> >> >> PROPERTY TES_POINT_MODE 0
>> >> >> DCL IN[][0][0], GENERIC[0]
>> >> >> DCL IN[][0][1], GENERIC[1]
>> >> >>
>> >> >> Perhaps the same issue happens for geometry shaders, but that
>> >> >> doesn't
>> >> >> make it right :) You might have to look at the printing logic to get
>> >> >> a
>> >> >> better understanding of what's going wrong.
>> >> >>
>> >> >> Also you should send patches to nouveau separately from patches to
>> >> >> the
>> >> >> rest of the infra. Ideally this would have been 2 patches, e.g.
>> >> >>
>> >> >> tgsi: set implicit array size for tess stages
>> >> >> nouveau: recognize tess stages in nouveau_compiler
>> >> >>
>> >> >> or something like that.
>> >> >>
>> >> >> On Wed, Aug 12, 2015 at 9:25 PM, Marcos Paulo de Souza
>> >> >> <marcos.souza.org at gmail.com> wrote:
>> >> >>>
>> >> >>> From: Marcos Paulo de Souza <marcos.souza.org at gmail.com>
>> >> >>>
>> >> >>> Signed-off-by: Marcos Paulo de Souza <marcos.souza.org>
>> >> >>> Suggested-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> >> >>> ---
>> >> >>> src/gallium/auxiliary/tgsi/tgsi_text.c | 6 +++++-
>> >> >>> src/gallium/drivers/nouveau/nouveau_compiler.c | 4 ++++
>> >> >>> 2 files changed, 9 insertions(+), 1 deletion(-)
>> >> >>>
>> >> >>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c
>> >> >>> b/src/gallium/auxiliary/tgsi/tgsi_text.c
>> >> >>> index a6675c5..8647e4e 100644
>> >> >>> --- a/src/gallium/auxiliary/tgsi/tgsi_text.c
>> >> >>> +++ b/src/gallium/auxiliary/tgsi/tgsi_text.c
>> >> >>> @@ -259,7 +259,7 @@ struct translate_ctx
>> >> >>> struct tgsi_token *tokens_end;
>> >> >>> struct tgsi_header *header;
>> >> >>> unsigned processor : 4;
>> >> >>> - int implied_array_size : 5;
>> >> >>> + int implied_array_size : 6;
>> >> >>> unsigned num_immediates;
>> >> >>> };
>> >> >>>
>> >> >>> @@ -1623,6 +1623,10 @@ static boolean translate( struct
>> >> >>> translate_ctx
>> >> >>> *ctx )
>> >> >>> if (!parse_header( ctx ))
>> >> >>> return FALSE;
>> >> >>>
>> >> >>> + if (ctx->processor == TGSI_PROCESSOR_TESS_CTRL ||
>> >> >>> + ctx->processor == TGSI_PROCESSOR_TESS_EVAL)
>> >> >>> + ctx->implied_array_size = 32 ;
>> >> >>> +
>> >> >>> while (*ctx->cur != '\0') {
>> >> >>> uint label_val = 0;
>> >> >>> if (!eat_white( &ctx->cur )) {
>> >> >>> diff --git a/src/gallium/drivers/nouveau/nouveau_compiler.c
>> >> >>> b/src/gallium/drivers/nouveau/nouveau_compiler.c
>> >> >>> index 8660498..495450b 100644
>> >> >>> --- a/src/gallium/drivers/nouveau/nouveau_compiler.c
>> >> >>> +++ b/src/gallium/drivers/nouveau/nouveau_compiler.c
>> >> >>> @@ -190,6 +190,10 @@ main(int argc, char *argv[])
>> >> >>> type = PIPE_SHADER_GEOMETRY;
>> >> >>> else if (!strncmp(text, "COMP", 4))
>> >> >>> type = PIPE_SHADER_COMPUTE;
>> >> >>> + else if (!strncmp(text, "TESS_CTRL", 9))
>> >> >>> + type = PIPE_SHADER_TESS_CTRL;
>> >> >>> + else if (!strncmp(text, "TESS_EVAL", 9))
>> >> >>> + type = PIPE_SHADER_TESS_EVAL;
>> >> >>> else {
>> >> >>> _debug_printf("Unrecognized TGSI header\n");
>> >> >>> return 1;
>> >> >>> --
>> >> >>> 2.4.3
>> >> >>>
>> >> >>> _______________________________________________
>> >> >>> mesa-dev mailing list
>> >> >>> mesa-dev at lists.freedesktop.org
>> >> >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> >> >
>> >> >
>> >
>> >
>> >
>> >
>> > --
>> > Att,
>> >
>> > Marcos Paulo de Souza
>> > Github: https://github.com/marcosps/
>
>
>
>
> --
> Att,
>
> Marcos Paulo de Souza
> Github: https://github.com/marcosps/
More information about the mesa-dev
mailing list