[Mesa-dev,v2,5/9] mesa: Handle QUERY_RESULT_NO_WAIT in GetQueryObject{ui64}v

Submitted by Rafal Mielniczuk on March 27, 2014, 8:59 p.m.

Details

Message ID 1395953950-28609-6-git-send-email-rafal.mielniczuk2@gmail.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Rafal Mielniczuk March 27, 2014, 8:59 p.m.
Just return and do nothing if query result is not yet available

Signed-off-by: Rafal Mielniczuk <rafal.mielniczuk2@gmail.com>
---
 src/mesa/main/queryobj.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c
index 86e7c3a..d2d9fa7 100644
--- a/src/mesa/main/queryobj.c
+++ b/src/mesa/main/queryobj.c
@@ -594,6 +594,10 @@  _mesa_GetQueryObjectiv(GLuint id, GLenum pname, GLint *params)
    }
 
    switch (pname) {
+      case GL_QUERY_RESULT_NO_WAIT:
+         if (!q->Ready)
+            return;
+         //else fall through
       case GL_QUERY_RESULT_ARB:
          if (!q->Ready)
             ctx->Driver.WaitQuery(ctx, q);
@@ -645,6 +649,10 @@  _mesa_GetQueryObjectuiv(GLuint id, GLenum pname, GLuint *params)
    }
 
    switch (pname) {
+      case GL_QUERY_RESULT_NO_WAIT:
+         if (!q->Ready)
+            return;
+         //else fall through
       case GL_QUERY_RESULT_ARB:
          if (!q->Ready)
             ctx->Driver.WaitQuery(ctx, q);
@@ -699,6 +707,10 @@  _mesa_GetQueryObjecti64v(GLuint id, GLenum pname, GLint64EXT *params)
    }
 
    switch (pname) {
+      case GL_QUERY_RESULT_NO_WAIT:
+         if (!q->Ready)
+            return;
+         //else fall through
       case GL_QUERY_RESULT_ARB:
          if (!q->Ready)
             ctx->Driver.WaitQuery(ctx, q);
@@ -739,6 +751,10 @@  _mesa_GetQueryObjectui64v(GLuint id, GLenum pname, GLuint64EXT *params)
    }
 
    switch (pname) {
+      case GL_QUERY_RESULT_NO_WAIT:
+         if (!q->Ready)
+            return;
+         //else fall through
       case GL_QUERY_RESULT_ARB:
          if (!q->Ready)
             ctx->Driver.WaitQuery(ctx, q);

Comments

On 03/27/2014 01:59 PM, Rafal Mielniczuk wrote:
> Just return and do nothing if query result is not yet available
> 
> Signed-off-by: Rafal Mielniczuk <rafal.mielniczuk2@gmail.com>
> ---
>  src/mesa/main/queryobj.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c
> index 86e7c3a..d2d9fa7 100644
> --- a/src/mesa/main/queryobj.c
> +++ b/src/mesa/main/queryobj.c
> @@ -594,6 +594,10 @@ _mesa_GetQueryObjectiv(GLuint id, GLenum pname, GLint *params)
>     }
>  
>     switch (pname) {
> +      case GL_QUERY_RESULT_NO_WAIT:
> +         if (!q->Ready)
> +            return;
> +         //else fall through

We don't usually use C++ style comments in Mesa.  I would do:

case GL_QUERY_NO_WAIT:
   if (!q->Ready)
       return;
   /* fallthrough */
case GL_QUERY_RESULT_ARB:

Other than that, patches 1-6 are:
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

>        case GL_QUERY_RESULT_ARB:
>           if (!q->Ready)
>              ctx->Driver.WaitQuery(ctx, q);
> @@ -645,6 +649,10 @@ _mesa_GetQueryObjectuiv(GLuint id, GLenum pname, GLuint *params)
>     }
>  
>     switch (pname) {
> +      case GL_QUERY_RESULT_NO_WAIT:
> +         if (!q->Ready)
> +            return;
> +         //else fall through
>        case GL_QUERY_RESULT_ARB:
>           if (!q->Ready)
>              ctx->Driver.WaitQuery(ctx, q);
> @@ -699,6 +707,10 @@ _mesa_GetQueryObjecti64v(GLuint id, GLenum pname, GLint64EXT *params)
>     }
>  
>     switch (pname) {
> +      case GL_QUERY_RESULT_NO_WAIT:
> +         if (!q->Ready)
> +            return;
> +         //else fall through
>        case GL_QUERY_RESULT_ARB:
>           if (!q->Ready)
>              ctx->Driver.WaitQuery(ctx, q);
> @@ -739,6 +751,10 @@ _mesa_GetQueryObjectui64v(GLuint id, GLenum pname, GLuint64EXT *params)
>     }
>  
>     switch (pname) {
> +      case GL_QUERY_RESULT_NO_WAIT:
> +         if (!q->Ready)
> +            return;
> +         //else fall through
>        case GL_QUERY_RESULT_ARB:
>           if (!q->Ready)
>              ctx->Driver.WaitQuery(ctx, q);
>
On 03/27/2014 11:34 PM, Kenneth Graunke wrote:
> On 03/27/2014 01:59 PM, Rafal Mielniczuk wrote:
>> Just return and do nothing if query result is not yet available
>>
>> Signed-off-by: Rafal Mielniczuk <rafal.mielniczuk2@gmail.com>
>> ---
>>  src/mesa/main/queryobj.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c
>> index 86e7c3a..d2d9fa7 100644
>> --- a/src/mesa/main/queryobj.c
>> +++ b/src/mesa/main/queryobj.c
>> @@ -594,6 +594,10 @@ _mesa_GetQueryObjectiv(GLuint id, GLenum pname, GLint *params)
>>     }
>>  
>>     switch (pname) {
>> +      case GL_QUERY_RESULT_NO_WAIT:
>> +         if (!q->Ready)
>> +            return;
>> +         //else fall through
> 
> We don't usually use C++ style comments in Mesa.  I would do:
> 
> case GL_QUERY_NO_WAIT:
>    if (!q->Ready)
>        return;
>    /* fallthrough */
> case GL_QUERY_RESULT_ARB:
> 
> Other than that, patches 1-6 are:
> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

Actually, I take that back...I don't think this is what we want for GPU
drivers.  (It's probably reasonable for software drivers though.)

When a buffer object is bound to GL_QUERY_BUFFER, the idea is that the
GL_QUERY_RESULT/GL_QUERY_RESULT_NO_WAIT queries should emit GPU commands
to deliver the query result into the buffer object.

The query result may not actually be available yet (so, q->Ready ==
false), but the GPU commands to obtain the result have already been
submitted.  Since any GPU commands we submit will happen after those,
they can work with the result as if it's available...because it will be
by the time they run.

At least, that's my understanding right now.  So, we need a way to know
if a query result is "in flight, but done" (i.e. all commands to compute
it have been submitted, but may not have run yet), and a way to ask the
driver to deliver it to a particular buffer object/offset.  That
probably means two new driver hooks, but I'm not quite sure what they
should look like just yet.

So, patches 1-4 and 6 are:
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

I won't be around next week, but I'd be happy to help look into this
when I'm back.  (Unless, of course, others beat me to it...) :)

Thanks again for your work on this!

--Ken
On 28.03.2014 08:01, Kenneth Graunke wrote:
> On 03/27/2014 11:34 PM, Kenneth Graunke wrote:
>> On 03/27/2014 01:59 PM, Rafal Mielniczuk wrote:
>>> Just return and do nothing if query result is not yet available
>>>
>>> Signed-off-by: Rafal Mielniczuk <rafal.mielniczuk2@gmail.com>
>>> ---
>>>   src/mesa/main/queryobj.c | 16 ++++++++++++++++
>>>   1 file changed, 16 insertions(+)
>>>
>>> diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c
>>> index 86e7c3a..d2d9fa7 100644
>>> --- a/src/mesa/main/queryobj.c
>>> +++ b/src/mesa/main/queryobj.c
>>> @@ -594,6 +594,10 @@ _mesa_GetQueryObjectiv(GLuint id, GLenum pname, GLint *params)
>>>      }
>>>   
>>>      switch (pname) {
>>> +      case GL_QUERY_RESULT_NO_WAIT:
>>> +         if (!q->Ready)
>>> +            return;
>>> +         //else fall through
>> We don't usually use C++ style comments in Mesa.  I would do:
>>
>> case GL_QUERY_NO_WAIT:
>>     if (!q->Ready)
>>         return;
>>     /* fallthrough */
>> case GL_QUERY_RESULT_ARB:
>>
>> Other than that, patches 1-6 are:
>> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
> Actually, I take that back...I don't think this is what we want for GPU
> drivers.  (It's probably reasonable for software drivers though.)
>
> When a buffer object is bound to GL_QUERY_BUFFER, the idea is that the
> GL_QUERY_RESULT/GL_QUERY_RESULT_NO_WAIT queries should emit GPU commands
> to deliver the query result into the buffer object.
>
> The query result may not actually be available yet (so, q->Ready ==
> false), but the GPU commands to obtain the result have already been
> submitted.  Since any GPU commands we submit will happen after those,
> they can work with the result as if it's available...because it will be
> by the time they run.
>
> At least, that's my understanding right now.  So, we need a way to know
> if a query result is "in flight, but done" (i.e. all commands to compute
> it have been submitted, but may not have run yet), and a way to ask the
> driver to deliver it to a particular buffer object/offset.  That
> probably means two new driver hooks, but I'm not quite sure what they
> should look like just yet.

Ok, I see there is no point in adding software version of the extension 
only.
I will play with it in the comming days, try to understand the driver side
more and come up with something working, hopefully.

>
> So, patches 1-4 and 6 are:
> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
>
> I won't be around next week, but I'd be happy to help look into this
> when I'm back.  (Unless, of course, others beat me to it...) :)
>
> Thanks again for your work on this!

Thanks for your time and extensive review! :)
I would be happy to finish this up.

Thanks again,
Rafal
>
> --Ken
>