[2/4]Allow hotplugging of primary DRM device

ui/ozone/platform/drm/host/drm_native_display_delegate.cc

Index: ui/ozone/platform/drm/host/drm_native_display_delegate.cc
diff --git a/ui/ozone/platform/drm/host/drm_native_display_delegate.cc b/ui/ozone/platform/drm/host/drm_native_display_delegate.cc
index b31b720351fa0e78c543425c4691755f60e67780..4dfc48f57cbe4765a396e863850e3a4ce2888ba0 100644
--- a/ui/ozone/platform/drm/host/drm_native_display_delegate.cc
+++ b/ui/ozone/platform/drm/host/drm_native_display_delegate.cc
@@ -5,6 +5,7 @@
 #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"
@@ -33,21 +34,44 @@ const char* kDisplayActionString[] = {
     "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);
+
+    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());
+      return file.Pass();
+    }
 
-  base::File::Info info;
-  file.GetInfo(&info);
+    break;

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

??

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

Oops, removed.

+    if (IsAuthenticated(file.GetPlatformFile()))
+      break;
 
-  CHECK(!info.is_directory);
-  CHECK(path.DirName() == base::FilePath("/dev/dri"));
+    LOG_IF(WARNING, print_warning) << "Failed to authenticate " << path.value();
 
-  if (!file.IsValid()) {
-    LOG(ERROR) << "Failed to open " << path.value() << ": "
-               << base::File::ErrorToString(file.error_details());
+    print_warning = false;
+    usleep(100000);
   }
 
+  VLOG(1) << "Succeeded authenticating " << path.value();
   return file.Pass();
 }
 
@@ -315,6 +339,19 @@ void DrmNativeDisplayDelegate::OnChannelEstablished(
     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());