Message-ID: <49739b30-bc48-4c4c-b1e1-f70fd9a65144@oss.qualcomm.com>
Date: Thu, 9 Jan 2025 17:32:18 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Stephan Gerhold <stephan.gerhold@...aro.org>,
        Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Bjorn Andersson <andersson@...nel.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, linux-arm-msm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Abel Vesa <abel.vesa@...aro.org>, Johan Hovold <johan@...nel.org>
Subject: Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC
 supplies

On 9.01.2025 4:45 PM, Stephan Gerhold wrote:
> On Thu, Jan 09, 2025 at 03:00:01PM +0100, Konrad Dybcio wrote:
>> 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.
>>
> 
> That's not entirely right. In our case here, the digital data from the
> DMIC goes directly to both the TX and VA macro. The power supply comes
> directly from the WCD983x codec. So the macro isn't involved in the bias
> voltage at all. Perhaps a picture will help:
> 
>                              +------+                         
>                         Data |      |  Power                  
>                           +--+ DMIC |<----------------+       
>                           |  |      |                 |       
>                           |  +------+                 |       
>     +---------------------+---+    +------------------+------+
>     | SoC  +----------+   |   |    | WCD983x       MIC_BIAS1 |
>     |      | TX Macro |<--+   |    |                  ^      |
>     |      +----------+   |   |    | +-----------+    |      |
>     |      +----------+   |   |    | | Regulator +----X--+   |
>     |      | VA Macro |<--+   |    | +-----------+       |   |
>     |      +----------+       |    |       ^          VDD_PX |
>     +-------------------------+    +-------+-----------------+
>                                            |             ^    
>                                            |             |    
> 
> X inside the WCD983x is where we can make the choice, if we want to use
> the internal regulator or output VDD_PX on MIC_BIAS1 directly. 
> 
> We can also choose to consume the microphone data either via the TX
> macro or the VA macro. IIRC there is no mux for this, the data just ends
> up in both at the same time.
> 
> Does that help explain it?

I think that's a "sadly, yes" ;)

Because that means we can switch the mics to e.g. the VA macro for
low power always-listening usecases at runtime (e.g. screen off), but we
may want to push it back to the RX macro for $reasons. And I'm assuming
there's probably $reasons2 to use the matching bias output from WCD..

Unless both $reasons are bogus, in which case we should probably stick
to keeping the bias and consuming macro paired to make the DT look sane

Konrad

Powered by blists - more mailing lists