On 05/20/2013 10:31 AM, Paul Berry wrote:
On 17 May 2013 10:17, Kenneth Graunke <kenn...@whitecape.org> wrote:
diff --git a/src/mesa/drivers/dri/i965/brw_context.c
b/src/mesa/drivers/dri/i965/brw_context.c
index 2f5fedb..beade5c 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -88,6 +88,8 @@ static void brwInitDriverFunctions(struct
intel_screen *screen,
brwInitFragProgFuncs( functions );
brw_init_queryobj_functions(functions);
+ if (screen->gen >= 6)
+ gen6_reinit_queryobj_functions(functions);
I find it confusing that we initialize the queryobj function pointers to
the pre-gen6 functions and then immediately override some of them to
gen6+ versions.
How about splitting brw_init_queryobj_functions() into
brw_init_common_queryobj_functions() and
brw_init_pre_gen6_queryobj_functions(), and renaming
gen6_reinit_queryobj_functions() to just gen6_init_queryobj_functions()?
Then this code would change to:
brw_init_common_queryobj_functions()
if (screen->gen < 6)
brw_init_pre_gen6_queryobj_functions()
else
gen6_init_queryobj_functions()
Sure. I've done that for v2.
functions->QuerySamplesForFormat = brw_query_samples_for_format;
functions->BeginTransformFeedback = brw_begin_transform_feedback;
diff --git a/src/mesa/drivers/dri/i965/brw_context.h
b/src/mesa/drivers/dri/i965/brw_context.h
index 9baf57b..9ef6aca 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1164,6 +1164,9 @@ void brw_init_queryobj_functions(struct
dd_function_table *functions);
void brw_emit_query_begin(struct brw_context *brw);
void brw_emit_query_end(struct brw_context *brw);
+/** gen6_queryobj.c */
+void gen6_reinit_queryobj_functions(struct dd_function_table
*functions);
+
/*======================================================================
* brw_state_dump.c
*/
diff --git a/src/mesa/drivers/dri/i965/brw_queryobj.c
b/src/mesa/drivers/dri/i965/brw_queryobj.c
index 40f926b..1c1e0b4 100644
--- a/src/mesa/drivers/dri/i965/brw_queryobj.c
+++ b/src/mesa/drivers/dri/i965/brw_queryobj.c
So is brw_queryobj.c now for pre-Gen6 only? If so, can we please add a
comment to that effect at the top of the file, and remove the remaining
intel->gen checks (e.g. in write_timestamp())? If not, can we somehow
document which functions are Pre-gen6 and which aren't (e.g. with
"assert(intel->gen < 6);" at the top of the pre-gen6 functions)? As it
stands this patch leaves the file in a condition where it's hard to tell
without a deep understanding of the driver which functions are for which
chip generations.
Not completely. The timer query functions are still shared, as are the
new/delete hooks. I've added a bunch of assertions and comments to clarify.
[snip]
case GL_SAMPLES_PASSED_ARB:
Below this code, in the GL_PRIMITIVES_GENERATED and
GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN cases, can we change the
comment so that instead of saying "We don't actually query the hardware
for this value, so query->bo should always be NULL and execution should
never reach here." it says something like "Transform feedback isn't
supported pre-gen6"?
Similarly, there's code in brw_begin_query() and brw_end_query() for
handling GL_PRIMITIVES_GENERATED and
GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN that is now unreachable because
we don't support transform feedback prior to Gen6. I think we should
replace it with a comment to that affect and an assertion.
I've done basically just that in a follow-up patch which reworks a bunch
of the transform feedback stuff. Unless it's a big deal, I'll just plan
on keeping it as a follow-up.
@@ -545,6 +502,9 @@ brw_emit_query_begin(struct brw_context *brw)
struct gl_context *ctx = &intel->ctx;
struct brw_query_object *query = brw->query.obj;
+ if (intel->hw_ctx)
+ return;
+
The comment above brw_emit_query_begin() says that we record
PS_DEPTH_COUNT at the beginning and end of each batch, regardless of
whether hardware contexts are in use. Can we please change the comment
to reflect the new behavioukr?
Done.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
