Message-ID: <0f9e456b-cd54-4496-a2d2-795aae744385@oss.qualcomm.com>
Date: Thu, 9 Jan 2025 13:57:17 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Stephan Gerhold <stephan.gerhold@...aro.org>,
        Bjorn Andersson <andersson@...nel.org>
Cc: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        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 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?

Konrad

> 
> It looks like there have been new users of this pattern added upstream.
> I'll resend this patch with all the new occurrences and will try further
> clarifying the commit messge while at it.
> 
> Thanks,
> Stephan

Powered by blists - more mailing lists