<div dir="ltr">On 1 February 2014 23:21, Jordan Justen <span dir="ltr"><<a href="mailto:jljusten@gmail.com" target="_blank">jljusten@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="im">On Thu, Jan 9, 2014 at 6:19 PM, Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> wrote:<br>
> This will allow testing of compute shader functionality before it is<br>
> completed.<br>
><br>
> To enable ARB_compute_shader functionality in the i965 driver, set<br>
> INTEL_COMPUTE_SHADER=1.<br>
> ---<br>
> src/mesa/drivers/dri/i965/brw_context.c | 11 ++++++++++-<br>
> src/mesa/drivers/dri/i965/intel_extensions.c | 2 ++<br>
> 2 files changed, 12 insertions(+), 1 deletion(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c<br>
> index 1b42751..76dd9be 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_context.c<br>
> +++ b/src/mesa/drivers/dri/i965/brw_context.c<br>
> @@ -298,10 +298,17 @@ brw_initialize_context_constants(struct brw_context *brw)<br>
> ctx->Const.Program[MESA_SHADER_GEOMETRY].MaxTextureImageUnits = BRW_MAX_TEX_UNIT;<br>
> else<br>
> ctx->Const.Program[MESA_SHADER_GEOMETRY].MaxTextureImageUnits = 0;<br>
> + if (getenv("INTEL_COMPUTE_SHADER")) {<br>
<br>
</div>What about trying to make use of MESA_EXTENSION_OVERRIDE=GL_ARB_compute_shader?<br>
<br>
We could add<br>
extensions.c:bool _mesa_is_extension_override_enabled(char *)<br>
<br>
And then<br>
if (_mesa_is_extension_override_enabled("GL_ARB_compute_shader"))<br>
<br>
Or, similarly, get overrides shoved into ctx->Extensions to allow<br>
checking for overrides at this early stage.<br>
<br>
-Jordan<br></blockquote><div><br></div><div>I looked into what would be necessary to do this, and it's unfortunately more complicated than it should be due to some flukes about initialization order (perhaps this is what you were alluding to when you said "get overrides shoved into ctx->Extensions to allow checking for overrides at this early stage"). Currently, our initialization order is:<br>
<br>1. brwCreateContext() calls brw_initialize_context_constants(), which needs to know whether compute shaders are supported in order to set constants like MAX_COMPUTE_TEXTURE_IMAGE_UNITS and MAX_UNIFORM_BUFFER_BINDINGS correctly.<br>
<br></div><div>2. Later, brwCreateContext() calls intelInitExtensions(), which is the function where we'll set ctx->Extensions.ARB_compute_shader to true once compute shaders are fully supported.<br><br></div><div>
3. Much later in initialization, when the context is being made current for the first time, core Mesa calls _mesa_make_extension_string(), which calls get_extension_override() to modify the the ctx->Extensions table based on the environment variable override.<br>
<br></div><div>To do what you suggested, I would either have to:<br><br>A. change 1 to call _mesa_is_extension_override_enabled(); that function, in turn, would have to parse the MESA_EXTENSION_OVERRIDE string, but we couldn't reuse the parsing code in _mesa_make_extension_string(), since that parsing code updates ctx->Extensions as a side effect, and it's not time to update ctx->Extensions yet. In addition to the code duplication, we would have a danger of bugs, since the override takes effect so late in initialization--if we ever accidentally introduced some code in between steps 2 and 3 that checked the value of ctx->Extensions.ARB_compute_shader, it would see false even if compute shaders were enabled by override.<br>
<br>Or:<br><br></div><div>B. change the order of initialization so that 2 happens first, followed by 3 and then 1. In the long run I think this would be a good thing, but it's a big change (since it affects all back ends) and I'm not sure it would be a good idea to hold up this patch series waiting for it.<br>
<br><br>How would you feel if I landed the patch series as is, and then worked on a follow up patch to do B?<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div class="h5"><br>
> + ctx->Const.Program[MESA_SHADER_COMPUTE].MaxTextureImageUnits = BRW_MAX_TEX_UNIT;<br>
> + ctx->Const.MaxUniformBufferBindings += 12;<br>
> + } else {<br>
> + ctx->Const.Program[MESA_SHADER_COMPUTE].MaxTextureImageUnits = 0;<br>
> + }<br>
> ctx->Const.MaxCombinedTextureImageUnits =<br>
> ctx->Const.Program[MESA_SHADER_VERTEX].MaxTextureImageUnits +<br>
> ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxTextureImageUnits +<br>
> - ctx->Const.Program[MESA_SHADER_GEOMETRY].MaxTextureImageUnits;<br>
> + ctx->Const.Program[MESA_SHADER_GEOMETRY].MaxTextureImageUnits +<br>
> + ctx->Const.Program[MESA_SHADER_COMPUTE].MaxTextureImageUnits;<br>
><br>
> ctx->Const.MaxTextureLevels = 14; /* 8192 */<br>
> if (ctx->Const.MaxTextureLevels > MAX_TEXTURE_LEVELS)<br>
> @@ -425,9 +432,11 @@ brw_initialize_context_constants(struct brw_context *brw)<br>
> ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxAtomicCounters = MAX_ATOMIC_COUNTERS;<br>
> ctx->Const.Program[MESA_SHADER_VERTEX].MaxAtomicCounters = MAX_ATOMIC_COUNTERS;<br>
> ctx->Const.Program[MESA_SHADER_GEOMETRY].MaxAtomicCounters = MAX_ATOMIC_COUNTERS;<br>
> + ctx->Const.Program[MESA_SHADER_COMPUTE].MaxAtomicCounters = MAX_ATOMIC_COUNTERS;<br>
> ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxAtomicBuffers = BRW_MAX_ABO;<br>
> ctx->Const.Program[MESA_SHADER_VERTEX].MaxAtomicBuffers = BRW_MAX_ABO;<br>
> ctx->Const.Program[MESA_SHADER_GEOMETRY].MaxAtomicBuffers = BRW_MAX_ABO;<br>
> + ctx->Const.Program[MESA_SHADER_COMPUTE].MaxAtomicBuffers = BRW_MAX_ABO;<br>
> ctx->Const.MaxCombinedAtomicBuffers = 3 * BRW_MAX_ABO;<br>
> }<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c<br>
> index de07b7f..27bc97b 100644<br>
> --- a/src/mesa/drivers/dri/i965/intel_extensions.c<br>
> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c<br>
> @@ -294,6 +294,8 @@ intelInitExtensions(struct gl_context *ctx)<br>
> ctx->Extensions.ARB_transform_feedback_instanced = true;<br>
> ctx->Extensions.ARB_draw_indirect = true;<br>
> }<br>
> + if (getenv("INTEL_COMPUTE_SHADER"))<br>
> + ctx->Extensions.ARB_compute_shader = true;<br>
> }<br>
><br>
> if (brw->gen == 5 || can_write_oacontrol(brw))<br>
> --<br>
> 1.8.5.2<br>
><br>
</div></div>> _______________________________________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div><br></div></div>