[PATCH hwc v2 12/18] drm_hwcomposer: Add utility function to create an initialized composition

[Sean Paul]

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 at arm.com>
> ---
>  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?

>  
>    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