[Mesa-dev] [PATCH 30/30] i965/cs: Allow ARB_compute_shader to be enabled via env var.

Jordan Justen jljusten at gmail.com
Tue Feb 4 10:03:51 PST 2014 

On Tue, Feb 4, 2014 at 8:43 AM, Paul Berry <stereotype441 at gmail.com> wrote:
> On 1 February 2014 23:21, Jordan Justen <jljusten at gmail.com> wrote:
>>
>> On Thu, Jan 9, 2014 at 6:19 PM, Paul Berry <stereotype441 at gmail.com>
>> wrote:
>> > This will allow testing of compute shader functionality before it is
>> > completed.
>> >
>> > To enable ARB_compute_shader functionality in the i965 driver, set
>> > INTEL_COMPUTE_SHADER=1.
>> > ---
>> >  src/mesa/drivers/dri/i965/brw_context.c      | 11 ++++++++++-
>> >  src/mesa/drivers/dri/i965/intel_extensions.c |  2 ++
>> >  2 files changed, 12 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/src/mesa/drivers/dri/i965/brw_context.c
>> > b/src/mesa/drivers/dri/i965/brw_context.c
>> > index 1b42751..76dd9be 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_context.c
>> > +++ b/src/mesa/drivers/dri/i965/brw_context.c
>> > @@ -298,10 +298,17 @@ brw_initialize_context_constants(struct
>> > brw_context *brw)
>> >        ctx->Const.Program[MESA_SHADER_GEOMETRY].MaxTextureImageUnits =
>> > BRW_MAX_TEX_UNIT;
>> >     else
>> >        ctx->Const.Program[MESA_SHADER_GEOMETRY].MaxTextureImageUnits =
>> > 0;
>> > +   if (getenv("INTEL_COMPUTE_SHADER")) {
>>
>> What about trying to make use of
>> MESA_EXTENSION_OVERRIDE=GL_ARB_compute_shader?
>>
>> We could add
>> extensions.c:bool _mesa_is_extension_override_enabled(char *)
>>
>> And then
>>     if (_mesa_is_extension_override_enabled("GL_ARB_compute_shader"))
>>
>> Or, similarly, get overrides shoved into ctx->Extensions to allow
>> checking for overrides at this early stage.
>>
>> -Jordan
>
>
> 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").

Yeah, I'll admit I looked at it a bit, and decided to keep my feedback
'hand wavy'. :)

>  Currently, our initialization order is:
>
> 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.
>
> 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.
>
> 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.
>
> To do what you suggested, I would either have to:
>
> 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.
>
> Or:
>
> 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.
>
>
> How would you feel if I landed the patch series as is, and then worked on a
> follow up patch to do B?

Sure, you can have my r-b for this then.

-Jordan

>> > +      ctx->Const.Program[MESA_SHADER_COMPUTE].MaxTextureImageUnits =
>> > BRW_MAX_TEX_UNIT;
>> > +      ctx->Const.MaxUniformBufferBindings += 12;
>> > +   } else {
>> > +      ctx->Const.Program[MESA_SHADER_COMPUTE].MaxTextureImageUnits = 0;
>> > +   }
>> >     ctx->Const.MaxCombinedTextureImageUnits =
>> >        ctx->Const.Program[MESA_SHADER_VERTEX].MaxTextureImageUnits +
>> >        ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxTextureImageUnits +
>> > -      ctx->Const.Program[MESA_SHADER_GEOMETRY].MaxTextureImageUnits;
>> > +      ctx->Const.Program[MESA_SHADER_GEOMETRY].MaxTextureImageUnits +
>> > +      ctx->Const.Program[MESA_SHADER_COMPUTE].MaxTextureImageUnits;
>> >
>> >     ctx->Const.MaxTextureLevels = 14; /* 8192 */
>> >     if (ctx->Const.MaxTextureLevels > MAX_TEXTURE_LEVELS)
>> > @@ -425,9 +432,11 @@ brw_initialize_context_constants(struct brw_context
>> > *brw)
>> >        ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxAtomicCounters =
>> > MAX_ATOMIC_COUNTERS;
>> >        ctx->Const.Program[MESA_SHADER_VERTEX].MaxAtomicCounters =
>> > MAX_ATOMIC_COUNTERS;
>> >        ctx->Const.Program[MESA_SHADER_GEOMETRY].MaxAtomicCounters =
>> > MAX_ATOMIC_COUNTERS;
>> > +      ctx->Const.Program[MESA_SHADER_COMPUTE].MaxAtomicCounters =
>> > MAX_ATOMIC_COUNTERS;
>> >        ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxAtomicBuffers =
>> > BRW_MAX_ABO;
>> >        ctx->Const.Program[MESA_SHADER_VERTEX].MaxAtomicBuffers =
>> > BRW_MAX_ABO;
>> >        ctx->Const.Program[MESA_SHADER_GEOMETRY].MaxAtomicBuffers =
>> > BRW_MAX_ABO;
>> > +      ctx->Const.Program[MESA_SHADER_COMPUTE].MaxAtomicBuffers =
>> > BRW_MAX_ABO;
>> >        ctx->Const.MaxCombinedAtomicBuffers = 3 * BRW_MAX_ABO;
>> >     }
>> >
>> > diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c
>> > b/src/mesa/drivers/dri/i965/intel_extensions.c
>> > index de07b7f..27bc97b 100644
>> > --- a/src/mesa/drivers/dri/i965/intel_extensions.c
>> > +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
>> > @@ -294,6 +294,8 @@ intelInitExtensions(struct gl_context *ctx)
>> >           ctx->Extensions.ARB_transform_feedback_instanced = true;
>> >           ctx->Extensions.ARB_draw_indirect = true;
>> >        }
>> > +      if (getenv("INTEL_COMPUTE_SHADER"))
>> > +         ctx->Extensions.ARB_compute_shader = true;
>> >     }
>> >
>> >     if (brw->gen == 5 || can_write_oacontrol(brw))
>> > --
>> > 1.8.5.2
>> >
>> > _______________________________________________
>> > mesa-dev mailing list
>> > mesa-dev at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>

More information about the mesa-dev mailing list