Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add remoteproc target driver #79605

Closed
wants to merge 6 commits into from
Closed

Conversation

mausys
Copy link
Contributor

@mausys mausys commented Oct 9, 2024

This is just a proof of concept for a deeper integration of OpenAMP in Zephyr. A lot of stuff is missing like a driver API. Also it needs a lot of cleanup. The idea is to configure the RemoteProc target in the devicetree and the application just needs to implement the RPMSG endpoint logic. The Zephyr RemoteProc target driver would be a counterpart to the Linux kernel RemoteProc master driver. So the dts file would include something like this:

remoteproc {
      #address-cells = <1>;
      #size-cells = <0>;
      compatible = "openamp,remoteproc";
      carveouts = <&vdev0vring0 &vdev0vring1 &vdev0buffer>;

      vdev0: vdev@0 {
        #address-cells = <1>;
        #size-cells = <0>;
        compatible = "openamp,vdev";
        vring0-carveout = <&vdev0vring0>;
        vring1-carveout = <&vdev0vring1>;
        buffer-carveout = <&vdev0buffer>;
        num-tx-buffers = <8>;
        num-rx-buffers = <8>;
        mboxes = <&mbox 1>, <&mbox 0>;
        mbox-names = "tx", "rx";
        reg = <0>;
    };
  };

For example I wrote a logging backend that sends the log messages over RPMSG to the master with a fallback to the ram console. That would be a rpmsg bus member. Obviously it needs a lot of work, but I think it's better to get feedback early. Thoughts?

@mausys
Copy link
Contributor Author

mausys commented Oct 9, 2024

btw I don't use an actual AMP SOC, my project uses remoteproc over PCIe (x86 master and a Zynq 7000 PCIe card).

@mausys mausys force-pushed the remoteproc branch 2 times, most recently from 6306368 to 3f8ced5 Compare October 9, 2024 15:09
@vaishnavachath vaishnavachath self-requested a review October 10, 2024 01:05
@mausys
Copy link
Contributor Author

mausys commented Oct 22, 2024

endpoint can now be added in the dts:

&vdev0 {
    ept0: endpoint@0 {
        status = "okay";
        compatible = "openamp,rpmsg-endpoint";
        type = "rpmsg-raw";
        reg = <0x0 0xffffffff>;
    };
};

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
add OpenAMP RemoteProc driver as a counterpart
to the Linux RemoteProc framework and a RPMSG
endpoint as a log backend.

Signed-off-by: Simon Maurer <mail@maurer.systems>
Signed-off-by: Simon Maurer <mail@maurer.systems>
    rename vdev to a more accurate name

Signed-off-by: Simon Maurer <mail@maurer.systems>
Signed-off-by: Simon Maurer <mail@maurer.systems>
Signed-off-by: Simon Maurer <mail@maurer.systems>
@JarmouniA JarmouniA added the RFC Request For Comments: want input from the community label Oct 29, 2024
@JarmouniA
Copy link
Collaborator

Signed-off-by: Simon Maurer <mail@maurer.systems>
@arnopo
Copy link
Collaborator

arnopo commented Nov 6, 2024

Hello @mausys
Your proposal seems like a good idea. I have not looked in detail at the code yet. Here are a few initial comments:

  • Concerning the remoteproc driver creation:
    The resource table is currently dedicated to declaring resources to Linux when Zephyr is running on a coprocessor. However, be careful, as Zephyr can also be the host and, in the future, may need to manage a coprocessor with or without a resource table, with or without managing the remote processor live cycle. To keep flexibility for future enhancements, I would rename remoteproc.c to remoteproc_rsctb.c.

  • concerning vdev:
    Did you look at https://elixir.bootlin.com/zephyr/v3.7.0/source/subsys/ipc/rpmsg_service ?
    Do you think it would be possible to reuse the service instead of duplicating the RPMsg part? The link between the application, the RPMsg framework, and the mailbox is not trivial to implement in a multi-threaded environment.

  • Concerning the devicetree configuration:
    could you use the standard memory-region bindings instead of careveout?

remoteproc_rs_table {
      #address-cells = <1>;
      #size-cells = <0>;
      compatible = "remoteproc,rsc_table";

      vdev0: vdev@0 {
        #address-cells = <1>;
        #size-cells = <0>;
        compatible = "openamp,vdev";
	memory-region =  <&vdev0vring0>,<&vdev0vring1>, <&vdev0buffer>;
        memory-region-names = "vring0", "vring1", "buffer";
        num-tx-buffers = <8>;
        num-rx-buffers = <8>;
        mboxes = <&mbox 1>, <&mbox 0>;
        mbox-names = "tx", "rx";
        reg = <0>;
    };
  };


@mausys
Copy link
Contributor Author

mausys commented Nov 20, 2024

Hi @arnopo
thanks for your comment and sorry for my late response.
I'm working on restructuring the devicetree to be more aligned with the OpenAMP API. So for example the resource table will have its own node.
The OpenAMP library already handles synchronization and has a good high level API. What my driver does, is extracting the parameters with macro magic from the device tree, so it doesn't make sense to reuse the code from rpmsg_service.

@mausys
Copy link
Contributor Author

mausys commented Dec 16, 2024

I'm preparing a new version of the driver in a new pull request. The DTS of the new version will probably look something like this:

chosen {
    ...
    openamp,resource-table = &rsc_tbl;
  };  
...
  mbox: mbox@40060000 {
...
  };

  vdev0vring0: memory@1400 {
    compatible= "zephyr,memory-region";
    zephyr,memory-region = "vdev0vring0";
    reg = <0x1400 0x2000>;
  };

  vdev0vring1: memory@3400 {
    compatible= "zephyr,memory-region";
    zephyr,memory-region = "vdev0vring1";
    reg = <0x3400 0x2000>;
  };

  vdev0buffer: memory@5400 {
    compatible= "zephyr,memory-region";
    zephyr,memory-region = "vdev0buffer";
    reg = <0x5400 0x8000>;
  };  

...

openamp {
    rsc_tbl: resource_table {
      compatible = "openamp,resource-table";
      carveouts = <&vdev0vring0 &vdev0vring1 &vdev0buffer>;
      vdevs = <&vdev0>;
    };

    vrings {
      #address-cells = <1>;
      #size-cells = <0>;

      vdev0_tx: vring@0 {
        compatible = "openamp,vring";
        #vring-cells = <0>;
        notify-id = <0>;
        memory-region = <&vdev0vring0>;
        num-buffers = <8>;
        mboxes = <&mbox 0>;
        reg = <0>;
      };

      vdev0_rx: vring@1 {
        compatible = "openamp,vring";
        #vring-cells = <0>;
        notify-id = <1>;
        memory-region = <&vdev0vring1>;
        num-buffers = <8>;
        mboxes = <&mbox 1>;
        reg = <1>;
      };
    };

    vdevs {
      #address-cells = <1>;
      #size-cells = <0>;

      vdev0: vdev@0 {
        compatible = "openamp,rpmsg-virtio-device";
        #address-cells = <1>;
        #size-cells = <0>;
        vrings = <&vdev0_tx &vdev0_rx>;
        vring-names = "tx", "rx";
        memory-region = <&vdev0buffer>;
        reg = <0>;
      };
    };
  };

@mausys
Copy link
Contributor Author

mausys commented Dec 19, 2024

New development branch: https://github.com/mausys/zephyr/tree/openamp
I will create a new pull request after some basic testing.

@mausys mausys mentioned this pull request Jan 9, 2025
@mausys mausys closed this Jan 9, 2025
@mausys
Copy link
Contributor Author

mausys commented Jan 9, 2025

New pull request: #83754

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: AMP Asymmetric Multi-processing (AMP) area: Open AMP RFC Request For Comments: want input from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants