Re: [PATCH] arm64: dts: qcom: sm8750: Add adsp fastrpc support

["Alexey Klimov" <alexey.klimov@linaro.org>]

On Tue May 13, 2025 at 2:09 PM BST, Konrad Dybcio wrote:
> On 5/9/25 5:42 AM, Ekansh Gupta wrote:
>> 
>> On 5/9/2025 4:27 AM, Konrad Dybcio wrote:
>>> On 5/9/25 12:20 AM, Alexey Klimov wrote:
>>>> On Fri May 2, 2025 at 10:38 AM BST, Konrad Dybcio wrote:
>>>>> On 5/2/25 3:15 AM, Alexey Klimov wrote:
>>>>>> While at this, also add required memory region for fastrpc.
>>>>>>
>>>>>> Tested on sm8750-mtp device with adsprpdcd.
>>>>>>
>>>>>> Cc: Ekansh Gupta <quic_ekangupt@quicinc.com>
>>>>>> Cc: Srinivas Kandagatla <srini@kernel.org>
>>>>>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>>>>>> ---
>>>>>>  arch/arm64/boot/dts/qcom/sm8750.dtsi | 70 ++++++++++++++++++++++++++++
>>>>>>  1 file changed, 70 insertions(+)

[...]

>>>>> IIUC the driver only considers this on the sensor DSP
>>>> Memory region is required for audio protection domain + adsprpdcd as far as I know.
>>> next-20250508
>>>
>>> rmem_node = of_parse_phandle(rdev->of_node, "memory-region", 0);
>>> if (domain_id == SDSP_DOMAIN_ID && rmem_node) {
>>> 	// ...
>>> }
>>>
>>> maybe some driver changes are still pending?
>> 
>> Would like to add some more details here:
>> 
>> Memory region is required for audio PD for dynamic loading and remote heap memory
>> requirements. Some initial memory(~2MB) is allocated initially when audio daemon
>> is getting attached[1] and this memory is added to audio PD memory pool.
>> 
>> Additionally, if there is some additional memory requirement from audio PD, the
>> PD can request for more memory using remote heap request[2]
>> 
>> The support for SDSP was added sometime back[3] to support SDSP usecases on some old
>> platform as there were no dedicated context banks for SDSP there. On recent platforms,
>> context banks are available wherever SDSP is supported. 
>> 
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/misc/fastrpc.c#n1273
>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/misc/fastrpc.c#n1884
>> [3] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/misc/fastrpc.c?id=c3c0363bc72d4d0907a6d446d7424b3f022ce82a
>
> Ok, this was mildly confusing given there's specific "normal" region
> handling, but the DMA allocator takes care of this one

Konrad, Srini, Ekansh,
what's left here -- I guess memory region is needed, 
so do you want me to test with secure flag as the only required change at this point?

Best regards,
Alexey