On Tue, Apr 17, 2018 at 12:37:15PM -0400, Sean Paul wrote:
> On Wed, Apr 11, 2018 at 04:22:23PM +0100, Alexandru Gheorghe wrote:
> > There is a lot of boilerplate for creating an initialized
> > drmdisplaycomposition. This patch gathers that in a separate method.
> >
> > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@xxxxxxx>
> > ---
> > drmdisplaycompositor.cpp | 23 +++++++++++++++++++++++
> > drmdisplaycompositor.h | 2 ++
> > 2 files changed, 25 insertions(+)
> >
> > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> > index e556e86..6e5be24 100644
> > --- a/drmdisplaycompositor.cpp
> > +++ b/drmdisplaycompositor.cpp
> > @@ -221,6 +221,7 @@ int DrmDisplayCompositor::Init(DrmResources *drm, int display) {
> > ALOGE("Failed to initialize drm compositor lock %d\n", ret);
> > return ret;
> > }
> > + planner_ = Planner::CreateInstance(drm);
>
> What's this?
We need a planner for the function bellow, I could re-write this to be
an argument of CreateComposition if you think that helps.
>
> >
> > initialized_ = true;
> > return 0;
> > @@ -231,6 +232,28 @@ std::unique_ptr<DrmDisplayComposition> DrmDisplayCompositor::CreateComposition()
> > return std::unique_ptr<DrmDisplayComposition>(new DrmDisplayComposition());
> > }
> >
> > +std::unique_ptr<DrmDisplayComposition>
> > +DrmDisplayCompositor::CreateInitializedComposition() const {
> > + DrmCrtc *crtc = drm_->GetCrtcForDisplay(display_);
> > + if (!crtc) {
> > + ALOGE("Failed to find crtc for display = %d", display_);
> > + return std::unique_ptr<DrmDisplayComposition>();
> > + }
> > + std::unique_ptr<DrmDisplayComposition> comp = CreateComposition();
> > + std::shared_ptr<Importer> importer =
> > + drm_->resource_manager()->GetImporter(display_);
> > + if (!importer) {
> > + ALOGE("Failed to find resources for display = %d", display_);
> > + return std::unique_ptr<DrmDisplayComposition>();
> > + }
> > + int ret = comp->Init(drm_, crtc, importer.get(), planner_.get(), 0);
> > + if (ret) {
> > + ALOGE("Failed to init composition for display = %d", display_);
> > + return std::unique_ptr<DrmDisplayComposition>();
> > + }
> > + return comp;
> > +}
> > +
>
> This seems sufficiently small that you can squash it into the patch that uses
> it. The same can be said for some of the other "Add function to do X" which
> don't use the function.
>
> > std::tuple<uint32_t, uint32_t, int>
> > DrmDisplayCompositor::GetActiveModeResolution() {
> > DrmConnector *connector = drm_->GetConnectorForDisplay(display_);
> > diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
> > index f1965fb..ccaffb4 100644
> > --- a/drmdisplaycompositor.h
> > +++ b/drmdisplaycompositor.h
> > @@ -87,6 +87,7 @@ class DrmDisplayCompositor {
> > int Init(DrmResources *drm, int display);
> >
> > std::unique_ptr<DrmDisplayComposition> CreateComposition() const;
> > + std::unique_ptr<DrmDisplayComposition> CreateInitializedComposition() const;
> > int ApplyComposition(std::unique_ptr<DrmDisplayComposition> composition);
> > int Composite();
> > int SquashAll();
> > @@ -155,6 +156,7 @@ class DrmDisplayCompositor {
> > // we need to reset them on every Dump() call.
> > mutable uint64_t dump_frames_composited_;
> > mutable uint64_t dump_last_timestamp_ns_;
> > + std::unique_ptr<Planner> planner_;
> > };
> > }
> >
> > --
> > 2.7.4
> >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
--
Cheers,
Alex G
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
