Message-ID: <Z+PTl4fg5tRoXmEE@opensource.cirrus.com>
Date: Wed, 26 Mar 2025 10:14:47 +0000
From: Charles Keepax <ckeepax@...nsource.cirrus.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.dev>
Cc: broonie@...nel.org, lgirdwood@...il.com, yung-chuan.liao@...ux.intel.com,
        peter.ujfalusi@...ux.intel.com, linux-sound@...r.kernel.org,
        linux-kernel@...r.kernel.org, patches@...nsource.cirrus.com
Subject: Re: [PATCH 1/3] ASoC: SDCA: Create DAPM widgets and routes from DisCo

On Tue, Mar 25, 2025 at 04:10:21PM -0500, Pierre-Louis Bossart wrote:
> On 3/25/25 06:19, Charles Keepax wrote:
> > On Mon, Mar 24, 2025 at 04:15:24PM -0500, Pierre-Louis Bossart wrote:
> In most cases the purpose of a GE is to control multiple SU
> states. However the SDCA spec took liberties with this concept and
> added new properties for the NDAI topologies, where a GE is present
> even if there is a single endpoint. It'd be worth double-checking
> that the way the GE is exposed in this patchset is forward-compatible
> with the 1.1 topologies.

I will go and double check but I can't see any reason why a GE
controlling only a single entity would be a problem here.

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

> >>> SDCA also has a slight habit of having fully connected paths, relying
> >>> more on activating the PDEs to enable functionality. This doesn't map
> >>> quite so perfectly to DAPM which considers the path a reason to power
> >>> the PDE. Whilst in the current specification Mixer Units are defined as
> >>> fixed-function, in DAPM we create a virtual control for each input. This
> >>> allows paths to be connected/disconnected, providing a more ASoC style
> >>> approach to managing the power.
> >>
> >> Humm, maybe my analysis was too naive but the SDCA PDE seemed
> >> like a DAPM power supply to me. When a path becomes active,
> >> DAPM turns on the power for you, and power is turned off some time
> >> after the path becomes inactive.
> > 
> > Correct, the PDEs are modeled as supply widgets and those are
> > powered up when the path is active as normal. The problem
> > alluded to in this paragraph is there a couple times where
> > SDCA topologies just have a permanently connected path so
> > things would always power up.
> 
> Ah yes those loops would indeed be problematic, but no more
> than in existing non-SDCA topologies where we used pin switches
> to disable such loops. All existing TDM-based solutions used
> pin switches, I was assuming we'd use them as well for SDCA.

I wanted to do a little more thinking on it. The general
concern I have is that typically the pin switches are added
along with the "fabric" widgets in the machine driver. As this
code here is effectively creating a single codec (function in
SDCA land), it feels like it is probably inappropriate to hook
up pin switchs at this level.

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.

> > My opinion is that even if we end up adding the pin switches as
> > well it still makes sense to allow connecting and disconnecting
> > the inputs of a Mixer Unit.  These are typically where two
> > audio streams come together and having the ability from the
> > host side to say if you want that connection or not seems very
> > valuable to me. As in SDCA land you basically make that choice by
> > directly flipping the PDE.
> 
> I have no objection if there are both pin switches and MU switches.
> 
> I view pin switches as a more generic mechanism that userpace
> has to set to use a specific endpoint.
> 
> The MU switches seem like debug capabilities to isolate which
> path has a problem. My experience fixing Baytrail issues is
> that you want a default mixer switch to be on, otherwise you'll
> get warnings on unconnected items or 'there is no sound' bug
> reports. In other words, the MU switches are a nice-to-have
> mechanism to disable default paths, so even if userspace
> doesn't touch those controls sound can be heard on endpoints.

The mixer switches do default to connected.

What I like about having mixer switches is it allows a
clearer specification of intent. For example one can power
the capture path and one can power the playback path, but the
mixer switch makes it clear user-space has an intention to
use the sidetone rather than just those two paths being
powered coincidentally.

I could drop them for now and just have the mixers permanently
connected, until we deal with pin switching, but I am inclined
to leave it as is and we can revisit later if necessary.

Thanks,
Charles

Powered by blists - more mailing lists