Hello,
Thanks for your patch.
Instead of having this in Kconfig, It's better to have buffer size decided dynamically. Probably by resource-table.
We still need implementation that achieves that goal. Meanwhile I think it's best to keep buffer size fixed.
Thanks.
On 9/28/23 10:38 AM, Divin Raj wrote:
> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
>
>
> From: Peter Hoyes <Peter.Hoyes@xxxxxxx>
>
> Replace the MAX_RPMSG_BUF_SIZE and MAX_RPMSG_NUM_BUFS #define in
> virtio_rpmsg_bus.c with the Kconfig parameters CONFIG_RPMSG_VIRTIO_BUF_SIZE
> and CONFIG_RPMSG_VIRTIO_MAX_NUM_BUFS, allowing user-provided customization.
>
> Making both the number of buffers and size configurable facilitates aligning
> memory requirements between vdev-buffer and vdev-vrings for client drivers
> that require larger buffer sizes, for example.
>
> Signed-off-by: Peter Hoyes <Peter.Hoyes@xxxxxxx>
> Signed-off-by: Divin Raj <divin.raj@xxxxxxx>
> ---
> drivers/rpmsg/Kconfig | 23 +++++++++++++++++++++++
> drivers/rpmsg/virtio_rpmsg_bus.c | 27 +++------------------------
> 2 files changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
> index d3795860f5c0..677f4a1ac8bb 100644
> --- a/drivers/rpmsg/Kconfig
> +++ b/drivers/rpmsg/Kconfig
> @@ -81,4 +81,27 @@ config RPMSG_VIRTIO
> select RPMSG_NS
> select VIRTIO
>
> +config RPMSG_VIRTIO_MAX_BUF_SIZE
> + int "Virtio RPMSG max buffer size (in bytes)"
> + default 512
> + depends on RPMSG_VIRTIO
> + help
> + This option allows you to configure the maximum buffer size (in bytes)
> + for Virtio RPMSG communications. The number of buffers will be computed
> + based on the number of buffers (CONFIG_RPMSG_VIRTIO_MAX_NUM_BUFS)
> + supported by the vring. By default, it supports up to a maximum of 512
> + buffers (256 in each direction). Each buffer consists of 16 bytes for the
> + message header and the remaining bytes for the payload.The default values
> + will utilize a maximum total space of 256KB for the buffers.
> +
> +config RPMSG_VIRTIO_MAX_NUM_BUFS
> + int "Virtio RPMSG max buffer count (even number for TX and Rx)"
> + default 512
> + depends on RPMSG_VIRTIO
> + help
> + This option allows you to configure the maximum number of buffers used
> + for Virtio RPMSG communication. By default, it supports up to a maximum
> + of 512 buffers (256 in each direction). Please note that this value
> + should be an even number, as it accounts for both transmit (TX) and
> + receive (Rx) buffers.
> endmenu
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> index 905ac7910c98..87a9a4fa30e0 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -109,27 +109,6 @@ struct virtio_rpmsg_channel {
> #define to_virtio_rpmsg_channel(_rpdev) \
> container_of(_rpdev, struct virtio_rpmsg_channel, rpdev)
>
> -/*
> - * We're allocating buffers of 512 bytes each for communications. The
> - * number of buffers will be computed from the number of buffers supported
> - * by the vring, upto a maximum of 512 buffers (256 in each direction).
> - *
> - * Each buffer will have 16 bytes for the msg header and 496 bytes for
> - * the payload.
> - *
> - * This will utilize a maximum total space of 256KB for the buffers.
> - *
> - * We might also want to add support for user-provided buffers in time.
> - * This will allow bigger buffer size flexibility, and can also be used
> - * to achieve zero-copy messaging.
> - *
> - * Note that these numbers are purely a decision of this driver - we
> - * can change this without changing anything in the firmware of the remote
> - * processor.
> - */
> -#define MAX_RPMSG_NUM_BUFS (512)
> -#define MAX_RPMSG_BUF_SIZE (512)
> -
> /*
> * Local addresses are dynamically allocated on-demand.
> * We do not dynamically assign addresses from the low 1024 range,
> @@ -902,12 +881,12 @@ static int rpmsg_probe(struct virtio_device *vdev)
> virtqueue_get_vring_size(vrp->svq));
>
> /* we need less buffers if vrings are small */
> - if (virtqueue_get_vring_size(vrp->rvq) < MAX_RPMSG_NUM_BUFS / 2)
> + if (virtqueue_get_vring_size(vrp->rvq) < CONFIG_RPMSG_VIRTIO_MAX_NUM_BUFS / 2)
> vrp->num_bufs = virtqueue_get_vring_size(vrp->rvq) * 2;
> else
> - vrp->num_bufs = MAX_RPMSG_NUM_BUFS;
> + vrp->num_bufs = CONFIG_RPMSG_VIRTIO_MAX_NUM_BUFS;
>
> - vrp->buf_size = MAX_RPMSG_BUF_SIZE;
> + vrp->buf_size = CONFIG_RPMSG_VIRTIO_MAX_BUF_SIZE;
>
> total_buf_space = vrp->num_bufs * vrp->buf_size;
>
> --
> 2.25.1
>
