[Ozone-DRM] Cache CTM, DEGAMMA and GAMMA and apply them on commit
Cache the properties and apply them on commit on DRM Atomic. This works
around the need to waste a vsync interval to applying the atomic
properties.
BUG=839487
TEST=Ran unittests
Change-Id: I23a300d98648c5f7dff86019c8f8b9cb11129fb8
Reviewed-on: https://chromium-review.googlesource.com/1108561
Commit-Queue: Daniel Nicoara <dnicoara@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578356}diff --git a/ui/ozone/platform/drm/BUILD.gn b/ui/ozone/platform/drm/BUILD.gn
index 567ffa04..1550f531 100644
--- a/ui/ozone/platform/drm/BUILD.gn
+++ b/ui/ozone/platform/drm/BUILD.gn
@@ -8,6 +8,13 @@
visibility = [ "//ui/ozone/*" ]
+declare_args() {
+ # Enables commiting of CRTC properties on page flip events.
+ # This is not enabled by default because of a bug on Intel:
+ # https://crbug.com/854753
+ drm_commit_properties_on_page_flip = false
+}
+
source_set("gbm") {
sources = [
"client_native_pixmap_factory_gbm.cc",
@@ -161,6 +168,10 @@
public_configs = [ "//third_party/khronos:khronos_headers" ]
defines = [ "OZONE_IMPLEMENTATION" ]
+
+ if (drm_commit_properties_on_page_flip) {
+ defines += [ "COMMIT_PROPERTIES_ON_PAGE_FLIP" ]
+ }
}
source_set("gbm_unittests") {
@@ -194,4 +205,8 @@
"//ui/ozone/common",
"//ui/ozone/common/linux",
]
+
+ if (drm_commit_properties_on_page_flip) {
+ defines = [ "COMMIT_PROPERTIES_ON_PAGE_FLIP" ]
+ }
}
diff --git a/ui/ozone/platform/drm/gpu/drm_gpu_util.cc b/ui/ozone/platform/drm/gpu/drm_gpu_util.cc
index 1924b6e..518916c 100644
--- a/ui/ozone/platform/drm/gpu/drm_gpu_util.cc
+++ b/ui/ozone/platform/drm/gpu/drm_gpu_util.cc
@@ -30,6 +30,24 @@
return false;
}
+bool AddPropertyIfValid(drmModeAtomicReq* property_set,
+ uint32_t object_id,
+ const DrmDevice::Property& property) {
+ if (!property.id)
+ return true;
+
+ int ret = drmModeAtomicAddProperty(property_set, object_id, property.id,
+ property.value);
+ if (ret < 0) {
+ LOG(ERROR) << "Failed to set property object_id=" << object_id
+ << " property_id=" << property.id
+ << " property_value=" << property.value << " error=" << -ret;
+ return false;
+ }
+
+ return true;
+}
+
ScopedDrmColorLutPtr CreateLutBlob(
const std::vector<display::GammaRampRGBEntry>& source) {
TRACE_EVENT0("drm", "CreateLutBlob");
diff --git a/ui/ozone/platform/drm/gpu/drm_gpu_util.h b/ui/ozone/platform/drm/gpu/drm_gpu_util.h
index e138ef8..1972bef 100644
--- a/ui/ozone/platform/drm/gpu/drm_gpu_util.h
+++ b/ui/ozone/platform/drm/gpu/drm_gpu_util.h
@@ -16,6 +16,11 @@
const std::string& name,
DrmDevice::Property* property);
+// If the |property| has a valid ID add it to the |property_set| request.
+bool AddPropertyIfValid(drmModeAtomicReq* property_set,
+ uint32_t object_id,
+ const DrmDevice::Property& property);
+
// Transforms the gamma ramp entries into the drm_color_lut format.
ScopedDrmColorLutPtr CreateLutBlob(
const std::vector<display::GammaRampRGBEntry>& source);
diff --git a/ui/ozone/platform/drm/gpu/hardware_display_plane_atomic.cc b/ui/ozone/platform/drm/gpu/hardware_display_plane_atomic.cc
index b1ab4c1..0a041d8 100644
--- a/ui/ozone/platform/drm/gpu/hardware_display_plane_atomic.cc
+++ b/ui/ozone/platform/drm/gpu/hardware_display_plane_atomic.cc
@@ -5,6 +5,7 @@
#include "ui/ozone/platform/drm/gpu/hardware_display_plane_atomic.h"
#include "ui/ozone/platform/drm/gpu/drm_device.h"
+#include "ui/ozone/platform/drm/gpu/drm_gpu_util.h"
namespace ui {
namespace {
@@ -41,21 +42,6 @@
return 0;
}
-bool AddProperty(drmModeAtomicReqPtr property_set,
- uint32_t object_id,
- const DrmDevice::Property& property) {
- int ret = drmModeAtomicAddProperty(property_set, object_id, property.id,
- property.value);
- if (ret < 0) {
- LOG(ERROR) << "Failed to set property object_id=" << object_id
- << " property_id=" << property.id
- << " property_value=" << property.value << " error=" << -ret;
- return false;
- }
-
- return true;
-}
-
} // namespace
HardwareDisplayPlaneAtomic::HardwareDisplayPlaneAtomic(uint32_t id)
@@ -101,29 +87,30 @@
properties_.src_h.value = src_rect.height();
bool plane_set_succeeded =
- AddProperty(property_set, id_, properties_.crtc_id) &&
- AddProperty(property_set, id_, properties_.crtc_x) &&
- AddProperty(property_set, id_, properties_.crtc_y) &&
- AddProperty(property_set, id_, properties_.crtc_w) &&
- AddProperty(property_set, id_, properties_.crtc_h) &&
- AddProperty(property_set, id_, properties_.fb_id) &&
- AddProperty(property_set, id_, properties_.src_x) &&
- AddProperty(property_set, id_, properties_.src_y) &&
- AddProperty(property_set, id_, properties_.src_w) &&
- AddProperty(property_set, id_, properties_.src_h);
+ AddPropertyIfValid(property_set, id_, properties_.crtc_id) &&
+ AddPropertyIfValid(property_set, id_, properties_.crtc_x) &&
+ AddPropertyIfValid(property_set, id_, properties_.crtc_y) &&
+ AddPropertyIfValid(property_set, id_, properties_.crtc_w) &&
+ AddPropertyIfValid(property_set, id_, properties_.crtc_h) &&
+ AddPropertyIfValid(property_set, id_, properties_.fb_id) &&
+ AddPropertyIfValid(property_set, id_, properties_.src_x) &&
+ AddPropertyIfValid(property_set, id_, properties_.src_y) &&
+ AddPropertyIfValid(property_set, id_, properties_.src_w) &&
+ AddPropertyIfValid(property_set, id_, properties_.src_h);
if (properties_.rotation.id) {
properties_.rotation.value =
OverlayTransformToDrmRotationPropertyValue(transform);
- plane_set_succeeded = plane_set_succeeded &&
- AddProperty(property_set, id_, properties_.rotation);
+ plane_set_succeeded =
+ plane_set_succeeded &&
+ AddPropertyIfValid(property_set, id_, properties_.rotation);
}
if (properties_.in_fence_fd.id && in_fence_fd >= 0) {
properties_.in_fence_fd.value = in_fence_fd;
plane_set_succeeded =
plane_set_succeeded &&
- AddProperty(property_set, id_, properties_.in_fence_fd);
+ AddPropertyIfValid(property_set, id_, properties_.in_fence_fd);
}
if (!plane_set_succeeded) {
@@ -140,7 +127,7 @@
return false;
properties_.plane_ctm.value = ctm_blob_id;
- return AddProperty(property_set, id_, properties_.plane_ctm);
+ return AddPropertyIfValid(property_set, id_, properties_.plane_ctm);
}
} // namespace ui
diff --git a/ui/ozone/platform/drm/gpu/hardware_display_plane_manager.cc b/ui/ozone/platform/drm/gpu/hardware_display_plane_manager.cc
index 10da4f49..b33793726 100644
--- a/ui/ozone/platform/drm/gpu/hardware_display_plane_manager.cc
+++ b/ui/ozone/platform/drm/gpu/hardware_display_plane_manager.cc
@@ -43,6 +43,12 @@
HardwareDisplayPlaneList::PageFlipInfo::~PageFlipInfo() {
}
+HardwareDisplayPlaneManager::CrtcState::CrtcState() = default;
+
+HardwareDisplayPlaneManager::CrtcState::~CrtcState() = default;
+
+HardwareDisplayPlaneManager::CrtcState::CrtcState(CrtcState&&) = default;
+
HardwareDisplayPlaneManager::HardwareDisplayPlaneManager() : drm_(nullptr) {
}
@@ -59,7 +65,7 @@
drm_->SetCapability(DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
#endif
- if (!InitializeCrtcProperties(drm))
+ if (!InitializeCrtcState(drm))
return false;
if (!InitializePlanes(drm))
@@ -95,8 +101,8 @@
}
int HardwareDisplayPlaneManager::LookupCrtcIndex(uint32_t crtc_id) const {
- for (size_t i = 0; i < crtc_properties_.size(); ++i)
- if (crtc_properties_[i].id == crtc_id)
+ for (size_t i = 0; i < crtc_state_.size(); ++i)
+ if (crtc_state_[i].properties.id == crtc_id)
return i;
return -1;
}
@@ -234,16 +240,16 @@
const int crtc_index = LookupCrtcIndex(crtc_id);
DCHECK_GE(crtc_index, 0);
- CrtcProperties* crtc_props = &crtc_properties_[crtc_index];
+ CrtcState* crtc_state = &crtc_state_[crtc_index];
ScopedDrmColorCtmPtr ctm_blob_data = CreateCTMBlob(color_matrix);
- if (!crtc_props->ctm.id)
+ if (!crtc_state->properties.ctm.id)
return SetColorCorrectionOnAllCrtcPlanes(crtc_id, std::move(ctm_blob_data));
- ScopedDrmPropertyBlob ctm_prop =
+ crtc_state->ctm_blob =
drm_->CreatePropertyBlob(ctm_blob_data.get(), sizeof(drm_color_ctm));
- crtc_props->ctm.value = ctm_prop->id();
- return CommitColorMatrix(*crtc_props);
+ crtc_state->properties.ctm.value = crtc_state->ctm_blob->id();
+ return CommitColorMatrix(crtc_state->properties);
}
bool HardwareDisplayPlaneManager::SetGammaCorrection(
@@ -256,7 +262,8 @@
return false;
}
- CrtcProperties* crtc_props = &crtc_properties_[crtc_index];
+ CrtcState* crtc_state = &crtc_state_[crtc_index];
+ CrtcProperties* crtc_props = &crtc_state->properties;
if (!degamma_lut.empty() &&
(!crtc_props->degamma_lut.id || !crtc_props->degamma_lut_size.id))
@@ -279,21 +286,20 @@
ScopedDrmColorLutPtr gamma_blob_data =
CreateLutBlob(ResampleLut(gamma_lut, crtc_props->gamma_lut_size.value));
- ScopedDrmPropertyBlob degamma_prop, gamma_prop;
if (degamma_blob_data) {
- degamma_prop = drm_->CreatePropertyBlob(
+ crtc_state->degamma_lut_blob = drm_->CreatePropertyBlob(
degamma_blob_data.get(),
sizeof(drm_color_lut) * crtc_props->degamma_lut_size.value);
- crtc_props->degamma_lut.value = degamma_prop->id();
+ crtc_props->degamma_lut.value = crtc_state->degamma_lut_blob->id();
} else {
crtc_props->degamma_lut.value = 0;
}
if (gamma_blob_data) {
- gamma_prop = drm_->CreatePropertyBlob(
+ crtc_state->gamma_lut_blob = drm_->CreatePropertyBlob(
gamma_blob_data.get(),
sizeof(drm_color_lut) * crtc_props->gamma_lut_size.value);
- crtc_props->gamma_lut.value = gamma_prop->id();
+ crtc_props->gamma_lut.value = crtc_state->gamma_lut_blob->id();
} else {
crtc_props->gamma_lut.value = 0;
}
@@ -301,7 +307,7 @@
return CommitGammaCorrection(*crtc_props);
}
-bool HardwareDisplayPlaneManager::InitializeCrtcProperties(DrmDevice* drm) {
+bool HardwareDisplayPlaneManager::InitializeCrtcState(DrmDevice* drm) {
ScopedDrmResourcesPtr resources(drm->GetResources());
if (!resources) {
PLOG(ERROR) << "Failed to get resources.";
@@ -311,30 +317,34 @@
unsigned int num_crtcs_with_out_fence_ptr = 0;
for (int i = 0; i < resources->count_crtcs; ++i) {
- CrtcProperties p{};
- p.id = resources->crtcs[i];
+ CrtcState state;
+ state.properties.id = resources->crtcs[i];
ScopedDrmObjectPropertyPtr props(
drm->GetObjectProperties(resources->crtcs[i], DRM_MODE_OBJECT_CRTC));
if (!props) {
- PLOG(ERROR) << "Failed to get CRTC properties for crtc_id=" << p.id;
+ PLOG(ERROR) << "Failed to get CRTC properties for crtc_id="
+ << state.properties.id;
continue;
}
// These properties are optional. If they don't exist we can tell by the
// invalid ID.
- GetDrmPropertyForName(drm, props.get(), "CTM", &p.ctm);
- GetDrmPropertyForName(drm, props.get(), "GAMMA_LUT", &p.gamma_lut);
+ GetDrmPropertyForName(drm, props.get(), "CTM", &state.properties.ctm);
+ GetDrmPropertyForName(drm, props.get(), "GAMMA_LUT",
+ &state.properties.gamma_lut);
GetDrmPropertyForName(drm, props.get(), "GAMMA_LUT_SIZE",
- &p.gamma_lut_size);
- GetDrmPropertyForName(drm, props.get(), "DEGAMMA_LUT", &p.degamma_lut);
+ &state.properties.gamma_lut_size);
+ GetDrmPropertyForName(drm, props.get(), "DEGAMMA_LUT",
+ &state.properties.degamma_lut);
GetDrmPropertyForName(drm, props.get(), "DEGAMMA_LUT_SIZE",
- &p.degamma_lut_size);
- GetDrmPropertyForName(drm, props.get(), "OUT_FENCE_PTR", &p.out_fence_ptr);
+ &state.properties.degamma_lut_size);
+ GetDrmPropertyForName(drm, props.get(), "OUT_FENCE_PTR",
+ &state.properties.out_fence_ptr);
- num_crtcs_with_out_fence_ptr += (p.out_fence_ptr.id != 0);
+ num_crtcs_with_out_fence_ptr += (state.properties.out_fence_ptr.id != 0);
- crtc_properties_.push_back(p);
+ crtc_state_.emplace_back(std::move(state));
}
// Check that either all or none of the crtcs support the OUT_FENCE_PTR
@@ -342,7 +352,7 @@
// out-fence set when we perform a commit involving the problematic
// crtcs.
if (num_crtcs_with_out_fence_ptr != 0 &&
- num_crtcs_with_out_fence_ptr != crtc_properties_.size()) {
+ num_crtcs_with_out_fence_ptr != crtc_state_.size()) {
LOG(ERROR) << "Only some of the crtcs support the OUT_FENCE_PTR property";
return false;
}
diff --git a/ui/ozone/platform/drm/gpu/hardware_display_plane_manager.h b/ui/ozone/platform/drm/gpu/hardware_display_plane_manager.h
index ec54ede..e60ddb68 100644
--- a/ui/ozone/platform/drm/gpu/hardware_display_plane_manager.h
+++ b/ui/ozone/platform/drm/gpu/hardware_display_plane_manager.h
@@ -143,7 +143,23 @@
DrmDevice::Property out_fence_ptr;
};
- bool InitializeCrtcProperties(DrmDevice* drm);
+ struct CrtcState {
+ CrtcState();
+ ~CrtcState();
+ CrtcState(CrtcState&&);
+
+ CrtcProperties properties = {};
+
+ // Cached blobs for the properties since the CRTC properties are applied on
+ // the next page flip and we need to keep the properties valid until then.
+ ScopedDrmPropertyBlob ctm_blob;
+ ScopedDrmPropertyBlob gamma_lut_blob;
+ ScopedDrmPropertyBlob degamma_lut_blob;
+
+ DISALLOW_COPY_AND_ASSIGN(CrtcState);
+ };
+
+ bool InitializeCrtcState(DrmDevice* drm);
virtual bool InitializePlanes(DrmDevice* drm) = 0;
@@ -188,7 +204,7 @@
bool has_universal_planes_ = false;
std::vector<std::unique_ptr<HardwareDisplayPlane>> planes_;
- std::vector<CrtcProperties> crtc_properties_;
+ std::vector<CrtcState> crtc_state_;
std::vector<uint32_t> supported_formats_;
DISALLOW_COPY_AND_ASSIGN(HardwareDisplayPlaneManager);
diff --git a/ui/ozone/platform/drm/gpu/hardware_display_plane_manager_atomic.cc b/ui/ozone/platform/drm/gpu/hardware_display_plane_manager_atomic.cc
index 3852840..ad365ad 100644
--- a/ui/ozone/platform/drm/gpu/hardware_display_plane_manager_atomic.cc
+++ b/ui/ozone/platform/drm/gpu/hardware_display_plane_manager_atomic.cc
@@ -17,6 +17,7 @@
#include "ui/ozone/platform/drm/common/drm_util.h"
#include "ui/ozone/platform/drm/gpu/crtc_controller.h"
#include "ui/ozone/platform/drm/gpu/drm_device.h"
+#include "ui/ozone/platform/drm/gpu/drm_gpu_util.h"
#include "ui/ozone/platform/drm/gpu/hardware_display_plane_atomic.h"
#include "ui/ozone/platform/drm/gpu/page_flip_request.h"
#include "ui/ozone/platform/drm/gpu/scanout_buffer.h"
@@ -81,6 +82,23 @@
crtcs.push_back(atomic_plane->crtc());
}
+#if defined(COMMIT_PROPERTIES_ON_PAGE_FLIP)
+ drmModeAtomicReqPtr request = plane_list->atomic_property_set.get();
+
+ // Apply all CRTC properties in the page-flip so we don't block the swap chain
+ // for a vsync.
+ // TODO(dnicoara): See if we can apply these properties async using
+ // DRM_MODE_ATOMIC_ASYNC_UPDATE flag when committing.
+ for (auto* const crtc : crtcs) {
+ int idx = LookupCrtcIndex(crtc->crtc());
+ AddPropertyIfValid(request, crtc->crtc(),
+ crtc_state_[idx].properties.degamma_lut);
+ AddPropertyIfValid(request, crtc->crtc(),
+ crtc_state_[idx].properties.gamma_lut);
+ AddPropertyIfValid(request, crtc->crtc(), crtc_state_[idx].properties.ctm);
+ }
+#endif
+
if (test_only) {
for (HardwareDisplayPlane* plane : plane_list->plane_list) {
plane->set_in_use(false);
@@ -254,6 +272,8 @@
bool HardwareDisplayPlaneManagerAtomic::CommitColorMatrix(
const CrtcProperties& crtc_props) {
+ DCHECK(crtc_props.ctm.id);
+#if !defined(COMMIT_PROPERTIES_ON_PAGE_FLIP)
ScopedDrmAtomicReqPtr property_set(drmModeAtomicAlloc());
int ret = drmModeAtomicAddProperty(property_set.get(), crtc_props.id,
crtc_props.ctm.id, crtc_props.ctm.value);
@@ -268,12 +288,15 @@
// TODO(dnicoara): Should cache these values locally and aggregate them with
// the page flip event otherwise this "steals" a vsync to apply the property.
return drm_->CommitProperties(property_set.get(), 0, 0, nullptr);
+#else
+ return true;
+#endif
}
bool HardwareDisplayPlaneManagerAtomic::CommitGammaCorrection(
const CrtcProperties& crtc_props) {
DCHECK(crtc_props.degamma_lut.id || crtc_props.gamma_lut.id);
-
+#if !defined(COMMIT_PROPERTIES_ON_PAGE_FLIP)
ScopedDrmAtomicReqPtr property_set(drmModeAtomicAlloc());
if (crtc_props.degamma_lut.id) {
int ret = drmModeAtomicAddProperty(property_set.get(), crtc_props.id,
@@ -303,6 +326,9 @@
// TODO(dnicoara): Should cache these values locally and aggregate them with
// the page flip event otherwise this "steals" a vsync to apply the property.
return drm_->CommitProperties(property_set.get(), 0, 0, nullptr);
+#else
+ return true;
+#endif
}
bool HardwareDisplayPlaneManagerAtomic::AddOutFencePtrProperties(
@@ -317,7 +343,8 @@
for (auto* crtc : crtcs) {
const auto crtc_index = LookupCrtcIndex(crtc->crtc());
DCHECK_GE(crtc_index, 0);
- const auto out_fence_ptr_id = crtc_properties_[crtc_index].out_fence_ptr.id;
+ const auto out_fence_ptr_id =
+ crtc_state_[crtc_index].properties.out_fence_ptr.id;
if (out_fence_ptr_id > 0) {
out_fence_fds->push_back(base::ScopedFD());
diff --git a/ui/ozone/platform/drm/gpu/hardware_display_plane_manager_legacy.cc b/ui/ozone/platform/drm/gpu/hardware_display_plane_manager_legacy.cc
index 5871d9a..1eb3c57 100644
--- a/ui/ozone/platform/drm/gpu/hardware_display_plane_manager_legacy.cc
+++ b/ui/ozone/platform/drm/gpu/hardware_display_plane_manager_legacy.cc
@@ -150,8 +150,8 @@
// https://crbug.com/464085: if driver reports no primary planes for a crtc,
// create a dummy plane for which we can assign exactly one overlay.
if (!has_universal_planes_) {
- for (size_t i = 0; i < crtc_properties_.size(); ++i) {
- uint32_t id = crtc_properties_[i].id - 1;
+ for (size_t i = 0; i < crtc_state_.size(); ++i) {
+ uint32_t id = crtc_state_[i].properties.id - 1;
if (std::find_if(
planes_.begin(), planes_.end(),
[id](const std::unique_ptr<HardwareDisplayPlane>& plane) {
diff --git a/ui/ozone/platform/drm/gpu/hardware_display_plane_manager_unittest.cc b/ui/ozone/platform/drm/gpu/hardware_display_plane_manager_unittest.cc
index 52517e0..4d14e58 100644
--- a/ui/ozone/platform/drm/gpu/hardware_display_plane_manager_unittest.cc
+++ b/ui/ozone/platform/drm/gpu/hardware_display_plane_manager_unittest.cc
@@ -48,9 +48,17 @@
HardwareDisplayPlaneManagerTest() {}
void InitializeDrmState(size_t crtc_count, size_t planes_per_crtc);
+
+ uint64_t GetObjectPropertyValue(uint32_t object_id,
+ uint32_t object_type,
+ const std::string& property_name);
+ uint64_t GetCrtcPropertyValue(uint32_t crtc,
+ const std::string& property_name);
uint64_t GetPlanePropertyValue(uint32_t plane,
const std::string& property_name);
+ void PerformPageFlip(size_t crtc_idx, ui::HardwareDisplayPlaneList* state);
+
void SetUp() override;
protected:
@@ -141,17 +149,47 @@
property_names_.insert({kOutFencePtrPropId, "OUT_FENCE_PTR"});
}
-uint64_t HardwareDisplayPlaneManagerTest::GetPlanePropertyValue(
- uint32_t plane,
+void HardwareDisplayPlaneManagerTest::PerformPageFlip(
+ size_t crtc_idx,
+ ui::HardwareDisplayPlaneList* state) {
+ ui::DrmOverlayPlaneList assigns;
+ ui::CrtcController crtc(fake_drm_, crtc_properties_[crtc_idx].id, 0);
+ scoped_refptr<ui::MockScanoutBuffer> xrgb_buffer =
+ new ui::MockScanoutBuffer(kDefaultBufferSize);
+ assigns.push_back(ui::DrmOverlayPlane(xrgb_buffer, nullptr));
+ fake_drm_->plane_manager()->BeginFrame(state);
+ ASSERT_TRUE(fake_drm_->plane_manager()->AssignOverlayPlanes(
+ state, assigns, crtc_properties_[crtc_idx].id, &crtc));
+ scoped_refptr<ui::PageFlipRequest> page_flip_request =
+ base::MakeRefCounted<ui::PageFlipRequest>(base::TimeDelta());
+ ASSERT_TRUE(
+ fake_drm_->plane_manager()->Commit(state, page_flip_request, nullptr));
+}
+
+uint64_t HardwareDisplayPlaneManagerTest::GetObjectPropertyValue(
+ uint32_t object_id,
+ uint32_t object_type,
const std::string& property_name) {
ui::DrmDevice::Property p{};
ui::ScopedDrmObjectPropertyPtr properties(
- fake_drm_->GetObjectProperties(plane, DRM_MODE_OBJECT_PLANE));
+ fake_drm_->GetObjectProperties(object_id, object_type));
EXPECT_TRUE(ui::GetDrmPropertyForName(fake_drm_.get(), properties.get(),
property_name, &p));
return p.value;
}
+uint64_t HardwareDisplayPlaneManagerTest::GetCrtcPropertyValue(
+ uint32_t crtc,
+ const std::string& property_name) {
+ return GetObjectPropertyValue(crtc, DRM_MODE_OBJECT_CRTC, property_name);
+}
+
+uint64_t HardwareDisplayPlaneManagerTest::GetPlanePropertyValue(
+ uint32_t plane,
+ const std::string& property_name) {
+ return GetObjectPropertyValue(plane, DRM_MODE_OBJECT_PLANE, property_name);
+}
+
using HardwareDisplayPlaneManagerLegacyTest = HardwareDisplayPlaneManagerTest;
using HardwareDisplayPlaneManagerAtomicTest = HardwareDisplayPlaneManagerTest;
@@ -450,10 +488,18 @@
EXPECT_TRUE(fake_drm_->plane_manager()->SetColorMatrix(
crtc_properties_[0].id, std::vector<float>(9)));
- if (use_atomic_)
+ if (use_atomic_) {
+ ui::HardwareDisplayPlaneList state;
+ PerformPageFlip(/*crtc_idx=*/0, &state);
+#if defined(COMMIT_PROPERTIES_ON_PAGE_FLIP)
EXPECT_EQ(1, fake_drm_->get_commit_count());
- else
+#else
+ EXPECT_EQ(2, fake_drm_->get_commit_count());
+#endif
+ EXPECT_NE(0u, GetCrtcPropertyValue(crtc_properties_[0].id, "CTM"));
+ } else {
EXPECT_EQ(1, fake_drm_->get_set_object_property_count());
+ }
}
TEST_P(HardwareDisplayPlaneManagerTest, SetColorMatrix_ErrorEmptyCtm) {
@@ -464,10 +510,14 @@
EXPECT_FALSE(
fake_drm_->plane_manager()->SetColorMatrix(crtc_properties_[0].id, {}));
- if (use_atomic_)
- EXPECT_EQ(0, fake_drm_->get_commit_count());
- else
+ if (use_atomic_) {
+ ui::HardwareDisplayPlaneList state;
+ PerformPageFlip(/*crtc_idx=*/0, &state);
+ EXPECT_EQ(1, fake_drm_->get_commit_count());
+ EXPECT_EQ(0u, GetCrtcPropertyValue(crtc_properties_[0].id, "CTM"));
+ } else {
EXPECT_EQ(0, fake_drm_->get_set_object_property_count());
+ }
}
TEST_P(HardwareDisplayPlaneManagerTest, SetGammaCorrection_MissingDegamma) {
@@ -478,10 +528,14 @@
EXPECT_FALSE(fake_drm_->plane_manager()->SetGammaCorrection(
crtc_properties_[0].id, {{0, 0, 0}}, {}));
- if (use_atomic_)
- EXPECT_EQ(0, fake_drm_->get_commit_count());
- else
+ if (use_atomic_) {
+ ui::HardwareDisplayPlaneList state;
+ PerformPageFlip(/*crtc_idx=*/0, &state);
+ // Page flip should succeed even if the properties failed to be updated.
+ EXPECT_EQ(1, fake_drm_->get_commit_count());
+ } else {
EXPECT_EQ(0, fake_drm_->get_set_object_property_count());
+ }
crtc_properties_[0].properties.push_back(
{.id = kDegammaLutSizePropId, .value = 1});
@@ -490,10 +544,14 @@
EXPECT_FALSE(fake_drm_->plane_manager()->SetGammaCorrection(
crtc_properties_[0].id, {{0, 0, 0}}, {}));
- if (use_atomic_)
- EXPECT_EQ(0, fake_drm_->get_commit_count());
- else
+ if (use_atomic_) {
+ ui::HardwareDisplayPlaneList state;
+ PerformPageFlip(/*crtc_idx=*/0, &state);
+ // Page flip should succeed even if the properties failed to be updated.
+ EXPECT_EQ(2, fake_drm_->get_commit_count());
+ } else {
EXPECT_EQ(0, fake_drm_->get_set_object_property_count());
+ }
}
TEST_P(HardwareDisplayPlaneManagerTest, SetGammaCorrection_MissingGamma) {
@@ -503,11 +561,15 @@
property_names_, use_atomic_);
EXPECT_FALSE(fake_drm_->plane_manager()->SetGammaCorrection(
- crtc_properties_[0].id, {{0, 0, 0}}, {}));
- if (use_atomic_)
- EXPECT_EQ(0, fake_drm_->get_commit_count());
- else
+ crtc_properties_[0].id, {}, {{0, 0, 0}}));
+ if (use_atomic_) {
+ ui::HardwareDisplayPlaneList state;
+ PerformPageFlip(/*crtc_idx=*/0, &state);
+ // Page flip should succeed even if the properties failed to be updated.
+ EXPECT_EQ(1, fake_drm_->get_commit_count());
+ } else {
EXPECT_EQ(0, fake_drm_->get_set_object_property_count());
+ }
crtc_properties_[0].properties.push_back(
{.id = kGammaLutSizePropId, .value = 1});
@@ -516,17 +578,21 @@
EXPECT_FALSE(fake_drm_->plane_manager()->SetGammaCorrection(
crtc_properties_[0].id, {}, {{0, 0, 0}}));
- if (use_atomic_)
- EXPECT_EQ(0, fake_drm_->get_commit_count());
- else
+ if (use_atomic_) {
+ ui::HardwareDisplayPlaneList state;
+ PerformPageFlip(/*crtc_idx=*/0, &state);
+ // Page flip should succeed even if the properties failed to be updated.
+ EXPECT_EQ(2, fake_drm_->get_commit_count());
+ } else {
EXPECT_EQ(0, fake_drm_->get_set_object_property_count());
+ }
fake_drm_->set_legacy_gamma_ramp_expectation(true);
EXPECT_TRUE(fake_drm_->plane_manager()->SetGammaCorrection(
crtc_properties_[0].id, {}, {{0, 0, 0}}));
// Going through the legacy API, so we shouldn't commit anything.
if (use_atomic_)
- EXPECT_EQ(0, fake_drm_->get_commit_count());
+ EXPECT_EQ(2, fake_drm_->get_commit_count());
else
EXPECT_EQ(0, fake_drm_->get_set_object_property_count());
}
@@ -551,20 +617,37 @@
fake_drm_->InitializeState(crtc_properties_, plane_properties_,
property_names_, use_atomic_);
+ ui::HardwareDisplayPlaneList state;
// Check that we reset the properties correctly.
EXPECT_TRUE(fake_drm_->plane_manager()->SetGammaCorrection(
crtc_properties_[0].id, {}, {}));
- if (use_atomic_)
+ if (use_atomic_) {
+ PerformPageFlip(/*crtc_idx=*/0, &state);
+#if defined(COMMIT_PROPERTIES_ON_PAGE_FLIP)
EXPECT_EQ(1, fake_drm_->get_commit_count());
- else
+#else
+ EXPECT_EQ(2, fake_drm_->get_commit_count());
+#endif
+ EXPECT_EQ(0u, GetCrtcPropertyValue(crtc_properties_[0].id, "GAMMA_LUT"));
+ EXPECT_EQ(0u, GetCrtcPropertyValue(crtc_properties_[0].id, "DEGAMMA_LUT"));
+ } else {
EXPECT_EQ(2, fake_drm_->get_set_object_property_count());
+ }
EXPECT_TRUE(fake_drm_->plane_manager()->SetGammaCorrection(
crtc_properties_[0].id, {{0, 0, 0}}, {{0, 0, 0}}));
- if (use_atomic_)
+ if (use_atomic_) {
+ PerformPageFlip(/*crtc_idx=*/0, &state);
+#if defined(COMMIT_PROPERTIES_ON_PAGE_FLIP)
EXPECT_EQ(2, fake_drm_->get_commit_count());
- else
+#else
+ EXPECT_EQ(4, fake_drm_->get_commit_count());
+#endif
+ EXPECT_NE(0u, GetCrtcPropertyValue(crtc_properties_[0].id, "GAMMA_LUT"));
+ EXPECT_NE(0u, GetCrtcPropertyValue(crtc_properties_[0].id, "DEGAMMA_LUT"));
+ } else {
EXPECT_EQ(4, fake_drm_->get_set_object_property_count());
+ }
}
TEST_P(HardwareDisplayPlaneManagerAtomicTest,
diff --git a/ui/ozone/platform/drm/gpu/mock_drm_device.cc b/ui/ozone/platform/drm/gpu/mock_drm_device.cc
index 7e1d70a..7b0845a 100644
--- a/ui/ozone/platform/drm/gpu/mock_drm_device.cc
+++ b/ui/ozone/platform/drm/gpu/mock_drm_device.cc
@@ -404,10 +404,12 @@
request->items[i].value));
}
- if (!page_flip_request)
+ if (page_flip_request)
+ callbacks_.push(page_flip_request->AddPageFlip());
+
+ if (flags & DRM_MODE_ATOMIC_TEST_ONLY)
return true;
- callbacks_.push(page_flip_request->AddPageFlip());
// Only update values if not testing.
for (uint32_t i = 0; i < request->cursor; ++i) {
EXPECT_TRUE(UpdateProperty(request->items[i].object_id,
