diff --git a/.github/actions/spell-check/expect.txt b/.github/actions/spell-check/expect.txt index 916f30f829..b79f362d73 100644 --- a/.github/actions/spell-check/expect.txt +++ b/.github/actions/spell-check/expect.txt @@ -356,6 +356,7 @@ DCBA DCOM DCR ddc +DDCCI DDEIf Deact debouncer diff --git a/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/DdcErrorClassifierTests.cs b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/DdcErrorClassifierTests.cs new file mode 100644 index 0000000000..c1f075b264 --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/DdcErrorClassifierTests.cs @@ -0,0 +1,85 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Microsoft.VisualStudio.TestTools.UnitTesting; +using PowerDisplay.Common.Drivers.DDC; + +namespace PowerDisplay.UnitTests; + +/// +/// Pins the membership of the DDC/CI error sets. The retry budget and the drop-the-monitor +/// decision both hang off these predicates, so a code silently moving between sets changes how +/// much I2C traffic a flaky panel attracts and whether it stays visible at all. +/// +[TestClass] +public sealed class DdcErrorClassifierTests +{ + /// + /// Pins the numeric values against winerror.h. Every other assertion in this file addresses the + /// codes by name, so a typo in a constant would move production and tests together and leave the + /// whole suite green. + /// + [TestMethod] + public void Constants_MatchWinerrorValues() + { + Assert.AreEqual(unchecked((int)0xC0262582), DdcErrorClassifier.ErrorGraphicsI2CErrorTransmittingData); + Assert.AreEqual(unchecked((int)0xC0262583), DdcErrorClassifier.ErrorGraphicsI2CErrorReceivingData); + Assert.AreEqual(unchecked((int)0xC0262584), DdcErrorClassifier.ErrorGraphicsDdcCiVcpNotSupported); + Assert.AreEqual(unchecked((int)0xC0262585), DdcErrorClassifier.ErrorGraphicsDdcCiInvalidData); + Assert.AreEqual(unchecked((int)0xC0262588), DdcErrorClassifier.ErrorGraphicsMcaInternalError); + Assert.AreEqual(unchecked((int)0xC0262589), DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageCommand); + Assert.AreEqual(unchecked((int)0xC026258A), DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageLength); + Assert.AreEqual(unchecked((int)0xC026258B), DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageChecksum); + Assert.AreEqual(unchecked((int)0xC026258C), DdcErrorClassifier.ErrorGraphicsInvalidPhysicalMonitorHandle); + Assert.AreEqual(unchecked((int)0xC026258D), DdcErrorClassifier.ErrorGraphicsMonitorNoLongerExists); + Assert.AreEqual( + unchecked((int)0xC02625D8), + DdcErrorClassifier.ErrorGraphicsDdcCiCurrentCurrentValueGreaterThanMaximumValue); + Assert.AreEqual(1460, DdcErrorClassifier.ErrorTimeout); + } + + [DataTestMethod] + [DataRow(DdcErrorClassifier.ErrorGraphicsI2CErrorTransmittingData)] + [DataRow(DdcErrorClassifier.ErrorGraphicsI2CErrorReceivingData)] + [DataRow(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidData)] + [DataRow(DdcErrorClassifier.ErrorGraphicsMcaInternalError)] + [DataRow(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageCommand)] + [DataRow(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageLength)] + [DataRow(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageChecksum)] + [DataRow(DdcErrorClassifier.ErrorGraphicsDdcCiCurrentCurrentValueGreaterThanMaximumValue)] + [DataRow(DdcErrorClassifier.ErrorTimeout)] + public void IsTransient_AcceptsRetryableFailures(int errorCode) => + Assert.IsTrue(DdcErrorClassifier.IsTransient(errorCode)); + + [DataTestMethod] + + // See DdcErrorClassifier.IsTransient's for why each of these stays out. The two + // handle-class rows are load-bearing here specifically: ProbeCodeAsync consults IsTransient to + // decide whether to retry, so an overlap with IsPhysicalMonitorUnavailable would keep hammering + // a handle ProbeAsync already knows is gone. + [DataRow(DdcErrorClassifier.ErrorGraphicsDdcCiVcpNotSupported)] + [DataRow(DdcErrorClassifier.ErrorGraphicsInvalidPhysicalMonitorHandle)] + [DataRow(DdcErrorClassifier.ErrorGraphicsMonitorNoLongerExists)] + [DataRow(unchecked((int)0xC0262580))] + [DataRow(unchecked((int)0xC0262581))] + [DataRow(unchecked((int)0xC0262587))] + [DataRow(unchecked((int)0xC0262586))] + [DataRow(0)] + public void IsTransient_RejectsEverythingElse(int errorCode) => + Assert.IsFalse(DdcErrorClassifier.IsTransient(errorCode)); + + [DataTestMethod] + [DataRow(DdcErrorClassifier.ErrorGraphicsInvalidPhysicalMonitorHandle)] + [DataRow(DdcErrorClassifier.ErrorGraphicsMonitorNoLongerExists)] + public void IsPhysicalMonitorUnavailable_AcceptsHandleClassFailures(int errorCode) => + Assert.IsTrue(DdcErrorClassifier.IsPhysicalMonitorUnavailable(errorCode)); + + [DataTestMethod] + [DataRow(DdcErrorClassifier.ErrorGraphicsDdcCiVcpNotSupported)] + [DataRow(DdcErrorClassifier.ErrorGraphicsI2CErrorTransmittingData)] + [DataRow(DdcErrorClassifier.ErrorTimeout)] + [DataRow(0)] + public void IsPhysicalMonitorUnavailable_RejectsFeatureLevelFailures(int errorCode) => + Assert.IsFalse(DdcErrorClassifier.IsPhysicalMonitorUnavailable(errorCode)); +} diff --git a/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/VcpFeatureProbeServiceTests.cs b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/VcpFeatureProbeServiceTests.cs new file mode 100644 index 0000000000..b1577c1a81 --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/VcpFeatureProbeServiceTests.cs @@ -0,0 +1,334 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using PowerDisplay.Common.Drivers.DDC; + +namespace PowerDisplay.UnitTests; + +[TestClass] +public sealed class VcpFeatureProbeServiceTests +{ + [TestMethod] + public async Task ProbeAsync_FirstSuccessReturnsValuesWithoutRetry() + { + var reader = new RecordingVcpReader(VcpReadAttempt.Success(current: 30, maximum: 100)); + var delays = new List(); + var service = CreateService(reader, delays); + + var result = await service.ProbeAsync(new IntPtr(1), CancellationToken.None); + + Assert.AreEqual(1, reader.CallCount); + Assert.IsTrue(result[0x10].IsSuccess); + Assert.AreEqual(30, result[0x10].Value.Current); + Assert.AreEqual(100, result[0x10].Value.Maximum); + Assert.AreEqual(1, result[0x10].Attempts); + Assert.IsNull(result[0x10].LastError); + CollectionAssert.AreEqual(new[] { TimeSpan.FromMilliseconds(100) }, delays); + } + + [TestMethod] + public async Task ProbeAsync_TransientFailureThenSuccessRetriesWithPacing() + { + var reader = new RecordingVcpReader( + VcpReadAttempt.Failure(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageCommand), + VcpReadAttempt.Success(current: 45, maximum: 100)); + var delays = new List(); + var service = CreateService(reader, delays); + + var result = await service.ProbeAsync(new IntPtr(1), CancellationToken.None); + + Assert.AreEqual(2, reader.CallCount); + Assert.IsTrue(result[0x10].IsSuccess); + Assert.AreEqual(2, result[0x10].Attempts); + Assert.AreEqual(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageCommand, result[0x10].LastError); + CollectionAssert.AreEqual( + new[] { TimeSpan.FromMilliseconds(100), TimeSpan.FromMilliseconds(100) }, + delays); + } + + [TestMethod] + public async Task ProbeAsync_ThreeTransientFailuresReturnIndeterminate() + { + var reader = new RecordingVcpReader( + VcpReadAttempt.Failure(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageCommand), + VcpReadAttempt.Failure(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageCommand), + VcpReadAttempt.Failure(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageCommand)); + var service = CreateService(reader, new List()); + + var result = await service.ProbeAsync(new IntPtr(1), CancellationToken.None); + + Assert.AreEqual(3, reader.CallCount); + Assert.IsFalse(result[0x10].IsSuccess); + Assert.AreEqual(3, result[0x10].Attempts); + Assert.AreEqual(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageCommand, result[0x10].LastError); + } + + [TestMethod] + public async Task ProbeAsync_NonTransientFailureDoesNotRetry() + { + var reader = new RecordingVcpReader(VcpReadAttempt.Failure(DdcErrorClassifier.ErrorGraphicsDdcCiVcpNotSupported)); + var service = CreateService(reader, new List()); + + var result = await service.ProbeAsync(new IntPtr(1), CancellationToken.None); + + Assert.AreEqual(1, reader.CallCount); + Assert.IsFalse(result[0x10].IsSuccess); + Assert.IsFalse(result[0x10].Replied); + Assert.AreEqual(1, result[0x10].Attempts); + Assert.AreEqual(DdcErrorClassifier.ErrorGraphicsDdcCiVcpNotSupported, result[0x10].LastError); + } + + [DataTestMethod] + [DataRow(DdcErrorClassifier.ErrorGraphicsInvalidPhysicalMonitorHandle)] + [DataRow(DdcErrorClassifier.ErrorGraphicsMonitorNoLongerExists)] + public async Task ProbeAsync_PhysicalMonitorUnavailableStopsRemainingFeatureProbes(int errorCode) + { + var reader = new RecordingVcpReader(VcpReadAttempt.Failure(errorCode)); + var service = CreateService(reader, new List(), new byte[] { 0x10, 0x12, 0x62 }); + + var result = await service.ProbeAsync(new IntPtr(1), CancellationToken.None); + + Assert.AreEqual(1, reader.CallCount); + CollectionAssert.AreEqual(new byte[] { 0x10 }, reader.Codes); + Assert.AreEqual(1, result.Count); + Assert.IsTrue(result[0x10].IsPhysicalMonitorUnavailable); + Assert.AreEqual(errorCode, result[0x10].LastError); + } + + [TestMethod] + public async Task ProbeAsync_VcpNotSupportedContinuesWithRemainingFeatures() + { + var reader = new RecordingVcpReader( + VcpReadAttempt.Failure(DdcErrorClassifier.ErrorGraphicsDdcCiVcpNotSupported), + VcpReadAttempt.Success(current: 20, maximum: 100)); + var service = CreateService(reader, new List(), new byte[] { 0x10, 0x12 }); + + var result = await service.ProbeAsync(new IntPtr(1), CancellationToken.None); + + Assert.AreEqual(2, reader.CallCount); + CollectionAssert.AreEqual(new byte[] { 0x10, 0x12 }, reader.Codes); + Assert.IsFalse(result[0x10].IsSuccess); + Assert.IsFalse(result[0x10].IsPhysicalMonitorUnavailable); + Assert.IsTrue(result[0x12].IsSuccess); + } + + [TestMethod] + public async Task ProbeAsync_InvalidSuccessfulRangeStopsWithoutRetry() + { + var reader = new RecordingVcpReader( + VcpReadAttempt.Success(current: 10, maximum: 0), + VcpReadAttempt.Success(current: 10, maximum: 0)); + var service = CreateService(reader, new List()); + + var result = await service.ProbeAsync(new IntPtr(1), CancellationToken.None); + + // The reply already settles the only question this probe asks, so the remaining budget is + // not spent on the I2C bus. + Assert.AreEqual(1, reader.CallCount); + Assert.IsFalse(result[0x10].IsSuccess); + Assert.AreEqual(1, result[0x10].Attempts); + Assert.IsNull(result[0x10].LastError); + + // The device answered, so support is proven even though no usable range was obtained. + Assert.IsTrue(result[0x10].Replied); + } + + [TestMethod] + public async Task ProbeAsync_TransientFailureThenInvalidRangeKeepsLastError() + { + var reader = new RecordingVcpReader( + VcpReadAttempt.Failure(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageChecksum), + VcpReadAttempt.Success(current: 10, maximum: 0)); + var service = CreateService(reader, new List()); + + var result = await service.ProbeAsync(new IntPtr(1), CancellationToken.None); + + Assert.AreEqual(2, reader.CallCount); + Assert.AreEqual(2, result[0x10].Attempts); + Assert.IsTrue(result[0x10].Replied); + Assert.IsFalse(result[0x10].IsSuccess); + Assert.IsFalse(result[0x10].IsPhysicalMonitorUnavailable); + + // Returning early on the reply must not discard the error the earlier attempt reported. + Assert.AreEqual(DdcErrorClassifier.ErrorGraphicsDdcCiInvalidMessageChecksum, result[0x10].LastError); + } + + [TestMethod] + public async Task ProbeAsync_ThrowingReadIsContainedToItsOwnCode() + { + // A throwing native read must cost the caller this one VCP code, not every monitor sharing + // the hMonitor through the pipeline-wide catch in DdcCiController. + var reader = new ThrowingReader(0x10, VcpReadAttempt.Success(current: 40, maximum: 100)); + var service = CreateService(reader, new List(), new byte[] { 0x10, 0x12 }); + + var result = await service.ProbeAsync(new IntPtr(1), CancellationToken.None); + + Assert.IsFalse(result[0x10].IsSuccess); + Assert.IsFalse(result[0x10].IsPhysicalMonitorUnavailable); + Assert.IsFalse(result[0x10].Replied); + Assert.IsTrue(result[0x12].IsSuccess); + Assert.AreEqual(40, result[0x12].Value.Current); + } + + [TestMethod] + public async Task ProbeAsync_MultipleCodesRemainSequential() + { + var reader = new RecordingVcpReader( + VcpReadAttempt.Success(10, 100), + VcpReadAttempt.Success(20, 100), + VcpReadAttempt.Success(30, 100)); + var service = CreateService(reader, new List(), new byte[] { 0x10, 0x12, 0x62 }); + + await service.ProbeAsync(new IntPtr(1), CancellationToken.None); + + CollectionAssert.AreEqual(new byte[] { 0x10, 0x12, 0x62 }, reader.Codes); + } + + [TestMethod] + public async Task ProbeAsync_PreCancelledTokenSkipsDelayAndNativeReads() + { + var reader = new RecordingVcpReader(VcpReadAttempt.Success(10, 100)); + var delays = new List(); + using var cancellation = new CancellationTokenSource(); + cancellation.Cancel(); + var service = CreateService(reader, delays, new byte[] { 0x10 }); + + await Assert.ThrowsExceptionAsync( + () => service.ProbeAsync(new IntPtr(1), cancellation.Token)); + + Assert.AreEqual(0, delays.Count); + Assert.AreEqual(0, reader.CallCount); + } + + [TestMethod] + public async Task ProbeAsync_CancellationDuringTransactionDelayStopsBeforeNativeRead() + { + var reader = new RecordingVcpReader(VcpReadAttempt.Success(10, 100)); + var delays = new List(); + var delayStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var cancellation = new CancellationTokenSource(); + var service = new VcpFeatureProbeService( + reader, + async (delay, token) => + { + delays.Add(delay); + delayStarted.TrySetResult(true); + await Task.Delay(Timeout.InfiniteTimeSpan, token); + }, + new byte[] { 0x10 }); + + var probeTask = service.ProbeAsync(new IntPtr(1), cancellation.Token); + + await delayStarted.Task; + cancellation.Cancel(); + + OperationCanceledException? exception = null; + try + { + await probeTask; + } + catch (OperationCanceledException ex) + { + exception = ex; + } + + Assert.IsNotNull(exception); + + CollectionAssert.AreEqual(new[] { TimeSpan.FromMilliseconds(100) }, delays); + Assert.AreEqual(0, reader.CallCount); + } + + [TestMethod] + [Timeout(5000)] + public async Task ProbeAsync_ReaderRunsOffCallerThread() + { + using var releaseReader = new ManualResetEventSlim(); + var callerThreadId = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var readerThreadId = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var invocation = new TaskCompletionSource>>( + TaskCreationOptions.RunContinuationsAsynchronously); + var reader = new CoordinatedReader(readerThreadId, releaseReader); + var service = CreateService(reader, new List()); + var caller = new Thread(() => + { + callerThreadId.TrySetResult(Environment.CurrentManagedThreadId); + invocation.TrySetResult(service.ProbeAsync(new IntPtr(1), CancellationToken.None)); + }); + + caller.Start(); + var callerId = await callerThreadId.Task; + var readerId = await readerThreadId.Task; + releaseReader.Set(); + + Assert.IsTrue(caller.Join(TimeSpan.FromSeconds(1))); + var result = await await invocation.Task; + + Assert.AreNotEqual(callerId, readerId); + Assert.IsTrue(result[0x10].IsSuccess); + } + + private static VcpFeatureProbeService CreateService( + IVcpFeatureReader reader, + List delays, + IReadOnlyList? codes = null) => + new( + reader, + (delay, _) => + { + delays.Add(delay); + return Task.CompletedTask; + }, + codes: codes ?? new byte[] { 0x10 }); + + private sealed class CoordinatedReader( + TaskCompletionSource readerThreadId, + ManualResetEventSlim releaseReader) : IVcpFeatureReader + { + public VcpReadAttempt Read(IntPtr handle, byte code) + { + readerThreadId.TrySetResult(Environment.CurrentManagedThreadId); + releaseReader.Wait(); + return VcpReadAttempt.Success(10, 100); + } + } + + private sealed class ThrowingReader(byte throwingCode, VcpReadAttempt otherwise) : IVcpFeatureReader + { + public VcpReadAttempt Read(IntPtr handle, byte code) => code == throwingCode + ? throw new InvalidOperationException("simulated native failure") + : otherwise; + } + + /// + /// Serves a scripted sequence of read results and records what it was asked for, so a test can + /// pin both how many native reads happened and against which codes. + /// + /// + /// Dequeuing past the end throws rather than yielding a default-valued result, so a fabricated + /// reply never reaches the assertions. The throw is not itself the failure message: it is + /// raised inside the reader, and 's catch-all turns it into + /// an indeterminate observation. An extra read is named by the and + /// assertions instead. + /// + private sealed class RecordingVcpReader(params VcpReadAttempt[] results) : IVcpFeatureReader + { + private readonly Queue _results = new(results); + + public int CallCount { get; private set; } + + public List Codes { get; } = new(); + + public VcpReadAttempt Read(IntPtr handle, byte code) + { + CallCount++; + Codes.Add(code); + return _results.Dequeue(); + } + } +} diff --git a/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/DdcCiController.cs b/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/DdcCiController.cs index 93d158df53..27623096ba 100644 --- a/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/DdcCiController.cs +++ b/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/DdcCiController.cs @@ -34,6 +34,7 @@ namespace PowerDisplay.Common.Drivers.DDC { private readonly PhysicalMonitorHandleManager _handleManager = new(); private readonly MonitorDiscoveryHelper _discoveryHelper; + private readonly VcpFeatureProbeService _probeService; private bool _disposed; @@ -47,6 +48,7 @@ namespace PowerDisplay.Common.Drivers.DDC public DdcCiController() { _discoveryHelper = new MonitorDiscoveryHelper(); + _probeService = new VcpFeatureProbeService(new NativeVcpFeatureReader()); } public string Name => "DDC/CI Monitor Controller"; @@ -445,9 +447,8 @@ namespace PowerDisplay.Common.Drivers.DDC Logger.LogInfo( $"DDC: [max-compat] caps unusable for handle=0x{hPhysicalMonitor:X}; probing VCP features directly"); - caps = await Task.Run( - () => DdcCiNative.ProbeSupportedVcpFeatures(hPhysicalMonitor), - cancellationToken); + var observations = await _probeService.ProbeAsync(hPhysicalMonitor, cancellationToken); + caps = BuildCapabilitiesFromProbe(observations); if (caps != null) { @@ -465,6 +466,34 @@ namespace PowerDisplay.Common.Drivers.DDC return (capsString ?? string.Empty, caps); } + /// + /// Synthesizes capabilities from the VCP codes the device answered for, or null when none + /// did — which the caller treats the same way as an unusable capabilities string. + /// + /// + /// Membership is decided by , not by whether the + /// value was usable. A reply proves the device implements the opcode even when the reported + /// range cannot scale a percentage; an unimplemented code fails with + /// DDCCI_VCP_NOT_SUPPORTED instead and never sets the flag. Friendly names come from + /// to keep a single source of truth. + /// + private static VcpCapabilities? BuildCapabilitiesFromProbe( + IReadOnlyDictionary observations) + { + var caps = new VcpCapabilities(); + + foreach (var observation in observations.Values) + { + if (observation.Replied) + { + caps.SupportedVcpCodes[observation.Code] = + new VcpCodeInfo(observation.Code, VcpNames.GetCodeName(observation.Code)); + } + } + + return caps.SupportedVcpCodes.Count > 0 ? caps : null; + } + /// /// Initialize input source value for a monitor using VCP 0x60. /// diff --git a/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/DdcCiNative.cs b/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/DdcCiNative.cs index 439d9c38ac..bb226e8c5f 100644 --- a/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/DdcCiNative.cs +++ b/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/DdcCiNative.cs @@ -5,31 +5,17 @@ using System; using System.Runtime.InteropServices; using ManagedCommon; -using PowerDisplay.Common.Models; -using PowerDisplay.Common.Utils; -using static PowerDisplay.Common.Drivers.NativeConstants; using static PowerDisplay.Common.Drivers.PInvoke; namespace PowerDisplay.Common.Drivers.DDC { /// - /// DDC/CI native API wrapper — Win32 primitives only. All retry / fallback - /// orchestration lives in . + /// DDC/CI capabilities-string wrapper — Win32 primitives only. All retry / fallback + /// orchestration lives in ; the VCP read primitive lives on + /// , behind the seam. /// public static class DdcCiNative { - // Continuous-range VCP features probed when running in max-compatibility mode. - // Discrete-value features (0x14 color preset, 0x60 input source, 0xD6 power mode) - // are excluded — GetVCPFeatureAndVCPFeatureReply returns only current+max, so we - // cannot synthesize a meaningful supported-value list for them. Friendly names - // come from VcpNames.GetCodeName to keep a single source of truth. - private static readonly byte[] ProbeableContinuousVcpCodes = - { - VcpCodeBrightness, - VcpCodeContrast, - VcpCodeVolume, - }; - /// /// One attempt to get the capabilities string from a physical monitor handle. /// Returns null on any failure. The orchestrator owns retry + warn-level @@ -74,49 +60,5 @@ namespace PowerDisplay.Common.Drivers.DDC return null; } } - - /// - /// Sequentially probes each VCP code in - /// via GetVCPFeatureAndVCPFeatureReply. Used as the max-compatibility-mode fallback - /// when the cap string is empty or unparsable. Returns null if zero probes succeed; - /// otherwise returns a synthetic with only the codes - /// that responded. - /// - /// - /// Sequential, not parallel — physical monitors share an I²C arbitration bus, - /// concurrent reads cause spurious failures. - /// - public static VcpCapabilities? ProbeSupportedVcpFeatures(IntPtr hPhysicalMonitor) - { - if (hPhysicalMonitor == IntPtr.Zero) - { - Logger.LogDebug("DDC: ProbeSupportedVcpFeatures called with IntPtr.Zero"); - return null; - } - - var caps = new VcpCapabilities(); - - foreach (var code in ProbeableContinuousVcpCodes) - { - try - { - if (GetVCPFeatureAndVCPFeatureReply(hPhysicalMonitor, code, IntPtr.Zero, out uint _, out uint _)) - { - caps.SupportedVcpCodes[code] = new VcpCodeInfo(code, VcpNames.GetCodeName(code)); - } - else - { - var lastError = Marshal.GetLastWin32Error(); - Logger.LogDebug($"DDC: [max-compat] probe of VCP 0x{code:X2} failed (handle=0x{hPhysicalMonitor:X}, error={lastError})"); - } - } - catch (Exception ex) when (ex is not OutOfMemoryException) - { - Logger.LogError($"DDC: [max-compat] probe of VCP 0x{code:X2} threw (handle=0x{hPhysicalMonitor:X}): {ex.Message}"); - } - } - - return caps.SupportedVcpCodes.Count > 0 ? caps : null; - } } } diff --git a/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/DdcErrorClassifier.cs b/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/DdcErrorClassifier.cs new file mode 100644 index 0000000000..d200264da8 --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/DdcErrorClassifier.cs @@ -0,0 +1,87 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace PowerDisplay.Common.Drivers.DDC +{ + /// + /// Classifies the Win32 error codes GetVCPFeatureAndVCPFeatureReply reports, so a caller + /// can tell a failure worth another I2C transaction from the device's final answer. Constant + /// names mirror winerror.h exactly. + /// + /// + /// is the only consumer today. The discovery-time value + /// reads in DdcCiController.TryGetVcpFeature are still single-shot and unclassified. + /// + internal static class DdcErrorClassifier + { + internal const int ErrorGraphicsI2CErrorTransmittingData = unchecked((int)0xC0262582); + internal const int ErrorGraphicsI2CErrorReceivingData = unchecked((int)0xC0262583); + + /// + /// The device's final answer that it does not implement the opcode. Deliberately a member of + /// neither classification below — it must not be retried, and it is not a handle-class failure — + /// so it is named here rather than left as a magic number in the tests that pin that exclusion + /// and in the discovery tests that drive a definitive refusal. + /// + internal const int ErrorGraphicsDdcCiVcpNotSupported = unchecked((int)0xC0262584); + + internal const int ErrorGraphicsDdcCiInvalidData = unchecked((int)0xC0262585); + internal const int ErrorGraphicsMcaInternalError = unchecked((int)0xC0262588); + internal const int ErrorGraphicsDdcCiInvalidMessageCommand = unchecked((int)0xC0262589); + internal const int ErrorGraphicsDdcCiInvalidMessageLength = unchecked((int)0xC026258A); + internal const int ErrorGraphicsDdcCiInvalidMessageChecksum = unchecked((int)0xC026258B); + internal const int ErrorGraphicsInvalidPhysicalMonitorHandle = unchecked((int)0xC026258C); + internal const int ErrorGraphicsMonitorNoLongerExists = unchecked((int)0xC026258D); + + // The doubled "CURRENT_CURRENT" is the SDK's own spelling, kept so the name greps against + // winerror.h. + internal const int ErrorGraphicsDdcCiCurrentCurrentValueGreaterThanMaximumValue = + unchecked((int)0xC02625D8); + + internal const int ErrorTimeout = 1460; + + /// + /// True when the error invalidates the physical-monitor handle itself rather than the one VCP + /// feature, so no further request may be issued against it. + /// + public static bool IsPhysicalMonitorUnavailable(int errorCode) => errorCode is + ErrorGraphicsInvalidPhysicalMonitorHandle or + ErrorGraphicsMonitorNoLongerExists; + + /// + /// True when the failure is a framing, arbitration or timing fault on the I2C bus that another + /// attempt can plausibly get past. + /// + /// + /// + /// Deliberate exclusions, all of which would burn the retry budget for nothing: + /// ERROR_GRAPHICS_DDCCI_VCP_NOT_SUPPORTED (0xC0262584) is the device's final answer that + /// it does not implement the opcode; ERROR_GRAPHICS_I2C_NOT_SUPPORTED (0xC0262580) and + /// ERROR_GRAPHICS_I2C_DEVICE_DOES_NOT_EXIST (0xC0262581) are permanent bus-level facts; + /// ERROR_GRAPHICS_MCA_INVALID_CAPABILITIES_STRING (0xC0262587) belongs to the + /// capabilities path, not to a VCP read; and the two handle-class codes are owned by + /// , which must abort rather than retry. + /// ERROR_GRAPHICS_DDCCI_MONITOR_RETURNED_INVALID_TIMING_STATUS_BYTE (0xC0262586) reads + /// like a sibling of the framing codes below but is raised only by the DDC/CI get-timing-report + /// command, never by GetVCPFeatureAndVCPFeatureReply, so it is unreachable here. + /// + /// + /// ERROR_GRAPHICS_DDCCI_CURRENT_CURRENT_VALUE_GREATER_THAN_MAXIMUM_VALUE (0xC02625D8) is + /// included on purpose: a device that genuinely reports current > maximum will keep doing so + /// and simply exhausts the budget, but the same code also results from a corrupted reply, which + /// a retry does fix. + /// + /// + public static bool IsTransient(int errorCode) => errorCode is + ErrorGraphicsI2CErrorTransmittingData or + ErrorGraphicsI2CErrorReceivingData or + ErrorGraphicsDdcCiInvalidData or + ErrorGraphicsMcaInternalError or + ErrorGraphicsDdcCiInvalidMessageCommand or + ErrorGraphicsDdcCiInvalidMessageLength or + ErrorGraphicsDdcCiInvalidMessageChecksum or + ErrorGraphicsDdcCiCurrentCurrentValueGreaterThanMaximumValue or + ErrorTimeout; + } +} diff --git a/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/NativeVcpFeatureReader.cs b/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/NativeVcpFeatureReader.cs new file mode 100644 index 0000000000..a92a739950 --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/NativeVcpFeatureReader.cs @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.InteropServices; +using static PowerDisplay.Common.Drivers.PInvoke; + +namespace PowerDisplay.Common.Drivers.DDC +{ + /// + /// The production : one GetVCPFeatureAndVCPFeatureReply + /// transaction, with no retry, pacing or logging. Those belong to the callers — see + /// . + /// + internal sealed class NativeVcpFeatureReader : IVcpFeatureReader + { + public VcpReadAttempt Read(IntPtr handle, byte code) => + GetVCPFeatureAndVCPFeatureReply(handle, code, IntPtr.Zero, out uint current, out uint maximum) + ? VcpReadAttempt.Success(current, maximum) + : VcpReadAttempt.Failure(Marshal.GetLastWin32Error()); + } +} diff --git a/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/VcpFeatureProbeService.cs b/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/VcpFeatureProbeService.cs new file mode 100644 index 0000000000..db69f975b0 --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/VcpFeatureProbeService.cs @@ -0,0 +1,204 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using ManagedCommon; +using PowerDisplay.Common.Models; +using static PowerDisplay.Common.Drivers.NativeConstants; + +namespace PowerDisplay.Common.Drivers.DDC +{ + internal sealed class VcpFeatureProbeService + { + private static readonly TimeSpan TransactionInterval = TimeSpan.FromMilliseconds(100); + private const int MaxAttempts = 3; + + private readonly IVcpFeatureReader _reader; + private readonly Func _delayAsync; + private readonly IReadOnlyList _codes; + + public VcpFeatureProbeService( + IVcpFeatureReader reader, + Func? delayAsync = null, + IReadOnlyList? codes = null) + { + _reader = reader; + _delayAsync = delayAsync ?? Task.Delay; + _codes = codes ?? ContinuousVcpCodes; + } + + public async Task> ProbeAsync( + IntPtr handle, + CancellationToken cancellationToken) + { + var observations = new Dictionary(); + + foreach (var code in _codes) + { + var observation = await ProbeCodeAsync(handle, code, cancellationToken).ConfigureAwait(false); + observations[code] = observation; + + // These errors invalidate the physical-monitor handle, not just the current + // VCP feature. Avoid issuing more I2C requests against a stale handle. + if (observation.IsPhysicalMonitorUnavailable) + { + break; + } + } + + return observations; + } + + private async Task ProbeCodeAsync( + IntPtr handle, + byte code, + CancellationToken cancellationToken) + { + int? lastError = null; + var attempts = 0; + + for (var attempt = 1; attempt <= MaxAttempts; attempt++) + { + cancellationToken.ThrowIfCancellationRequested(); + await _delayAsync(TransactionInterval, cancellationToken).ConfigureAwait(false); + cancellationToken.ThrowIfCancellationRequested(); + + attempts = attempt; + + VcpReadAttempt result; + try + { + result = await Task.Run( + () => _reader.Read(handle, code), + cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when ( + ex is not OperationCanceledException && + ex is not OutOfMemoryException) + { + // Containment belongs to the orchestration layer: a throwing read must cost the + // caller this one VCP code, not every monitor sharing the hMonitor via the + // pipeline-wide catch in DdcCiController. + Logger.LogError( + $"DDC: [max-compat] VCP probe threw " + + $"(handle=0x{handle:X}, code=0x{code:X2}, attempt={attempt}/{MaxAttempts}): {ex.Message}"); + break; + } + + if (result.IsSuccess) + { + // The device answered this opcode. Unimplemented codes fail with + // DDCCI_VCP_NOT_SUPPORTED instead, so a reply proves support even when the + // reported range cannot scale a percentage. + var value = new VcpFeatureValue((int)result.Current, 0, (int)result.Maximum); + var observation = value.IsValid + ? VcpProbeObservation.Success(code, value, attempt, lastError) + : VcpProbeObservation.Indeterminate(code, lastError, attempt, replied: true); + + Logger.LogDebug( + $"DDC: [max-compat] VCP probe attempt " + + $"(handle=0x{handle:X}, code=0x{code:X2}, attempt={attempt}/{MaxAttempts}, " + + $"status={(value.IsValid ? "success" : "invalid-range")}, " + + $"current={result.Current}, maximum={result.Maximum})"); + + // A reply settles the only question this probe asks, so the remaining budget is + // not spent on the I2C bus even when the reported range is degenerate. + return Complete(handle, observation); + } + else + { + lastError = result.ErrorCode; + Logger.LogDebug( + $"DDC: [max-compat] VCP probe attempt " + + $"(handle=0x{handle:X}, code=0x{code:X2}, attempt={attempt}/{MaxAttempts}, " + + $"status=failed, error={FormatError(lastError)})"); + if (!DdcErrorClassifier.IsTransient(result.ErrorCode)) + { + break; + } + } + } + + // Every reply returns from inside the loop, so reaching here means the device never + // answered: the retry budget was exhausted, a definitive refusal stopped it, or the + // read threw. + return Complete( + handle, + VcpProbeObservation.Indeterminate(code, lastError, attempts)); + } + + private static VcpProbeObservation Complete(IntPtr handle, VcpProbeObservation observation) + { + var status = observation.IsSuccess + ? "success" + : observation.IsPhysicalMonitorUnavailable + ? "physical-monitor-unavailable" + : "indeterminate"; + var message = + $"DDC: [max-compat] VCP probe outcome " + + $"(handle=0x{handle:X}, code=0x{observation.Code:X2}, attempts={observation.Attempts}, " + + $"status={status}, replied={observation.Replied}, lastError={FormatError(observation.LastError)})"; + + if (observation.IsSuccess) + { + Logger.LogInfo(message); + } + else + { + Logger.LogWarning(message); + } + + return observation; + } + + private static string FormatError(int? errorCode) => + errorCode.HasValue ? $"0x{unchecked((uint)errorCode.Value):X8}" : "none"; + } + + internal readonly record struct VcpReadAttempt(bool IsSuccess, uint Current, uint Maximum, int ErrorCode) + { + public static VcpReadAttempt Success(uint current, uint maximum) => new(true, current, maximum, 0); + + public static VcpReadAttempt Failure(int errorCode) => new(false, 0, 0, errorCode); + } + + internal interface IVcpFeatureReader + { + VcpReadAttempt Read(IntPtr handle, byte code); + } + + internal readonly record struct VcpProbeObservation( + byte Code, + VcpFeatureValue Value, + int Attempts, + int? LastError, + bool Replied = false) + { + public bool IsSuccess => Value.IsValid; + + /// + /// Gets a value indicating whether the failure invalidated the physical-monitor handle + /// itself, so no further request may be issued against it. + /// + public bool IsPhysicalMonitorUnavailable => + LastError is int errorCode && DdcErrorClassifier.IsPhysicalMonitorUnavailable(errorCode); + + public static VcpProbeObservation Success( + byte code, + VcpFeatureValue value, + int attempts = 1, + int? lastError = null) => + new(code, value, attempts, lastError, true); + + public static VcpProbeObservation Indeterminate( + byte code, + int? lastError, + int attempts = 1, + bool replied = false) => + new(code, VcpFeatureValue.Invalid, attempts, lastError, replied); + } +} diff --git a/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/NativeConstants.cs b/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/NativeConstants.cs index 7a3983cc4f..035b59e4ab 100644 --- a/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/NativeConstants.cs +++ b/src/modules/powerdisplay/PowerDisplay.Lib/Drivers/NativeConstants.cs @@ -28,6 +28,23 @@ namespace PowerDisplay.Common.Drivers /// public const byte VcpCodeVolume = 0x62; + /// + /// The percent-scaled VCP features, in the order discovery walks them. The probe and the + /// continuous initializers must agree on this set: a code missing from one of them is + /// probed but never applied, or applied but never proven. + /// + /// + /// Discrete-value features (0x14 color preset, 0x60 input source, 0xD6 power mode) are + /// excluded: GetVCPFeatureAndVCPFeatureReply returns only current+max, so there is no + /// way to synthesize a meaningful supported-value list for them. + /// + internal static readonly byte[] ContinuousVcpCodes = + { + VcpCodeBrightness, + VcpCodeContrast, + VcpCodeVolume, + }; + /// /// VCP code: Select Color Preset (0x14) /// Standard VESA MCCS color temperature preset selection. diff --git a/src/modules/powerdisplay/PowerDisplay.Lib/PowerDisplay.Lib.csproj b/src/modules/powerdisplay/PowerDisplay.Lib/PowerDisplay.Lib.csproj index f7a97c57c1..b68f20b3d6 100644 --- a/src/modules/powerdisplay/PowerDisplay.Lib/PowerDisplay.Lib.csproj +++ b/src/modules/powerdisplay/PowerDisplay.Lib/PowerDisplay.Lib.csproj @@ -22,6 +22,9 @@ false false + + +