[Mesa-dev] [Nouveau] [PATCH mesa 3/3] nouveau: Add support for clover / OpenCL kernel input parameters

[Samuel Pitoiset]

On 03/10/2016 05:03 PM, Pierre Moreau wrote:
> On 04:27 PM - Mar 10 2016, Samuel Pitoiset wrote:
>>
>>
>> On 03/10/2016 04:23 PM, Ilia Mirkin wrote:
>>> On Thu, Mar 10, 2016 at 10:14 AM, Hans de Goede <hdegoede at redhat.com> wrote:
>>>> Add support for clover / OpenCL kernel input parameters.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>>> ---
>>>>   .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp      | 18 +++++++++++++++---
>>>>   1 file changed, 15 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>> index a8258af..de0c72b 100644
>>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>>> @@ -1523,9 +1523,21 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int idx, int c, uint32_t address)
>>>>
>>>>      sym->reg.fileIndex = fileIdx;
>>>>
>>>> -   if (tgsiFile == TGSI_FILE_MEMORY &&
>>>> -       code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
>>>> -      sym->setFile(FILE_MEMORY_SHARED);
>>>> +   if (tgsiFile == TGSI_FILE_MEMORY) {
>>>> +      switch (code->memoryFiles[fileIdx].mem_type) {
>>>> +      case TGSI_MEMORY_TYPE_SHARED:
>>>> +         sym->setFile(FILE_MEMORY_SHARED);
>
> You might want to increment the address by at least
> `info->prop.cp.inputOffset`, and if inputs still end up in shared on Tesla,
> then increment further by the input size. This input offset of 0x10 (or is it
> 0x20?) is due to the card sticking the size of a block and of the grid inside
> `s[0x0..0x10]` (or maybe Nouveau is doing that, but I doubt it.). So even if
> the user inputs end up somewhere else in memory, you most likely still don't
> want to overwrite the grid information. This should be necessary only for Tesla
> cards.

cf. my previous comment. :-)

>
>>>> +         break;
>>>> +      case TGSI_MEMORY_TYPE_INPUT:
>>>> +         assert(prog->getType() == Program::TYPE_COMPUTE);
>>>> +         assert(idx == -1);
>>>> +         sym->setFile(FILE_SHADER_INPUT);
>>>> +         address += info->prop.cp.inputOffset;
>>>
>>> What's the idea here? i.e. what is the inputOffset, how is it set, and why?
>>
>> I don't get the idea too, btw.
>>
>> But prop.cp.inputOffset is only defined for compute on Kepler. It's the
>> offset of input parameters in the screen->parm BO but as you already know,
>> it is going to be removed because the idea is to use screen->uniform_bo
>> instead. I'll do this change *after* the compute shaders support on Kepler.
>
> If I understand correctly, the goal is to have user inputs in a
> `screen->uniform_bo`, and so for all generations?

Sure for fermi, and probably for Tesla.

>
> Pierre
>
>
>>
>>>
>>>    -ilia
>>>
>>>> +         break;
>>>> +      default:
>>>> +         assert(0); /* TODO: Add support for global and local memory */
>>>> +      }
>>>> +   }
>>>>
>>>>      if (idx >= 0) {
>>>>         if (sym->reg.file == FILE_SHADER_INPUT)
>>>> --
>>>> 2.7.2
>>>>
>>
>> --
>> -Samuel
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau

-- 
-Samuel