From 571ff40704529aa9992293a9fb6aa32c6c602eef Mon Sep 17 00:00:00 2001 From: Daniel Nouri Date: Sat, 4 Oct 2025 16:31:01 +0200 Subject: [PATCH 1/5] alsa: Fix IEC958 digital output not unmuted on path activation When selecting an HDMI/DisplayPort (IEC958) output path, the hardware mute switch remains in kernel default state (muted), causing no audio output despite correct software routing. Root cause: pa_alsa_path_select() only sets mute switches when mute_during_activation is enabled. No mixer paths enable this setting, making the switch configuration code unreachable for IEC958 paths. Solution: Always set mute switches to match device mute status after path activation, regardless of mute_during_activation setting. Testing: Added test-alsa-path-select tool to verify the fix. - Loads mixer path and calls pa_alsa_path_select() - Verifies switch states match expected values - Tested on AMD Radeon HDMI and Realtek ALC257 analog Manual verification: - Before: IEC958 switch OFF, no audio - After: IEC958 switch set correctly, audio works This bug was inherited from PulseAudio's ALSA mixer path code where HDMI path configurations lack IEC958 unmute sections. Fixes: https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/3261 See-Also: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/issues/562 See-Also: https://github.com/alsa-project/alsa-ucm-conf/commit/33be660e4b1e75c19d5332556c3d2636dd3344bf See-Also: https://bugs.launchpad.net/hundredpapercuts/+bug/681996 --- spa/plugins/alsa/acp/alsa-mixer.c | 24 +- spa/plugins/alsa/acp/meson.build | 9 + spa/plugins/alsa/acp/test-alsa-path-select.c | 348 +++++++++++++++++++ 3 files changed, 374 insertions(+), 7 deletions(-) create mode 100644 spa/plugins/alsa/acp/test-alsa-path-select.c diff --git a/spa/plugins/alsa/acp/alsa-mixer.c b/spa/plugins/alsa/acp/alsa-mixer.c index 1f494ee0b..64088177f 100644 --- a/spa/plugins/alsa/acp/alsa-mixer.c +++ b/spa/plugins/alsa/acp/alsa-mixer.c @@ -1479,13 +1479,23 @@ int pa_alsa_path_select(pa_alsa_path *p, pa_alsa_setting *s, snd_mixer_t *m, boo if (s) setting_select(s, m); - /* Finally restore hw mute to the device mute status. */ - if (p->mute_during_activation) { - PA_LLIST_FOREACH(e, p->elements) { - if (e->switch_use == PA_ALSA_SWITCH_MUTE) { - if (element_set_switch(e, m, !device_is_muted) < 0) - return -1; - } + /* Set hw mute switches to match device mute status. + * + * When mute_during_activation is enabled, switches are temporarily muted + * during path setup to avoid pops/clicks, and this restores them. + * + * When mute_during_activation is disabled (default), switches may still be + * in their kernel default state (often muted for digital outputs like IEC958). + * We must explicitly unmute them if the device isn't muted. + * + * This ensures HDMI/DisplayPort audio (IEC958) works correctly, as these + * digital output switches default to muted in ALSA drivers but are not + * automatically enabled when the path is selected. + */ + PA_LLIST_FOREACH(e, p->elements) { + if (e->switch_use == PA_ALSA_SWITCH_MUTE) { + if (element_set_switch(e, m, !device_is_muted) < 0) + return -1; } } diff --git a/spa/plugins/alsa/acp/meson.build b/spa/plugins/alsa/acp/meson.build index 0ec97e2b4..f5adfa193 100644 --- a/spa/plugins/alsa/acp/meson.build +++ b/spa/plugins/alsa/acp/meson.build @@ -20,3 +20,12 @@ acp_lib = static_library( dependencies : [ spa_dep, alsa_dep, mathlib, ] ) acp_dep = declare_dependency(link_with: acp_lib) + +executable('test-alsa-path-select', + [ 'test-alsa-path-select.c' ], + c_args : acp_c_args, + include_directories : [configinc, includes_inc ], + dependencies : [ spa_dep, alsa_dep, mathlib, ], + link_with : [ acp_lib ], + install : false, +) diff --git a/spa/plugins/alsa/acp/test-alsa-path-select.c b/spa/plugins/alsa/acp/test-alsa-path-select.c new file mode 100644 index 000000000..f30662d50 --- /dev/null +++ b/spa/plugins/alsa/acp/test-alsa-path-select.c @@ -0,0 +1,348 @@ +/* ALSA path select test tool */ +/* SPDX-FileCopyrightText: Copyright © 2025 Daniel Nouri */ +/* SPDX-License-Identifier: MIT */ + +/* + * Tests pa_alsa_path_select() to verify mute switches are correctly set + * based on device mute status, regardless of mute_during_activation setting. + * + * Regression test for HDMI/DisplayPort audio bug where IEC958 switches + * remain in kernel default state (OFF) because pa_alsa_path_select() only + * set switches when mute_during_activation=true, but no paths enable this. + * + * Test verification: + * Before fix: IEC958 switch stays OFF after path select → test FAILS + * After fix: IEC958 switch set correctly based on device state → test PASSES + * + * Usage: + * test-alsa-path-select -c [-p ] [-m] [-v] + * + * Examples: + * test-alsa-path-select -c 1 # IEC958 unmuted (expect ON) + * test-alsa-path-select -c 1 -m # IEC958 muted (expect OFF) + * test-alsa-path-select -c 0 -p analog-output # Regression test + * + * Exit codes: + * 0 = Test passed + * 1 = Test failed + * 2 = Setup error + */ + +#include +#include +#include +#include +#include +#include +#include + +#include "alsa-mixer.h" +#include "alsa-util.h" + +#define DEFAULT_PATHS_DIR "/usr/share/alsa-card-profile/mixer/paths" + +struct test_config { + int card; + const char *path_name; + const char *paths_dir; + bool device_muted; + bool verbose; +}; + +static void show_help(const char *name) +{ + printf("Usage: %s [options]\n", name); + printf("\n"); + printf("Test tool for pa_alsa_path_select() - verifies IEC958 unmute fix\n"); + printf("\n"); + printf("Options:\n"); + printf(" -c, --card ALSA card number (required)\n"); + printf(" -p, --path Mixer path name (default: iec958-stereo-output)\n"); + printf(" -d, --paths-dir Path to mixer path configs (default: %s)\n", DEFAULT_PATHS_DIR); + printf(" -m, --muted Test with device muted (default: unmuted)\n"); + printf(" -v, --verbose Verbose output\n"); + printf(" -h, --help Show this help\n"); + printf("\n"); + printf("Examples:\n"); + printf(" # Test IEC958 unmute on card 1\n"); + printf(" %s -c 1\n", name); + printf("\n"); + printf(" # Test analog output\n"); + printf(" %s -c 0 -p analog-output\n", name); + printf("\n"); + printf(" # Test with device muted\n"); + printf(" %s -c 1 -m\n", name); + printf("\n"); + printf("Exit codes:\n"); + printf(" 0 - Test passed (switches set correctly)\n"); + printf(" 1 - Test failed (switches not set as expected)\n"); + printf(" 2 - Setup error (path not found, hardware error, etc.)\n"); +} + +static int parse_args(int argc, char *argv[], struct test_config *config) +{ + static const struct option long_options[] = { + { "card", required_argument, NULL, 'c' }, + { "path", required_argument, NULL, 'p' }, + { "paths-dir", required_argument, NULL, 'd' }, + { "muted", no_argument, NULL, 'm' }, + { "verbose", no_argument, NULL, 'v' }, + { "help", no_argument, NULL, 'h' }, + { NULL, 0, NULL, 0 } + }; + + int c; + config->card = -1; + config->path_name = "iec958-stereo-output.conf"; + config->paths_dir = DEFAULT_PATHS_DIR; + config->device_muted = false; + config->verbose = false; + + while ((c = getopt_long(argc, argv, "c:p:d:mvh", long_options, NULL)) != -1) { + switch (c) { + case 'c': + config->card = atoi(optarg); + break; + case 'p': + config->path_name = optarg; + break; + case 'd': + config->paths_dir = optarg; + break; + case 'm': + config->device_muted = true; + break; + case 'v': + config->verbose = true; + break; + case 'h': + show_help(argv[0]); + return -1; + default: + show_help(argv[0]); + return -2; + } + } + + if (config->card < 0) { + fprintf(stderr, "Error: Card number is required (-c )\n\n"); + show_help(argv[0]); + return -2; + } + + return 0; +} + +static int get_switch_state(snd_mixer_t *mixer, const char *name, int index, bool *state) +{ + snd_mixer_elem_t *elem; + snd_mixer_selem_id_t *sid; + int val; + + snd_mixer_selem_id_alloca(&sid); + snd_mixer_selem_id_set_name(sid, name); + snd_mixer_selem_id_set_index(sid, index); + + elem = snd_mixer_find_selem(mixer, sid); + if (!elem) { + fprintf(stderr, "Warning: Element '%s',%d not found\n", name, index); + return -ENOENT; + } + + if (!snd_mixer_selem_has_playback_switch(elem)) { + fprintf(stderr, "Warning: Element '%s',%d has no playback switch\n", name, index); + return -EINVAL; + } + + if (snd_mixer_selem_get_playback_switch(elem, SND_MIXER_SCHN_MONO, &val) < 0) { + if (snd_mixer_selem_get_playback_switch(elem, SND_MIXER_SCHN_FRONT_LEFT, &val) < 0) { + fprintf(stderr, "Error: Cannot read switch state for '%s',%d\n", name, index); + return -EIO; + } + } + + *state = val ? true : false; + return 0; +} + +int main(int argc, char *argv[]) +{ + struct test_config config; + pa_alsa_path *path = NULL; + snd_mixer_t *mixer = NULL; + char card_name[64]; + int ret = 2; // Setup error by default + int err; + + /* Parse arguments */ + err = parse_args(argc, argv, &config); + if (err < 0) { + return err == -1 ? 0 : 2; // -1 is help, -2 is error + } + + printf("Testing pa_alsa_path_select() with:\n"); + printf(" Card: %d\n", config.card); + printf(" Path: %s\n", config.path_name); + printf(" Paths dir: %s\n", config.paths_dir); + printf(" Device muted: %s\n", config.device_muted ? "yes" : "no"); + printf("\n"); + + /* Load mixer path */ + path = pa_alsa_path_new(config.paths_dir, config.path_name, PA_ALSA_DIRECTION_OUTPUT); + if (!path) { + fprintf(stderr, "Error: Failed to load path '%s' from %s\n", + config.path_name, config.paths_dir); + fprintf(stderr, "Make sure the path file exists: %s/%s.conf\n", + config.paths_dir, config.path_name); + goto cleanup; + } + + /* Count elements BEFORE probing */ + int element_count_before = 0; + pa_alsa_element *e_temp; + PA_LLIST_FOREACH(e_temp, path->elements) { + element_count_before++; + } + printf("Elements in path BEFORE probing: %d\n", element_count_before); + + if (config.verbose && element_count_before > 0) { + printf("Loaded path: %s\n", path->name ? path->name : config.path_name); + printf(" mute_during_activation: %s\n", path->mute_during_activation ? "yes" : "no"); + printf(" Elements:\n"); + + PA_LLIST_FOREACH(e_temp, path->elements) { + const char *switch_use; + switch (e_temp->switch_use) { + case PA_ALSA_SWITCH_MUTE: switch_use = "mute"; break; + case PA_ALSA_SWITCH_ON: switch_use = "on"; break; + case PA_ALSA_SWITCH_OFF: switch_use = "off"; break; + default: switch_use = "ignore"; break; + } + printf(" - %s (index %d): switch=%s\n", + e_temp->alsa_id.name, e_temp->alsa_id.index, switch_use); + } + printf("\n"); + } + + /* Open ALSA mixer */ + snprintf(card_name, sizeof(card_name), "hw:%d", config.card); + + if (snd_mixer_open(&mixer, 0) < 0) { + fprintf(stderr, "Error: Failed to open mixer\n"); + goto cleanup; + } + + if (snd_mixer_attach(mixer, card_name) < 0) { + fprintf(stderr, "Error: Failed to attach mixer to %s\n", card_name); + fprintf(stderr, "Make sure card %d exists (check with: aplay -l)\n", config.card); + goto cleanup; + } + + if (snd_mixer_selem_register(mixer, NULL, NULL) < 0) { + fprintf(stderr, "Error: Failed to register mixer\n"); + goto cleanup; + } + + if (snd_mixer_load(mixer) < 0) { + fprintf(stderr, "Error: Failed to load mixer elements\n"); + goto cleanup; + } + + /* Probe the path to ensure elements are available */ + printf("Probing path...\n"); + if (pa_alsa_path_probe(path, NULL, mixer, false) < 0) { + fprintf(stderr, "Error: Path probe failed - required elements not found on card\n"); + goto cleanup; + } + printf("Path probe: OK\n\n"); + + /* Record switch states before activation */ + printf("Switch states BEFORE pa_alsa_path_select():\n"); + pa_alsa_element *e; + int mute_switch_count = 0; + PA_LLIST_FOREACH(e, path->elements) { + if (e->switch_use == PA_ALSA_SWITCH_MUTE) { + bool state; + mute_switch_count++; + if (get_switch_state(mixer, e->alsa_id.name, e->alsa_id.index, &state) == 0) { + printf(" %s,%d = %s\n", e->alsa_id.name, e->alsa_id.index, + state ? "ON" : "OFF"); + } + } + } + + if (mute_switch_count == 0) { + fprintf(stderr, "Warning: No mute switches found in path\n"); + printf("This test requires a path with switch=mute elements\n"); + ret = 2; + goto cleanup; + } + + /* Call pa_alsa_path_select() - THE FUNCTION UNDER TEST */ + printf("\nCalling pa_alsa_path_select(path, NULL, mixer, device_is_muted=%s)...\n", + config.device_muted ? "true" : "false"); + + err = pa_alsa_path_select(path, NULL, mixer, config.device_muted); + if (err < 0) { + fprintf(stderr, "Error: pa_alsa_path_select() failed with error %d\n", err); + ret = 2; + goto cleanup; + } + printf("pa_alsa_path_select(): OK\n\n"); + + /* Verify switch states after activation */ + printf("Switch states AFTER pa_alsa_path_select():\n"); + bool all_correct = true; + bool expected_state = !config.device_muted; // unmuted device = switches ON + + PA_LLIST_FOREACH(e, path->elements) { + if (e->switch_use == PA_ALSA_SWITCH_MUTE) { + bool actual_state; + if (get_switch_state(mixer, e->alsa_id.name, e->alsa_id.index, &actual_state) == 0) { + printf(" %s,%d = %s", e->alsa_id.name, e->alsa_id.index, + actual_state ? "ON" : "OFF"); + + if (actual_state == expected_state) { + printf(" ✓\n"); + } else { + printf(" ✗ (expected %s)\n", expected_state ? "ON" : "OFF"); + all_correct = false; + } + } + } + } + + /* Determine test result */ + printf("\n"); + printf("========================================\n"); + if (all_correct) { + printf("TEST PASSED\n"); + printf("All mute switches correctly set to %s\n", expected_state ? "ON" : "OFF"); + if (!path->mute_during_activation) { + printf("\nThis proves the fix works:\n"); + printf("- Path has mute_during_activation=false (default)\n"); + printf("- Switches were still set correctly\n"); + printf("- Bug would have left them in kernel default state\n"); + } + ret = 0; + } else { + printf("TEST FAILED\n"); + printf("One or more switches NOT set correctly\n"); + printf("Expected: %s, but some switches were %s\n", + expected_state ? "ON (unmuted)" : "OFF (muted)", + expected_state ? "OFF" : "ON"); + printf("\nThis indicates the bug is still present:\n"); + printf("- Switches not set when mute_during_activation=false\n"); + printf("- IEC958 fix may not be applied\n"); + ret = 1; + } + printf("========================================\n"); + +cleanup: + if (mixer) + snd_mixer_close(mixer); + if (path) + pa_alsa_path_free(path); + + return ret; +} From 5c3def51a429bdebbcbd862cdabe3eb2dc3ff9f1 Mon Sep 17 00:00:00 2001 From: Daniel Nouri Date: Sun, 5 Oct 2025 00:21:24 +0200 Subject: [PATCH 2/5] refactor: Remove redundant ret assignments in test-alsa-path-select The variable ret is initialized to 2 at function entry. Error paths that want this default value can simply goto cleanup without reassigning it. --- spa/plugins/alsa/acp/test-alsa-path-select.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/spa/plugins/alsa/acp/test-alsa-path-select.c b/spa/plugins/alsa/acp/test-alsa-path-select.c index f30662d50..e9bf4b3cc 100644 --- a/spa/plugins/alsa/acp/test-alsa-path-select.c +++ b/spa/plugins/alsa/acp/test-alsa-path-select.c @@ -274,7 +274,6 @@ int main(int argc, char *argv[]) if (mute_switch_count == 0) { fprintf(stderr, "Warning: No mute switches found in path\n"); printf("This test requires a path with switch=mute elements\n"); - ret = 2; goto cleanup; } @@ -285,7 +284,6 @@ int main(int argc, char *argv[]) err = pa_alsa_path_select(path, NULL, mixer, config.device_muted); if (err < 0) { fprintf(stderr, "Error: pa_alsa_path_select() failed with error %d\n", err); - ret = 2; goto cleanup; } printf("pa_alsa_path_select(): OK\n\n"); From 87d34335f33d4a8db8a359c5c55dcbdee3e02fbe Mon Sep 17 00:00:00 2001 From: Daniel Nouri Date: Sun, 5 Oct 2025 21:16:23 +0200 Subject: [PATCH 3/5] refactor: Remove test-alsa-path-select tool Not run by meson test and requires specific ALSA hardware. Fix verification already completed manually. --- spa/plugins/alsa/acp/meson.build | 9 - spa/plugins/alsa/acp/test-alsa-path-select.c | 346 ------------------- 2 files changed, 355 deletions(-) delete mode 100644 spa/plugins/alsa/acp/test-alsa-path-select.c diff --git a/spa/plugins/alsa/acp/meson.build b/spa/plugins/alsa/acp/meson.build index f5adfa193..0ec97e2b4 100644 --- a/spa/plugins/alsa/acp/meson.build +++ b/spa/plugins/alsa/acp/meson.build @@ -20,12 +20,3 @@ acp_lib = static_library( dependencies : [ spa_dep, alsa_dep, mathlib, ] ) acp_dep = declare_dependency(link_with: acp_lib) - -executable('test-alsa-path-select', - [ 'test-alsa-path-select.c' ], - c_args : acp_c_args, - include_directories : [configinc, includes_inc ], - dependencies : [ spa_dep, alsa_dep, mathlib, ], - link_with : [ acp_lib ], - install : false, -) diff --git a/spa/plugins/alsa/acp/test-alsa-path-select.c b/spa/plugins/alsa/acp/test-alsa-path-select.c deleted file mode 100644 index e9bf4b3cc..000000000 --- a/spa/plugins/alsa/acp/test-alsa-path-select.c +++ /dev/null @@ -1,346 +0,0 @@ -/* ALSA path select test tool */ -/* SPDX-FileCopyrightText: Copyright © 2025 Daniel Nouri */ -/* SPDX-License-Identifier: MIT */ - -/* - * Tests pa_alsa_path_select() to verify mute switches are correctly set - * based on device mute status, regardless of mute_during_activation setting. - * - * Regression test for HDMI/DisplayPort audio bug where IEC958 switches - * remain in kernel default state (OFF) because pa_alsa_path_select() only - * set switches when mute_during_activation=true, but no paths enable this. - * - * Test verification: - * Before fix: IEC958 switch stays OFF after path select → test FAILS - * After fix: IEC958 switch set correctly based on device state → test PASSES - * - * Usage: - * test-alsa-path-select -c [-p ] [-m] [-v] - * - * Examples: - * test-alsa-path-select -c 1 # IEC958 unmuted (expect ON) - * test-alsa-path-select -c 1 -m # IEC958 muted (expect OFF) - * test-alsa-path-select -c 0 -p analog-output # Regression test - * - * Exit codes: - * 0 = Test passed - * 1 = Test failed - * 2 = Setup error - */ - -#include -#include -#include -#include -#include -#include -#include - -#include "alsa-mixer.h" -#include "alsa-util.h" - -#define DEFAULT_PATHS_DIR "/usr/share/alsa-card-profile/mixer/paths" - -struct test_config { - int card; - const char *path_name; - const char *paths_dir; - bool device_muted; - bool verbose; -}; - -static void show_help(const char *name) -{ - printf("Usage: %s [options]\n", name); - printf("\n"); - printf("Test tool for pa_alsa_path_select() - verifies IEC958 unmute fix\n"); - printf("\n"); - printf("Options:\n"); - printf(" -c, --card ALSA card number (required)\n"); - printf(" -p, --path Mixer path name (default: iec958-stereo-output)\n"); - printf(" -d, --paths-dir Path to mixer path configs (default: %s)\n", DEFAULT_PATHS_DIR); - printf(" -m, --muted Test with device muted (default: unmuted)\n"); - printf(" -v, --verbose Verbose output\n"); - printf(" -h, --help Show this help\n"); - printf("\n"); - printf("Examples:\n"); - printf(" # Test IEC958 unmute on card 1\n"); - printf(" %s -c 1\n", name); - printf("\n"); - printf(" # Test analog output\n"); - printf(" %s -c 0 -p analog-output\n", name); - printf("\n"); - printf(" # Test with device muted\n"); - printf(" %s -c 1 -m\n", name); - printf("\n"); - printf("Exit codes:\n"); - printf(" 0 - Test passed (switches set correctly)\n"); - printf(" 1 - Test failed (switches not set as expected)\n"); - printf(" 2 - Setup error (path not found, hardware error, etc.)\n"); -} - -static int parse_args(int argc, char *argv[], struct test_config *config) -{ - static const struct option long_options[] = { - { "card", required_argument, NULL, 'c' }, - { "path", required_argument, NULL, 'p' }, - { "paths-dir", required_argument, NULL, 'd' }, - { "muted", no_argument, NULL, 'm' }, - { "verbose", no_argument, NULL, 'v' }, - { "help", no_argument, NULL, 'h' }, - { NULL, 0, NULL, 0 } - }; - - int c; - config->card = -1; - config->path_name = "iec958-stereo-output.conf"; - config->paths_dir = DEFAULT_PATHS_DIR; - config->device_muted = false; - config->verbose = false; - - while ((c = getopt_long(argc, argv, "c:p:d:mvh", long_options, NULL)) != -1) { - switch (c) { - case 'c': - config->card = atoi(optarg); - break; - case 'p': - config->path_name = optarg; - break; - case 'd': - config->paths_dir = optarg; - break; - case 'm': - config->device_muted = true; - break; - case 'v': - config->verbose = true; - break; - case 'h': - show_help(argv[0]); - return -1; - default: - show_help(argv[0]); - return -2; - } - } - - if (config->card < 0) { - fprintf(stderr, "Error: Card number is required (-c )\n\n"); - show_help(argv[0]); - return -2; - } - - return 0; -} - -static int get_switch_state(snd_mixer_t *mixer, const char *name, int index, bool *state) -{ - snd_mixer_elem_t *elem; - snd_mixer_selem_id_t *sid; - int val; - - snd_mixer_selem_id_alloca(&sid); - snd_mixer_selem_id_set_name(sid, name); - snd_mixer_selem_id_set_index(sid, index); - - elem = snd_mixer_find_selem(mixer, sid); - if (!elem) { - fprintf(stderr, "Warning: Element '%s',%d not found\n", name, index); - return -ENOENT; - } - - if (!snd_mixer_selem_has_playback_switch(elem)) { - fprintf(stderr, "Warning: Element '%s',%d has no playback switch\n", name, index); - return -EINVAL; - } - - if (snd_mixer_selem_get_playback_switch(elem, SND_MIXER_SCHN_MONO, &val) < 0) { - if (snd_mixer_selem_get_playback_switch(elem, SND_MIXER_SCHN_FRONT_LEFT, &val) < 0) { - fprintf(stderr, "Error: Cannot read switch state for '%s',%d\n", name, index); - return -EIO; - } - } - - *state = val ? true : false; - return 0; -} - -int main(int argc, char *argv[]) -{ - struct test_config config; - pa_alsa_path *path = NULL; - snd_mixer_t *mixer = NULL; - char card_name[64]; - int ret = 2; // Setup error by default - int err; - - /* Parse arguments */ - err = parse_args(argc, argv, &config); - if (err < 0) { - return err == -1 ? 0 : 2; // -1 is help, -2 is error - } - - printf("Testing pa_alsa_path_select() with:\n"); - printf(" Card: %d\n", config.card); - printf(" Path: %s\n", config.path_name); - printf(" Paths dir: %s\n", config.paths_dir); - printf(" Device muted: %s\n", config.device_muted ? "yes" : "no"); - printf("\n"); - - /* Load mixer path */ - path = pa_alsa_path_new(config.paths_dir, config.path_name, PA_ALSA_DIRECTION_OUTPUT); - if (!path) { - fprintf(stderr, "Error: Failed to load path '%s' from %s\n", - config.path_name, config.paths_dir); - fprintf(stderr, "Make sure the path file exists: %s/%s.conf\n", - config.paths_dir, config.path_name); - goto cleanup; - } - - /* Count elements BEFORE probing */ - int element_count_before = 0; - pa_alsa_element *e_temp; - PA_LLIST_FOREACH(e_temp, path->elements) { - element_count_before++; - } - printf("Elements in path BEFORE probing: %d\n", element_count_before); - - if (config.verbose && element_count_before > 0) { - printf("Loaded path: %s\n", path->name ? path->name : config.path_name); - printf(" mute_during_activation: %s\n", path->mute_during_activation ? "yes" : "no"); - printf(" Elements:\n"); - - PA_LLIST_FOREACH(e_temp, path->elements) { - const char *switch_use; - switch (e_temp->switch_use) { - case PA_ALSA_SWITCH_MUTE: switch_use = "mute"; break; - case PA_ALSA_SWITCH_ON: switch_use = "on"; break; - case PA_ALSA_SWITCH_OFF: switch_use = "off"; break; - default: switch_use = "ignore"; break; - } - printf(" - %s (index %d): switch=%s\n", - e_temp->alsa_id.name, e_temp->alsa_id.index, switch_use); - } - printf("\n"); - } - - /* Open ALSA mixer */ - snprintf(card_name, sizeof(card_name), "hw:%d", config.card); - - if (snd_mixer_open(&mixer, 0) < 0) { - fprintf(stderr, "Error: Failed to open mixer\n"); - goto cleanup; - } - - if (snd_mixer_attach(mixer, card_name) < 0) { - fprintf(stderr, "Error: Failed to attach mixer to %s\n", card_name); - fprintf(stderr, "Make sure card %d exists (check with: aplay -l)\n", config.card); - goto cleanup; - } - - if (snd_mixer_selem_register(mixer, NULL, NULL) < 0) { - fprintf(stderr, "Error: Failed to register mixer\n"); - goto cleanup; - } - - if (snd_mixer_load(mixer) < 0) { - fprintf(stderr, "Error: Failed to load mixer elements\n"); - goto cleanup; - } - - /* Probe the path to ensure elements are available */ - printf("Probing path...\n"); - if (pa_alsa_path_probe(path, NULL, mixer, false) < 0) { - fprintf(stderr, "Error: Path probe failed - required elements not found on card\n"); - goto cleanup; - } - printf("Path probe: OK\n\n"); - - /* Record switch states before activation */ - printf("Switch states BEFORE pa_alsa_path_select():\n"); - pa_alsa_element *e; - int mute_switch_count = 0; - PA_LLIST_FOREACH(e, path->elements) { - if (e->switch_use == PA_ALSA_SWITCH_MUTE) { - bool state; - mute_switch_count++; - if (get_switch_state(mixer, e->alsa_id.name, e->alsa_id.index, &state) == 0) { - printf(" %s,%d = %s\n", e->alsa_id.name, e->alsa_id.index, - state ? "ON" : "OFF"); - } - } - } - - if (mute_switch_count == 0) { - fprintf(stderr, "Warning: No mute switches found in path\n"); - printf("This test requires a path with switch=mute elements\n"); - goto cleanup; - } - - /* Call pa_alsa_path_select() - THE FUNCTION UNDER TEST */ - printf("\nCalling pa_alsa_path_select(path, NULL, mixer, device_is_muted=%s)...\n", - config.device_muted ? "true" : "false"); - - err = pa_alsa_path_select(path, NULL, mixer, config.device_muted); - if (err < 0) { - fprintf(stderr, "Error: pa_alsa_path_select() failed with error %d\n", err); - goto cleanup; - } - printf("pa_alsa_path_select(): OK\n\n"); - - /* Verify switch states after activation */ - printf("Switch states AFTER pa_alsa_path_select():\n"); - bool all_correct = true; - bool expected_state = !config.device_muted; // unmuted device = switches ON - - PA_LLIST_FOREACH(e, path->elements) { - if (e->switch_use == PA_ALSA_SWITCH_MUTE) { - bool actual_state; - if (get_switch_state(mixer, e->alsa_id.name, e->alsa_id.index, &actual_state) == 0) { - printf(" %s,%d = %s", e->alsa_id.name, e->alsa_id.index, - actual_state ? "ON" : "OFF"); - - if (actual_state == expected_state) { - printf(" ✓\n"); - } else { - printf(" ✗ (expected %s)\n", expected_state ? "ON" : "OFF"); - all_correct = false; - } - } - } - } - - /* Determine test result */ - printf("\n"); - printf("========================================\n"); - if (all_correct) { - printf("TEST PASSED\n"); - printf("All mute switches correctly set to %s\n", expected_state ? "ON" : "OFF"); - if (!path->mute_during_activation) { - printf("\nThis proves the fix works:\n"); - printf("- Path has mute_during_activation=false (default)\n"); - printf("- Switches were still set correctly\n"); - printf("- Bug would have left them in kernel default state\n"); - } - ret = 0; - } else { - printf("TEST FAILED\n"); - printf("One or more switches NOT set correctly\n"); - printf("Expected: %s, but some switches were %s\n", - expected_state ? "ON (unmuted)" : "OFF (muted)", - expected_state ? "OFF" : "ON"); - printf("\nThis indicates the bug is still present:\n"); - printf("- Switches not set when mute_during_activation=false\n"); - printf("- IEC958 fix may not be applied\n"); - ret = 1; - } - printf("========================================\n"); - -cleanup: - if (mixer) - snd_mixer_close(mixer); - if (path) - pa_alsa_path_free(path); - - return ret; -} From b7a2fcf27e34dff2806521ffe7cff5d20b401450 Mon Sep 17 00:00:00 2001 From: Daniel Nouri Date: Mon, 6 Oct 2025 08:17:06 +0200 Subject: [PATCH 4/5] alsa: Enable IEC958 switches on device activation IEC958 (S/PDIF, HDMI, DisplayPort) switches default to muted in ALSA drivers, causing no audio output on digital devices. While UCM configurations and mixer paths can handle IEC958 unmuting, several scenarios lack coverage: - Pro-audio profiles (bypass UCM and mixer paths by design) - Devices without UCM configurations - Devices with incomplete mixer path definitions - Cards with multiple HDMI/DP outputs (indexed switches) This ensures IEC958 switches are enabled during device activation and port changes. The implementation uses the device mixer when available, falls back to the card mixer for pro-audio profiles, and enables all IEC958 switches regardless of index. Safe for all configurations: the operation is idempotent and provides defense-in-depth even when UCM or mixer paths handle it correctly. Tested on AMD Rembrandt GPU with 3 HDMI outputs in pro-audio mode. --- spa/plugins/alsa/acp/acp.c | 57 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/spa/plugins/alsa/acp/acp.c b/spa/plugins/alsa/acp/acp.c index 7fc749a7e..276b6c553 100644 --- a/spa/plugins/alsa/acp/acp.c +++ b/spa/plugins/alsa/acp/acp.c @@ -1615,6 +1615,59 @@ static int device_disable(pa_card *impl, pa_alsa_mapping *mapping, pa_alsa_devic return 0; } +/* Synchronize IEC958 digital output/input switch states. + * + * IEC958 switches default to muted in ALSA drivers. Cards with multiple + * HDMI/DP outputs have indexed switches (IEC958,0 IEC958,1 etc). We enable + * all switches since we cannot reliably map device numbers to indices. + */ +static void sync_iec958_controls(pa_alsa_device *d) +{ + snd_mixer_t *mixer_handle; + snd_mixer_elem_t *elem; + pa_card *impl; + int r; + + mixer_handle = d->mixer_handle; + + /* Pro-audio profiles don't have per-device mixers, use card mixer */ + if (!mixer_handle) { + impl = d->card; + if (!impl || impl->card.index == ACP_INVALID_INDEX) + return; + + mixer_handle = pa_alsa_open_mixer(impl->ucm.mixers, impl->card.index, true); + if (!mixer_handle) + return; + } + + /* Enable all IEC958 switches */ + for (elem = snd_mixer_first_elem(mixer_handle); elem; + elem = snd_mixer_elem_next(elem)) { + + if (snd_mixer_elem_get_type(elem) != SND_MIXER_ELEM_SIMPLE) + continue; + + const char *name = snd_mixer_selem_get_name(elem); + if (!name || !pa_startswith(name, "IEC958")) + continue; + + if (snd_mixer_selem_has_playback_switch(elem)) { + r = snd_mixer_selem_set_playback_switch_all(elem, 1); + if (r < 0) + pa_log_warn("Failed to enable IEC958 playback switch: %s", + pa_alsa_strerror(r)); + } + + if (snd_mixer_selem_has_capture_switch(elem)) { + r = snd_mixer_selem_set_capture_switch_all(elem, 1); + if (r < 0) + pa_log_warn("Failed to enable IEC958 capture switch: %s", + pa_alsa_strerror(r)); + } + } +} + static int device_enable(pa_card *impl, pa_alsa_mapping *mapping, pa_alsa_device *dev) { const char *mod_name; @@ -1681,6 +1734,9 @@ static int device_enable(pa_card *impl, pa_alsa_mapping *mapping, pa_alsa_device } } + /* Enable IEC958 switches for digital outputs */ + sync_iec958_controls(dev); + return 0; } @@ -2179,6 +2235,7 @@ int acp_device_set_port(struct acp_device *dev, uint32_t port_index, uint32_t fl pa_sink_suspend(s, false, PA_SUSPEND_UNAVAILABLE); #endif } + sync_iec958_controls(d); if (impl->events && impl->events->port_changed) impl->events->port_changed(impl->user_data, old ? old->port.index : 0, p->port.index); From 1cd056aa1eb3a773e430c64882b7b38007dcb5b8 Mon Sep 17 00:00:00 2001 From: Daniel Nouri Date: Mon, 6 Oct 2025 09:06:01 +0200 Subject: [PATCH 5/5] revert: Remove mixer path IEC958 fix Reverts alsa-mixer.c changes from 32a3ffc74. The comprehensive ACP layer fix (bdb82be4e) handles all scenarios including pro-audio profiles that bypass mixer paths, making this approach redundant. --- spa/plugins/alsa/acp/alsa-mixer.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/spa/plugins/alsa/acp/alsa-mixer.c b/spa/plugins/alsa/acp/alsa-mixer.c index 64088177f..1f494ee0b 100644 --- a/spa/plugins/alsa/acp/alsa-mixer.c +++ b/spa/plugins/alsa/acp/alsa-mixer.c @@ -1479,23 +1479,13 @@ int pa_alsa_path_select(pa_alsa_path *p, pa_alsa_setting *s, snd_mixer_t *m, boo if (s) setting_select(s, m); - /* Set hw mute switches to match device mute status. - * - * When mute_during_activation is enabled, switches are temporarily muted - * during path setup to avoid pops/clicks, and this restores them. - * - * When mute_during_activation is disabled (default), switches may still be - * in their kernel default state (often muted for digital outputs like IEC958). - * We must explicitly unmute them if the device isn't muted. - * - * This ensures HDMI/DisplayPort audio (IEC958) works correctly, as these - * digital output switches default to muted in ALSA drivers but are not - * automatically enabled when the path is selected. - */ - PA_LLIST_FOREACH(e, p->elements) { - if (e->switch_use == PA_ALSA_SWITCH_MUTE) { - if (element_set_switch(e, m, !device_is_muted) < 0) - return -1; + /* Finally restore hw mute to the device mute status. */ + if (p->mute_during_activation) { + PA_LLIST_FOREACH(e, p->elements) { + if (e->switch_use == PA_ALSA_SWITCH_MUTE) { + if (element_set_switch(e, m, !device_is_muted) < 0) + return -1; + } } }