ozone: Add an opaque fb to ScanoutBuffer for primary planes.

ozone: Add an opaque fb to ScanoutBuffer for primary planes.

While all planes on rockchip support alpha, scanning out a primary
plane with an fb that was created with a format with alpha will result
in black pixels.

This patch adds an additional fb to a ScanoutBuffer to be used for the
primary plane.
When a buffer is created with a format that has alpha (e.g: RGBA),
an additional fb with a format without alpha (e.g. RGBX) will be added for
the same buffer.

In this way, when the buffer is scheduled for scanout as primary plane
we can use the fb added with a format without alpha and actually see
colors on the screen, instead of having everything black.

BUG=695296
Review-Url: https://codereview.chromium.org/2743403005
Cr-Commit-Position: refs/heads/master@{#457482}
Committed: https://chromium.googlesource.com/chromium/src/+/9d7a8aef95959ab06d2b565fe129fd27ed42b963

[Daniele Castagna, 2017-03-15 05:39:23]

The CQ bit was checked by dcastagna@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 05:39:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Daniele Castagna, 2017-03-15 16:20:02]

The CQ bit was checked by dcastagna@chromium.org to run a CQ dry run

[Daniele Castagna, 2017-03-15 16:20:10]

dcastagna@chromium.org changed reviewers:
+ hoegsberg@chromium.org, reveman@chromium.org

[commit-bot: I haz the power, 2017-03-15 16:20:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Daniele Castagna, 2017-03-15 16:25:29]

Description was changed from

==========
ozone: Add a second fb to ScanoutBuffer for primary planes.

While all the planes on rockchip support alpha, scanning out a primary
plane with an fb that was created with a format with alpha will result
in black pixels.

This patch adds an additional fb to a ScanoutBuffer to be used for the
primary plane.
When a buffer is created with a format that has alpha (e.g: RGBA),
an additional fb with a format without alpha (e.g. RGBX) will be added for
the same buffer.

In this way, when the buffer is scheduled for scanout as primary plane
we can use the fb added with a format without alpha and actually see
color on the screen, instead of having everything black.

BUG=695296
==========

to

==========
ozone: Add a second fb to ScanoutBuffer for primary planes.

While all planes on rockchip support alpha, scanning out a primary
plane with an fb that was created with a format with alpha will result
in black pixels.

This patch adds an additional fb to a ScanoutBuffer to be used for the
primary plane.
When a buffer is created with a format that has alpha (e.g: RGBA),
an additional fb with a format without alpha (e.g. RGBX) will be added for
the same buffer.

In this way, when the buffer is scheduled for scanout as primary plane
we can use the fb added with a format without alpha and actually see
color on the screen, instead of having everything black.

BUG=695296
==========

[commit-bot: I haz the power, 2017-03-15 16:49:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 16:49:23]

Dry run: This issue passed the CQ dry run.

[hoegsberg1, 2017-03-15 18:05:28]

hoegsberg@google.com changed reviewers:
+ hoegsberg@google.com

[hoegsberg1, 2017-03-15 18:05:29]

This looks good to me, though I'd suggest calling it OpaqueFramebuffer instead
of PrimaryPlaneFramebuffer throughout.

[reveman, 2017-03-15 18:21:46]

On 2017/03/15 at 18:05:29, hoegsberg wrote:
> This looks good to me, though I'd suggest calling it OpaqueFramebuffer instead
of PrimaryPlaneFramebuffer throughout.

+1 as we'll need this to support RGBA buffers without blending. This is
something Android will sometimes use as it gives the framework flexibility to
chose between RGB and RGBA without having to reallocate buffers.

[Daniele Castagna, 2017-03-15 21:56:03]

Description was changed from

==========
ozone: Add a second fb to ScanoutBuffer for primary planes.

While all planes on rockchip support alpha, scanning out a primary
plane with an fb that was created with a format with alpha will result
in black pixels.

This patch adds an additional fb to a ScanoutBuffer to be used for the
primary plane.
When a buffer is created with a format that has alpha (e.g: RGBA),
an additional fb with a format without alpha (e.g. RGBX) will be added for
the same buffer.

In this way, when the buffer is scheduled for scanout as primary plane
we can use the fb added with a format without alpha and actually see
color on the screen, instead of having everything black.

BUG=695296
==========

to

==========
ozone: Add an opaque fb to ScanoutBuffer for primary planes.

While all planes on rockchip support alpha, scanning out a primary
plane with an fb that was created with a format with alpha will result
in black pixels.

This patch adds an additional fb to a ScanoutBuffer to be used for the
primary plane.
When a buffer is created with a format that has alpha (e.g: RGBA),
an additional fb with a format without alpha (e.g. RGBX) will be added for
the same buffer.

In this way, when the buffer is scheduled for scanout as primary plane
we can use the fb added with a format without alpha and actually see
colors on the screen, instead of having everything black.

BUG=695296
==========

[Daniele Castagna, 2017-03-15 21:56:26]

Replaced PrimaryPlane with Opaque. PTAL.

[Daniele Castagna, 2017-03-15 21:56:45]

The CQ bit was checked by dcastagna@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 21:57:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[reveman, 2017-03-15 21:58:50]

lgtm

[Daniele Castagna, 2017-03-15 21:59:31]

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

[Daniele Castagna, 2017-03-15 21:59:32]

+dnicoara for ownership.

[commit-bot: I haz the power, 2017-03-15 22:28:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 22:28:32]

Dry run: This issue passed the CQ dry run.

[dnicoara, 2017-03-16 17:24:14]

lgtm

For clarity, is this a quirk in the driver (ie: rockchip)? This is what I gather
from the comment in drm_util.cc. Or is is a composition issue in Chrome where
the alpha isn't set correctly?

[hoegsberg1, 2017-03-16 17:29:41]

On 2017/03/16 17:24:14, dnicoara wrote:
> lgtm
> 
> For clarity, is this a quirk in the driver (ie: rockchip)? This is what I
gather
> from the comment in drm_util.cc. Or is is a composition issue in Chrome where
> the alpha isn't set correctly?

It's not so much a quirk - it if is it would be in the KMS API. There's no way
to tell KMS to not blend a buffer if you've created the KMS fb with a format
that has alpha channel. Alternatively, there is, and the way you do it is this -
create a KMS fb without the alpha channel format. (ie XRGB for ARGB).

[Daniele Castagna, 2017-03-16 17:38:58]

The CQ bit was checked by dcastagna@chromium.org

[commit-bot: I haz the power, 2017-03-16 17:39:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-16 17:45:09]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1489685938346340,
"parent_rev": "97ea0f4cf155818e36cc00338e2645881ee0ba02", "commit_rev":
"9d7a8aef95959ab06d2b565fe129fd27ed42b963"}

[commit-bot: I haz the power, 2017-03-16 17:45:48]

Description was changed from

==========
ozone: Add an opaque fb to ScanoutBuffer for primary planes.

While all planes on rockchip support alpha, scanning out a primary
plane with an fb that was created with a format with alpha will result
in black pixels.

This patch adds an additional fb to a ScanoutBuffer to be used for the
primary plane.
When a buffer is created with a format that has alpha (e.g: RGBA),
an additional fb with a format without alpha (e.g. RGBX) will be added for
the same buffer.

In this way, when the buffer is scheduled for scanout as primary plane
we can use the fb added with a format without alpha and actually see
colors on the screen, instead of having everything black.

BUG=695296
==========

to

==========
ozone: Add an opaque fb to ScanoutBuffer for primary planes.

While all planes on rockchip support alpha, scanning out a primary
plane with an fb that was created with a format with alpha will result
in black pixels.

This patch adds an additional fb to a ScanoutBuffer to be used for the
primary plane.
When a buffer is created with a format that has alpha (e.g: RGBA),
an additional fb with a format without alpha (e.g. RGBX) will be added for
the same buffer.

In this way, when the buffer is scheduled for scanout as primary plane
we can use the fb added with a format without alpha and actually see
colors on the screen, instead of having everything black.

BUG=695296

Review-Url: https://codereview.chromium.org/2743403005
Cr-Commit-Position: refs/heads/master@{#457482}
Committed:
https://chromium.googlesource.com/chromium/src/+/9d7a8aef95959ab06d2b565fe129...
==========

[commit-bot: I haz the power, 2017-03-16 17:45:50]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/9d7a8aef95959ab06d2b565fe129...

[dnicoara, 2017-03-16 17:47:49]

On 2017/03/16 17:29:41, hoegsberg1 wrote:
> On 2017/03/16 17:24:14, dnicoara wrote:
> > lgtm
> > 
> > For clarity, is this a quirk in the driver (ie: rockchip)? This is what I
> gather
> > from the comment in drm_util.cc. Or is is a composition issue in Chrome
where
> > the alpha isn't set correctly?
> 
> It's not so much a quirk - it if is it would be in the KMS API. There's no way
> to tell KMS to not blend a buffer if you've created the KMS fb with a format
> that has alpha channel. Alternatively, there is, and the way you do it is this
-
> create a KMS fb without the alpha channel format. (ie XRGB for ARGB).

Thanks, Daniele gave me more details as well.