Skip to content

Commit

Permalink
ALSA: hda: Fix the control element identification for multiple codecs
Browse files Browse the repository at this point in the history
Some motherboards have multiple HDA codecs connected to the serial bus.
The current code may create multiple mixer controls with the almost
identical identification.

The current code use id.device field from the control element structure
to store the codec address to avoid such clashes for multiple codecs.
Unfortunately, the user space do not handle this correctly. For mixer
controls, only name and index are used for the identifiers.

This patch fixes this problem to compose the index using the codec
address as an offset in case, when the control already exists. It is
really unlikely that one codec will create 10 similar controls.

This patch adds new kernel module parameter 'ctl_dev_id' to allow
select the old behaviour, too. The CONFIG_SND_HDA_CTL_DEV_ID Kconfig
option sets the default value.

BugLink: alsa-project/alsa-lib#294
BugLink: alsa-project/alsa-lib#205
Fixes: 54d1740 ("[ALSA] hda-codec - Fix connection list parsing")
Fixes: 1afe206 ("ALSA: hda - Try to find an empty control index when it's occupied")
Signed-off-by: Jaroslav Kysela <perex@perex.cz>

--

rfc..v1:
 - added CONFIG_SND_HDA_CTL_DEV_ID Kconfig option
  • Loading branch information
perexg authored and intel-lab-lkp committed Jan 31, 2023
1 parent 372a0d7 commit 8dfc91e
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 3 deletions.
1 change: 1 addition & 0 deletions include/sound/hda_codec.h
Expand Up @@ -259,6 +259,7 @@ struct hda_codec {
unsigned int relaxed_resume:1; /* don't resume forcibly for jack */
unsigned int forced_resume:1; /* forced resume for jack */
unsigned int no_stream_clean_at_suspend:1; /* do not clean streams at suspend */
unsigned int ctl_dev_id:1; /* old control element id build behaviour */

#ifdef CONFIG_PM
unsigned long power_on_acct;
Expand Down
14 changes: 14 additions & 0 deletions sound/pci/hda/Kconfig
Expand Up @@ -302,6 +302,20 @@ config SND_HDA_INTEL_HDMI_SILENT_STREAM
This feature can impact power consumption as resources
are kept reserved both at transmitter and receiver.

config SND_HDA_CTL_DEV_ID
bool "Use the device identifier field for controls"
depends on SND_HDA_INTEL
help
Say Y to use the device identifier field for (mixer)
controls (old behaviour until this option is available).

When enabled, the multiple HDA codecs may set the device
field in control (mixer) element identifiers. The use
of this field is not recommended and defined for mixer controls.

The old behaviour (Y) is obsolete and will be removed. Consider
to not enable this option.

endif

endmenu
13 changes: 10 additions & 3 deletions sound/pci/hda/hda_codec.c
Expand Up @@ -3389,7 +3389,12 @@ int snd_hda_add_new_ctls(struct hda_codec *codec,
kctl = snd_ctl_new1(knew, codec);
if (!kctl)
return -ENOMEM;
if (addr > 0)
/* Do not use the id.device field for MIXER elements.
* This field is for real device numbers (like PCM) but codecs
* are hidden components from the user space view (unrelated
* to the mixer element identification).
*/
if (addr > 0 && codec->ctl_dev_id)
kctl->id.device = addr;
if (idx > 0)
kctl->id.index = idx;
Expand All @@ -3400,9 +3405,11 @@ int snd_hda_add_new_ctls(struct hda_codec *codec,
* the codec addr; if it still fails (or it's the
* primary codec), then try another control index
*/
if (!addr && codec->core.addr)
if (!addr && codec->core.addr) {
addr = codec->core.addr;
else if (!idx && !knew->index) {
if (!codec->ctl_dev_id)
idx += 10 * addr;
} else if (!idx && !knew->index) {
idx = find_empty_mixer_ctl_idx(codec,
knew->name, 0);
if (idx <= 0)
Expand Down
1 change: 1 addition & 0 deletions sound/pci/hda/hda_controller.c
Expand Up @@ -1231,6 +1231,7 @@ int azx_probe_codecs(struct azx *chip, unsigned int max_slots)
continue;
codec->jackpoll_interval = chip->jackpoll_interval;
codec->beep_mode = chip->beep_mode;
codec->ctl_dev_id = chip->ctl_dev_id;
codecs++;
}
}
Expand Down
1 change: 1 addition & 0 deletions sound/pci/hda/hda_controller.h
Expand Up @@ -124,6 +124,7 @@ struct azx {
/* HD codec */
int codec_probe_mask; /* copied from probe_mask option */
unsigned int beep_mode;
bool ctl_dev_id;

#ifdef CONFIG_SND_HDA_PATCH_LOADER
const struct firmware *fw;
Expand Down
6 changes: 6 additions & 0 deletions sound/pci/hda/hda_intel.c
Expand Up @@ -119,6 +119,8 @@ static bool beep_mode[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] =
CONFIG_SND_HDA_INPUT_BEEP_MODE};
#endif
static bool dmic_detect = 1;
static bool ctl_dev_id[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] =
CONFIG_SND_HDA_CTL_DEV_ID};

module_param_array(index, int, NULL, 0444);
MODULE_PARM_DESC(index, "Index value for Intel HD audio interface.");
Expand Down Expand Up @@ -157,6 +159,8 @@ module_param(dmic_detect, bool, 0444);
MODULE_PARM_DESC(dmic_detect, "Allow DSP driver selection (bypass this driver) "
"(0=off, 1=on) (default=1); "
"deprecated, use snd-intel-dspcfg.dsp_driver option instead");
module_param_array(ctl_dev_id, bool, NULL, 0444);
MODULE_PARM_DESC(ctl_dev_id, "Use control device identifier (based on codec address).");

#ifdef CONFIG_PM
static int param_set_xint(const char *val, const struct kernel_param *kp);
Expand Down Expand Up @@ -2278,6 +2282,8 @@ static int azx_probe_continue(struct azx *chip)
chip->beep_mode = beep_mode[dev];
#endif

chip->ctl_dev_id = ctl_dev_id[dev];

/* create codec instances */
if (bus->codec_mask) {
err = azx_probe_codecs(chip, azx_max_codecs[chip->driver_type]);
Expand Down

0 comments on commit 8dfc91e

Please sign in to comment.