diff options
author | Tom <tomut@yahoo.com> | 2021-07-03 20:42:38 -0600 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2021-07-04 23:59:17 +0200 |
commit | 7fdf902e4a7431ee93b019fff98741f248108e1f (patch) | |
tree | c63a605e8e4cf1e22375f4cc24f462a77f36196c | |
parent | 6e792553f26a0fb0f4d763d9befe4f79716a81d0 (diff) | |
download | serenity-7fdf902e4a7431ee93b019fff98741f248108e1f.zip |
Kernel: Implement buffer flipping for VirtIOGPU framebuffers
This solves tearing issues and improves performance when updating the
VirtIOGPU framebuffers.
-rw-r--r-- | Kernel/Graphics/FramebufferDevice.cpp | 11 | ||||
-rw-r--r-- | Kernel/Graphics/VirtIOGPU/VirtIOFrameBufferDevice.cpp | 147 | ||||
-rw-r--r-- | Kernel/Graphics/VirtIOGPU/VirtIOFrameBufferDevice.h | 40 | ||||
-rw-r--r-- | Kernel/Graphics/VirtIOGPU/VirtIOGPU.cpp | 12 | ||||
-rw-r--r-- | Kernel/Graphics/VirtIOGPU/VirtIOGPU.h | 2 | ||||
-rw-r--r-- | Kernel/Graphics/VirtIOGPU/VirtIOGPUConsole.cpp | 2 | ||||
-rw-r--r-- | Kernel/Graphics/VirtIOGPU/VirtIOGraphicsAdapter.cpp | 1 |
7 files changed, 149 insertions, 66 deletions
diff --git a/Kernel/Graphics/FramebufferDevice.cpp b/Kernel/Graphics/FramebufferDevice.cpp index c77dd4b2e7..15672485aa 100644 --- a/Kernel/Graphics/FramebufferDevice.cpp +++ b/Kernel/Graphics/FramebufferDevice.cpp @@ -215,6 +215,17 @@ int FramebufferDevice::ioctl(FileDescription&, unsigned request, FlatPtr arg) return -EFAULT; return 0; } + case FB_IOCTL_GET_BUFFER_OFFSET: { + FBBufferOffset buffer_offset; + if (!copy_from_user(&buffer_offset, (FBBufferOffset*)arg)) + return -EFAULT; + if (buffer_offset.buffer_index != 0 && buffer_offset.buffer_index != 1) + return -EINVAL; + buffer_offset.offset = (size_t)buffer_offset.buffer_index * m_framebuffer_pitch * m_framebuffer_height; + if (!copy_to_user((FBBufferOffset*)arg, &buffer_offset)) + return -EFAULT; + return 0; + } case FB_IOCTL_FLUSH_BUFFERS: return -ENOTSUP; default: diff --git a/Kernel/Graphics/VirtIOGPU/VirtIOFrameBufferDevice.cpp b/Kernel/Graphics/VirtIOGPU/VirtIOFrameBufferDevice.cpp index 1b6d67e1f2..6e8bbce64d 100644 --- a/Kernel/Graphics/VirtIOGPU/VirtIOFrameBufferDevice.cpp +++ b/Kernel/Graphics/VirtIOGPU/VirtIOFrameBufferDevice.cpp @@ -15,10 +15,8 @@ VirtIOFrameBufferDevice::VirtIOFrameBufferDevice(VirtIOGPU& virtio_gpu, VirtIOGP , m_gpu(virtio_gpu) , m_scanout(scanout) { - if (display_info().enabled) { - Locker locker(m_gpu.operation_lock()); + if (display_info().enabled) create_framebuffer(); - } } VirtIOFrameBufferDevice::~VirtIOFrameBufferDevice() @@ -27,39 +25,53 @@ VirtIOFrameBufferDevice::~VirtIOFrameBufferDevice() void VirtIOFrameBufferDevice::create_framebuffer() { - auto& info = display_info(); - - size_t buffer_length = page_round_up(calculate_framebuffer_size(info.rect.width, info.rect.height)); - // First delete any existing framebuffers to free the memory first m_framebuffer = nullptr; m_framebuffer_sink_vmobject = nullptr; - // 1. Allocate frame buffer - m_framebuffer = MM.allocate_kernel_region(buffer_length, String::formatted("VirtGPU FrameBuffer #{}", m_scanout.value()), Region::Access::Read | Region::Access::Write, AllocationStrategy::AllocateNow); + // Allocate frame buffer for both front and back + auto& info = display_info(); + m_buffer_size = calculate_framebuffer_size(info.rect.width, info.rect.height); + m_framebuffer = MM.allocate_kernel_region(m_buffer_size * 2, String::formatted("VirtGPU FrameBuffer #{}", m_scanout.value()), Region::Access::Read | Region::Access::Write, AllocationStrategy::AllocateNow); auto write_sink_page = MM.allocate_user_physical_page(MemoryManager::ShouldZeroFill::No).release_nonnull(); auto num_needed_pages = m_framebuffer->vmobject().page_count(); + NonnullRefPtrVector<PhysicalPage> pages; for (auto i = 0u; i < num_needed_pages; ++i) { pages.append(write_sink_page); } m_framebuffer_sink_vmobject = AnonymousVMObject::create_with_physical_pages(move(pages)); - // 2. Create BUFFER using VIRTIO_GPU_CMD_RESOURCE_CREATE_2D - if (m_resource_id.value() != 0) - m_gpu.delete_resource(m_resource_id); - m_resource_id = m_gpu.create_2d_resource(info.rect); - - // 3. Attach backing storage using VIRTIO_GPU_CMD_RESOURCE_ATTACH_BACKING - m_gpu.ensure_backing_storage(*m_framebuffer, buffer_length, m_resource_id); - // 4. Use VIRTIO_GPU_CMD_SET_SCANOUT to link the framebuffer to a display scanout. - m_gpu.set_scanout_resource(m_scanout.value(), m_resource_id, info.rect); - // 5. Render our test pattern - draw_ntsc_test_pattern(); - // 6. Use VIRTIO_GPU_CMD_TRANSFER_TO_HOST_2D to update the host resource from guest memory. - transfer_framebuffer_data_to_host(info.rect); - // 7. Use VIRTIO_GPU_CMD_RESOURCE_FLUSH to flush the updated resource to the display. - flush_displayed_image(info.rect); + Locker locker(m_gpu.operation_lock()); + m_current_buffer = &buffer_from_index(m_last_set_buffer_index.load()); + create_buffer(m_main_buffer, 0, m_buffer_size); + create_buffer(m_back_buffer, m_buffer_size, m_buffer_size); +} + +void VirtIOFrameBufferDevice::create_buffer(Buffer& buffer, size_t framebuffer_offset, size_t framebuffer_size) +{ + buffer.framebuffer_offset = framebuffer_offset; + buffer.framebuffer_data = m_framebuffer->vaddr().as_ptr() + framebuffer_offset; + + auto& info = display_info(); + + // 1. Create BUFFER using VIRTIO_GPU_CMD_RESOURCE_CREATE_2D + if (buffer.resource_id.value() != 0) + m_gpu.delete_resource(buffer.resource_id); + buffer.resource_id = m_gpu.create_2d_resource(info.rect); + + // 2. Attach backing storage using VIRTIO_GPU_CMD_RESOURCE_ATTACH_BACKING + m_gpu.ensure_backing_storage(*m_framebuffer, buffer.framebuffer_offset, framebuffer_size, buffer.resource_id); + // 3. Use VIRTIO_GPU_CMD_SET_SCANOUT to link the framebuffer to a display scanout. + if (&buffer == m_current_buffer) + m_gpu.set_scanout_resource(m_scanout.value(), buffer.resource_id, info.rect); + // 4. Render our test pattern + draw_ntsc_test_pattern(buffer); + // 5. Use VIRTIO_GPU_CMD_TRANSFER_TO_HOST_2D to update the host resource from guest memory. + transfer_framebuffer_data_to_host(info.rect, buffer); + // 6. Use VIRTIO_GPU_CMD_RESOURCE_FLUSH to flush the updated resource to the display. + if (&buffer == m_current_buffer) + flush_displayed_image(info.rect, buffer); info.enabled = 1; } @@ -74,25 +86,19 @@ VirtIOGPURespDisplayInfo::VirtIOGPUDisplayOne& VirtIOFrameBufferDevice::display_ return m_gpu.display_info(m_scanout); } -size_t VirtIOFrameBufferDevice::size_in_bytes() const -{ - auto& info = display_info(); - return info.rect.width * info.rect.height * sizeof(u32); -} - -void VirtIOFrameBufferDevice::flush_dirty_window(VirtIOGPURect const& dirty_rect) +void VirtIOFrameBufferDevice::transfer_framebuffer_data_to_host(VirtIOGPURect const& rect, Buffer& buffer) { - m_gpu.flush_dirty_window(m_scanout, dirty_rect, m_resource_id); + m_gpu.transfer_framebuffer_data_to_host(m_scanout, rect, buffer.resource_id); } -void VirtIOFrameBufferDevice::transfer_framebuffer_data_to_host(VirtIOGPURect const& rect) +void VirtIOFrameBufferDevice::flush_dirty_window(VirtIOGPURect const& dirty_rect, Buffer& buffer) { - m_gpu.transfer_framebuffer_data_to_host(m_scanout, rect, m_resource_id); + m_gpu.flush_dirty_window(m_scanout, dirty_rect, buffer.resource_id); } -void VirtIOFrameBufferDevice::flush_displayed_image(VirtIOGPURect const& dirty_rect) +void VirtIOFrameBufferDevice::flush_displayed_image(VirtIOGPURect const& dirty_rect, Buffer& buffer) { - m_gpu.flush_displayed_image(dirty_rect, m_resource_id); + m_gpu.flush_displayed_image(dirty_rect, buffer.resource_id); } bool VirtIOFrameBufferDevice::try_to_set_resolution(size_t width, size_t height) @@ -114,13 +120,23 @@ bool VirtIOFrameBufferDevice::try_to_set_resolution(size_t width, size_t height) return true; } +void VirtIOFrameBufferDevice::set_buffer(int buffer_index) +{ + auto& buffer = buffer_index == 0 ? m_main_buffer : m_back_buffer; + Locker locker(m_gpu.operation_lock()); + if (&buffer == m_current_buffer) + return; + m_current_buffer = &buffer; + m_gpu.set_scanout_resource(m_scanout.value(), buffer.resource_id, display_info().rect); +} + int VirtIOFrameBufferDevice::ioctl(FileDescription&, unsigned request, FlatPtr arg) { REQUIRE_PROMISE(video); switch (request) { case FB_IOCTL_GET_SIZE_IN_BYTES: { auto* out = (size_t*)arg; - size_t value = size_in_bytes(); + size_t value = m_buffer_size * 2; if (!copy_to_user(out, &value)) return -EFAULT; return 0; @@ -147,30 +163,55 @@ int VirtIOFrameBufferDevice::ioctl(FileDescription&, unsigned request, FlatPtr a return -EFAULT; return 0; } + case FB_IOCTL_SET_BUFFER: { + auto buffer_index = (int)arg; + if (!is_valid_buffer_index(buffer_index)) + return -EINVAL; + if (m_last_set_buffer_index.exchange(buffer_index) != buffer_index && m_are_writes_active) + set_buffer(buffer_index); + return 0; + } case FB_IOCTL_FLUSH_BUFFERS: { FBFlushRects user_flush_rects; if (!copy_from_user(&user_flush_rects, (FBFlushRects*)arg)) return -EFAULT; - if (user_flush_rects.buffer_index != 0) + if (!is_valid_buffer_index(user_flush_rects.buffer_index)) return -EINVAL; if (Checked<unsigned>::multiplication_would_overflow(user_flush_rects.count, sizeof(FBRect))) return -EFAULT; - for (unsigned i = 0; i < user_flush_rects.count; i++) { - FBRect user_dirty_rect; - if (!copy_from_user(&user_dirty_rect, &user_flush_rects.rects[i])) - return -EFAULT; - if (m_are_writes_active) { + if (m_are_writes_active && user_flush_rects.count > 0) { + auto& buffer = buffer_from_index(user_flush_rects.buffer_index); + Locker locker(m_gpu.operation_lock()); + for (unsigned i = 0; i < user_flush_rects.count; i++) { + FBRect user_dirty_rect; + if (!copy_from_user(&user_dirty_rect, &user_flush_rects.rects[i])) + return -EFAULT; VirtIOGPURect dirty_rect { .x = user_dirty_rect.x, .y = user_dirty_rect.y, .width = user_dirty_rect.width, .height = user_dirty_rect.height }; - flush_dirty_window(dirty_rect); + transfer_framebuffer_data_to_host(dirty_rect, buffer); + if (&buffer == m_current_buffer) { + // Flushing directly to screen + flush_displayed_image(dirty_rect, buffer); + } } } return 0; } + case FB_IOCTL_GET_BUFFER_OFFSET: { + FBBufferOffset buffer_offset; + if (!copy_from_user(&buffer_offset, (FBBufferOffset*)arg)) + return -EFAULT; + if (!is_valid_buffer_index(buffer_offset.buffer_index)) + return -EINVAL; + buffer_offset.offset = (size_t)buffer_offset.buffer_index * m_buffer_size; + if (!copy_to_user((FBBufferOffset*)arg, &buffer_offset)) + return -EFAULT; + return 0; + } default: return -EINVAL; }; @@ -181,9 +222,9 @@ KResultOr<Region*> VirtIOFrameBufferDevice::mmap(Process& process, FileDescripti REQUIRE_PROMISE(video); if (!shared) return ENODEV; - if (offset != 0) + if (offset != 0 || !m_framebuffer) return ENXIO; - if (range.size() != page_round_up(size_in_bytes())) + if (range.size() > m_framebuffer->size()) return EOVERFLOW; // We only allow one process to map the region @@ -217,24 +258,28 @@ void VirtIOFrameBufferDevice::deactivate_writes() region->set_vmobject(vm_object.release_nonnull()); region->remap(); } + set_buffer(0); + clear_to_black(buffer_from_index(0)); } void VirtIOFrameBufferDevice::activate_writes() { m_are_writes_active = true; + auto last_set_buffer_index = m_last_set_buffer_index.load(); if (m_userspace_mmap_region) { auto* region = m_userspace_mmap_region.unsafe_ptr(); region->set_vmobject(m_framebuffer->vmobject()); region->remap(); } + set_buffer(last_set_buffer_index); } -void VirtIOFrameBufferDevice::clear_to_black() +void VirtIOFrameBufferDevice::clear_to_black(Buffer& buffer) { auto& info = display_info(); size_t width = info.rect.width; size_t height = info.rect.height; - u8* data = m_framebuffer->vaddr().as_ptr(); + u8* data = buffer.framebuffer_data; for (size_t i = 0; i < width * height; ++i) { data[4 * i + 0] = 0x00; data[4 * i + 1] = 0x00; @@ -243,7 +288,7 @@ void VirtIOFrameBufferDevice::clear_to_black() } } -void VirtIOFrameBufferDevice::draw_ntsc_test_pattern() +void VirtIOFrameBufferDevice::draw_ntsc_test_pattern(Buffer& buffer) { static constexpr u8 colors[12][4] = { { 0xff, 0xff, 0xff, 0xff }, // White @@ -262,7 +307,7 @@ void VirtIOFrameBufferDevice::draw_ntsc_test_pattern() auto& info = display_info(); size_t width = info.rect.width; size_t height = info.rect.height; - u8* data = m_framebuffer->vaddr().as_ptr(); + u8* data = buffer.framebuffer_data; // Draw NTSC test card for (size_t y = 0; y < height; ++y) { for (size_t x = 0; x < width; ++x) { @@ -302,7 +347,7 @@ void VirtIOFrameBufferDevice::draw_ntsc_test_pattern() u8* VirtIOFrameBufferDevice::framebuffer_data() { - return m_framebuffer->vaddr().as_ptr(); + return m_current_buffer->framebuffer_data; } } diff --git a/Kernel/Graphics/VirtIOGPU/VirtIOFrameBufferDevice.h b/Kernel/Graphics/VirtIOGPU/VirtIOFrameBufferDevice.h index 7cfea5dcc8..003ce823fd 100644 --- a/Kernel/Graphics/VirtIOGPU/VirtIOFrameBufferDevice.h +++ b/Kernel/Graphics/VirtIOGPU/VirtIOFrameBufferDevice.h @@ -14,6 +14,13 @@ namespace Kernel::Graphics { class VirtIOFrameBufferDevice final : public BlockDevice { + friend class VirtIOGPUConsole; + struct Buffer { + size_t framebuffer_offset { 0 }; + u8* framebuffer_data { nullptr }; + VirtIOGPUResourceID resource_id { 0 }; + }; + public: VirtIOFrameBufferDevice(VirtIOGPU& virtio_gpu, VirtIOGPUScanoutID); virtual ~VirtIOFrameBufferDevice() override; @@ -22,23 +29,23 @@ public: virtual void activate_writes(); bool try_to_set_resolution(size_t width, size_t height); - void clear_to_black(); + void clear_to_black(Buffer&); size_t width() const { return display_info().rect.width; } size_t height() const { return display_info().rect.height; } size_t pitch() const { return display_info().rect.width * 4; } - size_t size_in_bytes() const; static size_t calculate_framebuffer_size(size_t width, size_t height) { - return sizeof(u32) * width * height; + // VirtIO resources can only map on page boundaries! + return page_round_up(sizeof(u32) * width * height); } - void flush_dirty_window(VirtIOGPURect const&); - void transfer_framebuffer_data_to_host(VirtIOGPURect const&); - void flush_displayed_image(VirtIOGPURect const&); + void flush_dirty_window(VirtIOGPURect const&, Buffer&); + void transfer_framebuffer_data_to_host(VirtIOGPURect const&, Buffer&); + void flush_displayed_image(VirtIOGPURect const&, Buffer&); - void draw_ntsc_test_pattern(); + void draw_ntsc_test_pattern(Buffer&); u8* framebuffer_data(); @@ -49,6 +56,8 @@ private: VirtIOGPURespDisplayInfo::VirtIOGPUDisplayOne& display_info(); void create_framebuffer(); + void create_buffer(Buffer&, size_t, size_t); + void set_buffer(int); virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg) override; virtual KResultOr<Region*> mmap(Process&, FileDescription&, const Range&, u64 offset, int prot, bool shared) override; @@ -61,11 +70,26 @@ private: virtual mode_t required_mode() const override { return 0666; } virtual String device_name() const override { return String::formatted("fb{}", minor()); } + static bool is_valid_buffer_index(int buffer_index) + { + return buffer_index == 0 || buffer_index == 1; + } + + Buffer& buffer_from_index(int buffer_index) + { + return buffer_index == 0 ? m_main_buffer : m_back_buffer; + } + Buffer& current_buffer() const { return *m_current_buffer; } + VirtIOGPU& m_gpu; const VirtIOGPUScanoutID m_scanout; - VirtIOGPUResourceID m_resource_id { 0 }; + Buffer* m_current_buffer { nullptr }; + Atomic<int, AK::memory_order_relaxed> m_last_set_buffer_index { 0 }; + Buffer m_main_buffer; + Buffer m_back_buffer; OwnPtr<Region> m_framebuffer; RefPtr<VMObject> m_framebuffer_sink_vmobject; + size_t m_buffer_size { 0 }; bool m_are_writes_active { true }; // FIXME: This needs to be cleaned up if the WindowServer exits while we are in a tty WeakPtr<Region> m_userspace_mmap_region; diff --git a/Kernel/Graphics/VirtIOGPU/VirtIOGPU.cpp b/Kernel/Graphics/VirtIOGPU/VirtIOGPU.cpp index 39b606e408..e1139632ef 100644 --- a/Kernel/Graphics/VirtIOGPU/VirtIOGPU.cpp +++ b/Kernel/Graphics/VirtIOGPU/VirtIOGPU.cpp @@ -127,12 +127,12 @@ VirtIOGPUResourceID VirtIOGPU::create_2d_resource(VirtIOGPURect rect) return resource_id; } -void VirtIOGPU::ensure_backing_storage(Region& region, size_t buffer_length, VirtIOGPUResourceID resource_id) +void VirtIOGPU::ensure_backing_storage(Region& region, size_t buffer_offset, size_t buffer_length, VirtIOGPUResourceID resource_id) { VERIFY(m_operation_lock.is_locked()); // Allocate backing region auto& vm_object = region.vmobject(); - size_t desired_num_pages = page_round_up(buffer_length); + size_t desired_num_pages = page_round_up(buffer_offset + buffer_length); auto& pages = vm_object.physical_pages(); for (size_t i = pages.size(); i < desired_num_pages / PAGE_SIZE; ++i) { auto page = MM.allocate_user_physical_page(); @@ -141,7 +141,11 @@ void VirtIOGPU::ensure_backing_storage(Region& region, size_t buffer_length, Vir pages.append(move(page)); } region.remap(); - size_t num_mem_regions = vm_object.page_count(); + + VERIFY(buffer_offset % PAGE_SIZE == 0); + VERIFY(buffer_length % PAGE_SIZE == 0); + auto first_page_index = buffer_offset / PAGE_SIZE; + size_t num_mem_regions = buffer_length / PAGE_SIZE; // Send request auto& request = *reinterpret_cast<VirtIOGPUResourceAttachBacking*>(m_scratch_space->vaddr().as_ptr()); @@ -152,7 +156,7 @@ void VirtIOGPU::ensure_backing_storage(Region& region, size_t buffer_length, Vir request.resource_id = resource_id.value(); request.num_entries = num_mem_regions; for (size_t i = 0; i < num_mem_regions; ++i) { - request.entries[i].address = region.physical_page(i)->paddr().get(); + request.entries[i].address = region.physical_page(first_page_index + i)->paddr().get(); request.entries[i].length = PAGE_SIZE; } diff --git a/Kernel/Graphics/VirtIOGPU/VirtIOGPU.h b/Kernel/Graphics/VirtIOGPU/VirtIOGPU.h index af8b6126c2..e39225f0a2 100644 --- a/Kernel/Graphics/VirtIOGPU/VirtIOGPU.h +++ b/Kernel/Graphics/VirtIOGPU/VirtIOGPU.h @@ -220,7 +220,7 @@ private: void query_display_information(); VirtIOGPUResourceID create_2d_resource(VirtIOGPURect rect); void delete_resource(VirtIOGPUResourceID resource_id); - void ensure_backing_storage(Region& region, size_t buffer_length, VirtIOGPUResourceID resource_id); + void ensure_backing_storage(Region& region, size_t buffer_offset, size_t buffer_length, VirtIOGPUResourceID resource_id); void detach_backing_storage(VirtIOGPUResourceID resource_id); void set_scanout_resource(VirtIOGPUScanoutID scanout, VirtIOGPUResourceID resource_id, VirtIOGPURect rect); void transfer_framebuffer_data_to_host(VirtIOGPUScanoutID scanout, VirtIOGPURect const& rect, VirtIOGPUResourceID resource_id); diff --git a/Kernel/Graphics/VirtIOGPU/VirtIOGPUConsole.cpp b/Kernel/Graphics/VirtIOGPU/VirtIOGPUConsole.cpp index f3e3cb0348..7ab490bdcd 100644 --- a/Kernel/Graphics/VirtIOGPU/VirtIOGPUConsole.cpp +++ b/Kernel/Graphics/VirtIOGPU/VirtIOGPUConsole.cpp @@ -66,7 +66,7 @@ void VirtIOGPUConsole::enqueue_refresh_timer() .height = (u32)rect.height(), }; g_io_work->queue([this, dirty_rect]() { - m_framebuffer_device->flush_dirty_window(dirty_rect); + m_framebuffer_device->flush_dirty_window(dirty_rect, m_framebuffer_device->current_buffer()); m_dirty_rect.clear(); }); } diff --git a/Kernel/Graphics/VirtIOGPU/VirtIOGraphicsAdapter.cpp b/Kernel/Graphics/VirtIOGPU/VirtIOGraphicsAdapter.cpp index b41f1f5606..7f1e75758b 100644 --- a/Kernel/Graphics/VirtIOGPU/VirtIOGraphicsAdapter.cpp +++ b/Kernel/Graphics/VirtIOGPU/VirtIOGraphicsAdapter.cpp @@ -40,7 +40,6 @@ void VirtIOGraphicsAdapter::enable_consoles() dbgln_if(VIRTIO_DEBUG, "VirtIOGPU: Enabling consoles"); m_gpu_device->for_each_framebuffer([&](auto& framebuffer, auto& console) { framebuffer.deactivate_writes(); - framebuffer.clear_to_black(); console.enable(); return IterationDecision::Continue; }); |