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

Paul Berry stereotype441 at gmail.com
Tue Feb 4 08:43:52 PST 2014


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").  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?


>
> > +      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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140204/79f9d756/attachment.html>


More information about the mesa-dev mailing list