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

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 9.01.2025 2:29 PM, Stephan Gerhold wrote:
> 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.

So, in short, if I understood you correctly, audio comes in through a
hardwired connection to a given macro, but the bias pins can be configured
to output the bias voltage through any of the macros.

In that case, I agree only keeping the one matching the macro the mics
are connected to is the solution here

Konrad