On Mon, Jan 11, 2016 at 2:21 PM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
> On 11.01.2016 14:01, Ilia Mirkin wrote:
>>
>> On Mon, Jan 11, 2016 at 1:53 PM, Nicolai Hähnle <nhaeh...@gmail.com>
>> wrote:
>>>
>>> On 10.01.2016 00:14, Ilia Mirkin wrote:
>>>>
>>>>
>>>> Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu>
>>>> ---
>>>> src/gallium/docs/source/context.rst | 5 +++++
>>>> src/gallium/include/pipe/p_context.h | 24 ++++++++++++++++++++++++
>>>> src/gallium/include/pipe/p_defines.h | 2 ++
>>>> 3 files changed, 31 insertions(+)
>>>>
>>>> diff --git a/src/gallium/docs/source/context.rst
>>>> b/src/gallium/docs/source/context.rst
>>>> index 9a32716..a97308a 100644
>>>> --- a/src/gallium/docs/source/context.rst
>>>> +++ b/src/gallium/docs/source/context.rst
>>>> @@ -325,6 +325,11 @@ returned). Otherwise, if the ``wait`` parameter is
>>>> FALSE, the call
>>>> will not block and the return value will be TRUE if the query has
>>>> completed or FALSE otherwise.
>>>>
>>>> +``get_query_result_resource`` is used to store the result of a query
>>>> into
>>>> +a resource without synchronizing with the CPU. This write will
>>>> optionally
>>>> +wait for the query to complete, and will optionally write whether the
>>>> value
>>>> +is available instead of the value itself.
>>>> +
>>>> The interface currently includes the following types of queries:
>>>>
>>>> ``PIPE_QUERY_OCCLUSION_COUNTER`` counts the number of fragments which
>>>> diff --git a/src/gallium/include/pipe/p_context.h
>>>> b/src/gallium/include/pipe/p_context.h
>>>> index be7447d..4f21d37 100644
>>>> --- a/src/gallium/include/pipe/p_context.h
>>>> +++ b/src/gallium/include/pipe/p_context.h
>>>> @@ -150,6 +150,30 @@ struct pipe_context {
>>>> struct pipe_query *q,
>>>> boolean wait,
>>>> union pipe_query_result *result);
>>>> +
>>>> + /**
>>>> + * Get results of a query, storing into resource.
>>>> + * \param wait if true, this query will block until the result is
>>>> ready
>>>> + * \param result_type the type of the value being stored:
>>>> + * 0 - i32
>>>> + * 1 - u32
>>>> + * 2 - i64
>>>> + * 3 - u64
>>>
>>>
>>>
>>> Magic numbers :(
>>
>>
>> Documented! :)
>>
>>> Maybe use an enum? Or maybe the signed vs. unsigned distinction isn't
>>> really
>>> needed and you can go with the pipe_index_buffer precedence of using
>>> result_size instead (4 for i32, 8 for i64).
>>
>>
>> Seemed overkill to create defines or an enum for this. But I suppose I
>> can if others also like having enums for small lists of values used in
>> just a handful of places.
>>
>> The GL logic clamps i32 and u32 to their respective ranges (on the
>> upper end). I suspect this is done for a reason and a similar clamping
>> should be done by this function as well. If we all collectively agree
>> that we don't care about clamping, result_size sounds good to me.
>
>
> No, you're right, this matters for number of samples passed and so on. I
> know I'm being annoying, but I think this should be an enum.
>
> Upon further thought, two more issues:
>
> 1) Interaction with batch queries. For now, you'll probably just want to
> document that get_query_result_resource cannot be used with them.
Batch queries like pipeline stats? That's what the index is for. Or is
there something else?
>
> 2) Do we want to be more precise about what 'wait' means? AFAIU,
> get_query_result_resource should _never_ block the CPU, because it should be
> implemented by some command sent to the GPU (perhaps a small compute shader
> or transform feedback thing). The only way this command would not have the
> query results available are if (a) the downstream part of the pipeline is
> still running some other command or (b) the driver decides to use a separate
> ring buffer to implement this.
wait means whatever you want it to mean. At least that's what the GL
spec says. You can still block if wait = false. But yeah, ideally this
function should *never* block, although the whole wait thing seems a
bit unclear. I had originally wanted to just always write true for it
in the core, but then decided that was a bit much.
>
> So in a hardware implementation, the different between wait = true vs. false
> is that you'd issue a pipeline flush before the get_query_result_resource
> commands. Right?
I think so. My plan is to make available check whether the fence
attached to the query has passed, and for wait to stick in a sync into
the pipeline. All done with a macro or two. (Macros allow configurable
commands to be created on the GPU, evaluated by the graphics engine.)
-ilia
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
