* [PATCH 0/2] arm64: dts: qcom: Drop duplicate DMIC supplies for X13S/CRD @ 2024-12-03 17:44 Stephan Gerhold 2024-12-03 17:44 ` [PATCH 1/2] arm64: dts: qcom: sc8280xp-x13s: Drop duplicate DMIC supplies Stephan Gerhold ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Stephan Gerhold @ 2024-12-03 17:44 UTC (permalink / raw) To: Bjorn Andersson, Konrad Dybcio Cc: Rob Herring, Conor Dooley, Srinivas Kandagatla, Krzysztof Kozlowski, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold Drop the duplicated MIC BIAS supplies in "audio-routing" for the ThinkPad X13S and the X1E80100. "MIC BIASn" and "VA MIC BIASn" are mutually exclusive, only one of them can be active at the same time. Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> --- Stephan Gerhold (2): arm64: dts: qcom: sc8280xp-x13s: Drop duplicate DMIC supplies arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 3 --- arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 4 ---- 2 files changed, 7 deletions(-) --- base-commit: 12b080aaf4275c579c91106ed926044b4d5df0af change-id: 20241203-x1e80100-va-mic-bias-bab3bd5a7c57 Best regards, -- Stephan Gerhold <stephan.gerhold@linaro.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] arm64: dts: qcom: sc8280xp-x13s: Drop duplicate DMIC supplies 2024-12-03 17:44 [PATCH 0/2] arm64: dts: qcom: Drop duplicate DMIC supplies for X13S/CRD Stephan Gerhold @ 2024-12-03 17:44 ` Stephan Gerhold 2024-12-03 17:44 ` [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: " Stephan Gerhold 2025-03-14 20:01 ` [PATCH 0/2] arm64: dts: qcom: Drop duplicate DMIC supplies for X13S/CRD Bjorn Andersson 2 siblings, 0 replies; 16+ messages in thread From: Stephan Gerhold @ 2024-12-03 17:44 UTC (permalink / raw) To: Bjorn Andersson, Konrad Dybcio Cc: Rob Herring, Conor Dooley, Srinivas Kandagatla, Krzysztof Kozlowski, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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 ThinkPad X13s 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: 2e498f35c385 ("arm64: dts: qcom: sc8280xp-x13s: fix va dmic dai links and routing") Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> --- arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 3 --- 1 file changed, 3 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts index f3190f408f4b2c7ca9fe368d652d5d66bb3949af..0f1ebd869ce315bfed2c17ae90b2451b3892b96b 100644 --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts @@ -1202,9 +1202,6 @@ &sound { "VA DMIC0", "MIC BIAS1", "VA DMIC1", "MIC BIAS1", "VA DMIC2", "MIC BIAS3", - "VA DMIC0", "VA MIC BIAS1", - "VA DMIC1", "VA MIC BIAS1", - "VA DMIC2", "VA MIC BIAS3", "TX SWR_ADC1", "ADC2_OUTPUT"; wcd-playback-dai-link { -- 2.47.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies 2024-12-03 17:44 [PATCH 0/2] arm64: dts: qcom: Drop duplicate DMIC supplies for X13S/CRD Stephan Gerhold 2024-12-03 17:44 ` [PATCH 1/2] arm64: dts: qcom: sc8280xp-x13s: Drop duplicate DMIC supplies Stephan Gerhold @ 2024-12-03 17:44 ` Stephan Gerhold 2024-12-04 7:20 ` Krzysztof Kozlowski 2025-03-14 20:01 ` [PATCH 0/2] arm64: dts: qcom: Drop duplicate DMIC supplies for X13S/CRD Bjorn Andersson 2 siblings, 1 reply; 16+ messages in thread From: Stephan Gerhold @ 2024-12-03 17:44 UTC (permalink / raw) To: Bjorn Andersson, Konrad Dybcio Cc: Rob Herring, Conor Dooley, Srinivas Kandagatla, Krzysztof Kozlowski, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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(-) 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 { -- 2.47.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies 2024-12-03 17:44 ` [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: " Stephan Gerhold @ 2024-12-04 7:20 ` Krzysztof Kozlowski 2024-12-04 8:33 ` Stephan Gerhold 0 siblings, 1 reply; 16+ messages in thread From: Krzysztof Kozlowski @ 2024-12-04 7:20 UTC (permalink / raw) To: Stephan Gerhold, Bjorn Andersson, Konrad Dybcio Cc: Rob Herring, Conor Dooley, Srinivas Kandagatla, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies 2024-12-04 7:20 ` Krzysztof Kozlowski @ 2024-12-04 8:33 ` Stephan Gerhold 2024-12-05 17:11 ` Konrad Dybcio 0 siblings, 1 reply; 16+ messages in thread From: Stephan Gerhold @ 2024-12-04 8:33 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Conor Dooley, Srinivas Kandagatla, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies 2024-12-04 8:33 ` Stephan Gerhold @ 2024-12-05 17:11 ` Konrad Dybcio 2024-12-05 18:46 ` Stephan Gerhold 0 siblings, 1 reply; 16+ messages in thread From: Konrad Dybcio @ 2024-12-05 17:11 UTC (permalink / raw) To: Stephan Gerhold, Krzysztof Kozlowski Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Conor Dooley, Srinivas Kandagatla, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies 2024-12-05 17:11 ` Konrad Dybcio @ 2024-12-05 18:46 ` Stephan Gerhold 2025-01-08 23:07 ` Bjorn Andersson 0 siblings, 1 reply; 16+ messages in thread From: Stephan Gerhold @ 2024-12-05 18:46 UTC (permalink / raw) To: Konrad Dybcio, Srinivas Kandagatla Cc: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio, Rob Herring, Conor Dooley, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies 2024-12-05 18:46 ` Stephan Gerhold @ 2025-01-08 23:07 ` Bjorn Andersson 2025-01-09 9:32 ` Stephan Gerhold 0 siblings, 1 reply; 16+ messages in thread From: Bjorn Andersson @ 2025-01-08 23:07 UTC (permalink / raw) To: Stephan Gerhold Cc: Konrad Dybcio, Srinivas Kandagatla, Krzysztof Kozlowski, Konrad Dybcio, Rob Herring, Conor Dooley, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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? Regards, Bjorn ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies 2025-01-08 23:07 ` Bjorn Andersson @ 2025-01-09 9:32 ` Stephan Gerhold 2025-01-09 12:57 ` Konrad Dybcio 0 siblings, 1 reply; 16+ messages in thread From: Stephan Gerhold @ 2025-01-09 9:32 UTC (permalink / raw) To: Bjorn Andersson Cc: Konrad Dybcio, Srinivas Kandagatla, Krzysztof Kozlowski, Konrad Dybcio, Rob Herring, Conor Dooley, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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. 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies 2025-01-09 9:32 ` Stephan Gerhold @ 2025-01-09 12:57 ` Konrad Dybcio 2025-01-09 13:29 ` Stephan Gerhold 0 siblings, 1 reply; 16+ messages in thread From: Konrad Dybcio @ 2025-01-09 12:57 UTC (permalink / raw) To: Stephan Gerhold, Bjorn Andersson Cc: Konrad Dybcio, Srinivas Kandagatla, Krzysztof Kozlowski, Konrad Dybcio, Rob Herring, Conor Dooley, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies 2025-01-09 12:57 ` Konrad Dybcio @ 2025-01-09 13:29 ` Stephan Gerhold 2025-01-09 14:00 ` Konrad Dybcio 0 siblings, 1 reply; 16+ messages in thread From: Stephan Gerhold @ 2025-01-09 13:29 UTC (permalink / raw) To: Konrad Dybcio Cc: Bjorn Andersson, Srinivas Kandagatla, Krzysztof Kozlowski, Konrad Dybcio, Rob Herring, Conor Dooley, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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. Thanks, Stephan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies 2025-01-09 13:29 ` Stephan Gerhold @ 2025-01-09 14:00 ` Konrad Dybcio 2025-01-09 15:45 ` Stephan Gerhold 0 siblings, 1 reply; 16+ messages in thread From: Konrad Dybcio @ 2025-01-09 14:00 UTC (permalink / raw) To: Stephan Gerhold, Konrad Dybcio Cc: Bjorn Andersson, Srinivas Kandagatla, Krzysztof Kozlowski, Konrad Dybcio, Rob Herring, Conor Dooley, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies 2025-01-09 14:00 ` Konrad Dybcio @ 2025-01-09 15:45 ` Stephan Gerhold 2025-01-09 16:32 ` Konrad Dybcio 0 siblings, 1 reply; 16+ messages in thread From: Stephan Gerhold @ 2025-01-09 15:45 UTC (permalink / raw) To: Konrad Dybcio Cc: Bjorn Andersson, Srinivas Kandagatla, Krzysztof Kozlowski, Konrad Dybcio, Rob Herring, Conor Dooley, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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? Thanks, Stephan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies 2025-01-09 15:45 ` Stephan Gerhold @ 2025-01-09 16:32 ` Konrad Dybcio 2025-01-09 16:46 ` Stephan Gerhold 0 siblings, 1 reply; 16+ messages in thread From: Konrad Dybcio @ 2025-01-09 16:32 UTC (permalink / raw) To: Stephan Gerhold, Konrad Dybcio Cc: Bjorn Andersson, Srinivas Kandagatla, Krzysztof Kozlowski, Konrad Dybcio, Rob Herring, Conor Dooley, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies 2025-01-09 16:32 ` Konrad Dybcio @ 2025-01-09 16:46 ` Stephan Gerhold 0 siblings, 0 replies; 16+ messages in thread From: Stephan Gerhold @ 2025-01-09 16:46 UTC (permalink / raw) To: Konrad Dybcio Cc: Bjorn Andersson, Srinivas Kandagatla, Krzysztof Kozlowski, Konrad Dybcio, Rob Herring, Conor Dooley, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] arm64: dts: qcom: Drop duplicate DMIC supplies for X13S/CRD 2024-12-03 17:44 [PATCH 0/2] arm64: dts: qcom: Drop duplicate DMIC supplies for X13S/CRD Stephan Gerhold 2024-12-03 17:44 ` [PATCH 1/2] arm64: dts: qcom: sc8280xp-x13s: Drop duplicate DMIC supplies Stephan Gerhold 2024-12-03 17:44 ` [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: " Stephan Gerhold @ 2025-03-14 20:01 ` Bjorn Andersson 2 siblings, 0 replies; 16+ messages in thread From: Bjorn Andersson @ 2025-03-14 20:01 UTC (permalink / raw) To: Konrad Dybcio, Stephan Gerhold Cc: Rob Herring, Conor Dooley, Srinivas Kandagatla, Krzysztof Kozlowski, linux-arm-msm, devicetree, linux-kernel, Abel Vesa, Johan Hovold On Tue, 03 Dec 2024 18:44:01 +0100, Stephan Gerhold wrote: > Drop the duplicated MIC BIAS supplies in "audio-routing" for the ThinkPad > X13S and the X1E80100. "MIC BIASn" and "VA MIC BIASn" are mutually > exclusive, only one of them can be active at the same time. > > Applied, thanks! [1/2] arm64: dts: qcom: sc8280xp-x13s: Drop duplicate DMIC supplies commit: a2e617f4e6981aa514a569e927f90b0d39bb31b2 [2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies commit: b49e37de8e70bc433b526a9f4382f72b7ac6492e Best regards, -- Bjorn Andersson <andersson@kernel.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-03-14 20:02 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-12-03 17:44 [PATCH 0/2] arm64: dts: qcom: Drop duplicate DMIC supplies for X13S/CRD Stephan Gerhold 2024-12-03 17:44 ` [PATCH 1/2] arm64: dts: qcom: sc8280xp-x13s: Drop duplicate DMIC supplies Stephan Gerhold 2024-12-03 17:44 ` [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: " Stephan Gerhold 2024-12-04 7:20 ` Krzysztof Kozlowski 2024-12-04 8:33 ` Stephan Gerhold 2024-12-05 17:11 ` Konrad Dybcio 2024-12-05 18:46 ` Stephan Gerhold 2025-01-08 23:07 ` Bjorn Andersson 2025-01-09 9:32 ` Stephan Gerhold 2025-01-09 12:57 ` Konrad Dybcio 2025-01-09 13:29 ` Stephan Gerhold 2025-01-09 14:00 ` Konrad Dybcio 2025-01-09 15:45 ` Stephan Gerhold 2025-01-09 16:32 ` Konrad Dybcio 2025-01-09 16:46 ` Stephan Gerhold 2025-03-14 20:01 ` [PATCH 0/2] arm64: dts: qcom: Drop duplicate DMIC supplies for X13S/CRD Bjorn Andersson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).