[2/4]Allow hotplugging of primary DRM device

ui/ozone/platform/drm/host/drm_native_display_delegate.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/host/drm_native_display_delegate.h"
 
 #include <stdio.h>
+#include <xf86drm.h>
 
 #include "base/logging.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/threading/thread_restrictions.h"
 #include "ui/display/types/display_snapshot.h"
 #include "ui/display/types/native_display_observer.h"
 #include "ui/events/ozone/device/device_event.h"
 #include "ui/events/ozone/device/device_manager.h"
 #include "ui/ozone/common/display_snapshot_proxy.h"
 #include "ui/ozone/common/display_util.h"
 #include "ui/ozone/common/gpu/ozone_gpu_messages.h"
 #include "ui/ozone/platform/drm/host/display_manager.h"
 #include "ui/ozone/platform/drm/host/drm_gpu_platform_support_host.h"
 
 namespace ui {
 
 namespace {
 
 typedef base::Callback<void(const base::FilePath&, base::File)>
     OnOpenDeviceReplyCallback;
 
 const char* kDisplayActionString[] = {
     "ADD",
     "REMOVE",
     "CHANGE",
 };
 
+bool IsAuthenticated(int fd) {

[spang, 2015/04/22 18:50:58]

A function called IsBlah() should not have side effects like this.

s/IsAuthenticated/Authenticate/

[dnicoara, 2015/04/22 19:10:01]

Done.

+  drm_magic_t magic;
+  memset(&magic, 0, sizeof(magic));

[spang, 2015/04/22 18:50:58]

Seems like you can do "= 0;" instead of memset..

[dnicoara, 2015/04/22 19:10:01]

Done.

+  // We need to make sure the DRM device has enough privilege. Use the DRM
+  // authentication logic to figure out if the device has enough permissions.
+  return !drmGetMagic(fd, &magic) && !drmAuthMagic(fd, magic);
+}
+
 base::File OpenDrmDevice(const base::FilePath& path) {
-  base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ |
-                            base::File::FLAG_WRITE);
+  base::File file;
+  bool print_warning = true;
+  while (true) {
+    file = base::File(path, base::File::FLAG_OPEN | base::File::FLAG_READ |
+                                base::File::FLAG_WRITE);
 
-  base::File::Info info;
-  file.GetInfo(&info);
+    base::File::Info info;
+    file.GetInfo(&info);
 
-  CHECK(!info.is_directory);
-  CHECK(path.DirName() == base::FilePath("/dev/dri"));
+    CHECK(!info.is_directory);
+    CHECK(path.DirName() == base::FilePath("/dev/dri"));
 
-  if (!file.IsValid()) {
-    LOG(ERROR) << "Failed to open " << path.value() << ": "
-               << base::File::ErrorToString(file.error_details());
+    if (!file.IsValid()) {
+      LOG(ERROR) << "Failed to open " << path.value() << ": "
+                 << base::File::ErrorToString(file.error_details());
+      return file.Pass();
+    }
+
+    break;

[spang, 2015/04/22 18:50:58]

??

[dnicoara, 2015/04/22 19:10:01]

Oops, removed.

+    if (IsAuthenticated(file.GetPlatformFile()))
+      break;
+
+    LOG_IF(WARNING, print_warning) << "Failed to authenticate " << path.value();
+
+    print_warning = false;
+    usleep(100000);
   }
 
+  VLOG(1) << "Succeeded authenticating " << path.value();
   return file.Pass();
 }
 
 void OpenDeviceOnWorkerThread(
     const base::FilePath& path,
     const scoped_refptr<base::TaskRunner>& reply_runner,
     const OnOpenDeviceReplyCallback& callback) {
   base::File file = OpenDrmDevice(path);
   reply_runner->PostTask(FROM_HERE,
                          base::Bind(callback, path, base::Passed(file.Pass())));
@@ skipping 247 matching lines @@
   FOR_EACH_OBSERVER(NativeDisplayObserver, observers_,
                     OnConfigurationChanged());
 }
 
 void DrmNativeDisplayDelegate::OnChannelEstablished(
     int host_id,
     scoped_refptr<base::SingleThreadTaskRunner> send_runner,
     const base::Callback<void(IPC::Message*)>& send_callback) {
   drm_devices_.clear();
   drm_devices_.insert(primary_graphics_card_path_);
+  {
+    // First device needs to be treated specially. We need to open this
+    // synchronously since the GPU process will need it to initialize the
+    // graphics state.

[spang, 2015/04/22 18:50:58]

How does sync open in the browser help the GPU process?

(what goes wrong if you omit this part?)

[dnicoara, 2015/04/22 19:10:01]

The UI thread creates a window right away which creates a Compositor, which, in
turn, will IPC a surface creation message. We need to have the primary device
open and registered before the surface creation IPC arrives in the GPU process.

If we omit, the device is opened later via the worker thread and by that time
the Compositor will have already IPC-ed to the GPU process to create a surface
(and caused the GPU process to crash since we don't have a device to allocate
with).

+    base::ThreadRestrictions::ScopedAllowIO allow_io;
+    base::File file = OpenDrmDevice(primary_graphics_card_path_);
+    if (!file.IsValid()) {
+      LOG(FATAL) << "Failed to open primary graphics card";
+      return;
+    }
+    OnAddGraphicsDevice(primary_graphics_card_path_, file.Pass());
+  }
+
   device_manager_->ScanDevices(this);
   FOR_EACH_OBSERVER(NativeDisplayObserver, observers_,
                     OnConfigurationChanged());
 }
 
 void DrmNativeDisplayDelegate::OnChannelDestroyed(int host_id) {
   // If the channel got destroyed in the middle of a configuration then just
   // respond with failure.
   if (!get_displays_callback_.is_null()) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
@@ skipping 48 matching lines @@
     configure_callback_map_.erase(it);
   }
 }
 
 void DrmNativeDisplayDelegate::RunUpdateDisplaysCallback(
     const GetDisplaysCallback& callback) const {
   callback.Run(displays_.get());
 }
 
 }  // namespace ui