From 088fed0a54a6b338a04b49642a0e382f8687169c Mon Sep 17 00:00:00 2001 From: Andrii Chepurnyi Date: Mon, 3 Sep 2018 14:19:40 +0300 Subject: Fix actual clang-format version in readme Current .clang-format requires clang-format 5.0 Signed-off-by: Andrii Chepurnyi --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 260ac9b..05f1364 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ A short list of contribution guidelines: * drm_hwcomposer is Apache 2.0 Licensed and we require contributions to follow the developer's certificate of origin: http://developercertificate.org/ * When submitting new code please follow the naming conventions documented in the generated documentation. Also please make full use of all the helpers and convenience macros provided by drm_hwcomposer. The below command can help you with formatting of your patches: - `git diff | clang-format-diff-3.5 -p 1 -style=file` + `git diff | clang-format-diff-5.0 -p 1 -style=file` * Hardware specific changes should be tested on relevant platforms before committing. If you need inspiration, please checkout our [TODO issues](https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/issues?label_name%5B%5D=TODO) -- cgit v1.2.3 From 9b6cafd4e218556b56ccfa7b5e933a808c5353fd Mon Sep 17 00:00:00 2001 From: Lowry Li Date: Tue, 28 Aug 2018 17:58:21 +0800 Subject: drm_hwcomposer: Add support for pixel blend mode property The upstream version of pixel blend mode property will be added to the DRM core. So added support for pixel blend mode property to the DrmPlane. Created ValidatePlane() function in Planner to do the blend check, and also moved rotation and plane alpha property check there. Fixed the Emplace() call in platformhisi.cpp as was done with the other planner implementations. Change-Id: I7e6714699cf7c222a83de472060d4625e1e6945a Reviewed-by: Sean Paul Reviewed-by: Liviu Dudau Signed-off-by: Lowry Li Tested-by: John Stultz --- drmdisplaycompositor.cpp | 46 ++++++++++++++++++++++++++++++---------------- drmplane.cpp | 8 ++++++++ drmplane.h | 2 ++ drmproperty.cpp | 11 +++++++++++ drmproperty.h | 1 + platform.cpp | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- platform.h | 11 +++++++++-- platformhisi.cpp | 2 +- 8 files changed, 108 insertions(+), 21 deletions(-) diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index f479f10..cd6d4b2 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -316,6 +316,7 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, hwc_frect_t source_crop; uint64_t rotation = 0; uint64_t alpha = 0xFFFF; + uint64_t blend; if (comp_plane.type() != DrmCompositionPlane::Type::kDisable) { if (source_layers.size() > 1) { @@ -338,8 +339,25 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, fence_fd = layer.acquire_fence.get(); display_frame = layer.display_frame; source_crop = layer.source_crop; - if (layer.blending == DrmHwcBlending::kPreMult) - alpha = layer.alpha; + alpha = layer.alpha; + + if (plane->blend_property().id()) { + switch (layer.blending) { + case DrmHwcBlending::kPreMult: + std::tie(blend, ret) = plane->blend_property().GetEnumValueWithName( + "Pre-multiplied"); + break; + case DrmHwcBlending::kCoverage: + std::tie(blend, ret) = plane->blend_property().GetEnumValueWithName( + "Coverage"); + break; + case DrmHwcBlending::kNone: + default: + std::tie(blend, ret) = plane->blend_property().GetEnumValueWithName( + "None"); + break; + } + } rotation = 0; if (layer.transform & DrmHwcTransform::kFlipH) @@ -382,20 +400,6 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, continue; } - // TODO: Once we have atomic test, this should fall back to GL - if (rotation != DRM_MODE_ROTATE_0 && plane->rotation_property().id() == 0) { - ALOGV("Rotation is not supported on plane %d", plane->id()); - ret = -EINVAL; - break; - } - - // TODO: Once we have atomic test, this should fall back to GL - if (alpha != 0xFFFF && plane->alpha_property().id() == 0) { - ALOGV("Alpha is not supported on plane %d", plane->id()); - ret = -EINVAL; - break; - } - ret = drmModeAtomicAddProperty(pset, plane->id(), plane->crtc_property().id(), crtc->id()) < 0; ret |= drmModeAtomicAddProperty(pset, plane->id(), @@ -453,6 +457,16 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, break; } } + + if (plane->blend_property().id()) { + ret = drmModeAtomicAddProperty(pset, plane->id(), + plane->blend_property().id(), blend) < 0; + if (ret) { + ALOGE("Failed to add pixel blend mode property %d to plane %d", + plane->blend_property().id(), plane->id()); + break; + } + } } if (!ret) { diff --git a/drmplane.cpp b/drmplane.cpp index 2603e16..6747621 100644 --- a/drmplane.cpp +++ b/drmplane.cpp @@ -126,6 +126,10 @@ int DrmPlane::Init() { if (ret) ALOGI("Could not get alpha property"); + ret = drm_->GetPlaneProperty(*this, "pixel blend mode", &blend_property_); + if (ret) + ALOGI("Could not get pixel blend mode property"); + ret = drm_->GetPlaneProperty(*this, "IN_FENCE_FD", &in_fence_fd_property_); if (ret) ALOGI("Could not get IN_FENCE_FD property"); @@ -193,6 +197,10 @@ const DrmProperty &DrmPlane::alpha_property() const { return alpha_property_; } +const DrmProperty &DrmPlane::blend_property() const { + return blend_property_; +} + const DrmProperty &DrmPlane::in_fence_fd_property() const { return in_fence_fd_property_; } diff --git a/drmplane.h b/drmplane.h index 46dbc94..b7607ff 100644 --- a/drmplane.h +++ b/drmplane.h @@ -54,6 +54,7 @@ class DrmPlane { const DrmProperty &src_h_property() const; const DrmProperty &rotation_property() const; const DrmProperty &alpha_property() const; + const DrmProperty &blend_property() const; const DrmProperty &in_fence_fd_property() const; private: @@ -76,6 +77,7 @@ class DrmPlane { DrmProperty src_h_property_; DrmProperty rotation_property_; DrmProperty alpha_property_; + DrmProperty blend_property_; DrmProperty in_fence_fd_property_; }; } // namespace android diff --git a/drmproperty.cpp b/drmproperty.cpp index e71c159..dcab05e 100644 --- a/drmproperty.cpp +++ b/drmproperty.cpp @@ -99,4 +99,15 @@ int DrmProperty::value(uint64_t *value) const { return -EINVAL; } } + +std::tuple DrmProperty::GetEnumValueWithName( + std::string name) const { + for (auto it : enums_) { + if (it.name_.compare(name) == 0) { + return std::make_tuple(it.value_, 0); + } + } + + return std::make_tuple(UINT64_MAX, -EINVAL); +} } // namespace android diff --git a/drmproperty.h b/drmproperty.h index dc01c88..5e358be 100644 --- a/drmproperty.h +++ b/drmproperty.h @@ -40,6 +40,7 @@ class DrmProperty { DrmProperty &operator=(const DrmProperty &) = delete; void Init(drmModePropertyPtr p, uint64_t value); + std::tuple GetEnumValueWithName(std::string name) const; uint32_t id() const; std::string name() const; diff --git a/platform.cpp b/platform.cpp index af18124..9e0eb45 100644 --- a/platform.cpp +++ b/platform.cpp @@ -36,6 +36,50 @@ std::vector Planner::GetUsablePlanes( return usable_planes; } +int Planner::PlanStage::ValidatePlane(DrmPlane *plane, DrmHwcLayer *layer) { + int ret = 0; + uint64_t blend; + + if ((plane->rotation_property().id() == 0) && + layer->transform != DrmHwcTransform::kIdentity) { + ALOGE("Rotation is not supported on plane %d", plane->id()); + return -EINVAL; + } + + if (plane->alpha_property().id() == 0 && layer->alpha != 0xffff) { + ALOGE("Alpha is not supported on plane %d", plane->id()); + return -EINVAL; + } + + if (plane->blend_property().id() == 0) { + if ((layer->blending != DrmHwcBlending::kNone) && + (layer->blending != DrmHwcBlending::kPreMult)) { + ALOGE("Blending is not supported on plane %d", plane->id()); + return -EINVAL; + } + } else { + switch (layer->blending) { + case DrmHwcBlending::kPreMult: + std::tie(blend, ret) = plane->blend_property().GetEnumValueWithName( + "Pre-multiplied"); + break; + case DrmHwcBlending::kCoverage: + std::tie(blend, ret) = plane->blend_property().GetEnumValueWithName( + "Coverage"); + break; + case DrmHwcBlending::kNone: + default: + std::tie(blend, + ret) = plane->blend_property().GetEnumValueWithName("None"); + break; + } + if (ret) + ALOGE("Expected a valid blend mode on plane %d", plane->id()); + } + + return ret; +} + std::tuple> Planner::ProvisionPlanes( std::map &layers, DrmCrtc *crtc, std::vector *primary_planes, @@ -73,7 +117,7 @@ int PlanStageProtected::ProvisionPlanes( } ret = Emplace(composition, planes, DrmCompositionPlane::Type::kLayer, crtc, - i->first); + std::make_pair(i->first, i->second)); if (ret) ALOGE("Failed to dedicate protected layer! Dropping it."); @@ -91,7 +135,7 @@ int PlanStageGreedy::ProvisionPlanes( // Fill up the remaining planes for (auto i = layers.begin(); i != layers.end(); i = layers.erase(i)) { int ret = Emplace(composition, planes, DrmCompositionPlane::Type::kLayer, - crtc, i->first); + crtc, std::make_pair(i->first, i->second)); // We don't have any planes left if (ret == -ENOENT) break; diff --git a/platform.h b/platform.h index 37c4647..6c12fe9 100644 --- a/platform.h +++ b/platform.h @@ -73,16 +73,23 @@ class Planner { return plane; } + static int ValidatePlane(DrmPlane *plane, DrmHwcLayer *layer); + // Inserts the given layer:plane in the composition at the back static int Emplace(std::vector *composition, std::vector *planes, DrmCompositionPlane::Type type, DrmCrtc *crtc, - size_t source_layer) { + std::pair layer) { DrmPlane *plane = PopPlane(planes); + int ret; if (!plane) return -ENOENT; - composition->emplace_back(type, plane, crtc, source_layer); + ret = ValidatePlane(plane, layer.second); + if (ret) + return -EINVAL; + + composition->emplace_back(type, plane, crtc, layer.first); return 0; } }; diff --git a/platformhisi.cpp b/platformhisi.cpp index 68bb5a7..0e12ef1 100644 --- a/platformhisi.cpp +++ b/platformhisi.cpp @@ -153,7 +153,7 @@ class PlanStageHiSi : public Planner::PlanStage { continue; int ret = Emplace(composition, planes, DrmCompositionPlane::Type::kLayer, - crtc, i->first); + crtc, std::make_pair(i->first, i->second)); layers_added++; // We don't have any planes left if (ret == -ENOENT) -- cgit v1.2.3 From 592c98a0fe561320dd91dfbec6f40ccbc38c398c Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Tue, 4 Sep 2018 15:30:29 -0400 Subject: drm_hwcomposer: Pull CI check into dedicated script This will be useful for adding more functionality to it. It's not very practical to script inside the yml. Change-Id: I71aa6b40d282f750eb9bce65dd2cfd9e2828905b Signed-off-by: Sean Paul --- .gitlab-ci-checkcommit.sh | 9 +++++++++ .gitlab-ci.yml | 7 ++----- 2 files changed, 11 insertions(+), 5 deletions(-) create mode 100755 .gitlab-ci-checkcommit.sh diff --git a/.gitlab-ci-checkcommit.sh b/.gitlab-ci-checkcommit.sh new file mode 100755 index 0000000..9023410 --- /dev/null +++ b/.gitlab-ci-checkcommit.sh @@ -0,0 +1,9 @@ +#! /usr/bin/env bash + +git fetch https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer.git + +git diff -U0 --no-color FETCH_HEAD...HEAD -- | clang-format-diff-5.0 -p 1 -style=file > format-fixup.patch +if [ -s format-fixup.patch ]; then + cat format-fixup.patch + exit 1 +fi diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 24c4a0a..d0175f0 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -7,12 +7,9 @@ before_script: stages: - style -clang-format: +checkstyle: stage: style - script: - - git fetch https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer.git - - git diff -U0 --no-color FETCH_HEAD...HEAD -- | clang-format-diff-5.0 -p 1 -style=file > format-fixup.patch - - if [ -s format-fixup.patch ]; then cat format-fixup.patch && exit 1; fi + script: "./.gitlab-ci-checkcommit.sh" artifacts: when: on_failure paths: -- cgit v1.2.3 From 68a78ef6bd8b8ddf710e9a0654fa47cdc1f158d3 Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Tue, 4 Sep 2018 15:32:26 -0400 Subject: drm_hwcomposer: Add commit message validation to CI Check the subject prefix starts with "drm_hwcomposer: " and we have Signed-off-by tags for both author and committer. Change-Id: Ib26b8e5cbeae2156014f2bbfb8703545bdc1decb Signed-off-by: Sean Paul --- .gitlab-ci-checkcommit.sh | 38 +++++++++++++++++++++++++++++++++----- .gitlab-ci.yml | 3 +-- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci-checkcommit.sh b/.gitlab-ci-checkcommit.sh index 9023410..fc8963a 100755 --- a/.gitlab-ci-checkcommit.sh +++ b/.gitlab-ci-checkcommit.sh @@ -1,9 +1,37 @@ #! /usr/bin/env bash +echoerr() { + printf "ERROR: %s\n" "$*" >&2 +} + git fetch https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer.git -git diff -U0 --no-color FETCH_HEAD...HEAD -- | clang-format-diff-5.0 -p 1 -style=file > format-fixup.patch -if [ -s format-fixup.patch ]; then - cat format-fixup.patch - exit 1 -fi +git log --pretty='%h' FETCH_HEAD..HEAD | while read h; do + subject=$(git show -s --pretty='%s' "$h") + if [[ $subject != drm_hwcomposer:* ]]; then + echoerr "Invalid subject prefix: $subject" + exit 1 + fi + + commit_body=$(git show -s --pretty=%b "$h") + + author=$(git show -s --format='%an <%ae>') + sob=$(echo "$commit_body" | grep "Signed-off-by: $author") + if [ -z "$sob" ] ; then + echoerr "Author SoB tag is missing from commit $h" + exit 1 + fi + + committer=$(git show -s --format='%cn <%ce>') + sob=$(echo "$commit_body" | grep "Signed-off-by: $committer") + if [ -z "$sob" ] ; then + echoerr "Committer SoB tag is missing from commit $h" + exit 1 + fi + + git show "$h" -- | clang-format-diff-5.0 -p 1 -style=file > format-fixup.patch + if [ -s format-fixup.patch ]; then + cat format-fixup.patch >&2 + exit 1 + fi +done diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d0175f0..c97f4ff 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -12,5 +12,4 @@ checkstyle: script: "./.gitlab-ci-checkcommit.sh" artifacts: when: on_failure - paths: - - format-fixup.patch + untracked: true -- cgit v1.2.3 From 2234d3770463adca2be1a8bd9496c28e50bb3dd1 Mon Sep 17 00:00:00 2001 From: Alexandru Gheorghe Date: Tue, 9 Oct 2018 16:25:28 +0100 Subject: drm_hwcomposer: Propagate PlanStage error All existing stage planers(PlanStageGreedy, PlanStageProtected, PlanStageHiSi) silently ignore if they couldn't allocate a drmplane for a drmhwclayer and return success. That doesn't go well with the assumptions from ValidateDisplay, where if the atomic check succeeds we just assume that we could do Device composition for a maximum of num_of_drm_planes layers. But, since we silently dropped some drmhwclayer we never put those in the atomic check and we won't put it in the final atomic commit, so we end up with a wrong composition on the screen. What this proposes is propagating the error, which will make ValidateDisplay to fallback on client composition. Signed-off-by: Alexandru Gheorghe --- platform.cpp | 8 ++++++-- platformhisi.cpp | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/platform.cpp b/platform.cpp index 9e0eb45..b7a47c7 100644 --- a/platform.cpp +++ b/platform.cpp @@ -118,8 +118,10 @@ int PlanStageProtected::ProvisionPlanes( ret = Emplace(composition, planes, DrmCompositionPlane::Type::kLayer, crtc, std::make_pair(i->first, i->second)); - if (ret) + if (ret) { ALOGE("Failed to dedicate protected layer! Dropping it."); + return ret; + } protected_zorder = i->first; i = layers.erase(i); @@ -139,8 +141,10 @@ int PlanStageGreedy::ProvisionPlanes( // We don't have any planes left if (ret == -ENOENT) break; - else if (ret) + else if (ret) { ALOGE("Failed to emplace layer %zu, dropping it", i->first); + return ret; + } } return 0; diff --git a/platformhisi.cpp b/platformhisi.cpp index 0e12ef1..bf4033b 100644 --- a/platformhisi.cpp +++ b/platformhisi.cpp @@ -158,8 +158,10 @@ class PlanStageHiSi : public Planner::PlanStage { // We don't have any planes left if (ret == -ENOENT) break; - else if (ret) + else if (ret) { ALOGE("Failed to emplace layer %zu, dropping it", i->first); + return ret; + } } /* * If we only have one layer, but we didn't emplace anything, we -- cgit v1.2.3 From ea1c5e5a1bd4830b7c8b8da230e816e5efda1acb Mon Sep 17 00:00:00 2001 From: Alexandru Gheorghe Date: Mon, 17 Sep 2018 10:48:54 +0100 Subject: drm_hwcomposer: Add z order support Currently, the planner just pops the first available drm plane and if that can't be used for the DrmHwcLayer it just returns error. This proposes a slighlty smarter way to do that by trying to see if any of the DrmPlane can be used for the DrmHwcLayer in question. More, if the drm_plane doesn't have a fix zorder then we could re-add him to the list of unused planes, so it could be used for hosting other less demanding DrmHwcLayers. Signed-off-by: Alexandru Gheorghe --- drmdisplaycompositor.cpp | 11 +++++++++++ drmplane.cpp | 8 ++++++++ drmplane.h | 2 ++ drmproperty.cpp | 4 ++++ drmproperty.h | 1 + platform.h | 28 ++++++++++++++++++---------- 6 files changed, 44 insertions(+), 10 deletions(-) diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index cd6d4b2..a1ccdc7 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -359,6 +359,17 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, } } + if (plane->zpos_property().id() && !plane->zpos_property().immutable()) { + ret = drmModeAtomicAddProperty(pset, plane->id(), + plane->zpos_property().id(), + source_layers.front()) < 0; + if (ret) { + ALOGE("Failed to add zpos property %d to plane %d", + plane->zpos_property().id(), plane->id()); + break; + } + } + rotation = 0; if (layer.transform & DrmHwcTransform::kFlipH) rotation |= DRM_MODE_REFLECT_X; diff --git a/drmplane.cpp b/drmplane.cpp index 6747621..35f91b4 100644 --- a/drmplane.cpp +++ b/drmplane.cpp @@ -118,6 +118,10 @@ int DrmPlane::Init() { return ret; } + ret = drm_->GetPlaneProperty(*this, "zpos", &zpos_property_); + if (ret) + ALOGE("Could not get zpos property for plane %u", id()); + ret = drm_->GetPlaneProperty(*this, "rotation", &rotation_property_); if (ret) ALOGE("Could not get rotation property"); @@ -189,6 +193,10 @@ const DrmProperty &DrmPlane::src_h_property() const { return src_h_property_; } +const DrmProperty &DrmPlane::zpos_property() const { + return zpos_property_; +} + const DrmProperty &DrmPlane::rotation_property() const { return rotation_property_; } diff --git a/drmplane.h b/drmplane.h index b7607ff..43e0e8a 100644 --- a/drmplane.h +++ b/drmplane.h @@ -52,6 +52,7 @@ class DrmPlane { const DrmProperty &src_y_property() const; const DrmProperty &src_w_property() const; const DrmProperty &src_h_property() const; + const DrmProperty &zpos_property() const; const DrmProperty &rotation_property() const; const DrmProperty &alpha_property() const; const DrmProperty &blend_property() const; @@ -75,6 +76,7 @@ class DrmPlane { DrmProperty src_y_property_; DrmProperty src_w_property_; DrmProperty src_h_property_; + DrmProperty zpos_property_; DrmProperty rotation_property_; DrmProperty alpha_property_; DrmProperty blend_property_; diff --git a/drmproperty.cpp b/drmproperty.cpp index dcab05e..9faa37e 100644 --- a/drmproperty.cpp +++ b/drmproperty.cpp @@ -100,6 +100,10 @@ int DrmProperty::value(uint64_t *value) const { } } +bool DrmProperty::immutable() const { + return id_ && (flags_ & DRM_MODE_PROP_IMMUTABLE); +} + std::tuple DrmProperty::GetEnumValueWithName( std::string name) const { for (auto it : enums_) { diff --git a/drmproperty.h b/drmproperty.h index 5e358be..f1328fe 100644 --- a/drmproperty.h +++ b/drmproperty.h @@ -46,6 +46,7 @@ class DrmProperty { std::string name() const; int value(uint64_t *value) const; + bool immutable() const; private: class DrmPropertyEnum { diff --git a/platform.h b/platform.h index 6c12fe9..547a297 100644 --- a/platform.h +++ b/platform.h @@ -81,16 +81,24 @@ class Planner { DrmCompositionPlane::Type type, DrmCrtc *crtc, std::pair layer) { DrmPlane *plane = PopPlane(planes); - int ret; - if (!plane) - return -ENOENT; - - ret = ValidatePlane(plane, layer.second); - if (ret) - return -EINVAL; - - composition->emplace_back(type, plane, crtc, layer.first); - return 0; + std::vector unused_planes; + int ret = -ENOENT; + while (plane) { + ret = ValidatePlane(plane, layer.second); + if (!ret) + break; + if (!plane->zpos_property().immutable()) + unused_planes.push_back(plane); + plane = PopPlane(planes); + } + + if (!ret) { + composition->emplace_back(type, plane, crtc, layer.first); + planes->insert(planes->begin(), unused_planes.begin(), + unused_planes.end()); + } + + return ret; } }; -- cgit v1.2.3 From 18ec688fadef2319092389407f98bb1f377ef8d7 Mon Sep 17 00:00:00 2001 From: Alexey Firago Date: Wed, 21 Nov 2018 23:47:05 +0300 Subject: drm_hwcomposer: Add checking if we can import a buffer Add CanImportBuffer() function to the Importer interface. Platform specific importer should check in this function if it can import given buffer_handle_t. For example platformhisi will return false for buffers without GRALLOC_USAGE_HW_FB. This function should be used on ValidateDisplay step to avoid the need of 'fake-importing' of buffers. Signed-off-by: Alexey Firago --- drmhwctwo.cpp | 11 ++++++++--- platform.h | 3 +++ platformdrmgeneric.cpp | 6 ++++++ platformdrmgeneric.h | 1 + platformhisi.cpp | 6 ++++++ platformhisi.h | 2 ++ 6 files changed, 26 insertions(+), 3 deletions(-) diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp index c801f2e..cd79e7b 100644 --- a/drmhwctwo.cpp +++ b/drmhwctwo.cpp @@ -491,9 +491,13 @@ HWC2::Error DrmHwcTwo::HwcDisplay::CreateComposition(bool test) { std::map z_map; for (std::pair &l : layers_) { HWC2::Composition comp_type; - if (test) + if (test) { comp_type = l.second.sf_type(); - else + if (comp_type == HWC2::Composition::Device) { + if (!importer_->CanImportBuffer(l.second.buffer())) + comp_type = HWC2::Composition::Client; + } + } else comp_type = l.second.validated_type(); switch (comp_type) { @@ -735,7 +739,8 @@ HWC2::Error DrmHwcTwo::HwcDisplay::ValidateDisplay(uint32_t *num_types, for (std::pair &l : z_map) { if (comp_failed || !avail_planes--) break; - l.second->set_validated_type(HWC2::Composition::Device); + if (importer_->CanImportBuffer(l.second->buffer())) + l.second->set_validated_type(HWC2::Composition::Device); } for (std::pair &l : layers_) { diff --git a/platform.h b/platform.h index 547a297..a58d62e 100644 --- a/platform.h +++ b/platform.h @@ -49,6 +49,9 @@ class Importer { // Note: This can be called from a different thread than ImportBuffer. The // implementation is responsible for ensuring thread safety. virtual int ReleaseBuffer(hwc_drm_bo_t *bo) = 0; + + // Checks if importer can import the buffer. + virtual bool CanImportBuffer(buffer_handle_t handle) = 0; }; class Planner { diff --git a/platformdrmgeneric.cpp b/platformdrmgeneric.cpp index 24d0650..503c04a 100644 --- a/platformdrmgeneric.cpp +++ b/platformdrmgeneric.cpp @@ -161,6 +161,12 @@ int DrmGenericImporter::ReleaseBuffer(hwc_drm_bo_t *bo) { return 0; } +bool DrmGenericImporter::CanImportBuffer(buffer_handle_t handle) { + if (handle == NULL) + return false; + return true; +} + #ifdef USE_DRM_GENERIC_IMPORTER std::unique_ptr Planner::CreateInstance(DrmDevice *) { std::unique_ptr planner(new Planner); diff --git a/platformdrmgeneric.h b/platformdrmgeneric.h index d46e8b0..233ba55 100644 --- a/platformdrmgeneric.h +++ b/platformdrmgeneric.h @@ -33,6 +33,7 @@ class DrmGenericImporter : public Importer { int ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) override; int ReleaseBuffer(hwc_drm_bo_t *bo) override; + bool CanImportBuffer(buffer_handle_t handle) override; uint32_t ConvertHalFormatToDrm(uint32_t hal_format); uint32_t DrmFormatToBitsPerPixel(uint32_t drm_format); diff --git a/platformhisi.cpp b/platformhisi.cpp index bf4033b..c5cadf6 100644 --- a/platformhisi.cpp +++ b/platformhisi.cpp @@ -139,6 +139,12 @@ int HisiImporter::ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) { return ret; } +bool HisiImporter::CanImportBuffer(buffer_handle_t handle) { + private_handle_t const *hnd = reinterpret_cast( + handle); + return hnd && (hnd->usage & GRALLOC_USAGE_HW_FB); +} + class PlanStageHiSi : public Planner::PlanStage { public: int ProvisionPlanes(std::vector *composition, diff --git a/platformhisi.h b/platformhisi.h index 2b2d439..927da17 100644 --- a/platformhisi.h +++ b/platformhisi.h @@ -36,6 +36,8 @@ class HisiImporter : public DrmGenericImporter { int ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) override; + bool CanImportBuffer(buffer_handle_t handle) override; + private: DrmDevice *drm_; -- cgit v1.2.3 From c385afe4f7e53c4d4eff1069e48454748c2de940 Mon Sep 17 00:00:00 2001 From: Alexey Firago Date: Tue, 27 Nov 2018 14:17:55 +0300 Subject: drm_hwcomposer: platformhisi: Remove fake-importing With CanImportBuffer() in place we don't need fake importing anymore. Buffers should be checked before supplied to ImportBuffer() and plane planner. Instead of fake importing return -EINVAL for non HW_FB buffers in ImportBuffer() and skip non HW_FB in planner. Additionally, return error from planner if we didn't emplace any layer to force client compositing. Signed-off-by: Alexey Firago --- platformhisi.cpp | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/platformhisi.cpp b/platformhisi.cpp index c5cadf6..76fe1e7 100644 --- a/platformhisi.cpp +++ b/platformhisi.cpp @@ -78,10 +78,10 @@ int HisiImporter::ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) { if (!hnd) return -EINVAL; - // We can't import these types of buffers, so pretend we did and rely on the - // planner to skip them when choosing layers for planes + // We can't import these types of buffers. + // These buffers should have been filtered out with CanImportBuffer() if (!(hnd->usage & GRALLOC_USAGE_HW_FB)) - return 0; + return -EINVAL; uint32_t gem_handle; int ret = drmPrimeFDToHandle(drm_->fd(), hnd->share_fd, &gem_handle); @@ -151,9 +151,9 @@ class PlanStageHiSi : public Planner::PlanStage { std::map &layers, DrmCrtc *crtc, std::vector *planes) { int layers_added = 0; - int initial_layers = layers.size(); - // Fill up as many planes as we can with buffers that do not have HW_FB - // usage + // Fill up as many DRM planes as we can with buffers that have HW_FB usage. + // Buffers without HW_FB should have been filtered out with + // CanImportBuffer(), if we meet one here, just skip it. for (auto i = layers.begin(); i != layers.end(); i = layers.erase(i)) { if (!(i->second->gralloc_buffer_usage & GRALLOC_USAGE_HW_FB)) continue; @@ -169,14 +169,9 @@ class PlanStageHiSi : public Planner::PlanStage { return ret; } } - /* - * If we only have one layer, but we didn't emplace anything, we - * can run into trouble, as we might try to device composite a - * buffer we fake-imported, which can cause things to jamb up. - * So return an error in this case to ensure we force client - * compositing. - */ - if (!layers_added && (initial_layers <= 1)) + // If we didn't emplace anything, return an error to ensure we force client + // compositing. + if (!layers_added) return -EINVAL; return 0; -- cgit v1.2.3