New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 834015 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Task



Sign in to add a comment

Auto-detect atomic kms

Project Member Reported by hoegsberg@chromium.org, Apr 17 2018

Issue description

We only turn on the atomic kms backend in ozone if the flag --enable-drm-atomic is passed to chrome. We should be able to determine whether atomic is available and select it automatically.

If a chromeos kernel advertises atomic, but doesn't fully support it, we should fix the kernel/board combination to not do that.

We should remove the option once this is landed. There may be a case for being able to disable atomic kms for testing purposes, but I'm not convinced that's worth it.
 
Cc: alexst@chromium.org hoegsberg@chromium.org
 Issue 822860  has been merged into this issue.
How should we proceed with this? Should we land the change and see what breaks?
Is anyone aware of any board/kernel that claims to support atomic but we don't want to enable it there?
Cc: marc...@chromium.org
ccing Stephane that is aware of boards where atomic is "suppported" but where we shouldn't enable it.
On v3.8 I don't see any mention of atomic.
On v3.18 I see .driver_features = DRIVER_ATOMIC or similar for i915, mediatek and tegra, should we just remove those?

On intel we have a comment saying:
        /*                                                                                                                                                                                                                                                                                    
         * FIXME: Note that we're lying to the DRM core here so that we can get access                                                                                                                                                                                                        
         * to the atomic ioctl and the atomic properties.  Only plane operations on                                                                                                                                                                                                           
         * a single CRTC will actually work.                                                                                                                                                                                                                                                  
         */

Cc: seanpaul@chromium.org
Yeah, I think the i915 3.18 comment is the half-baked KMS that Stephane referred to. Should be safe to just drop the DRIVER_ATOMIC bit from all pre-4.4 KMS drivers.
3.8 has broken atomic backports but the atomic flag isn't exposed
3.10 doesn't have backports
3.14 has broken atomic backports but the atomic flag isn't exposed
3.18 has broken atomic backports and has the flag. I should be enough to just remove the flag there....

And looking at drm_setclientcap in the 3.18 kernel:

        case DRM_CLIENT_CAP_ATOMIC:
                /* for now, hide behind experimental drm.atomic moduleparam */
                if (!drm_atomic)
                        return -EINVAL;

which defaults to false and we don't set the moduleparam - I just checked elm, caroline and braswell.

So we don't actually need anything on the kernel side for this to work.
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a2d41ed8321c406c618cb8492bcf4aaf4aa7037d

commit a2d41ed8321c406c618cb8492bcf4aaf4aa7037d
Author: Daniele Castagna <dcastagna@chromium.org>
Date: Wed Apr 18 23:55:38 2018

ozone: drm: Use atomic where avaialable

Currently we enable drm-atomic only when the flag --enable-drm-atomic
is passed to chrome.

This CL removes 'enable-drm-atomic' flag and makes Chrome use drm atomic
when the device claims it is supported.

Bug:  834015 
Test: deploy chrome on eve.
Change-Id: I4e33120a0ae5055411fad3667dc125948acbe160
Reviewed-on: https://chromium-review.googlesource.com/1015705
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Kristian H. Kristensen <hoegsberg@chromium.org>
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551885}
[modify] https://crrev.com/a2d41ed8321c406c618cb8492bcf4aaf4aa7037d/chrome/browser/chromeos/login/chrome_restart_request.cc
[modify] https://crrev.com/a2d41ed8321c406c618cb8492bcf4aaf4aa7037d/components/exo/display.cc
[modify] https://crrev.com/a2d41ed8321c406c618cb8492bcf4aaf4aa7037d/content/browser/gpu/gpu_data_manager_impl_private.cc
[modify] https://crrev.com/a2d41ed8321c406c618cb8492bcf4aaf4aa7037d/ui/ozone/platform/drm/gpu/drm_device.cc
[modify] https://crrev.com/a2d41ed8321c406c618cb8492bcf4aaf4aa7037d/ui/ozone/platform/drm/gpu/drm_device.h
[modify] https://crrev.com/a2d41ed8321c406c618cb8492bcf4aaf4aa7037d/ui/ozone/platform/drm/gpu/drm_device_generator.cc
[modify] https://crrev.com/a2d41ed8321c406c618cb8492bcf4aaf4aa7037d/ui/ozone/platform/drm/gpu/drm_thread.cc
[modify] https://crrev.com/a2d41ed8321c406c618cb8492bcf4aaf4aa7037d/ui/ozone/platform/drm/gpu/gbm_device.cc
[modify] https://crrev.com/a2d41ed8321c406c618cb8492bcf4aaf4aa7037d/ui/ozone/platform/drm/gpu/gbm_device.h
[modify] https://crrev.com/a2d41ed8321c406c618cb8492bcf4aaf4aa7037d/ui/ozone/public/ozone_switches.cc
[modify] https://crrev.com/a2d41ed8321c406c618cb8492bcf4aaf4aa7037d/ui/ozone/public/ozone_switches.h

Yay, let's wait for that to make it into a chrome uprev, then drop the flags in the various board specific places and we're done.
Nice!
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 19 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8205cd9c7d6f91663be53dae9b091ba302f96ab

commit f8205cd9c7d6f91663be53dae9b091ba302f96ab
Author: Daniele Castagna <dcastagna@chromium.org>
Date: Thu Apr 19 16:56:25 2018

exo: Remove format list for overlay candidates

We used to pick which buffer should be an overlay candidate based on
formats.

We currently only support HW overlays on devices using drm atomic,
where we test the buffer configurations before scanning out.

We can now set every buffer coming from exo as overlay candidate, and
the atomic test will pick the valid configuration.

Bug:  834015 
Test: android apps as overlays on eve
Change-Id: If5dc7836762b29648dfe9a888f3950b2fa7707ae
Reviewed-on: https://chromium-review.googlesource.com/1018423
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552051}
[modify] https://crrev.com/f8205cd9c7d6f91663be53dae9b091ba302f96ab/components/exo/display.cc
[modify] https://crrev.com/f8205cd9c7d6f91663be53dae9b091ba302f96ab/components/exo/display.h

Project Member

Comment 12 by bugdroid1@chromium.org, Apr 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f634aa2d0ca1cb9c08c46e4266d43c46a7c34147

commit f634aa2d0ca1cb9c08c46e4266d43c46a7c34147
Author: Daniele Castagna <dcastagna@chromium.org>
Date: Mon Apr 30 19:36:09 2018

ozone: Add overlay strategies with drm atomic

Add overlay strategies single-fullscreen and single-on-top
when the primary DRM device has atomic capabilities.

Bug:  834015 
Test: checked on kevin HW overlays are used.
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I06ac101311384baeb49eb31f5fab0d2550fb332e
Reviewed-on: https://chromium-review.googlesource.com/1022872
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: Kristian H. Kristensen <hoegsberg@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554849}
[modify] https://crrev.com/f634aa2d0ca1cb9c08c46e4266d43c46a7c34147/components/viz/service/display_embedder/compositor_overlay_candidate_validator_ozone.cc
[modify] https://crrev.com/f634aa2d0ca1cb9c08c46e4266d43c46a7c34147/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/f634aa2d0ca1cb9c08c46e4266d43c46a7c34147/ui/ozone/common/stub_overlay_manager.cc
[modify] https://crrev.com/f634aa2d0ca1cb9c08c46e4266d43c46a7c34147/ui/ozone/common/stub_overlay_manager.h
[modify] https://crrev.com/f634aa2d0ca1cb9c08c46e4266d43c46a7c34147/ui/ozone/platform/cast/overlay_manager_cast.cc
[modify] https://crrev.com/f634aa2d0ca1cb9c08c46e4266d43c46a7c34147/ui/ozone/platform/cast/overlay_manager_cast.h
[modify] https://crrev.com/f634aa2d0ca1cb9c08c46e4266d43c46a7c34147/ui/ozone/platform/drm/host/drm_device_handle.cc
[modify] https://crrev.com/f634aa2d0ca1cb9c08c46e4266d43c46a7c34147/ui/ozone/platform/drm/host/drm_device_handle.h
[modify] https://crrev.com/f634aa2d0ca1cb9c08c46e4266d43c46a7c34147/ui/ozone/platform/drm/host/drm_display_host_manager.cc
[modify] https://crrev.com/f634aa2d0ca1cb9c08c46e4266d43c46a7c34147/ui/ozone/platform/drm/host/drm_overlay_manager.cc
[modify] https://crrev.com/f634aa2d0ca1cb9c08c46e4266d43c46a7c34147/ui/ozone/platform/drm/host/drm_overlay_manager.h
[modify] https://crrev.com/f634aa2d0ca1cb9c08c46e4266d43c46a7c34147/ui/ozone/public/overlay_manager_ozone.h

Project Member

Comment 13 by bugdroid1@chromium.org, May 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a5da0e2cf7a5288065afc5184c5d247fd94dab71

commit a5da0e2cf7a5288065afc5184c5d247fd94dab71
Author: Daniele Castagna <dcastagna@chromium.org>
Date: Sat May 05 03:37:44 2018

libchromeos-ui: Remove drm atomic flags

We now detect on the DRM device if atomic capabilities are available
and change our overlay strategies accordingly.

BUG= chromium:834015 
TEST=emerge and deploy libchromeos-ui on kevin
Change-Id: I2b894d98df9316fb61090324c62bc40bd9c4caa3
Reviewed-on: https://chromium-review.googlesource.com/1022991
Commit-Ready: Daniele Castagna <dcastagna@chromium.org>
Tested-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/a5da0e2cf7a5288065afc5184c5d247fd94dab71/libchromeos-ui/chromeos/ui/chromium_command_builder.cc

Project Member

Comment 14 by bugdroid1@chromium.org, May 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fc6cbcd44438602c87e1022e7479eee305eec99e

commit fc6cbcd44438602c87e1022e7479eee305eec99e
Author: Daniele Castagna <dcastagna@chromium.org>
Date: Mon May 07 21:31:03 2018

Avoid creating overlay validator with no strategies.

In crrev.com/c/1022872 we changed the logic of how we initialize
CompositorOverlayCandidateValidatorOzone.

We erroneously changed the logic to always instantiate
CompositorOverlayCandidateValidatorOzone even when no strategies were
specified.

This broke legacy devices. This CL makes sure not to instantiate
the validator if not needed.

Bug:  840267 ,  834015 
Test: Play video on veyron_minnie
Change-Id: Ibde65689c05d38a3fe22d81748db8d21f89d725e
Reviewed-on: https://chromium-review.googlesource.com/1048245
Reviewed-by: Kristian H. Kristensen <hoegsberg@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556575}
[modify] https://crrev.com/fc6cbcd44438602c87e1022e7479eee305eec99e/content/browser/compositor/gpu_process_transport_factory.cc

Owner: dcasta...@chromium.org
Status: Started (was: Untriaged)
This can be closed right?
Status: Fixed (was: Started)

Sign in to add a comment