From 749d93449da7382ade95020363a74b99a69f6711 Mon Sep 17 00:00:00 2001 From: Yongrong Wang Date: Tue, 30 Jan 2024 16:08:58 +0800 Subject: [PATCH 1/2] Revert "openamp: add new ops wait_notified() to avoid looping in tx buffer get" This reverts commit 95802c1d0328d2d0fe97fc744aa213eb8bc0e466. Signed-off-by: Yongrong Wang --- lib/include/openamp/remoteproc.h | 12 ---------- lib/include/openamp/remoteproc_virtio.h | 23 -------------------- lib/include/openamp/rpmsg.h | 1 - lib/include/openamp/virtio.h | 3 --- lib/remoteproc/remoteproc.c | 11 ---------- lib/remoteproc/remoteproc_virtio.c | 29 ------------------------- lib/rpmsg/rpmsg_virtio.c | 20 ++--------------- 7 files changed, 2 insertions(+), 97 deletions(-) diff --git a/lib/include/openamp/remoteproc.h b/lib/include/openamp/remoteproc.h index d7fb8338b..23f917861 100644 --- a/lib/include/openamp/remoteproc.h +++ b/lib/include/openamp/remoteproc.h @@ -462,17 +462,6 @@ struct remoteproc_ops { /** Notify the remote */ int (*notify)(struct remoteproc *rproc, uint32_t id); - /** - * @brief Wait for remote notified, when there is no TX buffer anymore. - * Set to NULL means use usleep to wait TX buffer available. - * - * @param rproc pointer to remoteproc instance - * @param id the notifyid - * - * return 0 means there is notify available, otherwise negative value. - */ - int (*wait_notified)(struct remoteproc *rproc, uint32_t id); - /** * @brief Get remoteproc memory I/O region by either name, virtual * address, physical address or device address. @@ -508,7 +497,6 @@ struct remoteproc_ops { #define RPROC_ERR_RSC_TAB_NP (RPROC_EBASE + 10) #define RPROC_ERR_RSC_TAB_NS (RPROC_EBASE + 11) #define RPROC_ERR_LOADER_STATE (RPROC_EBASE + 12) -#define RPROC_EOPNOTSUPP (RPROC_EBASE + 13) #define RPROC_EMAX (RPROC_EBASE + 16) #define RPROC_EPTR (void *)(-1) #define RPROC_EOF (void *)(-1) diff --git a/lib/include/openamp/remoteproc_virtio.h b/lib/include/openamp/remoteproc_virtio.h index eaa8dd37a..0b747cacc 100644 --- a/lib/include/openamp/remoteproc_virtio.h +++ b/lib/include/openamp/remoteproc_virtio.h @@ -39,12 +39,6 @@ extern "C" { /* define vdev notification function user should implement */ typedef int (*rpvdev_notify_func)(void *priv, uint32_t id); -/* - * Define vdev wait notified function, user can implement this - * function to wait available Tx buffers. - */ -typedef int (*rpvdev_wait_notified_func)(void *priv, uint32_t id); - /** @brief Virtio structure for remoteproc instance */ struct remoteproc_virtio { /** Pointer to private data */ @@ -59,9 +53,6 @@ struct remoteproc_virtio { /** Notification function */ rpvdev_notify_func notify; - /** Wait notification function (optional) */ - rpvdev_wait_notified_func wait_notified; - /** Virtio device */ struct virtio_device vdev; @@ -135,20 +126,6 @@ int rproc_virtio_notified(struct virtio_device *vdev, uint32_t notifyid); */ void rproc_virtio_wait_remote_ready(struct virtio_device *vdev); -/** - * @brief Set the remoteproc virtio wait notified function. - * - * This \ref wait_notified_cb function will be called to customize the wait, when - * no Tx buffer is available. - * - * @param vdev Pointer to the virtio device. - * @param wait_notified_cb The wait notified callback function. - * - * @return 0 for successful, negative value for failure. - */ -int rproc_virtio_set_wait_notified(struct virtio_device *vdev, - rpvdev_wait_notified_func wait_notified_cb); - #if defined __cplusplus } #endif diff --git a/lib/include/openamp/rpmsg.h b/lib/include/openamp/rpmsg.h index 82371ea77..17eaddec2 100644 --- a/lib/include/openamp/rpmsg.h +++ b/lib/include/openamp/rpmsg.h @@ -43,7 +43,6 @@ extern "C" { #define RPMSG_ERR_INIT (RPMSG_ERROR_BASE - 6) #define RPMSG_ERR_ADDR (RPMSG_ERROR_BASE - 7) #define RPMSG_ERR_PERM (RPMSG_ERROR_BASE - 8) -#define RPMSG_EOPNOTSUPP (RPMSG_ERROR_BASE - 9) struct rpmsg_endpoint; struct rpmsg_device; diff --git a/lib/include/openamp/virtio.h b/lib/include/openamp/virtio.h index 2829ab7ca..9febd1eaf 100644 --- a/lib/include/openamp/virtio.h +++ b/lib/include/openamp/virtio.h @@ -275,9 +275,6 @@ struct virtio_dispatch { /** Notify the other side that a virtio vring as been updated. */ void (*notify)(struct virtqueue *vq); - - /** Customize the wait, when no Tx buffer is available (optional) */ - int (*wait_notified)(struct virtio_device *dev, struct virtqueue *vq); }; /** diff --git a/lib/remoteproc/remoteproc.c b/lib/remoteproc/remoteproc.c index a0df0bb1a..daea80328 100644 --- a/lib/remoteproc/remoteproc.c +++ b/lib/remoteproc/remoteproc.c @@ -901,16 +901,6 @@ static int remoteproc_virtio_notify(void *priv, uint32_t id) return 0; } -static int remoteproc_wait_notified(void *priv, uint32_t id) -{ - struct remoteproc *rproc = priv; - - if (rproc->ops->wait_notified) - return rproc->ops->wait_notified(rproc, id); - - return -RPROC_EOPNOTSUPP; -} - struct virtio_device * remoteproc_create_virtio(struct remoteproc *rproc, int vdev_id, unsigned int role, @@ -969,7 +959,6 @@ remoteproc_create_virtio(struct remoteproc *rproc, rproc_virtio_wait_remote_ready(vdev); rpvdev = metal_container_of(vdev, struct remoteproc_virtio, vdev); - rproc_virtio_set_wait_notified(vdev, remoteproc_wait_notified); metal_list_add_tail(&rproc->vdevs, &rpvdev->node); num_vrings = vdev_rsc->num_of_vrings; diff --git a/lib/remoteproc/remoteproc_virtio.c b/lib/remoteproc/remoteproc_virtio.c index f6edd7450..bab277061 100644 --- a/lib/remoteproc/remoteproc_virtio.c +++ b/lib/remoteproc/remoteproc_virtio.c @@ -30,21 +30,6 @@ static void rproc_virtio_virtqueue_notify(struct virtqueue *vq) rpvdev->notify(rpvdev->priv, vring_info->notifyid); } -static int rproc_virtio_wait_notified(struct virtio_device *vdev, - struct virtqueue *vq) -{ - struct remoteproc_virtio *rpvdev; - struct virtio_vring_info *vring_info; - unsigned int vq_id = vq->vq_queue_index; - - rpvdev = metal_container_of(vdev, struct remoteproc_virtio, vdev); - vring_info = &vdev->vrings_info[vq_id]; - - return rpvdev->wait_notified ? - rpvdev->wait_notified(rpvdev->priv, vring_info->notifyid) : - -RPROC_EOPNOTSUPP; -} - static unsigned char rproc_virtio_get_status(struct virtio_device *vdev) { struct remoteproc_virtio *rpvdev; @@ -202,7 +187,6 @@ static const struct virtio_dispatch remoteproc_virtio_dispatch_funcs = { .get_features = rproc_virtio_get_features, .read_config = rproc_virtio_read_config, .notify = rproc_virtio_virtqueue_notify, - .wait_notified = rproc_virtio_wait_notified, #ifndef VIRTIO_DEVICE_ONLY /* * We suppose here that the vdev is in a shared memory so that can @@ -381,16 +365,3 @@ void rproc_virtio_wait_remote_ready(struct virtio_device *vdev) metal_cpu_yield(); } } - -int rproc_virtio_set_wait_notified(struct virtio_device *vdev, - rpvdev_wait_notified_func wait_notified_cb) -{ - struct remoteproc_virtio *rpvdev; - - if (!vdev || !wait_notified_cb) - return -RPROC_EINVAL; - rpvdev = metal_container_of(vdev, struct remoteproc_virtio, vdev); - rpvdev->wait_notified = wait_notified_cb; - - return 0; -} diff --git a/lib/rpmsg/rpmsg_virtio.c b/lib/rpmsg/rpmsg_virtio.c index 1384542e4..783b76b0d 100644 --- a/lib/rpmsg/rpmsg_virtio.c +++ b/lib/rpmsg/rpmsg_virtio.c @@ -358,14 +358,6 @@ static void rpmsg_virtio_release_rx_buffer(struct rpmsg_device *rdev, metal_mutex_release(&rdev->lock); } -static int rpmsg_virtio_wait_notified(struct rpmsg_virtio_device *rvdev, - struct virtqueue *vq) -{ - return rvdev->vdev->func->wait_notified ? - rvdev->vdev->func->wait_notified(rvdev->vdev, vq) : - RPMSG_EOPNOTSUPP; -} - static void *rpmsg_virtio_get_tx_payload_buffer(struct rpmsg_device *rdev, uint32_t *len, int wait) { @@ -395,16 +387,8 @@ static void *rpmsg_virtio_get_tx_payload_buffer(struct rpmsg_device *rdev, metal_mutex_release(&rdev->lock); if (rp_hdr || !tick_count) break; - - /* - * Try to use wait loop implemented in the virtio dispatcher and - * use metal_sleep_usec() method by default. - */ - status = rpmsg_virtio_wait_notified(rvdev, rvdev->rvq); - if (status != RPMSG_SUCCESS) { - metal_sleep_usec(RPMSG_TICKS_PER_INTERVAL); - tick_count--; - } + metal_sleep_usec(RPMSG_TICKS_PER_INTERVAL); + tick_count--; } if (!rp_hdr) From 8043658dd7550aa705b6f45cef03918c6517f25b Mon Sep 17 00:00:00 2001 From: Yongrong Wang Date: Tue, 23 Jan 2024 10:45:25 +0800 Subject: [PATCH 2/2] openamp: add notify_wait_cb to avoid looping in tx buffer get Give users a chance to handle the no tx buffer situation when get tx buffer, with this patch, user can set the notify_wait_cb in driver and this callback function will be called to handle the wait when no tx buffer in tx virtqueue. Signed-off-by: Yongrong Wang --- lib/include/openamp/rpmsg.h | 1 + lib/include/openamp/rpmsg_virtio.h | 9 +++++++++ lib/rpmsg/rpmsg_virtio.c | 26 ++++++++++++++++++++++++-- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/lib/include/openamp/rpmsg.h b/lib/include/openamp/rpmsg.h index 17eaddec2..82371ea77 100644 --- a/lib/include/openamp/rpmsg.h +++ b/lib/include/openamp/rpmsg.h @@ -43,6 +43,7 @@ extern "C" { #define RPMSG_ERR_INIT (RPMSG_ERROR_BASE - 6) #define RPMSG_ERR_ADDR (RPMSG_ERROR_BASE - 7) #define RPMSG_ERR_PERM (RPMSG_ERROR_BASE - 8) +#define RPMSG_EOPNOTSUPP (RPMSG_ERROR_BASE - 9) struct rpmsg_endpoint; struct rpmsg_device; diff --git a/lib/include/openamp/rpmsg_virtio.h b/lib/include/openamp/rpmsg_virtio.h index ae2446638..dcc8422d1 100644 --- a/lib/include/openamp/rpmsg_virtio.h +++ b/lib/include/openamp/rpmsg_virtio.h @@ -41,6 +41,9 @@ extern "C" { #define BUFFER_INVALIDATE(x, s) do { } while (0) #endif /* VIRTIO_CACHED_BUFFERS || VIRTIO_USE_DCACHE */ +/* Callback handler for rpmsg virtio service */ +typedef int (*rpmsg_virtio_notify_wait_cb)(struct rpmsg_device *rdev, uint32_t id); + /** @brief Shared memory pool used for RPMsg buffers */ struct rpmsg_virtio_shm_pool { /** Base address of the memory pool */ @@ -98,6 +101,12 @@ struct rpmsg_virtio_device { * \ref rpmsg_virtio_release_tx_buffer function */ struct metal_list reclaimer; + + /** + * Callback handler for rpmsg virtio service, called when service + * can't get tx buffer + */ + rpmsg_virtio_notify_wait_cb notify_wait_cb; }; #define RPMSG_REMOTE VIRTIO_DEV_DEVICE diff --git a/lib/rpmsg/rpmsg_virtio.c b/lib/rpmsg/rpmsg_virtio.c index 783b76b0d..b0eae7e0d 100644 --- a/lib/rpmsg/rpmsg_virtio.c +++ b/lib/rpmsg/rpmsg_virtio.c @@ -358,6 +358,18 @@ static void rpmsg_virtio_release_rx_buffer(struct rpmsg_device *rdev, metal_mutex_release(&rdev->lock); } +static int rpmsg_virtio_notify_wait(struct rpmsg_virtio_device *rvdev, struct virtqueue *vq) +{ + struct virtio_vring_info *vring_info; + + vring_info = &rvdev->vdev->vrings_info[vq->vq_queue_index]; + + if (!rvdev->notify_wait_cb) + return RPMSG_EOPNOTSUPP; + + return rvdev->notify_wait_cb(&rvdev->rdev, vring_info->notifyid); +} + static void *rpmsg_virtio_get_tx_payload_buffer(struct rpmsg_device *rdev, uint32_t *len, int wait) { @@ -387,8 +399,18 @@ static void *rpmsg_virtio_get_tx_payload_buffer(struct rpmsg_device *rdev, metal_mutex_release(&rdev->lock); if (rp_hdr || !tick_count) break; - metal_sleep_usec(RPMSG_TICKS_PER_INTERVAL); - tick_count--; + + /* + * Try to use wait loop implemented in the virtio dispatcher and + * use metal_sleep_usec() method by default. + */ + status = rpmsg_virtio_notify_wait(rvdev, rvdev->rvq); + if (status == RPMSG_EOPNOTSUPP) { + metal_sleep_usec(RPMSG_TICKS_PER_INTERVAL); + tick_count--; + } else if (status == RPMSG_SUCCESS) { + break; + } } if (!rp_hdr)