[Mesa-dev] [PATCH v2 6/7] i965: Enable arb_transform_feedback_overflow_query.

[Rafael Antognolli]

On Mon, Dec 12, 2016 at 10:00:42AM -0800, Jordan Justen wrote:
> On 2016-12-12 07:06:50, Rafael Antognolli wrote:
> > On Fri, Dec 09, 2016 at 10:35:58PM -0800, Jordan Justen wrote:
> > > On 2016-12-09 13:39:52, Rafael Antognolli wrote:
> > > > This extension adds new query types which can be used to detect overflow
> > > > of transform feedback buffers. The new query types are also accepted by
> > > > conditional rendering commands.
> > > > 
> > > > Signed-off-by: Rafael Antognolli <rafael.antognolli at intel.com>
> > > > ---
> > > >  docs/features.txt                            | 2 +-
> > > >  docs/relnotes/13.1.0.html                    | 1 +
> > > >  src/mesa/drivers/dri/i965/intel_extensions.c | 1 +
> > > >  3 files changed, 3 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/docs/features.txt b/docs/features.txt
> > > > index c27d521..bb7925e 100644
> > > > --- a/docs/features.txt
> > > > +++ b/docs/features.txt
> > > > @@ -303,7 +303,7 @@ Khronos, ARB, and OES extensions that are not part of any OpenGL or OpenGL ES ve
> > > >    GL_ARB_sparse_texture2                                not started
> > > >    GL_ARB_sparse_texture_clamp                           not started
> > > >    GL_ARB_texture_filter_minmax                          not started
> > > > -  GL_ARB_transform_feedback_overflow_query              not started
> > > > +  GL_ARB_transform_feedback_overflow_query              DONE (i965/gen7+)
> > > >    GL_KHR_blend_equation_advanced_coherent               DONE (i965/gen9+)
> > > >    GL_KHR_no_error                                       not started
> > > >    GL_KHR_texture_compression_astc_hdr                   DONE (core only)
> > > > diff --git a/docs/relnotes/13.1.0.html b/docs/relnotes/13.1.0.html
> > > > index 5b8b016..4f52cd1 100644
> > > > --- a/docs/relnotes/13.1.0.html
> > > > +++ b/docs/relnotes/13.1.0.html
> > > > @@ -45,6 +45,7 @@ Note: some of the new features are only available with certain drivers.
> > > >  
> > > >  <ul>
> > > >  <li>GL_ARB_post_depth_coverage on i965/gen9+</li>
> > > > +<li>GL_ARB_transform_feedback_overflow_query on i965/gen7+</li>
> > > >  <li>GL_NV_image_formats on any driver supporting GL_ARB_shader_image_load_store (i965, nvc0, radeonsi, softpipe)</li>
> > > >  </ul>
> > > >  
> > > > diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c
> > > > index c1f42aa..d5e4164 100644
> > > > --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> > > > +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> > > > @@ -320,6 +320,7 @@ intelInitExtensions(struct gl_context *ctx)
> > > >        ctx->Extensions.EXT_framebuffer_multisample = true;
> > > >        ctx->Extensions.EXT_framebuffer_multisample_blit_scaled = true;
> > > >        ctx->Extensions.EXT_transform_feedback = true;
> > > > +      ctx->Extensions.ARB_transform_feedback_overflow_query = true;
> > > 
> > > Is this enabling the extension on gen6?
> > 
> > Yes.
> > 
> > > Should it depend on brw->predicate.supported (and thus the next
> > > patch)?
> > 
> > It is possible to query for overflow on gen6, so in theory there's no
> > reason to not enable it.
> 
> The code (gen6+) doesn't match with the doc changes (gen7+).
> 
> I think the patch subject should also indicate the gen. For example:
> 
>   i965/gen6+: Enable arb_transform_feedback_overflow_query

Good point, I'll update the patch to make it match gen6+.

> > However, predication only becomes available on gen7, and having
> > transform feedback overflow query without conditional render support
> > might not be that useful.
> > 
> > PS: The next patch enables predication on gen >= 8, but it was
> > already enabled on haswell (gen7).
> 
> I guess I was concerned that after patch 6, the driver might be in a
> state where gen8 was advertising something that wouldn't work until
> after patch 7. So, this isn't the case?

This patch advertises the query, which is available, regardless of
predication support.

> I expected to see the last patch in a series enables the extension,
> which is why I asked about it.

It definitely makes sense. I'll move the patch that enables predication
for gen8 to be the first one in the series.

Thanks,
Rafael