[Mesa-dev] [PATCH v2 103/103] i965/gen7: expose OpenGL 4.0 on Haswell

[Iago Toral]

On Wed, 2016-10-19 at 10:32 -0700, Kenneth Graunke wrote:
> On Wednesday, October 19, 2016 10:52:32 AM PDT Iago Toral wrote:
> > 
> > On Tue, 2016-10-18 at 17:57 -0700, Kenneth Graunke wrote:
> > > 
> > > On Tuesday, October 18, 2016 5:12:27 PM PDT Ian Romanick wrote:
> > > > 
> > > > 
> > > > On 10/11/2016 02:02 AM, Iago Toral Quiroga wrote:
> > > > > 
> > > > > 
> > > > > ARB_gpu_shader_fp64 was the last piece missing. Notice that
> > > > > some
> > > > > hardware and kernel combinations do not support pipelined
> > > > > register
> > > > > writes, which are required for some OpenGL 4.0 features, in
> > > > > which
> > > > > case the driver won't expose 4.0.
> > > > > ---
> > > > >  src/mesa/drivers/dri/i965/intel_extensions.c | 2 ++
> > > > >  src/mesa/drivers/dri/i965/intel_screen.c     | 2 +-
> > > > >  2 files changed, 3 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c
> > > > > b/src/mesa/drivers/dri/i965/intel_extensions.c
> > > > > index 0491145..a291cd5 100644
> > > > > --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> > > > > +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> > > > > @@ -272,6 +272,8 @@ intelInitExtensions(struct gl_context
> > > > > *ctx)
> > > > >  
> > > > >     if (brw->gen >= 8)
> > > > >        ctx->Const.GLSLVersion = 440;
> > > > > +   else if (brw->is_haswell)
> > > > > +      ctx->Const.GLSLVersion = 400;
> > > > >     else if (brw->gen >= 6)
> > > > >        ctx->Const.GLSLVersion = 330;
> > > > >     else
> > > > > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> > > > > b/src/mesa/drivers/dri/i965/intel_screen.c
> > > > > index 9b23bac..1af7fe6 100644
> > > > > --- a/src/mesa/drivers/dri/i965/intel_screen.c
> > > > > +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> > > > > @@ -1445,7 +1445,7 @@ set_max_gl_versions(struct intel_screen
> > > > > *screen)
> > > > >        dri_screen->max_gl_es2_version = has_astc ? 32 : 31;
> > > > >        break;
> > > > >     case 7:
> > > > > -      dri_screen->max_gl_core_version = 33;
> > > > > +      dri_screen->max_gl_core_version = screen-
> > > > > > 
> > > > > > devinfo.is_haswell ? 40 : 33;
> > > > I *think* this needs to take the pipelined register writes into
> > > > consideration.  My understanding is if you say 40 here, then
> > > > glXCreateContextAttribs will allow creation of an OpenGL 4.0
> > > > context...
> > > > but the context may only be 3.3.
> > > Good catch, Ian.  Checking brw->can_do_pipelined_register_writes
> > > here
> > > would be right...but it's awkward, since it's stored in the
> > > context,
> > > and
> > > doesn't get populated until we actually make a context and run
> > > things
> > > on
> > > the GPU.  That's probably not too feasible here in screen init
> > > time,
> > > where we're trying to decide what kind of contexts to even
> > > support.
> > > 
> > > To make life easier, I might just do:
> > > 
> > >    dri_screen->max_gl_core_version = screen->has_mi_math_and_lrr
> > > ? 40
> > > : 33;
> > > 
> > > which is the check we use for ARB_query_buffer_object.  On
> > > Haswell,
> > > it implies a high enough command parser version that we can do
> > > everything we need.  (We could actually get away with an older
> > > kernel
> > > version, but I'm not sure I care...as we move toward 4.1/4.2/4.3
> > > we'd
> > > need to bump it higher anyway...)
> > Ok, I'll do that. Thank you both for the feedback!
> > 
> > Of course, this begs the question: what about IvyBridge?
> > has_mi_math_and_lrr is only ever set to true for HSW+, does this
> > mean
> > that IVB hardware cannot expose GL4 contexts or that we need to
> > find a
> > different way to query the same thing for IVB?
> In that case, you probably want to check screen->cmd_parser_version
> directly.  I think version 2 should be sufficient for GL 4.0, and
> you want version 5 for compute shaders.
> 
> From the kernel's i915_cmd_parser.c:
> 
>    /*
>     * Command parser version history
>     *
>     * 1. Initial version. Checks batches and reports violations, but
> leaves
>     *    hardware parsing enabled (so does not allow new use cases).
>     * 2. Allow access to the MI_PREDICATE_SRC0 and
>     *    MI_PREDICATE_SRC1 registers.
>     * 3. Allow access to the GPGPU_THREADS_DISPATCHED register.
>     * 4. L3 atomic chicken bits of HSW_SCRATCH1 and HSW_ROW_CHICKEN3.
>     * 5. GPGPU dispatch compute indirect registers.
>     * 6. TIMESTAMP register and Haswell CS GPR registers
>     * 7. Allow MI_LOAD_REGISTER_REG between whitelisted registers.
>     */
> 
> Checking that would work for Haswell too.

I think in that case I'll go with checking the parser version directly
in both HSW and IVB. Thanks!

Iago

> --Ken