[Mesa-dev] [PATCH] tgsi/nouveau: Add support for tesselation ctrl and tesselation eval

[Marcos Paulo de souza]

Hi Ilia,

Em 14-08-2015 01:45, Ilia Mirkin escreveu:
> On Fri, Aug 14, 2015 at 12:43 AM, Marcos Souza
> <marcos.souza.org at gmail.com> wrote:
>> HI Ilia
>>
>> 2015-08-14 1:31 GMT-03:00 Ilia Mirkin <imirkin at alum.mit.edu>:
>>> 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.
>>>
>> Sorry for flooding the list, but I'm relaly excited about it :)
>>
>> So, this is the change you asked. It also solved the problem:
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c
>> b/src/gallium/auxiliary/tgsi/tgsi_text.c
>> index 8647e4e..95c1daf 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_text.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_text.c
>> @@ -684,7 +684,12 @@ parse_register_dcl(
>>          * 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) {
>> +
>> +      /* similarly from tessalation */
> tessellation
OK.
>
>> +      int exclude = (ctx->processor == TGSI_PROCESSOR_TESS_EVAL && *file ==
>> TGSI_FILE_INPUT) ||
>> +          (ctx->processor == TGSI_PROCESSOR_TESS_CTRL && (*file ==
>> TGSI_FILE_INPUT ||
>> +             *file == TGSI_FILE_OUTPUT));
> Why is this separate from the geometry thing?
I just separated because it's easier to read, since we have a lot of 
ANDs and ORs. But, if you think it's better, I could put it all together 
will geometry.
>
>> +      if ((ctx->processor == TGSI_PROCESSOR_GEOMETRY && *file ==
>> TGSI_FILE_INPUT) || exclude) {
>>            brackets[0] = brackets[1];
>>            *num_brackets = 1;
>>         } else {
>>
>> What do you think Ilia?
> Generally sounds good.
Now I'll take a look about the last problem of LRP and MOV.
>
>    -ilia