Re: [Mesa-dev] [PATCH 3/3] nvc0: rework nvc0_compute_validate_program()

2016-02-25 Thread Ilia Mirkin 
On Thu, Feb 25, 2016 at 12:42 PM, Samuel Pitoiset
 wrote:
> It would be easy to make the validate functions return a boolean to handle
> errors. I will think more about that stuff. But currently, I prefer to
> follow the existing design and drop this boolean.

This would need to be done as part of a larger error-handling
strategy. Right now we don't handle errors very well at all :(
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/3] nvc0: rework nvc0_compute_validate_program()

2016-02-25 Thread Samuel Pitoiset 



On 02/25/2016 06:44 PM, Ilia Mirkin wrote:

On Thu, Feb 25, 2016 at 12:42 PM, Samuel Pitoiset
 wrote:

It would be easy to make the validate functions return a boolean to handle
errors. I will think more about that stuff. But currently, I prefer to
follow the existing design and drop this boolean.


This would need to be done as part of a larger error-handling
strategy. Right now we don't handle errors very well at all :(


Yeah, it's a long time effort. :-)



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

Re: [Mesa-dev] [PATCH 3/3] nvc0: rework nvc0_compute_validate_program()

2016-02-25 Thread Samuel Pitoiset 



On 02/25/2016 06:35 PM, Ilia Mirkin wrote:

On Wed, Feb 24, 2016 at 12:44 PM, Samuel Pitoiset
 wrote:

Reduce the amount of duplicated code by re-using
nvc0_program_validate(). While we are at it, change the prototype
to return void and remove nvc0_compute.h which is now useless.

Signed-off-by: Samuel Pitoiset 
---
  src/gallium/drivers/nouveau/Makefile.sources   |  1 -
  src/gallium/drivers/nouveau/nvc0/nvc0_compute.c| 34 ++
  src/gallium/drivers/nouveau/nvc0/nvc0_compute.h|  9 --
  src/gallium/drivers/nouveau/nvc0/nvc0_context.h|  1 +
  .../drivers/nouveau/nvc0/nvc0_shader_state.c   | 15 ++
  src/gallium/drivers/nouveau/nvc0/nve4_compute.c|  4 +--
  6 files changed, 20 insertions(+), 44 deletions(-)
  delete mode 100644 src/gallium/drivers/nouveau/nvc0/nvc0_compute.h

diff --git a/src/gallium/drivers/nouveau/Makefile.sources 
b/src/gallium/drivers/nouveau/Makefile.sources
index 43ffce6..65f08c7 100644
--- a/src/gallium/drivers/nouveau/Makefile.sources
+++ b/src/gallium/drivers/nouveau/Makefile.sources
@@ -150,7 +150,6 @@ NVC0_C_SOURCES := \
 nvc0/gm107_texture.xml.h \
 nvc0/nvc0_3d.xml.h \
 nvc0/nvc0_compute.c \
-   nvc0/nvc0_compute.h \
 nvc0/nvc0_compute.xml.h \
 nvc0/nvc0_context.c \
 nvc0/nvc0_context.h \
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
index a664aaf..060f59d 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
@@ -23,7 +23,8 @@
   */

  #include "nvc0/nvc0_context.h"
-#include "nvc0/nvc0_compute.h"
+
+#include "nvc0/nvc0_compute.xml.h"

  int
  nvc0_screen_compute_setup(struct nvc0_screen *screen,
@@ -120,34 +121,6 @@ nvc0_screen_compute_setup(struct nvc0_screen *screen,
 return 0;
  }

-bool
-nvc0_compute_validate_program(struct nvc0_context *nvc0)
-{
-   struct nvc0_program *prog = nvc0->compprog;
-
-   if (prog->mem)
-  return true;
-
-   if (!prog->translated) {
-  prog->translated = nvc0_program_translate(
- prog, nvc0->screen->base.device->chipset, >base.debug);
-  if (!prog->translated)
- return false;
-   }
-   if (unlikely(!prog->code_size))
-  return false;
-
-   if (likely(prog->code_size)) {
-  if (nvc0_program_upload_code(nvc0, prog)) {
- struct nouveau_pushbuf *push = nvc0->base.pushbuf;
- BEGIN_NVC0(push, NVC0_CP(FLUSH), 1);
- PUSH_DATA (push, NVC0_COMPUTE_FLUSH_CODE);
- return true;
-  }
-   }
-   return false;
-}
-
  static void
  nvc0_compute_validate_samplers(struct nvc0_context *nvc0)
  {
@@ -292,8 +265,7 @@ nvc0_compute_validate_globals(struct nvc0_context *nvc0)
  static bool
  nvc0_compute_state_validate(struct nvc0_context *nvc0)
  {
-   if (!nvc0_compute_validate_program(nvc0))
-  return false;
+   nvc0_compprog_validate(nvc0);
 if (nvc0->dirty_cp & NVC0_NEW_CP_CONSTBUF)
nvc0_compute_validate_constbufs(nvc0);
 if (nvc0->dirty_cp & NVC0_NEW_CP_DRIVERCONST)
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h 
b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
deleted file mode 100644
index a23f7f3..000
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef NVC0_COMPUTE_H
-#define NVC0_COMPUTE_H
-
-#include "nvc0/nvc0_compute.xml.h"
-
-bool
-nvc0_compute_validate_program(struct nvc0_context *nvc0);
-
-#endif /* NVC0_COMPUTE_H */
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h 
b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
index 7aa4b62..0f1ebb0 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
@@ -254,6 +254,7 @@ void nvc0_tctlprog_validate(struct nvc0_context *);
  void nvc0_tevlprog_validate(struct nvc0_context *);
  void nvc0_gmtyprog_validate(struct nvc0_context *);
  void nvc0_fragprog_validate(struct nvc0_context *);
+void nvc0_compprog_validate(struct nvc0_context *);

  void nvc0_tfb_validate(struct nvc0_context *);

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
index 2f46c43..6b02ed5 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
@@ -28,6 +28,8 @@
  #include "nvc0/nvc0_context.h"
  #include "nvc0/nvc0_query_hw.h"

+#include "nvc0/nvc0_compute.xml.h"
+
  static inline void
  nvc0_program_update_context_state(struct nvc0_context *nvc0,
struct nvc0_program *prog, int stage)
@@ -257,6 +259,19 @@ nvc0_gmtyprog_validate(struct nvc0_context *nvc0)
  }

  void
+nvc0_compprog_validate(struct nvc0_context *nvc0)
+{
+   struct nouveau_pushbuf *push = nvc0->base.pushbuf;
+   struct nvc0_program *cp = nvc0->compprog;
+
+   if (cp && !nvc0_program_validate(nvc0, cp))
+

Re: [Mesa-dev] [PATCH 3/3] nvc0: rework nvc0_compute_validate_program()

2016-02-25 Thread Ilia Mirkin 
On Wed, Feb 24, 2016 at 12:44 PM, Samuel Pitoiset
 wrote:
> Reduce the amount of duplicated code by re-using
> nvc0_program_validate(). While we are at it, change the prototype
> to return void and remove nvc0_compute.h which is now useless.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/gallium/drivers/nouveau/Makefile.sources   |  1 -
>  src/gallium/drivers/nouveau/nvc0/nvc0_compute.c| 34 
> ++
>  src/gallium/drivers/nouveau/nvc0/nvc0_compute.h|  9 --
>  src/gallium/drivers/nouveau/nvc0/nvc0_context.h|  1 +
>  .../drivers/nouveau/nvc0/nvc0_shader_state.c   | 15 ++
>  src/gallium/drivers/nouveau/nvc0/nve4_compute.c|  4 +--
>  6 files changed, 20 insertions(+), 44 deletions(-)
>  delete mode 100644 src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
>
> diff --git a/src/gallium/drivers/nouveau/Makefile.sources 
> b/src/gallium/drivers/nouveau/Makefile.sources
> index 43ffce6..65f08c7 100644
> --- a/src/gallium/drivers/nouveau/Makefile.sources
> +++ b/src/gallium/drivers/nouveau/Makefile.sources
> @@ -150,7 +150,6 @@ NVC0_C_SOURCES := \
> nvc0/gm107_texture.xml.h \
> nvc0/nvc0_3d.xml.h \
> nvc0/nvc0_compute.c \
> -   nvc0/nvc0_compute.h \
> nvc0/nvc0_compute.xml.h \
> nvc0/nvc0_context.c \
> nvc0/nvc0_context.h \
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> index a664aaf..060f59d 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> @@ -23,7 +23,8 @@
>   */
>
>  #include "nvc0/nvc0_context.h"
> -#include "nvc0/nvc0_compute.h"
> +
> +#include "nvc0/nvc0_compute.xml.h"
>
>  int
>  nvc0_screen_compute_setup(struct nvc0_screen *screen,
> @@ -120,34 +121,6 @@ nvc0_screen_compute_setup(struct nvc0_screen *screen,
> return 0;
>  }
>
> -bool
> -nvc0_compute_validate_program(struct nvc0_context *nvc0)
> -{
> -   struct nvc0_program *prog = nvc0->compprog;
> -
> -   if (prog->mem)
> -  return true;
> -
> -   if (!prog->translated) {
> -  prog->translated = nvc0_program_translate(
> - prog, nvc0->screen->base.device->chipset, >base.debug);
> -  if (!prog->translated)
> - return false;
> -   }
> -   if (unlikely(!prog->code_size))
> -  return false;
> -
> -   if (likely(prog->code_size)) {
> -  if (nvc0_program_upload_code(nvc0, prog)) {
> - struct nouveau_pushbuf *push = nvc0->base.pushbuf;
> - BEGIN_NVC0(push, NVC0_CP(FLUSH), 1);
> - PUSH_DATA (push, NVC0_COMPUTE_FLUSH_CODE);
> - return true;
> -  }
> -   }
> -   return false;
> -}
> -
>  static void
>  nvc0_compute_validate_samplers(struct nvc0_context *nvc0)
>  {
> @@ -292,8 +265,7 @@ nvc0_compute_validate_globals(struct nvc0_context *nvc0)
>  static bool
>  nvc0_compute_state_validate(struct nvc0_context *nvc0)
>  {
> -   if (!nvc0_compute_validate_program(nvc0))
> -  return false;
> +   nvc0_compprog_validate(nvc0);
> if (nvc0->dirty_cp & NVC0_NEW_CP_CONSTBUF)
>nvc0_compute_validate_constbufs(nvc0);
> if (nvc0->dirty_cp & NVC0_NEW_CP_DRIVERCONST)
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
> deleted file mode 100644
> index a23f7f3..000
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
> +++ /dev/null
> @@ -1,9 +0,0 @@
> -#ifndef NVC0_COMPUTE_H
> -#define NVC0_COMPUTE_H
> -
> -#include "nvc0/nvc0_compute.xml.h"
> -
> -bool
> -nvc0_compute_validate_program(struct nvc0_context *nvc0);
> -
> -#endif /* NVC0_COMPUTE_H */
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> index 7aa4b62..0f1ebb0 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> @@ -254,6 +254,7 @@ void nvc0_tctlprog_validate(struct nvc0_context *);
>  void nvc0_tevlprog_validate(struct nvc0_context *);
>  void nvc0_gmtyprog_validate(struct nvc0_context *);
>  void nvc0_fragprog_validate(struct nvc0_context *);
> +void nvc0_compprog_validate(struct nvc0_context *);
>
>  void nvc0_tfb_validate(struct nvc0_context *);
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> index 2f46c43..6b02ed5 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> @@ -28,6 +28,8 @@
>  #include "nvc0/nvc0_context.h"
>  #include "nvc0/nvc0_query_hw.h"
>
> +#include "nvc0/nvc0_compute.xml.h"
> +
>  static inline void
>  nvc0_program_update_context_state(struct nvc0_context *nvc0,
>struct nvc0_program *prog, int stage)
> @@ -257,6 +259,19 @@ nvc0_gmtyprog_validate(struct nvc0_context *nvc0)
>  }
>
>  void
>

Re: [Mesa-dev] [PATCH 3/3] nvc0: rework nvc0_compute_validate_program()

2016-02-24 Thread Pierre Moreau 
On 08:44 PM - Feb 24 2016, Samuel Pitoiset wrote:
> 
> 
> On 02/24/2016 08:30 PM, Pierre Moreau wrote:
> >Hi Samuel,
> >
> >On 06:44 PM - Feb 24 2016, Samuel Pitoiset wrote:
> >>Reduce the amount of duplicated code by re-using
> >>nvc0_program_validate(). While we are at it, change the prototype
> >>to return void and remove nvc0_compute.h which is now useless.
> >
> >Why don't you want to know whether the validation worked or not? If the
> >validation failed, the program has a bug and Nouveau shouldn't try to run it,
> >so allocating buffers that will be unused seems wasteful. Am I missing
> >something here?
> 
> Because it's useless? If the program can't be correctly validated it's going
> to break the universe. If you look at nvc0_gmtyprog_validate() for example
> you will see that I just follow the same design.

I should have checked the other `*_prog_validate()` functions indeed. And I had
forgotten that when those `*_prog_validate()` functions are called, the shaders
have already been transformed from GLSL to TGSI, so user errors in the input
shaders should have been catched already and only translation errors could
occur at this point.

Pierre


Acked-by: Pierre Moreau 


> >
> >Pierre
> >
> >>
> >>Signed-off-by: Samuel Pitoiset 
> >>---
> >>  src/gallium/drivers/nouveau/Makefile.sources   |  1 -
> >>  src/gallium/drivers/nouveau/nvc0/nvc0_compute.c| 34 
> >> ++
> >>  src/gallium/drivers/nouveau/nvc0/nvc0_compute.h|  9 --
> >>  src/gallium/drivers/nouveau/nvc0/nvc0_context.h|  1 +
> >>  .../drivers/nouveau/nvc0/nvc0_shader_state.c   | 15 ++
> >>  src/gallium/drivers/nouveau/nvc0/nve4_compute.c|  4 +--
> >>  6 files changed, 20 insertions(+), 44 deletions(-)
> >>  delete mode 100644 src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
> >>
> >>diff --git a/src/gallium/drivers/nouveau/Makefile.sources 
> >>b/src/gallium/drivers/nouveau/Makefile.sources
> >>index 43ffce6..65f08c7 100644
> >>--- a/src/gallium/drivers/nouveau/Makefile.sources
> >>+++ b/src/gallium/drivers/nouveau/Makefile.sources
> >>@@ -150,7 +150,6 @@ NVC0_C_SOURCES := \
> >>nvc0/gm107_texture.xml.h \
> >>nvc0/nvc0_3d.xml.h \
> >>nvc0/nvc0_compute.c \
> >>-   nvc0/nvc0_compute.h \
> >>nvc0/nvc0_compute.xml.h \
> >>nvc0/nvc0_context.c \
> >>nvc0/nvc0_context.h \
> >>diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c 
> >>b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> >>index a664aaf..060f59d 100644
> >>--- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> >>+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> >>@@ -23,7 +23,8 @@
> >>   */
> >>
> >>  #include "nvc0/nvc0_context.h"
> >>-#include "nvc0/nvc0_compute.h"
> >>+
> >>+#include "nvc0/nvc0_compute.xml.h"
> >>
> >>  int
> >>  nvc0_screen_compute_setup(struct nvc0_screen *screen,
> >>@@ -120,34 +121,6 @@ nvc0_screen_compute_setup(struct nvc0_screen *screen,
> >> return 0;
> >>  }
> >>
> >>-bool
> >>-nvc0_compute_validate_program(struct nvc0_context *nvc0)
> >>-{
> >>-   struct nvc0_program *prog = nvc0->compprog;
> >>-
> >>-   if (prog->mem)
> >>-  return true;
> >>-
> >>-   if (!prog->translated) {
> >>-  prog->translated = nvc0_program_translate(
> >>- prog, nvc0->screen->base.device->chipset, >base.debug);
> >>-  if (!prog->translated)
> >>- return false;
> >>-   }
> >>-   if (unlikely(!prog->code_size))
> >>-  return false;
> >>-
> >>-   if (likely(prog->code_size)) {
> >>-  if (nvc0_program_upload_code(nvc0, prog)) {
> >>- struct nouveau_pushbuf *push = nvc0->base.pushbuf;
> >>- BEGIN_NVC0(push, NVC0_CP(FLUSH), 1);
> >>- PUSH_DATA (push, NVC0_COMPUTE_FLUSH_CODE);
> >>- return true;
> >>-  }
> >>-   }
> >>-   return false;
> >>-}
> >>-
> >>  static void
> >>  nvc0_compute_validate_samplers(struct nvc0_context *nvc0)
> >>  {
> >>@@ -292,8 +265,7 @@ nvc0_compute_validate_globals(struct nvc0_context *nvc0)
> >>  static bool
> >>  nvc0_compute_state_validate(struct nvc0_context *nvc0)
> >>  {
> >>-   if (!nvc0_compute_validate_program(nvc0))
> >>-  return false;
> >>+   nvc0_compprog_validate(nvc0);
> >> if (nvc0->dirty_cp & NVC0_NEW_CP_CONSTBUF)
> >>nvc0_compute_validate_constbufs(nvc0);
> >> if (nvc0->dirty_cp & NVC0_NEW_CP_DRIVERCONST)
> >>diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h 
> >>b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
> >>deleted file mode 100644
> >>index a23f7f3..000
> >>--- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
> >>+++ /dev/null
> >>@@ -1,9 +0,0 @@
> >>-#ifndef NVC0_COMPUTE_H
> >>-#define NVC0_COMPUTE_H
> >>-
> >>-#include "nvc0/nvc0_compute.xml.h"
> >>-
> >>-bool
> >>-nvc0_compute_validate_program(struct nvc0_context *nvc0);
> >>-
> >>-#endif /* NVC0_COMPUTE_H */
> >>diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h 
> >>b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
>

Re: [Mesa-dev] [PATCH 3/3] nvc0: rework nvc0_compute_validate_program()

2016-02-24 Thread Samuel Pitoiset 



On 02/24/2016 08:30 PM, Pierre Moreau wrote:

Hi Samuel,

On 06:44 PM - Feb 24 2016, Samuel Pitoiset wrote:

Reduce the amount of duplicated code by re-using
nvc0_program_validate(). While we are at it, change the prototype
to return void and remove nvc0_compute.h which is now useless.


Why don't you want to know whether the validation worked or not? If the
validation failed, the program has a bug and Nouveau shouldn't try to run it,
so allocating buffers that will be unused seems wasteful. Am I missing
something here?


Because it's useless? If the program can't be correctly validated it's 
going to break the universe. If you look at nvc0_gmtyprog_validate() for 
example you will see that I just follow the same design.


Pierre



Signed-off-by: Samuel Pitoiset 
---
  src/gallium/drivers/nouveau/Makefile.sources   |  1 -
  src/gallium/drivers/nouveau/nvc0/nvc0_compute.c| 34 ++
  src/gallium/drivers/nouveau/nvc0/nvc0_compute.h|  9 --
  src/gallium/drivers/nouveau/nvc0/nvc0_context.h|  1 +
  .../drivers/nouveau/nvc0/nvc0_shader_state.c   | 15 ++
  src/gallium/drivers/nouveau/nvc0/nve4_compute.c|  4 +--
  6 files changed, 20 insertions(+), 44 deletions(-)
  delete mode 100644 src/gallium/drivers/nouveau/nvc0/nvc0_compute.h

diff --git a/src/gallium/drivers/nouveau/Makefile.sources 
b/src/gallium/drivers/nouveau/Makefile.sources
index 43ffce6..65f08c7 100644
--- a/src/gallium/drivers/nouveau/Makefile.sources
+++ b/src/gallium/drivers/nouveau/Makefile.sources
@@ -150,7 +150,6 @@ NVC0_C_SOURCES := \
nvc0/gm107_texture.xml.h \
nvc0/nvc0_3d.xml.h \
nvc0/nvc0_compute.c \
-   nvc0/nvc0_compute.h \
nvc0/nvc0_compute.xml.h \
nvc0/nvc0_context.c \
nvc0/nvc0_context.h \
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
index a664aaf..060f59d 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
@@ -23,7 +23,8 @@
   */

  #include "nvc0/nvc0_context.h"
-#include "nvc0/nvc0_compute.h"
+
+#include "nvc0/nvc0_compute.xml.h"

  int
  nvc0_screen_compute_setup(struct nvc0_screen *screen,
@@ -120,34 +121,6 @@ nvc0_screen_compute_setup(struct nvc0_screen *screen,
 return 0;
  }

-bool
-nvc0_compute_validate_program(struct nvc0_context *nvc0)
-{
-   struct nvc0_program *prog = nvc0->compprog;
-
-   if (prog->mem)
-  return true;
-
-   if (!prog->translated) {
-  prog->translated = nvc0_program_translate(
- prog, nvc0->screen->base.device->chipset, >base.debug);
-  if (!prog->translated)
- return false;
-   }
-   if (unlikely(!prog->code_size))
-  return false;
-
-   if (likely(prog->code_size)) {
-  if (nvc0_program_upload_code(nvc0, prog)) {
- struct nouveau_pushbuf *push = nvc0->base.pushbuf;
- BEGIN_NVC0(push, NVC0_CP(FLUSH), 1);
- PUSH_DATA (push, NVC0_COMPUTE_FLUSH_CODE);
- return true;
-  }
-   }
-   return false;
-}
-
  static void
  nvc0_compute_validate_samplers(struct nvc0_context *nvc0)
  {
@@ -292,8 +265,7 @@ nvc0_compute_validate_globals(struct nvc0_context *nvc0)
  static bool
  nvc0_compute_state_validate(struct nvc0_context *nvc0)
  {
-   if (!nvc0_compute_validate_program(nvc0))
-  return false;
+   nvc0_compprog_validate(nvc0);
 if (nvc0->dirty_cp & NVC0_NEW_CP_CONSTBUF)
nvc0_compute_validate_constbufs(nvc0);
 if (nvc0->dirty_cp & NVC0_NEW_CP_DRIVERCONST)
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h 
b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
deleted file mode 100644
index a23f7f3..000
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef NVC0_COMPUTE_H
-#define NVC0_COMPUTE_H
-
-#include "nvc0/nvc0_compute.xml.h"
-
-bool
-nvc0_compute_validate_program(struct nvc0_context *nvc0);
-
-#endif /* NVC0_COMPUTE_H */
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h 
b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
index 7aa4b62..0f1ebb0 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
@@ -254,6 +254,7 @@ void nvc0_tctlprog_validate(struct nvc0_context *);
  void nvc0_tevlprog_validate(struct nvc0_context *);
  void nvc0_gmtyprog_validate(struct nvc0_context *);
  void nvc0_fragprog_validate(struct nvc0_context *);
+void nvc0_compprog_validate(struct nvc0_context *);

  void nvc0_tfb_validate(struct nvc0_context *);

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
index 2f46c43..6b02ed5 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
@@ -28,6 +28,8 @@
  #include "nvc0/nvc0_context.h"
  #include "nvc0/nvc0_query_hw.h"

+#include "nvc0/nvc0_compute.xml.h"
+
  static inline void

Re: [Mesa-dev] [PATCH 3/3] nvc0: rework nvc0_compute_validate_program()

2016-02-24 Thread Pierre Moreau 
Hi Samuel,

On 06:44 PM - Feb 24 2016, Samuel Pitoiset wrote:
> Reduce the amount of duplicated code by re-using
> nvc0_program_validate(). While we are at it, change the prototype
> to return void and remove nvc0_compute.h which is now useless.

Why don't you want to know whether the validation worked or not? If the
validation failed, the program has a bug and Nouveau shouldn't try to run it,
so allocating buffers that will be unused seems wasteful. Am I missing
something here?

Pierre

> 
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/gallium/drivers/nouveau/Makefile.sources   |  1 -
>  src/gallium/drivers/nouveau/nvc0/nvc0_compute.c| 34 
> ++
>  src/gallium/drivers/nouveau/nvc0/nvc0_compute.h|  9 --
>  src/gallium/drivers/nouveau/nvc0/nvc0_context.h|  1 +
>  .../drivers/nouveau/nvc0/nvc0_shader_state.c   | 15 ++
>  src/gallium/drivers/nouveau/nvc0/nve4_compute.c|  4 +--
>  6 files changed, 20 insertions(+), 44 deletions(-)
>  delete mode 100644 src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
> 
> diff --git a/src/gallium/drivers/nouveau/Makefile.sources 
> b/src/gallium/drivers/nouveau/Makefile.sources
> index 43ffce6..65f08c7 100644
> --- a/src/gallium/drivers/nouveau/Makefile.sources
> +++ b/src/gallium/drivers/nouveau/Makefile.sources
> @@ -150,7 +150,6 @@ NVC0_C_SOURCES := \
>   nvc0/gm107_texture.xml.h \
>   nvc0/nvc0_3d.xml.h \
>   nvc0/nvc0_compute.c \
> - nvc0/nvc0_compute.h \
>   nvc0/nvc0_compute.xml.h \
>   nvc0/nvc0_context.c \
>   nvc0/nvc0_context.h \
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> index a664aaf..060f59d 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> @@ -23,7 +23,8 @@
>   */
>  
>  #include "nvc0/nvc0_context.h"
> -#include "nvc0/nvc0_compute.h"
> +
> +#include "nvc0/nvc0_compute.xml.h"
>  
>  int
>  nvc0_screen_compute_setup(struct nvc0_screen *screen,
> @@ -120,34 +121,6 @@ nvc0_screen_compute_setup(struct nvc0_screen *screen,
> return 0;
>  }
>  
> -bool
> -nvc0_compute_validate_program(struct nvc0_context *nvc0)
> -{
> -   struct nvc0_program *prog = nvc0->compprog;
> -
> -   if (prog->mem)
> -  return true;
> -
> -   if (!prog->translated) {
> -  prog->translated = nvc0_program_translate(
> - prog, nvc0->screen->base.device->chipset, >base.debug);
> -  if (!prog->translated)
> - return false;
> -   }
> -   if (unlikely(!prog->code_size))
> -  return false;
> -
> -   if (likely(prog->code_size)) {
> -  if (nvc0_program_upload_code(nvc0, prog)) {
> - struct nouveau_pushbuf *push = nvc0->base.pushbuf;
> - BEGIN_NVC0(push, NVC0_CP(FLUSH), 1);
> - PUSH_DATA (push, NVC0_COMPUTE_FLUSH_CODE);
> - return true;
> -  }
> -   }
> -   return false;
> -}
> -
>  static void
>  nvc0_compute_validate_samplers(struct nvc0_context *nvc0)
>  {
> @@ -292,8 +265,7 @@ nvc0_compute_validate_globals(struct nvc0_context *nvc0)
>  static bool
>  nvc0_compute_state_validate(struct nvc0_context *nvc0)
>  {
> -   if (!nvc0_compute_validate_program(nvc0))
> -  return false;
> +   nvc0_compprog_validate(nvc0);
> if (nvc0->dirty_cp & NVC0_NEW_CP_CONSTBUF)
>nvc0_compute_validate_constbufs(nvc0);
> if (nvc0->dirty_cp & NVC0_NEW_CP_DRIVERCONST)
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
> deleted file mode 100644
> index a23f7f3..000
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.h
> +++ /dev/null
> @@ -1,9 +0,0 @@
> -#ifndef NVC0_COMPUTE_H
> -#define NVC0_COMPUTE_H
> -
> -#include "nvc0/nvc0_compute.xml.h"
> -
> -bool
> -nvc0_compute_validate_program(struct nvc0_context *nvc0);
> -
> -#endif /* NVC0_COMPUTE_H */
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> index 7aa4b62..0f1ebb0 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> @@ -254,6 +254,7 @@ void nvc0_tctlprog_validate(struct nvc0_context *);
>  void nvc0_tevlprog_validate(struct nvc0_context *);
>  void nvc0_gmtyprog_validate(struct nvc0_context *);
>  void nvc0_fragprog_validate(struct nvc0_context *);
> +void nvc0_compprog_validate(struct nvc0_context *);
>  
>  void nvc0_tfb_validate(struct nvc0_context *);
>  
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> index 2f46c43..6b02ed5 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> @@ -28,6 +28,8 @@
>  #include "nvc0/nvc0_context.h"
>  #include "nvc0/nvc0_query_hw.h"
>  
> +#include "nvc0/nvc0_compute.xml.h"
> +
>  static inline void
>