| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Z3_9bZDi4Bt47P7s@linaro.org>
Date: Thu, 9 Jan 2025 17:46:37 +0100
From: Stephan Gerhold <stephan.gerhold@...aro.org>
To: 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 Thu, Jan 09, 2025 at 05:32:18PM +0100, Konrad Dybcio wrote:
> 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
I think both $reasons boil down to saving power. If you do
always-listening use cases you likely care more about power than perfect
quality or functionality. So you either pick
- Quality + functionality => Internal MIC bias regulator + TX macro, or
- Low-power => Pull-up MIC BIAS + VA macro
_Maybe_ someone wants one of the other combinations, but I don't think
those are useful enough to warrant refactoring of the driver and
potentially breaking DTB compatibility.
Thanks,
Stephan
Powered by blists - more mailing lists