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

[Konrad Dybcio <konrad.dybcio@linaro.org>]

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