Re: [PATCH] ALSA: hda: Notify IEC958 Default PCM switch state changes

[Takashi Iwai]

On Thu, 02 Apr 2026 02:20:21 +0200,
Cássio Gabriel wrote:
> 
> The "IEC958 Default PCM Playback Switch" control is backed directly by
> mout->share_spdif. The share-switch callbacks currently access that state
> without serialization, and spdif_share_sw_put() always returns 0, so
> normal userspace writes never emit the standard ALSA control value
> notification.
> 
> snd_hda_multi_out_analog_open() may also clear mout->share_spdif when the
> analog PCM capabilities and the SPDIF capabilities no longer intersect.
> That fallback is still needed to avoid creating an impossible hw
> constraint set, but it changes the mixer backing value without notifying
> subscribers.
> 
> Protect the share-switch callbacks with spdif_mutex like the other SPDIF
> control handlers, return the actual change value from spdif_share_sw_put(),
> and notify the matching control when the open path forcibly disables
> shared SPDIF mode after dropping spdif_mutex.
> 
> This keeps the existing auto-disable behavior while making switch state
> changes visible to userspace.
> 
> Fixes: 9a08160bdbe3 ("[ALSA] hda-codec - Add "IEC958 Default PCM" switch")
> Fixes: 022b466fc353 ("ALSA: hda - Avoid invalid formats and rates with shared SPDIF")
> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@xxxxxxxxx>
> ---
>  sound/hda/common/codec.c | 51 ++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 47 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/hda/common/codec.c b/sound/hda/common/codec.c
> index 09b1329bb8f3..5e74b02c45f6 100644
> --- a/sound/hda/common/codec.c
> +++ b/sound/hda/common/codec.c
> @@ -2529,7 +2529,10 @@ EXPORT_SYMBOL_GPL(snd_hda_spdif_ctls_assign);
>  static int spdif_share_sw_get(struct snd_kcontrol *kcontrol,
>  			      struct snd_ctl_elem_value *ucontrol)
>  {
> +	struct hda_codec *codec = (void *)kcontrol->private_value;
>  	struct hda_multi_out *mout = snd_kcontrol_chip(kcontrol);
> +
> +	guard(mutex)(&codec->spdif_mutex);
>  	ucontrol->value.integer.value[0] = mout->share_spdif;
>  	return 0;
>  }

I think it's better to assign codec to private_data like others, and
put mout to private_value instead.

> @@ -2550,6 +2559,33 @@ static const struct snd_kcontrol_new spdif_share_sw = {
>  	.put = spdif_share_sw_put,
>  };
>  
> +static struct snd_kcontrol *find_spdif_share_sw(struct hda_codec *codec,
> +						struct hda_multi_out *mout)
> +{
> +	struct hda_nid_item *items = codec->mixers.list;
> +	int i;
> +
> +	for (i = 0; i < codec->mixers.used; i++) {
> +		struct snd_kcontrol *kctl = items[i].kctl;
> +
> +		if (snd_kcontrol_chip(kctl) == mout &&
> +		    !strcmp(kctl->id.name, spdif_share_sw.name))
> +			return kctl;
> +	}
> +
> +	return NULL;
> +}
> +
> +static void notify_spdif_share_sw(struct hda_codec *codec,
> +				  struct hda_multi_out *mout)
> +{
> +	struct snd_kcontrol *kctl = find_spdif_share_sw(codec, mout);
> +
> +	if (kctl)
> +		snd_ctl_notify_one(codec->card, SNDRV_CTL_EVENT_MASK_VALUE,
> +				   kctl, 0);

We may extend struct hda_multi_out to store the assigned shared SPDIF
kcontrol pointer.  Then it's called directly without parsing at each
time.


thanks,

Takashi