public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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

  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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox