Commit 57af9203 authored by James Cook's avatar James Cook Committed by Commit Bot

Revert "ozone/drm: Use minigbm mmap for modeset buffers"

This reverts commit b7eae9f9.

Reason for revert: May be causing crashes in desktopui_MashLogin autotest in the Chrome OS lab. See crbug.com/882073

 0  libminigbm.so.1.0.0!gbm_bo_destroy [gbm.c : 153 + 0x0]
 1  chrome!gbm_wrapper::Buffer::~Buffer() [gbm_wrapper.cc : 121 + 0x5]
 2  chrome!ui::GbmPixmap::~GbmPixmap() [memory : 2321 + 0x5]
 3  chrome!gl::GLImageNativePixmap::~GLImageNativePixmap()  4  chrome!std::__1::__vector_base<gpu::gles2::Texture::LevelInfo, std::__1::allocator<gpu::gles2::Texture::LevelInfo> >::~__vector_base() [ref_counted.h : 352 + 0x3]
 5  chrome!gpu::gles2::Texture::~Texture() [texture_manager.cc : 652 + 0x9]
 6  chrome!gpu::gles2::Texture::RemoveTextureRef(gpu::gles2::TextureRef*, bool) [texture_manager.cc : 602 + 0x8]

Original change's description:
> ozone/drm: Use minigbm mmap for modeset buffers
> 
> FillModesetBuffer assumed all the buffers could be mmaped as dumb
> buffers.
> This doesn't work on some platforms.
> 
> This CL mmaps the modeset buffer using gbm_map instead, that should
> work with tiled formats too.
> 
> Bug: b/78892556
> Change-Id: I20ff9e54ea12e9866360021d8d9c5f7940f5b551
> Reviewed-on: https://chromium-review.googlesource.com/1198451
> Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
> Reviewed-by: Michael Spang <spang@chromium.org>
> Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#589556}

TBR=spang@chromium.org,dnicoara@chromium.org,dcastagna@chromium.org,ddavenport@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: b/78892556
Change-Id: I7ceefac024f76d8f7b945ead19a36fb4a6a2e34a
Reviewed-on: https://chromium-review.googlesource.com/1217022Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589959}
parent 9122f39b
...@@ -7,13 +7,10 @@ ...@@ -7,13 +7,10 @@
#include <inttypes.h> #include <inttypes.h>
#include "third_party/skia/include/core/SkRefCnt.h"
#include "ui/gfx/buffer_types.h" #include "ui/gfx/buffer_types.h"
#include "ui/gfx/geometry/size.h" #include "ui/gfx/geometry/size.h"
#include "ui/gfx/native_pixmap_handle.h" #include "ui/gfx/native_pixmap_handle.h"
class SkSurface;
namespace ui { namespace ui {
class GbmBuffer { class GbmBuffer {
...@@ -37,7 +34,6 @@ class GbmBuffer { ...@@ -37,7 +34,6 @@ class GbmBuffer {
virtual size_t GetPlaneSize(size_t plane) const = 0; virtual size_t GetPlaneSize(size_t plane) const = 0;
virtual uint32_t GetHandle() const = 0; virtual uint32_t GetHandle() const = 0;
virtual gfx::NativePixmapHandle ExportHandle() const = 0; virtual gfx::NativePixmapHandle ExportHandle() const = 0;
virtual sk_sp<SkSurface> GetSurface() = 0;
}; };
} // namespace ui } // namespace ui
......
...@@ -9,7 +9,6 @@ ...@@ -9,7 +9,6 @@
#include <xf86drm.h> #include <xf86drm.h>
#include "base/posix/eintr_wrapper.h" #include "base/posix/eintr_wrapper.h"
#include "third_party/skia/include/core/SkSurface.h"
#include "ui/gfx/buffer_format_util.h" #include "ui/gfx/buffer_format_util.h"
#include "ui/ozone/common/linux/drm_util_linux.h" #include "ui/ozone/common/linux/drm_util_linux.h"
#include "ui/ozone/common/linux/gbm_buffer.h" #include "ui/ozone/common/linux/gbm_buffer.h"
...@@ -119,10 +118,7 @@ class Buffer final : public ui::GbmBuffer { ...@@ -119,10 +118,7 @@ class Buffer final : public ui::GbmBuffer {
size_(size), size_(size),
planes_(std::move(planes)) {} planes_(std::move(planes)) {}
~Buffer() override { ~Buffer() override { gbm_bo_destroy(bo_); }
DCHECK(!mmap_data_);
gbm_bo_destroy(bo_);
}
uint32_t GetFormat() const override { return format_; } uint32_t GetFormat() const override { return format_; }
uint64_t GetFormatModifier() const override { return format_modifier_; } uint64_t GetFormatModifier() const override { return format_modifier_; }
...@@ -188,24 +184,8 @@ class Buffer final : public ui::GbmBuffer { ...@@ -188,24 +184,8 @@ class Buffer final : public ui::GbmBuffer {
return handle; return handle;
} }
sk_sp<SkSurface> GetSurface() override {
DCHECK(!mmap_data_);
uint32_t stride;
void* addr;
addr = gbm_bo_map(bo_, 0, 0, gbm_bo_get_width(bo_), gbm_bo_get_height(bo_),
GBM_BO_TRANSFER_READ_WRITE, &stride, &mmap_data_, 0);
if (!addr)
return nullptr;
SkImageInfo info =
SkImageInfo::MakeN32Premul(size_.width(), size_.height());
return SkSurface::MakeRasterDirectReleaseProc(info, addr, stride,
&Buffer::UnmapGbmBo, this);
}
private: private:
gbm_bo* bo_ = nullptr; gbm_bo* bo_ = nullptr;
void* mmap_data_ = nullptr;
uint32_t format_ = 0; uint32_t format_ = 0;
uint64_t format_modifier_ = 0; uint64_t format_modifier_ = 0;
...@@ -217,12 +197,6 @@ class Buffer final : public ui::GbmBuffer { ...@@ -217,12 +197,6 @@ class Buffer final : public ui::GbmBuffer {
std::vector<gfx::NativePixmapPlane> planes_; std::vector<gfx::NativePixmapPlane> planes_;
static void UnmapGbmBo(void* pixels, void* context) {
Buffer* buffer = static_cast<Buffer*>(context);
gbm_bo_unmap(buffer->bo_, buffer->mmap_data_);
buffer->mmap_data_ = nullptr;
}
DISALLOW_COPY_AND_ASSIGN(Buffer); DISALLOW_COPY_AND_ASSIGN(Buffer);
}; };
......
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#include "base/logging.h" #include "base/logging.h"
#include "base/numerics/safe_math.h" #include "base/numerics/safe_math.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/core/SkSurface.h"
#include "ui/ozone/common/linux/drm_util_linux.h" #include "ui/ozone/common/linux/drm_util_linux.h"
#include "ui/ozone/common/linux/gbm_buffer.h" #include "ui/ozone/common/linux/gbm_buffer.h"
...@@ -70,8 +69,6 @@ class MockGbmBuffer final : public ui::GbmBuffer { ...@@ -70,8 +69,6 @@ class MockGbmBuffer final : public ui::GbmBuffer {
return gfx::NativePixmapHandle(); return gfx::NativePixmapHandle();
} }
sk_sp<SkSurface> GetSurface() override { return nullptr; }
private: private:
uint32_t format_ = 0; uint32_t format_ = 0;
uint64_t format_modifier_ = 0; uint64_t format_modifier_ = 0;
......
...@@ -15,7 +15,6 @@ ...@@ -15,7 +15,6 @@
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
#include "ui/gfx/geometry/size.h" #include "ui/gfx/geometry/size.h"
#include "ui/gfx/gpu_fence.h" #include "ui/gfx/gpu_fence.h"
#include "ui/gfx/skia_util.h"
#include "ui/ozone/common/linux/gbm_buffer.h" #include "ui/ozone/common/linux/gbm_buffer.h"
#include "ui/ozone/platform/drm/common/drm_util.h" #include "ui/ozone/platform/drm/common/drm_util.h"
#include "ui/ozone/platform/drm/gpu/crtc_controller.h" #include "ui/ozone/platform/drm/gpu/crtc_controller.h"
...@@ -33,7 +32,13 @@ namespace { ...@@ -33,7 +32,13 @@ namespace {
// to the new modeset buffer |buffer|. // to the new modeset buffer |buffer|.
void FillModesetBuffer(const scoped_refptr<DrmDevice>& drm, void FillModesetBuffer(const scoped_refptr<DrmDevice>& drm,
HardwareDisplayController* controller, HardwareDisplayController* controller,
GbmBuffer* buffer) { DrmFramebuffer* buffer) {
DrmConsoleBuffer modeset_buffer(drm, buffer->opaque_framebuffer_id());
if (!modeset_buffer.Initialize()) {
VLOG(2) << "Failed to grab framebuffer " << buffer->opaque_framebuffer_id();
return;
}
DCHECK(!controller->crtc_controllers().empty()); DCHECK(!controller->crtc_controllers().empty());
CrtcController* first_crtc = controller->crtc_controllers()[0].get(); CrtcController* first_crtc = controller->crtc_controllers()[0].get();
ScopedDrmCrtcPtr saved_crtc(drm->GetCrtc(first_crtc->crtc())); ScopedDrmCrtcPtr saved_crtc(drm->GetCrtc(first_crtc->crtc()));
...@@ -42,7 +47,7 @@ void FillModesetBuffer(const scoped_refptr<DrmDevice>& drm, ...@@ -42,7 +47,7 @@ void FillModesetBuffer(const scoped_refptr<DrmDevice>& drm,
return; return;
} }
uint32_t fourcc_format = buffer->GetFormat(); uint32_t fourcc_format = buffer->framebuffer_pixel_format();
const auto& modifiers = controller->GetFormatModifiers(fourcc_format); const auto& modifiers = controller->GetFormatModifiers(fourcc_format);
for (const uint64_t modifier : modifiers) { for (const uint64_t modifier : modifiers) {
// A value of 0 means DRM_FORMAT_MOD_NONE. If the CRTC has any other // A value of 0 means DRM_FORMAT_MOD_NONE. If the CRTC has any other
...@@ -66,20 +71,15 @@ void FillModesetBuffer(const scoped_refptr<DrmDevice>& drm, ...@@ -66,20 +71,15 @@ void FillModesetBuffer(const scoped_refptr<DrmDevice>& drm,
// Don't copy anything if the sizes mismatch. This can happen when the user // Don't copy anything if the sizes mismatch. This can happen when the user
// changes modes. // changes modes.
if (saved_buffer.canvas()->getBaseLayerSize() != if (saved_buffer.canvas()->getBaseLayerSize() !=
SizeToSkISize(buffer->GetSize())) { modeset_buffer.canvas()->getBaseLayerSize()) {
VLOG(2) << "Previous buffer has a different size than modeset buffer"; VLOG(2) << "Previous buffer has a different size than modeset buffer";
return; return;
} }
sk_sp<SkSurface> surface = buffer->GetSurface();
if (!surface) {
VLOG(2) << "Can't get a SkSurface from the modeset gbm buffer.";
return;
}
SkPaint paint; SkPaint paint;
// Copy the source buffer. Do not perform any blending. // Copy the source buffer. Do not perform any blending.
paint.setBlendMode(SkBlendMode::kSrc); paint.setBlendMode(SkBlendMode::kSrc);
surface->getCanvas()->drawImage(saved_buffer.image(), 0, 0, &paint); modeset_buffer.canvas()->drawImage(saved_buffer.image(), 0, 0, &paint);
} }
CrtcController* GetCrtcController(HardwareDisplayController* controller, CrtcController* GetCrtcController(HardwareDisplayController* controller,
...@@ -400,7 +400,7 @@ DrmOverlayPlane ScreenManager::GetModesetBuffer( ...@@ -400,7 +400,7 @@ DrmOverlayPlane ScreenManager::GetModesetBuffer(
LOG(ERROR) << "Failed to add framebuffer for scanout buffer"; LOG(ERROR) << "Failed to add framebuffer for scanout buffer";
return DrmOverlayPlane::Error(); return DrmOverlayPlane::Error();
} }
FillModesetBuffer(drm, controller, buffer.get()); FillModesetBuffer(drm, controller, framebuffer.get());
return DrmOverlayPlane(framebuffer, nullptr); return DrmOverlayPlane(framebuffer, nullptr);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment