Re: [Mesa-dev] [PATCH 27/27] i965: Enable the AMD_performance_monitor extension on Gen5+.

2013-11-18 Thread Eric Anholt
Kenneth Graunke kenn...@whitecape.org writes:

 XXX: Gen6+ needs to be predicated on register writes.

 our register write checking function doesn't work on Gen6.

Even if you can just enable it on gen7, this series is:

Reviewed-by: Eric Anholt e...@anholt.net



pgpu618mL0Xbn.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 27/27] i965: Enable the AMD_performance_monitor extension on Gen5+.

2013-11-18 Thread Kenneth Graunke
On 11/18/2013 10:33 AM, Eric Anholt wrote:
 Kenneth Graunke kenn...@whitecape.org writes:
 
 XXX: Gen6+ needs to be predicated on register writes.

 our register write checking function doesn't work on Gen6.
 
 Even if you can just enable it on gen7, this series is:
 
 Reviewed-by: Eric Anholt e...@anholt.net

Now I'm confused.  I thought you and Carl found regressions in patch 3
(the tri-state ring enum patch), and that you basically NAK'd patch 04
because it adds code to BEGIN_BATCH.

I had thought I needed to rewrite patch 4 before I could upstream this.
 Please clarify.

Thanks,
--Ken
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 27/27] i965: Enable the AMD_performance_monitor extension on Gen5+.

2013-11-18 Thread Eric Anholt
Kenneth Graunke kenn...@whitecape.org writes:

 On 11/18/2013 10:33 AM, Eric Anholt wrote:
 Kenneth Graunke kenn...@whitecape.org writes:
 
 XXX: Gen6+ needs to be predicated on register writes.

 our register write checking function doesn't work on Gen6.
 
 Even if you can just enable it on gen7, this series is:
 
 Reviewed-by: Eric Anholt e...@anholt.net

 Now I'm confused.  I thought you and Carl found regressions in patch 3
 (the tri-state ring enum patch), and that you basically NAK'd patch 04
 because it adds code to BEGIN_BATCH.

 I had thought I needed to rewrite patch 4 before I could upstream this.
  Please clarify.

We found some slight flushing behavior change in patch 3, which we
talked over and I thought you'd squashed in the fix for already (the
missed true/false - *_RING).

As far as patch 4: I'd almost always rather avoid BEGIN_BATCH overhead
since we call it so much, but the last other solution we talked about
(explicit ring switching) seemed like a scary maintenance problem
because you wouldn't notice when you forgot to add a switch to render,
since the ring's almost always in render already anyway.


pgp3PLfJluFyn.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 27/27] i965: Enable the AMD_performance_monitor extension on Gen5+.

2013-11-18 Thread Kenneth Graunke
On 11/18/2013 05:22 PM, Eric Anholt wrote:
 Kenneth Graunke kenn...@whitecape.org writes:
 
 On 11/18/2013 10:33 AM, Eric Anholt wrote:
 Kenneth Graunke kenn...@whitecape.org writes:

 XXX: Gen6+ needs to be predicated on register writes.

 our register write checking function doesn't work on Gen6.

 Even if you can just enable it on gen7, this series is:

 Reviewed-by: Eric Anholt e...@anholt.net

 Now I'm confused.  I thought you and Carl found regressions in patch 3
 (the tri-state ring enum patch), and that you basically NAK'd patch 04
 because it adds code to BEGIN_BATCH.

 I had thought I needed to rewrite patch 4 before I could upstream this.
  Please clarify.
 
 We found some slight flushing behavior change in patch 3, which we
 talked over and I thought you'd squashed in the fix for already (the
 missed true/false - *_RING).

Right.  I don't remember hearing conclusively that it was resolved.

 As far as patch 4: I'd almost always rather avoid BEGIN_BATCH overhead
 since we call it so much, but the last other solution we talked about
 (explicit ring switching) seemed like a scary maintenance problem
 because you wouldn't notice when you forgot to add a switch to render,
 since the ring's almost always in render already anyway.

Okay, I thought you NAK'd both solutions and were hoping I would come up
with something else.  I guess we can go with this for now, and improve
later...

I did run OpenArena with INTEL_NO_HW=1 on my Ivybridge with master vs.
explicit ring switches, and saw no difference (n=9).  Hardly conclusive,
but removing the existing implicit-flush overhead didn't seem to be much
of win.

--Ken
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 27/27] i965: Enable the AMD_performance_monitor extension on Gen5+.

2013-11-13 Thread Kenneth Graunke
XXX: Gen6+ needs to be predicated on register writes.

our register write checking function doesn't work on Gen6.

(this patch will be replaced; I'd just like to send the others up for
review sooner rather than later)

Signed-off-by: Kenneth Graunke kenn...@whitecape.org
Cc: Eric Anholt e...@anholt.net
Cc: Carl Worth cwo...@cworth.org
Cc: Juha-Pekka Heikkilä juha-pekka.heikk...@intel.com
---
 src/mesa/drivers/dri/i965/intel_extensions.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
b/src/mesa/drivers/dri/i965/intel_extensions.c
index 62c0b15..2b07fc6 100644
--- a/src/mesa/drivers/dri/i965/intel_extensions.c
+++ b/src/mesa/drivers/dri/i965/intel_extensions.c
@@ -222,6 +222,7 @@ intelInitExtensions(struct gl_context *ctx)
   ctx-Extensions.EXT_timer_query = true;
   ctx-Extensions.EXT_shader_integer_mix = ctx-Const.GLSLVersion = 130;
   ctx-Extensions.ARB_texture_query_levels = ctx-Const.GLSLVersion = 130;
+  ctx-Extensions.AMD_performance_monitor = true;
}
 
if (brw-gen = 7) {
-- 
1.8.3.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev