Kernel: Abstract FramebufferConsole away from contiguous physical range

Currently, Kernel::Graphics::FramebufferConsole is written assuming that
the underlying framebuffer memory exists in physically contiguous
memory. There are a bunch of framebuffer devices that would need to use
the components of FramebufferConsole (in particular access to the kernel
bitmap font rendering logic). To reduce code duplication, framebuffer
console has been split into two parts, the abstract
GenericFramebufferConsole class which does the rendering, and the
ContiguousFramebufferConsole class which contains all logic related to
managing the underling vm object.

Also, a new flush method has been added to the class, to support devices
that require an extra flush step to render.

Kernel/CMakeLists.txt

@@ -59,7 +59,8 @@ set(KERNEL_SOURCES
     Devices/HID/PS2KeyboardDevice.cpp
     Devices/HID/PS2MouseDevice.cpp
     Devices/HID/VMWareMouseDevice.cpp
-    Graphics/Console/FramebufferConsole.cpp
+    Graphics/Console/GenericFramebufferConsole.cpp
+    Graphics/Console/ContiguousFramebufferConsole.cpp
     Graphics/Console/TextModeConsole.cpp
     Graphics/Console/VGAConsole.cpp
     Graphics/BochsGraphicsAdapter.cpp

Kernel/Graphics/BochsGraphicsAdapter.cpp

@@ -10,7 +10,7 @@
 #include <Kernel/Debug.h>
 #include <Kernel/Graphics/Bochs.h>
 #include <Kernel/Graphics/BochsGraphicsAdapter.h>
-#include <Kernel/Graphics/Console/FramebufferConsole.h>
+#include <Kernel/Graphics/Console/ContiguousFramebufferConsole.h>
 #include <Kernel/Graphics/GraphicsManagement.h>
 #include <Kernel/IO.h>
 #include <Kernel/PCI/Access.h>
@@ -55,7 +55,7 @@ UNMAP_AFTER_INIT BochsGraphicsAdapter::BochsGraphicsAdapter(PCI::Address pci_add
     , m_mmio_registers(PCI::get_BAR2(pci_address) & 0xfffffff0)
 {
     // We assume safe resolutio is 1024x768x32
-    m_framebuffer_console = Graphics::FramebufferConsole::initialize(PhysicalAddress(PCI::get_BAR0(pci_address) & 0xfffffff0), 1024, 768, 1024 * sizeof(u32));
+    m_framebuffer_console = Graphics::ContiguousFramebufferConsole::initialize(PhysicalAddress(PCI::get_BAR0(pci_address) & 0xfffffff0), 1024, 768, 1024 * sizeof(u32));
     // FIXME: This is a very wrong way to do this...
     GraphicsManagement::the().m_console = m_framebuffer_console;
 

Kernel/Graphics/BochsGraphicsAdapter.h

@@ -8,7 +8,7 @@
 
 #include <AK/String.h>
 #include <AK/Types.h>
-#include <Kernel/Graphics/Console/FramebufferConsole.h>
+#include <Kernel/Graphics/Console/GenericFramebufferConsole.h>
 #include <Kernel/Graphics/FramebufferDevice.h>
 #include <Kernel/Graphics/GraphicsDevice.h>
 #include <Kernel/PCI/DeviceController.h>
@@ -56,7 +56,7 @@ private:
 
     PhysicalAddress m_mmio_registers;
     RefPtr<FramebufferDevice> m_framebuffer_device;
-    RefPtr<Graphics::FramebufferConsole> m_framebuffer_console;
+    RefPtr<Graphics::GenericFramebufferConsole> m_framebuffer_console;
     SpinLock<u8> m_console_mode_switch_lock;
     bool m_console_enabled { false };
     bool m_io_required { false };

Kernel/Graphics/Console/Console.h

@@ -54,10 +54,10 @@ public:
     virtual void hide_cursor() = 0;
     virtual void show_cursor() = 0;
 
-    virtual void clear(size_t x, size_t y, size_t length) const = 0;
-    virtual void write(size_t x, size_t y, char ch, Color background, Color foreground, bool critical = false) const = 0;
-    virtual void write(size_t x, size_t y, char ch, bool critical = false) const = 0;
-    virtual void write(char ch, bool critical = false) const = 0;
+    virtual void clear(size_t x, size_t y, size_t length) = 0;
+    virtual void write(size_t x, size_t y, char ch, Color background, Color foreground, bool critical = false) = 0;
+    virtual void write(size_t x, size_t y, char ch, bool critical = false) = 0;
+    virtual void write(char ch, bool critical = false) = 0;
 
     virtual ~Console() { }
 

Kernel/Graphics/Console/ContiguousFramebufferConsole.cpp

@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2021, Sahan Fernando <sahan.h.fernando@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <Kernel/Graphics/Console/ContiguousFramebufferConsole.h>
+#include <Kernel/TTY/ConsoleManagement.h>
+
+namespace Kernel::Graphics {
+
+NonnullRefPtr<ContiguousFramebufferConsole> ContiguousFramebufferConsole::initialize(PhysicalAddress framebuffer_address, size_t width, size_t height, size_t pitch)
+{
+    return adopt_ref(*new ContiguousFramebufferConsole(framebuffer_address, width, height, pitch));
+}
+
+ContiguousFramebufferConsole::ContiguousFramebufferConsole(PhysicalAddress framebuffer_address, size_t width, size_t height, size_t pitch)
+    : GenericFramebufferConsole(width, height, pitch)
+    , m_framebuffer_address(framebuffer_address)
+{
+    set_resolution(width, height, pitch);
+}
+
+void ContiguousFramebufferConsole::set_resolution(size_t width, size_t height, size_t pitch)
+{
+    m_width = width;
+    m_height = height;
+    m_pitch = pitch;
+
+    dbgln("Framebuffer Console: taking {} bytes", page_round_up(pitch * height));
+    m_framebuffer_region = MM.allocate_kernel_region(m_framebuffer_address, page_round_up(pitch * height), "Framebuffer Console", Region::Access::Read | Region::Access::Write, Region::Cacheable::Yes);
+    VERIFY(m_framebuffer_region);
+
+    // Just to start cleanly, we clean the entire framebuffer
+    memset(m_framebuffer_region->vaddr().as_ptr(), 0, pitch * height);
+
+    ConsoleManagement::the().resolution_was_changed();
+}
+
+}

Kernel/Graphics/Console/ContiguousFramebufferConsole.h

@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2021, Sahan Fernando <sahan.h.fernando@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <Kernel/Graphics/Console/GenericFramebufferConsole.h>
+
+namespace Kernel::Graphics {
+
+class ContiguousFramebufferConsole final : public GenericFramebufferConsole {
+public:
+    static NonnullRefPtr<ContiguousFramebufferConsole> initialize(PhysicalAddress, size_t width, size_t height, size_t pitch);
+
+    virtual void set_resolution(size_t width, size_t height, size_t pitch) override;
+    virtual void flush() override { }
+
+private:
+    virtual u8* framebuffer_data() override
+    {
+        return m_framebuffer_region->vaddr().as_ptr();
+    }
+    OwnPtr<Region> m_framebuffer_region;
+    ContiguousFramebufferConsole(PhysicalAddress, size_t width, size_t height, size_t pitch);
+    PhysicalAddress m_framebuffer_address;
+};
+
+}

Kernel/Graphics/Console/GenericFramebufferConsole.cpp

@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include <Kernel/Graphics/Console/FramebufferConsole.h>
+#include <Kernel/Graphics/Console/GenericFramebufferConsole.h>
 #include <Kernel/TTY/ConsoleManagement.h>
 
 namespace Kernel::Graphics {
@@ -200,65 +200,39 @@ static inline BGRColor convert_standard_color_to_bgr_color(Console::Color color)
     }
 }
 
-NonnullRefPtr<FramebufferConsole> FramebufferConsole::initialize(PhysicalAddress framebuffer_address, size_t width, size_t height, size_t pitch)
-{
-    return adopt_ref(*new FramebufferConsole(framebuffer_address, width, height, pitch));
-}
-
-void FramebufferConsole::set_resolution(size_t width, size_t height, size_t pitch)
-{
-    m_width = width;
-    m_height = height;
-    m_pitch = pitch;
-
-    dbgln("Framebuffer Console: taking {} bytes", page_round_up(pitch * height));
-    m_framebuffer_region = MM.allocate_kernel_region(m_framebuffer_address, page_round_up(pitch * height), "Framebuffer Console", Region::Access::Read | Region::Access::Write, Region::Cacheable::Yes);
-    VERIFY(m_framebuffer_region);
-
-    // Just to start cleanly, we clean the entire framebuffer
-    memset(m_framebuffer_region->vaddr().as_ptr(), 0, pitch * height);
-
-    ConsoleManagement::the().resolution_was_changed();
-}
-
-FramebufferConsole::FramebufferConsole(PhysicalAddress framebuffer_address, size_t width, size_t height, size_t pitch)
-    : Console(width, height)
-    , m_framebuffer_address(framebuffer_address)
-    , m_pitch(pitch)
-{
-    set_resolution(width, height, pitch);
-}
-
-size_t FramebufferConsole::bytes_per_base_glyph() const
+size_t GenericFramebufferConsole::bytes_per_base_glyph() const
 {
     // FIXME: We assume we have 32 bit bpp framebuffer.
     return 8 * 32;
 }
-size_t FramebufferConsole::chars_per_line() const
+size_t GenericFramebufferConsole::chars_per_line() const
 {
     return width() / bytes_per_base_glyph();
 }
 
-void FramebufferConsole::set_cursor(size_t, size_t)
-{
-}
-void FramebufferConsole::hide_cursor()
-{
-}
-void FramebufferConsole::show_cursor()
+void GenericFramebufferConsole::set_cursor(size_t, size_t)
 {
 }
 
-void FramebufferConsole::clear(size_t x, size_t y, size_t length) const
+void GenericFramebufferConsole::hide_cursor()
+{
+}
+
+void GenericFramebufferConsole::show_cursor()
+{
+}
+
+void GenericFramebufferConsole::clear(size_t x, size_t y, size_t length)
 {
     ScopedSpinLock lock(m_lock);
     if (x == 0 && length == max_column()) {
         // if we need to clear the entire row, just clean it with quick memset :)
-        auto* offset_in_framebuffer = (u32*)m_framebuffer_region->vaddr().offset(x * sizeof(u32) * 8).offset(y * 8 * sizeof(u32) * width()).as_ptr();
+        auto* offset_in_framebuffer = (u32*)&framebuffer_data()[x * sizeof(u32) * 8 + y * 8 * sizeof(u32) * width()];
         for (size_t current_x = 0; current_x < 8; current_x++) {
             memset(offset_in_framebuffer, 0, width() * sizeof(u32));
             offset_in_framebuffer = (u32*)((u8*)offset_in_framebuffer + width() * 4);
         }
+        flush();
         return;
     }
     for (size_t index = 0; index < length; index++) {
@@ -268,33 +242,39 @@ void FramebufferConsole::clear(size_t x, size_t y, size_t length) const
             if (y >= max_row())
                 y = 0;
         }
-        clear_glyph(x, y);
-    }
-}
-
-void FramebufferConsole::clear_glyph(size_t x, size_t y) const
-{
-    VERIFY(m_lock.is_locked());
-    auto* offset_in_framebuffer = (u32*)m_framebuffer_region->vaddr().offset(x * sizeof(u32) * 8).offset(y * 8 * sizeof(u32) * width()).as_ptr();
+        auto* offset_in_framebuffer = (u32*)&framebuffer_data()[x * sizeof(u32) * 8 + y * 8 * sizeof(u32) * width()];
         for (size_t current_x = 0; current_x < 8; current_x++) {
             memset(offset_in_framebuffer, 0, 8 * sizeof(u32));
             offset_in_framebuffer = (u32*)((u8*)offset_in_framebuffer + width() * sizeof(u32));
         }
     }
+    flush();
+}
 
-void FramebufferConsole::enable()
+void GenericFramebufferConsole::clear_glyph(size_t x, size_t y)
+{
+    VERIFY(m_lock.is_locked());
+    auto* offset_in_framebuffer = (u32*)&framebuffer_data()[x * sizeof(u32) * 8 + y * 8 * sizeof(u32) * width()];
+    for (size_t current_x = 0; current_x < 8; current_x++) {
+        memset(offset_in_framebuffer, 0, 8 * sizeof(u32));
+        offset_in_framebuffer = (u32*)((u8*)offset_in_framebuffer + width() * sizeof(u32));
+    }
+    flush();
+}
+
+void GenericFramebufferConsole::enable()
 {
     ScopedSpinLock lock(m_lock);
-    memset(m_framebuffer_region->vaddr().as_ptr(), 0, height() * width() * sizeof(u32));
+    memset(framebuffer_data(), 0, height() * width() * sizeof(u32));
     m_enabled.store(true);
 }
-void FramebufferConsole::disable()
+void GenericFramebufferConsole::disable()
 {
     ScopedSpinLock lock(m_lock);
     m_enabled.store(false);
 }
 
-void FramebufferConsole::write(size_t x, size_t y, char ch, Color background, Color foreground, bool critical) const
+void GenericFramebufferConsole::write(size_t x, size_t y, char ch, Color background, Color foreground, bool critical)
 {
     ScopedSpinLock lock(m_lock);
     if (!m_enabled.load())
@@ -315,7 +295,7 @@ void FramebufferConsole::write(size_t x, size_t y, char ch, Color background, Co
         return;
     }
     clear_glyph(x, y);
-    auto* offset_in_framebuffer = (u32*)m_framebuffer_region->vaddr().offset(x * sizeof(u32) * 8).offset(y * 8 * sizeof(u32) * width()).as_ptr();
+    auto* offset_in_framebuffer = (u32*)&framebuffer_data()[x * sizeof(u32) * 8 + y * 8 * sizeof(u32) * width()];
     int current_bitpixels = 0;
     int current_bitpixel = 0;
     auto bitmap = font8x8_basic[(int)ch];
@@ -340,14 +320,15 @@ void FramebufferConsole::write(size_t x, size_t y, char ch, Color background, Co
         if (m_y >= max_row())
             m_y = 0;
     }
+    flush();
 }
 
-void FramebufferConsole::write(size_t x, size_t y, char ch, bool critical) const
+void GenericFramebufferConsole::write(size_t x, size_t y, char ch, bool critical)
 {
     write(x, y, ch, m_default_background_color, m_default_foreground_color, critical);
 }
 
-void FramebufferConsole::write(char ch, bool critical) const
+void GenericFramebufferConsole::write(char ch, bool critical)
 {
     write(m_x, m_y, ch, m_default_background_color, m_default_foreground_color, critical);
 }

Kernel/Graphics/Console/GenericFramebufferConsole.h

@@ -12,12 +12,9 @@
 #include <Kernel/PhysicalAddress.h>
 
 namespace Kernel::Graphics {
-class FramebufferConsole final : public Console {
+
+class GenericFramebufferConsole : public Console {
 public:
-    static NonnullRefPtr<FramebufferConsole> initialize(PhysicalAddress, size_t width, size_t height, size_t pitch);
-
-    void set_resolution(size_t width, size_t height, size_t pitch);
-
     virtual size_t bytes_per_base_glyph() const override;
     virtual size_t chars_per_line() const override;
 
@@ -31,19 +28,25 @@ public:
     virtual void hide_cursor() override;
     virtual void show_cursor() override;
 
-    virtual void clear(size_t x, size_t y, size_t length) const override;
-    virtual void write(size_t x, size_t y, char ch, Color background, Color foreground, bool critical = false) const override;
-    virtual void write(size_t x, size_t y, char ch, bool critical = false) const override;
-    virtual void write(char ch, bool critical = false) const override;
+    virtual void clear(size_t x, size_t y, size_t length) override;
+    virtual void write(size_t x, size_t y, char ch, Color background, Color foreground, bool critical = false) override;
+    virtual void write(size_t x, size_t y, char ch, bool critical = false) override;
+    virtual void write(char ch, bool critical = false) override;
 
     virtual void enable() override;
     virtual void disable() override;
 
+    virtual void set_resolution(size_t width, size_t height, size_t pitch) = 0;
+    virtual void flush() = 0;
+
 protected:
-    void clear_glyph(size_t x, size_t y) const;
-    FramebufferConsole(PhysicalAddress, size_t width, size_t height, size_t pitch);
-    OwnPtr<Region> m_framebuffer_region;
-    PhysicalAddress m_framebuffer_address;
+    GenericFramebufferConsole(size_t width, size_t height, size_t pitch)
+        : Console(width, height)
+        , m_pitch(pitch)
+    {
+    }
+    virtual u8* framebuffer_data() = 0;
+    void clear_glyph(size_t x, size_t y);
     size_t m_pitch;
     mutable SpinLock<u8> m_lock;
 };

Kernel/Graphics/Console/TextModeConsole.cpp

@@ -112,7 +112,7 @@ void TextModeConsole::show_cursor()
     IO::out8(0x3D5, 0x20);
 }
 
-void TextModeConsole::clear(size_t x, size_t y, size_t length) const
+void TextModeConsole::clear(size_t x, size_t y, size_t length)
 {
     ScopedSpinLock lock(m_vga_lock);
     auto* buf = (u16*)(m_current_vga_window + (x * 2) + (y * width() * 2));
@@ -120,12 +120,12 @@ void TextModeConsole::clear(size_t x, size_t y, size_t length) const
         buf[index] = 0x0720;
     }
 }
-void TextModeConsole::write(size_t x, size_t y, char ch, bool critical) const
+void TextModeConsole::write(size_t x, size_t y, char ch, bool critical)
 {
     write(x, y, ch, m_default_background_color, m_default_foreground_color, critical);
 }
 
-void TextModeConsole::write(size_t x, size_t y, char ch, Color background, Color foreground, bool critical) const
+void TextModeConsole::write(size_t x, size_t y, char ch, Color background, Color foreground, bool critical)
 {
     ScopedSpinLock lock(m_vga_lock);
     // If we are in critical printing mode, we need to handle new lines here
@@ -172,7 +172,7 @@ void TextModeConsole::set_vga_start_row(u16 row)
     IO::out8(0x3d5, LSB(m_current_vga_start_address));
 }
 
-void TextModeConsole::write(char ch, bool critical) const
+void TextModeConsole::write(char ch, bool critical)
 {
     write(m_x, m_y, ch, critical);
 }

Kernel/Graphics/Console/TextModeConsole.h

@@ -24,10 +24,10 @@ public:
     virtual void set_cursor(size_t x, size_t y) override;
     virtual void hide_cursor() override;
     virtual void show_cursor() override;
-    virtual void clear(size_t x, size_t y, size_t length) const override;
-    virtual void write(size_t x, size_t y, char ch, bool critical = false) const override;
-    virtual void write(size_t x, size_t y, char ch, Color background, Color foreground, bool critical = false) const override;
-    virtual void write(char ch, bool critical = false) const override;
+    virtual void clear(size_t x, size_t y, size_t length) override;
+    virtual void write(size_t x, size_t y, char ch, bool critical = false) override;
+    virtual void write(size_t x, size_t y, char ch, Color background, Color foreground, bool critical = false) override;
+    virtual void write(char ch, bool critical = false) override;
 
     virtual void enable() override { }
     virtual void disable() override { VERIFY_NOT_REACHED(); }

Kernel/Graphics/FramebufferDevice.h

@@ -38,7 +38,7 @@ public:
 
 private:
     // ^File
-    virtual const char* class_name() const { return "FramebufferDevice"; }
+    virtual const char* class_name() const override { return "FramebufferDevice"; }
 
     virtual bool can_read(const FileDescription&, size_t) const override final { return true; }
     virtual bool can_write(const FileDescription&, size_t) const override final { return true; }

Kernel/Graphics/GraphicsManagement.cpp

@@ -9,7 +9,6 @@
 #include <Kernel/CommandLine.h>
 #include <Kernel/Debug.h>
 #include <Kernel/Graphics/BochsGraphicsAdapter.h>
-#include <Kernel/Graphics/Console/FramebufferConsole.h>
 #include <Kernel/Graphics/Console/TextModeConsole.h>
 #include <Kernel/Graphics/GraphicsManagement.h>
 #include <Kernel/Graphics/IntelNativeGraphicsAdapter.h>

Kernel/Graphics/IntelNativeGraphicsAdapter.cpp

@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include <Kernel/Graphics/Console/FramebufferConsole.h>
+#include <Kernel/Graphics/Console/ContiguousFramebufferConsole.h>
 #include <Kernel/Graphics/Definitions.h>
 #include <Kernel/Graphics/GraphicsManagement.h>
 #include <Kernel/Graphics/IntelNativeGraphicsAdapter.h>
@@ -206,7 +206,7 @@ IntelNativeGraphicsAdapter::IntelNativeGraphicsAdapter(PCI::Address address)
     VERIFY(m_framebuffer_pitch != 0);
     VERIFY(m_framebuffer_height != 0);
     VERIFY(m_framebuffer_width != 0);
-    m_framebuffer_console = Graphics::FramebufferConsole::initialize(framebuffer_address, m_framebuffer_width, m_framebuffer_height, m_framebuffer_pitch);
+    m_framebuffer_console = Graphics::ContiguousFramebufferConsole::initialize(framebuffer_address, m_framebuffer_width, m_framebuffer_height, m_framebuffer_pitch);
     // FIXME: This is a very wrong way to do this...
     GraphicsManagement::the().m_console = m_framebuffer_console;
 }

Kernel/Graphics/VGACompatibleAdapter.cpp

@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include <Kernel/Graphics/Console/FramebufferConsole.h>
+#include <Kernel/Graphics/Console/ContiguousFramebufferConsole.h>
 #include <Kernel/Graphics/Console/TextModeConsole.h>
 #include <Kernel/Graphics/GraphicsManagement.h>
 #include <Kernel/Graphics/VGACompatibleAdapter.h>
@@ -53,7 +53,7 @@ UNMAP_AFTER_INIT VGACompatibleAdapter::VGACompatibleAdapter(PCI::Address address
     , m_framebuffer_height(framebuffer_height)
     , m_framebuffer_pitch(framebuffer_pitch)
 {
-    m_framebuffer_console = Graphics::FramebufferConsole::initialize(framebuffer_address, framebuffer_width, framebuffer_height, framebuffer_pitch);
+    m_framebuffer_console = Graphics::ContiguousFramebufferConsole::initialize(framebuffer_address, framebuffer_width, framebuffer_height, framebuffer_pitch);
     // FIXME: This is a very wrong way to do this...
     GraphicsManagement::the().m_console = m_framebuffer_console;
 }