commit a5df1defa12bc38ed2e5c9cb2fdef85fe7ea0cd7
author Sean Paul <seanpaul@chromium.org> Wed Feb 10 10:00:08 2016 -0800
committer Robert Foss <robert.foss@collabora.com> Tue Feb 28 13:53:06 2017 -0500
tree 818eb50bdc149a05007a939b117d47c986335457
parent 8600e347e14b42872b1c2762d51b27102ddc40f9

drm_hwcomposer: Don't pass hwc_procs_t to VsyncWorker

Introduce a new class to limit the hwc_procs_t callback
structure scope to hwcomposer.cpp

Change-Id: I68ec62e7947ca87702b3d6d0169ae850cfbf5d85
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Robert Foss <robert.foss@collabora.com>

hwcomposer.cpp

diff --git a/hwcomposer.cpp b/hwcomposer.cpp
index b2e9643..4bc4670 100644
--- a/hwcomposer.cpp
+++ b/hwcomposer.cpp
@@ -299,6 +299,18 @@
   }
 }
 
+class DrmVsyncCallback : public VsyncCallback {
+ public:
+  DrmVsyncCallback(hwc_procs_t const *procs) : procs_(procs) {
+  }
+
+  void Callback(int display, int64_t timestamp) {
+    procs_->vsync(procs_, display, timestamp);
+  }
+ private:
+  hwc_procs_t const *procs_;
+};
+
 int DrmHwcLayer::InitFromHwcLayer(hwc_layer_1_t *sf_layer, Importer *importer,
                                   const gralloc_module_t *gralloc) {
   sf_handle = sf_layer->handle;
@@ -683,8 +695,10 @@
 
   ctx->procs = procs;
 
-  for (std::pair<const int, hwc_drm_display> &display_entry : ctx->displays)
-    display_entry.second.vsync_worker.SetProcs(procs);
+  for (std::pair<const int, hwc_drm_display> &display_entry : ctx->displays) {
+    auto callback = std::make_shared<DrmVsyncCallback>(procs);
+    display_entry.second.vsync_worker.RegisterCallback(std::move(callback));
+  }
 
   ctx->hotplug_handler.Init(&ctx->drm, procs);
   ctx->drm.event_listener()->RegisterHotplugHandler(&ctx->hotplug_handler);

vsyncworker.cpp

diff --git a/vsyncworker.cpp b/vsyncworker.cpp
index 29709f7..cc9c96b 100644
--- a/vsyncworker.cpp
+++ b/vsyncworker.cpp
@@ -34,7 +34,6 @@
 VSyncWorker::VSyncWorker()
     : Worker("vsync", HAL_PRIORITY_URGENT_DISPLAY),
       drm_(NULL),
-      procs_(NULL),
       display_(-1),
       last_timestamp_(-1) {
 }
@@ -49,14 +48,14 @@
   return InitWorker();
 }
 
-int VSyncWorker::SetProcs(hwc_procs_t const *procs) {
+int VSyncWorker::RegisterCallback(std::shared_ptr<VsyncCallback> callback) {
   int ret = Lock();
   if (ret) {
     ALOGE("Failed to lock vsync worker lock %d\n", ret);
     return ret;
   }
 
-  procs_ = procs;
+  callback_ = callback;
 
   ret = Unlock();
   if (ret) {
@@ -151,7 +150,7 @@
 
   bool enabled = enabled_;
   int display = display_;
-  hwc_procs_t const *procs = procs_;
+  std::shared_ptr<VsyncCallback> callback(callback_);
 
   ret = Unlock();
   if (ret) {
@@ -188,16 +187,16 @@
   }
 
   /*
-   * There's a race here where a change in procs_ will not take effect until
+   * There's a race here where a change in callback_ will not take effect until
    * the next subsequent requested vsync. This is unavoidable since we can't
    * call the vsync hook while holding the thread lock.
    *
-   * We could shorten the race window by caching procs_ right before calling
-   * the hook. However, in practice, procs_ is only updated once, so it's not
+   * We could shorten the race window by caching callback_ right before calling
+   * the hook. However, in practice, callback_ is only updated once, so it's not
    * worth the overhead.
    */
-  if (procs && procs->vsync)
-    procs->vsync(procs, display, timestamp);
+  if (callback)
+    callback->Callback(display, timestamp);
   last_timestamp_ = timestamp;
 }
 }

vsyncworker.h

diff --git a/vsyncworker.h b/vsyncworker.h
index 98ac546..a1ba1a5 100644
--- a/vsyncworker.h
+++ b/vsyncworker.h
@@ -28,13 +28,20 @@
 
 namespace android {
 
+class VsyncCallback {
+ public:
+  virtual ~VsyncCallback() {
+  }
+  virtual void Callback(int display, int64_t timestamp) = 0;
+};
+
 class VSyncWorker : public Worker {
  public:
   VSyncWorker();
   ~VSyncWorker() override;
 
   int Init(DrmResources *drm, int display);
-  int SetProcs(hwc_procs_t const *procs);
+  int RegisterCallback(std::shared_ptr<VsyncCallback> callback);
 
   int VSyncControl(bool enabled);
 
@@ -46,7 +53,11 @@
   int SyntheticWaitVBlank(int64_t *timestamp);
 
   DrmResources *drm_;
-  hwc_procs_t const *procs_;
+
+  // shared_ptr since we need to use this outside of the thread lock (to
+  // actually call the hook) and we don't want the memory freed until we're
+  // done
+  std::shared_ptr<VsyncCallback> callback_ = NULL;
 
   int display_;
   bool enabled_;