Message-ID: <77b8a156-49af-4900-b17a-b2b3fd11eba0@linux.dev>
Date: Mon, 14 Apr 2025 14:43:50 -0500
From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.dev>
To: Charles Keepax <ckeepax@...nsource.cirrus.com>
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

Delayed answer, sorry...

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

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

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

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

I bet we'll revisit this multiple times :-)

Powered by blists - more mailing lists