| Message ID | 20241203-x1e80100-va-mic-bias-v1-2-0dfd4d9b492c@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: qcom: Drop duplicate DMIC supplies for X13S/CRD | expand |
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. > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> > --- > arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 4 ---- > 1 file changed, 4 deletions(-) > Best regards, Krzysztof
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. Thanks, Stephan
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? Konrad
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? Thanks, Stephan
diff --git a/arch/arm64/boot/dts/qcom/x1e80100-crd.dts b/arch/arm64/boot/dts/qcom/x1e80100-crd.dts index 39f9d9cdc10d8e79824b72288e2529536144fa9e..a4d8da81bb4468e03ba8a9d37efaae10f7a6b53e 100644 --- a/arch/arm64/boot/dts/qcom/x1e80100-crd.dts +++ b/arch/arm64/boot/dts/qcom/x1e80100-crd.dts @@ -188,10 +188,6 @@ sound { "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", "TX SWR_INPUT1", "ADC2_OUTPUT"; wcd-playback-dai-link {
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") Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> --- arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 4 ---- 1 file changed, 4 deletions(-)