content: Partial copy workaround for devices with known upload performance problems.

content: Partial copy workaround for devices with known upload performance problems.

1/4 MB per copy operation on Mali pre-Lollipop.

BUG=460288, 498443
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/e92ab037dbaab094d7b2253d63fd60cd48e9f9dc
Cr-Commit-Position: refs/heads/master@{#333641}

[reveman, 2015-05-26 17:29:42]

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

[reveman, 2015-05-26 17:29:43]

This is the last change we need before we could try to enable one-copy on
Android. This list of partial copy adjustments needs to be extended. It's
limited to Nexus 10 on KitKat and earlier right now as that's what I'm aware of.
I was thinking we can land this with an initial set of known devices that need
partial one-copy and then extend it as we find more devices that need it. Or
maybe we should use a 1/4MB limit on all pre-L devices?

[no sievers, 2015-05-26 21:12:07]

On 2015/05/26 17:29:43, reveman wrote:
> This is the last change we need before we could try to enable one-copy on
> Android. This list of partial copy adjustments needs to be extended. It's
> limited to Nexus 10 on KitKat and earlier right now as that's what I'm aware
of.
> I was thinking we can land this with an initial set of known devices that need
> partial one-copy and then extend it as we find more devices that need it. Or
> maybe we should use a 1/4MB limit on all pre-L devices?

I thought you had mentioned Nexus 4 also, in which case (if it affects Qualcomm
in general) we might consider making it the default.

Do we flush the cmdbuffer after every partial upload?

[reveman, 2015-05-26 22:42:23]

On 2015/05/26 at 21:12:07, sievers wrote:
> On 2015/05/26 17:29:43, reveman wrote:
> > This is the last change we need before we could try to enable one-copy on
> > Android. This list of partial copy adjustments needs to be extended. It's
> > limited to Nexus 10 on KitKat and earlier right now as that's what I'm aware
of.
> > I was thinking we can land this with an initial set of known devices that
need
> > partial one-copy and then extend it as we find more devices that need it. Or
> > maybe we should use a 1/4MB limit on all pre-L devices?
> 
> I thought you had mentioned Nexus 4 also, in which case (if it affects
Qualcomm in general) we might consider making it the default.

Default on all Qualcomm or KitKit in general? If just Qualcomm, what's the best
way to detect that? Ideally we'd be able to do this from RenderWidgetCompositor
code where we determine similar constants based on device limitations.

> 
> Do we flush the cmdbuffer after every partial upload?

I haven't changed the flush interval. It's still 4 copy operations per flush but
now that I think of it, probably makes more sense to define this limit as the
number of bytes we can issue copy operations for before we need to flush.

[no sievers, 2015-05-26 23:00:23]

On 2015/05/26 22:42:23, reveman wrote:
> On 2015/05/26 at 21:12:07, sievers wrote:
> > On 2015/05/26 17:29:43, reveman wrote:
> > > This is the last change we need before we could try to enable one-copy on
> > > Android. This list of partial copy adjustments needs to be extended. It's
> > > limited to Nexus 10 on KitKat and earlier right now as that's what I'm
aware
> of.
> > > I was thinking we can land this with an initial set of known devices that
> need
> > > partial one-copy and then extend it as we find more devices that need it.
Or
> > > maybe we should use a 1/4MB limit on all pre-L devices?
> > 
> > I thought you had mentioned Nexus 4 also, in which case (if it affects
> Qualcomm in general) we might consider making it the default.
> 
> Default on all Qualcomm or KitKit in general? If just Qualcomm, what's the
best
> way to detect that? Ideally we'd be able to do this from
RenderWidgetCompositor
> code where we determine similar constants based on device limitations.
> 
> > 
> > Do we flush the cmdbuffer after every partial upload?
> 
> I haven't changed the flush interval. It's still 4 copy operations per flush
but
> now that I think of it, probably makes more sense to define this limit as the
> number of bytes we can issue copy operations for before we need to flush.

That make sense. And with the kUIPrioritizeInGpuProcess this should cause
pending flushes from the browser compositor to be preferred over ones from the
renderer.

[aelias_OOO_until_Jul13, 2015-06-02 18:34:02]

We usually toggle GPU workarounds based on the GL_RENDERER string.  We've almost
completely avoided putting them in the render process though (most are in the
GPU service, a few in the browser) so I'm not sure exactly how to get to that
here offhand.

[no sievers, 2015-06-02 19:55:01]

On 2015/06/02 18:34:02, aelias wrote:
> We usually toggle GPU workarounds based on the GL_RENDERER string.  We've
almost
> completely avoided putting them in the render process though (most are in the
> GPU service, a few in the browser) so I'm not sure exactly how to get to that
> here offhand.

We talked offline and thought what might work is let the gpu service decide
(through workaround list or other logic) and pass this back (as either a bool or
number) to the client through the gpu capabilities structure.

[reveman, 2015-06-09 21:00:32]

Latest patch implements this using the workaround list. PTAL.

[no sievers, 2015-06-09 23:01:16]

lgtm

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

https://codereview.chromium.org/1159833002/diff/40001/gpu/config/gpu_driver_b...
gpu/config/gpu_driver_bug_workaround_type.h:135:
GPU_OP(MAX_COPY_TEXTURE_CHROMIUM_SIZE_262144,              \
nit: Can you order this alphabetically (same for DISABLE_GL_RGB_FORMAT above)?

[reveman, 2015-06-09 23:17:00]

reveman@chromium.org changed reviewers:
+ vmpstr@chromium.org

[reveman, 2015-06-09 23:17:00]

+vmpstr for cc/

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

https://codereview.chromium.org/1159833002/diff/40001/gpu/config/gpu_driver_b...
gpu/config/gpu_driver_bug_workaround_type.h:135:
GPU_OP(MAX_COPY_TEXTURE_CHROMIUM_SIZE_262144,              \
On 2015/06/09 at 23:01:15, sievers wrote:
> nit: Can you order this alphabetically (same for DISABLE_GL_RGB_FORMAT above)?

Done.

[vmpstr, 2015-06-09 23:17:57]

cc lgtm

[reveman, 2015-06-09 23:26:23]

The CQ bit was checked by reveman@chromium.org

[reveman, 2015-06-09 23:26:23]

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

[commit-bot: I haz the power, 2015-06-09 23:28:12]

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

[commit-bot: I haz the power, 2015-06-10 01:39:33]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-06-10 01:40:36]

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