Re: [PATCH hwc v2 15/18] drm_hwcomposer: Add worker to trigger scene flattenning

[Sean Paul <seanpaul@chromium.org>]

On Wed, Apr 11, 2018 at 04:22:26PM +0100, Alexandru Gheorghe wrote:
> Add a vsync worker that calls back into the DrmDisplayCompositor,
> for now at every 60 vsyncs if the scene does not change we trigger
> the flattening of the scene using the writeback connector.
> Other, more complex and proper heuristics could be implemented later
> on.
> 
> Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> ---
>  drmdisplaycompositor.cpp | 45 ++++++++++++++++++++++++++++++++++++++++++---
>  drmdisplaycompositor.h   | 12 +++++++++++-
>  2 files changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index 576539b..e535e8a 100644
> --- a/drmdisplaycompositor.cpp
> +++ b/drmdisplaycompositor.cpp
> @@ -39,6 +39,20 @@
>  
>  namespace android {
>  
> +class CompositorVsyncCallback : public VsyncCallback {
> + public:
> +  CompositorVsyncCallback(DrmDisplayCompositor *compositor)
> +      : compositor_(compositor) {
> +  }
> +
> +  void Callback(int display, int64_t timestamp) {
> +    compositor_->Vsync(display, timestamp);
> +  }
> +
> + private:
> +  DrmDisplayCompositor *compositor_;
> +};
> +
>  void SquashState::Init(DrmHwcLayer *layers, size_t num_layers) {
>    generation_number_++;
>    valid_history_ = 0;
> @@ -183,7 +197,8 @@ DrmDisplayCompositor::DrmDisplayCompositor()
>        framebuffer_index_(0),
>        squash_framebuffer_index_(0),
>        dump_frames_composited_(0),
> -      dump_last_timestamp_ns_(0) {
> +      dump_last_timestamp_ns_(0),
> +      flatten_countdown_(FLATTEN_COUNTDOWN_INIT) {
>    struct timespec ts;
>    if (clock_gettime(CLOCK_MONOTONIC, &ts))
>      return;
> @@ -193,7 +208,7 @@ DrmDisplayCompositor::DrmDisplayCompositor()
>  DrmDisplayCompositor::~DrmDisplayCompositor() {
>    if (!initialized_)
>      return;
> -
> +  vsync_worker_.Exit();
>    int ret = pthread_mutex_lock(&lock_);
>    if (ret)
>      ALOGE("Failed to acquire compositor lock %d", ret);
> @@ -222,7 +237,9 @@ int DrmDisplayCompositor::Init(DrmResources *drm, int display) {
>      return ret;
>    }
>    planner_ = Planner::CreateInstance(drm);
> -
> +  vsync_worker_.Init(drm_, display_);
> +  auto callback = std::make_shared<CompositorVsyncCallback>(this);
> +  vsync_worker_.RegisterCallback(callback);
>    initialized_ = true;
>    return 0;
>  }
> @@ -896,6 +913,10 @@ int DrmDisplayCompositor::ApplyComposition(
>    return ret;
>  }
>  
> +int DrmDisplayCompositor::FlattenScene() {
> +  return -EINVAL;
> +}

Hmm... not sure this is a useful inclusion.

> +
>  int DrmDisplayCompositor::SquashAll() {
>    AutoLock lock(&lock_, "compositor");
>    int ret = lock.Lock();
> @@ -1044,6 +1065,24 @@ move_layers_back:
>    return ret;
>  }
>  
> +bool DrmDisplayCompositor::CountdownExpired() const {
> +  return flatten_countdown_ <= 0;
> +}
> +
> +void DrmDisplayCompositor::Vsync(int display, int64_t timestamp) {
> +  AutoLock lock(&lock_, __FUNCTION__);

We use __func__ elsewhere, we should probably be consistent (same goes for other
instances).

> +  if (lock.Lock())
> +    return;
> +  flatten_countdown_--;
> +  if (CountdownExpired()) {

Doing:

if (--flatten_countdown_ > 0)
  return;

Allows you to remove the CountdownExpired() function and save a level of
indentation for the rest of the function.


> +    lock.Unlock();
> +    int ret = FlattenScene();
> +    ALOGI("scene flattening triggered for display %d at timestamp %" PRIu64
> +          " result = %d \n",
> +          display, timestamp, ret);
> +  }
> +}
> +
>  void DrmDisplayCompositor::Dump(std::ostringstream *out) const {
>    int ret = pthread_mutex_lock(&lock_);
>    if (ret)
> diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
> index b35ef70..26201b9 100644
> --- a/drmdisplaycompositor.h
> +++ b/drmdisplaycompositor.h
> @@ -29,11 +29,16 @@
>  
>  #include <hardware/hardware.h>
>  #include <hardware/hwcomposer.h>
> +#include <vsyncworker.h>

"vsyncworker.h"

>  
>  // One for the front, one for the back, and one for cases where we need to
>  // squash a frame that the hw can't display with hw overlays.
>  #define DRM_DISPLAY_BUFFERS 3
>  
> +// If a scene is still for this number of vblanks flatten it to reduce power
> +// consumption.
> +#define FLATTEN_COUNTDOWN_INIT 60
> +
>  namespace android {
>  
>  class GLWorkerCompositor;
> @@ -92,7 +97,7 @@ class DrmDisplayCompositor {
>    int Composite();
>    int SquashAll();
>    void Dump(std::ostringstream *out) const;
> -
> +  void Vsync(int display, int64_t timestamp);
>    std::tuple<uint32_t, uint32_t, int> GetActiveModeResolution();
>  
>    SquashState *squash_state() {
> @@ -128,6 +133,9 @@ class DrmDisplayCompositor {
>    void ClearDisplay();
>    void ApplyFrame(std::unique_ptr<DrmDisplayComposition> composition,
>                    int status, bool writeback = false);
> +  int FlattenScene();
> +
> +  bool CountdownExpired() const;
>  
>    std::tuple<int, uint32_t> CreateModeBlob(const DrmMode &mode);
>  
> @@ -157,6 +165,8 @@ class DrmDisplayCompositor {
>    // we need to reset them on every Dump() call.
>    mutable uint64_t dump_frames_composited_;
>    mutable uint64_t dump_last_timestamp_ns_;
> +  VSyncWorker vsync_worker_;
> +  int64_t flatten_countdown_;
>    std::unique_ptr<Planner> planner_;
>  };
>  }
> -- 
> 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