dwc_eqos - use ACPI DSM to change TX_CLOCK (#29)

- Remove hard-coded TX_CLOCK register management. - Replace direct TX_CLOCK register update with ACPI DSM. - Add guard against running UpdateLinkState work item more than once at a time. - Update readme.
worproject · Jan 2, 2024 · 43c80a1 · 43c80a1
1 parent a004e69
commit 43c80a1
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 57 deletions.
diff --git a/README.md b/README.md
@@ -26,7 +26,7 @@ This repository contains drivers for RK35xx-based platforms, with a focus on RK3
 |Multimedia codecs||🔴 Not working||
 |DSI||🔴 Not working||
 |CSI||🔴 Not working||
-|GMAC Ethernet|[dwc_eqos](drivers/net/dwc_eqos)|🟡 Partially working|1000Mbps only (gigabit Ethernet only - 10/100 not yet supported)|
+|GMAC Ethernet|[dwc_eqos](drivers/net/dwc_eqos)|🟢 Working|Requires latest UEFI (edk2-rk3588 master 2024/01/03 or later).<br> Note that this driver supports the on-chip Ethernet (e.g. it supports the 1GB Ethernet port on Opi5) but does not support the separate Ethernet chip that is added to some higher-tier boards (e.g. it does not support the 2.5GB Ethernet ports on Opi5+).
 |UART||🔴 Not working|No OS driver but debugging does work on UART2, being configured by UEFI.|
 |GPIO|[rk3xgpio](https://github.com/worproject/Rockchip-Windows-Drivers/tree/master/drivers/gpio)|🟢 Working||
 |I2C|[rk3xi2c](https://github.com/worproject/Rockchip-Windows-Drivers/tree/master/drivers/i2c)|🟢 Working||

diff --git a/drivers/net/dwc_eqos/device.cpp b/drivers/net/dwc_eqos/device.cpp
@@ -31,6 +31,11 @@ static auto constexpr QueuesSupported = 1u; // TODO: Support multiple queues?
 static auto constexpr InterruptLinkStatus = 0x80000000u;
 static auto constexpr InterruptChannel0Status = ~InterruptLinkStatus;
 
+// D637828D-556C-4829-966A-237072F00FF1
+static GUID constexpr DsmGuid = { 0xD637828D, 0x556C, 0x4829, 0x96, 0x6A, 0x23, 0x70, 0x72, 0xF0, 0x0F, 0xF1 };
+static auto constexpr DsmSetTxClockRevision = 0u;
+static auto constexpr DsmSetTxClockIndex = 1u;
+
 // Updated by DevicePerfRegister/DevicePerfUnregister.
 static WDFWAITLOCK g_devicesLock = nullptr; // Guards g_devices.
 static WDFCOLLECTION g_devices = nullptr;   // Guarded by g_devicesLock.
@@ -52,9 +57,10 @@ struct DeviceContext
     MacRegisters* regs;
     NETADAPTER adapter;
     WDFSPINLOCK queueLock;
+    WDFWORKITEM updateLinkStateWorkItem;
+    DEVICE_OBJECT* devicePdo;
     WDFINTERRUPT interrupt;
     WDFDMAENABLER dma;
-    WDFWORKITEM linkStateWorkItem;
     UINT32 perfCounterDeviceId; // = (regs physical address) >> 4
     MacHwFeature0_t feature0;
     MacHwFeature1_t feature1;
@@ -64,13 +70,10 @@ struct DeviceContext
     UINT8 currentMacAddress[ETHERNET_LENGTH_OF_ADDRESS];
     DeviceConfig config;
 
-    // TODO: call ACPI to change phy clock speed.
-    UINT8 php_grf_clk_con1_gmac_shift; // 0 for GMAC0, 5 for GMAC1.
-    UINT32* php_grf_clk_con1_regs;
-
     // Mutable.
 
-    ChannelStatus_t interruptStatus; // Channel0 + InterruptLinkStatus. Interlocked update.
+    ChannelStatus_t interruptStatus;    // Channel0 + InterruptLinkStatus. Interlocked update.
+    char updateLinkStateBusy;           // 0 = idle, 1 = busy. Interlocked update.
 
     InterruptsWanted interruptsWanted;  // Guarded by interrupt lock.
     NETPACKETQUEUE rxQueue;             // Guarded by queueLock.
@@ -161,49 +164,50 @@ UpdateLinkState(_In_ DeviceContext const* context)
     auto newConfig = oldConfig;
     newConfig.FullDuplex = controlStatus.FullDuplex;
 
-    // TODO: call ACPI to change phy clock speed.
-    // Write-enable for bits 3:2, plus constants for bits 3:2.
-    // Shift left by 0 for GMAC0.
-    // Shift left by 5 for GMAC1.
     UINT32 clockVal;
-    enum mii_tx_clk_sel_gamc : UINT32
-    {
-        mii_tx_clk_sel_gamc_rgmii_002_5 = 0xC0008, // 2.5Mhz = b'10
-        mii_tx_clk_sel_gamc_rgmii_025 = 0xC000C, // 25Mhz = b'11
-        mii_tx_clk_sel_gamc_rgmii_125 = 0xC0000, // 125Mhz = b'00
-    };
-
     UINT32 speed;
     switch (controlStatus.Speed)
     {
     case PhyIfSpeed_2_5M:
         speed = 10'000'000u;
-        clockVal = mii_tx_clk_sel_gamc_rgmii_002_5;
+        clockVal = 10;
         newConfig.PortSelectSpeed = PortSelectSpeed_10M;
         break;
     case PhyIfSpeed_25M:
         speed = 100'000'000u;
-        clockVal = mii_tx_clk_sel_gamc_rgmii_025;
+        clockVal = 100;
         newConfig.PortSelectSpeed = PortSelectSpeed_100M;
         break;
     case PhyIfSpeed_125M:
         speed = 1'000'000'000u;
-        clockVal = mii_tx_clk_sel_gamc_rgmii_125;
+        clockVal = 1000;
         newConfig.PortSelectSpeed = PortSelectSpeed_1000M;
         break;
     default:
         speed = 0;
-        clockVal = mii_tx_clk_sel_gamc_rgmii_125;
+        clockVal = 1000;
         break;
     }
 
-    if (context->php_grf_clk_con1_regs != nullptr)
+    ACPI_EVAL_OUTPUT_BUFFER UNALIGNED* returnBufferPtr = nullptr;
+    ACPI_METHOD_ARGUMENT clockArg = {};
+    ACPI_METHOD_SET_ARGUMENT_INTEGER((&clockArg), clockVal);
+    NTSTATUS status = AcpiExecuteDsmFunction(
+        context->devicePdo,
+        &DsmGuid,
+        DsmSetTxClockRevision,
+        DsmSetTxClockIndex,
+        &clockArg,
+        sizeof(clockArg),
+        &returnBufferPtr);
+    if (returnBufferPtr != nullptr)
     {
-        clockVal <<= context->php_grf_clk_con1_gmac_shift;
-        Write32(context->php_grf_clk_con1_regs, clockVal);
-        TraceWrite("UpdateLinkState-clock", LEVEL_INFO,
-            TraceLoggingHexInt32(clockVal),
-            TraceLoggingPointer(context->php_grf_clk_con1_regs, "php_grf_clk_con1_regs"));
+        ExFreePoolWithTag(returnBufferPtr, ACPI_TAG_EVAL_OUTPUT_BUFFER);
+    }
+    if (!NT_SUCCESS(status))
+    {
+        TraceWrite("UpdateLinkState-SetTxClock", LEVEL_WARNING,
+            TraceLoggingNTStatus(status));
     }
 
     if (oldConfig.Value32 != newConfig.Value32)
@@ -389,7 +393,7 @@ DeviceInterruptDpc(
         {
             context->dpcLinkState += 1;
             Read32(&context->regs->Mac_PhyIf_Control_Status); // Clears LinkStatus interrupt.
-            WdfWorkItemEnqueue(context->linkStateWorkItem);
+            WdfWorkItemEnqueue(context->updateLinkStateWorkItem);
         }
 
         if (newInterruptStatus.Rx || newInterruptStatus.Tx)
@@ -446,8 +450,12 @@ DeviceLinkStateWorkItem(
 {
     // PASSIVE_LEVEL, nonpaged (resume path)
     auto const context = DeviceGetContext(WdfWorkItemGetParentObject(workItem));
-    NT_ASSERT(context->linkStateWorkItem == workItem);
-    UpdateLinkState(context);
+    NT_ASSERT(context->updateLinkStateWorkItem == workItem);
+    if (0 == InterlockedExchangeNoFence8(&context->updateLinkStateBusy, 1))
+    {
+        UpdateLinkState(context);
+        InterlockedExchangeNoFence8(&context->updateLinkStateBusy, 0);
+    }
 }
 
 static EVT_NET_ADAPTER_CREATE_TXQUEUE AdapterCreateTxQueue;
@@ -637,6 +645,7 @@ DeviceD0Entry(
 
     // Clear and then enable interrupts.
 
+    NT_ASSERT(ReadNoFence8(&context->updateLinkStateBusy) == 0);
     UpdateLinkState(context); // Clears LinkStatus interrupt.
     Write32(&context->regs->Dma_Ch[0].Status, ChannelStatus_t(~0u));
     DeviceInterruptEnable(context, InterruptsState);
@@ -663,6 +672,8 @@ DeviceD0Exit(
     auto const context = DeviceGetContext(device);
 
     DeviceInterruptDisable(context, InterruptsAll);
+    WdfWorkItemFlush(context->updateLinkStateWorkItem);
+    NT_ASSERT(ReadNoFence8(&context->updateLinkStateBusy) == 0);
 
     NT_ASSERT(context->txQueue == nullptr);
     NT_ASSERT(context->rxQueue == nullptr);
@@ -776,20 +787,6 @@ DevicePrepareHardware(
                     }
 
                     context->perfCounterDeviceId = static_cast<UINT32>(descRaw->u.Memory.Start.QuadPart >> 4);
-
-                    context->php_grf_clk_con1_gmac_shift = 0;
-                    switch (descRaw->u.Memory.Start.QuadPart)
-                    {
-                    case 0xFE1C0000:
-                        context->php_grf_clk_con1_gmac_shift = 5;
-                        __fallthrough;
-                    case 0xFE1B0000:
-                        context->php_grf_clk_con1_regs = static_cast<UINT32*>(MmMapIoSpaceEx(
-                            PHYSICAL_ADDRESS{ 0xFD5B0070, 0 },
-                            sizeof(UINT32),
-                            PAGE_READWRITE | PAGE_NOCACHE));
-                        break;
-                    }
                 }
                 break;
 
@@ -1255,13 +1252,6 @@ DeviceReleaseHardware(
     UNREFERENCED_PARAMETER(resourcesTranslated);
 
     auto const context = DeviceGetContext(device);
-
-    if (context->php_grf_clk_con1_regs != nullptr)
-    {
-        MmUnmapIoSpace(context->php_grf_clk_con1_regs, sizeof(UINT32));
-        context->php_grf_clk_con1_regs = nullptr;
-    }
-
     if (context->regs != nullptr)
     {
         DeviceReset(context->regs, context->permanentMacAddress);
@@ -1437,13 +1427,15 @@ DeviceAdd(
 
         WDF_WORKITEM_CONFIG workItemConfig;
         WDF_WORKITEM_CONFIG_INIT(&workItemConfig, DeviceLinkStateWorkItem);
-        status = WdfWorkItemCreate(&workItemConfig, &attributes, &context->linkStateWorkItem);
+        status = WdfWorkItemCreate(&workItemConfig, &attributes, &context->updateLinkStateWorkItem);
         if (!NT_SUCCESS(status))
         {
             TraceWrite("WdfWorkItemCreate-failed", LEVEL_ERROR,
                 TraceLoggingNTStatus(status));
             goto Done;
         }
+
+        context->devicePdo = WdfDeviceWdmGetPhysicalDevice(device);
     }
 
     // Create adapter.

diff --git a/drivers/net/dwc_eqos/driver.cpp b/drivers/net/dwc_eqos/driver.cpp
@@ -3,11 +3,11 @@
 #include "trace.h"
 
 /*
-TODO list:
-- Use ACPI DSM to change TX_CLK instead of hard-coding clock address in driver.
+Possible areas for improvement:
+- Run against network test suites and fix any issues.
+- Interrupt moderation and/or other CPU usage improvements.
 - Jumbo frames.
-- Receive queue memory optimization?
-- Configuration in registry (e.g. flow control).
+- Configuration in registry (e.g. flow control, speed, duplex).
 - Tx segmentation offload.
 - Rx segmentation offload.
 - Wake-on-LAN.