[Mesa-dev] [PATCH] tgsi/nouveau: Add support for tesselation ctrl and tesselation eval
Marcos Souza
marcos.souza.org at gmail.com
Thu Aug 13 21:25:58 PDT 2015
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?
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/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150814/50e4e437/attachment-0001.html>
More information about the mesa-dev
mailing list