ozone: Allocate scanout buffer according to KMS modifiers

ui/ozone/platform/drm/gpu/gbm_buffer.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/gbm_buffer.h"
 
 #include <drm.h>
 #include <fcntl.h>
 #include <gbm.h>
 #include <xf86drm.h>
@@ skipping 12 matching lines @@
 #include "ui/ozone/platform/drm/gpu/gbm_surfaceless.h"
 #include "ui/ozone/public/ozone_platform.h"
 #include "ui/ozone/public/surface_factory_ozone.h"
 
 namespace ui {
 
 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)) {}
 
 GbmBuffer::~GbmBuffer() {
   if (bo())
     gbm_bo_destroy(bo());
 }
 
 bool GbmBuffer::AreFdsValid() const {
@@ skipping 73 matching lines @@
         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, 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;
+
   return buffer;
 }
 
 // static
 scoped_refptr<GbmBuffer> GbmBuffer::CreateBufferFromFds(
     const scoped_refptr<GbmDevice>& gbm,
     uint32_t format,
     const gfx::Size& size,
     std::vector<base::ScopedFD>&& fds,
     const std::vector<gfx::NativePixmapPlane>& planes) {
@@ skipping 24 matching lines @@
     if (!bo) {
       LOG(ERROR) << "nullptr returned from gbm_bo_import";
       return nullptr;
     }
   }
 
   uint32_t flags = GBM_BO_USE_RENDERING;
   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())
     return nullptr;
 
   return buffer;
 }
 
 GbmPixmap::GbmPixmap(GbmSurfaceFactory* surface_manager,
                      const scoped_refptr<GbmBuffer>& buffer)
@@ skipping 108 matching lines @@
   DCHECK(!processing_callback_.is_null());
   if (!processing_callback_.Run(this, processed_pixmap_)) {
     LOG(ERROR) << "Failed processing NativePixmap";
     return nullptr;
   }
 
   return processed_pixmap_->buffer();
 }
 
 }  // namespace ui