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

[Pierre Moreau]

On 11:05 AM - Mar 10 2016, Ilia Mirkin wrote:
> On Thu, Mar 10, 2016 at 11:03 AM, Pierre Moreau  wrote:
> > You might want to increment the address by at least
> > `info->prop.cp.inputOffset`, and if inputs still end up in shared on Tesla,
> 
> There's a cp.sharedOffset just for that :) However it doesn't appear
> to get set anywhere...

Oh really?! I completely missed that one… Well, I have some changes to make on
my own code then! :-D Thanks for pointing that out!

Pierre


> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Ilia Mirkin]

On Thu, Mar 10, 2016 at 11:05 AM, Samuel Pitoiset
 wrote:
>> 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.

I think continuing to use the USER_PARAMS or whatever mechanism on
telsa makes sense. That's why I agreed to keep the MEMORY, INPUT
concept.

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [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  wrote:

Add support for clover / OpenCL kernel input parameters.

Signed-off-by: Hans de Goede 
---
  .../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
nouv...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


--
-Samuel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Ilia Mirkin]

On Thu, Mar 10, 2016 at 11:03 AM, Pierre Moreau  wrote:
> You might want to increment the address by at least
> `info->prop.cp.inputOffset`, and if inputs still end up in shared on Tesla,

There's a cp.sharedOffset just for that :) However it doesn't appear
to get set anywhere...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Pierre Moreau]

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  wrote:
> >>Add support for clover / OpenCL kernel input parameters.
> >>
> >>Signed-off-by: Hans de Goede 
> >>---
> >>  .../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.

> >>+ 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?

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
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev