[Mesa-dev] [PATCH 02/13] i965: remove ARB_compute_shader extension override

[Emil Velikov]

On 10 November 2017 at 23:39, Jordan Justen <jordan.l.justen at intel.com> wrote:
> On 2017-11-10 08:19:38, Emil Velikov wrote:
>> On 7 November 2017 at 11:54, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> > From: Emil Velikov <emil.velikov at collabora.com>
>> >
>> > Checking the override was useful in the early stages of developing the
>> > extension.
>> >
>> > Now that everything is wired, where possible, we can drop the check.
>> > Doing so allows us to simplify some of the related code.
>> >
>> > Cc: Jordan Justen <jordan.l.justen at intel.com>
>> > Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> > ---
>> >  src/mesa/drivers/dri/i965/brw_context.c | 3 +--
>> >  1 file changed, 1 insertion(+), 2 deletions(-)
>> >
>> > diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
>> > index 1bf39b07382..3947a71a46a 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_context.c
>> > +++ b/src/mesa/drivers/dri/i965/brw_context.c
>> > @@ -348,8 +348,7 @@ brw_initialize_context_constants(struct brw_context *brw)
>> >           (_mesa_is_desktop_gl(ctx) &&
>> >            ctx->Const.MaxComputeWorkGroupSize[0] >= 1024) ||
>> >           (ctx->API == API_OPENGLES2 &&
>> > -          ctx->Const.MaxComputeWorkGroupSize[0] >= 128) ||
>> > -         _mesa_extension_override_enables.ARB_compute_shader,
>> > +          ctx->Const.MaxComputeWorkGroupSize[0] >= 128),
>>
>> Jordan can you throw an Ack on this patch if it makes sense?
>> Brian already reviewed the series, but I'd appreciate input from
>> someone familiar with the Intel specifics.
>
> Regarding patches 2 & 8, the point of those being non-static was so
> the driver could take some action if the user requested an extension
> override.
>
> I can't remember, but maybe with SIMD32 being supported, this might no
> longer affect any i965 devices.
>
> I still think it could be good to allow the driver to easily know what
> extensions were overridden, but I'll concede that it is not too
> important. So, go ahead with the change if you want.
>
Agreed allowing the driver to query the extension status is useful,
esp. during development.

Since they're not used I made them static for now, but can add a
comment as below to make it clearer.
What do you think? Can I get your official blessing (ack/r-b) with that?

Thanks
Emil


/* Sometimes the driver wants to query the extensions status before a context
 * is created. To achieve that one should use the following two variables. This
 * is very useful during extension bring-up, but can be a double-edged sword.
 *
 * Use it with care and keep access read-only.
 */