PowerDisplay: Reuse the values the max-compatibility probe already read (#49596)

## Summary of the Pull Request

In Maximum compatibility mode, when a monitor's capabilities string is
unusable, discovery probes each continuous VCP code directly to find out
which ones the panel implements — and then **throws the values away**.
`BuildMonitorFromPhysical` immediately re-reads every one of those
codes.

That doubles the I2C traffic on exactly the hardware that cannot take
it, and the re-read is the one whose result the user actually sees: a
panel that answered the probe a moment ago but fails the re-read shows
its brightness slider parked at the never-read default instead of where
the panel really is.

This makes the probe's values survive into the build stage.

Extracted from #49445, which bundles it with a persisted discovery cache
it does not depend on.

## PR Checklist

- [ ] Closes: #xxx — partially addresses #49342; the remaining cause is
in #49445
- [ ] **Communication:** I've discussed this with core contributors
already. If the work hasn't been agreed, this work might be rejected
- [x] **Tests:** Added/updated and all pass
- [x] **Localization:** All end-user-facing strings can be localized —
this PR adds none
- [ ] **Dev docs:** Added/updated
- [x] **New binaries:** Added on the required places — none added; no
new project, so no signing JSON, installer WXS or CI YML change is
required
- [ ] **Documentation updated**

## Detailed Description of the Pull Request / Additional comments

### What the re-read costs

Worth being precise about, because it is not the slider's *existence*:

| decided by | set from | affected by a failed re-read |
| --- | --- | --- |
| slider visible (`MonitorViewModel.ShowBrightness`) |
`Monitor.Capabilities`, via `UpdateMonitorCapabilitiesFromVcp` before
the initializer runs | no |
| slider position (`Monitor.CurrentBrightness`) | the read | yes — stays
at the never-read default |
| `powerdisplay get` reporting a live reading (`Monitor.ReadValues`) |
the read | yes — reported as unknown |
| relative `powerdisplay adjust` (`AdjustCommandExecutor`) |
`Monitor.ReadValues` | yes — no before-value to adjust from |

So the flyout keeps the control either way; what the second read decides
is whether it is pointed anywhere real, and whether the CLI will admit
to a value. Halving the transactions on a bus that is both slow and, on
this hardware, unreliable is the other half of the win.

### The seam

`FetchCapabilitiesWithFallbackAsync` used to return `(string capsString,
VcpCapabilities? caps)` — capabilities only, no values. It now returns a
`VcpDiscoveryEvidence`, which carries the same two things plus the
values the probe already read and a flag for a handle that died
mid-probe.

`VcpDiscoveryEvidence.Reconcile` folds the probe observations into the
parsed capabilities in one place:

| observation | capabilities | value carried |
| --- | --- | --- |
| read succeeded | code marked supported | yes |
| device replied, range unusable (e.g. `max=0`) | code marked supported
| no — the initializer still owes it a read |
| no reply | unchanged | no |
| handle-class failure | everything discarded | — |

The second row is why membership keys off `Replied` rather than the
value being usable: an unimplemented code fails with
`DDCCI_VCP_NOT_SUPPORTED` and never sets the flag, so a reply proves the
opcode exists even when the reported range cannot scale a percentage.
That is the same rule `BuildCapabilitiesFromProbe` used before this PR;
it just moves next to the value handling.

Like that method, `Reconcile` iterates the observations rather than
`NativeConstants.ContinuousVcpCodes`, which `VcpFeatureProbeService`
only takes as the default for its constructor-injected sweep list. That
keeps a widened sweep from silently dropping a code that answered, but
it is not sufficient on its own: the carried value is consumed only for
codes `ContinuousVcpInitializer` walks, so widening the sweep still
needs a matching edit there. The comment and
`Reconcile_ProbedCodeOutsideTheDefaultSweepIsStillHonoured` both say so
rather than claiming the seam alone covers it.

On the normal path nothing changes: the probe only runs when the caps
string is unusable, so `live` is empty and `Reconcile` is a
pass-through. `Reconcile_ParsedCapabilitiesSurviveWhenNoProbeRan` pins
that.

### Continuous-VCP initialization moves out of the controller

`DdcCiController` carried six near-identical `Initialize*` methods. The
three percent-scaled ones become **`ContinuousVcpInitializer`** —
brightness, contrast, volume. It skips any code the evidence already has
a value for, and returns `false` when a read fails with a handle-class
error, because `Monitor.Handle` is captured once per discovery pass and
never refreshed: a monitor kept alive on a dead handle would send every
later read and write into the void.

It reads through the `IVcpFeatureReader` seam introduced in #49579, so
it is testable without hardware.

The three discrete-enum ones — color preset, input source, power mode —
stay in `DdcCiController`, unchanged. The probe sweeps only
`NativeConstants.ContinuousVcpCodes`, so no discrete value is ever
carried across the seam and extracting them would be a refactor this
change does not need; see *What is deliberately left out*.

Only the continuous stage discards the monitor. That is a policy choice,
not a property of the stage: losing a whole display because `0xD6`
answered badly is worse than showing it without a power control.

`DdcCiController.TryGetVcpFeature` therefore still has three discovery
callers plus `GetVcpFeatureAsync`, the runtime refresh path.

### Behaviour change outside Maximum compatibility mode

**A handle-class error during continuous VCP initialization now discards
the monitor.** Before, the failure was logged, the read flag left unset,
and the monitor kept — so its handle reached
`PhysicalMonitorHandleManager` and every later operation went to a
handle already known to be dead. The cost is that the monitor stays out
of the flyout until a rediscovery: `DisplayChangeWatcher` schedules one
for device-arrival/removal and console-display-state notifications, and
the flyout's Refresh button forces one on demand.

Note this check is not on every path. A caps string that parses but
advertises none of `0x10`/`0x12`/`0x62` leaves
`ContinuousVcpInitializer` nothing to read and suppresses the probe, so
such a monitor is still published with a handle no VCP read has
exercised — and its first VCP read then happens in the discrete stage,
which never discards.

### What is deliberately left out

**Extracting the discrete-VCP initialization.** The probe sweeps only
the continuous codes, so no discrete value is ever reused and moving
`0x14`/`0x60`/`0xD6` out of the controller would be a drive-by refactor
with no bearing on this change. It is worth doing on its own, where the
added test coverage can be reviewed for what it is.

**Remembering a probe value across discoveries.** A probe value is only
useful for the pass that produced it. Carrying one forward — so a later
failing pass can still show the control — is the persisted known-good
cache in #49445, a much larger change with an open design question
attached. This PR is complete without it.

## Validation Steps Performed

- built `PowerDisplay.Lib.UnitTests` and `PowerDisplay` for x64 Debug
with VS MSBuild — 0 errors, 0 warnings
- ran `PowerDisplay.Lib.UnitTests.dll` with `vstest.console.exe`: **240
passed, 0 failed** — 16 of those cases are added here (8 in
`ContinuousVcpInitializerTests`, 7 in `VcpDiscoveryEvidenceTests`, 1 in
`DdcErrorClassifierTests`)
- `VcpDiscoveryEvidenceTests` pins each row of the table above, plus
that a probed code outside the default sweep is still honoured
- `ContinuousVcpInitializerTests` pins that a probed code is never
re-read (the reader is primed with a failure it must not reach), that a
handle-class error stops the remaining codes, and that a feature-level
refusal does not. `Initialize_EveryContinuousCodeIsReadAndApplied` walks
the whole `ContinuousVcpCodes` array with a distinct range and
percentage per feature, so a code added to that array without an arm in
both `IsSupported` and `ApplyValue` fails rather than being silently
skipped or silently discarded — checked by mutation: removing either
volume switch arm fails that test and
`Initialize_ProbedVolumeIsAppliedWithoutReadingAgain`
- not covered by tests: the `DdcCiController` side of the contract —
that `evidence.IsPhysicalMonitorUnavailable` skips the monitor and
releases the physical, and that a `false` from
`ContinuousVcpInitializer` does the same. That layer takes no injectable
dependencies today
- no hardware validation performed: this path is reachable only on a
panel whose capabilities string is unusable, which needs an incomplete
or unreliable DDC/CI implementation

---------

Co-authored-by: Yu Leng (from Dev Box) <yuleng@microsoft.com>
This commit is contained in:
moooyo
2026-07-31 15:48:03 +08:00
committed by GitHub
parent bb99c30edc
commit 3cb3bdcd34
10 changed files with 725 additions and 168 deletions

View File

@@ -0,0 +1,209 @@
// 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 Microsoft.VisualStudio.TestTools.UnitTesting;
using PowerDisplay.Common.Drivers;
using PowerDisplay.Common.Drivers.DDC;
using PowerDisplay.Common.Models;
using static PowerDisplay.UnitTests.DdcFakes;
namespace PowerDisplay.UnitTests;
[TestClass]
public sealed class ContinuousVcpInitializerTests
{
[TestMethod]
public void Initialize_ProbedValueIsAppliedWithoutReadingAgain()
{
// The reader is primed with a failure it must never reach: if the initializer re-reads a
// code the probe already answered, this test fails on the CallCount assertion rather than
// on a fabricated value. The probed range is deliberately not 0-100, so both the raw
// maximum and the percent scaling have to survive the seam for the assertions to hold.
var reader = new RecordingVcpReader(VcpReadAttempt.Failure(1));
var initializer = new ContinuousVcpInitializer(reader);
var monitor = BrightnessMonitor();
var result = initializer.Initialize(monitor, Evidence((0x10, new VcpFeatureValue(15, 0, 50))));
Assert.IsTrue(result);
Assert.AreEqual(0, reader.CallCount);
Assert.AreEqual(30, monitor.CurrentBrightness);
Assert.AreEqual(50, monitor.BrightnessVcpMax);
Assert.IsTrue(monitor.ReadValues.HasFlag(MonitorReadFlags.Brightness));
}
[TestMethod]
public void Initialize_CodeWithoutAProbedValueIsReadOnce()
{
var reader = new RecordingVcpReader(VcpReadAttempt.Success(55, 100));
var initializer = new ContinuousVcpInitializer(reader);
var monitor = BrightnessMonitor();
var result = initializer.Initialize(monitor, Evidence());
Assert.IsTrue(result);
Assert.AreEqual(1, reader.CallCount);
Assert.AreEqual(55, monitor.CurrentBrightness);
Assert.IsTrue(monitor.ReadValues.HasFlag(MonitorReadFlags.Brightness));
}
[TestMethod]
public void Initialize_InvalidReadRangeIsNotApplied()
{
var reader = new RecordingVcpReader(VcpReadAttempt.Success(55, 0));
var initializer = new ContinuousVcpInitializer(reader);
var monitor = BrightnessMonitor();
var result = initializer.Initialize(monitor, Evidence());
Assert.IsTrue(result);
Assert.AreEqual(1, reader.CallCount);
Assert.AreEqual(0, monitor.CurrentBrightness);
Assert.IsFalse(monitor.ReadValues.HasFlag(MonitorReadFlags.Brightness));
}
[DataTestMethod]
[DataRow(DdcErrorClassifier.ErrorGraphicsInvalidPhysicalMonitorHandle)]
[DataRow(DdcErrorClassifier.ErrorGraphicsMonitorNoLongerExists)]
public void Initialize_HandleClassFailureStopsAndReportsToTheCaller(int errorCode)
{
var reader = new RecordingVcpReader(VcpReadAttempt.Failure(errorCode));
var initializer = new ContinuousVcpInitializer(reader);
var monitor = BrightnessAndContrastMonitor();
var result = initializer.Initialize(monitor, Evidence());
Assert.IsFalse(result, "A handle-class read failure must tell the caller to drop the monitor.");
CollectionAssert.AreEqual(new byte[] { 0x10 }, reader.Codes, "The dead handle must not be used again.");
Assert.AreEqual(MonitorReadFlags.None, monitor.ReadValues);
}
[TestMethod]
public void Initialize_FeatureLevelFailureSkipsOnlyThatCode()
{
// DDCCI_VCP_NOT_SUPPORTED is the device's answer about one opcode, not about the handle,
// so the remaining codes still get their read.
var reader = new RecordingVcpReader(
VcpReadAttempt.Failure(DdcErrorClassifier.ErrorGraphicsDdcCiVcpNotSupported),
VcpReadAttempt.Success(60, 100));
var initializer = new ContinuousVcpInitializer(reader);
var monitor = BrightnessAndContrastMonitor();
var result = initializer.Initialize(monitor, Evidence());
Assert.IsTrue(result);
CollectionAssert.AreEqual(new byte[] { 0x10, 0x12 }, reader.Codes);
Assert.IsFalse(monitor.ReadValues.HasFlag(MonitorReadFlags.Brightness));
Assert.AreEqual(60, monitor.CurrentContrast);
Assert.IsTrue(monitor.ReadValues.HasFlag(MonitorReadFlags.Contrast));
}
[TestMethod]
public void Initialize_EveryContinuousCodeIsReadAndApplied()
{
// Pins the invariant ContinuousVcpInitializer's own remarks declare but nothing else
// enforced: every entry in ContinuousVcpCodes needs an arm in both IsSupported and
// ApplyValue. Without an IsSupported arm the code is never read, which the Codes assertion
// catches; without an ApplyValue arm it is read and then discarded, which the per-feature
// assertions catch. Ranges and percentages are all distinct so a cross-wired arm cannot
// pass by coincidence.
Assert.AreEqual(
3,
NativeConstants.ContinuousVcpCodes.Length,
"ContinuousVcpCodes grew — give the new code an IsSupported and an ApplyValue arm, then extend this test.");
var reader = new RecordingVcpReader(
VcpReadAttempt.Success(15, 50),
VcpReadAttempt.Success(20, 40),
VcpReadAttempt.Success(7, 10));
var initializer = new ContinuousVcpInitializer(reader);
var monitor = AllContinuousMonitor();
var result = initializer.Initialize(monitor, Evidence());
Assert.IsTrue(result);
CollectionAssert.AreEqual(NativeConstants.ContinuousVcpCodes, reader.Codes);
Assert.AreEqual(30, monitor.CurrentBrightness);
Assert.AreEqual(50, monitor.BrightnessVcpMax);
Assert.AreEqual(50, monitor.CurrentContrast);
Assert.AreEqual(40, monitor.ContrastVcpMax);
Assert.AreEqual(70, monitor.CurrentVolume);
Assert.AreEqual(10, monitor.VolumeVcpMax);
Assert.AreEqual(
MonitorReadFlags.Brightness | MonitorReadFlags.Contrast | MonitorReadFlags.Volume,
monitor.ReadValues);
}
[TestMethod]
public void Initialize_ProbedVolumeIsAppliedWithoutReadingAgain()
{
// Volume is the one continuous code the probe path is not otherwise exercised against, and
// it is the one whose ApplyValue arm has no neighbour to shadow a mistake.
var reader = new RecordingVcpReader(VcpReadAttempt.Failure(1));
var initializer = new ContinuousVcpInitializer(reader);
var monitor = VolumeMonitor();
var result = initializer.Initialize(monitor, Evidence((0x62, new VcpFeatureValue(7, 0, 10))));
Assert.IsTrue(result);
Assert.AreEqual(0, reader.CallCount);
Assert.AreEqual(70, monitor.CurrentVolume);
Assert.AreEqual(10, monitor.VolumeVcpMax);
Assert.IsTrue(monitor.ReadValues.HasFlag(MonitorReadFlags.Volume));
}
private static Monitor BrightnessMonitor() => new()
{
Id = MonitorId,
Handle = new IntPtr(1),
Capabilities = MonitorCapabilities.DdcCi | MonitorCapabilities.Brightness,
};
private static Monitor BrightnessAndContrastMonitor() => new()
{
Id = MonitorId,
Handle = new IntPtr(1),
Capabilities = MonitorCapabilities.DdcCi |
MonitorCapabilities.Brightness |
MonitorCapabilities.Contrast,
};
private static Monitor VolumeMonitor() => new()
{
Id = MonitorId,
Handle = new IntPtr(1),
Capabilities = MonitorCapabilities.DdcCi | MonitorCapabilities.Volume,
};
private static Monitor AllContinuousMonitor() => new()
{
Id = MonitorId,
Handle = new IntPtr(1),
Capabilities = MonitorCapabilities.DdcCi |
MonitorCapabilities.Brightness |
MonitorCapabilities.Contrast |
MonitorCapabilities.Volume,
};
/// <summary>
/// Builds evidence carrying only the probed values: <see cref="ContinuousVcpInitializer"/>
/// reads <see cref="VcpDiscoveryEvidence.InitialValues"/> and nothing else, so a capabilities
/// object here would be scaffolding no assertion depends on. Support is expressed by the
/// fixture's <see cref="MonitorCapabilities"/> flags instead, which is what the initializer
/// actually gates on.
/// </summary>
private static VcpDiscoveryEvidence Evidence(params (byte Code, VcpFeatureValue Value)[] probed)
{
var values = new Dictionary<byte, VcpFeatureValue>();
foreach (var (code, value) in probed)
{
values[code] = value;
}
return new VcpDiscoveryEvidence(string.Empty, new VcpCapabilities(), values);
}
}

View File

@@ -82,4 +82,16 @@ public sealed class DdcErrorClassifierTests
[DataRow(0)]
public void IsPhysicalMonitorUnavailable_RejectsFeatureLevelFailures(int errorCode) =>
Assert.IsFalse(DdcErrorClassifier.IsPhysicalMonitorUnavailable(errorCode));
[TestMethod]
public void Format_RendersTheUnsignedHexTheSdkDocuments()
{
// Marshal.GetLastWin32Error hands 0xC026258C back as a negative int, which greps against
// nothing. The log has to carry the spelling winerror.h uses.
Assert.AreEqual(
"0xC026258C",
DdcErrorClassifier.Format(DdcErrorClassifier.ErrorGraphicsInvalidPhysicalMonitorHandle));
Assert.AreEqual("0x000005B4", DdcErrorClassifier.Format(DdcErrorClassifier.ErrorTimeout));
Assert.AreEqual("none", DdcErrorClassifier.Format(null));
}
}

View File

@@ -0,0 +1,48 @@
// 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 PowerDisplay.Common.Drivers.DDC;
namespace PowerDisplay.UnitTests;
/// <summary>
/// Test doubles and fixtures shared by the DDC discovery tests. They live nested inside one
/// container so the file keeps a single top-level type; call sites pull them into scope with
/// <c>using static PowerDisplay.UnitTests.DdcFakes;</c> and use them unqualified.
/// </summary>
internal static class DdcFakes
{
/// <summary>
/// A canonical DevicePath-shaped monitor Id, so a change to what a canonical Id looks like
/// shows up in one place.
/// </summary>
internal const string MonitorId = @"\\?\DISPLAY#AOCB326#5&ABC&0&UID1";
/// <summary>
/// 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.
/// </summary>
/// <remarks>
/// 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: an extra
/// read is named by the <see cref="CallCount"/> and <see cref="Codes"/> assertions instead.
/// </remarks>
internal sealed class RecordingVcpReader(params VcpReadAttempt[] results) : IVcpFeatureReader
{
private readonly Queue<VcpReadAttempt> _results = new(results);
public int CallCount { get; private set; }
public List<byte> Codes { get; } = new();
public VcpReadAttempt Read(IntPtr handle, byte code)
{
CallCount++;
Codes.Add(code);
return _results.Dequeue();
}
}
}

View File

@@ -0,0 +1,130 @@
// 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.Collections.Generic;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using PowerDisplay.Common.Drivers.DDC;
using PowerDisplay.Common.Models;
namespace PowerDisplay.UnitTests;
/// <summary>
/// Covers what discovery concludes from a set of probe observations: which VCP codes count as
/// supported, and which of them carry a value the build stage may use instead of re-reading.
/// </summary>
[TestClass]
public sealed class VcpDiscoveryEvidenceTests
{
[TestMethod]
public void Reconcile_SuccessfulProbeAdvertisesSupportAndCarriesTheValue()
{
// The whole point of the evidence: a code the probe read must not be read again.
var result = VcpDiscoveryEvidence.Reconcile(
capabilitiesRaw: string.Empty,
parsedCapabilities: null,
live: Observations((0x10, VcpProbeObservation.Success(0x10, new VcpFeatureValue(30, 0, 100)))));
Assert.IsTrue(result.Capabilities!.SupportsVcpCode(0x10));
Assert.AreEqual(30, result.InitialValues[0x10].Current);
Assert.AreEqual(100, result.InitialValues[0x10].Maximum);
}
[TestMethod]
public void Reconcile_RepliedProbeWithUnusableRangeAdvertisesSupportWithoutAValue()
{
// The device answered, which proves the opcode exists, but max=0 cannot scale a percentage.
// Support is kept so the control stays reachable; the absent value sends the initializer
// back to the hardware for one more read.
var result = VcpDiscoveryEvidence.Reconcile(
capabilitiesRaw: string.Empty,
parsedCapabilities: null,
live: Observations((0x62, VcpProbeObservation.Indeterminate(0x62, lastError: null, attempts: 1, replied: true))));
Assert.IsTrue(result.Capabilities!.SupportsVcpCode(0x62));
Assert.IsFalse(result.InitialValues.ContainsKey(0x62));
}
[TestMethod]
public void Reconcile_UnansweredProbeDoesNotAdvertiseSupport()
{
var result = VcpDiscoveryEvidence.Reconcile(
capabilitiesRaw: string.Empty,
parsedCapabilities: null,
live: Observations((0x62, VcpProbeObservation.Indeterminate(
0x62,
DdcErrorClassifier.ErrorGraphicsDdcCiVcpNotSupported))));
Assert.IsNull(result.Capabilities);
Assert.AreEqual(0, result.InitialValues.Count);
}
[DataTestMethod]
[DataRow(DdcErrorClassifier.ErrorGraphicsInvalidPhysicalMonitorHandle)]
[DataRow(DdcErrorClassifier.ErrorGraphicsMonitorNoLongerExists)]
public void Reconcile_PhysicalMonitorUnavailableDiscardsEverything(int errorCode)
{
// A dead handle invalidates the whole pass, including the codes that answered before it
// died — nothing gathered through it can be trusted.
var parsed = new VcpCapabilities();
parsed.SupportedVcpCodes[0x10] = new VcpCodeInfo(0x10, "Brightness");
var result = VcpDiscoveryEvidence.Reconcile(
capabilitiesRaw: "caps",
parsedCapabilities: parsed,
live: Observations(
(0x10, VcpProbeObservation.Success(0x10, new VcpFeatureValue(30, 0, 100))),
(0x12, VcpProbeObservation.Indeterminate(0x12, errorCode))));
Assert.IsTrue(result.IsPhysicalMonitorUnavailable);
Assert.IsNull(result.Capabilities);
Assert.AreEqual(0, result.InitialValues.Count);
}
[TestMethod]
public void Reconcile_ProbedCodeOutsideTheDefaultSweepIsStillHonoured()
{
// Reconcile is driven by what the probe reported, not by NativeConstants.ContinuousVcpCodes,
// so widening VcpFeatureProbeService's constructor-injected sweep list does not silently drop
// a code that answered. Honouring it here is necessary but not sufficient: the carried value
// is only consumed for codes ContinuousVcpInitializer walks, so a widened sweep still needs a
// matching edit there.
var result = VcpDiscoveryEvidence.Reconcile(
capabilitiesRaw: string.Empty,
parsedCapabilities: null,
live: Observations((0x60, VcpProbeObservation.Success(0x60, new VcpFeatureValue(0x11, 0, 0x12)))));
Assert.IsTrue(result.Capabilities!.SupportsVcpCode(0x60));
Assert.AreEqual(0x11, result.InitialValues[0x60].Current);
}
[TestMethod]
public void Reconcile_ParsedCapabilitiesSurviveWhenNoProbeRan()
{
// The probe only runs when the caps string is unusable, so the parsed path must be a
// pass-through: no codes added, no values invented.
var parsed = new VcpCapabilities();
parsed.SupportedVcpCodes[0x10] = new VcpCodeInfo(0x10, "Brightness");
var result = VcpDiscoveryEvidence.Reconcile(
capabilitiesRaw: "caps",
parsedCapabilities: parsed,
live: new Dictionary<byte, VcpProbeObservation>());
Assert.AreSame(parsed, result.Capabilities);
Assert.AreEqual(0, result.InitialValues.Count);
Assert.AreEqual("caps", result.CapabilitiesRaw);
}
private static Dictionary<byte, VcpProbeObservation> Observations(
params (byte Code, VcpProbeObservation Observation)[] entries)
{
var observations = new Dictionary<byte, VcpProbeObservation>();
foreach (var (code, observation) in entries)
{
observations[code] = observation;
}
return observations;
}
}

View File

@@ -8,6 +8,7 @@ using System.Threading;
using System.Threading.Tasks;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using PowerDisplay.Common.Drivers.DDC;
using static PowerDisplay.UnitTests.DdcFakes;
namespace PowerDisplay.UnitTests;
@@ -304,31 +305,4 @@ public sealed class VcpFeatureProbeServiceTests
? throw new InvalidOperationException("simulated native failure")
: otherwise;
}
/// <summary>
/// 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.
/// </summary>
/// <remarks>
/// 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 <see cref="VcpFeatureProbeService"/>'s catch-all turns it into
/// an indeterminate observation. An extra read is named by the <see cref="CallCount"/> and
/// <see cref="Codes"/> assertions instead.
/// </remarks>
private sealed class RecordingVcpReader(params VcpReadAttempt[] results) : IVcpFeatureReader
{
private readonly Queue<VcpReadAttempt> _results = new(results);
public int CallCount { get; private set; }
public List<byte> Codes { get; } = new();
public VcpReadAttempt Read(IntPtr handle, byte code)
{
CallCount++;
Codes.Add(code);
return _results.Dequeue();
}
}
}

View File

@@ -0,0 +1,138 @@
// 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 ManagedCommon;
using PowerDisplay.Common.Models;
using PowerDisplay.Common.Utils;
using static PowerDisplay.Common.Drivers.NativeConstants;
namespace PowerDisplay.Common.Drivers.DDC
{
/// <summary>
/// Applies the percent-scaled VCP features — brightness, contrast, volume — to a freshly built
/// <see cref="Monitor"/>.
/// </summary>
internal sealed class ContinuousVcpInitializer
{
private readonly IVcpFeatureReader _reader;
public ContinuousVcpInitializer(IVcpFeatureReader reader)
{
_reader = reader;
}
/// <summary>
/// Applies the continuous VCP features to <paramref name="monitor"/>, reading from
/// <see cref="Monitor.Handle"/> only for codes the evidence does not already carry a value
/// for.
/// </summary>
/// <returns>
/// False when a read failed with a handle-class error, which invalidates
/// <see cref="Monitor.Handle"/> itself rather than the one feature — the caller must then
/// discard the monitor instead of publishing it.
/// </returns>
public bool Initialize(Monitor monitor, VcpDiscoveryEvidence evidence)
{
foreach (var code in ContinuousVcpCodes)
{
if (!InitializeFeature(monitor, evidence, code))
{
return false;
}
}
return true;
}
private bool InitializeFeature(Monitor monitor, VcpDiscoveryEvidence evidence, byte code)
{
if (!IsSupported(monitor, code))
{
return true;
}
// The probe already spent at least one transaction on every code it answered for, so
// reading one of those again in the same pass is pure I2C noise on a bus that is slow
// and, on the hardware this path exists for, unreliable.
if (evidence.InitialValues.TryGetValue(code, out var probed))
{
ApplyValue(monitor, code, probed);
return true;
}
var read = _reader.Read(monitor.Handle, code);
if (!read.IsSuccess)
{
Logger.LogError(
$"DDC: [{monitor.Id}] Failed to read VCP 0x{code:X2}, " +
$"error={DdcErrorClassifier.Format(read.ErrorCode)}");
if (DdcErrorClassifier.IsPhysicalMonitorUnavailable(read.ErrorCode))
{
// Dropping the monitor is deliberate: Monitor.Handle is captured once per
// discovery pass and never refreshed, so a monitor kept here would send every
// later read and write to a handle already known to be dead. A rediscovery is
// what repairs it — DisplayChangeWatcher schedules one for the topology changes
// that invalidate a handle, and the flyout's Refresh button forces one on
// demand.
return false;
}
return true;
}
var value = new VcpFeatureValue((int)read.Current, 0, (int)read.Maximum);
if (!value.IsValid)
{
Logger.LogWarning(
$"DDC: [{monitor.Id}] Ignoring invalid {VcpNames.GetCodeName(code).ToLowerInvariant()} " +
$"range current={read.Current}, max={read.Maximum}");
return true;
}
ApplyValue(monitor, code, value);
return true;
}
/// <summary>
/// Whether <paramref name="monitor"/> advertises <paramref name="code"/>.
/// </summary>
/// <remarks>
/// This switch and <see cref="ApplyValue"/>'s must between them cover every entry in
/// <see cref="NativeConstants.ContinuousVcpCodes"/>: a code added to that array without an
/// arm here is skipped, and one without an arm there is read and then discarded. Both are
/// silent, so the array and the two switches are edited together.
/// </remarks>
private static bool IsSupported(Monitor monitor, byte code) => code switch
{
VcpCodeBrightness => monitor.SupportsBrightness,
VcpCodeContrast => monitor.SupportsContrast,
VcpCodeVolume => monitor.SupportsVolume,
_ => false,
};
private static void ApplyValue(Monitor monitor, byte code, VcpFeatureValue value)
{
switch (code)
{
case VcpCodeBrightness:
monitor.BrightnessVcpMax = value.Maximum;
monitor.CurrentBrightness = value.ToPercentage();
monitor.ReadValues |= MonitorReadFlags.Brightness;
break;
case VcpCodeContrast:
monitor.ContrastVcpMax = value.Maximum;
monitor.CurrentContrast = value.ToPercentage();
monitor.ReadValues |= MonitorReadFlags.Contrast;
break;
case VcpCodeVolume:
monitor.VolumeVcpMax = value.Maximum;
monitor.CurrentVolume = value.ToPercentage();
monitor.ReadValues |= MonitorReadFlags.Volume;
break;
}
}
}
}

View File

@@ -35,6 +35,7 @@ namespace PowerDisplay.Common.Drivers.DDC
private readonly PhysicalMonitorHandleManager _handleManager = new();
private readonly MonitorDiscoveryHelper _discoveryHelper;
private readonly VcpFeatureProbeService _probeService;
private readonly ContinuousVcpInitializer _continuousInitializer;
private bool _disposed;
@@ -48,7 +49,9 @@ namespace PowerDisplay.Common.Drivers.DDC
public DdcCiController()
{
_discoveryHelper = new MonitorDiscoveryHelper();
_probeService = new VcpFeatureProbeService(new NativeVcpFeatureReader());
var vcpReader = new NativeVcpFeatureReader();
_probeService = new VcpFeatureProbeService(vcpReader);
_continuousInitializer = new ContinuousVcpInitializer(vcpReader);
}
public string Name => "DDC/CI Monitor Controller";
@@ -285,10 +288,11 @@ namespace PowerDisplay.Common.Drivers.DDC
private Monitor? BuildMonitorFromPhysical(
PHYSICAL_MONITOR physical,
MonitorDisplayInfo info,
string capsString,
VcpCapabilities? caps)
VcpDiscoveryEvidence evidence)
{
if (caps == null)
// The caller already skipped the unusable-capabilities and dead-handle cases, each with
// its own log line; this null check only carries that contract into the nullable flow.
if (evidence.Capabilities == null)
{
return null;
}
@@ -301,31 +305,26 @@ namespace PowerDisplay.Common.Drivers.DDC
return null;
}
if (!string.IsNullOrEmpty(capsString))
if (!string.IsNullOrEmpty(evidence.CapabilitiesRaw))
{
monitor.CapabilitiesRaw = capsString;
monitor.CapabilitiesRaw = evidence.CapabilitiesRaw;
}
monitor.VcpCapabilitiesInfo = caps;
UpdateMonitorCapabilitiesFromVcp(monitor, caps);
monitor.VcpCapabilitiesInfo = evidence.Capabilities;
UpdateMonitorCapabilitiesFromVcp(monitor, evidence.Capabilities);
// Initialize current values for every VCP feature the device reports
// support for, ordered continuous-range first (percent-scaled),
// then discrete-enum VCPs. Each guard is independent — a controller
// can support any subset.
if (monitor.SupportsBrightness)
// Continuous (percent-scaled) VCPs first, then the discrete-enum ones. Only the
// continuous stage can discard the monitor, and that is a policy choice rather
// than a property of the stage: losing a whole display because 0xD6 answered
// badly is worse than showing it without a power control. Note the check is not
// on every path — a caps string that parses but advertises none of 0x10/0x12/0x62
// leaves the continuous stage nothing to read and suppresses the probe, so such a
// monitor is published with a handle no VCP read has exercised.
if (!_continuousInitializer.Initialize(monitor, evidence))
{
InitializeBrightness(monitor, physical.HPhysicalMonitor);
}
if (monitor.SupportsContrast)
{
InitializeContrast(monitor, physical.HPhysicalMonitor);
}
if (monitor.SupportsVolume)
{
InitializeVolume(monitor, physical.HPhysicalMonitor);
Logger.LogWarning(
$"DDC: [DevicePath={info.DevicePath}] monitor ignored — physical monitor handle became unavailable during continuous VCP initialization");
return null;
}
if (monitor.SupportsColorTemperature)
@@ -389,7 +388,7 @@ namespace PowerDisplay.Common.Drivers.DDC
/// (cooperative — checked between attempts) and during the 1 s delay between
/// retries.
/// </summary>
private async Task<(string CapsString, VcpCapabilities? Caps)> FetchCapabilitiesWithFallbackAsync(
private async Task<VcpDiscoveryEvidence> FetchCapabilitiesWithFallbackAsync(
IntPtr hPhysicalMonitor,
CancellationToken cancellationToken)
{
@@ -442,56 +441,37 @@ namespace PowerDisplay.Common.Drivers.DDC
$"DDC: cap string still empty after {maxAttempts} attempts (handle=0x{hPhysicalMonitor:X})");
}
IReadOnlyDictionary<byte, VcpProbeObservation> live =
new Dictionary<byte, VcpProbeObservation>();
if (caps == null && MaxCompatibilityMode)
{
Logger.LogInfo(
$"DDC: [max-compat] caps unusable for handle=0x{hPhysicalMonitor:X}; probing VCP features directly");
var observations = await _probeService.ProbeAsync(hPhysicalMonitor, cancellationToken);
caps = BuildCapabilitiesFromProbe(observations);
live = await _probeService.ProbeAsync(hPhysicalMonitor, cancellationToken);
}
if (caps != null)
var evidence = VcpDiscoveryEvidence.Reconcile(capsString ?? string.Empty, caps, live);
if (live.Count > 0)
{
var message =
$"DDC: [max-compat] probe outcome for handle=0x{hPhysicalMonitor:X}: " +
$"{evidence.InitialValues.Count}/{live.Count} feature(s) read, " +
$"{evidence.Capabilities?.SupportedVcpCodes.Count ?? 0} advertised";
if (evidence.Capabilities != null)
{
Logger.LogInfo(
$"DDC: [max-compat] recovered monitor (handle=0x{hPhysicalMonitor:X}) " +
$"with {caps.SupportedVcpCodes.Count} probed feature(s)");
Logger.LogInfo(message);
}
else
{
Logger.LogWarning(
$"DDC: [max-compat] probe returned no supported features for handle=0x{hPhysicalMonitor:X}");
Logger.LogWarning(message);
}
}
return (capsString ?? string.Empty, caps);
}
/// <summary>
/// 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.
/// </summary>
/// <remarks>
/// Membership is decided by <see cref="VcpProbeObservation.Replied"/>, 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
/// <c>DDCCI_VCP_NOT_SUPPORTED</c> instead and never sets the flag. Friendly names come from
/// <see cref="VcpNames.GetCodeName"/> to keep a single source of truth.
/// </remarks>
private static VcpCapabilities? BuildCapabilitiesFromProbe(
IReadOnlyDictionary<byte, VcpProbeObservation> 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;
return evidence;
}
/// <summary>
@@ -530,72 +510,6 @@ namespace PowerDisplay.Common.Drivers.DDC
}
}
/// <summary>
/// Initialize brightness value for a monitor using VCP 0x10.
/// Persists the device-reported raw maximum so subsequent writes can scale percent → raw.
/// </summary>
private static void InitializeBrightness(Monitor monitor, IntPtr handle)
{
if (TryGetVcpFeature(handle, VcpCodeBrightness, monitor.Id, out uint current, out uint max))
{
var brightnessInfo = new VcpFeatureValue((int)current, 0, (int)max);
if (!brightnessInfo.IsValid)
{
Logger.LogWarning(
$"DDC: [{monitor.Id}] Ignoring invalid brightness range current={current}, max={max}");
return;
}
monitor.BrightnessVcpMax = (int)max;
monitor.CurrentBrightness = brightnessInfo.ToPercentage();
monitor.ReadValues |= MonitorReadFlags.Brightness;
}
}
/// <summary>
/// Initialize contrast value for a monitor using VCP 0x12.
/// Persists the device-reported raw maximum so subsequent writes can scale percent → raw.
/// </summary>
private static void InitializeContrast(Monitor monitor, IntPtr handle)
{
if (TryGetVcpFeature(handle, VcpCodeContrast, monitor.Id, out uint current, out uint max))
{
var contrastInfo = new VcpFeatureValue((int)current, 0, (int)max);
if (!contrastInfo.IsValid)
{
Logger.LogWarning(
$"DDC: [{monitor.Id}] Ignoring invalid contrast range current={current}, max={max}");
return;
}
monitor.ContrastVcpMax = (int)max;
monitor.CurrentContrast = contrastInfo.ToPercentage();
monitor.ReadValues |= MonitorReadFlags.Contrast;
}
}
/// <summary>
/// Initialize volume value for a monitor using VCP 0x62.
/// Persists the device-reported raw maximum so subsequent writes can scale percent → raw.
/// </summary>
private static void InitializeVolume(Monitor monitor, IntPtr handle)
{
if (TryGetVcpFeature(handle, VcpCodeVolume, monitor.Id, out uint current, out uint max))
{
var volumeInfo = new VcpFeatureValue((int)current, 0, (int)max);
if (!volumeInfo.IsValid)
{
Logger.LogWarning(
$"DDC: [{monitor.Id}] Ignoring invalid volume range current={current}, max={max}");
return;
}
monitor.VolumeVcpMax = (int)max;
monitor.CurrentVolume = volumeInfo.ToPercentage();
monitor.ReadValues |= MonitorReadFlags.Volume;
}
}
/// <summary>
/// Wrapper for GetVCPFeatureAndVCPFeatureReply that logs errors on failure.
/// </summary>
@@ -800,10 +714,18 @@ namespace PowerDisplay.Common.Drivers.DDC
// Async caps fetch (retry + max-compat probe). Awaits Task.Delay between
// retries instead of blocking the threadpool.
var (capsString, caps) = await FetchCapabilitiesWithFallbackAsync(
var evidence = await FetchCapabilitiesWithFallbackAsync(
physical.HPhysicalMonitor, cancellationToken);
if (caps == null)
if (evidence.IsPhysicalMonitorUnavailable)
{
Logger.LogWarning(
$"DDC: [DevicePath={info.DevicePath}] monitor ignored — physical monitor handle is no longer valid");
ReleaseAbandonedPhysical(physical);
continue;
}
if (evidence.Capabilities == null)
{
Logger.LogWarning(
$"DDC: [DevicePath={info.DevicePath}] monitor ignored — capabilities unavailable");
@@ -811,11 +733,11 @@ namespace PowerDisplay.Common.Drivers.DDC
continue;
}
// Heavy sync block (~6 × ~100 ms VCP reads on this one I2C bus).
// Dispatch to the threadpool; await before the next physical because
// they share the same hMonitor's I2C arbitration.
// Heavy sync block (VCP reads on this one I2C bus, minus whatever the probe
// already answered). Dispatch to the threadpool; await before the next physical
// because they share the same hMonitor's I2C arbitration.
var monitor = await Task.Run(
() => BuildMonitorFromPhysical(physical, info, capsString, caps),
() => BuildMonitorFromPhysical(physical, info, evidence),
cancellationToken);
if (monitor != null)

View File

@@ -10,8 +10,12 @@ namespace PowerDisplay.Common.Drivers.DDC
/// names mirror <c>winerror.h</c> exactly.
/// </summary>
/// <remarks>
/// <see cref="VcpFeatureProbeService"/> is the only consumer today. The discovery-time value
/// reads in <c>DdcCiController.TryGetVcpFeature</c> are still single-shot and unclassified.
/// Consumed by <see cref="VcpFeatureProbeService"/>, which retries only what
/// <see cref="IsTransient"/> admits, and by <see cref="ContinuousVcpInitializer"/>, which uses
/// <see cref="IsPhysicalMonitorUnavailable"/> to tell a dead handle from one feature the device
/// refused. The discrete-VCP reads in <c>DdcCiController.Initialize*</c> and the runtime value
/// reads behind <c>DdcCiController.GetVcpFeatureAsync</c> are still single-shot and
/// unclassified.
/// </remarks>
internal static class DdcErrorClassifier
{
@@ -83,5 +87,13 @@ namespace PowerDisplay.Common.Drivers.DDC
ErrorGraphicsDdcCiInvalidMessageChecksum or
ErrorGraphicsDdcCiCurrentCurrentValueGreaterThanMaximumValue or
ErrorTimeout;
/// <summary>
/// Renders an error for a log line as the unsigned hex the SDK documents it under, so a
/// reader can grep it against winerror.h. <c>Marshal.GetLastWin32Error</c> hands these back
/// as a negative int, which matches nothing.
/// </summary>
public static string Format(int? errorCode) =>
errorCode.HasValue ? $"0x{unchecked((uint)errorCode.Value):X8}" : "none";
}
}

View File

@@ -0,0 +1,115 @@
// 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.Collections.Generic;
using PowerDisplay.Common.Models;
using PowerDisplay.Common.Utils;
namespace PowerDisplay.Common.Drivers.DDC
{
/// <summary>
/// What discovery learned about one physical monitor before it is turned into a
/// <see cref="Monitor"/>: the capabilities it advertises and the values already read off it.
/// </summary>
/// <remarks>
/// This is the seam between the async fetch stage, which owns the I2C traffic, and the
/// synchronous build stage, which owns the <see cref="Monitor"/> object. Carrying the probe's
/// values across it is the point: without them the build stage re-reads every code the probe
/// just answered.
/// </remarks>
internal sealed class VcpDiscoveryEvidence
{
public VcpDiscoveryEvidence(
string capabilitiesRaw,
VcpCapabilities? capabilities,
IReadOnlyDictionary<byte, VcpFeatureValue> initialValues,
bool isPhysicalMonitorUnavailable = false)
{
CapabilitiesRaw = capabilitiesRaw;
Capabilities = capabilities;
InitialValues = initialValues;
IsPhysicalMonitorUnavailable = isPhysicalMonitorUnavailable;
}
public string CapabilitiesRaw { get; }
public VcpCapabilities? Capabilities { get; }
/// <summary>
/// Gets the values this pass already read off the hardware, keyed by VCP code. A code that
/// is absent still owes the hardware a read; a code that is present must not be re-read.
/// </summary>
public IReadOnlyDictionary<byte, VcpFeatureValue> InitialValues { get; }
public bool IsPhysicalMonitorUnavailable { get; }
/// <summary>
/// Folds this pass's probe observations into the parsed capabilities.
/// </summary>
/// <remarks>
/// The probe only runs when the capabilities string is unusable, so on the parsed path
/// <paramref name="live"/> is empty and this is a pass-through.
/// </remarks>
public static VcpDiscoveryEvidence Reconcile(
string capabilitiesRaw,
VcpCapabilities? parsedCapabilities,
IReadOnlyDictionary<byte, VcpProbeObservation> live)
{
foreach (var observation in live.Values)
{
if (observation.IsPhysicalMonitorUnavailable)
{
// The handle itself is gone, so nothing gathered against it can be trusted and
// nothing more may be issued through it.
return new VcpDiscoveryEvidence(
capabilitiesRaw,
capabilities: null,
new Dictionary<byte, VcpFeatureValue>(),
isPhysicalMonitorUnavailable: true);
}
}
var capabilities = parsedCapabilities;
var values = new Dictionary<byte, VcpFeatureValue>();
// Driven by what the probe reported rather than by NativeConstants.ContinuousVcpCodes,
// which VcpFeatureProbeService only takes as the default for its constructor-injected
// sweep list. Widening that sweep still needs a matching edit in
// ContinuousVcpInitializer for the carried value to be used — this loop only keeps the
// code from being dropped on the way there.
foreach (var (code, observation) in live)
{
if (!observation.Replied)
{
continue;
}
// A reply proves the device implements the opcode. Membership does not depend on
// the value being usable — an unimplemented code fails with DDCCI_VCP_NOT_SUPPORTED
// instead and never sets Replied.
capabilities = MarkSupported(capabilities, code);
if (observation.IsSuccess)
{
values[code] = observation.Value;
}
}
return new VcpDiscoveryEvidence(capabilitiesRaw, capabilities, values);
}
/// <summary>
/// Records that <paramref name="code"/> is supported, creating the container when discovery
/// produced no parsed capabilities. Adds only: an entry parsed from the capabilities string
/// carries discrete-value and custom-name metadata a synthesized <see cref="VcpCodeInfo"/>
/// does not.
/// </summary>
private static VcpCapabilities MarkSupported(VcpCapabilities? capabilities, byte code)
{
capabilities ??= new VcpCapabilities();
capabilities.SupportedVcpCodes.TryAdd(code, new VcpCodeInfo(code, VcpNames.GetCodeName(code)));
return capabilities;
}
}
}

View File

@@ -115,7 +115,7 @@ namespace PowerDisplay.Common.Drivers.DDC
Logger.LogDebug(
$"DDC: [max-compat] VCP probe attempt " +
$"(handle=0x{handle:X}, code=0x{code:X2}, attempt={attempt}/{MaxAttempts}, " +
$"status=failed, error={FormatError(lastError)})");
$"status=failed, error={DdcErrorClassifier.Format(lastError)})");
if (!DdcErrorClassifier.IsTransient(result.ErrorCode))
{
break;
@@ -141,7 +141,7 @@ namespace PowerDisplay.Common.Drivers.DDC
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)})";
$"status={status}, replied={observation.Replied}, lastError={DdcErrorClassifier.Format(observation.LastError)})";
if (observation.IsSuccess)
{
@@ -154,9 +154,6 @@ namespace PowerDisplay.Common.Drivers.DDC
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)