2023-09-26 18:27:36

by Stephan Gerhold

[permalink] [raw]
Subject: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem

Enable sound and modem for the Samsung A2015 based devices (A3, A5, E5,
E7, Grand Max). The setup is similar to most MSM8916 devices, i.e.:

- QDSP6 audio
- Earpiece/headphones/microphones via digital/analog codec in
MSM8916/PM8916
- WWAN Internet via BAM-DMUX

except:

- NXP TFA9895 codec for speaker on Quaternary MI2S
- Samsung-specific audio jack detection (not supported yet)

[Lin: Add e2015 and grandmax]
Co-developed-by: "Lin, Meng-Bo" <[email protected]>
Signed-off-by: "Lin, Meng-Bo" <[email protected]>
Signed-off-by: Stephan Gerhold <[email protected]>
---
.../dts/qcom/msm8916-samsung-a2015-common.dtsi | 55 ++++++++++++++++++++++
.../dts/qcom/msm8916-samsung-e2015-common.dtsi | 4 ++
.../boot/dts/qcom/msm8916-samsung-grandmax.dts | 4 ++
3 files changed, 63 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
index 0b29132b74e1..f71b18d89bf9 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
@@ -1,10 +1,13 @@
// SPDX-License-Identifier: GPL-2.0-only

#include "msm8916-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
+
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/input/input.h>
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
+#include <dt-bindings/sound/apq8016-lpass.h>

/ {
aliases {
@@ -196,6 +199,18 @@ vibrator: vibrator {
};
};

+&blsp_i2c1 {
+ status = "okay";
+
+ speaker_codec: audio-codec@34 {
+ compatible = "nxp,tfa9895";
+ reg = <0x34>;
+ vddd-supply = <&pm8916_l5>;
+ sound-name-prefix = "Speaker";
+ #sound-dai-cells = <0>;
+ };
+};
+
&blsp_i2c2 {
status = "okay";

@@ -243,6 +258,13 @@ &gpu {
status = "okay";
};

+&lpass {
+ dai-link@3 {
+ reg = <MI2S_QUATERNARY>;
+ qcom,playback-sd-lines = <1>;
+ };
+};
+
&mdss {
status = "okay";
};
@@ -253,6 +275,10 @@ &mdss_dsi0 {
pinctrl-1 = <&mdss_sleep>;
};

+&mpss_mem {
+ reg = <0x0 0x86800000 0x0 0x5400000>;
+};
+
&pm8916_resin {
status = "okay";
linux,code = <KEY_VOLUMEDOWN>;
@@ -265,6 +291,13 @@ pm8916_l17: l17 {
};
};

+&q6afedai {
+ dai@22 {
+ reg = <QUATERNARY_MI2S_RX>;
+ qcom,sd-lines = <1>;
+ };
+};
+
&sdhc_1 {
status = "okay";
};
@@ -279,6 +312,28 @@ &sdhc_2 {
cd-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
};

+&sound {
+ status = "okay";
+
+ model = "samsung-a2015";
+ audio-routing =
+ "AMIC1", "MIC BIAS External1",
+ "AMIC2", "MIC BIAS Internal2",
+ "AMIC3", "MIC BIAS External1";
+
+ pinctrl-0 = <&cdc_pdm_default &sec_mi2s_default>;
+ pinctrl-1 = <&cdc_pdm_sleep &sec_mi2s_sleep>;
+ pinctrl-names = "default", "sleep";
+};
+
+&sound_dai_quaternary {
+ status = "okay";
+
+ codec {
+ sound-dai = <&speaker_codec>;
+ };
+};
+
&usb {
status = "okay";
extcon = <&muic>, <&muic>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-e2015-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-e2015-common.dtsi
index 0824ab041d80..3c49dac92d2d 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-e2015-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-e2015-common.dtsi
@@ -65,6 +65,10 @@ accelerometer@1d {
};
};

+&mpss_mem {
+ reg = <0x0 0x86800000 0x0 0x5a00000>;
+};
+
&reg_motor_vdd {
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3300000>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-grandmax.dts b/arch/arm64/boot/dts/qcom/msm8916-samsung-grandmax.dts
index 3f145dde4059..5882b3a593b8 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-grandmax.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-grandmax.dts
@@ -49,6 +49,10 @@ &reg_touch_key {
status = "disabled";
};

+&sound {
+ model = "samsung-gmax"; /* No secondary microphone */
+};
+
&tlmm {
gpio_leds_default: gpio-led-default-state {
pins = "gpio60";

--
2.42.0


2023-09-26 21:21:20

by Stephan Gerhold

[permalink] [raw]
Subject: Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem

On Tue, Sep 26, 2023 at 08:54:29PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 18:51, Stephan Gerhold wrote:
> > Enable sound and modem for the Samsung A2015 based devices (A3, A5, E5,
> > E7, Grand Max). The setup is similar to most MSM8916 devices, i.e.:
> >
> > - QDSP6 audio
> > - Earpiece/headphones/microphones via digital/analog codec in
> > MSM8916/PM8916
> > - WWAN Internet via BAM-DMUX
> >
> > except:
> >
> > - NXP TFA9895 codec for speaker on Quaternary MI2S
> > - Samsung-specific audio jack detection (not supported yet)
> >
> > [Lin: Add e2015 and grandmax]
> > Co-developed-by: "Lin, Meng-Bo" <[email protected]>
> > Signed-off-by: "Lin, Meng-Bo" <[email protected]>
> > Signed-off-by: Stephan Gerhold <[email protected]>
> > ---
> > .../dts/qcom/msm8916-samsung-a2015-common.dtsi | 55 ++++++++++++++++++++++
> > .../dts/qcom/msm8916-samsung-e2015-common.dtsi | 4 ++
> > .../boot/dts/qcom/msm8916-samsung-grandmax.dts | 4 ++
> > 3 files changed, 63 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> > index 0b29132b74e1..f71b18d89bf9 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> > @@ -1,10 +1,13 @@
> > // SPDX-License-Identifier: GPL-2.0-only
> >
> > #include "msm8916-pm8916.dtsi"
> > +#include "msm8916-modem-qdsp6.dtsi"
> > +
> > #include <dt-bindings/gpio/gpio.h>
> > #include <dt-bindings/input/input.h>
> > #include <dt-bindings/interrupt-controller/irq.h>
> > #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> > +#include <dt-bindings/sound/apq8016-lpass.h>
> >
> > / {
> > aliases {
> > @@ -196,6 +199,18 @@ vibrator: vibrator {
> > };
> > };
> >
> > +&blsp_i2c1 {
> > + status = "okay";
> > +
> > + speaker_codec: audio-codec@34 {
> > + compatible = "nxp,tfa9895";
> > + reg = <0x34>;
> > + vddd-supply = <&pm8916_l5>;
> > + sound-name-prefix = "Speaker";
> > + #sound-dai-cells = <0>;
> > + };
> > +};
> > +
> > &blsp_i2c2 {
> > status = "okay";
> >
> > @@ -243,6 +258,13 @@ &gpu {
> > status = "okay";
> > };
> >
> > +&lpass {
> > + dai-link@3 {
> > + reg = <MI2S_QUATERNARY>;
> > + qcom,playback-sd-lines = <1>;
> > + };
> > +};
> Is that not status = reserved?
>

Correct. This is here to simplify switching to the modem-bypass audio
routing if someone does not need (or want) the modem. The direct audio
path with the LPASS drivers tends to be more reliable and configurable
(especially wrt bit formats, sampling rates, latency, channels etc).
I know that at some point this helped someone who tried to use an old
phone as some kind of portable musical instrument / synthesizer.

It's not too obvious that these definitions would be needed when making
those changes (because devices using the standard SD lines (i.e <0>) do
not need it). If you forget about this you get non-functional audio with
no error or any hint what could be wrong.

To simplify switching between the different audio routing options, the
lk2nd bootloader actually has an option to do this transformation in the
DTB automagically during boot. It's sort of like a DTB overlay that
disables the QDSP6 audio path and enables this node instead. The DAI
links are also adjusted where necessary.

Do you think a comment would help here?

Thanks,
Stephan

2023-09-26 21:41:05

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem

On 26.09.2023 18:51, Stephan Gerhold wrote:
> Enable sound and modem for the Samsung A2015 based devices (A3, A5, E5,
> E7, Grand Max). The setup is similar to most MSM8916 devices, i.e.:
>
> - QDSP6 audio
> - Earpiece/headphones/microphones via digital/analog codec in
> MSM8916/PM8916
> - WWAN Internet via BAM-DMUX
>
> except:
>
> - NXP TFA9895 codec for speaker on Quaternary MI2S
> - Samsung-specific audio jack detection (not supported yet)
>
> [Lin: Add e2015 and grandmax]
> Co-developed-by: "Lin, Meng-Bo" <[email protected]>
> Signed-off-by: "Lin, Meng-Bo" <[email protected]>
> Signed-off-by: Stephan Gerhold <[email protected]>
> ---
> .../dts/qcom/msm8916-samsung-a2015-common.dtsi | 55 ++++++++++++++++++++++
> .../dts/qcom/msm8916-samsung-e2015-common.dtsi | 4 ++
> .../boot/dts/qcom/msm8916-samsung-grandmax.dts | 4 ++
> 3 files changed, 63 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> index 0b29132b74e1..f71b18d89bf9 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> @@ -1,10 +1,13 @@
> // SPDX-License-Identifier: GPL-2.0-only
>
> #include "msm8916-pm8916.dtsi"
> +#include "msm8916-modem-qdsp6.dtsi"
> +
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/input/input.h>
> #include <dt-bindings/interrupt-controller/irq.h>
> #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> +#include <dt-bindings/sound/apq8016-lpass.h>
>
> / {
> aliases {
> @@ -196,6 +199,18 @@ vibrator: vibrator {
> };
> };
>
> +&blsp_i2c1 {
> + status = "okay";
> +
> + speaker_codec: audio-codec@34 {
> + compatible = "nxp,tfa9895";
> + reg = <0x34>;
> + vddd-supply = <&pm8916_l5>;
> + sound-name-prefix = "Speaker";
> + #sound-dai-cells = <0>;
> + };
> +};
> +
> &blsp_i2c2 {
> status = "okay";
>
> @@ -243,6 +258,13 @@ &gpu {
> status = "okay";
> };
>
> +&lpass {
> + dai-link@3 {
> + reg = <MI2S_QUATERNARY>;
> + qcom,playback-sd-lines = <1>;
> + };
> +};
Is that not status = reserved?

Konrad

2023-09-26 22:01:51

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem

[...]

>>> +
>>> &blsp_i2c2 {
>>> status = "okay";
>>>
>>> @@ -243,6 +258,13 @@ &gpu {
>>> status = "okay";
>>> };
>>>
>>> +&lpass {
>>> + dai-link@3 {
>>> + reg = <MI2S_QUATERNARY>;
>>> + qcom,playback-sd-lines = <1>;
>>> + };
>>> +};
>> Is that not status = reserved?
>>
>
> Correct. This is here to simplify switching to the modem-bypass audio
> routing if someone does not need (or want) the modem. The direct audio
> path with the LPASS drivers tends to be more reliable and configurable
> (especially wrt bit formats, sampling rates, latency, channels etc).
> I know that at some point this helped someone who tried to use an old
> phone as some kind of portable musical instrument / synthesizer.
>
> It's not too obvious that these definitions would be needed when making
> those changes (because devices using the standard SD lines (i.e <0>) do
> not need it). If you forget about this you get non-functional audio with
> no error or any hint what could be wrong.
>
> To simplify switching between the different audio routing options, the
> lk2nd bootloader actually has an option to do this transformation in the
> DTB automagically during boot. It's sort of like a DTB overlay that
> disables the QDSP6 audio path and enables this node instead. The DAI
> links are also adjusted where necessary.
>
> Do you think a comment would help here?
I'd say a comment would be necessary here :D

While I understand this use-case, I believe this is better suited
for an actual DTBO or something entirely kept inside lk2nd.
Otherwise this looks very confusing to an outside reader.

Konrad

2023-09-27 00:56:15

by Stephan Gerhold

[permalink] [raw]
Subject: Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem

On Tue, Sep 26, 2023 at 10:29:29PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 22:27, Stephan Gerhold wrote:
> > On Tue, Sep 26, 2023 at 10:18:22PM +0200, Konrad Dybcio wrote:
> >> On 26.09.2023 22:09, Stephan Gerhold wrote:
> >>> On Tue, Sep 26, 2023 at 09:56:12PM +0200, Konrad Dybcio wrote:
> >>>> [...]
> >>>>
> >>>>>>> +
> >>>>>>> &blsp_i2c2 {
> >>>>>>> status = "okay";
> >>>>>>>
> >>>>>>> @@ -243,6 +258,13 @@ &gpu {
> >>>>>>> status = "okay";
> >>>>>>> };
> >>>>>>>
> >>>>>>> +&lpass {
> >>>>>>> + dai-link@3 {
> >>>>>>> + reg = <MI2S_QUATERNARY>;
> >>>>>>> + qcom,playback-sd-lines = <1>;
> >>>>>>> + };
> >>>>>>> +};
> >>>>>> Is that not status = reserved?
> >>>>>>
> >>>>>
> >>>>> Correct. This is here to simplify switching to the modem-bypass audio
> >>>>> routing if someone does not need (or want) the modem. The direct audio
> >>>>> path with the LPASS drivers tends to be more reliable and configurable
> >>>>> (especially wrt bit formats, sampling rates, latency, channels etc).
> >>>>> I know that at some point this helped someone who tried to use an old
> >>>>> phone as some kind of portable musical instrument / synthesizer.
> >>>>>
> >>>>> It's not too obvious that these definitions would be needed when making
> >>>>> those changes (because devices using the standard SD lines (i.e <0>) do
> >>>>> not need it). If you forget about this you get non-functional audio with
> >>>>> no error or any hint what could be wrong.
> >>>>>
> >>>>> To simplify switching between the different audio routing options, the
> >>>>> lk2nd bootloader actually has an option to do this transformation in the
> >>>>> DTB automagically during boot. It's sort of like a DTB overlay that
> >>>>> disables the QDSP6 audio path and enables this node instead. The DAI
> >>>>> links are also adjusted where necessary.
> >>>>>
> >>>>> Do you think a comment would help here?
> >>>> I'd say a comment would be necessary here :D
> >>>>
> >>>
> >>> No problem, I will try to add something simple.
> >>>
> >>>> While I understand this use-case, I believe this is better suited
> >>>> for an actual DTBO or something entirely kept inside lk2nd.
> >>>> Otherwise this looks very confusing to an outside reader.
> >>>>
> >>>
> >>> Translating from the QDSP6 audio setup to the LPASS one is mostly simple
> >>> but not entirely trivial (especially the patching needed for the DAI
> >>> links). Main blocker for DTBOs is that you can only add or change, but
> >>> AFAIK there is no mechanism to _delete_ or fully recreate nodes.
> >> Correct.
> >>
> >>>
> >>> I guess I could maybe derive this from the QDSP6 definitions using
> >>> custom magic code, but the code complexity for that is much higher than
> >>> adding these nodes here for completeness.
> >> I hate to be the bearer of bad news, but this is probably more
> >> of a "do we reasonably want this upstream" type question, as
> >> you'll probably get some grumpy emails about upstream not caring
> >> about what's outside the mainline tree..
> >>
> >
> > IMHO this is hardly a "downstream" type of situation. The whole point of
> > the magic in lk2nd is to make the life for the mainline code base easier
> > and simpler, while still allowing the "bypass modem audio" feature with
> > an unmodified kernel.
> >
> > Before implementing it like this I used to have this directly in the
> > kernel tree, by duplicating each device into a normal XYZ.dts and
> > "XYZ-no-modem.dts". If you apply this to the total of ~45 different
> > MSM8916/MSM8939 DTBs I would like to get upstream this options gets
> > extremely ugly. :'D
> Maybe a kernel module parameter could be somehow useful here?
>

Maybe, but ultimately this affecs multiple modules that do not
necessarily know about each other. You need to enable the &lpass node
but also adjust the DAI links in the &sound node to refer to the lpass
DAIs instead of q6afe/q6routing. Implementing this in the kernel would
be complexity everyone would have to carry, while in lk2nd it's local to
the devices that can actually make use of it.

With the device tree we have the flexibility to adjust it based on
device-specifics (RAM size, display panels, broken CPU cores, ...).
I think it's perfectly fine if the bootloader makes good use of this
capability to keep Linux code simple and focused.

Thanks,
Stephan

2023-09-27 05:17:44

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem

On 26.09.2023 22:09, Stephan Gerhold wrote:
> On Tue, Sep 26, 2023 at 09:56:12PM +0200, Konrad Dybcio wrote:
>> [...]
>>
>>>>> +
>>>>> &blsp_i2c2 {
>>>>> status = "okay";
>>>>>
>>>>> @@ -243,6 +258,13 @@ &gpu {
>>>>> status = "okay";
>>>>> };
>>>>>
>>>>> +&lpass {
>>>>> + dai-link@3 {
>>>>> + reg = <MI2S_QUATERNARY>;
>>>>> + qcom,playback-sd-lines = <1>;
>>>>> + };
>>>>> +};
>>>> Is that not status = reserved?
>>>>
>>>
>>> Correct. This is here to simplify switching to the modem-bypass audio
>>> routing if someone does not need (or want) the modem. The direct audio
>>> path with the LPASS drivers tends to be more reliable and configurable
>>> (especially wrt bit formats, sampling rates, latency, channels etc).
>>> I know that at some point this helped someone who tried to use an old
>>> phone as some kind of portable musical instrument / synthesizer.
>>>
>>> It's not too obvious that these definitions would be needed when making
>>> those changes (because devices using the standard SD lines (i.e <0>) do
>>> not need it). If you forget about this you get non-functional audio with
>>> no error or any hint what could be wrong.
>>>
>>> To simplify switching between the different audio routing options, the
>>> lk2nd bootloader actually has an option to do this transformation in the
>>> DTB automagically during boot. It's sort of like a DTB overlay that
>>> disables the QDSP6 audio path and enables this node instead. The DAI
>>> links are also adjusted where necessary.
>>>
>>> Do you think a comment would help here?
>> I'd say a comment would be necessary here :D
>>
>
> No problem, I will try to add something simple.
>
>> While I understand this use-case, I believe this is better suited
>> for an actual DTBO or something entirely kept inside lk2nd.
>> Otherwise this looks very confusing to an outside reader.
>>
>
> Translating from the QDSP6 audio setup to the LPASS one is mostly simple
> but not entirely trivial (especially the patching needed for the DAI
> links). Main blocker for DTBOs is that you can only add or change, but
> AFAIK there is no mechanism to _delete_ or fully recreate nodes.
Correct.

>
> I guess I could maybe derive this from the QDSP6 definitions using
> custom magic code, but the code complexity for that is much higher than
> adding these nodes here for completeness.
I hate to be the bearer of bad news, but this is probably more
of a "do we reasonably want this upstream" type question, as
you'll probably get some grumpy emails about upstream not caring
about what's outside the mainline tree..

>
> Let me try to add some comment first.
Please try to explicitly explain the reasoning of why one would
want this change and what are the drawbacks etc.

Konrad

2023-09-27 05:57:09

by Stephan Gerhold

[permalink] [raw]
Subject: Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem

On Tue, Sep 26, 2023 at 10:18:22PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 22:09, Stephan Gerhold wrote:
> > On Tue, Sep 26, 2023 at 09:56:12PM +0200, Konrad Dybcio wrote:
> >> [...]
> >>
> >>>>> +
> >>>>> &blsp_i2c2 {
> >>>>> status = "okay";
> >>>>>
> >>>>> @@ -243,6 +258,13 @@ &gpu {
> >>>>> status = "okay";
> >>>>> };
> >>>>>
> >>>>> +&lpass {
> >>>>> + dai-link@3 {
> >>>>> + reg = <MI2S_QUATERNARY>;
> >>>>> + qcom,playback-sd-lines = <1>;
> >>>>> + };
> >>>>> +};
> >>>> Is that not status = reserved?
> >>>>
> >>>
> >>> Correct. This is here to simplify switching to the modem-bypass audio
> >>> routing if someone does not need (or want) the modem. The direct audio
> >>> path with the LPASS drivers tends to be more reliable and configurable
> >>> (especially wrt bit formats, sampling rates, latency, channels etc).
> >>> I know that at some point this helped someone who tried to use an old
> >>> phone as some kind of portable musical instrument / synthesizer.
> >>>
> >>> It's not too obvious that these definitions would be needed when making
> >>> those changes (because devices using the standard SD lines (i.e <0>) do
> >>> not need it). If you forget about this you get non-functional audio with
> >>> no error or any hint what could be wrong.
> >>>
> >>> To simplify switching between the different audio routing options, the
> >>> lk2nd bootloader actually has an option to do this transformation in the
> >>> DTB automagically during boot. It's sort of like a DTB overlay that
> >>> disables the QDSP6 audio path and enables this node instead. The DAI
> >>> links are also adjusted where necessary.
> >>>
> >>> Do you think a comment would help here?
> >> I'd say a comment would be necessary here :D
> >>
> >
> > No problem, I will try to add something simple.
> >
> >> While I understand this use-case, I believe this is better suited
> >> for an actual DTBO or something entirely kept inside lk2nd.
> >> Otherwise this looks very confusing to an outside reader.
> >>
> >
> > Translating from the QDSP6 audio setup to the LPASS one is mostly simple
> > but not entirely trivial (especially the patching needed for the DAI
> > links). Main blocker for DTBOs is that you can only add or change, but
> > AFAIK there is no mechanism to _delete_ or fully recreate nodes.
> Correct.
>
> >
> > I guess I could maybe derive this from the QDSP6 definitions using
> > custom magic code, but the code complexity for that is much higher than
> > adding these nodes here for completeness.
> I hate to be the bearer of bad news, but this is probably more
> of a "do we reasonably want this upstream" type question, as
> you'll probably get some grumpy emails about upstream not caring
> about what's outside the mainline tree..
>

IMHO this is hardly a "downstream" type of situation. The whole point of
the magic in lk2nd is to make the life for the mainline code base easier
and simpler, while still allowing the "bypass modem audio" feature with
an unmodified kernel.

Before implementing it like this I used to have this directly in the
kernel tree, by duplicating each device into a normal XYZ.dts and
"XYZ-no-modem.dts". If you apply this to the total of ~45 different
MSM8916/MSM8939 DTBs I would like to get upstream this options gets
extremely ugly. :'D

Thanks,
Stephan

2023-09-27 08:29:28

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem

On 26.09.2023 22:27, Stephan Gerhold wrote:
> On Tue, Sep 26, 2023 at 10:18:22PM +0200, Konrad Dybcio wrote:
>> On 26.09.2023 22:09, Stephan Gerhold wrote:
>>> On Tue, Sep 26, 2023 at 09:56:12PM +0200, Konrad Dybcio wrote:
>>>> [...]
>>>>
>>>>>>> +
>>>>>>> &blsp_i2c2 {
>>>>>>> status = "okay";
>>>>>>>
>>>>>>> @@ -243,6 +258,13 @@ &gpu {
>>>>>>> status = "okay";
>>>>>>> };
>>>>>>>
>>>>>>> +&lpass {
>>>>>>> + dai-link@3 {
>>>>>>> + reg = <MI2S_QUATERNARY>;
>>>>>>> + qcom,playback-sd-lines = <1>;
>>>>>>> + };
>>>>>>> +};
>>>>>> Is that not status = reserved?
>>>>>>
>>>>>
>>>>> Correct. This is here to simplify switching to the modem-bypass audio
>>>>> routing if someone does not need (or want) the modem. The direct audio
>>>>> path with the LPASS drivers tends to be more reliable and configurable
>>>>> (especially wrt bit formats, sampling rates, latency, channels etc).
>>>>> I know that at some point this helped someone who tried to use an old
>>>>> phone as some kind of portable musical instrument / synthesizer.
>>>>>
>>>>> It's not too obvious that these definitions would be needed when making
>>>>> those changes (because devices using the standard SD lines (i.e <0>) do
>>>>> not need it). If you forget about this you get non-functional audio with
>>>>> no error or any hint what could be wrong.
>>>>>
>>>>> To simplify switching between the different audio routing options, the
>>>>> lk2nd bootloader actually has an option to do this transformation in the
>>>>> DTB automagically during boot. It's sort of like a DTB overlay that
>>>>> disables the QDSP6 audio path and enables this node instead. The DAI
>>>>> links are also adjusted where necessary.
>>>>>
>>>>> Do you think a comment would help here?
>>>> I'd say a comment would be necessary here :D
>>>>
>>>
>>> No problem, I will try to add something simple.
>>>
>>>> While I understand this use-case, I believe this is better suited
>>>> for an actual DTBO or something entirely kept inside lk2nd.
>>>> Otherwise this looks very confusing to an outside reader.
>>>>
>>>
>>> Translating from the QDSP6 audio setup to the LPASS one is mostly simple
>>> but not entirely trivial (especially the patching needed for the DAI
>>> links). Main blocker for DTBOs is that you can only add or change, but
>>> AFAIK there is no mechanism to _delete_ or fully recreate nodes.
>> Correct.
>>
>>>
>>> I guess I could maybe derive this from the QDSP6 definitions using
>>> custom magic code, but the code complexity for that is much higher than
>>> adding these nodes here for completeness.
>> I hate to be the bearer of bad news, but this is probably more
>> of a "do we reasonably want this upstream" type question, as
>> you'll probably get some grumpy emails about upstream not caring
>> about what's outside the mainline tree..
>>
>
> IMHO this is hardly a "downstream" type of situation. The whole point of
> the magic in lk2nd is to make the life for the mainline code base easier
> and simpler, while still allowing the "bypass modem audio" feature with
> an unmodified kernel.
>
> Before implementing it like this I used to have this directly in the
> kernel tree, by duplicating each device into a normal XYZ.dts and
> "XYZ-no-modem.dts". If you apply this to the total of ~45 different
> MSM8916/MSM8939 DTBs I would like to get upstream this options gets
> extremely ugly. :'D
Maybe a kernel module parameter could be somehow useful here?

Konrad

2023-09-27 09:56:39

by Stephan Gerhold

[permalink] [raw]
Subject: Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem

On Tue, Sep 26, 2023 at 09:56:12PM +0200, Konrad Dybcio wrote:
> [...]
>
> >>> +
> >>> &blsp_i2c2 {
> >>> status = "okay";
> >>>
> >>> @@ -243,6 +258,13 @@ &gpu {
> >>> status = "okay";
> >>> };
> >>>
> >>> +&lpass {
> >>> + dai-link@3 {
> >>> + reg = <MI2S_QUATERNARY>;
> >>> + qcom,playback-sd-lines = <1>;
> >>> + };
> >>> +};
> >> Is that not status = reserved?
> >>
> >
> > Correct. This is here to simplify switching to the modem-bypass audio
> > routing if someone does not need (or want) the modem. The direct audio
> > path with the LPASS drivers tends to be more reliable and configurable
> > (especially wrt bit formats, sampling rates, latency, channels etc).
> > I know that at some point this helped someone who tried to use an old
> > phone as some kind of portable musical instrument / synthesizer.
> >
> > It's not too obvious that these definitions would be needed when making
> > those changes (because devices using the standard SD lines (i.e <0>) do
> > not need it). If you forget about this you get non-functional audio with
> > no error or any hint what could be wrong.
> >
> > To simplify switching between the different audio routing options, the
> > lk2nd bootloader actually has an option to do this transformation in the
> > DTB automagically during boot. It's sort of like a DTB overlay that
> > disables the QDSP6 audio path and enables this node instead. The DAI
> > links are also adjusted where necessary.
> >
> > Do you think a comment would help here?
> I'd say a comment would be necessary here :D
>

No problem, I will try to add something simple.

> While I understand this use-case, I believe this is better suited
> for an actual DTBO or something entirely kept inside lk2nd.
> Otherwise this looks very confusing to an outside reader.
>

Translating from the QDSP6 audio setup to the LPASS one is mostly simple
but not entirely trivial (especially the patching needed for the DAI
links). Main blocker for DTBOs is that you can only add or change, but
AFAIK there is no mechanism to _delete_ or fully recreate nodes.

I guess I could maybe derive this from the QDSP6 definitions using
custom magic code, but the code complexity for that is much higher than
adding these nodes here for completeness.

Let me try to add some comment first.

Thanks,
Stephan