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

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

[Cássio Gabriel]

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@gmail.com>
---
 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;
 }
@@ -2537,9 +2540,15 @@ static int spdif_share_sw_get(struct snd_kcontrol *kcontrol,
 static int spdif_share_sw_put(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);
-	mout->share_spdif = !!ucontrol->value.integer.value[0];
-	return 0;
+	bool val = !!ucontrol->value.integer.value[0];
+	int change;
+
+	guard(mutex)(&codec->spdif_mutex);
+	change = mout->share_spdif != val;
+	mout->share_spdif = val;
+	return change;
 }
 
 static const struct snd_kcontrol_new spdif_share_sw = {
@@ -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);
+}
+
 /**
  * snd_hda_create_spdif_share_sw - create Default PCM switch
  * @codec: the HDA codec
@@ -2566,7 +2602,10 @@ int snd_hda_create_spdif_share_sw(struct hda_codec *codec,
 	kctl = snd_ctl_new1(&spdif_share_sw, mout);
 	if (!kctl)
 		return -ENOMEM;
-	/* ATTENTION: here mout is passed as private_data, instead of codec */
+	/* snd_ctl_new1() stores @mout in private_data; stash @codec in
+	 * private_value for the share-switch callbacks.
+	 */
+	kctl->private_value = (unsigned long)codec;
 	return snd_hda_ctl_add(codec, mout->dig_out_nid, kctl);
 }
 EXPORT_SYMBOL_GPL(snd_hda_create_spdif_share_sw);
@@ -3701,6 +3740,8 @@ int snd_hda_multi_out_analog_open(struct hda_codec *codec,
 				  struct hda_pcm_stream *hinfo)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
+	bool notify_share_sw = false;
+
 	runtime->hw.channels_max = mout->max_channels;
 	if (mout->dig_out_nid) {
 		if (!mout->analog_rates) {
@@ -3729,10 +3770,12 @@ int snd_hda_multi_out_analog_open(struct hda_codec *codec,
 					hinfo->maxbps = mout->spdif_maxbps;
 			} else {
 				mout->share_spdif = 0;
-				/* FIXME: need notify? */
+				notify_share_sw = true;
 			}
 		}
 	}
+	if (notify_share_sw)
+		notify_spdif_share_sw(codec, mout);
 	return snd_pcm_hw_constraint_step(substream->runtime, 0,
 					  SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }

---
base-commit: 85d2c76df1f21fdd599cf2dbee29d59da47f8fa4
change-id: 20260330-hda-spdif-share-notify-22fa4ac00a14

Best regards,
--  
Cássio Gabriel <cassiogabrielcontato@gmail.com>

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@gmail.com>
> ---
>  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

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

[Cássio Gabriel Monteiro Pires]

On 4/2/26 08:01, Takashi Iwai wrote:
> 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@gmail.com>
>> ---
>>  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.

Agreed, I'll rework it that way for v2.

> 
>> @@ -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.

I’ll switch the callbacks to private_data/private_value and
use a cached shared SPDIF kctl, while keeping teardown safe.

> 
> 
> thanks,
> 
> Takashi

-- 
Thanks,
Cássio