Merge pull request #13079 from liamwhite/vi3

vi: misc fixes

src/core/hle/service/nvnflinger/display.h

@@ -3,8 +3,6 @@
 
 #pragma once
 
-#include <list>
-
 #include "core/hle/service/nvnflinger/buffer_item_consumer.h"
 #include "core/hle/service/nvnflinger/hwc_layer.h"
 
@@ -26,18 +24,12 @@ struct Layer {
 };
 
 struct LayerStack {
-    std::list<Layer> layers;
-};
-
-struct Display {
-    explicit Display(u64 id_) {
-        id = id_;
-    }
+    std::vector<std::shared_ptr<Layer>> layers;
 
-    Layer* FindLayer(s32 consumer_id) {
-        for (auto& layer : stack.layers) {
-            if (layer.consumer_id == consumer_id) {
-                return &layer;
+    std::shared_ptr<Layer> FindLayer(s32 consumer_id) {
+        for (auto& layer : layers) {
+            if (layer->consumer_id == consumer_id) {
+                return layer;
             }
         }
 
@@ -45,7 +37,13 @@ struct Display {
     }
 
     bool HasLayers() {
-        return !stack.layers.empty();
+        return !layers.empty();
+    }
+};
+
+struct Display {
+    explicit Display(u64 id_) {
+        id = id_;
     }
 
     u64 id;

src/core/hle/service/nvnflinger/hardware_composer.cpp

@@ -55,10 +55,10 @@ u32 HardwareComposer::ComposeLocked(f32* out_speed_scale, Display& display,
 
     // Acquire all necessary framebuffers.
     for (auto& layer : display.stack.layers) {
-        auto consumer_id = layer.consumer_id;
+        auto consumer_id = layer->consumer_id;
 
         // Try to fetch the framebuffer (either new or stale).
-        const auto result = this->CacheFramebufferLocked(layer, consumer_id);
+        const auto result = this->CacheFramebufferLocked(*layer, consumer_id);
 
         // If we failed, skip this layer.
         if (result == CacheStatus::NoBufferAvailable) {
@@ -75,7 +75,7 @@ u32 HardwareComposer::ComposeLocked(f32* out_speed_scale, Display& display,
         const auto& igbp_buffer = *item.graphic_buffer;
 
         // TODO: get proper Z-index from layer
-        if (layer.visible) {
+        if (layer->visible) {
             composition_stack.emplace_back(HwcLayer{
                 .buffer_handle = igbp_buffer.BufferId(),
                 .offset = igbp_buffer.Offset(),
@@ -84,7 +84,7 @@ u32 HardwareComposer::ComposeLocked(f32* out_speed_scale, Display& display,
                 .height = igbp_buffer.Height(),
                 .stride = igbp_buffer.Stride(),
                 .z_index = 0,
-                .blending = layer.blending,
+                .blending = layer->blending,
                 .transform = static_cast<android::BufferTransformFlags>(item.transform),
                 .crop_rect = item.crop,
                 .acquire_fence = item.fence,
@@ -134,7 +134,7 @@ u32 HardwareComposer::ComposeLocked(f32* out_speed_scale, Display& display,
             continue;
         }
 
-        if (auto* layer = display.FindLayer(layer_id); layer != nullptr) {
+        if (const auto layer = display.stack.FindLayer(layer_id); layer != nullptr) {
             // TODO: support release fence
             // This is needed to prevent screen tearing
             layer->buffer_item_consumer->ReleaseBuffer(framebuffer.item, android::Fence::NoFence());
@@ -153,7 +153,7 @@ void HardwareComposer::RemoveLayerLocked(Display& display, ConsumerId consumer_i
     }
 
     // Try to release the buffer item.
-    auto* const layer = display.FindLayer(consumer_id);
+    const auto layer = display.stack.FindLayer(consumer_id);
     if (layer && it->second.is_acquired) {
         layer->buffer_item_consumer->ReleaseBuffer(it->second.item, android::Fence::NoFence());
     }

src/core/hle/service/nvnflinger/surface_flinger.cpp

@@ -36,7 +36,7 @@ void SurfaceFlinger::RemoveDisplay(u64 display_id) {
 bool SurfaceFlinger::ComposeDisplay(s32* out_swap_interval, f32* out_compose_speed_scale,
                                     u64 display_id) {
     auto* const display = this->FindDisplay(display_id);
-    if (!display || !display->HasLayers()) {
+    if (!display || !display->stack.HasLayers()) {
         return false;
     }
 
@@ -46,19 +46,34 @@ bool SurfaceFlinger::ComposeDisplay(s32* out_swap_interval, f32* out_compose_spe
     return true;
 }
 
-void SurfaceFlinger::AddLayerToDisplayStack(u64 display_id, s32 consumer_binder_id) {
-    auto* const display = this->FindDisplay(display_id);
+void SurfaceFlinger::CreateLayer(s32 consumer_binder_id) {
     auto binder = std::static_pointer_cast<android::BufferQueueConsumer>(
         m_server.TryGetBinder(consumer_binder_id));
-
-    if (!display || !binder) {
+    if (!binder) {
         return;
     }
 
     auto buffer_item_consumer = std::make_shared<android::BufferItemConsumer>(std::move(binder));
     buffer_item_consumer->Connect(false);
 
-    display->stack.layers.emplace_back(std::move(buffer_item_consumer), consumer_binder_id);
+    m_layers.layers.emplace_back(
+        std::make_shared<Layer>(std::move(buffer_item_consumer), consumer_binder_id));
+}
+
+void SurfaceFlinger::DestroyLayer(s32 consumer_binder_id) {
+    std::erase_if(m_layers.layers,
+                  [&](auto& layer) { return layer->consumer_id == consumer_binder_id; });
+}
+
+void SurfaceFlinger::AddLayerToDisplayStack(u64 display_id, s32 consumer_binder_id) {
+    auto* const display = this->FindDisplay(display_id);
+    auto layer = this->FindLayer(consumer_binder_id);
+
+    if (!display || !layer) {
+        return;
+    }
+
+    display->stack.layers.emplace_back(std::move(layer));
 }
 
 void SurfaceFlinger::RemoveLayerFromDisplayStack(u64 display_id, s32 consumer_binder_id) {
@@ -69,18 +84,18 @@ void SurfaceFlinger::RemoveLayerFromDisplayStack(u64 display_id, s32 consumer_bi
 
     m_composer.RemoveLayerLocked(*display, consumer_binder_id);
     std::erase_if(display->stack.layers,
-                  [&](auto& layer) { return layer.consumer_id == consumer_binder_id; });
+                  [&](auto& layer) { return layer->consumer_id == consumer_binder_id; });
 }
 
 void SurfaceFlinger::SetLayerVisibility(s32 consumer_binder_id, bool visible) {
-    if (auto* layer = this->FindLayer(consumer_binder_id); layer != nullptr) {
+    if (const auto layer = this->FindLayer(consumer_binder_id); layer != nullptr) {
         layer->visible = visible;
         return;
     }
 }
 
 void SurfaceFlinger::SetLayerBlending(s32 consumer_binder_id, LayerBlending blending) {
-    if (auto* layer = this->FindLayer(consumer_binder_id); layer != nullptr) {
+    if (const auto layer = this->FindLayer(consumer_binder_id); layer != nullptr) {
         layer->blending = blending;
         return;
     }
@@ -96,9 +111,9 @@ Display* SurfaceFlinger::FindDisplay(u64 display_id) {
     return nullptr;
 }
 
-Layer* SurfaceFlinger::FindLayer(s32 consumer_binder_id) {
-    for (auto& display : m_displays) {
-        if (auto* layer = display.FindLayer(consumer_binder_id); layer != nullptr) {
+std::shared_ptr<Layer> SurfaceFlinger::FindLayer(s32 consumer_binder_id) {
+    for (auto& layer : m_layers.layers) {
+        if (layer->consumer_id == consumer_binder_id) {
             return layer;
         }
     }

src/core/hle/service/nvnflinger/surface_flinger.h

@@ -36,6 +36,9 @@ public:
     void RemoveDisplay(u64 display_id);
     bool ComposeDisplay(s32* out_swap_interval, f32* out_compose_speed_scale, u64 display_id);
 
+    void CreateLayer(s32 consumer_binder_id);
+    void DestroyLayer(s32 consumer_binder_id);
+
     void AddLayerToDisplayStack(u64 display_id, s32 consumer_binder_id);
     void RemoveLayerFromDisplayStack(u64 display_id, s32 consumer_binder_id);
 
@@ -44,7 +47,7 @@ public:
 
 private:
     Display* FindDisplay(u64 display_id);
-    Layer* FindLayer(s32 consumer_binder_id);
+    std::shared_ptr<Layer> FindLayer(s32 consumer_binder_id);
 
 public:
     // TODO: these don't belong here
@@ -57,6 +60,7 @@ private:
     KernelHelpers::ServiceContext m_context;
 
     std::vector<Display> m_displays;
+    LayerStack m_layers;
     std::shared_ptr<Nvidia::Module> nvdrv;
     s32 disp_fd;
     HardwareComposer m_composer;

src/core/hle/service/vi/container.cpp

@@ -43,11 +43,7 @@ void Container::OnTerminate() {
 
     m_is_shut_down = true;
 
-    m_layers.ForEachLayer([&](auto& layer) {
-        if (layer.IsOpen()) {
-            this->DestroyBufferQueueLocked(&layer);
-        }
-    });
+    m_layers.ForEachLayer([&](auto& layer) { this->DestroyLayerLocked(layer.GetId()); });
 
     m_displays.ForEachDisplay(
         [&](auto& display) { m_surface_flinger->RemoveDisplay(display.GetId()); });
@@ -161,16 +157,29 @@ Result Container::CreateLayerLocked(u64* out_layer_id, u64 display_id, u64 owner
     auto* const display = m_displays.GetDisplayById(display_id);
     R_UNLESS(display != nullptr, VI::ResultNotFound);
 
-    auto* const layer = m_layers.CreateLayer(owner_aruid, display);
+    s32 consumer_binder_id, producer_binder_id;
+    m_surface_flinger->CreateBufferQueue(&consumer_binder_id, &producer_binder_id);
+
+    auto* const layer =
+        m_layers.CreateLayer(owner_aruid, display, consumer_binder_id, producer_binder_id);
     R_UNLESS(layer != nullptr, VI::ResultNotFound);
 
+    m_surface_flinger->CreateLayer(consumer_binder_id);
+
     *out_layer_id = layer->GetId();
     R_SUCCEED();
 }
 
 Result Container::DestroyLayerLocked(u64 layer_id) {
-    R_SUCCEED_IF(m_layers.DestroyLayer(layer_id));
-    R_THROW(VI::ResultNotFound);
+    auto* const layer = m_layers.GetLayerById(layer_id);
+    R_UNLESS(layer != nullptr, VI::ResultNotFound);
+
+    m_surface_flinger->DestroyLayer(layer->GetConsumerBinderId());
+    m_surface_flinger->DestroyBufferQueue(layer->GetConsumerBinderId(),
+                                          layer->GetProducerBinderId());
+    m_layers.DestroyLayer(layer_id);
+
+    R_SUCCEED();
 }
 
 Result Container::OpenLayerLocked(s32* out_producer_binder_id, u64 layer_id, u64 aruid) {
@@ -181,7 +190,12 @@ Result Container::OpenLayerLocked(s32* out_producer_binder_id, u64 layer_id, u64
     R_UNLESS(!layer->IsOpen(), VI::ResultOperationFailed);
     R_UNLESS(layer->GetOwnerAruid() == aruid, VI::ResultPermissionDenied);
 
-    this->CreateBufferQueueLocked(layer);
+    layer->Open();
+
+    if (auto* display = layer->GetDisplay(); display != nullptr) {
+        m_surface_flinger->AddLayerToDisplayStack(display->GetId(), layer->GetConsumerBinderId());
+    }
+
     *out_producer_binder_id = layer->GetProducerBinderId();
 
     R_SUCCEED();
@@ -192,30 +206,14 @@ Result Container::CloseLayerLocked(u64 layer_id) {
     R_UNLESS(layer != nullptr, VI::ResultNotFound);
     R_UNLESS(layer->IsOpen(), VI::ResultOperationFailed);
 
-    this->DestroyBufferQueueLocked(layer);
-
-    R_SUCCEED();
-}
-
-void Container::CreateBufferQueueLocked(Layer* layer) {
-    s32 consumer_binder_id, producer_binder_id;
-    m_surface_flinger->CreateBufferQueue(&consumer_binder_id, &producer_binder_id);
-    layer->Open(consumer_binder_id, producer_binder_id);
-
-    if (auto* display = layer->GetDisplay(); display != nullptr) {
-        m_surface_flinger->AddLayerToDisplayStack(display->GetId(), consumer_binder_id);
-    }
-}
-
-void Container::DestroyBufferQueueLocked(Layer* layer) {
     if (auto* display = layer->GetDisplay(); display != nullptr) {
         m_surface_flinger->RemoveLayerFromDisplayStack(display->GetId(),
                                                        layer->GetConsumerBinderId());
     }
 
     layer->Close();
-    m_surface_flinger->DestroyBufferQueue(layer->GetConsumerBinderId(),
-                                          layer->GetProducerBinderId());
+
+    R_SUCCEED();
 }
 
 bool Container::ComposeOnDisplay(s32* out_swap_interval, f32* out_compose_speed_scale,

src/core/hle/service/vi/container.h

@@ -72,9 +72,6 @@ private:
     Result OpenLayerLocked(s32* out_producer_binder_id, u64 layer_id, u64 aruid);
     Result CloseLayerLocked(u64 layer_id);
 
-    void CreateBufferQueueLocked(Layer* layer);
-    void DestroyBufferQueueLocked(Layer* layer);
-
 public:
     bool ComposeOnDisplay(s32* out_swap_interval, f32* out_compose_speed_scale, u64 display_id);
 

src/core/hle/service/vi/layer.h

@@ -13,29 +13,31 @@ class Layer {
 public:
     constexpr Layer() = default;
 
-    void Initialize(u64 id, u64 owner_aruid, Display* display) {
+    void Initialize(u64 id, u64 owner_aruid, Display* display, s32 consumer_binder_id,
+                    s32 producer_binder_id) {
         m_id = id;
         m_owner_aruid = owner_aruid;
         m_display = display;
+        m_consumer_binder_id = consumer_binder_id;
+        m_producer_binder_id = producer_binder_id;
         m_is_initialized = true;
     }
 
     void Finalize() {
         m_id = {};
+        m_owner_aruid = {};
         m_display = {};
+        m_consumer_binder_id = {};
+        m_producer_binder_id = {};
         m_is_initialized = {};
     }
 
-    void Open(s32 consumer_binder_id, s32 producer_binder_id) {
-        m_consumer_binder_id = consumer_binder_id;
-        m_producer_binder_id = producer_binder_id;
+    void Open() {
         m_is_open = true;
     }
 
     void Close() {
-        m_producer_binder_id = {};
-        m_consumer_binder_id = {};
-        m_is_open = {};
+        m_is_open = false;
     }
 
     u64 GetId() const {

src/core/hle/service/vi/layer_list.h

@@ -11,13 +11,15 @@ class LayerList {
 public:
     constexpr LayerList() = default;
 
-    Layer* CreateLayer(u64 owner_aruid, Display* display) {
+    Layer* CreateLayer(u64 owner_aruid, Display* display, s32 consumer_binder_id,
+                       s32 producer_binder_id) {
         Layer* const layer = GetFreeLayer();
         if (!layer) {
             return nullptr;
         }
 
-        layer->Initialize(++m_next_id, owner_aruid, display);
+        layer->Initialize(++m_next_id, owner_aruid, display, consumer_binder_id,
+                          producer_binder_id);
         return layer;
     }
 

src/core/hle/service/vi/shared_buffer_manager.cpp

@@ -285,7 +285,7 @@ void SharedBufferManager::DestroySession(Kernel::KProcess* owner_process) {
     auto& session = it->second;
 
     // Destroy the layer.
-    R_ASSERT(m_container.DestroyStrayLayer(session.layer_id));
+    m_container.DestroyStrayLayer(session.layer_id);
 
     // Close nvmap handle.
     FreeHandle(session.buffer_nvmap_handle, *m_nvdrv, session.nvmap_fd);
@@ -322,8 +322,6 @@ Result SharedBufferManager::GetSharedBufferMemoryHandleId(u64* out_buffer_size,
 Result SharedBufferManager::AcquireSharedFrameBuffer(android::Fence* out_fence,
                                                      std::array<s32, 4>& out_slot_indexes,
                                                      s64* out_target_slot, u64 layer_id) {
-    std::scoped_lock lk{m_guard};
-
     // Get the producer.
     std::shared_ptr<android::BufferQueueProducer> producer;
     R_TRY(m_container.GetLayerProducerHandle(std::addressof(producer), layer_id));
@@ -347,8 +345,6 @@ Result SharedBufferManager::PresentSharedFrameBuffer(android::Fence fence,
                                                      Common::Rectangle<s32> crop_region,
                                                      u32 transform, s32 swap_interval, u64 layer_id,
                                                      s64 slot) {
-    std::scoped_lock lk{m_guard};
-
     // Get the producer.
     std::shared_ptr<android::BufferQueueProducer> producer;
     R_TRY(m_container.GetLayerProducerHandle(std::addressof(producer), layer_id));
@@ -379,8 +375,6 @@ Result SharedBufferManager::PresentSharedFrameBuffer(android::Fence fence,
 }
 
 Result SharedBufferManager::CancelSharedFrameBuffer(u64 layer_id, s64 slot) {
-    std::scoped_lock lk{m_guard};
-
     // Get the producer.
     std::shared_ptr<android::BufferQueueProducer> producer;
     R_TRY(m_container.GetLayerProducerHandle(std::addressof(producer), layer_id));
@@ -394,8 +388,6 @@ Result SharedBufferManager::CancelSharedFrameBuffer(u64 layer_id, s64 slot) {
 
 Result SharedBufferManager::GetSharedFrameBufferAcquirableEvent(Kernel::KReadableEvent** out_event,
                                                                 u64 layer_id) {
-    std::scoped_lock lk{m_guard};
-
     // Get the producer.
     std::shared_ptr<android::BufferQueueProducer> producer;
     R_TRY(m_container.GetLayerProducerHandle(std::addressof(producer), layer_id));