From 016fedbd207e171e843d69aab992e21b138d90aa Mon Sep 17 00:00:00 2001 From: Liav A Date: Sat, 9 Apr 2022 17:39:27 +0300 Subject: [PATCH] Kernel/IntelGraphics: Move DisplayPlane enable code to derived classes Instead of doing that on the IntelDisplayPlane class, let's have this in derived classes so these classes can decide how to use the settings that were provided before calling the enable method. --- .../Graphics/Intel/DisplayConnectorGroup.cpp | 9 +++++- Kernel/Graphics/Intel/Plane/DisplayPlane.cpp | 31 ++++++++++++++++--- Kernel/Graphics/Intel/Plane/DisplayPlane.h | 17 ++++++++-- .../Graphics/Intel/Plane/G33DisplayPlane.cpp | 19 ++++++------ Kernel/Graphics/Intel/Plane/G33DisplayPlane.h | 2 +- 5 files changed, 60 insertions(+), 18 deletions(-) diff --git a/Kernel/Graphics/Intel/DisplayConnectorGroup.cpp b/Kernel/Graphics/Intel/DisplayConnectorGroup.cpp index bc8f7b7b16..4d3ba51cb5 100644 --- a/Kernel/Graphics/Intel/DisplayConnectorGroup.cpp +++ b/Kernel/Graphics/Intel/DisplayConnectorGroup.cpp @@ -342,7 +342,14 @@ bool IntelDisplayConnectorGroup::set_crt_resolution(DisplayConnector::ModeSettin VERIFY(!m_transcoders[0]->pipe_enabled({})); MUST(m_transcoders[0]->enable_pipe({})); - MUST(m_planes[0]->set_plane_settings({}, m_mmio_second_region.pci_bar_paddr, IntelDisplayPlane::PipeSelect::PipeA, mode_setting.horizontal_active)); + + MUST(m_planes[0]->set_aperture_base({}, m_mmio_second_region.pci_bar_paddr)); + MUST(m_planes[0]->set_pipe({}, IntelDisplayPlane::PipeSelect::PipeA)); + MUST(m_planes[0]->set_horizontal_stride({}, mode_setting.horizontal_active * 4)); + MUST(m_planes[0]->set_horizontal_active_pixels_count({}, mode_setting.horizontal_active)); + // Note: This doesn't affect anything on the plane settings for Gen4, but we still + // do it for the sake of "completeness". + MUST(m_planes[0]->set_vertical_active_pixels_count({}, mode_setting.vertical_active)); MUST(m_planes[0]->enable({})); enable_dac_output(); diff --git a/Kernel/Graphics/Intel/Plane/DisplayPlane.cpp b/Kernel/Graphics/Intel/Plane/DisplayPlane.cpp index 21010ea012..c9681f6472 100644 --- a/Kernel/Graphics/Intel/Plane/DisplayPlane.cpp +++ b/Kernel/Graphics/Intel/Plane/DisplayPlane.cpp @@ -20,13 +20,34 @@ IntelDisplayPlane::ShadowRegisters IntelDisplayPlane::shadow_registers() const return m_shadow_registers; } -ErrorOr IntelDisplayPlane::enable(Badge) +ErrorOr IntelDisplayPlane::set_horizontal_active_pixels_count(Badge, size_t horizontal_active_pixels_count) { SpinlockLocker locker(m_access_lock); - // Note: We use the shadow register so we don't have the already set - // settings being lost. - m_plane_registers->control = m_shadow_registers.control | (1 << 31); - m_shadow_registers.control |= (1 << 31); + m_horizontal_active_pixels_count = horizontal_active_pixels_count; + return {}; +} +ErrorOr IntelDisplayPlane::set_vertical_active_pixels_count(Badge, size_t vertical_active_pixels_count) +{ + SpinlockLocker locker(m_access_lock); + m_vertical_active_pixels_count = vertical_active_pixels_count; + return {}; +} +ErrorOr IntelDisplayPlane::set_horizontal_stride(Badge, size_t horizontal_stride) +{ + SpinlockLocker locker(m_access_lock); + m_horizontal_stride = horizontal_stride; + return {}; +} +ErrorOr IntelDisplayPlane::set_aperture_base(Badge, PhysicalAddress aperture_start) +{ + SpinlockLocker locker(m_access_lock); + m_aperture_start.set(aperture_start.get()); + return {}; +} +ErrorOr IntelDisplayPlane::set_pipe(Badge, PipeSelect pipe_select) +{ + SpinlockLocker locker(m_access_lock); + m_pipe_select = pipe_select; return {}; } diff --git a/Kernel/Graphics/Intel/Plane/DisplayPlane.h b/Kernel/Graphics/Intel/Plane/DisplayPlane.h index 47f08ab111..8dfecbffbf 100644 --- a/Kernel/Graphics/Intel/Plane/DisplayPlane.h +++ b/Kernel/Graphics/Intel/Plane/DisplayPlane.h @@ -38,8 +38,13 @@ public: public: static ErrorOr> create_with_physical_address(PhysicalAddress plane_registers_start_address); - virtual ErrorOr set_plane_settings(Badge, PhysicalAddress aperture_start, PipeSelect, size_t horizontal_active_pixels_count) = 0; - ErrorOr enable(Badge); + ErrorOr set_horizontal_active_pixels_count(Badge, size_t horizontal_active_pixels_count); + ErrorOr set_vertical_active_pixels_count(Badge, size_t vertical_active_pixels_count); + ErrorOr set_horizontal_stride(Badge, size_t horizontal_stride); + ErrorOr set_aperture_base(Badge, PhysicalAddress aperture_start); + ErrorOr set_pipe(Badge, PipeSelect); + + virtual ErrorOr enable(Badge) = 0; bool is_enabled(Badge); ErrorOr disable(Badge); @@ -60,5 +65,13 @@ protected: mutable Spinlock m_access_lock; ShadowRegisters m_shadow_registers {}; Memory::TypedMapping m_plane_registers; + + // Note: The PipeSelect value is used only in planes until Skylake graphics. + PipeSelect m_pipe_select { PipeSelect::PipeA }; + + PhysicalAddress m_aperture_start; + size_t m_horizontal_stride { 0 }; + size_t m_horizontal_active_pixels_count { 0 }; + size_t m_vertical_active_pixels_count { 0 }; }; } diff --git a/Kernel/Graphics/Intel/Plane/G33DisplayPlane.cpp b/Kernel/Graphics/Intel/Plane/G33DisplayPlane.cpp index c71635f205..3a45116b28 100644 --- a/Kernel/Graphics/Intel/Plane/G33DisplayPlane.cpp +++ b/Kernel/Graphics/Intel/Plane/G33DisplayPlane.cpp @@ -20,15 +20,15 @@ IntelG33DisplayPlane::IntelG33DisplayPlane(Memory::TypedMapping IntelG33DisplayPlane::set_plane_settings(Badge, PhysicalAddress aperture_start, PipeSelect pipe_select, size_t horizontal_active_pixels_count) +ErrorOr IntelG33DisplayPlane::enable(Badge) { SpinlockLocker locker(m_access_lock); - VERIFY(((horizontal_active_pixels_count * 4) % 64 == 0)); - VERIFY(aperture_start < PhysicalAddress(0x1'0000'0000)); + VERIFY(((m_horizontal_active_pixels_count * 4) % 64 == 0)); + VERIFY(((m_horizontal_stride) % 64 == 0)); u32 control_value = 0; - switch (pipe_select) { + switch (m_pipe_select) { case PipeSelect::PipeA: control_value |= (0b00 << 24); break; @@ -44,14 +44,15 @@ ErrorOr IntelG33DisplayPlane::set_plane_settings(Badgestride = horizontal_active_pixels_count * 4; - m_shadow_registers.stride = horizontal_active_pixels_count * 4; + m_plane_registers->stride = m_horizontal_stride; + m_shadow_registers.stride = m_horizontal_stride; m_plane_registers->linear_offset = 0; m_shadow_registers.linear_offset = 0; - m_plane_registers->surface_base = aperture_start.get(); - m_shadow_registers.surface_base = aperture_start.get(); + m_plane_registers->surface_base = m_aperture_start.get(); + m_shadow_registers.surface_base = m_aperture_start.get(); m_plane_registers->control = control_value; m_shadow_registers.control = control_value; return {}; diff --git a/Kernel/Graphics/Intel/Plane/G33DisplayPlane.h b/Kernel/Graphics/Intel/Plane/G33DisplayPlane.h index de0d02d4fc..4c1ae3a540 100644 --- a/Kernel/Graphics/Intel/Plane/G33DisplayPlane.h +++ b/Kernel/Graphics/Intel/Plane/G33DisplayPlane.h @@ -18,7 +18,7 @@ class IntelG33DisplayPlane final : public IntelDisplayPlane { public: static ErrorOr> create_with_physical_address(PhysicalAddress plane_registers_start_address); - virtual ErrorOr set_plane_settings(Badge, PhysicalAddress aperture_start, PipeSelect, size_t horizontal_active_pixels_count) override; + virtual ErrorOr enable(Badge) override; private: explicit IntelG33DisplayPlane(Memory::TypedMapping plane_registers_mapping);