Re: [Mesa-dev] [PATCH 11/16] nvc0: use of new counter types
On Mon, Jul 7, 2014 at 3:45 PM, Samuel Pitoiset
wrote:
> On 07/07/2014 06:32 PM, Ilia Mirkin wrote:
>>
>> On Mon, Jul 7, 2014 at 11:47 AM, Samuel Pitoiset
>> wrote:
>>>
>>> Signed-off-by: Samuel Pitoiset
>>> ---
>>> src/gallium/drivers/nouveau/nvc0/nvc0_query.c | 19 +--
>>> 1 file changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c
>>> b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c
>>> index 3e8c90b..2ce4378 100644
>>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c
>>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c
>>> @@ -1405,6 +1405,12 @@ nvc0_screen_get_driver_query_info(struct
>>> pipe_screen *pscreen,
>>> if (!info)
>>> return count;
>>>
>>> + /* Init default parameters. */
>>> + info->max_value.ui = 0;
>>> + info->is_percentage = 0;
>>> + info->is_float = 0;
>>> + info->uses_byte_units = FALSE;
>>> +
>>> #ifdef NOUVEAU_ENABLE_DRIVER_STATISTICS
>>> if (id < NVC0_QUERY_DRV_STAT_COUNT) {
>>> info->name = nvc0_drv_stat_names[id];
>>> @@ -1420,9 +1426,13 @@ nvc0_screen_get_driver_query_info(struct
>>> pipe_screen *pscreen,
>>>info->name = nve4_pm_query_names[id -
>>> NVC0_QUERY_DRV_STAT_COUNT];
>>>info->query_type = NVE4_PM_QUERY(id -
>>> NVC0_QUERY_DRV_STAT_COUNT);
>>>info->group_id = NVC0_QUERY_PM_GROUP;
>>> - info->max_value.ui = (id < NVE4_PM_QUERY_METRIC_MP_OCCUPANCY) ?
>>> -~0ULL : 100;
>>> - info->uses_byte_units = FALSE;
>>> + if (id < NVE4_PM_QUERY_METRIC_MP_OCCUPANCY) {
>>> + info->max_value.ui = ~0ULL;
>>> + } else {
>>> + info->max_value.f = 100.0;
>>> + info->is_float = 1;
>>> + info->is_percentage = 1;
>>
>> But the value is still to be returned the same way as a uint64?
>> Presumably you'd want to find a way to use the float bits for more
>> precision than just the integers 0..100, no? Otherwise it seems that
>> the ->is_float thing has no meaning...
>
>
> Yes, the value returned with pipe_driver_query is still a uint64, so this
> code
> is just useless as well. :)
>
> I'll keep the uint64 stuff and improve the precision later.
So then ->is_float is a lie, right? The st might then be tempted to
actually return a floating point value despite it being a u64...
IMHO this series would make the most sense if structured as:
(a) add generic group interface, including the float/etc stuff,
without requiring that a driver actually make use of that. this patch
subgroup should include the patch that updates the begin_query
interface.
(b) add the mesa/st bits to make use of this interface (although it
won't actually get exposed since no drivers would support the new
hotness)
(c) update drivers to implement this interface in a simple manner
(e.g. by just returning what they already return).
(d) make further updates to drivers to implement fanciness like doing
floats/percentages/multiple groups
At least that's how I've been structuring my feature-adding patch
series, and I'm fairly sure others have as well. [And I'll push out
your nvc0 not-enough-space fix soonish, as that's unrelated to this
series and is just a fix in its own right.]
And lastly, I know that it's inconsistently done, but if it's not too
difficult, try to add relevant documentation to the gallium docs
(src/gallium/docs/source, viewable at gallium.rtfd.org). This will
help future driver implementors as well as code reviewers to
understand the intended usage of your interface.
>
>
>>
>>> + }
>>>return 1;
>>> } else
>>> if (screen->compute) {
>>> @@ -1430,7 +1440,6 @@ nvc0_screen_get_driver_query_info(struct
>>> pipe_screen *pscreen,
>>>info->query_type = NVC0_PM_QUERY(id -
>>> NVC0_QUERY_DRV_STAT_COUNT);
>>>info->group_id = NVC0_QUERY_PM_GROUP;
>>>info->max_value.ui = ~0ULL;
>>> - info->uses_byte_units = FALSE;
>>>return 1;
>>> }
>>> }
>>> @@ -1438,8 +1447,6 @@ nvc0_screen_get_driver_query_info(struct
>>> pipe_screen *pscreen,
>>> info->name = "this_is_not_the_query_you_are_looking_for";
>>> info->query_type = 0xdeadd01d;
>>> info->group_id = 0;
>>> - info->max_value.ui = 0;
>>> - info->uses_byte_units = FALSE;
>>> return 0;
>>> }
>>>
>>> --
>>> 2.0.0
>>>
>>> ___
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/16] nvc0: use of new counter types
On 07/07/2014 06:32 PM, Ilia Mirkin wrote:
On Mon, Jul 7, 2014 at 11:47 AM, Samuel Pitoiset
wrote:
Signed-off-by: Samuel Pitoiset
---
src/gallium/drivers/nouveau/nvc0/nvc0_query.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c
b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c
index 3e8c90b..2ce4378 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c
@@ -1405,6 +1405,12 @@ nvc0_screen_get_driver_query_info(struct pipe_screen
*pscreen,
if (!info)
return count;
+ /* Init default parameters. */
+ info->max_value.ui = 0;
+ info->is_percentage = 0;
+ info->is_float = 0;
+ info->uses_byte_units = FALSE;
+
#ifdef NOUVEAU_ENABLE_DRIVER_STATISTICS
if (id < NVC0_QUERY_DRV_STAT_COUNT) {
info->name = nvc0_drv_stat_names[id];
@@ -1420,9 +1426,13 @@ nvc0_screen_get_driver_query_info(struct pipe_screen
*pscreen,
info->name = nve4_pm_query_names[id - NVC0_QUERY_DRV_STAT_COUNT];
info->query_type = NVE4_PM_QUERY(id - NVC0_QUERY_DRV_STAT_COUNT);
info->group_id = NVC0_QUERY_PM_GROUP;
- info->max_value.ui = (id < NVE4_PM_QUERY_METRIC_MP_OCCUPANCY) ?
-~0ULL : 100;
- info->uses_byte_units = FALSE;
+ if (id < NVE4_PM_QUERY_METRIC_MP_OCCUPANCY) {
+ info->max_value.ui = ~0ULL;
+ } else {
+ info->max_value.f = 100.0;
+ info->is_float = 1;
+ info->is_percentage = 1;
But the value is still to be returned the same way as a uint64?
Presumably you'd want to find a way to use the float bits for more
precision than just the integers 0..100, no? Otherwise it seems that
the ->is_float thing has no meaning...
Yes, the value returned with pipe_driver_query is still a uint64, so
this code
is just useless as well. :)
I'll keep the uint64 stuff and improve the precision later.
+ }
return 1;
} else
if (screen->compute) {
@@ -1430,7 +1440,6 @@ nvc0_screen_get_driver_query_info(struct pipe_screen
*pscreen,
info->query_type = NVC0_PM_QUERY(id - NVC0_QUERY_DRV_STAT_COUNT);
info->group_id = NVC0_QUERY_PM_GROUP;
info->max_value.ui = ~0ULL;
- info->uses_byte_units = FALSE;
return 1;
}
}
@@ -1438,8 +1447,6 @@ nvc0_screen_get_driver_query_info(struct pipe_screen
*pscreen,
info->name = "this_is_not_the_query_you_are_looking_for";
info->query_type = 0xdeadd01d;
info->group_id = 0;
- info->max_value.ui = 0;
- info->uses_byte_units = FALSE;
return 0;
}
--
2.0.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/16] nvc0: use of new counter types
On Mon, Jul 7, 2014 at 11:47 AM, Samuel Pitoiset
wrote:
> Signed-off-by: Samuel Pitoiset
> ---
> src/gallium/drivers/nouveau/nvc0/nvc0_query.c | 19 +--
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c
> b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c
> index 3e8c90b..2ce4378 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_query.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_query.c
> @@ -1405,6 +1405,12 @@ nvc0_screen_get_driver_query_info(struct pipe_screen
> *pscreen,
> if (!info)
>return count;
>
> + /* Init default parameters. */
> + info->max_value.ui = 0;
> + info->is_percentage = 0;
> + info->is_float = 0;
> + info->uses_byte_units = FALSE;
> +
> #ifdef NOUVEAU_ENABLE_DRIVER_STATISTICS
> if (id < NVC0_QUERY_DRV_STAT_COUNT) {
>info->name = nvc0_drv_stat_names[id];
> @@ -1420,9 +1426,13 @@ nvc0_screen_get_driver_query_info(struct pipe_screen
> *pscreen,
> info->name = nve4_pm_query_names[id - NVC0_QUERY_DRV_STAT_COUNT];
> info->query_type = NVE4_PM_QUERY(id - NVC0_QUERY_DRV_STAT_COUNT);
> info->group_id = NVC0_QUERY_PM_GROUP;
> - info->max_value.ui = (id < NVE4_PM_QUERY_METRIC_MP_OCCUPANCY) ?
> -~0ULL : 100;
> - info->uses_byte_units = FALSE;
> + if (id < NVE4_PM_QUERY_METRIC_MP_OCCUPANCY) {
> + info->max_value.ui = ~0ULL;
> + } else {
> + info->max_value.f = 100.0;
> + info->is_float = 1;
> + info->is_percentage = 1;
But the value is still to be returned the same way as a uint64?
Presumably you'd want to find a way to use the float bits for more
precision than just the integers 0..100, no? Otherwise it seems that
the ->is_float thing has no meaning...
> + }
> return 1;
>} else
>if (screen->compute) {
> @@ -1430,7 +1440,6 @@ nvc0_screen_get_driver_query_info(struct pipe_screen
> *pscreen,
> info->query_type = NVC0_PM_QUERY(id - NVC0_QUERY_DRV_STAT_COUNT);
> info->group_id = NVC0_QUERY_PM_GROUP;
> info->max_value.ui = ~0ULL;
> - info->uses_byte_units = FALSE;
> return 1;
>}
> }
> @@ -1438,8 +1447,6 @@ nvc0_screen_get_driver_query_info(struct pipe_screen
> *pscreen,
> info->name = "this_is_not_the_query_you_are_looking_for";
> info->query_type = 0xdeadd01d;
> info->group_id = 0;
> - info->max_value.ui = 0;
> - info->uses_byte_units = FALSE;
> return 0;
> }
>
> --
> 2.0.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
