On 15 November 2017 at 19:30, Jordan Justen <jordan.l.jus...@intel.com> wrote: > On 2017-11-14 07:37:27, Emil Velikov wrote: >> On 10 November 2017 at 23:39, Jordan Justen <jordan.l.jus...@intel.com> >> wrote: >> > On 2017-11-10 08:19:38, Emil Velikov wrote: >> >> On 7 November 2017 at 11:54, Emil Velikov <emil.l.veli...@gmail.com> >> >> wrote: >> >> > From: Emil Velikov <emil.veli...@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.jus...@intel.com> >> >> > Signed-off-by: Emil Velikov <emil.veli...@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? > > For this patch, Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com> > > I'm not sure what you are proposing for patch 8. I would prefer to > just drop 8, but I'm not sure how big of an impact that makes to your > series. > > If you are proposing to drop patch 8, and instead add the comment > below on the variables... What about this small tweak? > I was split between dropping or keeping with the comment squashed. Let's drop 08 - the dead trivial conflict in 12/13 is fine.
> /* Sometimes the driver wants to query the extension override status before > * a context is created. These variables are filled with extension override > * information before context creation. > * > * This can be useful during extension bring-up when an extension is > * partially implemented, but cannot yet be advertised as supported. > * > * Use it with care and keep access read-only. > */ > Nice - it reads a lot easier. I'll whip it into a patch form and send out (for posterity) tomorrow. Thanks Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
