Re: [PATCH v6 06/11] arm64: dts: ti: k3-am62a7-sk: Enable IPC with remote processors

Jai Luthra
2025-04-11T06:45:58Z

[-- Attachment #1: Type: text/plain, Size: 9203 bytes --]

On Apr 10, 2025 at 13:22:29 -0500, 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.
>

I see, if the not-yet-upstream firmware can be updated, then I don't see
any reason to hardcode carveouts here.

For this patch as is,
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>

> *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.
> >

PS: Devarsh your mail client has format=flowed enabled, which causes
line length overflows when replying to it.

Please disable it, as advised here:
https://www.kernel.org/doc/html/latest/process/email-clients.html#thunderbird-gui

> > 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@ti.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";
> > > > > > > > +};
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > >
> >

--
Thanks,
Jai

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]