All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Mark Brown <broonie@kernel.org>
Cc: Takashi Iwai <tiwai@suse.de>,
	"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	Takashi Iwai <tiwai@suse.com>
Subject: Re: DAPM PIN switches do not update in alsamixer when changed through UCM profile
Date: Thu, 7 Oct 2021 12:51:03 +0200	[thread overview]
Message-ID: <99e34b9e-f3ac-217f-80f2-81f54fc625ab@redhat.com> (raw)
In-Reply-To: <YVr6Xkm0tm0IghON@sirena.org.uk>

Hi,

On 10/4/21 2:58 PM, Mark Brown wrote:
> On Sun, Oct 03, 2021 at 06:32:13PM +0200, Hans de Goede wrote:
>> On 10/3/21 4:46 PM, Takashi Iwai wrote:
> 
>> But it does not work for the "Headphone" and "Line Out" switches,
>> these are actually hooked up to jack-detect, so I guess that the
>> jack-detection is already flipping them and then when the UCM
>> profile changes them it is a no-op causing the UCM setting of
>> the control to not cause an event because it is not a change.
> 
> It's not meaningful or sensible to have a pin switch and jack detection
> connected to the same pin, any machine driver doing that is buggy.  It's
> unclear how the two would be supposed to interact and there's nothing
> that makes an effort to keep them in sync.  Either jack detection should
> be disconnected from DAPM and userspace responsible for managing the
> paths via the pin switches or the pin switches should be removed.

Right, so the way this code evolved is that in the beginning there
was no jack-detection support and then there was a pins-witch for
each of the "Speaker" and "Headphone" outputs.

Later jack-detect was added later to the "Headphone" pin.

As you say this is not meaningful / a machine driver bug but 
unfortunately we cannot change this, the UCM profile for e.g.
the bytcr-rt5640 card contains:

                        EnableSequence [
                                cset "name='Headphone Switch' on"
                        ]

                        DisableSequence [
                                cset "name='Headphone Switch' off"
                        ]

And:

        Value {
                JackControl "Headphone Jack"
	}


And AFAIK there is no way to get the soc-jack code to still make userspace
see "Headphone Jack" events without adding a "Headphone" snd_soc_jack_pin.
Likewise UCM will error out if the 'Headphone Switch' control goes away.
So even though it is confusing for userspace at the same time we need to
keep both the SOC_DAPM_PIN_SWITCH("Headphone") kcontrol and the
"Headphone" snd_soc_jack_pin since userspace depends on them now.

This does explain why the 'Headphone Switch' kcontrol is not getting
change events when the output is changed based on a jack-detection event.

When the headphone is plugged in the Headphone pin gets enabled and
a "Headphone Jack" event is sent then e.g. pulseaudio will switch
the UCM output profile to the Headhpone output, doing:

                        EnableSequence [
                                cset "name='Headphone Switch' on"
                        ]

But this is a no-up since the soc-jack code has already enabled the pin.

With Takashi's "ASoC: DAPM: Fix missing kctl change notifications" we
will get a kcontrol change event for the 'Headphone Switch' however
when manually (1) switching between speakers/headphones.

TL;DR: you're right the missing kcontrol change events are caused
by the SOC_DAPM_PIN_SWITCH and the snd_soc_jack_pin both pointing to
the same pin. But we cannot fix this because userspace UCM profiles
depend on the pin name not changing in either case.

This is not a problem, the missing change events do not cause any actual
issues, its just something which I noticed when debugging/monitoring
UCM profile output switching with alsamixer. So IMHO the missing events
is just something which we will have to live with.

One leason to learn from this is to make sure to not have identical
named SOC_DAPM_PIN_SWITCH-es and snd_soc_jack_pin-s in new machine
drivers.

Regards,

Hans


1) When either a jack is inserted but the user wants the speaker output
anyways or on a board where jack-detect is not supported




      reply	other threads:[~2021-10-07 10:52 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-03 13:12 DAPM PIN switches do not update in alsamixer when changed through UCM profile Hans de Goede
2021-10-03 14:46 ` Takashi Iwai
2021-10-03 16:32   ` Hans de Goede
2021-10-04  6:43     ` Takashi Iwai
2021-10-04 12:58     ` Mark Brown
2021-10-07 10:51       ` Hans de Goede [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=99e34b9e-f3ac-217f-80f2-81f54fc625ab@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=tiwai@suse.com \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.