nxp-mcuxpresso / rpmsg-lite

RPMsg implementation for small MCUs
BSD 3-Clause "New" or "Revised" License
235 stars 74 forks source link

RPMSG buffer size and buffer count #17

Closed mtudan closed 3 years ago

mtudan commented 3 years ago

Hi @MichalPrincNXP,

I was trying to increase the RPMSG buffer size and buffer count both on A7 and M4 side. Our goal is to use (16 * 16KB) buffers. Below are test results with different configurations performed on 5.4 linux-imx based kernel: Buffer size Buffer count Boot result
512 512
1024 256
2048 128
4096 64
8192 32
16384 16
32768 8

If I disable CONFIG_REGULATOR_PF1550_RPMSG driver, Linux boots normally, the /dev/ttyRPMSG* char device is created and everything works as expected.

Parts of our device tree that might be relevant:

...
    pf1550-rpmsg {
        compatible = "rohm,bd70528-rpmsg";
        sw1_reg: SW1 {
                regulator-name = "SW1";
                regulator-min-microvolt = <1200000>;
                regulator-max-microvolt = <3400000>;
                regulator-boot-on;
                regulator-always-on;
        };

        sw2_reg: SW2 {
                regulator-name = "SW2";
                regulator-min-microvolt = <1200000>;
                regulator-max-microvolt = <3300000>;
                regulator-boot-on;
                regulator-always-on;
        };

        sw3_reg: SW3 {
                regulator-name = "SW3";
                regulator-min-microvolt = <1200000>;
                regulator-max-microvolt = <1200000>;
                regulator-boot-on;
                regulator-always-on;
        };

        vldo1_reg: LDO1 {
                regulator-name = "LDO1";
                regulator-min-microvolt = <1650000>;
                regulator-max-microvolt = <3300000>;
                regulator-boot-on;
                regulator-always-on;
        };

        vldo2_reg: LDO2 {
                regulator-name = "LDO2";
                regulator-min-microvolt = <1650000>;
                regulator-max-microvolt = <3300000>;
                regulator-boot-on;
                regulator-always-on;
        };

        vldo3_reg: LDO3 {
                regulator-name = "LDO3";
                regulator-min-microvolt = <1650000>;
                regulator-max-microvolt = <3300000>;
                regulator-always-on;
        };
    };

    imx7ulp-cm4 {
        compatible = "fsl,imx7ulp-cm4";
        ipc-only;
        rsc-da=<0x1fff8000>;
        mbox-names = "tx", "rx", "rxdb";
        mboxes = <&mu 0 1
              &mu 1 1
              &mu 3 1>;
        memory-region = <&vdev0vring0>, <&vdev0vring1>,
                <&vdev1vring0>, <&vdev1vring1>;
    };
...
&rpmsg{
    /*
     * 64K for one rpmsg instance, default using 2 rpmsg instances:
     * --0x9DF00000~0x9DF0FFFF: pmic,pm,audio,keys,gpio,sensor
     * --0x9DF10000~0x9DF1FFFF: pingpong,virtual tty
     */
    vdev-nums = <2>;
    reg = <0x9DF00000 0x20000>;
    status = "okay";
};

Below are diffs used in tests (showing (16 * 16KB) buffers in this case). Linux side:

diff --git a/drivers/rpmsg/imx_rpmsg.c b/drivers/rpmsg/imx_rpmsg.c
index 9dd704ae5a64..5d3617e02a49 100644
--- a/drivers/rpmsg/imx_rpmsg.c
+++ b/drivers/rpmsg/imx_rpmsg.c
@@ -69,8 +69,8 @@ struct imx_rpmsg_vproc {
  */
 #define REMOTE_READY_WAIT_MAX_RETRIES  500

-#define RPMSG_NUM_BUFS         (512)
-#define RPMSG_BUF_SIZE         (512)
+#define RPMSG_NUM_BUFS         (16)
+#define RPMSG_BUF_SIZE         (16384)
 #define RPMSG_BUFS_SPACE       (RPMSG_NUM_BUFS * RPMSG_BUF_SIZE)
 #define RPMSG_VRING_ALIGN      (4096)
 #define RPMSG_RING_SIZE        ((DIV_ROUND_UP(vring_size(RPMSG_NUM_BUFS / 2, \

M4 side:

diff --git a/Firmware/pilot/source/rpmsg_config.h b/Firmware/pilot/source/rpmsg_config.h
index 7b15fa84..c51affef 100755
--- a/Firmware/pilot/source/rpmsg_config.h
+++ b/Firmware/pilot/source/rpmsg_config.h
@@ -31,13 +31,13 @@
 //! Size of the buffer payload, it must be equal to (240, 496, 1008, ...)
 //! [2^n - 16].
 //! The default value is 496U.
-#define RL_BUFFER_PAYLOAD_SIZE (496U)
+#define RL_BUFFER_PAYLOAD_SIZE (16368U)

 //! @def RL_BUFFER_COUNT
 //!
 //! Number of the buffers, it must be power of two (2, 4, ...).
 //! The default value is 2U.
-#define RL_BUFFER_COUNT (256U)
+#define RL_BUFFER_COUNT (8U)

 //! @def RL_API_HAS_ZEROCOPY
 //!
MichalPrincNXP commented 3 years ago

Hello @mtudan , the only thing I have spot from you description is the incorrect RL_BUFFER_COUNT in rpmsg_config.h - it should be 16 to match with imx_rpmsg.c, but I guess this is just a copy/paste problem. Anyway, will try to ask sb. internally.

Hadatko commented 3 years ago

Hello @mtudan , the only thing I have spot from you description is the incorrect RL_BUFFER_COUNT in rpmsg_config.h - it should be 16 to match with imx_rpmsg.c, but I guess this is just a copy/paste problem. Anyway, will try to ask sb. internally.

@MichalPrincNXP if buffer count is wrong then RPMSG example is wrong as previously there was 512Count on one side and 256 on other side.

Hadatko commented 3 years ago

@MichalPrincNXP in rpmsg-lite there is several places where buffer count is multiplied like:

 if ((2U * (uint32_t)RL_BUFFER_COUNT) >
        ((RL_WORD_ALIGN_DOWN(shmem_length - (uint32_t)RL_VRING_OVERHEAD)) / (uint32_t)RL_BUFFER_SIZE))
    {
        return RL_NULL;
    }
MichalPrincNXP commented 3 years ago

@MichalPrincNXP in rpmsg-lite there is several places where buffer count is multiplied like:

 if ((2U * (uint32_t)RL_BUFFER_COUNT) >
        ((RL_WORD_ALIGN_DOWN(shmem_length - (uint32_t)RL_VRING_OVERHEAD)) / (uint32_t)RL_BUFFER_SIZE))
    {
        return RL_NULL;
    }

Yes, it could be sth. in rpmsg_lite itself, but

If I disable CONFIG_REGULATOR_PF1550_RPMSG driver, Linux boots normally, the /dev/ttyRPMSG* char device is created and everything works as expected.

... so it seems there could be sth. in this, I have asked Wayne for an advice.

mtudan commented 3 years ago

As @Hadatko mentioned, the default configuration is 512 buffers on Linux side, while this is defined as 256 on M4 side.

More info about boot logs. With the default configuration (512B * 512 buffers), the CONFIG_REGULATOR_PF1550_RPMSG driver will show the following logs:

[    1.435511] regulator_rpmsg virtio0.rpmsg-regulator-channel.-1.1: new channel: 0x402 -> 0x1!
[    1.445256] SW1: failed to get the current voltage(-1073741824)
[    1.451802] SW2: failed to get the current voltage(-1073741824)
[    1.458249] SW3: failed to get the current voltage(-1073741824)
[    1.464588] LDO1: failed to get the current voltage(-1073741824)
[    1.471071] LDO2: failed to get the current voltage(-1073741824)
[    1.477493] LDO3: failed to get the current voltage(-1073741824)

and with (16KB * 16 buffers) when the kernel panic happens, this will be shown:

[    3.112287] regulator_rpmsg virtio0.rpmsg-regulator-channel.-1.1: new channel: 0x402 -> 0x1!
[    3.122465] SW1: Bringing 536870912uV into 3400000-3400000uV
[    3.132863] SW2: Bringing 536870912uV into 3300000-3300000uV
[    3.144022] LDO1: Bringing 536870912uV into 3300000-3300000uV
[    3.151987] ------------[ cut here ]------------
[    3.156395] kernel BUG at drivers/regulator/helpers.c:662!
[    3.161850] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
[    3.167654] Modules linked in:
[    3.170691] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.4.47-lmp-standard #1
[    3.177709] Hardware name: Freescale i.MX7ULP (Device Tree)
[    3.183275] PC is at regulator_list_voltage_table+0x38/0x3c
[    3.188805] LR is at _regulator_do_set_voltage+0x12c/0x6f4
[    3.194260] pc : [<c062b500>]    lr : [<c0624a08>]    psr: 60000013
[    3.200497] sp : ef129c10  ip : ef129c20  fp : ef129c1c
[    3.205696] r10: ee505c34  r9 : c1089264  r8 : 00000000
[    3.210899] r7 : c10d1998  r6 : 00325aa0  r5 : c06313fc  r4 : ee505c00
[    3.217401] r3 : c108881c  r2 : 00000000  r1 : ef129d24  r0 : ee505c00
[    3.223907] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[    3.231014] Control: 10c5387d  Table: 6000406a  DAC: 00000051
[    3.236729] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
[    3.242710] Stack: (0xef129c10 to 0xef12a000)

Note the difference in SW1: message for example.

mtudan commented 3 years ago

After some debugging, I realized that in case of kernel panic the pf1550-regulator-rpmsg.c kernel driver sends multiple RPMsg messages to M4. It will send 5 messages for SW1, 5 messages for SW2, and 3 messages for LDO1 and then kernel panic will happen. In case of normal boot, it will just send one message per regulator. So, the question is what could cause the pf1550-regulator-rpmsg.c kernel driver to send multiple messages in case of 16 buffers?

Hadatko commented 3 years ago

I would say that maybe 16 is too small number as it means 8 for one side. If the messages are not freed on other side we can get out of free buffers. But this is not explaining why 32 buffers works and 64 doesn't

mtudan commented 3 years ago

Exactly, note that even 8 buffers of 32KB work.

mtudan commented 3 years ago

Closing this since it was related to our pf1550-regulator-rpmsg.c kernel driver which had some changes in order to support BD70528 as well.