Re: [PATCH hwc v2 6/6] drm_hwcomposer: Add out-fence support

[Robert Foss <robert.foss@collabora.com>]

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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel