Optimize plane usage when primary is obscured by Overlay

Optimize plane usage when primary is obscured by Overlay

Currently, when playing Video in fullscreen and using Overlays, we
update both Primary and Overlay planes. This means hardware needs to
scan out primary and Overlay plane. This is redundant in case buffer
bound to Overlay is not transparent and covers the complete region of
primary. In such cases, if primary can support the given format, we
can bound the buffer to primary itself and avoid updating the overlay
plane. This avoids the need to scan an extra plane with size equal
to the resolution.

BUG=370522
Committed: https://crrev.com/e7c278226f369deb471963d68dc79ea02ee9ab31
Cr-Commit-Position: refs/heads/master@{#356938}

[kalyank, 2015-10-26 00:09:15]

Description was changed from

==========
Optimize plane usage when primary is Obscured by Overlay

Currently, when playing Video in fullscreen and using Overlays, we
update both Primary and Overlay planes. This means hardware needs to
scan out primary and Overlay plane. This is redundant in case buffer
bound to Overlay is not transparent and covers the complete region of
primary. In such cases, if primary can support the given format, we
can bound the buffer to primary itself and avoid updating the overlay
plane. This avoids the need to scan an extra plane with size equal
to the resolution.

BUG=370522
==========

to

==========
Optimize plane usage when primary is Obscured by Overlay

Currently, when playing Video in fullscreen and using Overlays, we
update both Primary and Overlay planes. This means hardware needs to
scan out primary and Overlay plane. This is redundant in case buffer
bound to Overlay is not transparent and covers the complete region of
primary. In such cases, if primary can support the given format, we
can bound the buffer to primary itself and avoid updating the overlay
plane. This avoids the need to scan an extra plane with size equal
to the resolution.

BUG=370522
==========

[kalyank, 2015-10-26 00:09:16]

kalyan.kondapally@intel.com changed reviewers:
+ achaulk@chromium.org, dnicoara@chromium.org

[kalyank, 2015-10-26 00:11:10]

@dnicoara @achaulk PTAL

[kalyank, 2015-10-26 06:42:13]

Description was changed from

==========
Optimize plane usage when primary is Obscured by Overlay

Currently, when playing Video in fullscreen and using Overlays, we
update both Primary and Overlay planes. This means hardware needs to
scan out primary and Overlay plane. This is redundant in case buffer
bound to Overlay is not transparent and covers the complete region of
primary. In such cases, if primary can support the given format, we
can bound the buffer to primary itself and avoid updating the overlay
plane. This avoids the need to scan an extra plane with size equal
to the resolution.

BUG=370522
==========

to

==========
Optimize plane usage when primary is obscured by Overlay

Currently, when playing Video in fullscreen and using Overlays, we
update both Primary and Overlay planes. This means hardware needs to
scan out primary and Overlay plane. This is redundant in case buffer
bound to Overlay is not transparent and covers the complete region of
primary. In such cases, if primary can support the given format, we
can bound the buffer to primary itself and avoid updating the overlay
plane. This avoids the need to scan an extra plane with size equal
to the resolution.

BUG=370522
==========

[kalyank, 2015-10-27 16:51:00]

@dnicoara ping

[dnicoara, 2015-10-29 15:28:40]

dnicoara@chromium.org changed reviewers:
+ alexst@chromium.org

[dnicoara, 2015-10-29 15:28:41]

+ alexst@

Would it be possible to have the Compositor drop the primary plane in this case?
I'm trying to understand why the Compositor wouldn't ask for a 1 plane UYVY
configuration? I'd imagine the overlay validator would be able to say yes to
that and we wouldn't have a workaround like this.

[achaulk, 2015-10-29 15:34:54]

lgtm but it probably does make more sense to detect this in the compositor, yes

[alexst (slow to review), 2015-10-29 16:07:12]

lgtm, I think it's okay to optimize this in the platform.

Although ideally CC would do this for us, what CC really needs is a general
purpose flexible overlay damage tracker in the browser process,   and adding
little things like this one by one is not the way to go about that. It'll take a
while for that to happen either way.

[kalyank, 2015-10-29 16:15:05]

On 2015/10/29 16:07:12, alexst (slow to review) wrote:
> lgtm, I think it's okay to optimize this in the platform.
> 
> Although ideally CC would do this for us, what CC really needs is a general
> purpose flexible overlay damage tracker in the browser process,   and adding
> little things like this one by one is not the way to go about that. It'll take
a
> while for that to happen either way.

Ya and also we would need to expose more platform specific information like the
formats
supported before we can collapse the planes to one.

[dnicoara, 2015-10-29 16:17:20]

On 2015/10/29 16:07:12, alexst (slow to review) wrote:
> lgtm, I think it's okay to optimize this in the platform.
> 
> Although ideally CC would do this for us, what CC really needs is a general
> purpose flexible overlay damage tracker in the browser process,   and adding
> little things like this one by one is not the way to go about that. It'll take
a
> while for that to happen either way.

There should at least be a bug tracking that and a TODO to move this in there.

kalyank@ also, I am concerned that page-flipping a buffer with a different
format will fail. We've had this issue when trying to modeset with a dumb buffer
but page flip with a GBM buffer. This may show up in this case since the
alignment of different buffer formats will be change.

[kalyank, 2015-10-29 16:36:05]

On 2015/10/29 16:17:20, dnicoara wrote:
> On 2015/10/29 16:07:12, alexst (slow to review) wrote:
> > lgtm, I think it's okay to optimize this in the platform.
> > 
> > Although ideally CC would do this for us, what CC really needs is a general
> > purpose flexible overlay damage tracker in the browser process,   and adding
> > little things like this one by one is not the way to go about that. It'll
take
> a
> > while for that to happen either way.
> 
> There should at least be a bug tracking that and a TODO to move this in there.
> 
> kalyank@ also, I am concerned that page-flipping a buffer with a different
> format will fail. We've had this issue when trying to modeset with a dumb
buffer
> but page flip with a GBM buffer. This may show up in this case since the
> alignment of different buffer formats will be change.

We could restrict this to XRGB format only/ check format of primary and overlay
buffers
are same.

[kalyank, 2015-10-29 16:40:35]

On 2015/10/29 16:36:05, kalyank wrote:
> On 2015/10/29 16:17:20, dnicoara wrote:
> > On 2015/10/29 16:07:12, alexst (slow to review) wrote:
> > > lgtm, I think it's okay to optimize this in the platform.
> > > 
> > > Although ideally CC would do this for us, what CC really needs is a
general
> > > purpose flexible overlay damage tracker in the browser process,   and
adding
> > > little things like this one by one is not the way to go about that. It'll
> take
> > a
> > > while for that to happen either way.
> > 
> > There should at least be a bug tracking that and a TODO to move this in
there.
> > 
> > kalyank@ also, I am concerned that page-flipping a buffer with a different
> > format will fail. We've had this issue when trying to modeset with a dumb
> buffer
> > but page flip with a GBM buffer. This may show up in this case since the
> > alignment of different buffer formats will be change.
> 
> We could restrict this to XRGB format only/ check format of primary and
overlay
> buffers
> are same.

Also, I will add a TODO, till we can build general logic in
OverlayProcessor/validator
for this kind of scenario.

[dnicoara, 2015-10-29 16:40:52]

On 2015/10/29 16:36:05, kalyank wrote:
> On 2015/10/29 16:17:20, dnicoara wrote:
> > On 2015/10/29 16:07:12, alexst (slow to review) wrote:
> > > lgtm, I think it's okay to optimize this in the platform.
> > > 
> > > Although ideally CC would do this for us, what CC really needs is a
general
> > > purpose flexible overlay damage tracker in the browser process,   and
adding
> > > little things like this one by one is not the way to go about that. It'll
> take
> > a
> > > while for that to happen either way.
> > 
> > There should at least be a bug tracking that and a TODO to move this in
there.
> > 
> > kalyank@ also, I am concerned that page-flipping a buffer with a different
> > format will fail. We've had this issue when trying to modeset with a dumb
> buffer
> > but page flip with a GBM buffer. This may show up in this case since the
> > alignment of different buffer formats will be change.
> 
> We could restrict this to XRGB format only/ check format of primary and
overlay
> buffers
> are same.

That sounds fine. Just curious, do you know if there are any such restrictions
for atomic?

[kalyank, 2015-10-29 16:50:14]

On 2015/10/29 16:40:52, dnicoara wrote:
> On 2015/10/29 16:36:05, kalyank wrote:
> > On 2015/10/29 16:17:20, dnicoara wrote:
> > > On 2015/10/29 16:07:12, alexst (slow to review) wrote:
> > > > lgtm, I think it's okay to optimize this in the platform.
> > > > 
> > > > Although ideally CC would do this for us, what CC really needs is a
> general
> > > > purpose flexible overlay damage tracker in the browser process,   and
> adding
> > > > little things like this one by one is not the way to go about that.
It'll
> > take
> > > a
> > > > while for that to happen either way.
> > > 
> > > There should at least be a bug tracking that and a TODO to move this in
> there.
> > > 
> > > kalyank@ also, I am concerned that page-flipping a buffer with a different
> > > format will fail. We've had this issue when trying to modeset with a dumb
> > buffer
> > > but page flip with a GBM buffer. This may show up in this case since the
> > > alignment of different buffer formats will be change.
> > 
> > We could restrict this to XRGB format only/ check format of primary and
> overlay
> > buffers
> > are same.
> 
> That sounds fine. Just curious, do you know if there are any such restrictions
> for atomic?

Atomic doesn't seem to have these restrictions even though the legacy path does
(Even though Hardware might have supported the format conversion). 
Ideally all these scenarios go through the normal test commit, so we are 
absolutely sure that the actual commit will pass.

[kalyank, 2015-10-29 18:14:03]

On 2015/10/29 16:40:52, dnicoara wrote:
> On 2015/10/29 16:36:05, kalyank wrote:
> > On 2015/10/29 16:17:20, dnicoara wrote:
> > > On 2015/10/29 16:07:12, alexst (slow to review) wrote:
> > > > lgtm, I think it's okay to optimize this in the platform.
> > > > 
> > > > Although ideally CC would do this for us, what CC really needs is a
> general
> > > > purpose flexible overlay damage tracker in the browser process,   and
> adding
> > > > little things like this one by one is not the way to go about that.
It'll
> > take
> > > a
> > > > while for that to happen either way.
> > > 
> > > There should at least be a bug tracking that and a TODO to move this in
> there.
> > > 
> > > kalyank@ also, I am concerned that page-flipping a buffer with a different
> > > format will fail. We've had this issue when trying to modeset with a dumb
> > buffer
> > > but page flip with a GBM buffer. This may show up in this case since the
> > > alignment of different buffer formats will be change.
> > 
> > We could restrict this to XRGB format only/ check format of primary and
> overlay
> > buffers
> > are same.
> 
> That sounds fine. Just curious, do you know if there are any such restrictions
> for atomic?

Actually, I misread this. I think the failure when using dumb buffer for modeset
and
trying to use GBM buffer for page flip could be due to the tiling (linear vs x).

However, I need to check. I am ok restricting this to XRGB format for now and
expand 
it as we get to actually test with other formats.

[dnicoara, 2015-10-29 18:20:41]

On 2015/10/29 18:14:03, kalyank wrote:
> On 2015/10/29 16:40:52, dnicoara wrote:
> > On 2015/10/29 16:36:05, kalyank wrote:
> > > On 2015/10/29 16:17:20, dnicoara wrote:
> > > > On 2015/10/29 16:07:12, alexst (slow to review) wrote:
> > > > > lgtm, I think it's okay to optimize this in the platform.
> > > > > 
> > > > > Although ideally CC would do this for us, what CC really needs is a
> > general
> > > > > purpose flexible overlay damage tracker in the browser process,   and
> > adding
> > > > > little things like this one by one is not the way to go about that.
> It'll
> > > take
> > > > a
> > > > > while for that to happen either way.
> > > > 
> > > > There should at least be a bug tracking that and a TODO to move this in
> > there.
> > > > 
> > > > kalyank@ also, I am concerned that page-flipping a buffer with a
different
> > > > format will fail. We've had this issue when trying to modeset with a
dumb
> > > buffer
> > > > but page flip with a GBM buffer. This may show up in this case since the
> > > > alignment of different buffer formats will be change.
> > > 
> > > We could restrict this to XRGB format only/ check format of primary and
> > overlay
> > > buffers
> > > are same.
> > 
> > That sounds fine. Just curious, do you know if there are any such
restrictions
> > for atomic?
> 
> Actually, I misread this. I think the failure when using dumb buffer for
modeset
> and
> trying to use GBM buffer for page flip could be due to the tiling (linear vs
x).
> 
> However, I need to check. I am ok restricting this to XRGB format for now and
> expand 
> it as we get to actually test with other formats.

If I remember correctly the issue was that the stride was different on devices
whose mode width wasn't divisible by 4. Though, that could have been a side
effect of tiling as well.

Based on my understanding any setting that is set during the modeset and
implicitly used during page flips may cause failures if they no longer match.
Though mileage may vary since I've seen cases where the driver can handle some
changes.

[kalyank, 2015-10-29 18:59:52]

> If I remember correctly the issue was that the stride was different on devices
> whose mode width wasn't divisible by 4. Though, that could have been a side
> effect of tiling as well.

K, I will check this and get back to you. 


I have added format checks now. Can you PTAL

[dnicoara, 2015-10-29 19:30:34]

On 2015/10/29 18:59:52, kalyank wrote:
> > If I remember correctly the issue was that the stride was different on
devices
> > whose mode width wasn't divisible by 4. Though, that could have been a side
> > effect of tiling as well.
> 
> K, I will check this and get back to you. 
> 
> 
> I have added format checks now. Can you PTAL

Thank you.

lgtm

[kalyank, 2015-10-29 20:18:41]

The CQ bit was checked by kalyan.kondapally@intel.com

[kalyank, 2015-10-29 20:18:41]

The patchset sent to the CQ was uploaded after l-g-t-m from alexst@chromium.org,
achaulk@chromium.org
Link to the patchset: https://codereview.chromium.org/1423793003/#ps60001
(title: "Check that primary and Overlay formats are same")

[commit-bot: I haz the power, 2015-10-29 20:19:29]

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

[commit-bot: I haz the power, 2015-10-29 20:37:35]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-29 20:38:28]

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