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

[Stephan Gerhold]

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