Kernel: Split I2C functionality from IntelNativeDisplayConnector code

Splitting the I2C-related code lets the DisplayConnector code to utilize
I2C operations without caring about the specific details of the hardware
and allow future expansion of the driver to other newer generations
sharing the same GMBus code.

We should require a timeout for GMBus operations always, because faulty
hardware could let us just spin forever. Also, if nothing is listening
to the bus (which should result in a NAK), we could also spin forever.

Kernel/CMakeLists.txt

@@ -74,6 +74,7 @@ set(KERNEL_SOURCES
     Graphics/DisplayConnector.cpp
     Graphics/Generic/DisplayConnector.cpp
     Graphics/GraphicsManagement.cpp
+    Graphics/Intel/Auxiliary/GMBusConnector.cpp
     Graphics/Intel/NativeDisplayConnector.cpp
     Graphics/Intel/NativeGraphicsAdapter.cpp
     Graphics/VMWare/Console.cpp

Kernel/Graphics/Intel/Auxiliary/GMBusConnector.cpp

@@ -0,0 +1,123 @@
+/*
+ * Copyright (c) 2022, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <Kernel/Arch/Delay.h>
+#include <Kernel/Graphics/Intel/Auxiliary/GMBusConnector.h>
+#include <Kernel/PhysicalAddress.h>
+
+namespace Kernel {
+
+enum class GMBusStatus {
+    TransactionCompletion,
+    HardwareReady
+};
+
+enum GMBusCycle {
+    Wait = 1,
+    Stop = 4,
+};
+
+ErrorOr<NonnullOwnPtr<GMBusConnector>> GMBusConnector::create_with_physical_address(PhysicalAddress gmbus_start_address)
+{
+    auto registers_mapping = TRY(map_typed<GMBusRegisters volatile>(gmbus_start_address, sizeof(GMBusRegisters), Memory::Region::Access::ReadWrite));
+    return adopt_nonnull_own_or_enomem(new (nothrow) GMBusConnector(move(registers_mapping)));
+}
+
+GMBusConnector::GMBusConnector(Memory::TypedMapping<GMBusRegisters volatile> registers_mapping)
+    : m_gmbus_registers(move(registers_mapping))
+{
+    set_default_rate();
+    set_pin_pair(PinPair::DedicatedAnalog);
+}
+
+bool GMBusConnector::wait_for(GMBusStatus desired_status, size_t milliseconds_timeout)
+{
+    VERIFY(m_access_lock.is_locked());
+    size_t milliseconds_passed = 0;
+    while (1) {
+        if (milliseconds_timeout < milliseconds_passed)
+            return false;
+        full_memory_barrier();
+        u32 status = m_gmbus_registers->status;
+        full_memory_barrier();
+        VERIFY(!(status & (1 << 10))); // error happened
+        switch (desired_status) {
+        case GMBusStatus::HardwareReady:
+            if (status & (1 << 11))
+                return true;
+            break;
+        case GMBusStatus::TransactionCompletion:
+            if (status & (1 << 14))
+                return true;
+            break;
+        default:
+            VERIFY_NOT_REACHED();
+        }
+        microseconds_delay(1000);
+        milliseconds_passed++;
+    }
+}
+
+ErrorOr<void> GMBusConnector::write(unsigned address, u32 data)
+{
+    VERIFY(address < 256);
+    SpinlockLocker locker(m_access_lock);
+    full_memory_barrier();
+    m_gmbus_registers->data = data;
+    full_memory_barrier();
+    m_gmbus_registers->command = ((address << 1) | (1 << 16) | (GMBusCycle::Wait << 25) | (1 << 30));
+    full_memory_barrier();
+    if (!wait_for(GMBusStatus::TransactionCompletion, 250))
+        return Error::from_errno(EBUSY);
+    return {};
+}
+
+void GMBusConnector::set_default_rate()
+{
+    // FIXME: Verify GMBUS Rate Select is set only when GMBUS is idle
+    SpinlockLocker locker(m_access_lock);
+    // Set the rate to 100KHz
+    m_gmbus_registers->clock = m_gmbus_registers->clock & ~(0b111 << 8);
+}
+
+void GMBusConnector::set_pin_pair(PinPair pin_pair)
+{
+    // FIXME: Verify GMBUS is idle
+    SpinlockLocker locker(m_access_lock);
+    m_gmbus_registers->clock = (m_gmbus_registers->clock & (~0b111)) | (pin_pair & 0b111);
+}
+
+ErrorOr<void> GMBusConnector::read(unsigned address, u8* buf, size_t length)
+{
+    VERIFY(address < 256);
+    SpinlockLocker locker(m_access_lock);
+    size_t nread = 0;
+    auto read_set = [&] {
+        full_memory_barrier();
+        u32 data = m_gmbus_registers->data;
+        full_memory_barrier();
+        for (size_t index = 0; index < 4; index++) {
+            if (nread == length)
+                break;
+            buf[nread] = (data >> (8 * index)) & 0xFF;
+            nread++;
+        }
+    };
+
+    full_memory_barrier();
+    m_gmbus_registers->command = (1 | (address << 1) | (length << 16) | (GMBusCycle::Wait << 25) | (1 << 30));
+    full_memory_barrier();
+    while (nread < length) {
+        if (!wait_for(GMBusStatus::HardwareReady, 250))
+            return Error::from_errno(EBUSY);
+        read_set();
+    }
+    if (!wait_for(GMBusStatus::TransactionCompletion, 250))
+        return Error::from_errno(EBUSY);
+    return {};
+}
+
+}

Kernel/Graphics/Intel/Auxiliary/GMBusConnector.h

@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2022, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <AK/RefPtr.h>
+#include <AK/Try.h>
+#include <AK/Types.h>
+#include <Kernel/Locking/Spinlock.h>
+#include <Kernel/Memory/TypedMapping.h>
+
+namespace Kernel {
+
+struct [[gnu::packed]] GMBusRegisters {
+    u32 clock;
+    u32 command;
+    u32 status;
+    u32 data;
+};
+
+enum class GMBusStatus;
+
+class GMBusConnector {
+public:
+    enum PinPair : u8 {
+        None = 0,
+        DedicatedControl = 1,
+        DedicatedAnalog = 0b10,
+        IntegratedDigital = 0b11,
+        sDVO = 0b101,
+        Dconnector = 0b111,
+    };
+
+public:
+    static ErrorOr<NonnullOwnPtr<GMBusConnector>> create_with_physical_address(PhysicalAddress gmbus_start_address);
+
+    ErrorOr<void> write(unsigned address, u32 data);
+    ErrorOr<void> read(unsigned address, u8* buf, size_t length);
+    void set_default_rate();
+
+private:
+    void set_pin_pair(PinPair pin_pair);
+
+    bool wait_for(GMBusStatus desired_status, size_t milliseconds_timeout);
+
+    explicit GMBusConnector(Memory::TypedMapping<GMBusRegisters volatile>);
+    Spinlock<LockRank::None> m_access_lock;
+    Memory::TypedMapping<GMBusRegisters volatile> m_gmbus_registers;
+};
+}

Kernel/Graphics/Intel/NativeDisplayConnector.cpp

@@ -178,7 +178,11 @@ Optional<IntelGraphics::PLLSettings> IntelNativeDisplayConnector::create_pll_set
 ErrorOr<NonnullLockRefPtr<IntelNativeDisplayConnector>> IntelNativeDisplayConnector::try_create(PhysicalAddress framebuffer_address, size_t framebuffer_resource_size, PhysicalAddress registers_region_address, size_t registers_region_length)
 {
     auto registers_region = TRY(MM.allocate_kernel_region(PhysicalAddress(registers_region_address), registers_region_length, "Intel Native Graphics Registers"sv, Memory::Region::Access::ReadWrite));
-    auto connector = TRY(DeviceManagement::try_create_device<IntelNativeDisplayConnector>(framebuffer_address, framebuffer_resource_size, move(registers_region)));
+
+    // FIXME: Try to put the address as parameter to this function to allow creating this DisplayConnector for many generations...
+    auto gmbus_connector = TRY(GMBusConnector::create_with_physical_address(registers_region_address.offset(to_underlying(IntelGraphics::RegisterIndex::GMBusClock))));
+
+    auto connector = TRY(DeviceManagement::try_create_device<IntelNativeDisplayConnector>(framebuffer_address, framebuffer_resource_size, move(gmbus_connector), move(registers_region)));
     TRY(connector->initialize_gmbus_settings_and_read_edid());
     // Note: This is very important to set the resolution to something safe so we
     // can create a framebuffer console with valid resolution.
@@ -232,15 +236,11 @@ ErrorOr<void> IntelNativeDisplayConnector::create_attached_framebuffer_console()
     return {};
 }
 
-IntelNativeDisplayConnector::IntelNativeDisplayConnector(PhysicalAddress framebuffer_address, size_t framebuffer_resource_size, NonnullOwnPtr<Memory::Region> registers_region)
+IntelNativeDisplayConnector::IntelNativeDisplayConnector(PhysicalAddress framebuffer_address, size_t framebuffer_resource_size, NonnullOwnPtr<GMBusConnector> gmbus_connector, NonnullOwnPtr<Memory::Region> registers_region)
     : DisplayConnector(framebuffer_address, framebuffer_resource_size, true)
     , m_registers_region(move(registers_region))
+    , m_gmbus_connector(move(gmbus_connector))
 {
-    {
-        SpinlockLocker control_lock(m_control_lock);
-        set_gmbus_default_rate();
-        set_gmbus_pin_pair(IntelGraphics::GMBusPinPair::DedicatedAnalog);
-    }
 }
 
 ErrorOr<void> IntelNativeDisplayConnector::set_mode_setting(DisplayConnector::ModeSetting const&)
@@ -346,79 +346,13 @@ bool IntelNativeDisplayConnector::pipe_b_enabled() const
     return read_from_register(IntelGraphics::RegisterIndex::PipeBConf) & (1 << 30);
 }
 
-bool IntelNativeDisplayConnector::gmbus_wait_for(IntelGraphics::GMBusStatus desired_status, Optional<size_t> milliseconds_timeout)
-{
-    VERIFY(m_control_lock.is_locked());
-    size_t milliseconds_passed = 0;
-    while (1) {
-        if (milliseconds_timeout.has_value() && milliseconds_timeout.value() < milliseconds_passed)
-            return false;
-        full_memory_barrier();
-        u32 status = read_from_register(IntelGraphics::RegisterIndex::GMBusStatus);
-        full_memory_barrier();
-        VERIFY(!(status & (1 << 10))); // error happened
-        switch (desired_status) {
-        case IntelGraphics::GMBusStatus::HardwareReady:
-            if (status & (1 << 11))
-                return true;
-            break;
-        case IntelGraphics::GMBusStatus::TransactionCompletion:
-            if (status & (1 << 14))
-                return true;
-            break;
-        default:
-            VERIFY_NOT_REACHED();
-        }
-        microseconds_delay(1000);
-        milliseconds_passed++;
-    }
-}
-
-void IntelNativeDisplayConnector::gmbus_write(unsigned address, u32 byte)
-{
-    VERIFY(m_control_lock.is_locked());
-    VERIFY(address < 256);
-    full_memory_barrier();
-    write_to_register(IntelGraphics::RegisterIndex::GMBusData, byte);
-    full_memory_barrier();
-    write_to_register(IntelGraphics::RegisterIndex::GMBusCommand, ((address << 1) | (1 << 16) | (IntelGraphics::GMBusCycle::Wait << 25) | (1 << 30)));
-    full_memory_barrier();
-    gmbus_wait_for(IntelGraphics::GMBusStatus::TransactionCompletion, {});
-}
-void IntelNativeDisplayConnector::gmbus_read(unsigned address, u8* buf, size_t length)
-{
-    VERIFY(address < 256);
-    VERIFY(m_control_lock.is_locked());
-    size_t nread = 0;
-    auto read_set = [&] {
-        full_memory_barrier();
-        u32 data = read_from_register(IntelGraphics::RegisterIndex::GMBusData);
-        full_memory_barrier();
-        for (size_t index = 0; index < 4; index++) {
-            if (nread == length)
-                break;
-            buf[nread] = (data >> (8 * index)) & 0xFF;
-            nread++;
-        }
-    };
-
-    full_memory_barrier();
-    write_to_register(IntelGraphics::RegisterIndex::GMBusCommand, (1 | (address << 1) | (length << 16) | (IntelGraphics::GMBusCycle::Wait << 25) | (1 << 30)));
-    full_memory_barrier();
-    while (nread < length) {
-        gmbus_wait_for(IntelGraphics::GMBusStatus::HardwareReady, {});
-        read_set();
-    }
-    gmbus_wait_for(IntelGraphics::GMBusStatus::TransactionCompletion, {});
-}
-
 void IntelNativeDisplayConnector::gmbus_read_edid()
 {
     Array<u8, 128> crt_edid_bytes {};
     {
         SpinlockLocker control_lock(m_control_lock);
-        gmbus_write(DDC2_I2C_ADDRESS, 0);
-        gmbus_read(DDC2_I2C_ADDRESS, crt_edid_bytes.data(), crt_edid_bytes.size());
+        MUST(m_gmbus_connector->write(DDC2_I2C_ADDRESS, 0));
+        MUST(m_gmbus_connector->read(DDC2_I2C_ADDRESS, crt_edid_bytes.data(), crt_edid_bytes.size()));
     }
     set_edid_bytes(crt_edid_bytes);
 }
@@ -604,14 +538,6 @@ void IntelNativeDisplayConnector::disable_pipe_b()
     dbgln_if(INTEL_GRAPHICS_DEBUG, "Disabling Pipe B - done.");
 }
 
-void IntelNativeDisplayConnector::set_gmbus_default_rate()
-{
-    // FIXME: Verify GMBUS Rate Select is set only when GMBUS is idle
-    VERIFY(m_control_lock.is_locked());
-    // Set the rate to 100KHz
-    write_to_register(IntelGraphics::RegisterIndex::GMBusClock, read_from_register(IntelGraphics::RegisterIndex::GMBusClock) & ~(0b111 << 8));
-}
-
 void IntelNativeDisplayConnector::enable_pipe_a()
 {
     VERIFY(m_control_lock.is_locked());
@@ -667,13 +593,6 @@ void IntelNativeDisplayConnector::enable_dpll_without_vga(IntelGraphics::PLLSett
     VERIFY(read_from_register(IntelGraphics::RegisterIndex::DPLLControlA) & (1 << 31));
 }
 
-void IntelNativeDisplayConnector::set_gmbus_pin_pair(IntelGraphics::GMBusPinPair pin_pair)
-{
-    // FIXME: Verify GMBUS is idle
-    VERIFY(m_control_lock.is_locked());
-    write_to_register(IntelGraphics::RegisterIndex::GMBusClock, (read_from_register(IntelGraphics::RegisterIndex::GMBusClock) & (~0b111)) | (pin_pair & 0b111));
-}
-
 void IntelNativeDisplayConnector::disable_dac_output()
 {
     VERIFY(m_control_lock.is_locked());

Kernel/Graphics/Intel/NativeDisplayConnector.h

@@ -10,6 +10,7 @@
 #include <Kernel/Graphics/Console/GenericFramebufferConsole.h>
 #include <Kernel/Graphics/Definitions.h>
 #include <Kernel/Graphics/DisplayConnector.h>
+#include <Kernel/Graphics/Intel/Auxiliary/GMBusConnector.h>
 #include <Kernel/Library/LockRefPtr.h>
 #include <Kernel/Memory/TypedMapping.h>
 
@@ -54,25 +55,6 @@ struct PLLParameterLimit {
 struct PLLMaxSettings {
     PLLParameterLimit dot_clock, vco, n, m, m1, m2, p, p1, p2;
 };
-
-enum GMBusPinPair : u8 {
-    None = 0,
-    DedicatedControl = 1,
-    DedicatedAnalog = 0b10,
-    IntegratedDigital = 0b11,
-    sDVO = 0b101,
-    Dconnector = 0b111,
-};
-
-enum class GMBusStatus {
-    TransactionCompletion,
-    HardwareReady,
-};
-
-enum GMBusCycle {
-    Wait = 1,
-    Stop = 4,
-};
 }
 
 class IntelNativeDisplayConnector final
@@ -101,11 +83,10 @@ private:
     // Note: Paravirtualized hardware doesn't require a defined refresh rate for modesetting.
     virtual bool refresh_rate_support() const override { return true; }
 
-    ErrorOr<void> initialize_gmbus_settings_and_read_edid();
-
-    IntelNativeDisplayConnector(PhysicalAddress framebuffer_address, size_t framebuffer_resource_size, NonnullOwnPtr<Memory::Region> registers_region);
+    IntelNativeDisplayConnector(PhysicalAddress framebuffer_address, size_t framebuffer_resource_size, NonnullOwnPtr<GMBusConnector>, NonnullOwnPtr<Memory::Region> registers_region);
 
     ErrorOr<void> create_attached_framebuffer_console();
+    ErrorOr<void> initialize_gmbus_settings_and_read_edid();
 
     void write_to_register(IntelGraphics::RegisterIndex, u32 value) const;
     u32 read_from_register(IntelGraphics::RegisterIndex) const;
@@ -142,14 +123,7 @@ private:
     bool wait_for_disabled_pipe_a(size_t milliseconds_timeout) const;
     bool wait_for_disabled_pipe_b(size_t milliseconds_timeout) const;
 
-    void set_gmbus_default_rate();
-    void set_gmbus_pin_pair(IntelGraphics::GMBusPinPair pin_pair);
-
-    // FIXME: It would be better if we generalize the I2C access later on
     void gmbus_read_edid();
-    void gmbus_write(unsigned address, u32 byte);
-    void gmbus_read(unsigned address, u8* buf, size_t length);
-    bool gmbus_wait_for(IntelGraphics::GMBusStatus desired_status, Optional<size_t> milliseconds_timeout);
 
     Optional<IntelGraphics::PLLSettings> create_pll_settings(u64 target_frequency, u64 reference_clock, IntelGraphics::PLLMaxSettings const&);
 
@@ -158,5 +132,6 @@ private:
 
     const PhysicalAddress m_registers;
     NonnullOwnPtr<Memory::Region> m_registers_region;
+    NonnullOwnPtr<GMBusConnector> m_gmbus_connector;
 };
 }