ozone: Implement zero/one-copy texture for Ozone GBM.

ui/ozone/platform/drm/common/client_native_pixmap_factory_gbm.cc

Index: ui/ozone/platform/drm/common/client_native_pixmap_factory_gbm.cc
diff --git a/ui/ozone/platform/drm/common/client_native_pixmap_factory_gbm.cc b/ui/ozone/platform/drm/common/client_native_pixmap_factory_gbm.cc
index e12c31d36287c8b04733dff81cd1573fc4dee9c5..b3462a244ea63e38f800a044720822bef0fea3cf 100644
--- a/ui/ozone/platform/drm/common/client_native_pixmap_factory_gbm.cc
+++ b/ui/ozone/platform/drm/common/client_native_pixmap_factory_gbm.cc
@@ -8,6 +8,11 @@
 #include "ui/gfx/native_pixmap_handle_ozone.h"
 #include "ui/ozone/public/client_native_pixmap_factory.h"
 
+#if defined(OZONE_USE_VGEM_MAP)
+#include <fcntl.h>
+#include "ui/ozone/platform/drm/gpu/client_native_pixmap_vgem.h"
+#endif
+
 namespace ui {
 
 namespace {
@@ -27,7 +32,15 @@ class ClientNativePixmapGbm : public ClientNativePixmap {
 
 class ClientNativePixmapFactoryGbm : public ClientNativePixmapFactory {
  public:
-  ClientNativePixmapFactoryGbm() {}
+  ClientNativePixmapFactoryGbm() {
+#if defined(OZONE_USE_VGEM_MAP)
+    // TODO(dshwang): remove ad-hoc file open. crrev.com/1248713002
+    static const char kVgemPath[] = "/dev/dri/renderD129";
+    int vgem_fd = open(kVgemPath, O_RDWR | O_CLOEXEC);
+    vgem_fd_.reset(vgem_fd);
+    DCHECK_GE(vgem_fd_.get(), 0) << "Failed to open: " << kVgemPath;
+#endif
+  }
   ~ClientNativePixmapFactoryGbm() override {}
 
   // ClientNativePixmapFactory:
@@ -37,6 +50,12 @@ class ClientNativePixmapFactoryGbm : public ClientNativePixmapFactory {
         {gfx::BufferFormat::RGBX_8888, gfx::BufferUsage::SCANOUT}};
     std::vector<Configuration> configurations(
         kConfiguratioins, kConfiguratioins + arraysize(kConfiguratioins));
+#if defined(OZONE_USE_VGEM_MAP)
+    // Map requires VGEM supports.
+    DCHECK_GE(vgem_fd_.get(), 0);

[reveman, 2015/08/12 22:32:25]

nit: you already have this DCHECK in the ctor. I think you can remove it and the
comment

[dshwang, 2015/08/13 11:30:55]

Done.

+    configurations.push_back(
+        {gfx::BufferFormat::BGRA_8888, gfx::BufferUsage::MAP});
+#endif
     return configurations;
   }
   scoped_ptr<ClientNativePixmap> ImportFromHandle(
@@ -44,11 +63,44 @@ class ClientNativePixmapFactoryGbm : public ClientNativePixmapFactory {
       const gfx::Size& size,
       gfx::BufferFormat format,

[reveman, 2015/08/12 22:32:25]

Format is not used for anything but a DCHECK. Can we remove it or make sure it's
passed to where it might be used?

[dshwang, 2015/08/13 11:30:55]

remove |format| in this API

       gfx::BufferUsage usage) override {
+    DCHECK(IsConfigurationSupported(format, usage));
+    DCHECK(handle.fd.auto_close);
     base::ScopedFD close_fd(handle.fd.fd);
-    return make_scoped_ptr<ClientNativePixmapGbm>(new ClientNativePixmapGbm);
+
+    switch (usage) {

[reveman, 2015/08/12 22:32:25]

To keep things simple, I'd ignore the usage here and simply create a
ClientNativePixmapVgem in a vgem supported build and ClientNativePixmapGbm
otherwise but the current code is fine with me.

[dshwang, 2015/08/13 11:30:55]

Done.

+      case gfx::BufferUsage::MAP:
+        break;

[spang, 2015/08/12 21:40:03]

This is pretty strange control flow - can you move the code below right here? A
function would work too.

[dshwang, 2015/08/13 11:30:55]

Done.

+      case gfx::BufferUsage::PERSISTENT_MAP:

[reveman, 2015/08/12 22:32:25]

nit: PERSISTENT_MAP is not supported. NOTREACHED here?

[dshwang, 2015/08/13 11:30:55]

As changing control flow, PERSISTENT_MAP branch disappears

+      case gfx::BufferUsage::SCANOUT:
+        return make_scoped_ptr<ClientNativePixmapGbm>(
+            new ClientNativePixmapGbm);
+    }

[reveman, 2015/08/12 22:32:25]

nit: please NOTREACHED for when usage is an invalid enum value. fixing the
control flow as spang suggested should take care of this.

[dshwang, 2015/08/13 11:30:55]

Done.

+
+#if defined(OZONE_USE_VGEM_MAP)
+    DCHECK_GE(vgem_fd_.get(), 0);
+    return ClientNativePixmapVgem::Create(
+        handle, base::FileDescriptor(vgem_fd_.get(), false), size);
+#else
+    NOTREACHED();
+    return nullptr;
+#endif
   }
 
  private:
+  bool IsConfigurationSupported(gfx::BufferFormat format,

[spang, 2015/08/12 21:40:03]

I'd suggest removing this to keep the signal/noise ratio high. It doesn't seem
like a very valuable assertion.

[reveman, 2015/08/12 22:32:25]

I agree, especially if we ignore the |usage| and |format| in ImportFromHandle.
Removing this and related DCHECKs sgtm.

[dshwang, 2015/08/13 11:30:55]

Done.

+                                gfx::BufferUsage usage) {
+    std::vector<Configuration> configurations(GetSupportedConfigurations());
+    for (auto& configuration : configurations) {
+      if (configuration.format == format && configuration.usage == usage)
+        return true;
+    }
+    return false;
+  }
+
+#if defined(OZONE_USE_VGEM_MAP)
+  base::ScopedFD vgem_fd_;
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(ClientNativePixmapFactoryGbm);
 };