From ce895807499ef664f244eb9f435b076484f54d7c Mon Sep 17 00:00:00 2001 From: Liam Date: Wed, 17 Jan 2024 18:45:39 -0500 Subject: [PATCH 1/2] nvnflinger: ensure display abandonment considers all layers and future layers --- src/core/hle/service/nvnflinger/nvnflinger.cpp | 4 +--- src/core/hle/service/vi/display/vi_display.cpp | 11 +++++++++++ src/core/hle/service/vi/display/vi_display.h | 3 +++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/core/hle/service/nvnflinger/nvnflinger.cpp b/src/core/hle/service/nvnflinger/nvnflinger.cpp index 0469110e8..e05ff66ff 100644 --- a/src/core/hle/service/nvnflinger/nvnflinger.cpp +++ b/src/core/hle/service/nvnflinger/nvnflinger.cpp @@ -112,9 +112,7 @@ void Nvnflinger::ShutdownLayers() { { const auto lock_guard = Lock(); for (auto& display : displays) { - for (size_t layer = 0; layer < display.GetNumLayers(); ++layer) { - display.GetLayer(layer).GetConsumer().Abandon(); - } + display.Abandon(); } is_abandoned = true; diff --git a/src/core/hle/service/vi/display/vi_display.cpp b/src/core/hle/service/vi/display/vi_display.cpp index e2d9cd98a..725311c53 100644 --- a/src/core/hle/service/vi/display/vi_display.cpp +++ b/src/core/hle/service/vi/display/vi_display.cpp @@ -91,6 +91,10 @@ void Display::CreateLayer(u64 layer_id, u32 binder_id, layers.emplace_back(std::make_unique(layer_id, binder_id, *core, *producer, std::move(buffer_item_consumer))); + if (is_abandoned) { + this->FindLayer(layer_id)->GetConsumer().Abandon(); + } + hos_binder_driver_server.RegisterProducer(std::move(producer)); } @@ -103,6 +107,13 @@ void Display::DestroyLayer(u64 layer_id) { [layer_id](const auto& layer) { return layer->GetLayerId() == layer_id; }); } +void Display::Abandon() { + for (auto& layer : layers) { + layer->GetConsumer().Abandon(); + } + is_abandoned = true; +} + Layer* Display::FindLayer(u64 layer_id) { const auto itr = std::find_if(layers.begin(), layers.end(), [layer_id](const std::unique_ptr& layer) { diff --git a/src/core/hle/service/vi/display/vi_display.h b/src/core/hle/service/vi/display/vi_display.h index 7e68ee79b..8eb8a5155 100644 --- a/src/core/hle/service/vi/display/vi_display.h +++ b/src/core/hle/service/vi/display/vi_display.h @@ -98,6 +98,8 @@ public: layers.clear(); } + void Abandon(); + /// Attempts to find a layer with the given ID. /// /// @param layer_id The layer ID. @@ -124,6 +126,7 @@ private: std::vector> layers; Kernel::KEvent* vsync_event{}; + bool is_abandoned{}; }; } // namespace Service::VI From e4bbb24dcf9048ac23e6c12f2ab5af1e988af764 Mon Sep 17 00:00:00 2001 From: Liam Date: Wed, 17 Jan 2024 22:03:40 -0500 Subject: [PATCH 2/2] vi: check layer state before opening or closing --- src/core/hle/service/nvnflinger/nvnflinger.cpp | 12 ++++++++---- src/core/hle/service/nvnflinger/nvnflinger.h | 4 ++-- src/core/hle/service/vi/layer/vi_layer.h | 9 +++++---- src/core/hle/service/vi/vi.cpp | 14 ++++++++++++-- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/core/hle/service/nvnflinger/nvnflinger.cpp b/src/core/hle/service/nvnflinger/nvnflinger.cpp index e05ff66ff..af6591370 100644 --- a/src/core/hle/service/nvnflinger/nvnflinger.cpp +++ b/src/core/hle/service/nvnflinger/nvnflinger.cpp @@ -174,24 +174,28 @@ void Nvnflinger::CreateLayerAtId(VI::Display& display, u64 layer_id) { display.CreateLayer(layer_id, buffer_id, nvdrv->container); } -void Nvnflinger::OpenLayer(u64 layer_id) { +bool Nvnflinger::OpenLayer(u64 layer_id) { const auto lock_guard = Lock(); for (auto& display : displays) { if (auto* layer = display.FindLayer(layer_id); layer) { - layer->Open(); + return layer->Open(); } } + + return false; } -void Nvnflinger::CloseLayer(u64 layer_id) { +bool Nvnflinger::CloseLayer(u64 layer_id) { const auto lock_guard = Lock(); for (auto& display : displays) { if (auto* layer = display.FindLayer(layer_id); layer) { - layer->Close(); + return layer->Close(); } } + + return false; } void Nvnflinger::DestroyLayer(u64 layer_id) { diff --git a/src/core/hle/service/nvnflinger/nvnflinger.h b/src/core/hle/service/nvnflinger/nvnflinger.h index 871285764..a60e0ae6b 100644 --- a/src/core/hle/service/nvnflinger/nvnflinger.h +++ b/src/core/hle/service/nvnflinger/nvnflinger.h @@ -74,10 +74,10 @@ public: [[nodiscard]] std::optional CreateLayer(u64 display_id); /// Opens a layer on all displays for the given layer ID. - void OpenLayer(u64 layer_id); + bool OpenLayer(u64 layer_id); /// Closes a layer on all displays for the given layer ID. - void CloseLayer(u64 layer_id); + bool CloseLayer(u64 layer_id); /// Destroys the given layer ID. void DestroyLayer(u64 layer_id); diff --git a/src/core/hle/service/vi/layer/vi_layer.h b/src/core/hle/service/vi/layer/vi_layer.h index 295005e23..f95e2dc71 100644 --- a/src/core/hle/service/vi/layer/vi_layer.h +++ b/src/core/hle/service/vi/layer/vi_layer.h @@ -4,6 +4,7 @@ #pragma once #include +#include #include "common/common_types.h" @@ -75,12 +76,12 @@ public: return open; } - void Close() { - open = false; + bool Close() { + return std::exchange(open, false); } - void Open() { - open = true; + bool Open() { + return !std::exchange(open, true); } private: diff --git a/src/core/hle/service/vi/vi.cpp b/src/core/hle/service/vi/vi.cpp index 39d5be90d..bfcc27ddc 100644 --- a/src/core/hle/service/vi/vi.cpp +++ b/src/core/hle/service/vi/vi.cpp @@ -719,7 +719,12 @@ private: return; } - nvnflinger.OpenLayer(layer_id); + if (!nvnflinger.OpenLayer(layer_id)) { + LOG_WARNING(Service_VI, "Tried to open layer which was already open"); + IPC::ResponseBuilder rb{ctx, 2}; + rb.Push(ResultOperationFailed); + return; + } android::OutputParcel parcel; parcel.WriteInterface(NativeWindow{*buffer_queue_id}); @@ -737,7 +742,12 @@ private: LOG_DEBUG(Service_VI, "called. layer_id=0x{:016X}", layer_id); - nvnflinger.CloseLayer(layer_id); + if (!nvnflinger.CloseLayer(layer_id)) { + LOG_WARNING(Service_VI, "Tried to close layer which was not open"); + IPC::ResponseBuilder rb{ctx, 2}; + rb.Push(ResultOperationFailed); + return; + } IPC::ResponseBuilder rb{ctx, 2}; rb.Push(ResultSuccess);