Message-ID: <ad637368-5fa7-45fa-8bb3-3a2cc754ed9b@ti.com>
Date: Fri, 11 Apr 2025 10:20:26 +0530
From: Beleswar Prasad Padhi <b-padhi@...com>
To: Andrew Davis <afd@...com>, Devarsh Thakkar <devarsht@...com>,
        Jai Luthra
	<jai.luthra@...asonboard.com>
CC: Judith Mendez <jm@...com>, Nishanth Menon <nm@...com>,
        Hari Nagalla
	<hnagalla@...com>, Tero Kristo <kristo@...nel.org>,
        Rob Herring
	<robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
	<conor+dt@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, Vignesh
 Raghavendra <vigneshr@...com>,
        Markus Schneider-Pargmann <msp@...libre.com>, <praneeth@...com>,
        "Khasim, Syed Mohammed" <khasim@...com>,
        <tomi.valkeinen@...asonboard.com>, <v-krishnamoorthy@...com>,
        <s-tripathy@...com>, <s-tripathi1@...com>, <c-shilwant@...com>,
        <r-ravikumar@...com>
Subject: Re: [PATCH v6 06/11] arm64: dts: ti: k3-am62a7-sk: Enable IPC with
 remote processors


On 10/04/25 23:52, Andrew Davis wrote:
> On 4/10/25 6:38 AM, Devarsh Thakkar wrote:
>> Hi Jai,
>>
>> On 10/04/25 15:48, Jai Luthra wrote:
>>> Hi Devarsh,
>>>
>>> Thanks for the cc here.
>>
>> Thanks for the quick comments.
>>
>>>
>> <snip>
>>
>>> On the basic camera + ISP usecase, afaiu the downstream edgeAI SDK uses
>>> custom gstreamer elements that make calls to the aforementioned R5 core
>>> that controls the ISP. On top of that there are additional gstreamer
>>> patches that are not yet posted upstream for review from the community,
>>> so the userspace design isn't really set in stone, or upstream-friendly
>>> yet.
>>>
>>
>> I don't see much relation of carve-outs with Gstreamer or it's 
>> pending downstream patches. The memory is mainly managed from 
>> firmwares (mainly openvx layer being used underneath) and there are 
>> even non-gstreamer pure openvx based use-cases/tests which use these 
>> carveouts. At the end of the day, the firmwares from the only SDK 
>> which is released publicly for AM62A uses all these carveouts.
>>
>
> These are programmable cores, you can run whatever you want on them. 
> You can
> make your own firmware if you like, we have support for them in our 
> MCU+(FreeRTOS)
> offering today[0](look at all these firmware you can build/run!).
>
> In a week or so I'll start pushing support for these cores into Zephyr,
> bringing in even more firmware options for these cores.
>
> I simply do not see why one firmware, shipped with one of our SDKs*, 
> doing
> things wrong should force us to hack up our DT here in upstream Linux.


Agreed. I don't see why we should incline towards supporting one of the 
SDKs.

For this patch as it as,

Reviewed-by: Beleswar Padhi <b-padhi@...com>

Thanks,
Beleswar

>
> *Speaking of the "only" SDK's firmware, if you take our Yocto meta-ti 
> layer and
> build an SDK yourself, you get firmware by default that *doesn't need 
> extra
> carveouts*! [1][2]
>
> Andrew
>
> [0] 
> https://github.com/TexasInstruments/mcupsdk-core-k3/blob/k3_main/makefile.am62ax
> [1] 
> https://git.yoctoproject.org/meta-ti/tree/meta-ti-bsp/recipes-bsp/ti-rtos-fw/ti-rtos-echo-test-fw.bb
> [2] 
> https://git.ti.com/cgit/processor-firmware/ti-linux-firmware/tree/ti-ipc/am62axx?h=ti-linux-firmware
>
>>
>>> IMO if that architecture is still under discussion, it might be better
>>> to keep the edgeAI specific carveouts out of the upstream DTs.. just in
>>> case the carevouts have to go away, or change significantly.
>>>
>>> If you are sure that the regions and firmware architecture is set in
>>> stone and won't be updated even if there is a complete redesign of the
>>> userspace/application level stack for accessing the ISP (let's say u
>> sing> libcamera), only then it makes sense to add the carveouts right 
>> now.
>>
>>
>> Yes as I said if whole firmware arch is getting updated then better 
>> to wait. I think probably the firmware team marked in cc can comment 
>> on that. Moreover I don't see any point of adding only half the 
>> regions as that would anyway not work with SDK supplied firmwares, 
>> for e.g. RTOS-to-RTOS ipc test run by firmwares on bootup would fail, 
>> along with other camera+ISP and AI use-cases.
>>
>> Regards
>> Devarsh
>>
>>>>>>
>>>>>> I understand your point, currently with this patch remoteproc 
>>>>>> loading
>>>>>> will not work for some cores. However, the goal here is to 
>>>>>> standardize
>>>>>> as much as possible the memory carveout sizes, push the "demo 
>>>>>> firmware"
>>>>>> to request resources the correct way from resource table, and 
>>>>>> move away
>>>>>> from this dependency and limitations that we have with our firmware.
>>>>
>>>> I understand this, but my view is that w.r.t firmware only goal 
>>>> should not
>>>> just be tp demonstrate correct way of requesting resources from
>>>> resource-tables, optimize the carve-outs etc but also to 
>>>> demonstrate the
>>>> primary use-cases (camera+ISP+edgeAI) which the device is capable of.
>>>>
>>>>>> should soon be able to generate our own firmware using Zephyr,  
>>>>>> which
>>>>>> Andrew is pioneering, so with this firmware we should move to the
>>>>>> correct direction upstream. Downstream we are still using the memory
>>>>>> carveout sizes that the firmware folk want so desperately to 
>>>>>> keep, for
>>>>>> now..
>>>>>>
>>>>>
>>>>> +1
>>>>>
>>>>> I have this Zephyr based firmware for AM62A working and it uses the
>>>>> standard IPC regions as specified in this patch. I'll be posting 
>>>>> the PR
>>>>> for it in Zephyr upstream by the end of week.
>>>>>
>>>>
>>>> I understand this, but will this zephyr based firmware support 
>>>> vision +
>>>> edgeAI analytics ? Does it demonstrate all the unique capabilities 
>>>> of AM62A
>>>> SoC ? If not, then what would be utility of such firmware on AM62A 
>>>> where
>>>> these are the primary use-cases w.r.t AM62A ?
>>>>
>>>> Why should upstream device-tree use carve-outs which match to this 
>>>> demo
>>>> zephyr based firmware (which apparently not many are using and is 
>>>> not going
>>>> into any official SDK release) instead of official firmwares going 
>>>> into SDK
>>>> ? SDK released firmwares are being used by so many customers and SDK
>>>> documentation maps to it, but zephyr firmware that is being pitched 
>>>> here,
>>>> who would be the potential users and what would be it's utility ?
>>>>
>>>> [1]: https://www.ti.com/tool/PROCESSOR-SDK-J721E
>>>>
>>>> Regards
>>>> Devarsh
>>>>
>>>>> For this patch as it is:
>>>>>
>>>>> Acked-by: Andrew Davis <afd@...com>
>>>>>
>>>>
>>>>
>>>>> Andrew
>>>>>
>>>>> [0] 
>>>>> https://lore.kernel.org/lkml/20241011123922.23135-1-richard@nod.at/
>>>>> [1] https://git.ti.com/cgit/edgeai/meta-edgeai/tree/recipes-kernel/
>>>>> linux/linux-ti-staging/j721e-evm/0001-arm64-dts-ti-Add-DTB-overlays-for- 
>>>>>
>>>>> vision-apps-and-ed.patch?h=kirkstone
>>>>>
>>>>>> ~ Judith
>>>>>>
>>>>>>>
>>>>>>> [1]:
>>>>>>> https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/tree/
>>>>>>> arch/arm64/boot/dts/ti/k3-am62a7-sk.dts?h=ti-linux-6.6.y-cicd#n103
>>>>>>> [2]: https://www.ti.com/tool/PROCESSOR-SDK-AM62A
>>>>>>>
>>>>>>> Regards
>>>>>>> Devarsh
>>>>>>>
>>>>>>>>        opp-table {
>>>>>>>> @@ -741,3 +771,57 @@ dpi1_out: endpoint {
>>>>>>>>            };
>>>>>>>>        };
>>>>>>>>    };
>>>>>>>> +
>>>>>>>> +&mailbox0_cluster0 {
>>>>>>>> +    status = "okay";
>>>>>>>> +
>>>>>>>> +    mbox_r5_0: mbox-r5-0 {
>>>>>>>> +        ti,mbox-rx = <0 0 0>;
>>>>>>>> +        ti,mbox-tx = <1 0 0>;
>>>>>>>> +    };
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +&mailbox0_cluster1 {
>>>>>>>> +    status = "okay";
>>>>>>>> +
>>>>>>>> +    mbox_c7x_0: mbox-c7x-0 {
>>>>>>>> +        ti,mbox-rx = <0 0 0>;
>>>>>>>> +        ti,mbox-tx = <1 0 0>;
>>>>>>>> +    };
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +&mailbox0_cluster2 {
>>>>>>>> +    status = "okay";
>>>>>>>> +
>>>>>>>> +    mbox_mcu_r5_0: mbox-mcu-r5-0 {
>>>>>>>> +        ti,mbox-rx = <0 0 0>;
>>>>>>>> +        ti,mbox-tx = <1 0 0>;
>>>>>>>> +    };
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +&wkup_r5fss0 {
>>>>>>>> +    status = "okay";
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +&wkup_r5fss0_core0 {
>>>>>>>> +    mboxes = <&mailbox0_cluster0>, <&mbox_r5_0>;
>>>>>>>> +    memory-region = <&wkup_r5fss0_core0_dma_memory_region>,
>>>>>>>> + <&wkup_r5fss0_core0_memory_region>;
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +&mcu_r5fss0 {
>>>>>>>> +    status = "okay";
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +&mcu_r5fss0_core0 {
>>>>>>>> +    mboxes = <&mailbox0_cluster2>, <&mbox_mcu_r5_0>;
>>>>>>>> +    memory-region = <&mcu_r5fss0_core0_dma_memory_region>,
>>>>>>>> + <&mcu_r5fss0_core0_memory_region>;
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +&c7x_0 {
>>>>>>>> +    mboxes = <&mailbox0_cluster1>, <&mbox_c7x_0>;
>>>>>>>> +    memory-region = <&c7x_0_dma_memory_region>,
>>>>>>>> +            <&c7x_0_memory_region>;
>>>>>>>> +    status = "okay";
>>>>>>>> +};
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>

Powered by blists - more mailing lists