| Message ID | 20180517220816.31504-1-robh@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [hwc] drm_hwcomposer: Support assigning planes in ValidateDisplay | expand |
Hey Rob, On Fri, May 18, 2018 at 12:08 AM, Rob Herring <robh@kernel.org> wrote: > In order to assign planes to layers in ValidateDisplay, testing > compositing with a DRM atomic modeset test is needed as PresentDisplay > is too late. This means most of PresentDisplay needs to be run from > ValidateDisplay, so refactor PresentDisplay and plumb a test flag down > the stack. > > Signed-off-by: Rob Herring <robh@kernel.org> > --- > Based on my GL comp removal work. Adding atomic test path turned out to > be easier than using the Planner directly. > > drmdisplaycompositor.cpp | 4 +- > drmdisplaycompositor.h | 2 +- > drmhwctwo.cpp | 79 ++++++++++++++++++++++++++++++++++------ > drmhwctwo.h | 1 + > 4 files changed, 73 insertions(+), 13 deletions(-) > > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp > index 2f9f6c6772da..ceee6b1a4bd6 100644 > --- a/drmdisplaycompositor.cpp > +++ b/drmdisplaycompositor.cpp > @@ -469,7 +469,7 @@ void DrmDisplayCompositor::ApplyFrame( > } > > int DrmDisplayCompositor::ApplyComposition( > - std::unique_ptr<DrmDisplayComposition> composition) { > + std::unique_ptr<DrmDisplayComposition> composition, bool test) { Since ApplyComposition just calls CommitFrame which already has test support, can we reuse that for a TestComposition function? > int ret = 0; > switch (composition->type()) { > case DRM_COMPOSITION_TYPE_FRAME: > @@ -481,6 +481,8 @@ int DrmDisplayCompositor::ApplyComposition( > ALOGE("Commit test failed for display %d, FIXME", display_); > return ret; > } > + if (test) > + return 0; Then we could also drop this hunk. > } > > ApplyFrame(std::move(composition), ret); > diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h > index ed46873c3409..7c5ebb005dc4 100644 > --- a/drmdisplaycompositor.h > +++ b/drmdisplaycompositor.h > @@ -43,7 +43,7 @@ class DrmDisplayCompositor { > int Init(DrmResources *drm, int display); > > std::unique_ptr<DrmDisplayComposition> CreateComposition() const; > - int ApplyComposition(std::unique_ptr<DrmDisplayComposition> composition); > + int ApplyComposition(std::unique_ptr<DrmDisplayComposition> composition, bool test); And this one. > int Composite(); > void Dump(std::ostringstream *out) const; > > diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp > index deaf14363e8c..19f73e721154 100644 > --- a/drmhwctwo.cpp > +++ b/drmhwctwo.cpp > @@ -480,8 +480,7 @@ void DrmHwcTwo::HwcDisplay::AddFenceToRetireFence(int fd) { > } > } > > -HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { > - supported(__func__); > +HWC2::Error DrmHwcTwo::HwcDisplay::CreateComposition(bool test) { This is called CreateComposition, but it also tests and even commits. If this only sets up the DrmDisplayComposition, we could just have the test and commit in Validate/Present respectively and drop the flag. Unfortunately it also uses and mutates the layer state, and moving that into Present/Validate from here looks more difficult. > std::vector<DrmCompositionDisplayLayersMap> layers_map; > layers_map.emplace_back(); > DrmCompositionDisplayLayersMap &map = layers_map.back(); > @@ -494,7 +493,13 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { > uint32_t client_z_order = 0; > std::map<uint32_t, DrmHwcTwo::HwcLayer *> z_map; > for (std::pair<const hwc2_layer_t, DrmHwcTwo::HwcLayer> &l : layers_) { > - switch (l.second.validated_type()) { > + HWC2::Composition comp_type; > + if (test) > + comp_type = l.second.sf_type(); > + else > + comp_type = l.second.validated_type(); > + > + switch (comp_type) { > case HWC2::Composition::Device: > z_map.emplace(std::make_pair(l.second.z_order(), &l.second)); > break; > @@ -510,6 +515,10 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { > if (use_client_layer) > z_map.emplace(std::make_pair(client_z_order, &client_layer_)); > > + if (z_map.empty()) > + return HWC2::Error::BadLayer; > + > + Extra newline. > // now that they're ordered by z, add them to the composition > for (std::pair<const uint32_t, DrmHwcTwo::HwcLayer *> &l : z_map) { > DrmHwcLayer layer; > @@ -521,10 +530,6 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { > } > map.layers.emplace_back(std::move(layer)); > } > - if (map.layers.empty()) { > - *retire_fence = -1; > - return HWC2::Error::None; > - } > > std::unique_ptr<DrmDisplayComposition> composition = > compositor_.CreateComposition(); > @@ -555,13 +560,29 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { > i = overlay_planes.erase(i); > } > > - AddFenceToRetireFence(composition->take_out_fence()); > + if (!test) > + AddFenceToRetireFence(composition->take_out_fence()); > > - ret = compositor_.ApplyComposition(std::move(composition)); > + ret = compositor_.ApplyComposition(std::move(composition), test); > if (ret) { > ALOGE("Failed to apply the frame composition ret=%d", ret); > return HWC2::Error::BadParameter; > } > + return HWC2::Error::None; > +} > + > +HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { > + supported(__func__); > + HWC2::Error ret; > + > + ret = CreateComposition(false); > + if (ret == HWC2::Error::BadLayer) { > + // Can we really have no client or device layers? > + *retire_fence = -1; > + return HWC2::Error::None; > + } > + if (ret != HWC2::Error::None) > + return ret; > > // The retire fence returned here is for the last frame, so return it and > // promote the next retire fence > @@ -586,7 +607,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetActiveConfig(hwc2_config_t config) { > compositor_.CreateComposition(); > composition->Init(drm_, crtc_, importer_.get(), planner_.get(), frame_no_); > int ret = composition->SetDisplayMode(*mode); > - ret = compositor_.ApplyComposition(std::move(composition)); > + ret = compositor_.ApplyComposition(std::move(composition), false); > if (ret) { > ALOGE("Failed to queue dpms composition on %d", ret); > return HWC2::Error::BadConfig; > @@ -666,7 +687,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetPowerMode(int32_t mode_in) { > compositor_.CreateComposition(); > composition->Init(drm_, crtc_, importer_.get(), planner_.get(), frame_no_); > composition->SetDpmsMode(dpms_value); > - int ret = compositor_.ApplyComposition(std::move(composition)); > + int ret = compositor_.ApplyComposition(std::move(composition), false); With a TestComposition we can drop the two hunks here. > if (ret) { > ALOGE("Failed to apply the dpms composition ret=%d", ret); > return HWC2::Error::BadParameter; > @@ -683,11 +704,47 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetVsyncEnabled(int32_t enabled) { > HWC2::Error DrmHwcTwo::HwcDisplay::ValidateDisplay(uint32_t *num_types, > uint32_t *num_requests) { > supported(__func__); > + size_t plane_count = 0; > *num_types = 0; > *num_requests = 0; > + size_t avail_planes = primary_planes_.size() + overlay_planes_.size(); > + > + HWC2::Error ret; > + > + for (std::pair<const hwc2_layer_t, DrmHwcTwo::HwcLayer> &l : layers_) > + l.second.set_validated_type(HWC2::Composition::Invalid); > + > + ret = CreateComposition(true); > + if (ret != HWC2::Error::None) > + // Assume the test failed due to overlay planes > + avail_planes = 1; > + > + std::map<uint32_t, DrmHwcTwo::HwcLayer *> z_map; > + for (std::pair<const hwc2_layer_t, DrmHwcTwo::HwcLayer> &l : layers_) { > + if (l.second.sf_type() == HWC2::Composition::Device) > + z_map.emplace(std::make_pair(l.second.z_order(), &l.second)); > + } > + > + /* > + * If more layers then planes, save one plane > + * for client composited layers > + */ > + if (avail_planes < layers_.size()) > + avail_planes--; > + > + for (std::pair<const uint32_t, DrmHwcTwo::HwcLayer *> &l : z_map) { > + if (!avail_planes--) > + break; > + l.second->set_validated_type(HWC2::Composition::Device); > + } > + > for (std::pair<const hwc2_layer_t, DrmHwcTwo::HwcLayer> &l : layers_) { > DrmHwcTwo::HwcLayer &layer = l.second; > switch (layer.sf_type()) { > + case HWC2::Composition::Device: > + if (layer.validated_type() == HWC2::Composition::Device) > + break; > + // fall thru > case HWC2::Composition::SolidColor: > case HWC2::Composition::Cursor: > case HWC2::Composition::Sideband: > diff --git a/drmhwctwo.h b/drmhwctwo.h > index 0490e2ac7f85..bbf55d9a08bc 100644 > --- a/drmhwctwo.h > +++ b/drmhwctwo.h > @@ -171,6 +171,7 @@ class DrmHwcTwo : public hwc2_device_t { > float *min_luminance); > HWC2::Error GetReleaseFences(uint32_t *num_elements, hwc2_layer_t *layers, > int32_t *fences); > + HWC2::Error CreateComposition(bool test); This is more of a private function than a HWC2 hook. > HWC2::Error PresentDisplay(int32_t *retire_fence); > HWC2::Error SetActiveConfig(hwc2_config_t config); > HWC2::Error SetClientTarget(buffer_handle_t target, int32_t acquire_fence, > -- > 2.17.0 > Thanks for working on this! Fixup for the nits above: https://gitlab.freedesktop.org/stschake/drm-hwcomposer/commit/3aa0eb88 Took a short look at isolating the composition, too, but it's pretty interwoven with the layer state. Regards, Stefan
On Thu, May 17, 2018 at 6:25 PM, Stefan Schake <stschake@gmail.com> wrote: > Hey Rob, > > On Fri, May 18, 2018 at 12:08 AM, Rob Herring <robh@kernel.org> wrote: >> In order to assign planes to layers in ValidateDisplay, testing >> compositing with a DRM atomic modeset test is needed as PresentDisplay >> is too late. This means most of PresentDisplay needs to be run from >> ValidateDisplay, so refactor PresentDisplay and plumb a test flag down >> the stack. >> >> Signed-off-by: Rob Herring <robh@kernel.org> >> --- >> Based on my GL comp removal work. Adding atomic test path turned out to >> be easier than using the Planner directly. >> >> drmdisplaycompositor.cpp | 4 +- >> drmdisplaycompositor.h | 2 +- >> drmhwctwo.cpp | 79 ++++++++++++++++++++++++++++++++++------ >> drmhwctwo.h | 1 + >> 4 files changed, 73 insertions(+), 13 deletions(-) >> >> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp >> index 2f9f6c6772da..ceee6b1a4bd6 100644 >> --- a/drmdisplaycompositor.cpp >> +++ b/drmdisplaycompositor.cpp >> @@ -469,7 +469,7 @@ void DrmDisplayCompositor::ApplyFrame( >> } >> >> int DrmDisplayCompositor::ApplyComposition( >> - std::unique_ptr<DrmDisplayComposition> composition) { >> + std::unique_ptr<DrmDisplayComposition> composition, bool test) { > > Since ApplyComposition just calls CommitFrame which already has test > support, can we reuse that for a TestComposition function? > >> int ret = 0; >> switch (composition->type()) { >> case DRM_COMPOSITION_TYPE_FRAME: >> @@ -481,6 +481,8 @@ int DrmDisplayCompositor::ApplyComposition( >> ALOGE("Commit test failed for display %d, FIXME", display_); >> return ret; >> } >> + if (test) >> + return 0; > > Then we could also drop this hunk. > >> } >> >> ApplyFrame(std::move(composition), ret); >> diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h >> index ed46873c3409..7c5ebb005dc4 100644 >> --- a/drmdisplaycompositor.h >> +++ b/drmdisplaycompositor.h >> @@ -43,7 +43,7 @@ class DrmDisplayCompositor { >> int Init(DrmResources *drm, int display); >> >> std::unique_ptr<DrmDisplayComposition> CreateComposition() const; >> - int ApplyComposition(std::unique_ptr<DrmDisplayComposition> composition); >> + int ApplyComposition(std::unique_ptr<DrmDisplayComposition> composition, bool test); > > And this one. > >> int Composite(); >> void Dump(std::ostringstream *out) const; >> >> diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp >> index deaf14363e8c..19f73e721154 100644 >> --- a/drmhwctwo.cpp >> +++ b/drmhwctwo.cpp >> @@ -480,8 +480,7 @@ void DrmHwcTwo::HwcDisplay::AddFenceToRetireFence(int fd) { >> } >> } >> >> -HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { >> - supported(__func__); >> +HWC2::Error DrmHwcTwo::HwcDisplay::CreateComposition(bool test) { > > This is called CreateComposition, but it also tests and even commits. > If this only sets up the DrmDisplayComposition, we could just have the > test and commit in Validate/Present respectively and drop the flag. > Unfortunately it also uses and mutates the layer state, and moving that > into Present/Validate from here looks more difficult. > >> std::vector<DrmCompositionDisplayLayersMap> layers_map; >> layers_map.emplace_back(); >> DrmCompositionDisplayLayersMap &map = layers_map.back(); >> @@ -494,7 +493,13 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { >> uint32_t client_z_order = 0; >> std::map<uint32_t, DrmHwcTwo::HwcLayer *> z_map; >> for (std::pair<const hwc2_layer_t, DrmHwcTwo::HwcLayer> &l : layers_) { >> - switch (l.second.validated_type()) { >> + HWC2::Composition comp_type; >> + if (test) >> + comp_type = l.second.sf_type(); >> + else >> + comp_type = l.second.validated_type(); >> + >> + switch (comp_type) { >> case HWC2::Composition::Device: >> z_map.emplace(std::make_pair(l.second.z_order(), &l.second)); >> break; >> @@ -510,6 +515,10 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { >> if (use_client_layer) >> z_map.emplace(std::make_pair(client_z_order, &client_layer_)); >> >> + if (z_map.empty()) >> + return HWC2::Error::BadLayer; >> + >> + > > Extra newline. > >> // now that they're ordered by z, add them to the composition >> for (std::pair<const uint32_t, DrmHwcTwo::HwcLayer *> &l : z_map) { >> DrmHwcLayer layer; >> @@ -521,10 +530,6 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { >> } >> map.layers.emplace_back(std::move(layer)); >> } >> - if (map.layers.empty()) { >> - *retire_fence = -1; >> - return HWC2::Error::None; >> - } >> >> std::unique_ptr<DrmDisplayComposition> composition = >> compositor_.CreateComposition(); >> @@ -555,13 +560,29 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { >> i = overlay_planes.erase(i); >> } >> >> - AddFenceToRetireFence(composition->take_out_fence()); >> + if (!test) >> + AddFenceToRetireFence(composition->take_out_fence()); >> >> - ret = compositor_.ApplyComposition(std::move(composition)); >> + ret = compositor_.ApplyComposition(std::move(composition), test); >> if (ret) { >> ALOGE("Failed to apply the frame composition ret=%d", ret); >> return HWC2::Error::BadParameter; >> } >> + return HWC2::Error::None; >> +} >> + >> +HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { >> + supported(__func__); >> + HWC2::Error ret; >> + >> + ret = CreateComposition(false); >> + if (ret == HWC2::Error::BadLayer) { >> + // Can we really have no client or device layers? >> + *retire_fence = -1; >> + return HWC2::Error::None; >> + } >> + if (ret != HWC2::Error::None) >> + return ret; >> >> // The retire fence returned here is for the last frame, so return it and >> // promote the next retire fence >> @@ -586,7 +607,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetActiveConfig(hwc2_config_t config) { >> compositor_.CreateComposition(); >> composition->Init(drm_, crtc_, importer_.get(), planner_.get(), frame_no_); >> int ret = composition->SetDisplayMode(*mode); >> - ret = compositor_.ApplyComposition(std::move(composition)); >> + ret = compositor_.ApplyComposition(std::move(composition), false); >> if (ret) { >> ALOGE("Failed to queue dpms composition on %d", ret); >> return HWC2::Error::BadConfig; >> @@ -666,7 +687,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetPowerMode(int32_t mode_in) { >> compositor_.CreateComposition(); >> composition->Init(drm_, crtc_, importer_.get(), planner_.get(), frame_no_); >> composition->SetDpmsMode(dpms_value); >> - int ret = compositor_.ApplyComposition(std::move(composition)); >> + int ret = compositor_.ApplyComposition(std::move(composition), false); > > With a TestComposition we can drop the two hunks here. > >> if (ret) { >> ALOGE("Failed to apply the dpms composition ret=%d", ret); >> return HWC2::Error::BadParameter; >> @@ -683,11 +704,47 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetVsyncEnabled(int32_t enabled) { >> HWC2::Error DrmHwcTwo::HwcDisplay::ValidateDisplay(uint32_t *num_types, >> uint32_t *num_requests) { >> supported(__func__); >> + size_t plane_count = 0; >> *num_types = 0; >> *num_requests = 0; >> + size_t avail_planes = primary_planes_.size() + overlay_planes_.size(); >> + >> + HWC2::Error ret; >> + >> + for (std::pair<const hwc2_layer_t, DrmHwcTwo::HwcLayer> &l : layers_) >> + l.second.set_validated_type(HWC2::Composition::Invalid); >> + >> + ret = CreateComposition(true); >> + if (ret != HWC2::Error::None) >> + // Assume the test failed due to overlay planes >> + avail_planes = 1; >> + >> + std::map<uint32_t, DrmHwcTwo::HwcLayer *> z_map; >> + for (std::pair<const hwc2_layer_t, DrmHwcTwo::HwcLayer> &l : layers_) { >> + if (l.second.sf_type() == HWC2::Composition::Device) >> + z_map.emplace(std::make_pair(l.second.z_order(), &l.second)); >> + } >> + >> + /* >> + * If more layers then planes, save one plane >> + * for client composited layers >> + */ >> + if (avail_planes < layers_.size()) >> + avail_planes--; >> + >> + for (std::pair<const uint32_t, DrmHwcTwo::HwcLayer *> &l : z_map) { >> + if (!avail_planes--) >> + break; >> + l.second->set_validated_type(HWC2::Composition::Device); >> + } >> + >> for (std::pair<const hwc2_layer_t, DrmHwcTwo::HwcLayer> &l : layers_) { >> DrmHwcTwo::HwcLayer &layer = l.second; >> switch (layer.sf_type()) { >> + case HWC2::Composition::Device: >> + if (layer.validated_type() == HWC2::Composition::Device) >> + break; >> + // fall thru >> case HWC2::Composition::SolidColor: >> case HWC2::Composition::Cursor: >> case HWC2::Composition::Sideband: >> diff --git a/drmhwctwo.h b/drmhwctwo.h >> index 0490e2ac7f85..bbf55d9a08bc 100644 >> --- a/drmhwctwo.h >> +++ b/drmhwctwo.h >> @@ -171,6 +171,7 @@ class DrmHwcTwo : public hwc2_device_t { >> float *min_luminance); >> HWC2::Error GetReleaseFences(uint32_t *num_elements, hwc2_layer_t *layers, >> int32_t *fences); >> + HWC2::Error CreateComposition(bool test); > > This is more of a private function than a HWC2 hook. > >> HWC2::Error PresentDisplay(int32_t *retire_fence); >> HWC2::Error SetActiveConfig(hwc2_config_t config); >> HWC2::Error SetClientTarget(buffer_handle_t target, int32_t acquire_fence, >> -- >> 2.17.0 >> > > Thanks for working on this! Fixup for the nits above: > > https://gitlab.freedesktop.org/stschake/drm-hwcomposer/commit/3aa0eb88 Thanks. That's all much better. I've folded that in and updated the merge request. Rob
diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index 2f9f6c6772da..ceee6b1a4bd6 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -469,7 +469,7 @@ void DrmDisplayCompositor::ApplyFrame( } int DrmDisplayCompositor::ApplyComposition( - std::unique_ptr<DrmDisplayComposition> composition) { + std::unique_ptr<DrmDisplayComposition> composition, bool test) { int ret = 0; switch (composition->type()) { case DRM_COMPOSITION_TYPE_FRAME: @@ -481,6 +481,8 @@ int DrmDisplayCompositor::ApplyComposition( ALOGE("Commit test failed for display %d, FIXME", display_); return ret; } + if (test) + return 0; } ApplyFrame(std::move(composition), ret); diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h index ed46873c3409..7c5ebb005dc4 100644 --- a/drmdisplaycompositor.h +++ b/drmdisplaycompositor.h @@ -43,7 +43,7 @@ class DrmDisplayCompositor { int Init(DrmResources *drm, int display); std::unique_ptr<DrmDisplayComposition> CreateComposition() const; - int ApplyComposition(std::unique_ptr<DrmDisplayComposition> composition); + int ApplyComposition(std::unique_ptr<DrmDisplayComposition> composition, bool test); int Composite(); void Dump(std::ostringstream *out) const; diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp index deaf14363e8c..19f73e721154 100644 --- a/drmhwctwo.cpp +++ b/drmhwctwo.cpp @@ -480,8 +480,7 @@ void DrmHwcTwo::HwcDisplay::AddFenceToRetireFence(int fd) { } } -HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { - supported(__func__); +HWC2::Error DrmHwcTwo::HwcDisplay::CreateComposition(bool test) { std::vector<DrmCompositionDisplayLayersMap> layers_map; layers_map.emplace_back(); DrmCompositionDisplayLayersMap &map = layers_map.back(); @@ -494,7 +493,13 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { uint32_t client_z_order = 0; std::map<uint32_t, DrmHwcTwo::HwcLayer *> z_map; for (std::pair<const hwc2_layer_t, DrmHwcTwo::HwcLayer> &l : layers_) { - switch (l.second.validated_type()) { + HWC2::Composition comp_type; + if (test) + comp_type = l.second.sf_type(); + else + comp_type = l.second.validated_type(); + + switch (comp_type) { case HWC2::Composition::Device: z_map.emplace(std::make_pair(l.second.z_order(), &l.second)); break; @@ -510,6 +515,10 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { if (use_client_layer) z_map.emplace(std::make_pair(client_z_order, &client_layer_)); + if (z_map.empty()) + return HWC2::Error::BadLayer; + + // now that they're ordered by z, add them to the composition for (std::pair<const uint32_t, DrmHwcTwo::HwcLayer *> &l : z_map) { DrmHwcLayer layer; @@ -521,10 +530,6 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { } map.layers.emplace_back(std::move(layer)); } - if (map.layers.empty()) { - *retire_fence = -1; - return HWC2::Error::None; - } std::unique_ptr<DrmDisplayComposition> composition = compositor_.CreateComposition(); @@ -555,13 +560,29 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { i = overlay_planes.erase(i); } - AddFenceToRetireFence(composition->take_out_fence()); + if (!test) + AddFenceToRetireFence(composition->take_out_fence()); - ret = compositor_.ApplyComposition(std::move(composition)); + ret = compositor_.ApplyComposition(std::move(composition), test); if (ret) { ALOGE("Failed to apply the frame composition ret=%d", ret); return HWC2::Error::BadParameter; } + return HWC2::Error::None; +} + +HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { + supported(__func__); + HWC2::Error ret; + + ret = CreateComposition(false); + if (ret == HWC2::Error::BadLayer) { + // Can we really have no client or device layers? + *retire_fence = -1; + return HWC2::Error::None; + } + if (ret != HWC2::Error::None) + return ret; // The retire fence returned here is for the last frame, so return it and // promote the next retire fence @@ -586,7 +607,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetActiveConfig(hwc2_config_t config) { compositor_.CreateComposition(); composition->Init(drm_, crtc_, importer_.get(), planner_.get(), frame_no_); int ret = composition->SetDisplayMode(*mode); - ret = compositor_.ApplyComposition(std::move(composition)); + ret = compositor_.ApplyComposition(std::move(composition), false); if (ret) { ALOGE("Failed to queue dpms composition on %d", ret); return HWC2::Error::BadConfig; @@ -666,7 +687,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetPowerMode(int32_t mode_in) { compositor_.CreateComposition(); composition->Init(drm_, crtc_, importer_.get(), planner_.get(), frame_no_); composition->SetDpmsMode(dpms_value); - int ret = compositor_.ApplyComposition(std::move(composition)); + int ret = compositor_.ApplyComposition(std::move(composition), false); if (ret) { ALOGE("Failed to apply the dpms composition ret=%d", ret); return HWC2::Error::BadParameter; @@ -683,11 +704,47 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetVsyncEnabled(int32_t enabled) { HWC2::Error DrmHwcTwo::HwcDisplay::ValidateDisplay(uint32_t *num_types, uint32_t *num_requests) { supported(__func__); + size_t plane_count = 0; *num_types = 0; *num_requests = 0; + size_t avail_planes = primary_planes_.size() + overlay_planes_.size(); + + HWC2::Error ret; + + for (std::pair<const hwc2_layer_t, DrmHwcTwo::HwcLayer> &l : layers_) + l.second.set_validated_type(HWC2::Composition::Invalid); + + ret = CreateComposition(true); + if (ret != HWC2::Error::None) + // Assume the test failed due to overlay planes + avail_planes = 1; + + std::map<uint32_t, DrmHwcTwo::HwcLayer *> z_map; + for (std::pair<const hwc2_layer_t, DrmHwcTwo::HwcLayer> &l : layers_) { + if (l.second.sf_type() == HWC2::Composition::Device) + z_map.emplace(std::make_pair(l.second.z_order(), &l.second)); + } + + /* + * If more layers then planes, save one plane + * for client composited layers + */ + if (avail_planes < layers_.size()) + avail_planes--; + + for (std::pair<const uint32_t, DrmHwcTwo::HwcLayer *> &l : z_map) { + if (!avail_planes--) + break; + l.second->set_validated_type(HWC2::Composition::Device); + } + for (std::pair<const hwc2_layer_t, DrmHwcTwo::HwcLayer> &l : layers_) { DrmHwcTwo::HwcLayer &layer = l.second; switch (layer.sf_type()) { + case HWC2::Composition::Device: + if (layer.validated_type() == HWC2::Composition::Device) + break; + // fall thru case HWC2::Composition::SolidColor: case HWC2::Composition::Cursor: case HWC2::Composition::Sideband: diff --git a/drmhwctwo.h b/drmhwctwo.h index 0490e2ac7f85..bbf55d9a08bc 100644 --- a/drmhwctwo.h +++ b/drmhwctwo.h @@ -171,6 +171,7 @@ class DrmHwcTwo : public hwc2_device_t { float *min_luminance); HWC2::Error GetReleaseFences(uint32_t *num_elements, hwc2_layer_t *layers, int32_t *fences); + HWC2::Error CreateComposition(bool test); HWC2::Error PresentDisplay(int32_t *retire_fence); HWC2::Error SetActiveConfig(hwc2_config_t config); HWC2::Error SetClientTarget(buffer_handle_t target, int32_t acquire_fence,
In order to assign planes to layers in ValidateDisplay, testing compositing with a DRM atomic modeset test is needed as PresentDisplay is too late. This means most of PresentDisplay needs to be run from ValidateDisplay, so refactor PresentDisplay and plumb a test flag down the stack. Signed-off-by: Rob Herring <robh@kernel.org> --- Based on my GL comp removal work. Adding atomic test path turned out to be easier than using the Planner directly. drmdisplaycompositor.cpp | 4 +- drmdisplaycompositor.h | 2 +- drmhwctwo.cpp | 79 ++++++++++++++++++++++++++++++++++------ drmhwctwo.h | 1 + 4 files changed, 73 insertions(+), 13 deletions(-)