diff --git a/config/dot/quickshell/scripts/panama-brightness b/config/dot/quickshell/scripts/panama-brightness index 5e3af27..11a5f72 100755 --- a/config/dot/quickshell/scripts/panama-brightness +++ b/config/dot/quickshell/scripts/panama-brightness @@ -57,6 +57,39 @@ has_accessible_bus() { return 1 } +# The I2C bus that carries DDC/CI for one connector. +# +# There are two, and picking the wrong one finds no monitor at all: +# +# DisplayPort carries DDC/CI over the AUX channel. That adapter shows up as a +# child directory of the connector -- /sys/class/drm/card1-DP-2/i2c-9 -- and +# is the one ddcutil talks to. +# +# The connector's `ddc` symlink points at the classic I2C line used for EDID +# on HDMI and DVI. On a DisplayPort connector it still exists and still +# resolves, but nothing answers on it: this machine's DP-2 has ddc -> i2c-5, +# where ddcutil reports "No monitor detected", while i2c-9 answers VCP 0x10 +# immediately. +# +# So prefer the AUX child and fall back to the symlink. Note the readlink: the +# entries under /sys/class/drm are symlinks, and `find` does not follow the path +# it is given, so searching the unresolved path silently finds nothing. +bus_for_connector() { + local path="$1" real aux ddc + + real="$(readlink -f "$path")" + aux="$(find "$real" -maxdepth 1 -name 'i2c-*' -printf '%f' -quit 2>/dev/null)" + if [[ -n "$aux" ]]; then + printf '%s' "${aux#i2c-}" + return 0 + fi + + ddc="$(readlink -f "$path/ddc" 2>/dev/null)" || return 1 + [[ -n "$ddc" ]] || return 1 + ddc="$(basename "$ddc")" + printf '%s' "${ddc#i2c-}" +} + cmd_list() { has_accessible_bus || emit_error 'no I2C bus is accessible. ddcutil ships a udev rule that grants this, but only to devices created after it was installed. Run: sudo udevadm control --reload-rules && sudo udevadm trigger --subsystem-match=i2c-dev --subsystem-match=drm' @@ -69,8 +102,7 @@ cmd_list() { connector="$(basename "$path")" connector="${connector#card*-}" - bus="$(basename "$(readlink -f "$path/ddc")")" - bus="${bus#i2c-}" + bus="$(bus_for_connector "$path")" [[ "$bus" =~ ^[0-9]+$ ]] || continue # A monitor that does not implement 0x10 is not an error; it simply diff --git a/tests/quickshell/brightness-helper-contract.sh b/tests/quickshell/brightness-helper-contract.sh index 085ae65..62203d4 100755 --- a/tests/quickshell/brightness-helper-contract.sh +++ b/tests/quickshell/brightness-helper-contract.sh @@ -34,15 +34,35 @@ mkdir -p "$fixture/drm" "$fixture/dev" "$fixture/bin" "$fixture/i2c" # Two connectors with a monitor, two without. DP-2 answers DDC; DP-3 is # connected but does not implement brightness. HDMI-A-1 and DP-1 are empty and # must never be probed at all. +# A connector has an EDID bus (the `ddc` symlink) and, on DisplayPort, an AUX +# bus that appears as a child directory. DDC/CI rides the AUX channel where one +# exists, and the `ddc` line answers nothing on a DP connector even though it +# still resolves -- so the aux argument here is what a real DisplayPort monitor +# looks like, and omitting it is what HDMI and DVI look like. make_connector() { - local name="$1" bus="$2" status="$3" - mkdir -p "$fixture/drm/$name" - printf '%s\n' "$status" >"$fixture/drm/$name/status" - mkdir -p "$fixture/i2c/i2c-$bus" - ln -sfn "$fixture/i2c/i2c-$bus" "$fixture/drm/$name/ddc" + local name="$1" ddc_bus="$2" status="$3" aux_bus="${4:-}" + local device="$fixture/devices/$name" + + # /sys/class/drm/ is a SYMLINK to the real device directory, and + # this fixture mirrors that rather than using a plain directory. It matters: + # `find` does not follow the path it is given, so code that searches the + # unresolved path finds nothing while appearing to work anywhere the entry + # happens to be a real directory. + mkdir -p "$device" + printf '%s\n' "$status" >"$device/status" + mkdir -p "$fixture/i2c/i2c-$ddc_bus" + ln -sfn "$fixture/i2c/i2c-$ddc_bus" "$device/ddc" + [[ -n "$aux_bus" ]] && mkdir -p "$device/i2c-$aux_bus" + + mkdir -p "$fixture/drm" + ln -sfn "$device" "$fixture/drm/$name" + return 0 } +# DP-2 is the DisplayPort case: its EDID line is bus 5, which answers nothing, +# and its AUX child is bus 9, which does. Choosing bus 5 here finds no monitor +# at all, which is exactly the bug this pins down. make_connector card1-DP-1 4 disconnected -make_connector card1-DP-2 5 connected +make_connector card1-DP-2 5 connected 9 make_connector card1-DP-3 6 connected make_connector card1-HDMI-A-1 7 disconnected @@ -75,7 +95,7 @@ for arg in "$@"; do done case "$bus" in - 5) printf 'VCP 10 C 120 200\n'; exit 0 ;; + 9) printf 'VCP 10 C 120 200\n'; exit 0 ;; *) exit 1 ;; esac STUB @@ -103,8 +123,9 @@ jq -e . >/dev/null 2>&1 <<<"$listing" || fail "list did not emit JSON: $listing" [[ "$(jq -r '.displays[0].connector' <<<"$listing")" == "DP-2" ]] \ || fail "the connector name must match Hyprland's output name: $listing" -[[ "$(jq -r '.displays[0].bus' <<<"$listing")" == "5" ]] \ - || fail "the display was mapped to the wrong I2C bus: $listing" +# The AUX bus, not the EDID bus its `ddc` symlink points at. +[[ "$(jq -r '.displays[0].bus' <<<"$listing")" == "9" ]] \ + || fail "the display was mapped to its EDID bus instead of its DisplayPort AUX bus, where nothing answers: $listing" # 120 of a maximum of 200 is 60%. [[ "$(jq -r '.displays[0].value' <<<"$listing")" == "60" ]] \ @@ -123,12 +144,12 @@ if grep -qxE '4|7' "$DDCUTIL_PROBE_LOG"; then fi # ── Writes scale to the reported maximum ───────────────────────────────────── -run_helper set 5 40 -[[ "$(tail -1 "$DDCUTIL_SET_LOG")" == "set 5 80" ]] \ +run_helper set 9 40 +[[ "$(tail -1 "$DDCUTIL_SET_LOG")" == "set 9 80" ]] \ || fail "set did not scale to the display's maximum: $(cat "$DDCUTIL_SET_LOG")" -run_helper set 5 500 -[[ "$(tail -1 "$DDCUTIL_SET_LOG")" == "set 5 200" ]] \ +run_helper set 9 500 +[[ "$(tail -1 "$DDCUTIL_SET_LOG")" == "set 9 200" ]] \ || fail "an out-of-range percent was not clamped: $(cat "$DDCUTIL_SET_LOG")" # ── No I2C access explains itself ────────────────────────────────────────────