Re: [Mesa-dev] [PATCH] st/mesa: fix PRIMITIVES_GENERATED query after the "pipeline stat single" changes

2019-01-23 Thread Kenneth Graunke 
On Wednesday, January 23, 2019 1:26:25 AM PST Erik Faye-Lund wrote:
> On Fri, 2019-01-18 at 11:27 -0500, Marek Olšák wrote:
> > From: Marek Olšák 
> > 
> > ---
> >  src/mesa/state_tracker/st_cb_queryobj.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/mesa/state_tracker/st_cb_queryobj.c
> > b/src/mesa/state_tracker/st_cb_queryobj.c
> > index abb126547c9..642b901d05a 100644
> > --- a/src/mesa/state_tracker/st_cb_queryobj.c
> > +++ b/src/mesa/state_tracker/st_cb_queryobj.c
> > @@ -84,21 +84,22 @@ st_DeleteQuery(struct gl_context *ctx, struct
> > gl_query_object *q)
> > struct st_query_object *stq = st_query_object(q);
> >  
> > free_queries(pipe, stq);
> >  
> > free(stq);
> >  }
> >  
> >  static int
> >  target_to_index(const struct st_context *st, const struct
> > gl_query_object *q)
> >  {
> > -   if (q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
> > +   if (q->Target == GL_PRIMITIVES_GENERATED ||
> > +   q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
> > q->Target == GL_TRANSFORM_FEEDBACK_STREAM_OVERFLOW_ARB)
> >return q->Stream;
> >  
> > if (st->has_single_pipe_stat) {
> >switch (q->Target) {
> >case GL_VERTICES_SUBMITTED_ARB:
> >   return PIPE_STAT_QUERY_IA_VERTICES;
> >case GL_PRIMITIVES_SUBMITTED_ARB:
> >   return PIPE_STAT_QUERY_IA_PRIMITIVES;
> >case GL_VERTEX_SHADER_INVOCATIONS_ARB:
> 
> The change itself looks good to me.
> 
> However, I think a commit message saying, well, *something* would be a
> good idea (and probably would have made it easier to find reviewers in
> the first place). Something like this, perhaps?
> 
> "When this functionality was added, the PRIMITIVES_GENERATED query was
> accidentally omitted. This causes issues for drivers that support
> transform feedback."
> 
> I also think this should have a Fixes tag:
> 
> Fixes: d644698b443 ("gallium: Add the ability to query a single
> pipeline statistics counter")
> 
> With those things changed:
> 
> Reviewed-by: Erik Faye-Lund 
> 
> Also, I added Kenneth Grauke who wrote the commit in question to the CC
> list. Perhaps he has some thoughts.

Yikes.  Sorry, Marek, not sure how I didn't catch this. :(

Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/mesa: fix PRIMITIVES_GENERATED query after the "pipeline stat single" changes

2019-01-23 Thread Erik Faye-Lund 
On Fri, 2019-01-18 at 11:27 -0500, Marek Olšák wrote:
> From: Marek Olšák 
> 
> ---
>  src/mesa/state_tracker/st_cb_queryobj.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/state_tracker/st_cb_queryobj.c
> b/src/mesa/state_tracker/st_cb_queryobj.c
> index abb126547c9..642b901d05a 100644
> --- a/src/mesa/state_tracker/st_cb_queryobj.c
> +++ b/src/mesa/state_tracker/st_cb_queryobj.c
> @@ -84,21 +84,22 @@ st_DeleteQuery(struct gl_context *ctx, struct
> gl_query_object *q)
> struct st_query_object *stq = st_query_object(q);
>  
> free_queries(pipe, stq);
>  
> free(stq);
>  }
>  
>  static int
>  target_to_index(const struct st_context *st, const struct
> gl_query_object *q)
>  {
> -   if (q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
> +   if (q->Target == GL_PRIMITIVES_GENERATED ||
> +   q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
> q->Target == GL_TRANSFORM_FEEDBACK_STREAM_OVERFLOW_ARB)
>return q->Stream;
>  
> if (st->has_single_pipe_stat) {
>switch (q->Target) {
>case GL_VERTICES_SUBMITTED_ARB:
>   return PIPE_STAT_QUERY_IA_VERTICES;
>case GL_PRIMITIVES_SUBMITTED_ARB:
>   return PIPE_STAT_QUERY_IA_PRIMITIVES;
>case GL_VERTEX_SHADER_INVOCATIONS_ARB:

The change itself looks good to me.

However, I think a commit message saying, well, *something* would be a
good idea (and probably would have made it easier to find reviewers in
the first place). Something like this, perhaps?

"When this functionality was added, the PRIMITIVES_GENERATED query was
accidentally omitted. This causes issues for drivers that support
transform feedback."

I also think this should have a Fixes tag:

Fixes: d644698b443 ("gallium: Add the ability to query a single
pipeline statistics counter")

With those things changed:

Reviewed-by: Erik Faye-Lund 

Also, I added Kenneth Grauke who wrote the commit in question to the CC
list. Perhaps he has some thoughts.

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

Re: [Mesa-dev] [PATCH] st/mesa: fix PRIMITIVES_GENERATED query after the "pipeline stat single" changes

2019-01-22 Thread Marek Olšák 
ping

On Fri, Jan 18, 2019 at 11:27 AM Marek Olšák  wrote:

> From: Marek Olšák 
>
> ---
>  src/mesa/state_tracker/st_cb_queryobj.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/state_tracker/st_cb_queryobj.c
> b/src/mesa/state_tracker/st_cb_queryobj.c
> index abb126547c9..642b901d05a 100644
> --- a/src/mesa/state_tracker/st_cb_queryobj.c
> +++ b/src/mesa/state_tracker/st_cb_queryobj.c
> @@ -84,21 +84,22 @@ st_DeleteQuery(struct gl_context *ctx, struct
> gl_query_object *q)
> struct st_query_object *stq = st_query_object(q);
>
> free_queries(pipe, stq);
>
> free(stq);
>  }
>
>  static int
>  target_to_index(const struct st_context *st, const struct gl_query_object
> *q)
>  {
> -   if (q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
> +   if (q->Target == GL_PRIMITIVES_GENERATED ||
> +   q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
> q->Target == GL_TRANSFORM_FEEDBACK_STREAM_OVERFLOW_ARB)
>return q->Stream;
>
> if (st->has_single_pipe_stat) {
>switch (q->Target) {
>case GL_VERTICES_SUBMITTED_ARB:
>   return PIPE_STAT_QUERY_IA_VERTICES;
>case GL_PRIMITIVES_SUBMITTED_ARB:
>   return PIPE_STAT_QUERY_IA_PRIMITIVES;
>case GL_VERTEX_SHADER_INVOCATIONS_ARB:
> --
> 2.17.1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] st/mesa: fix PRIMITIVES_GENERATED query after the "pipeline stat single" changes

2019-01-18 Thread Marek Olšák 
From: Marek Olšák 

---
 src/mesa/state_tracker/st_cb_queryobj.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/mesa/state_tracker/st_cb_queryobj.c 
b/src/mesa/state_tracker/st_cb_queryobj.c
index abb126547c9..642b901d05a 100644
--- a/src/mesa/state_tracker/st_cb_queryobj.c
+++ b/src/mesa/state_tracker/st_cb_queryobj.c
@@ -84,21 +84,22 @@ st_DeleteQuery(struct gl_context *ctx, struct 
gl_query_object *q)
struct st_query_object *stq = st_query_object(q);
 
free_queries(pipe, stq);
 
free(stq);
 }
 
 static int
 target_to_index(const struct st_context *st, const struct gl_query_object *q)
 {
-   if (q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
+   if (q->Target == GL_PRIMITIVES_GENERATED ||
+   q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
q->Target == GL_TRANSFORM_FEEDBACK_STREAM_OVERFLOW_ARB)
   return q->Stream;
 
if (st->has_single_pipe_stat) {
   switch (q->Target) {
   case GL_VERTICES_SUBMITTED_ARB:
  return PIPE_STAT_QUERY_IA_VERTICES;
   case GL_PRIMITIVES_SUBMITTED_ARB:
  return PIPE_STAT_QUERY_IA_PRIMITIVES;
   case GL_VERTEX_SHADER_INVOCATIONS_ARB:
-- 
2.17.1

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