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

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 6/13/25 4:48 PM, Alexey Klimov wrote:
> 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?

I think so, please give it a shot

Konrad