[Mesa-dev] [PATCH 08/15] svga: implement pipe_screen::get_driver_query_group_info v3

[Marek Olšák]

Ah, alright. It was just a minor suggestion.

I didn't have a look at all the patches, but I did review some of them
and they looked good to me, including this one.

Marek

On Fri, Jul 11, 2014 at 2:23 PM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
> On 07/10/2014 12:42 AM, Marek Olšák wrote:
>>
>> Well, the array can always be moved outside of the function and be
>> declared as static.
>
>
> Yes, that's possible for svga and freedreno, but not for radeon because the
> parameter max_value of some counters depends of the screen. I prefer to keep
> these constants like that, except if you are really disagree with them.
>
>
>
>>
>> Marek
>>
>> On Thu, Jul 10, 2014 at 12:27 AM, Samuel Pitoiset
>> <samuel.pitoiset at gmail.com> wrote:
>>>
>>> On 07/09/2014 09:49 PM, Marek Olšák wrote:
>>>>
>>>> If the driver queries are defined as an array, you can also use the
>>>> Elements macro on the array.
>>>
>>>
>>> For sure, but we can't do that here since the array is local to the
>>> get_driver_query_info() callback.
>>>
>>>
>>>> Marek
>>>>
>>>> On Wed, Jul 9, 2014 at 3:41 PM, Samuel Pitoiset
>>>> <samuel.pitoiset at gmail.com> wrote:
>>>>>
>>>>> On 07/09/2014 03:00 PM, Brian Paul wrote:
>>>>>>
>>>>>> On 07/09/2014 08:34 AM, Samuel Pitoiset wrote:
>>>>>>>
>>>>>>> This enables GL_AMD_performance_monitor for svga.
>>>>>>>
>>>>>>> V2:
>>>>>>>     - s/pipe_context/pipe_screen in the commit msg
>>>>>>>
>>>>>>> V3:
>>>>>>>     - use util_get_driver_query_group_info
>>>>>>>
>>>>>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>>>>>> ---
>>>>>>>     src/gallium/drivers/svga/svga_screen.c | 11 +++++++++++
>>>>>>>     1 file changed, 11 insertions(+)
>>>>>>>
>>>>>>> diff --git a/src/gallium/drivers/svga/svga_screen.c
>>>>>>> b/src/gallium/drivers/svga/svga_screen.c
>>>>>>> index f34664d..29257ab 100644
>>>>>>> --- a/src/gallium/drivers/svga/svga_screen.c
>>>>>>> +++ b/src/gallium/drivers/svga/svga_screen.c
>>>>>>> @@ -28,6 +28,7 @@
>>>>>>>     #include "util/u_inlines.h"
>>>>>>>     #include "util/u_string.h"
>>>>>>>     #include "util/u_math.h"
>>>>>>> +#include "util/u_query.h"
>>>>>>>
>>>>>>>     #include "svga_winsys.h"
>>>>>>>     #include "svga_public.h"
>>>>>>> @@ -555,6 +556,15 @@ svga_get_driver_query_info(struct pipe_screen
>>>>>>> *screen,
>>>>>>>     }
>>>>>>>
>>>>>>>
>>>>>>> +static int
>>>>>>> +svga_get_driver_query_group_info(struct pipe_screen *screen,
>>>>>>> +                                 unsigned index,
>>>>>>> +                                 struct pipe_driver_query_group_info
>>>>>>> *info)
>>>>>>> +{
>>>>>>> +   return util_get_driver_query_group_info(index, 3, info);
>>>>>>
>>>>>>
>>>>>> What does 3 indicate?  Please add some documentation for this.
>>>>>
>>>>>
>>>>> Yes, it's the number of driver-specific queries.
>>>>>
>>>>>
>>>>>> I suspect it's the number of driver-specific queries supported. If so,
>>>>>> it
>>>>>> would be better to have a new #define SVGA_QUERY_COUNT 3 in
>>>>>> svga_context.h
>>>>>> so that if someone adds a new query they won't have to hunt down this
>>>>>> magic
>>>>>> number here.
>>>>>
>>>>>
>>>>> Okay, I'll add SVGA_QUERY_COUNT and same for others drivers which use
>>>>> that
>>>>> extension (freedreno, radeon and nvc0).
>>>>>
>>>>> Thanks.
>>>>>
>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>>
>>>>>>> +}
>>>>>>> +
>>>>>>> +
>>>>>>>     static void
>>>>>>>     svga_destroy_screen( struct pipe_screen *screen )
>>>>>>>     {
>>>>>>> @@ -615,6 +625,7 @@ svga_screen_create(struct svga_winsys_screen
>>>>>>> *sws)
>>>>>>>        screen->fence_signalled = svga_fence_signalled;
>>>>>>>        screen->fence_finish = svga_fence_finish;
>>>>>>>        screen->get_driver_query_info = svga_get_driver_query_info;
>>>>>>> +   screen->get_driver_query_group_info =
>>>>>>> svga_get_driver_query_group_info;
>>>>>>>        svgascreen->sws = sws;
>>>>>>>
>>>>>>>        svga_init_screen_resource_functions(svgascreen);
>>>>>>>
>>>>>> _______________________________________________
>>>>>> mesa-dev mailing list
>>>>>> mesa-dev at lists.freedesktop.org
>>>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> mesa-dev mailing list
>>>>> mesa-dev at lists.freedesktop.org
>>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>>
>