From d147bfc1fc8c416293f99d12f8e32e633b2ae5c4 Mon Sep 17 00:00:00 2001 From: Eugene Shalygin Date: Mon, 7 Feb 2022 15:53:23 +0000 Subject: [PATCH] Unify EC sensors implementation with Linux HWMON driver (#648) --- .../Hardware/Motherboard/Identification.cs | 12 + .../Motherboard/Lpc/EC/EmbeddedController.cs | 234 +++++++++++------- .../Lpc/EC/EmbeddedControllerSource.cs | 4 +- .../Lpc/EC/WindowsEmbeddedController.cs | 2 +- .../Lpc/EC/WindowsEmbeddedControllerIO.cs | 29 ++- .../Hardware/Motherboard/Model.cs | 6 + 6 files changed, 190 insertions(+), 97 deletions(-) diff --git a/LibreHardwareMonitorLib/Hardware/Motherboard/Identification.cs b/LibreHardwareMonitorLib/Hardware/Motherboard/Identification.cs index fbf8879..1230a0d 100644 --- a/LibreHardwareMonitorLib/Hardware/Motherboard/Identification.cs +++ b/LibreHardwareMonitorLib/Hardware/Motherboard/Identification.cs @@ -149,6 +149,8 @@ namespace LibreHardwareMonitor.Hardware.Motherboard return Model.ROG_CROSSHAIR_VIII_DARK_HERO; case var _ when name.Equals("ROG CROSSHAIR VIII FORMULA", StringComparison.OrdinalIgnoreCase): return Model.ROG_CROSSHAIR_VIII_FORMULA; + case var _ when name.Equals("ROG CROSSHAIR VIII IMPACT", StringComparison.OrdinalIgnoreCase): + return Model.ROG_CROSSHAIR_VIII_IMPACT; case var _ when name.Equals("M2N-SLI DELUXE", StringComparison.OrdinalIgnoreCase): return Model.M2N_SLI_Deluxe; case var _ when name.Equals("M4A79XTD EVO", StringComparison.OrdinalIgnoreCase): @@ -182,10 +184,16 @@ namespace LibreHardwareMonitor.Hardware.Motherboard return Model.ROG_STRIX_B550_F_GAMING_WIFI; case var _ when name.Equals("ROG STRIX X470-I GAMING", StringComparison.OrdinalIgnoreCase): return Model.ROG_STRIX_X470_I; + case var _ when name.Equals("ROG STRIX B550-E GAMING", StringComparison.OrdinalIgnoreCase): + return Model.ROG_STRIX_B550_E_GAMING; case var _ when name.Equals("ROG STRIX B550-I GAMING", StringComparison.OrdinalIgnoreCase): return Model.ROG_STRIX_B550_I_GAMING; case var _ when name.Equals("ROG STRIX X570-E GAMING", StringComparison.OrdinalIgnoreCase): return Model.ROG_STRIX_X570_E_GAMING; + case var _ when name.Equals("ROG STRIX X570-I GAMING", StringComparison.OrdinalIgnoreCase): + return Model.ROG_STRIX_X570_I_GAMING; + case var _ when name.Equals("ROG STRIX X570-F GAMING", StringComparison.OrdinalIgnoreCase): + return Model.ROG_STRIX_X570_F_GAMING; case var _ when name.Equals("LP DK P55-T3eH9", StringComparison.OrdinalIgnoreCase): return Model.LP_DK_P55_T3EH9; case var _ when name.Equals("A890GXM-A", StringComparison.OrdinalIgnoreCase): @@ -289,6 +297,10 @@ namespace LibreHardwareMonitor.Hardware.Motherboard return Model.AX370_Gaming_K7; case var _ when name.Equals("PRIME X370-PRO", StringComparison.OrdinalIgnoreCase): return Model.PRIME_X370_PRO; + case var _ when name.Equals("PRIME X570-PRO", StringComparison.OrdinalIgnoreCase): + return Model.PRIME_X570_PRO; + case var _ when name.Equals("Pro WS X570-ACE", StringComparison.OrdinalIgnoreCase): + return Model.PRO_WS_X570_ACE; case var _ when name.Equals("ROG MAXIMUS X APEX", StringComparison.OrdinalIgnoreCase): return Model.ROG_MAXIMUS_X_APEX; case var _ when name.Equals("AB350-Gaming 3-CF", StringComparison.OrdinalIgnoreCase): diff --git a/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/EmbeddedController.cs b/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/EmbeddedController.cs index f169c7e..7a52baf 100644 --- a/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/EmbeddedController.cs +++ b/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/EmbeddedController.cs @@ -6,20 +6,150 @@ using System; using System.Collections.Generic; using System.Globalization; +using System.Linq; using System.Text; namespace LibreHardwareMonitor.Hardware.Motherboard.Lpc.EC { public abstract class EmbeddedController : Hardware { + private enum ECSensor + { + /// Chipset temperature [℃] + TempChipset, + /// CPU temperature [℃] + TempCPU, + /// motherboard temperature [℃] + TempMB, + /// "T_Sensor" temperature sensor reading [℃] + TempTSensor, + /// VRM temperature [℃] + TempVrm, + /// CPU_Opt fan [RPM] + FanCPUOpt, + /// VRM heat sink fan [RPM] + FanVrmHS, + /// Chipset fan [RPM] + FanChipset, + /// Water flow sensor reading [RPM] + FanWaterFlow, + /// CPU current [A] + CurrCPU, + /// "Water_In" temperature sensor reading [℃] + TempWaterIn, + /// "Water_Out" temperature sensor reading [℃] + TempWaterOut, + Max + }; + + private static readonly Dictionary _knownSensors = new() + { + { ECSensor.TempChipset, new EmbeddedControllerSource("Chipset", SensorType.Temperature, 0x003a) }, + { ECSensor.TempCPU, new EmbeddedControllerSource("CPU", SensorType.Temperature, 0x003b) }, + { ECSensor.TempMB, new EmbeddedControllerSource("Motherboard", SensorType.Temperature, 0x003c) }, + { ECSensor.TempTSensor, new EmbeddedControllerSource("T Sensor", SensorType.Temperature, 0x003d, blank: -40) }, + { ECSensor.TempVrm, new EmbeddedControllerSource("VRM", SensorType.Temperature, 0x003e) }, + { ECSensor.FanCPUOpt, new EmbeddedControllerSource("CPU Optional Fan", SensorType.Fan, 0x00b0, 2) }, + { ECSensor.FanVrmHS, new EmbeddedControllerSource("VRM Heat Sink Fan", SensorType.Fan, 0x00b2, 2) }, + { ECSensor.FanChipset, new EmbeddedControllerSource("Chipset Fan", SensorType.Fan, 0x00b4, 2) }, + // TODO: "why 42?" is a silly question, I know, but still, why? On the serious side, it might be 41.6(6) + { ECSensor.FanWaterFlow, new EmbeddedControllerSource("Water flow", SensorType.Flow, 0x00bc, 2, factor: 1.0f / 42f * 60f) }, + { ECSensor.CurrCPU, new EmbeddedControllerSource("CPU", SensorType.Current, 0x00f4) }, + { ECSensor.TempWaterIn, new EmbeddedControllerSource("Water In", SensorType.Temperature, 0x0100, blank: -40) }, + { ECSensor.TempWaterOut, new EmbeddedControllerSource("Water Out", SensorType.Temperature, 0x0101, blank: -40) }, + }; + + private static readonly Dictionary _boardSensors = new() + { + { + Model.PRIME_X570_PRO, + new ECSensor[] { ECSensor.TempChipset, ECSensor.TempCPU, ECSensor.TempMB, + ECSensor.TempVrm, ECSensor.TempTSensor, ECSensor.FanChipset } + }, + { + Model.PRO_WS_X570_ACE, + new ECSensor[] { ECSensor.TempChipset, ECSensor.TempCPU, ECSensor.TempMB , + ECSensor.TempVrm, ECSensor.FanChipset, ECSensor.CurrCPU} + }, + { + Model.ROG_CROSSHAIR_VIII_HERO, + new ECSensor[] { ECSensor.TempChipset, ECSensor.TempCPU, ECSensor.TempMB, + ECSensor.TempTSensor, ECSensor.TempVrm, ECSensor.TempWaterIn, ECSensor.TempWaterOut, + ECSensor.FanCPUOpt, ECSensor.FanChipset, ECSensor.FanWaterFlow, ECSensor.CurrCPU} + }, + { + Model.ROG_CROSSHAIR_VIII_HERO_WIFI, + new ECSensor[] { ECSensor.TempChipset, ECSensor.TempCPU, ECSensor.TempMB, + ECSensor.TempTSensor, ECSensor.TempVrm, ECSensor.TempWaterIn, ECSensor.TempWaterOut, + ECSensor.FanCPUOpt, ECSensor.FanChipset, ECSensor.FanWaterFlow, ECSensor.CurrCPU} + }, + { + Model.ROG_CROSSHAIR_VIII_DARK_HERO, + new ECSensor[] { ECSensor.TempChipset, ECSensor.TempCPU, ECSensor.TempMB, + ECSensor.TempTSensor, ECSensor.TempVrm, ECSensor.TempWaterIn, ECSensor.TempWaterOut, + ECSensor.FanCPUOpt, ECSensor.FanWaterFlow, ECSensor.CurrCPU + } + }, + { + Model.CROSSHAIR_III_FORMULA, + new ECSensor[] { ECSensor.TempChipset, ECSensor.TempCPU, ECSensor.TempMB, + ECSensor.TempTSensor, ECSensor.TempVrm, + ECSensor.FanCPUOpt, ECSensor.FanChipset, ECSensor.CurrCPU } + }, + { + Model.ROG_CROSSHAIR_VIII_IMPACT, + new ECSensor[] { ECSensor.TempChipset, ECSensor.TempCPU, ECSensor.TempMB, + ECSensor.TempTSensor, ECSensor.TempVrm, + ECSensor.FanChipset, ECSensor.CurrCPU } + }, + { + Model.ROG_STRIX_B550_E_GAMING, + new ECSensor[] { ECSensor.TempChipset, ECSensor.TempCPU, ECSensor.TempMB, + ECSensor.TempTSensor, ECSensor.TempVrm, ECSensor.FanCPUOpt } + }, + { + Model.ROG_STRIX_B550_I_GAMING, + new ECSensor[] { ECSensor.TempChipset, ECSensor.TempCPU, ECSensor.TempMB, + ECSensor.TempTSensor, ECSensor.TempVrm, + ECSensor.FanVrmHS, ECSensor.CurrCPU } + }, + { + Model.ROG_STRIX_X570_E_GAMING, + new ECSensor[] { ECSensor.TempChipset, ECSensor.TempCPU, ECSensor.TempMB, + ECSensor.TempTSensor, ECSensor.TempVrm, + ECSensor.FanChipset, ECSensor.CurrCPU } + }, + { + Model.ROG_STRIX_X570_F_GAMING, + new ECSensor[]{ECSensor.TempChipset, ECSensor.TempCPU, ECSensor.TempMB, + ECSensor.TempTSensor, ECSensor.FanChipset} + }, + { + Model.ROG_STRIX_X570_I_GAMING, + new ECSensor[] { + ECSensor.TempTSensor, ECSensor.FanVrmHS, ECSensor.FanChipset, ECSensor.CurrCPU } + } + }; + + static EmbeddedController() + { + System.Diagnostics.Debug.Assert(_knownSensors.Count == ((int)ECSensor.Max)); + } + private readonly IReadOnlyList _sources; private readonly List _sensors; private readonly ushort[] _registers; private readonly byte[] _data; - protected EmbeddedController(List sources, ISettings settings) : base("Embedded Controller", new Identifier("lpc", "ec"), settings) + protected EmbeddedController(IEnumerable sources, ISettings settings) : base("Embedded Controller", new Identifier("lpc", "ec"), settings) { - _sources = sources; + // sorting by address, which implies sorting by bank, for optimized EC access + var sourcesList = sources.ToList(); + sourcesList.Sort((left, right) => + { + return left.Register.CompareTo(right.Register); + }); + _sources = sourcesList; var indices = new Dictionary(); foreach (SensorType t in Enum.GetValues(typeof(SensorType))) { @@ -28,7 +158,7 @@ namespace LibreHardwareMonitor.Hardware.Motherboard.Lpc.EC _sensors = new List(); List registers = new(); - foreach (EmbeddedControllerSource s in sources) + foreach (EmbeddedControllerSource s in _sources) { int index = indices[s.Type]; indices[s.Type] = index + 1; @@ -49,82 +179,10 @@ namespace LibreHardwareMonitor.Hardware.Motherboard.Lpc.EC internal static EmbeddedController Create(Model model, ISettings settings) { - var sources = new List(); - - switch (model) + if (_boardSensors.TryGetValue(model, out ECSensor[] sensors)) { - case Model.ROG_STRIX_X570_E_GAMING: - case Model.ROG_CROSSHAIR_VIII_HERO: - case Model.ROG_CROSSHAIR_VIII_HERO_WIFI: - case Model.ROG_CROSSHAIR_VIII_DARK_HERO: - case Model.ROG_CROSSHAIR_VIII_FORMULA: - case Model.ROG_STRIX_B550_I_GAMING: - { - sources.AddRange(new EmbeddedControllerSource[] - { - new("Chipset", SensorType.Temperature, 0x003A, 1), - new("CPU", SensorType.Temperature, 0x003B, 1), - new("Motherboard", SensorType.Temperature, 0x003C, 1), - new("T Sensor", SensorType.Temperature, 0x003D, 1, blank: 0xD8), - new("CPU", SensorType.Current, 0x00F4, 1) - }); + var sources = sensors.Select(ecs => _knownSensors[ecs]); - break; - } - } - - switch (model) - { - case Model.ROG_STRIX_X570_E_GAMING: - case Model.ROG_CROSSHAIR_VIII_HERO: - case Model.ROG_CROSSHAIR_VIII_HERO_WIFI: - case Model.ROG_CROSSHAIR_VIII_DARK_HERO: - case Model.ROG_CROSSHAIR_VIII_FORMULA: - { - sources.Add(new EmbeddedControllerSource("VRM", SensorType.Temperature, 0x003E, 1)); - sources.Add(new EmbeddedControllerSource("CPU Opt", SensorType.Fan, 0x00B0, 2)); - break; - } - } - - switch (model) - { - case Model.ROG_STRIX_X570_E_GAMING: - case Model.ROG_CROSSHAIR_VIII_HERO: - case Model.ROG_CROSSHAIR_VIII_HERO_WIFI: - case Model.ROG_CROSSHAIR_VIII_FORMULA: - { - sources.Add(new EmbeddedControllerSource("Chipset", SensorType.Fan, 0x00B4, 2)); - break; - } - } - - switch (model) - { - case Model.ROG_STRIX_B550_I_GAMING: - { - sources.Add(new EmbeddedControllerSource("VRM HS Fan", SensorType.Fan, 0x00B2, 2)); - break; - } - } - - switch (model) - { - case Model.ROG_CROSSHAIR_VIII_HERO: - case Model.ROG_CROSSHAIR_VIII_HERO_WIFI: - case Model.ROG_CROSSHAIR_VIII_DARK_HERO: - case Model.ROG_CROSSHAIR_VIII_FORMULA: - { - // TODO: "why 42?" is a silly question, I know, but still, why? On the serious side, it might be 41.6(6) - sources.Add(new EmbeddedControllerSource("Flow Rate", SensorType.Flow, 0x00BC, 2, 1.0f / 42f * 60f)); - sources.Add(new EmbeddedControllerSource("Water In", SensorType.Temperature, 0x0100, 1, blank: 0xD8)); - sources.Add(new EmbeddedControllerSource("Water Out", SensorType.Temperature, 0x0101, 1, blank: 0xD8)); - break; - } - } - - if (sources.Count > 0) - { return Environment.OSVersion.Platform switch { PlatformID.Win32NT => new WindowsEmbeddedController(sources, settings), @@ -142,16 +200,18 @@ namespace LibreHardwareMonitor.Hardware.Motherboard.Lpc.EC // just skip this update cycle? return; } - + int readRegister = 0; for (int si = 0; si < _sensors.Count; ++si) { - int val = 0; - for (int i = 0; i < _sources[si].Size; ++i, ++readRegister) + int val = _sources[si].Size switch { - val = (val << 8) + _data[readRegister]; - } - + 1 => unchecked((sbyte)_data[readRegister]), + 2 => unchecked((short)((_data[readRegister] << 8) + _data[readRegister + 1])), + _ => 0, + }; + readRegister += _sources[si].Size; + _sensors[si].Value = val != _sources[si].Blank ? val * _sources[si].Factor : null; } } @@ -191,7 +251,7 @@ namespace LibreHardwareMonitor.Hardware.Motherboard.Lpc.EC r.AppendLine(); } } - catch (WindowsEmbeddedControllerIO.BusMutexLockingFailedException e) + catch (IOException e) { r.AppendLine(e.Message); } @@ -209,10 +269,14 @@ namespace LibreHardwareMonitor.Hardware.Motherboard.Lpc.EC embeddedControllerIO.Read(_registers, _data); return true; } - catch (WindowsEmbeddedControllerIO.BusMutexLockingFailedException) + catch (IOException) { return false; } } + + public class IOException: System.IO.IOException { + public IOException(string message): base($"ACPI embedded controller I/O error: {message}") { } + } } } diff --git a/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/EmbeddedControllerSource.cs b/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/EmbeddedControllerSource.cs index a804184..a605bd2 100644 --- a/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/EmbeddedControllerSource.cs +++ b/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/EmbeddedControllerSource.cs @@ -7,7 +7,7 @@ namespace LibreHardwareMonitor.Hardware.Motherboard.Lpc.EC { public class EmbeddedControllerSource { - public EmbeddedControllerSource(string name, SensorType type, ushort register, byte size, float factor = 1.0f, uint blank = uint.MaxValue) + public EmbeddedControllerSource(string name, SensorType type, ushort register, byte size = 1, float factor = 1.0f, int blank = int.MaxValue) { Name = name; @@ -23,7 +23,7 @@ namespace LibreHardwareMonitor.Hardware.Motherboard.Lpc.EC public byte Size { get; } public float Factor { get; } - public uint Blank { get; } + public int Blank { get; } public EmbeddedControllerReader Reader { get; } diff --git a/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/WindowsEmbeddedController.cs b/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/WindowsEmbeddedController.cs index ec0e66b..a49bf6e 100644 --- a/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/WindowsEmbeddedController.cs +++ b/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/WindowsEmbeddedController.cs @@ -9,7 +9,7 @@ namespace LibreHardwareMonitor.Hardware.Motherboard.Lpc.EC { public class WindowsEmbeddedController : EmbeddedController { - public WindowsEmbeddedController(List sources, ISettings settings) : base(sources, settings) + public WindowsEmbeddedController(IEnumerable sources, ISettings settings) : base(sources, settings) { } protected override IEmbeddedControllerIO AcquireIOInterface() diff --git a/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/WindowsEmbeddedControllerIO.cs b/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/WindowsEmbeddedControllerIO.cs index e8d16de..74e7d18 100644 --- a/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/WindowsEmbeddedControllerIO.cs +++ b/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/WindowsEmbeddedControllerIO.cs @@ -41,15 +41,26 @@ namespace LibreHardwareMonitor.Hardware.Motherboard.Lpc.EC Trace.Assert(registers.Length <= data.Length, "data buffer length has to be greater or equal to the registers array length"); - for (int i = 0; i < registers.Length; ++i) - { - byte bank = (byte)(registers[i] >> 8); - byte index = (byte)(registers[i] & 0xFF); + byte bank = 0; + byte prevBank = SwitchBank(bank); - SwitchBank(bank); - data[i] = ReadByte(index); + // oops... somebody else is working with the EC too + Trace.WriteLineIf(prevBank != 0, "Concurrent access to the ACPI EC detected.\nRace condition possible."); + + // read registers minimizing bank switches. + for (int i = 0; i < registers.Length; i++) + { + byte regBank = (byte)(registers[i] >> 8); + byte regIndex = (byte)(registers[i] & 0xFF); + // registers are sorted by bank + if (regBank > bank) + { + bank = SwitchBank(regBank); + } + data[i] = ReadByte(regIndex); } - SwitchBank(0); + + SwitchBank(prevBank); } private byte ReadByte(byte register) @@ -154,10 +165,10 @@ namespace LibreHardwareMonitor.Hardware.Motherboard.Lpc.EC Ring0.WriteIoPort((uint)port, datum); } - public class BusMutexLockingFailedException : ApplicationException + public class BusMutexLockingFailedException : EmbeddedController.IOException { public BusMutexLockingFailedException() - : base("Could not lock ISA bus mutex") + : base("could not lock ISA bus mutex") { } } diff --git a/LibreHardwareMonitorLib/Hardware/Motherboard/Model.cs b/LibreHardwareMonitorLib/Hardware/Motherboard/Model.cs index 10af551..de542ec 100644 --- a/LibreHardwareMonitorLib/Hardware/Motherboard/Model.cs +++ b/LibreHardwareMonitorLib/Hardware/Motherboard/Model.cs @@ -41,8 +41,12 @@ namespace LibreHardwareMonitor.Hardware.Motherboard ROG_CROSSHAIR_VIII_HERO_WIFI, ROG_CROSSHAIR_VIII_DARK_HERO, ROG_CROSSHAIR_VIII_FORMULA, + ROG_CROSSHAIR_VIII_IMPACT, ROG_STRIX_X470_I, ROG_STRIX_X570_E_GAMING, + ROG_STRIX_X570_F_GAMING, + ROG_STRIX_X570_I_GAMING, + ROG_STRIX_B550_E_GAMING, ROG_STRIX_B550_F_GAMING_WIFI, ROG_STRIX_B550_I_GAMING, M2N_SLI_Deluxe, @@ -57,6 +61,8 @@ namespace LibreHardwareMonitor.Hardware.Motherboard P8Z77_V, P9X79, PRIME_X370_PRO, + PRIME_X570_PRO, + PRO_WS_X570_ACE, RAMPAGE_EXTREME, RAMPAGE_II_GENE, ROG_MAXIMUS_X_APEX,