ozone: Allocate scanout buffer according to KMS modifiers

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

Index: ui/ozone/platform/drm/gpu/gbm_buffer.cc
diff --git a/ui/ozone/platform/drm/gpu/gbm_buffer.cc b/ui/ozone/platform/drm/gpu/gbm_buffer.cc
index de628ac8007d89274d1472f5118bc4c130671000..528e4ad595e41ba5a8e49c1ca82a0b7253402205 100644
--- a/ui/ozone/platform/drm/gpu/gbm_buffer.cc
+++ b/ui/ozone/platform/drm/gpu/gbm_buffer.cc
@@ -30,12 +30,14 @@ GbmBuffer::GbmBuffer(const scoped_refptr<GbmDevice>& gbm,
                      gbm_bo* bo,
                      uint32_t format,
                      uint32_t flags,
+                     uint64_t modifier,
                      std::vector<base::ScopedFD>&& fds,
                      const gfx::Size& size,
                      const std::vector<gfx::NativePixmapPlane>&& planes)
     : GbmBufferBase(gbm, bo, format, flags),
       format_(format),
       flags_(flags),
+      modifier_(modifier),
       fds_(std::move(fds)),
       size_(size),
       planes_(std::move(planes)) {}
@@ -129,7 +131,57 @@ scoped_refptr<GbmBuffer> GbmBuffer::CreateBuffer(
         gbm_bo_get_plane_size(bo, i), gbm_bo_get_plane_format_modifier(bo, i));
   }
   scoped_refptr<GbmBuffer> buffer(new GbmBuffer(
-      gbm, bo, format, flags, std::move(fds), size, std::move(planes)));
+      gbm, bo, format, flags, 0, std::move(fds), size, std::move(planes)));
+  if (flags & GBM_BO_USE_SCANOUT && !buffer->GetFramebufferId())
+    return nullptr;
+
+  return buffer;
+}
+
+// static
+scoped_refptr<GbmBuffer> GbmBuffer::CreateBufferWithModifiers(

[dnicoara, 2016/12/19 19:44:46]

It seems that only the gbm_bo_create()/gbm_bo_create_with_modifiers() is
different between CreateBuffer() and CreateBufferWithModifiers(); would it be
possible to have a helper function to share most of the common code?

[Daniele Castagna, 2016/12/20 18:03:28]

Maybe we could have just GbmBuffer::CreateBuffer that always takes a vector
modifiers.
We can then change behavior of this method based on modifiers.lenght here,
basically moving the logic that is now in DrmThread::CreateBuffer to this
method.

[hoegsberg1, 2016/12/22 23:05:20]

I wanted to keep the two paths separate: either you allocate with flags or with
modifiers. But since a few places check for (flags & SCANOUT) to see if a buffer
is created for scanout, I decided to take both flags and modifiers in the
modifier case.

Longer term we should move away from flags to modifiers, so I wanted to avoid
conflating the two code paths. I suppose we can refactor towards that later on,
so it's not too important.

The important thing that's missing here is that we can't actually create the FB
with modifiers if we didn't create the bo with modifiers. So I need to pass a
bool into the GbmBufferBase constructor to indicate whether or not to pass the
modifiers there...

+    const scoped_refptr<GbmDevice>& gbm,
+    uint32_t format,
+    const gfx::Size& size,
+    uint32_t flags,
+    const std::vector<uint64_t>& modifiers) {
+  TRACE_EVENT2("drm", "GbmBuffer::CreateBufferWithModifiers", "device",
+               gbm->device_path().value(), "size", size.ToString());
+
+  gbm_bo* bo =
+      gbm_bo_create_with_modifiers(gbm->device(), size.width(), size.height(),
+                                   format, &modifiers[0], modifiers.size());

[dnicoara, 2016/12/19 19:44:46]

nit: Prefer modifiers.data().

[hoegsberg1, 2016/12/22 23:05:20]

Acknowledged.

+  if (!bo)
+    return nullptr;
+
+  uint64_t modifier = gbm_bo_get_format_modifier(bo);
+
+  std::vector<base::ScopedFD> fds;
+  std::vector<gfx::NativePixmapPlane> planes;
+
+  for (size_t i = 0; i < gbm_bo_get_num_planes(bo); ++i) {
+    // The fd returned by gbm_bo_get_fd is not ref-counted and need to be
+    // kept open for the lifetime of the buffer.
+    base::ScopedFD fd(gbm_bo_get_plane_fd(bo, i));
+
+    // TODO(dcastagna): support multiple fds.
+    // crbug.com/642410
+    if (!i) {
+      if (!fd.is_valid()) {
+        PLOG(ERROR) << "Failed to export buffer to dma_buf";
+        gbm_bo_destroy(bo);
+        return nullptr;
+      }
+      fds.emplace_back(std::move(fd));
+    }
+
+    planes.emplace_back(
+        gbm_bo_get_plane_stride(bo, i), gbm_bo_get_plane_offset(bo, i),
+        gbm_bo_get_plane_size(bo, i), gbm_bo_get_plane_format_modifier(bo, i));
+  }
+  scoped_refptr<GbmBuffer> buffer(new GbmBuffer(gbm, bo, format, flags,
+                                                modifier, std::move(fds), size,
+                                                std::move(planes)));
   if (flags & GBM_BO_USE_SCANOUT && !buffer->GetFramebufferId())
     return nullptr;
 
@@ -177,7 +229,7 @@ scoped_refptr<GbmBuffer> GbmBuffer::CreateBufferFromFds(
   if (use_scanout)
     flags |= GBM_BO_USE_SCANOUT;
   scoped_refptr<GbmBuffer> buffer(new GbmBuffer(
-      gbm, bo, format, flags, std::move(fds), size, std::move(planes)));
+      gbm, bo, format, 0, flags, std::move(fds), size, std::move(planes)));
   // If scanout support for buffer is expected then make sure we managed to
   // create a framebuffer for it as otherwise using it for scanout will fail.
   if (use_scanout && !buffer->GetFramebufferId())