| Message ID | 20170927115841.29134-7-robert.foss@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.com> wrote: > Add support for out-fences through the OUT_FENCE_PTR property. > Out-fences signal when their associated buffer may be read by a device. > > Signed-off-by: Robert Foss <robert.foss@collabora.com> > --- > > Changes since v1: > Sergi Granell > - Set atomic property to be out_fences[crtc->pipe()] not out_fences[0] > > drmdisplaycomposition.h | 9 +++++++++ > drmdisplaycompositor.cpp | 16 ++++++++++++++++ > drmhwctwo.cpp | 9 ++------- > 3 files changed, 27 insertions(+), 7 deletions(-) > > diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h > index b165adc..0586d58 100644 > --- a/drmdisplaycomposition.h > +++ b/drmdisplaycomposition.h > @@ -189,6 +189,14 @@ class DrmDisplayComposition { > return planner_; > } > > + int take_out_fence() { > + return out_fence_.Release(); > + } > + > + void set_out_fence(int out_fence) { > + out_fence_.Set(dup(out_fence)); Why dup if you're just going to close the original? I think the helper functions actually hurt you here. It would be easier to understand what was going on if you just manipulated out_fence_ directly in CommitFrame (then you wouldn't need the dup/close). > + } > + > void Dump(std::ostringstream *out) const; > > private: > @@ -215,6 +223,7 @@ class DrmDisplayComposition { > int timeline_current_ = 0; > int timeline_squash_done_ = 0; > int timeline_pre_comp_done_ = 0; > + UniqueFd out_fence_ = -1; > > bool geometry_changed_; > std::vector<DrmHwcLayer> layers_; > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp > index 71c0451..a1427d3 100644 > --- a/drmdisplaycompositor.cpp > +++ b/drmdisplaycompositor.cpp > @@ -492,6 +492,7 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, > display_comp->composition_planes(); > std::vector<DrmCompositionRegion> &pre_comp_regions = > display_comp->pre_comp_regions(); > + uint64_t out_fences[drm_->GetCrtcCount()]; Huh. I didn't know you could do this. > > DrmConnector *connector = drm_->GetConnectorForDisplay(display_); > if (!connector) { > @@ -510,6 +511,16 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, > return -ENOMEM; > } > > + if (crtc->out_fence_ptr_property().id() != 0) { > + ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->out_fence_ptr_property().id(), > + (uint64_t) &out_fences[crtc->pipe()]); > + if (ret < 0) { > + ALOGE("Failed to add OUT_FENCE_PTR property to pset: %d", ret); > + drmModeAtomicFree(pset); > + return ret; > + } > + } > + > if (mode_.needs_modeset) { > ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->mode_property().id(), > mode_.blob_id) < 0 || > @@ -708,6 +719,11 @@ out: > mode_.needs_modeset = false; > } > > + if (crtc->out_fence_ptr_property().id()) { > + display_comp->set_out_fence((int) out_fences[crtc->pipe()]); > + close((int) out_fences[crtc->pipe()]); > + } > + > return ret; > } > > diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp > index 762ee8c..89399bf 100644 > --- a/drmhwctwo.cpp > +++ b/drmhwctwo.cpp > @@ -557,19 +557,14 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { > i = overlay_planes.erase(i); > } > > + AddFenceToRetireFence(composition->take_out_fence()); > + > ret = compositor_.ApplyComposition(std::move(composition)); > if (ret) { > ALOGE("Failed to apply the frame composition ret=%d", ret); > return HWC2::Error::BadParameter; > } > > - // Now that the release fences have been generated by the compositor, make > - // sure they're managed properly > - for (std::pair<const uint32_t, DrmHwcTwo::HwcLayer *> &l : z_map) { > - l.second->manage_release_fence(); > - AddFenceToRetireFence(l.second->release_fence()); > - } > - > // The retire fence returned here is for the last frame, so return it and > // promote the next retire fence > *retire_fence = retire_fence_.Release(); > -- > 2.11.0 >
On Wed, 2017-09-27 at 15:11 -0400, Sean Paul wrote: > On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.c > om> wrote: > > Add support for out-fences through the OUT_FENCE_PTR property. > > Out-fences signal when their associated buffer may be read by a > > device. > > > > Signed-off-by: Robert Foss <robert.foss@collabora.com> > > --- > > > > Changes since v1: > > Sergi Granell > > - Set atomic property to be out_fences[crtc->pipe()] not > > out_fences[0] > > > > drmdisplaycomposition.h | 9 +++++++++ > > drmdisplaycompositor.cpp | 16 ++++++++++++++++ > > drmhwctwo.cpp | 9 ++------- > > 3 files changed, 27 insertions(+), 7 deletions(-) > > > > diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h > > index b165adc..0586d58 100644 > > --- a/drmdisplaycomposition.h > > +++ b/drmdisplaycomposition.h > > @@ -189,6 +189,14 @@ class DrmDisplayComposition { > > return planner_; > > } > > > > + int take_out_fence() { > > + return out_fence_.Release(); > > + } > > + > > + void set_out_fence(int out_fence) { > > + out_fence_.Set(dup(out_fence)); > > Why dup if you're just going to close the original? I think the > helper > functions actually hurt you here. It would be easier to understand > what was going on if you just manipulated out_fence_ directly in > CommitFrame (then you wouldn't need the dup/close). Yeah, that makes a lot of sense, and is a lot easier to read too. > > > + } > > + > > void Dump(std::ostringstream *out) const; > > > > private: > > @@ -215,6 +223,7 @@ class DrmDisplayComposition { > > int timeline_current_ = 0; > > int timeline_squash_done_ = 0; > > int timeline_pre_comp_done_ = 0; > > + UniqueFd out_fence_ = -1; > > > > bool geometry_changed_; > > std::vector<DrmHwcLayer> layers_; > > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp > > index 71c0451..a1427d3 100644 > > --- a/drmdisplaycompositor.cpp > > +++ b/drmdisplaycompositor.cpp > > @@ -492,6 +492,7 @@ int > > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition > > *display_comp, > > display_comp->composition_planes(); > > std::vector<DrmCompositionRegion> &pre_comp_regions = > > display_comp->pre_comp_regions(); > > + uint64_t out_fences[drm_->GetCrtcCount()]; > > Huh. I didn't know you could do this. C99 and variable length arrays man. Progress at a glacial pace is still happening in C-land. Rob.
diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h index b165adc..0586d58 100644 --- a/drmdisplaycomposition.h +++ b/drmdisplaycomposition.h @@ -189,6 +189,14 @@ class DrmDisplayComposition { return planner_; } + int take_out_fence() { + return out_fence_.Release(); + } + + void set_out_fence(int out_fence) { + out_fence_.Set(dup(out_fence)); + } + void Dump(std::ostringstream *out) const; private: @@ -215,6 +223,7 @@ class DrmDisplayComposition { int timeline_current_ = 0; int timeline_squash_done_ = 0; int timeline_pre_comp_done_ = 0; + UniqueFd out_fence_ = -1; bool geometry_changed_; std::vector<DrmHwcLayer> layers_; diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index 71c0451..a1427d3 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -492,6 +492,7 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, display_comp->composition_planes(); std::vector<DrmCompositionRegion> &pre_comp_regions = display_comp->pre_comp_regions(); + uint64_t out_fences[drm_->GetCrtcCount()]; DrmConnector *connector = drm_->GetConnectorForDisplay(display_); if (!connector) { @@ -510,6 +511,16 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, return -ENOMEM; } + if (crtc->out_fence_ptr_property().id() != 0) { + ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->out_fence_ptr_property().id(), + (uint64_t) &out_fences[crtc->pipe()]); + if (ret < 0) { + ALOGE("Failed to add OUT_FENCE_PTR property to pset: %d", ret); + drmModeAtomicFree(pset); + return ret; + } + } + if (mode_.needs_modeset) { ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->mode_property().id(), mode_.blob_id) < 0 || @@ -708,6 +719,11 @@ out: mode_.needs_modeset = false; } + if (crtc->out_fence_ptr_property().id()) { + display_comp->set_out_fence((int) out_fences[crtc->pipe()]); + close((int) out_fences[crtc->pipe()]); + } + return ret; } diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp index 762ee8c..89399bf 100644 --- a/drmhwctwo.cpp +++ b/drmhwctwo.cpp @@ -557,19 +557,14 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { i = overlay_planes.erase(i); } + AddFenceToRetireFence(composition->take_out_fence()); + ret = compositor_.ApplyComposition(std::move(composition)); if (ret) { ALOGE("Failed to apply the frame composition ret=%d", ret); return HWC2::Error::BadParameter; } - // Now that the release fences have been generated by the compositor, make - // sure they're managed properly - for (std::pair<const uint32_t, DrmHwcTwo::HwcLayer *> &l : z_map) { - l.second->manage_release_fence(); - AddFenceToRetireFence(l.second->release_fence()); - } - // The retire fence returned here is for the last frame, so return it and // promote the next retire fence *retire_fence = retire_fence_.Release();
Add support for out-fences through the OUT_FENCE_PTR property. Out-fences signal when their associated buffer may be read by a device. Signed-off-by: Robert Foss <robert.foss@collabora.com> --- Changes since v1: Sergi Granell - Set atomic property to be out_fences[crtc->pipe()] not out_fences[0] drmdisplaycomposition.h | 9 +++++++++ drmdisplaycompositor.cpp | 16 ++++++++++++++++ drmhwctwo.cpp | 9 ++------- 3 files changed, 27 insertions(+), 7 deletions(-)