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

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