[Mesa-dev] [PATCH] Revert "i915: Always enable GL 2.0 support."

[Matt Turner]

On Sun, Jan 29, 2017 at 8:29 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Sunday, January 29, 2017 6:20:10 PM PST Matt Turner wrote:
>> This partially reverts commit 97217a40f97cdeae0304798b607f704deb0c3558.
>> It leaves ES 2.0 support in place per Ian's suggestion, because ES 2.0
>> is designed to work on hardware like i915.
>
> Your commit message should mention why dropping from OpenGL 2.1 to 1.4
> is a good thing.
>
> (IIRC it's because Chrome (and other apps?) use really slow paths with
> 2.1, and so the general usability of the system is likely to be worse.)

Yeah, I'll add

Chrome only uses the GPU if you have GL >= 2.0, and using i915 (and
prog_execute) actually hurt performance compared with the software
paths.

>> The piglit results look like:
>>
>>        name: before-revert-i915 after-revert-i915
>>        ----  ------------------ -----------------
>>        pass:               7171              2169
>>        fail:                933               201
>>       crash:                  8                 7
>>        skip:              32997             38676
>>     timeout:                  0                 0
>>        warn:                  3                 1
>>  incomplete:                  0                 0
>>  dmesg-warn:                  0                 0
>>  dmesg-fail:                  0                 0
>>     changes:                  0              6040
>>       fixes:                  0               292
>> regressions:                  0                 2
>>       total:              41112             41054
>
> Why are these interesting?

I thought it was interesting to see that GL 2.1 support actually
regressed 292 tests of GL 1.0 and 1.1 functionality (most of them in
spec/!opengl 1.1/clipflat/). I can remove it from the commit message.

>
>> Cc: "17.0" <mesa-stable at lists.freedesktop.org>
>> ---
>>  src/mesa/drivers/dri/i915/intel_extensions.c |  8 ++++++--
>>  src/mesa/drivers/dri/i915/intel_screen.c     | 21 +++++++++++++++++++--
>>  2 files changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i915/intel_extensions.c b/src/mesa/drivers/dri/i915/intel_extensions.c
>> index ab7820f..4f2c6fa 100644
>> --- a/src/mesa/drivers/dri/i915/intel_extensions.c
>> +++ b/src/mesa/drivers/dri/i915/intel_extensions.c
>> @@ -92,8 +92,12 @@ intelInitExtensions(struct gl_context *ctx)
>>        ctx->Extensions.ATI_separate_stencil = true;
>>        ctx->Extensions.ATI_texture_env_combine3 = true;
>>        ctx->Extensions.NV_texture_env_combine4 = true;
>> -      ctx->Extensions.ARB_fragment_shader = true;
>> -      ctx->Extensions.ARB_occlusion_query = true;
>> +
>> +      if (driQueryOptionb(&intel->optionCache, "fragment_shader"))
>> +         ctx->Extensions.ARB_fragment_shader = true;
>
> I get dropping GLSL support, but isn't GL_ARB_fragment_shader sort of
> reasonable for this hardware?

I don't know. I'm just reverting it to how it was before commit 97217a40f97cd.

Ian, do you have an opinion?