Disable framebuffer discarding on Mali-4xx and clean up.

Disable framebuffer discarding on Mali-4xx and clean up.

It seems all of the known bugs with Mali-400 are related to framebuffer
discarding.  Blacklist framebuffer discarding universally on Mali-400
and reenable ganesh and canvas where it was still blacklisted on Mali-400.

There was a preexisting workaround applied to all ARM < 4.4 which was
originally written in the context of Mali-T604; this explains why the
bugs only reproed on some Mali-400 devices.  Add a new one for all 4xx.

Also, a recently added Nexus 6 workaround did essentially the same
thing, but at the level of making the discard do nothing instead of not
advertising the extension.  In OpenGL ES 3, this is a core method so
failing to advertise won't be sufficient, so merge the two workarounds
and remove the "_ext_" from the name.

BUG=449488, 451230
Committed: https://crrev.com/8ef99b8a51842a038c64cd2b1ab1896228651ec2
Cr-Commit-Position: refs/heads/master@{#324152}

[aelias_OOO_until_Jul13, 2015-04-07 00:36:04]

aelias@chromium.org changed reviewers:
+ dyen@chromium.org, sievers@chromium.org

[aelias_OOO_until_Jul13, 2015-04-07 00:36:50]

Hi Daniel and David, this is an alternate variant for
https://codereview.chromium.org/1062043002/ that goes a bit further in cleaning
up the workaround, after we looked a bit further into it.

[David Yen, 2015-04-07 16:25:59]

LGTM with the 2 fixes, I'll close the other CL.

https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
File gpu/config/gpu_driver_bug_list_json.cc (right):

https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
gpu/config/gpu_driver_bug_list_json.cc:1204: },
I believe you cannot have trailing comma's in the parser used here.

https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
File gpu/config/gpu_driver_bug_workaround_type.h (right):

https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
gpu/config/gpu_driver_bug_workaround_type.h:35:
GPU_OP(disable_discard_framebuffer,                        \
This should be capitalized

[David Yen, 2015-04-07 16:35:16]

On 2015/04/07 16:25:59, David Yen wrote:
> LGTM with the 2 fixes, I'll close the other CL.
> 
>
https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
> File gpu/config/gpu_driver_bug_list_json.cc (right):
> 
>
https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
> gpu/config/gpu_driver_bug_list_json.cc:1204: },
> I believe you cannot have trailing comma's in the parser used here.
> 
>
https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
> File gpu/config/gpu_driver_bug_workaround_type.h (right):
> 
>
https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
> gpu/config/gpu_driver_bug_workaround_type.h:35:
> GPU_OP(disable_discard_framebuffer,                        \
> This should be capitalized

FYI another workaround is landing: https://codereview.chromium.org/1064773002/

So this needs to be rebased and changed to version "7.21".

[David Yen, 2015-04-07 17:13:21]

On 2015/04/07 16:35:16, David Yen wrote:
> On 2015/04/07 16:25:59, David Yen wrote:
> > LGTM with the 2 fixes, I'll close the other CL.
> > 
> >
>
https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
> > File gpu/config/gpu_driver_bug_list_json.cc (right):
> > 
> >
>
https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
> > gpu/config/gpu_driver_bug_list_json.cc:1204: },
> > I believe you cannot have trailing comma's in the parser used here.
> > 
> >
>
https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
> > File gpu/config/gpu_driver_bug_workaround_type.h (right):
> > 
> >
>
https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
> > gpu/config/gpu_driver_bug_workaround_type.h:35:
> > GPU_OP(disable_discard_framebuffer,                        \
> > This should be capitalized
> 
> FYI another workaround is landing: https://codereview.chromium.org/1064773002/
> 
> So this needs to be rebased and changed to version "7.21".

I just tested this on an android one which has a Mali400 running 4.4.4. It looks
like this problem is fixed in the driver version it has. Should we gate this to
"< android 4.4.4"? The only problem is we would have to rely on vendors updating
drivers.

[aelias_OOO_until_Jul13, 2015-04-07 19:43:31]

Daniel, ping for OWNERS review?

On 2015/04/07 at 17:13:21, dyen wrote:
> I just tested this on an android one which has a Mali400 running 4.4.4. It
looks like this problem is fixed in the driver version it has. Should we gate
this to "< android 4.4.4"? The only problem is we would have to rely on vendors
updating drivers.

No, this sort of gating is not reliable and has failed us before.  It's common
for vendors to update devices to new Android releases without changing the old
driver at all.

[aelias_OOO_until_Jul13, 2015-04-07 19:47:11]

https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
File gpu/config/gpu_driver_bug_list_json.cc (right):

https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
gpu/config/gpu_driver_bug_list_json.cc:1204: },
On 2015/04/07 16:25:59, David Yen wrote:
> I believe you cannot have trailing comma's in the parser used here.

Done.

https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
File gpu/config/gpu_driver_bug_workaround_type.h (right):

https://codereview.chromium.org/1068573004/diff/20001/gpu/config/gpu_driver_b...
gpu/config/gpu_driver_bug_workaround_type.h:35:
GPU_OP(disable_discard_framebuffer,                        \
On 2015/04/07 16:25:59, David Yen wrote:
> This should be capitalized

Done.

[vmiura, 2015-04-07 21:00:15]

> No, this sort of gating is not reliable and has failed us before.  It's common
> for vendors to update devices to new Android releases without changing the old
> driver at all.

If we can't narrow this, we may need to look into more light weight
workardounds,
as removing all framebuffer discards will have a performance impact.

We could perhaps remove the discard only in this scenario, where no drawing
ocurred to the FBO.

[no sievers, 2015-04-07 21:12:09]

On 2015/04/07 19:43:31, aelias wrote:
> Daniel, ping for OWNERS review?
> 
> On 2015/04/07 at 17:13:21, dyen wrote:
> > I just tested this on an android one which has a Mali400 running 4.4.4. It
> looks like this problem is fixed in the driver version it has. Should we gate
> this to "< android 4.4.4"? The only problem is we would have to rely on
vendors
> updating drivers.
> 
> No, this sort of gating is not reliable and has failed us before.  It's common
> for vendors to update devices to new Android releases without changing the old
> driver at all.

lgtm.

Generally, I think it's good to always try to deprecate. The main risk of a
mismatch between workaround list and new OS version shipping (OTA or new device)
is probably either when the releases badly overlap so that it misses testing. Or
the update is minor and doesn't get enough testing (or any testing with Chrome).
Maybe deprecating for more major future OS versions is more practicable.
 
Don't feel strongly here, since it's an older chip/driver, and the feature is
more of an optional optmization.

[aelias_OOO_until_Jul13, 2015-04-07 21:23:19]

On 2015/04/07 at 21:00:15, vmiura wrote:
> If we can't narrow this, we may need to look into more light weight
workardounds,
> as removing all framebuffer discards will have a performance impact.
> 
> We could perhaps remove the discard only in this scenario, where no drawing
> ocurred to the FBO.

That's something we can explore in the future, but shouldn't block landing this
patch I think.

On 2015/04/07 at 21:12:09, sievers wrote:
> Generally, I think it's good to always try to deprecate. The main risk of a
mismatch between workaround list and new OS version shipping (OTA or new device)
is probably either when the releases badly overlap so that it misses testing. Or
the update is minor and doesn't get enough testing (or any testing with Chrome).
Maybe deprecating for more major future OS versions is more practicable.

The Galaxy S5 mini has this bug and currently has Android 4.4.2.  Given that
it's new, that device will likely get Lollipop, and based on past experience
there's no reason to expect it will get a driver fix at the same time.  So I
really think we'd end up forced to go back and remove any Android-based version
check.

[no sievers, 2015-04-07 21:27:34]

On 2015/04/07 21:23:19, aelias wrote:
> On 2015/04/07 at 21:00:15, vmiura wrote:
> > If we can't narrow this, we may need to look into more light weight
> workardounds,
> > as removing all framebuffer discards will have a performance impact.
> > 
> > We could perhaps remove the discard only in this scenario, where no drawing
> > ocurred to the FBO.
> 
> That's something we can explore in the future, but shouldn't block landing
this
> patch I think.
> 
> On 2015/04/07 at 21:12:09, sievers wrote:
> > Generally, I think it's good to always try to deprecate. The main risk of a
> mismatch between workaround list and new OS version shipping (OTA or new
device)
> is probably either when the releases badly overlap so that it misses testing.
Or
> the update is minor and doesn't get enough testing (or any testing with
Chrome).
> Maybe deprecating for more major future OS versions is more practicable.
> 
> The Galaxy S5 mini has this bug and currently has Android 4.4.2.  Given that
> it's new, that device will likely get Lollipop, and based on past experience
> there's no reason to expect it will get a driver fix at the same time.  So I
> really think we'd end up forced to go back and remove any Android-based
version
> check.

The cost of doing this is low though. I guess we should really look at it the
other way around:
Not enabling the workaround is the only way to get it fixed, so it comes down to
how much we care about potentially getting it fixed.
And I don't feel strongly either way for this combination of chip and issue.

[aelias_OOO_until_Jul13, 2015-04-07 21:39:35]

We don't have any real leverage to force OEMs to fix their drivers with version
cutoffs.  We already had to roll back the L Ganesh whitelist after multiple
different devices got L with broken drivers.  OEMs will either go ahead and
break Chrome (because they also have their own browser and either don't test
Chrome or assume Chrome-team will deal with it), or they'll fake their
GL_RENDERER string to trick us into getting the workaround back.  Both of those
things have happened in the past.  So not only does this not seem important
enough to justify the high likelihood of the corruption going back, I don't
think that measure turns out to be effective.

[aelias_OOO_until_Jul13, 2015-04-07 22:32:20]

OK, I chatted with Victor offline and we can land this and revisit later.

[aelias_OOO_until_Jul13, 2015-04-07 22:32:28]

The CQ bit was checked by aelias@chromium.org

[aelias_OOO_until_Jul13, 2015-04-07 22:32:28]

The patchset sent to the CQ was uploaded after l-g-t-m from dyen@chromium.org,
sievers@chromium.org
Link to the patchset: https://codereview.chromium.org/1068573004/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2015-04-07 22:34:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1068573004/60001

[commit-bot: I haz the power, 2015-04-07 23:19:45]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-07 23:21:30]

Patchset 4 (id:??) landed as
https://crrev.com/8ef99b8a51842a038c64cd2b1ab1896228651ec2
Cr-Commit-Position: refs/heads/master@{#324152}