Commit 31dcb64a authored by Saman Sami's avatar Saman Sami Committed by Commit Bot

Fix non-existent AcceleratedWidgets in DrmThread

Tasks posted from the GPU thread might reference AcceleratedWidgets
that don't exist yet because their registered via a different mojo
pipe coming from the Browser's UI thread. When this happens,
postpone running these tasks until the AcceleratedWidget is registered.

Bug: 620927
Change-Id: I6f399d5a8069fa6a6333bece1d9fffea5eb2f0a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1664655
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: default avatarMichael Spang <spang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672603}
parent 573a9db8
......@@ -177,6 +177,7 @@ source_set("gbm_unittests") {
"common/drm_overlay_manager_unittest.cc",
"common/drm_util_unittest.cc",
"gpu/drm_overlay_validator_unittest.cc",
"gpu/drm_thread_unittest.cc",
"gpu/drm_window_unittest.cc",
"gpu/hardware_display_controller_unittest.cc",
"gpu/hardware_display_plane_manager_unittest.cc",
......
......@@ -19,7 +19,6 @@
#include "ui/gfx/presentation_feedback.h"
#include "ui/ozone/common/linux/drm_util_linux.h"
#include "ui/ozone/common/linux/gbm_device.h"
#include "ui/ozone/common/linux/gbm_wrapper.h"
#include "ui/ozone/platform/drm/common/drm_util.h"
#include "ui/ozone/platform/drm/gpu/drm_device_generator.h"
#include "ui/ozone/platform/drm/gpu/drm_device_manager.h"
......@@ -84,34 +83,14 @@ void CreateBufferWithGbmFlags(const scoped_refptr<DrmDevice>& drm,
*out_framebuffer = std::move(framebuffer);
}
class GbmDeviceGenerator : public DrmDeviceGenerator {
public:
GbmDeviceGenerator() {}
~GbmDeviceGenerator() override {}
// DrmDeviceGenerator:
scoped_refptr<DrmDevice> CreateDevice(const base::FilePath& path,
base::File file,
bool is_primary_device) override {
auto gbm = CreateGbmDevice(file.GetPlatformFile());
if (!gbm) {
PLOG(ERROR) << "Unable to initialize GBM for " << path.value();
return nullptr;
}
auto drm = base::MakeRefCounted<DrmDevice>(
path, std::move(file), is_primary_device, std::move(gbm));
if (!drm->Initialize())
return nullptr;
return drm;
}
} // namespace
private:
DrmThread::TaskInfo::TaskInfo(base::OnceClosure task, base::WaitableEvent* done)
: task(std::move(task)), done(done) {}
DISALLOW_COPY_AND_ASSIGN(GbmDeviceGenerator);
};
DrmThread::TaskInfo::TaskInfo(TaskInfo&& other) = default;
} // namespace
DrmThread::TaskInfo::~TaskInfo() = default;
DrmThread::DrmThread() : base::Thread("DrmThread"), weak_ptr_factory_(this) {}
......@@ -119,8 +98,11 @@ DrmThread::~DrmThread() {
Stop();
}
void DrmThread::Start(base::OnceClosure binding_completer) {
void DrmThread::Start(base::OnceClosure binding_completer,
std::unique_ptr<DrmDeviceGenerator> device_generator) {
complete_early_binding_requests_ = std::move(binding_completer);
device_generator_ = std::move(device_generator);
base::Thread::Options thread_options;
thread_options.message_loop_type = base::MessageLoop::TYPE_IO;
thread_options.priority = base::ThreadPriority::DISPLAY;
......@@ -129,11 +111,22 @@ void DrmThread::Start(base::OnceClosure binding_completer) {
LOG(FATAL) << "Failed to create DRM thread";
}
void DrmThread::RunTaskAfterWindowReady(gfx::AcceleratedWidget window,
base::OnceClosure task,
base::WaitableEvent* done) {
if (!device_manager_->GetDrmDevices().empty() &&
window <= last_created_window_) {
std::move(task).Run();
if (done)
done->Signal();
return;
}
pending_tasks_[window].emplace_back(std::move(task), done);
}
void DrmThread::Init() {
device_manager_.reset(
new DrmDeviceManager(std::make_unique<GbmDeviceGenerator>()));
device_manager_.reset(new DrmDeviceManager(std::move(device_generator_)));
screen_manager_.reset(new ScreenManager());
display_manager_.reset(
new DrmGpuDisplayManager(screen_manager_.get(), device_manager_.get()));
......@@ -249,10 +242,16 @@ void DrmThread::IsDeviceAtomic(gfx::AcceleratedWidget widget, bool* is_atomic) {
}
void DrmThread::CreateWindow(gfx::AcceleratedWidget widget) {
DCHECK_GT(widget, last_created_window_);
last_created_window_ = widget;
std::unique_ptr<DrmWindow> window(
new DrmWindow(widget, device_manager_.get(), screen_manager_.get()));
window->Initialize();
screen_manager_->AddWindow(widget, std::move(window));
// There might be tasks that were waiting for |widget| to become available.
ProcessPendingTasks();
}
void DrmThread::DestroyWindow(gfx::AcceleratedWidget widget) {
......@@ -325,6 +324,9 @@ void DrmThread::RelinquishDisplayControl(
void DrmThread::AddGraphicsDevice(const base::FilePath& path, base::File file) {
device_manager_->AddDrmDevice(path, std::move(file));
// There might be tasks that were blocked on a DrmDevice becoming available.
ProcessPendingTasks();
}
void DrmThread::RemoveGraphicsDevice(const base::FilePath& path) {
......@@ -370,4 +372,20 @@ void DrmThread::AddBindingDrmDevice(ozone::mojom::DrmDeviceRequest request) {
drm_bindings_.AddBinding(this, std::move(request));
}
void DrmThread::ProcessPendingTasks() {
DCHECK(!device_manager_->GetDrmDevices().empty());
auto it = pending_tasks_.begin();
for (; it != pending_tasks_.end() && it->first <= last_created_window_;
++it) {
for (auto& task_info : it->second) {
std::move(task_info.task).Run();
if (task_info.done)
task_info.done->Signal();
}
}
pending_tasks_.erase(pending_tasks_.begin(), it);
}
} // namespace ui
......@@ -19,6 +19,7 @@
#include "ui/gfx/vsync_provider.h"
#include "ui/ozone/common/gpu/ozone_gpu_message_params.h"
#include "ui/ozone/platform/drm/common/display_types.h"
#include "ui/ozone/platform/drm/gpu/drm_device_generator.h"
#include "ui/ozone/public/interfaces/device_cursor.mojom.h"
#include "ui/ozone/public/interfaces/drm_device.mojom.h"
#include "ui/ozone/public/swap_completion_callback.h"
......@@ -61,7 +62,14 @@ class DrmThread : public base::Thread,
DrmThread();
~DrmThread() override;
void Start(base::OnceClosure binding_completer);
void Start(base::OnceClosure binding_completer,
std::unique_ptr<DrmDeviceGenerator> device_generator);
// Runs |task| once a DrmDevice is registered and |window| was created via
// CreateWindow(). |done| will be signaled if it's not null.
void RunTaskAfterWindowReady(gfx::AcceleratedWidget window,
base::OnceClosure task,
base::WaitableEvent* done);
// Must be called on the DRM thread. All methods for use from the GPU thread.
// DrmThreadProxy (on GPU)thread) is the client for these methods.
......@@ -141,11 +149,24 @@ class DrmThread : public base::Thread,
void Init() override;
private:
struct TaskInfo {
base::OnceClosure task;
base::WaitableEvent* done;
TaskInfo(base::OnceClosure task, base::WaitableEvent* done);
TaskInfo(TaskInfo&& other);
~TaskInfo();
};
void OnPlanesReadyForPageFlip(gfx::AcceleratedWidget widget,
SwapCompletionOnceCallback submission_callback,
PresentationOnceCallback presentation_callback,
std::vector<DrmOverlayPlane> planes);
// Called when a DrmDevice or DrmWindow is created. Runs tasks that are now
// unblocked.
void ProcessPendingTasks();
std::unique_ptr<DrmDeviceManager> device_manager_;
std::unique_ptr<ScreenManager> screen_manager_;
std::unique_ptr<DrmGpuDisplayManager> display_manager_;
......@@ -161,6 +182,17 @@ class DrmThread : public base::Thread,
// mus mode
mojo::BindingSet<ozone::mojom::DrmDevice> drm_bindings_;
// The AcceleratedWidget from the last call to CreateWindow.
gfx::AcceleratedWidget last_created_window_ = gfx::kNullAcceleratedWidget;
// The tasks that are blocked on a DrmDevice and a certain AcceleratedWidget
// becoming available.
base::flat_map<gfx::AcceleratedWidget, std::vector<TaskInfo>> pending_tasks_;
// Holds the DrmDeviceGenerator that DrmDeviceManager will use. Will be passed
// on to DrmDeviceManager after the thread starts.
std::unique_ptr<DrmDeviceGenerator> device_generator_;
base::WeakPtrFactory<DrmThread> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(DrmThread);
......
......@@ -8,6 +8,9 @@
#include <utility>
#include "base/bind.h"
#include "ui/ozone/common/linux/gbm_wrapper.h"
#include "ui/ozone/platform/drm/gpu/drm_device.h"
#include "ui/ozone/platform/drm/gpu/drm_device_generator.h"
#include "ui/ozone/platform/drm/gpu/drm_thread_message_proxy.h"
#include "ui/ozone/platform/drm/gpu/drm_window_proxy.h"
#include "ui/ozone/platform/drm/gpu/gbm_pixmap.h"
......@@ -15,6 +18,36 @@
namespace ui {
namespace {
class GbmDeviceGenerator : public DrmDeviceGenerator {
public:
GbmDeviceGenerator() {}
~GbmDeviceGenerator() override {}
// DrmDeviceGenerator:
scoped_refptr<DrmDevice> CreateDevice(const base::FilePath& path,
base::File file,
bool is_primary_device) override {
auto gbm = CreateGbmDevice(file.GetPlatformFile());
if (!gbm) {
PLOG(ERROR) << "Unable to initialize GBM for " << path.value();
return nullptr;
}
auto drm = base::MakeRefCounted<DrmDevice>(
path, std::move(file), is_primary_device, std::move(gbm));
if (!drm->Initialize())
return nullptr;
return drm;
}
private:
DISALLOW_COPY_AND_ASSIGN(GbmDeviceGenerator);
};
} // namespace
DrmThreadProxy::DrmThreadProxy() = default;
DrmThreadProxy::~DrmThreadProxy() = default;
......@@ -26,7 +59,8 @@ void DrmThreadProxy::BindThreadIntoMessagingProxy(
}
void DrmThreadProxy::StartDrmThread(base::OnceClosure binding_drainer) {
drm_thread_.Start(std::move(binding_drainer));
drm_thread_.Start(std::move(binding_drainer),
std::make_unique<GbmDeviceGenerator>());
}
std::unique_ptr<DrmWindowProxy> DrmThreadProxy::CreateDrmWindowProxy(
......@@ -43,10 +77,13 @@ void DrmThreadProxy::CreateBuffer(gfx::AcceleratedWidget widget,
scoped_refptr<DrmFramebuffer>* framebuffer) {
DCHECK(drm_thread_.task_runner())
<< "no task runner! in DrmThreadProxy::CreateBuffer";
base::OnceClosure task =
base::BindOnce(&DrmThread::CreateBuffer, base::Unretained(&drm_thread_),
widget, size, format, usage, flags, buffer, framebuffer);
PostSyncTask(
drm_thread_.task_runner(),
base::BindOnce(&DrmThread::CreateBuffer, base::Unretained(&drm_thread_),
widget, size, format, usage, flags, buffer, framebuffer));
base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
base::Unretained(&drm_thread_), widget, std::move(task)));
}
void DrmThreadProxy::CreateBufferFromHandle(
......@@ -56,10 +93,13 @@ void DrmThreadProxy::CreateBufferFromHandle(
gfx::NativePixmapHandle handle,
std::unique_ptr<GbmBuffer>* buffer,
scoped_refptr<DrmFramebuffer>* framebuffer) {
PostSyncTask(drm_thread_.task_runner(),
base::BindOnce(&DrmThread::CreateBufferFromHandle,
base::Unretained(&drm_thread_), widget, size,
format, std::move(handle), buffer, framebuffer));
base::OnceClosure task = base::BindOnce(
&DrmThread::CreateBufferFromHandle, base::Unretained(&drm_thread_),
widget, size, format, std::move(handle), buffer, framebuffer);
PostSyncTask(
drm_thread_.task_runner(),
base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
base::Unretained(&drm_thread_), widget, std::move(task)));
}
void DrmThreadProxy::SetClearOverlayCacheCallback(
......@@ -78,12 +118,14 @@ void DrmThreadProxy::CheckOverlayCapabilities(
const std::vector<OverlaySurfaceCandidate>& candidates,
OverlayCapabilitiesCallback callback) {
DCHECK(drm_thread_.task_runner());
base::OnceClosure task = base::BindOnce(
&DrmThread::CheckOverlayCapabilities, base::Unretained(&drm_thread_),
widget, candidates, CreateSafeOnceCallback(std::move(callback)));
drm_thread_.task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&DrmThread::CheckOverlayCapabilities,
base::Unretained(&drm_thread_), widget, candidates,
CreateSafeOnceCallback(std::move(callback))));
FROM_HERE, base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
base::Unretained(&drm_thread_), widget,
std::move(task), nullptr));
}
void DrmThreadProxy::AddBindingCursorDevice(
......
// Copyright 2019 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/drm_thread.h"
#include "base/bind_helpers.h"
#include "base/files/scoped_temp_dir.h"
#include "base/test/scoped_task_environment.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/ozone/platform/drm/gpu/drm_device_generator.h"
#include "ui/ozone/platform/drm/gpu/mock_drm_device.h"
#include "ui/ozone/platform/drm/gpu/mock_gbm_device.h"
namespace ui {
namespace {
class FakeDrmDeviceGenerator : public DrmDeviceGenerator {
// DrmDeviceGenerator:
scoped_refptr<DrmDevice> CreateDevice(const base::FilePath& path,
base::File file,
bool is_primary_device) override {
auto gbm_device = std::make_unique<MockGbmDevice>();
return base::MakeRefCounted<MockDrmDevice>(std::move(gbm_device));
}
};
void StubTask() {}
void StubTaskWithDoneFeedback(bool* done) {
*done = true;
}
} // namespace
class DrmThreadTest : public testing::Test {
protected:
// Overridden from testing::Test
void SetUp() override {
drm_thread_.Start(base::DoNothing(),
std::make_unique<FakeDrmDeviceGenerator>());
drm_thread_.task_runner()->PostTask(
FROM_HERE, base::BindOnce(&DrmThread::AddBindingDrmDevice,
base::Unretained(&drm_thread_),
mojo::MakeRequest(&drm_device_ptr_)));
drm_thread_.FlushForTesting();
}
std::unique_ptr<base::WaitableEvent> PostStubTaskWithWaitableEvent(
gfx::AcceleratedWidget window) {
base::OnceClosure task = base::BindOnce(StubTask);
auto event = std::make_unique<base::WaitableEvent>(
base::WaitableEvent::ResetPolicy::AUTOMATIC,
base::WaitableEvent::InitialState::NOT_SIGNALED);
drm_thread_.task_runner()->PostTask(
FROM_HERE, base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
base::Unretained(&drm_thread_), window,
std::move(task), event.get()));
return event;
}
void PostStubTask(gfx::AcceleratedWidget window, bool* done) {
*done = false;
base::OnceClosure task = base::BindOnce(StubTaskWithDoneFeedback, done);
drm_thread_.task_runner()->PostTask(
FROM_HERE, base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
base::Unretained(&drm_thread_), window,
std::move(task), nullptr));
}
void AddGraphicsDevice() {
base::FilePath file_path("/dev/null");
base::File file(file_path, base::File::FLAG_OPEN | base::File::FLAG_WRITE |
base::File::FLAG_READ);
drm_device_ptr_->AddGraphicsDevice(file_path, std::move(file));
}
base::test::ScopedTaskEnvironment env_;
DrmThread drm_thread_;
ozone::mojom::DrmDevicePtr drm_device_ptr_;
};
TEST_F(DrmThreadTest, RunTaskAfterWindowReady) {
bool called1 = false, called2 = false;
gfx::AcceleratedWidget widget1 = 1, widget2 = 2;
// Post a task not blocked on any window. It should still block on a graphics
// device becoming available.
PostStubTask(gfx::kNullAcceleratedWidget, &called1);
drm_thread_.FlushForTesting();
EXPECT_FALSE(called1);
// Add the graphics device. The task should run.
AddGraphicsDevice();
drm_thread_.FlushForTesting();
ASSERT_TRUE(called1);
// Now that a graphics device is available, further tasks that don't block on
// any window should execute immediately.
PostStubTask(gfx::kNullAcceleratedWidget, &called1);
drm_thread_.FlushForTesting();
ASSERT_TRUE(called1);
// Post a task blocked on |widget1|. It shouldn't run.
PostStubTask(widget1, &called1);
drm_thread_.FlushForTesting();
ASSERT_FALSE(called1);
// Post two tasks blocked on |widget2|, one with a WaitableEvent and one
// without. They shouldn't run.
std::unique_ptr<base::WaitableEvent> event =
PostStubTaskWithWaitableEvent(widget2);
PostStubTask(widget2, &called2);
drm_thread_.FlushForTesting();
ASSERT_FALSE(event->IsSignaled());
ASSERT_FALSE(called2);
// Now create |widget1|. The first task should run.
drm_device_ptr_->CreateWindow(widget1);
drm_thread_.FlushForTesting();
ASSERT_TRUE(called1);
ASSERT_FALSE(event->IsSignaled());
ASSERT_FALSE(called2);
// Now that |widget1| is created. any further task depending on it should run
// immediately.
PostStubTask(widget1, &called1);
drm_thread_.FlushForTesting();
ASSERT_TRUE(called1);
ASSERT_FALSE(event->IsSignaled());
ASSERT_FALSE(called2);
// Destroy |widget1| and post a task blocked on it. The task should still run
// immediately even though the window is destroyed.
drm_device_ptr_->DestroyWindow(widget1);
PostStubTask(widget1, &called1);
drm_thread_.FlushForTesting();
ASSERT_TRUE(called1);
ASSERT_FALSE(event->IsSignaled());
ASSERT_FALSE(called2);
// Create |widget2|. The two blocked tasks should run.
drm_device_ptr_->CreateWindow(widget2);
drm_thread_.FlushForTesting();
ASSERT_TRUE(event->IsSignaled());
ASSERT_TRUE(called2);
// Post another task blocked on |widget1| with a WaitableEvent. It should run
// immediately.
event = PostStubTaskWithWaitableEvent(widget1);
drm_thread_.FlushForTesting();
ASSERT_TRUE(event->IsSignaled());
// Post another task blocked on |widget2| with a WaitableEvent. It should run
// immediately.
event = PostStubTaskWithWaitableEvent(widget2);
drm_thread_.FlushForTesting();
ASSERT_TRUE(event->IsSignaled());
// Destroy |widget2| to avoid failures during tear down.
drm_device_ptr_->DestroyWindow(widget2);
drm_thread_.FlushForTesting();
}
} // namespace ui
......@@ -30,21 +30,26 @@ void DrmWindowProxy::SchedulePageFlip(
std::vector<DrmOverlayPlane> planes,
SwapCompletionOnceCallback submission_callback,
PresentationOnceCallback presentation_callback) {
base::OnceClosure task = base::BindOnce(
&DrmThread::SchedulePageFlip, base::Unretained(drm_thread_), widget_,
base::Passed(&planes),
CreateSafeOnceCallback(std::move(submission_callback)),
CreateSafeOnceCallback(std::move(presentation_callback)));
drm_thread_->task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&DrmThread::SchedulePageFlip,
base::Unretained(drm_thread_), widget_,
base::Passed(&planes),
CreateSafeOnceCallback(std::move(submission_callback)),
CreateSafeOnceCallback(std::move(presentation_callback))));
FROM_HERE, base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
base::Unretained(drm_thread_), widget_,
std::move(task), nullptr));
}
bool DrmWindowProxy::SupportsGpuFences() const {
bool is_atomic = false;
base::OnceClosure task =
base::BindOnce(&DrmThread::IsDeviceAtomic, base::Unretained(drm_thread_),
widget_, &is_atomic);
PostSyncTask(
drm_thread_->task_runner(),
base::BindOnce(&DrmThread::IsDeviceAtomic, base::Unretained(drm_thread_),
widget_, &is_atomic));
base::BindOnce(&DrmThread::RunTaskAfterWindowReady,
base::Unretained(drm_thread_), widget_, std::move(task)));
return is_atomic && !base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableExplicitDmaFences);
}
......
......@@ -6,28 +6,15 @@
#include <utility>
#include "base/synchronization/waitable_event.h"
namespace ui {
namespace {
void OnRunPostedTaskAndSignal(base::OnceClosure callback,
base::WaitableEvent* wait) {
std::move(callback).Run();
wait->Signal();
}
} // namespace
void PostSyncTask(
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
base::OnceClosure callback) {
base::OnceCallback<void(base::WaitableEvent*)> callback) {
base::WaitableEvent wait(base::WaitableEvent::ResetPolicy::AUTOMATIC,
base::WaitableEvent::InitialState::NOT_SIGNALED);
bool success = task_runner->PostTask(
FROM_HERE,
base::BindOnce(OnRunPostedTaskAndSignal, std::move(callback), &wait));
FROM_HERE, base::BindOnce(std::move(callback), &wait));
if (success)
wait.Wait();
}
......
......@@ -10,6 +10,7 @@
#include "base/bind.h"
#include "base/location.h"
#include "base/memory/ref_counted.h"
#include "base/synchronization/waitable_event.h"
#include "base/threading/thread_task_runner_handle.h"
namespace ui {
......@@ -31,7 +32,7 @@ void PostAsyncTask(
// executing.
void PostSyncTask(
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
base::OnceClosure callback);
base::OnceCallback<void(base::WaitableEvent*)> callback);
// Creates a RepeatingCallback that will run |callback| on the calling thread.
// Useful when posting a task on a different thread and expecting a callback
......
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