[3/4][Ozone-Drm] Use numeric ID to map to callback

ui/ozone/platform/drm/gpu/drm_device.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/ozone/platform/drm/gpu/drm_device.h"
 
 #include <fcntl.h>
 #include <sys/mman.h>
 #include <unistd.h>
 #include <xf86drm.h>
@@ skipping 11 matching lines @@
 #include "ui/ozone/platform/drm/gpu/hardware_display_plane_manager_legacy.h"
 
 #if defined(USE_DRM_ATOMIC)
 #include "ui/ozone/platform/drm/gpu/hardware_display_plane_manager_atomic.h"
 #endif
 
 namespace ui {
 
 namespace {
 
+typedef base::Callback<void(uint32_t /* frame */,
+                            uint32_t /* seconds */,
+                            uint32_t /* useconds */,
+                            uint64_t /* id */)> DrmEventHandler;
+
 struct PageFlipPayload {
   PageFlipPayload(const scoped_refptr<base::TaskRunner>& task_runner,
                   const DrmDevice::PageFlipCallback& callback)
       : task_runner(task_runner), callback(callback) {}
 
   // Task runner for the thread scheduling the page flip event. This is used to
   // run the callback on the same thread the callback was created on.
   scoped_refptr<base::TaskRunner> task_runner;
   DrmDevice::PageFlipCallback callback;
 };
@@ skipping 24 matching lines @@
   return true;
 }
 
 void DrmDestroyDumbBuffer(int fd, uint32_t handle) {
   struct drm_mode_destroy_dumb destroy_request;
   memset(&destroy_request, 0, sizeof(destroy_request));
   destroy_request.handle = handle;
   drmIoctl(fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy_request);
 }
 
-void HandlePageFlipEventOnIO(int fd,
-                             unsigned int frame,
-                             unsigned int seconds,
-                             unsigned int useconds,
-                             void* data) {
-  scoped_ptr<PageFlipPayload> payload(static_cast<PageFlipPayload*>(data));
-  payload->task_runner->PostTask(
-      FROM_HERE, base::Bind(payload->callback, frame, seconds, useconds));
-}
+void ProcessDrmEvent(int fd, const DrmEventHandler& callback) {
+  char buffer[1024];
+  int len = read(fd, buffer, sizeof(buffer));
+  if (len == 0)
+    return;
 
-void HandlePageFlipEventOnUI(int fd,
-                             unsigned int frame,
-                             unsigned int seconds,
-                             unsigned int useconds,
-                             void* data) {
-  scoped_ptr<PageFlipPayload> payload(static_cast<PageFlipPayload*>(data));
-  payload->callback.Run(frame, seconds, useconds);
+  if (len < static_cast<int>(sizeof(drm_event))) {
+    PLOG(ERROR) << "Failed to read DRM event";
+    return;
+  }
+
+  int idx = 0;
+  while (idx < len) {
+    drm_event* event = reinterpret_cast<drm_event*>(&buffer[idx]);

[spang, 2015/04/27 17:23:38]

You can't cast a char* to an arbitrary struct. It's undefined behavior.

You can memcpy() into a drm_event instead.

[dnicoara, 2015/04/27 20:22:22]

Why would it be undefined behavior to cast between 2 pointer types? Are you
afraid of bounds checks? DRM is supposed to guarantee complete events, but I can
add some checks.
Seems silly to copy the event twice.

[dnicoara, 2015/04/28 14:42:49]

Done.

+    switch (event->type) {
+      case DRM_EVENT_FLIP_COMPLETE: {
+        drm_event_vblank* vblank = reinterpret_cast<drm_event_vblank*>(event);
+        callback.Run(vblank->sequence, vblank->tv_sec, vblank->tv_usec,
+                     vblank->user_data);
+      } break;
+      case DRM_EVENT_VBLANK:
+        break;
+      default:
+        NOTREACHED();
+        break;
+    }
+
+    idx += event->length;
+  }
 }
 
 bool CanQueryForResources(int fd) {
   drm_mode_card_res resources;
   memset(&resources, 0, sizeof(resources));
   // If there is no error getting DRM resources then assume this is a
   // modesetting device.
   return !drmIoctl(fd, DRM_IOCTL_MODE_GETRESOURCES, &resources);
 }
 
 }  // namespace
 
+class DrmDevice::PageFlipManager
+    : public base::RefCountedThreadSafe<DrmDevice::PageFlipManager> {
+ public:
+  PageFlipManager() : next_id_(0) {}
+  ~PageFlipManager() {}
+
+  void OnPageFlip(uint32_t frame,
+                  uint32_t seconds,
+                  uint32_t useconds,
+                  uint64_t id) {
+    auto it = callback_map_.find(id);
+    if (it == callback_map_.end()) {
+      LOG(WARNING) << "Could not find callback for page flip id=" << id;
+      return;
+    }
+
+    it->second.Run(frame, seconds, useconds);
+    callback_map_.erase(it);
+  }
+
+  uint64_t GetNextId() { return next_id_++; }
+
+  void RegisterCallback(uint64_t id,
+                        const DrmDevice::PageFlipCallback& callback) {
+    DCHECK(callback_map_.find(id) == callback_map_.end());
+    callback_map_.insert(std::make_pair(id, callback));
+  }
+
+ private:
+  uint64_t next_id_;
+
+  std::map<uint64_t, DrmDevice::PageFlipCallback> callback_map_;

[spang, 2015/04/27 17:23:38]

vector?

[dnicoara, 2015/04/27 20:22:22]

Why vector? ... I suppose the number of callbacks registered won't be large, but
this is easier to read.

[spang, 2015/04/27 20:53:42]

You'll only have 1-3 elements right? vector is a better choice in that case.

[dnicoara, 2015/04/28 14:42:48]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(PageFlipManager);
+};
+
 class DrmDevice::IOWatcher
     : public base::RefCountedThreadSafe<DrmDevice::IOWatcher>,
       public base::MessagePumpLibevent::Watcher {
  public:
   IOWatcher(int fd,
-            const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner)
-      : io_task_runner_(io_task_runner), paused_(true), fd_(fd) {}
+            const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
+            const scoped_refptr<DrmDevice::PageFlipManager>& page_flip_manager)
+      : main_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+        io_task_runner_(io_task_runner),
+        page_flip_manager_(page_flip_manager),
+        paused_(true),
+        fd_(fd) {}
 
   void SetPaused(bool paused) {
     if (paused_ == paused)
       return;
 
     paused_ = paused;
     base::WaitableEvent done(false, false);
     io_task_runner_->PostTask(
         FROM_HERE, base::Bind(&IOWatcher::SetPausedOnIO, this, &done));
     done.Wait();
@@ skipping 23 matching lines @@
 
   void SetPausedOnIO(base::WaitableEvent* done) {
     DCHECK(base::MessageLoopForIO::IsCurrent());
     if (paused_)
       UnregisterOnIO();
     else
       RegisterOnIO();
     done->Signal();
   }
 
+  void OnPageFlipOnIO(uint32_t frame,
+                      uint32_t seconds,
+                      uint32_t useconds,
+                      uint64_t id) {
+    main_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&DrmDevice::PageFlipManager::OnPageFlip, page_flip_manager_,
+                   frame, seconds, useconds, id));
+  }
+
   // base::MessagePumpLibevent::Watcher overrides:
   void OnFileCanReadWithoutBlocking(int fd) override {
     DCHECK(base::MessageLoopForIO::IsCurrent());
     TRACE_EVENT1("drm", "OnDrmEvent", "socket", fd);
 
-    drmEventContext event;
-    event.version = DRM_EVENT_CONTEXT_VERSION;
-    event.page_flip_handler = HandlePageFlipEventOnIO;
-    event.vblank_handler = nullptr;
-
-    drmHandleEvent(fd, &event);
+    ProcessDrmEvent(fd,
+                    base::Bind(&DrmDevice::IOWatcher::OnPageFlipOnIO, this));
   }
 
   void OnFileCanWriteWithoutBlocking(int fd) override { NOTREACHED(); }
 
+  scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
 
+  scoped_refptr<DrmDevice::PageFlipManager> page_flip_manager_;
+
   base::MessagePumpLibevent::FileDescriptorWatcher controller_;
 
   bool paused_;
   int fd_;
 
   DISALLOW_COPY_AND_ASSIGN(IOWatcher);
 };
 
 DrmDevice::DrmDevice(const base::FilePath& device_path)
     : device_path_(device_path),
       file_(device_path,
             base::File::FLAG_OPEN | base::File::FLAG_READ |
-                base::File::FLAG_WRITE) {
+                base::File::FLAG_WRITE),
+      page_flip_manager_(new PageFlipManager()) {
   LOG_IF(FATAL, !file_.IsValid())
       << "Failed to open '" << device_path_.value()
       << "': " << base::File::ErrorToString(file_.error_details());
 }
 
 DrmDevice::DrmDevice(const base::FilePath& device_path, base::File file)
-    : device_path_(device_path), file_(file.Pass()) {
+    : device_path_(device_path),
+      file_(file.Pass()),
+      page_flip_manager_(new PageFlipManager()) {
 }
 
 DrmDevice::~DrmDevice() {
   if (watcher_)
     watcher_->Shutdown();
 }
 
 bool DrmDevice::Initialize() {
   // Ignore devices that cannot perform modesetting.
   if (!CanQueryForResources(file_.GetPlatformFile())) {
@@ skipping 14 matching lines @@
     return false;
   }
 
   return true;
 }
 
 void DrmDevice::InitializeTaskRunner(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
   DCHECK(!task_runner_);
   task_runner_ = task_runner;
-  watcher_ = new IOWatcher(file_.GetPlatformFile(), task_runner_);
+  watcher_ =
+      new IOWatcher(file_.GetPlatformFile(), task_runner_, page_flip_manager_);
 }
 
 ScopedDrmCrtcPtr DrmDevice::GetCrtc(uint32_t crtc_id) {
   DCHECK(file_.IsValid());
   return ScopedDrmCrtcPtr(drmModeGetCrtc(file_.GetPlatformFile(), crtc_id));
 }
 
 bool DrmDevice::SetCrtc(uint32_t crtc_id,
                         uint32_t framebuffer,
                         std::vector<uint32_t> connectors,
@@ skipping 63 matching lines @@
                          const PageFlipCallback& callback) {
   DCHECK(file_.IsValid());
   TRACE_EVENT2("drm", "DrmDevice::PageFlip", "crtc", crtc_id, "framebuffer",
                framebuffer);
 
   if (watcher_)
     watcher_->SetPaused(is_sync);
 
   // NOTE: Calling drmModeSetCrtc will immediately update the state, though
   // callbacks to already scheduled page flips will be honored by the kernel.
-  scoped_ptr<PageFlipPayload> payload(
-      new PageFlipPayload(base::ThreadTaskRunnerHandle::Get(), callback));
+  uint64_t id = page_flip_manager_->GetNextId();
   if (!drmModePageFlip(file_.GetPlatformFile(), crtc_id, framebuffer,
-                       DRM_MODE_PAGE_FLIP_EVENT, payload.get())) {
+                       DRM_MODE_PAGE_FLIP_EVENT, reinterpret_cast<void*>(id))) {
     // If successful the payload will be removed by a PageFlip event.
-    ignore_result(payload.release());
+    page_flip_manager_->RegisterCallback(id, callback);
 
     // If the flip was requested synchronous or if no watcher has been installed
     // yet, then synchronously handle the page flip events.
     if (is_sync || !watcher_) {
       TRACE_EVENT1("drm", "OnDrmEvent", "socket", file_.GetPlatformFile());
 
-      drmEventContext event;
-      event.version = DRM_EVENT_CONTEXT_VERSION;
-      event.page_flip_handler = HandlePageFlipEventOnUI;
-      event.vblank_handler = nullptr;
-
-      drmHandleEvent(file_.GetPlatformFile(), &event);
+      ProcessDrmEvent(
+          file_.GetPlatformFile(),
+          base::Bind(&PageFlipManager::OnPageFlip, page_flip_manager_));
     }
 
     return true;
   }
 
   return false;
 }
 
 bool DrmDevice::PageFlipOverlay(uint32_t crtc_id,
                                 uint32_t framebuffer,
@@ skipping 132 matching lines @@
   DCHECK(file_.IsValid());
   return (drmSetMaster(file_.GetPlatformFile()) == 0);
 }
 
 bool DrmDevice::DropMaster() {
   DCHECK(file_.IsValid());
   return (drmDropMaster(file_.GetPlatformFile()) == 0);
 }
 
 }  // namespace ui