Re: [Mesa-dev] [PATCH 11/16] nvc0: use of new counter types

2014-07-07 Thread Ilia Mirkin
On Mon, Jul 7, 2014 at 11:47 AM, Samuel Pitoiset
samuel.pitoi...@gmail.com wrote:
 Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com
 ---
  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


Re: [Mesa-dev] [PATCH 11/16] nvc0: use of new counter types

2014-07-07 Thread Samuel Pitoiset

On 07/07/2014 06:32 PM, Ilia Mirkin wrote:

On Mon, Jul 7, 2014 at 11:47 AM, Samuel Pitoiset
samuel.pitoi...@gmail.com wrote:

Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com
---
  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

2014-07-07 Thread Ilia Mirkin
On Mon, Jul 7, 2014 at 3:45 PM, Samuel Pitoiset
samuel.pitoi...@gmail.com wrote:
 On 07/07/2014 06:32 PM, Ilia Mirkin wrote:

 On Mon, Jul 7, 2014 at 11:47 AM, Samuel Pitoiset
 samuel.pitoi...@gmail.com wrote:

 Signed-off-by: Samuel Pitoiset samuel.pitoi...@gmail.com
 ---
   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