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

2016-10-26 Thread Francisco Jerez 
Iago Toral  writes:

> On Thu, 2016-10-20 at 16:25 -0700, Ian Romanick wrote:
>> On 10/20/2016 12:09 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.
>> > 
>> > v2 (Ian, Ken):
>> >   - We should not set max_gl_core_version to 40 if we don't support
>> > pipelined register writes. If we did that, then we would allow
>> > the creation of GL4 contexts but the actual context we create
>> > might only be 3.3. Check that we support this by looking at the
>> > Kernel's supported command parser version. This requires
>> > that we get the supported command parser version before we call
>> > set_max_gl_versions() too.

I don't think that doing this by checking the command parser version
alone is sufficient, because hardware security may still be enabled on
HSW depending on the kernel version, which would prevent some GL4.0
extensions from working even if the relevant registers are already
whitelisted in the software command checker.  You probably need to check
the result of can_do_pipelined_register_writes(), which may take some
effort to get to work before the GL context is created.  There is some
related discussion going on here:

https://lists.freedesktop.org/archives/mesa-dev/2016-October/133431.html

>> > ---
>> >  src/mesa/drivers/dri/i965/intel_extensions.c |  2 ++
>> >  src/mesa/drivers/dri/i965/intel_screen.c | 13 +++--
>> >  2 files changed, 9 insertions(+), 6 deletions(-)
>> > 
>> > diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c
>> > b/src/mesa/drivers/dri/i965/intel_extensions.c
>> > index 9cc22cc..8370dc4 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 = 450;
>> > +   else if (brw->is_haswell && brw->screen->cmd_parser_version >=
>> > 2)
>> > +  ctx->Const.GLSLVersion = 400;
>> The GLSL version is (eventually) clamped by the GL version, so this
>> doesn't matter as much.  I'm fine with it either way.
>
> In that case I'll remove the check for simplicity.
>
>> Reviewed-by: Ian Romanick 
>
> Thanks!
>
>> > 
>> > 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 e1c3c19..20d75a2 100644
>> > --- a/src/mesa/drivers/dri/i965/intel_screen.c
>> > +++ b/src/mesa/drivers/dri/i965/intel_screen.c
>> > @@ -1445,7 +1445,8 @@ 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 && screen->cmd_parser_version
>> > >= 2 ? 40 : 33;
>> >    dri_screen->max_gl_compat_version = 30;
>> >    dri_screen->max_gl_es1_version = 11;
>> >    dri_screen->max_gl_es2_version = screen->devinfo.is_haswell
>> > ? 31 : 30;
>> > @@ -1650,6 +1651,11 @@ __DRIconfig **intelInitScreen2(__DRIscreen
>> > *dri_screen)
>> >    screen->winsys_msaa_samples_override = -1;
>> > }
>> >  
>> > +   if (intel_get_param(screen, I915_PARAM_CMD_PARSER_VERSION,
>> > +   &screen->cmd_parser_version) < 0) {
>> > +  screen->cmd_parser_version = 0;
>> > +   }
>> > +
>> > set_max_gl_versions(screen);
>> >  
>> > /* Notification of GPU resets requires hardware contexts and a
>> > kernel new
>> > @@ -1671,11 +1677,6 @@ __DRIconfig **intelInitScreen2(__DRIscreen
>> > *dri_screen)
>> >   (ret != -1 || errno != EINVAL);
>> > }
>> >  
>> > -   if (intel_get_param(screen, I915_PARAM_CMD_PARSER_VERSION,
>> > -   &screen->cmd_parser_version) < 0) {
>> > -  screen->cmd_parser_version = 0;
>> > -   }
>> > -
>> > /* Haswell requires command parser version 6 in order to write
>> > to the
>> >  * MI_MATH GPR registers, and version 7 in order to use
>> >  * MI_LOAD_REGISTER_REG (which all users of MI_MATH use).
>> > 
>> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-10-20 Thread Iago Toral 
On Thu, 2016-10-20 at 16:25 -0700, Ian Romanick wrote:
> On 10/20/2016 12:09 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.
> > 
> > v2 (Ian, Ken):
> >   - We should not set max_gl_core_version to 40 if we don't support
> > pipelined register writes. If we did that, then we would allow
> > the creation of GL4 contexts but the actual context we create
> > might only be 3.3. Check that we support this by looking at the
> > Kernel's supported command parser version. This requires
> > that we get the supported command parser version before we call
> > set_max_gl_versions() too.
> > ---
> >  src/mesa/drivers/dri/i965/intel_extensions.c |  2 ++
> >  src/mesa/drivers/dri/i965/intel_screen.c | 13 +++--
> >  2 files changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c
> > b/src/mesa/drivers/dri/i965/intel_extensions.c
> > index 9cc22cc..8370dc4 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 = 450;
> > +   else if (brw->is_haswell && brw->screen->cmd_parser_version >=
> > 2)
> > +  ctx->Const.GLSLVersion = 400;
> The GLSL version is (eventually) clamped by the GL version, so this
> doesn't matter as much.  I'm fine with it either way.

In that case I'll remove the check for simplicity.

> Reviewed-by: Ian Romanick 

Thanks!

> > 
> > 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 e1c3c19..20d75a2 100644
> > --- a/src/mesa/drivers/dri/i965/intel_screen.c
> > +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> > @@ -1445,7 +1445,8 @@ 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 && screen->cmd_parser_version
> > >= 2 ? 40 : 33;
> >    dri_screen->max_gl_compat_version = 30;
> >    dri_screen->max_gl_es1_version = 11;
> >    dri_screen->max_gl_es2_version = screen->devinfo.is_haswell
> > ? 31 : 30;
> > @@ -1650,6 +1651,11 @@ __DRIconfig **intelInitScreen2(__DRIscreen
> > *dri_screen)
> >    screen->winsys_msaa_samples_override = -1;
> > }
> >  
> > +   if (intel_get_param(screen, I915_PARAM_CMD_PARSER_VERSION,
> > +   &screen->cmd_parser_version) < 0) {
> > +  screen->cmd_parser_version = 0;
> > +   }
> > +
> > set_max_gl_versions(screen);
> >  
> > /* Notification of GPU resets requires hardware contexts and a
> > kernel new
> > @@ -1671,11 +1677,6 @@ __DRIconfig **intelInitScreen2(__DRIscreen
> > *dri_screen)
> >   (ret != -1 || errno != EINVAL);
> > }
> >  
> > -   if (intel_get_param(screen, I915_PARAM_CMD_PARSER_VERSION,
> > -   &screen->cmd_parser_version) < 0) {
> > -  screen->cmd_parser_version = 0;
> > -   }
> > -
> > /* Haswell requires command parser version 6 in order to write
> > to the
> >  * MI_MATH GPR registers, and version 7 in order to use
> >  * MI_LOAD_REGISTER_REG (which all users of MI_MATH use).
> > 
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-10-20 Thread Ian Romanick 
On 10/20/2016 12:09 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.
> 
> v2 (Ian, Ken):
>   - We should not set max_gl_core_version to 40 if we don't support
> pipelined register writes. If we did that, then we would allow
> the creation of GL4 contexts but the actual context we create
> might only be 3.3. Check that we support this by looking at the
> Kernel's supported command parser version. This requires
> that we get the supported command parser version before we call
> set_max_gl_versions() too.
> ---
>  src/mesa/drivers/dri/i965/intel_extensions.c |  2 ++
>  src/mesa/drivers/dri/i965/intel_screen.c | 13 +++--
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
> b/src/mesa/drivers/dri/i965/intel_extensions.c
> index 9cc22cc..8370dc4 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 = 450;
> +   else if (brw->is_haswell && brw->screen->cmd_parser_version >= 2)
> +  ctx->Const.GLSLVersion = 400;

The GLSL version is (eventually) clamped by the GL version, so this
doesn't matter as much.  I'm fine with it either way.

Reviewed-by: Ian Romanick 

> 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 e1c3c19..20d75a2 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1445,7 +1445,8 @@ 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 && screen->cmd_parser_version >= 2 ? 40 
> : 33;
>dri_screen->max_gl_compat_version = 30;
>dri_screen->max_gl_es1_version = 11;
>dri_screen->max_gl_es2_version = screen->devinfo.is_haswell ? 31 : 30;
> @@ -1650,6 +1651,11 @@ __DRIconfig **intelInitScreen2(__DRIscreen *dri_screen)
>screen->winsys_msaa_samples_override = -1;
> }
>  
> +   if (intel_get_param(screen, I915_PARAM_CMD_PARSER_VERSION,
> +   &screen->cmd_parser_version) < 0) {
> +  screen->cmd_parser_version = 0;
> +   }
> +
> set_max_gl_versions(screen);
>  
> /* Notification of GPU resets requires hardware contexts and a kernel new
> @@ -1671,11 +1677,6 @@ __DRIconfig **intelInitScreen2(__DRIscreen *dri_screen)
>   (ret != -1 || errno != EINVAL);
> }
>  
> -   if (intel_get_param(screen, I915_PARAM_CMD_PARSER_VERSION,
> -   &screen->cmd_parser_version) < 0) {
> -  screen->cmd_parser_version = 0;
> -   }
> -
> /* Haswell requires command parser version 6 in order to write to the
>  * MI_MATH GPR registers, and version 7 in order to use
>  * MI_LOAD_REGISTER_REG (which all users of MI_MATH use).
> 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-10-20 Thread Iago Toral Quiroga 
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.

v2 (Ian, Ken):
  - We should not set max_gl_core_version to 40 if we don't support
pipelined register writes. If we did that, then we would allow
the creation of GL4 contexts but the actual context we create
might only be 3.3. Check that we support this by looking at the
Kernel's supported command parser version. This requires
that we get the supported command parser version before we call
set_max_gl_versions() too.
---
 src/mesa/drivers/dri/i965/intel_extensions.c |  2 ++
 src/mesa/drivers/dri/i965/intel_screen.c | 13 +++--
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
b/src/mesa/drivers/dri/i965/intel_extensions.c
index 9cc22cc..8370dc4 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 = 450;
+   else if (brw->is_haswell && brw->screen->cmd_parser_version >= 2)
+  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 e1c3c19..20d75a2 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -1445,7 +1445,8 @@ 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 && screen->cmd_parser_version >= 2 ? 40 : 
33;
   dri_screen->max_gl_compat_version = 30;
   dri_screen->max_gl_es1_version = 11;
   dri_screen->max_gl_es2_version = screen->devinfo.is_haswell ? 31 : 30;
@@ -1650,6 +1651,11 @@ __DRIconfig **intelInitScreen2(__DRIscreen *dri_screen)
   screen->winsys_msaa_samples_override = -1;
}
 
+   if (intel_get_param(screen, I915_PARAM_CMD_PARSER_VERSION,
+   &screen->cmd_parser_version) < 0) {
+  screen->cmd_parser_version = 0;
+   }
+
set_max_gl_versions(screen);
 
/* Notification of GPU resets requires hardware contexts and a kernel new
@@ -1671,11 +1677,6 @@ __DRIconfig **intelInitScreen2(__DRIscreen *dri_screen)
  (ret != -1 || errno != EINVAL);
}
 
-   if (intel_get_param(screen, I915_PARAM_CMD_PARSER_VERSION,
-   &screen->cmd_parser_version) < 0) {
-  screen->cmd_parser_version = 0;
-   }
-
/* Haswell requires command parser version 6 in order to write to the
 * MI_MATH GPR registers, and version 7 in order to use
 * MI_LOAD_REGISTER_REG (which all users of MI_MATH use).
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev