<br><br>Looks good to me.<div><br></div><div> Tested-by: Fernando Guzman Lugo <<a href="mailto:fernando.lugo@ti.com">fernando.lugo@ti.com</a>></div><div><br></div><div>Regards,</div><div>Fernando.<br><br><div class="gmail_quote">
On Sun, May 20, 2012 at 7:00 AM, Ohad Ben-Cohen <span dir="ltr"><<a href="mailto:ohad@wizery.com" target="_blank">ohad@wizery.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Dynamically allocate the vrings' DMA when the remote processor<br>
is about to be powered on (i.e. when ->find_vqs() is invoked),<br>
and release them as soon as it is powered off (i.e. when ->del_vqs()<br>
is invoked).<br> </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The obvious and immediate benefit is better memory utilization, since<br>
memory for the vrings is now only allocated when the relevant remote<br>
processor is being used.<br>
<br>
Additionally, this approach also makes recovery of a (crashing)<br>
remote processor easier: one just needs to remove the relevant<br>
vdevs, and the entire vrings cleanup takes place automagically.<br>
<br>
Tested-by: Fernando Guzman Lugo <<a href="mailto:fernando.lugo@ti.com">fernando.lugo@ti.com</a>><br>
Signed-off-by: Ohad Ben-Cohen <<a href="mailto:ohad@wizery.com">ohad@wizery.com</a>><br>
---<br>
drivers/remoteproc/remoteproc_core.c | 109 +++++++++++++++---------------<br>
drivers/remoteproc/remoteproc_internal.h | 2 +<br>
drivers/remoteproc/remoteproc_virtio.c | 13 +++-<br>
3 files changed, 67 insertions(+), 57 deletions(-)<br>
<br>
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c<br>
index e756a0d..40e2b2d 100644<br>
--- a/drivers/remoteproc/remoteproc_core.c<br>
+++ b/drivers/remoteproc/remoteproc_core.c<br>
@@ -279,34 +279,17 @@ rproc_load_segments(struct rproc *rproc, const u8 *elf_data, size_t len)<br>
return ret;<br>
}<br>
<br>
-static int<br>
-__rproc_handle_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i)<br>
+int rproc_alloc_vring(struct rproc_vdev *rvdev, int i)<br>
{<br>
struct rproc *rproc = rvdev->rproc;<br>
struct device *dev = rproc->dev;<br>
- struct fw_rsc_vdev_vring *vring = &rsc->vring[i];<br>
+ struct rproc_vring *rvring = &rvdev->vring[i];<br>
dma_addr_t dma;<br>
void *va;<br>
int ret, size, notifyid;<br>
<br>
- dev_dbg(dev, "vdev rsc: vring%d: da %x, qsz %d, align %d\n",<br>
- i, vring->da, vring->num, vring->align);<br>
-<br>
- /* make sure reserved bytes are zeroes */<br>
- if (vring->reserved) {<br>
- dev_err(dev, "vring rsc has non zero reserved bytes\n");<br>
- return -EINVAL;<br>
- }<br>
-<br>
- /* verify queue size and vring alignment are sane */<br>
- if (!vring->num || !vring->align) {<br>
- dev_err(dev, "invalid qsz (%d) or alignment (%d)\n",<br>
- vring->num, vring->align);<br>
- return -EINVAL;<br>
- }<br>
-<br>
/* actual size of vring (in bytes) */<br>
- size = PAGE_ALIGN(vring_size(vring->num, vring->align));<br>
+ size = PAGE_ALIGN(vring_size(rvring->len, rvring->align));<br>
<br>
if (!idr_pre_get(&rproc->notifyids, GFP_KERNEL)) {<br>
dev_err(dev, "idr_pre_get failed\n");<br>
@@ -316,6 +299,7 @@ __rproc_handle_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i)<br>
/*<br>
* Allocate non-cacheable memory for the vring. In the future<br>
* this call will also configure the IOMMU for us<br>
+ * TODO: let the rproc know the da of this vring<br>
*/<br>
va = dma_alloc_coherent(dev, size, &dma, GFP_KERNEL);<br>
if (!va) {<br>
@@ -323,44 +307,67 @@ __rproc_handle_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i)<br>
return -EINVAL;<br>
}<br>
<br>
- /* assign an rproc-wide unique index for this vring */<br>
- /* TODO: assign a notifyid for rvdev updates as well */<br>
- ret = idr_get_new(&rproc->notifyids, &rvdev->vring[i], ¬ifyid);<br>
+ /*<br>
+ * Assign an rproc-wide unique index for this vring<br>
+ * TODO: assign a notifyid for rvdev updates as well<br>
+ * TODO: let the rproc know the notifyid of this vring<br>
+ * TODO: support predefined notifyids (via resource table)<br>
+ */<br>
+ ret = idr_get_new(&rproc->notifyids, rvring, ¬ifyid);<br>
if (ret) {<br>
dev_err(dev, "idr_get_new failed: %d\n", ret);<br>
dma_free_coherent(dev, size, va, dma);<br>
return ret;<br>
}<br>
<br>
- /* let the rproc know the da and notifyid of this vring */<br>
- /* TODO: expose this to remote processor */<br>
- vring->da = dma;<br>
- vring->notifyid = notifyid;<br>
-<br>
dev_dbg(dev, "vring%d: va %p dma %x size %x idr %d\n", i, va,<br>
dma, size, notifyid);<br>
<br>
- rvdev->vring[i].len = vring->num;<br>
- rvdev->vring[i].align = vring->align;<br>
- rvdev->vring[i].va = va;<br>
- rvdev->vring[i].dma = dma;<br>
- rvdev->vring[i].notifyid = notifyid;<br>
- rvdev->vring[i].rvdev = rvdev;<br>
+ rvring->va = va;<br>
+ rvring->dma = dma;<br>
+ rvring->notifyid = notifyid;<br>
<br>
return 0;<br>
}<br>
<br>
-static void __rproc_free_vrings(struct rproc_vdev *rvdev, int i)<br>
+static int<br>
+rproc_parse_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i)<br>
{<br>
struct rproc *rproc = rvdev->rproc;<br>
+ struct device *dev = rproc->dev;<br>
+ struct fw_rsc_vdev_vring *vring = &rsc->vring[i];<br>
+ struct rproc_vring *rvring = &rvdev->vring[i];<br>
<br>
- for (i--; i >= 0; i--) {<br>
- struct rproc_vring *rvring = &rvdev->vring[i];<br>
- int size = PAGE_ALIGN(vring_size(rvring->len, rvring->align));<br>
+ dev_dbg(dev, "vdev rsc: vring%d: da %x, qsz %d, align %d\n",<br>
+ i, vring->da, vring->num, vring->align);<br>
+<br>
+ /* make sure reserved bytes are zeroes */<br>
+ if (vring->reserved) {<br>
+ dev_err(dev, "vring rsc has non zero reserved bytes\n");<br>
+ return -EINVAL;<br>
+ }<br>
<br>
- dma_free_coherent(rproc->dev, size, rvring->va, rvring->dma);<br>
- idr_remove(&rproc->notifyids, rvring->notifyid);<br>
+ /* verify queue size and vring alignment are sane */<br>
+ if (!vring->num || !vring->align) {<br>
+ dev_err(dev, "invalid qsz (%d) or alignment (%d)\n",<br>
+ vring->num, vring->align);<br>
+ return -EINVAL;<br>
}<br>
+<br>
+ rvring->len = vring->num;<br>
+ rvring->align = vring->align;<br>
+ rvring->rvdev = rvdev;<br>
+<br>
+ return 0;<br>
+}<br>
+<br>
+void rproc_free_vring(struct rproc_vring *rvring)<br>
+{<br>
+ int size = PAGE_ALIGN(vring_size(rvring->len, rvring->align));<br>
+ struct rproc *rproc = rvring->rvdev->rproc;<br>
+<br>
+ dma_free_coherent(rproc->dev, size, rvring->va, rvring->dma);<br>
+ idr_remove(&rproc->notifyids, rvring->notifyid);<br>
}<br>
<br>
/**<br>
@@ -425,11 +432,11 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,<br>
<br>
rvdev->rproc = rproc;<br>
<br>
- /* allocate the vrings */<br>
+ /* parse the vrings */<br>
for (i = 0; i < rsc->num_of_vrings; i++) {<br>
- ret = __rproc_handle_vring(rvdev, rsc, i);<br>
+ ret = rproc_parse_vring(rvdev, rsc, i);<br>
if (ret)<br>
- goto free_vrings;<br>
+ goto free_rvdev;<br>
}<br>
<br>
/* remember the device features */<br>
@@ -440,12 +447,11 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,<br>
/* it is now safe to add the virtio device */<br>
ret = rproc_add_virtio_dev(rvdev, rsc->id);<br>
if (ret)<br>
- goto free_vrings;<br>
+ goto free_rvdev;<br>
<br>
return 0;<br>
<br>
-free_vrings:<br>
- __rproc_free_vrings(rvdev, i);<br>
+free_rvdev:<br>
kfree(rvdev);<br>
return ret;<br>
}<br>
@@ -1265,18 +1271,11 @@ EXPORT_SYMBOL(rproc_shutdown);<br>
void rproc_release(struct kref *kref)<br>
{<br>
struct rproc *rproc = container_of(kref, struct rproc, refcount);<br>
- struct rproc_vdev *rvdev, *rvtmp;<br>
<br>
dev_info(rproc->dev, "removing %s\n", rproc->name);<br>
<br>
rproc_delete_debug_dir(rproc);<br>
<br>
- /* clean up remote vdev entries */<br>
- list_for_each_entry_safe(rvdev, rvtmp, &rproc->rvdevs, node) {<br>
- __rproc_free_vrings(rvdev, RVDEV_NUM_VRINGS);<br>
- list_del(&rvdev->node);<br>
- }<br>
-<br>
/*<br>
* At this point no one holds a reference to rproc anymore,<br>
* so we can directly unroll rproc_alloc()<br>
@@ -1547,7 +1546,7 @@ EXPORT_SYMBOL(rproc_free);<br>
*/<br>
int rproc_unregister(struct rproc *rproc)<br>
{<br>
- struct rproc_vdev *rvdev;<br>
+ struct rproc_vdev *rvdev, *tmp;<br>
<br>
if (!rproc)<br>
return -EINVAL;<br>
@@ -1556,7 +1555,7 @@ int rproc_unregister(struct rproc *rproc)<br>
wait_for_completion(&rproc->firmware_loading_complete);<br>
<br>
/* clean up remote vdev entries */<br>
- list_for_each_entry(rvdev, &rproc->rvdevs, node)<br>
+ list_for_each_entry_safe(rvdev, tmp, &rproc->rvdevs, node)<br>
rproc_remove_virtio_dev(rvdev);<br>
<br>
/* the rproc is downref'ed as soon as it's removed from the klist */<br>
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h<br>
index 9f336d6..f4957cf 100644<br>
--- a/drivers/remoteproc/remoteproc_internal.h<br>
+++ b/drivers/remoteproc/remoteproc_internal.h<br>
@@ -41,4 +41,6 @@ void rproc_create_debug_dir(struct rproc *rproc);<br>
void rproc_init_debugfs(void);<br>
void rproc_exit_debugfs(void);<br>
<br>
+void rproc_free_vring(struct rproc_vring *rvring);<br>
+int rproc_alloc_vring(struct rproc_vdev *rvdev, int i);<br>
#endif /* REMOTEPROC_INTERNAL_H */<br>
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c<br>
index ecf6121..26a7144 100644<br>
--- a/drivers/remoteproc/remoteproc_virtio.c<br>
+++ b/drivers/remoteproc/remoteproc_virtio.c<br>
@@ -77,14 +77,17 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev,<br>
struct rproc_vring *rvring;<br>
struct virtqueue *vq;<br>
void *addr;<br>
- int len, size;<br>
+ int len, size, ret;<br>
<br>
/* we're temporarily limited to two virtqueues per rvdev */<br>
if (id >= ARRAY_SIZE(rvdev->vring))<br>
return ERR_PTR(-EINVAL);<br>
<br>
- rvring = &rvdev->vring[id];<br>
+ ret = rproc_alloc_vring(rvdev, id);<br>
+ if (ret)<br>
+ return ERR_PTR(ret);<br>
<br>
+ rvring = &rvdev->vring[id];<br>
addr = rvring->va;<br>
len = rvring->len;<br>
<br>
@@ -103,6 +106,7 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev,<br>
rproc_virtio_notify, callback, name);<br>
if (!vq) {<br>
dev_err(rproc->dev, "vring_new_virtqueue %s failed\n", name);<br>
+ rproc_free_vring(rvring);<br>
return ERR_PTR(-ENOMEM);<br>
}<br>
<br>
@@ -125,6 +129,7 @@ static void rproc_virtio_del_vqs(struct virtio_device *vdev)<br>
rvring = vq->priv;<br>
rvring->vq = NULL;<br>
vring_del_virtqueue(vq);<br>
+ rproc_free_vring(rvring);<br>
}<br>
}<br>
<br>
@@ -228,8 +233,12 @@ static struct virtio_config_ops rproc_virtio_config_ops = {<br>
static void rproc_vdev_release(struct device *dev)<br>
{<br>
struct virtio_device *vdev = dev_to_virtio(dev);<br>
+ struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);<br>
struct rproc *rproc = vdev_to_rproc(vdev);<br>
<br>
+ list_del(&rvdev->node);<br>
+ kfree(rvdev);<br>
+<br>
kref_put(&rproc->refcount, rproc_release);<br>
}<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
1.7.5.4<br>
<br>
--<br>
To unsubscribe from this list: send the line "unsubscribe linux-omap" in<br>
the body of a message to <a href="mailto:majordomo@vger.kernel.org">majordomo@vger.kernel.org</a><br>
More majordomo info at <a href="http://vger.kernel.org/majordomo-info.html" target="_blank">http://vger.kernel.org/majordomo-info.html</a><br>
</font></span></blockquote></div><br></div>