Re: [RFC PATCH hwc] drm_hwcomposer: set CRTC background color when available

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

Hey Stefan,

On 02/22/2018 04:54 AM, Stefan Schake wrote:
> Android assumes an implicit black background layer is always present
> behind all layers it specifies for composition. drm_hwcomposer currently
> punts responsibility for this to the kernel/DRM platform and puts layers
> with per-pixel alpha content on the primary plane when requested.

I wasn't aware of this assumption, but given that it is the case,
the patch looks like a good fix for the problem.

Unfortunately I don't have a hardware platform up and running to test the patch 
with at the moment.

> 
> On some platforms (e.g. VC4) a background color fill has a cycle cost for
> the hardware composer and is not enabled by default. Instead, userland can
> request a background color through a CRTC property. Use this property to
> specify the implicit black background Android expects.
> 
> Signed-off-by: Stefan Schake <stschake@gmail.com>
> ---
> Kernel changes for this (background_color) are available here:
> 
> https://github.com/stschake/linux/commits/background-upstream
> 
> Sending as RFC because I'm not entirely clear on whose responsibility
> this should be, on most DRM drivers it seems to be implicit. I think
> a case could also be made that VC4 should not accept alpha formats on
> the lowest layer or enable background color fill when given one anyway.
> On the other hand, userland control over background color seems desirable
> regardless and is a feature of multiple hardware composers (i915, vc4, omap).
> 
>   drmcrtc.cpp              | 11 +++++++++++
>   drmcrtc.h                |  2 ++
>   drmdisplaycompositor.cpp | 15 +++++++++++++++
>   3 files changed, 28 insertions(+)
> 
> diff --git a/drmcrtc.cpp b/drmcrtc.cpp
> index 1b354fe..d7bcd7a 100644
> --- a/drmcrtc.cpp
> +++ b/drmcrtc.cpp
> @@ -52,6 +52,13 @@ int DrmCrtc::Init() {
>       ALOGE("Failed to get OUT_FENCE_PTR property");
>       return ret;
>     }
> +
> +  ret = drm_->GetCrtcProperty(*this, "background_color",
> +                              &background_color_property_);
> +  if (ret) {
> +    ALOGI("Failed to get background_color property");
> +    // This is an optional property
> +  }
>     return 0;
>   }
>   
> @@ -86,4 +93,8 @@ const DrmProperty &DrmCrtc::mode_property() const {
>   const DrmProperty &DrmCrtc::out_fence_ptr_property() const {
>     return out_fence_ptr_property_;
>   }
> +
> +const DrmProperty &DrmCrtc::background_color_property() const {
> +  return background_color_property_;
> +}
>   }
> diff --git a/drmcrtc.h b/drmcrtc.h
> index c5a5599..704573a 100644
> --- a/drmcrtc.h
> +++ b/drmcrtc.h
> @@ -46,6 +46,7 @@ class DrmCrtc {
>     const DrmProperty &active_property() const;
>     const DrmProperty &mode_property() const;
>     const DrmProperty &out_fence_ptr_property() const;
> +  const DrmProperty &background_color_property() const;
>   
>    private:
>     DrmResources *drm_;
> @@ -59,6 +60,7 @@ class DrmCrtc {
>     DrmProperty active_property_;
>     DrmProperty mode_property_;
>     DrmProperty out_fence_ptr_property_;
> +  DrmProperty background_color_property_;
>   };
>   }
>   
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index e556e86..69c52dd 100644
> --- a/drmdisplaycompositor.cpp
> +++ b/drmdisplaycompositor.cpp
> @@ -527,6 +527,21 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
>         return ret;
>       }
>   
> +    if (crtc->background_color_property().id() != 0) {
> +      // Background color is specified as a 16 bit per channel RGBA value.
> +      // Set a fully opaque black background as Android HWC expects.
> +      const uint64_t background_color = 0xFFFF;
> +
> +      ret = drmModeAtomicAddProperty(pset, crtc->id(),
> +                                     crtc->background_color_property().id(),
> +                                     background_color) < 0;
> +      if (ret) {
> +        ALOGE("Failed to add CRTC background_color to pset: %d", ret);
> +        drmModeAtomicFree(pset);
> +        return ret;
> +      }
> +    }
> +
>       ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->mode_property().id(),
>                                      mode_.blob_id) < 0 ||
>             drmModeAtomicAddProperty(pset, connector->id(),
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel