Re: [PATCH hwc v2 10/18] drm_hwcomposer: hwcutils: Add function for cloning a DrmHwcLayer

[Sean Paul <seanpaul@chromium.org>]

On Wed, Apr 11, 2018 at 04:22:21PM +0100, Alexandru Gheorghe wrote:
> When doing flattening of a composition on a different CRTC we need to be
> able to clone a layer in order to import it and then pass it to another CRTC.
> 
> Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> ---
>  drmhwcomposer.h |  1 +
>  hwcutils.cpp    | 11 +++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drmhwcomposer.h b/drmhwcomposer.h
> index f8440fb..b256caf 100644
> --- a/drmhwcomposer.h
> +++ b/drmhwcomposer.h
> @@ -150,6 +150,7 @@ struct DrmHwcLayer {
>  
>    int InitFromHwcLayer(hwc_layer_1_t *sf_layer, Importer *importer,
>                         const gralloc_module_t *gralloc);
> +  int PopulateFromDrmHwcLayer(DrmHwcLayer *layer);
>    int ImportBuffer(Importer *importer, const gralloc_module_t *gralloc);
>  
>    void SetTransform(int32_t sf_transform);
> diff --git a/hwcutils.cpp b/hwcutils.cpp
> index 53a7d82..ff37c3b 100644
> --- a/hwcutils.cpp
> +++ b/hwcutils.cpp
> @@ -149,6 +149,17 @@ int DrmHwcLayer::InitFromHwcLayer(hwc_layer_1_t *sf_layer, Importer *importer,
>    return ImportBuffer(importer, gralloc);
>  }
>  
> +int DrmHwcLayer::PopulateFromDrmHwcLayer(DrmHwcLayer *src_layer) {
> +  blending = src_layer->blending;
> +  sf_handle = src_layer->sf_handle;
> +  acquire_fence = dup(src_layer->acquire_fence.get());

Hmm, I think this is the only place where we duplicate a UniqueFd. I _think_
this will be Ok, but do you need to use this? Could you instead defer trying to
flatten if the original acquire_fence hasn't fired? Also, you don't check the
return value.

> +  display_frame = src_layer->display_frame;
> +  alpha = src_layer->alpha;
> +  source_crop = src_layer->source_crop;
> +  transform = src_layer->transform;

It also doesn't seem like you're populating all of the fields so the function
name is misleading.


> +  return 0;
> +}
> +
>  int DrmHwcLayer::ImportBuffer(Importer *importer,
>                                const gralloc_module_t *gralloc) {
>    int ret = buffer.ImportBuffer(sf_handle, importer);
> -- 
> 2.7.4
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel