From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
Cc: broonie@kernel.org, lgirdwood@gmail.com,
yung-chuan.liao@linux.intel.com, peter.ujfalusi@linux.intel.com,
linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
patches@opensource.cirrus.com
Subject: Re: [PATCH 1/3] ASoC: SDCA: Create DAPM widgets and routes from DisCo
Date: Wed, 16 Apr 2025 10:41:31 +0100 [thread overview]
Message-ID: <Z/97SwfaLp2VIzf4@opensource.cirrus.com> (raw)
In-Reply-To: <77b8a156-49af-4900-b17a-b2b3fd11eba0@linux.dev>
On Mon, Apr 14, 2025 at 02:43:50PM -0500, Pierre-Louis Bossart wrote:
> >> How would the state of those DAPM SU widgets be updated
> >> though? I think we need to 'translate' the GE settings to tell
> >> DAPM which paths can become active, but the SUs state is set
> >> by hardware so I could see a possible racy disconnect if we
> >> make a path activable but hardware hasn't done so yet.
> >
> > All the SU DAPM widgets are linked to the single GE control,
> > the same ALSA control is shared across all the widgets. So
> > all the paths are updated in a single DAPM sync, and on the
> > hardware side with a single write to the GE control.
>
> The race I am concerned about is between SU values
> represented in DAPM and actual values propagated inside the
> SDCA device. There could be a delay between writing a GE
> register and the SU register values changing.
Fair enough, but I don't see why this matters. Those SU registers
are device level, it is the devices business to manage them. Why
does it matter if there is a delay updating them? All the widgets
on the host side are controlled by the GE control.
> > To put that as a more concrete example this code will create
> > input widgets for IT 31, 32, 33 (the UAJ mics), however it
> > would be unusual to hook a pin switch to those. Something
> > should be creating an actual microphone widget, attaching
> > that to the input widgets and attaching a pin switch to that.
> > Typically those actions are handled in the machine driver,
> > there is possibly an argument for handling them in the codec
> > driver for SDCA but I felt it would make more sense to progress
> > things a little further until resolving that one.
>
> The SDCA spec is supposed to describe what's physically
> connected, so when we parse the DisCo descriptors we should
> only see the level that is typically present in machine
> drivers.
That's not entirely true, none of the interconnections between
codecs are contained. Microsoft invented that composition stuff
to hold that sort of stuff. Although perhaps there is a strong
case that the IT/OTs here are defined as having a mic/speaker
connected so it is the right place to define them?
> The codec descriptors are not generic at all, they
> should only describe a specific way of how a SDCA codec is
> used. The traditional split between codec and machine drivers
> does not really apply for SDCA, the SDCA descriptors represent
> the *machine* already.
I guess that makes your opinion very much that we do add the
mic/speaker widgets at this level. I mean it wouldn't take long
to add them. I am still not totally as sure, but we can probably
live with a little more effort to back it out if we need it. I
will do a v4 and see what adding those in looks like.
Thanks,
Charles
next prev parent reply other threads:[~2025-04-16 9:41 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-21 16:39 [PATCH 0/3] Add DAPM/ASoC helpers to create SDCA drivers Charles Keepax
2025-03-21 16:39 ` [PATCH 1/3] ASoC: SDCA: Create DAPM widgets and routes from DisCo Charles Keepax
2025-03-24 21:15 ` Pierre-Louis Bossart
2025-03-25 11:19 ` Charles Keepax
2025-03-25 21:10 ` Pierre-Louis Bossart
2025-03-26 10:14 ` Charles Keepax
2025-04-14 19:43 ` Pierre-Louis Bossart
2025-04-16 9:41 ` Charles Keepax [this message]
2025-03-25 16:27 ` Charles Keepax
2025-03-21 16:39 ` [PATCH 2/3] ASoC: SDCA: Create ALSA controls " Charles Keepax
2025-03-21 16:39 ` [PATCH 3/3] ASoC: SDCA: Create DAI drivers " Charles Keepax
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=Z/97SwfaLp2VIzf4@opensource.cirrus.com \
--to=ckeepax@opensource.cirrus.com \
--cc=broonie@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=patches@opensource.cirrus.com \
--cc=peter.ujfalusi@linux.intel.com \
--cc=pierre-louis.bossart@linux.dev \
--cc=yung-chuan.liao@linux.intel.com \
/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.