Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies

[Stephan Gerhold <stephan.gerhold@linaro.org>]

On Thu, Jan 09, 2025 at 01:57:17PM +0100, Konrad Dybcio wrote:
> On 9.01.2025 10:32 AM, Stephan Gerhold wrote:
> > On Wed, Jan 08, 2025 at 05:07:47PM -0600, Bjorn Andersson wrote:
> >> On Thu, Dec 05, 2024 at 07:46:28PM +0100, Stephan Gerhold wrote:
> >>> On Thu, Dec 05, 2024 at 06:11:47PM +0100, Konrad Dybcio wrote:
> >>>> On 4.12.2024 9:33 AM, Stephan Gerhold wrote:
> >>>>> On Wed, Dec 04, 2024 at 08:20:15AM +0100, Krzysztof Kozlowski wrote:
> >>>>>> On 03/12/2024 18:44, Stephan Gerhold wrote:
> >>>>>>> The WCD938x codec provides two controls for each of the MIC_BIASn outputs:
> >>>>>>>
> >>>>>>>  - "MIC BIASn" enables an internal regulator to generate the output
> >>>>>>>    with a configurable voltage (qcom,micbiasN-microvolt).
> >>>>>>>
> >>>>>>>  - "VA MIC BIASn" enables "pull-up mode" that bypasses the internal
> >>>>>>>    regulator and directly outputs fixed 1.8V from the VDD_PX pin.
> >>>>>>>    This is intended for low-power VA (voice activation) use cases.
> >>>>>>>
> >>>>>>> The audio-routing setup for the X1E80100 CRD currently specifies both
> >>>>>>> as power supplies for the DMICs, but only one of them can be active
> >>>>>>> at the same time. In practice, only the internal regulator is used
> >>>>>>> with the current setup because the driver prefers it over pull-up mode.
> >>>>>>>
> >>>>>>> Make this more clear by dropping the redundant routes to the pull-up
> >>>>>>> "VA MIC BIASn" supply. There is no functional difference except that we
> >>>>>>> skip briefly switching to pull-up mode when shutting down the microphone.
> >>>>>>>
> >>>>>>> Fixes: 4442a67eedc1 ("arm64: dts: qcom: x1e80100-crd: add sound card")
> >>>>>>
> >>>>>> If there is no functional difference and this is just redundant, then
> >>>>>> there is nothing to fix, so drop the tag. But the point is that users
> >>>>>> might want the low-power VA. You claim they don't want... sure, I am
> >>>>>> fine with that but there is nothing to fix in such case.
> >>>>>>
> >>>>>
> >>>>> The fix here is that two mutually exclusive power supplies for the DMIC
> >>>>> are specified in the device tree. You can only have one of them active
> >>>>> at the same time. The Linux driver handles that gracefully, but the
> >>>>> device tree is still wrong and IMO deserves a fixes tag.
> >>>>>
> >>>>> The functional difference is that we skip briefly switching to pull-up
> >>>>> mode when shutting down the microphone. Users won't notice that, but
> >>>>> it's not the intended behavior.
> >>>>>
> >>>>> I don't claim that users don't want to switch to the low-power pull-up
> >>>>> mode (VA MIC BIASn). However, we would need a different mechanism to
> >>>>> give them the option to switch at runtime. "audio-routing" just
> >>>>> specifies static routes, so the current description does not allow
> >>>>> switching between the two modes either.
> >>>>
> >>>> Is there no existing mechanism to alter this at runtime?
> >>>>
> >>>
> >>> I don't think so... Since it's currently exposed as two separate DAPM
> >>> supplies (instead of a mux or similar) you can only choose between one
> >>> of them in the static routes specified by "audio-routing" in the DT.
> >>>
> >>> I tried looking at how downstream handles this, but this left me even
> >>> more confused than I was before. :-) On CRD we currently have the
> >>> following routes in DT:
> >>>
> >>> 	"VA DMIC0", "MIC BIAS3",
> >>> 	"VA DMIC1", "MIC BIAS3",
> >>> 	"VA DMIC2", "MIC BIAS1",
> >>> 	"VA DMIC3", "MIC BIAS1",
> >>> 	"VA DMIC0", "VA MIC BIAS3",
> >>> 	"VA DMIC1", "VA MIC BIAS3",
> >>> 	"VA DMIC2", "VA MIC BIAS1",
> >>> 	"VA DMIC3", "VA MIC BIAS1",
> >>>
> >>> MIC BIAS and VA MIC BIAS are mutually exclusive, so this is not correct.
> >>> But if you look at e.g. SM8550 downstream they have:
> >>>
> >>> 	"TX DMIC0", "MIC BIAS3",
> >>> 	"TX DMIC1", "MIC BIAS3",
> >>> 	"TX DMIC2", "MIC BIAS1",
> >>> 	"TX DMIC3", "MIC BIAS1",
> >>> 	"VA DMIC0", "VA MIC BIAS3",
> >>> 	"VA DMIC1", "VA MIC BIAS3",
> >>> 	"VA DMIC2", "VA MIC BIAS1",
> >>> 	"VA DMIC3", "VA MIC BIAS1";
> >>>
> >>> Note the TX DMIC vs VA DMIC. So they specify one of the supplies for the
> >>> TX macro DMIC, and the low-power one for the VA macro DMIC. That would
> >>> be fine.
> >>>
> >>> Now the question is: If we can use the DMIC through both the TX and the
> >>> VA macro, and we're not doing voice activation, why are we using the VA
> >>> macro in the first place?
> >>>
> >>> @Srini: Do you remember why?
> >>>
> >>
> >> What's the verdict regarding this?
> >>
> > 
> > We started discussing this, but did not come to a conclusion yet if we
> > should be recording from the DMICs using the TX macro instead of the VA
> > macro.
> > 
> > The patch I submitted is still valid though, independent of that
> > question. Since we're not doing voice activation we want to have the
> > "full quality" MIC BIAS supply, not the low-power one.
> 
> Can/should we discuss a new sound API to make this toggleable?
> 
> Do these microphones physically connect to muxable inputs, or does this
> depend on board wiring?
> 

The WCD938x codec has 4 MIC_BIAS output pins that are typically used as
power supply for microphones. Inside the codec there is an option to
drive these output pins in one of two modes:

 1. Internal regulator to generate the output with a configurable
    voltage (qcom,micbiasN-microvolt). Exposed as "MIC BIASn" supply in
    the Linux driver.

 2. "Pull-up mode" that bypasses the internal regulator and directly
    outputs fixed 1.8V from the VDD_PX pin. Exposed as "VA MIC BIASn"
    supply in the Linux driver.

The board-specific part here is only which microphone is wired to which
MIC BIAS pin (e.g. DMIC0 -> MIC BIAS3, DMIC2 -> MIC BIAS1 etc). 

Both options will work if the microphone can operate at 1.8V. In that
case, I think generally we want (1) for normal audio use cases and (2)
for low-power use cases (like "voice activation").

Apparently the same applies for the "macro" to use. TX macro should be
used for normal audio, and VA macro only for low-power use cases. With
that there is a clear mapping:

 - TX macro DMICs -> full power "MIC BIAS" supply
 - VA macro DMICs -> low-power "VA MIC BIAS" supply

I don't see why someone would want to change this mapping, so I don't
think it's worth making this user configurable.

Given that we're currently using the VA macro for normal audio, we
should describe VA macro DMICs -> full power "MIC BIAS" supply for now
and ideally migrate to using the TX macro later.

Thanks,
Stephan