This patch adds another constructor for an sg table. An sg table is created from an existing sg table. The new sg table is allocated and initialized with same data from the original sg table. The user has to call 'sg_clone_table' with the required sg table, the existing sg table and the gfp allocation mask.
This function can be used in the dma-buf framework. If a buffer needs to be shared across multiple devices using the dma_buf framework, an sg table needs to be created and mapped to the target device's address space. This is done in most drivers by creating the sg table from the pages of the buffer (e.g. calling sg_alloc_table_from_pages function). In case this needs to be done frequently (e.g. a framebuffer is repeatedly shared with the GPU, video accelerator, CSC etc), it is efficient to create an sg table once during buffer allocation and then create temporary sg tables for dma mapping from the original sg table.
Signed-off-by: Prathyush K prathyush.k@samsung.com --- include/linux/scatterlist.h | 1 + lib/scatterlist.c | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 4bd6c06..fd12525 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -215,6 +215,7 @@ void sg_free_table(struct sg_table *); int __sg_alloc_table(struct sg_table *, unsigned int, unsigned int, gfp_t, sg_alloc_fn *); int sg_alloc_table(struct sg_table *, unsigned int, gfp_t); +int sg_clone_table(struct sg_table *, struct sg_table *, gfp_t); int sg_alloc_table_from_pages(struct sg_table *sgt, struct page **pages, unsigned int n_pages, unsigned long offset, unsigned long size, diff --git a/lib/scatterlist.c b/lib/scatterlist.c index 3675452..4f106b3 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -329,6 +329,42 @@ int sg_alloc_table(struct sg_table *table, unsigned int nents, gfp_t gfp_mask) } EXPORT_SYMBOL(sg_alloc_table);
+/* + * sg_clone_table - Allocate and initialize an sg table from an existing + * sg table + * @sgt_in: The sg table to clone + * @sgt_out: The output sg table cloned from the original sg table. + * @gfp_mask: GFP allocation mask + * + * Description: + * Allocate and initialize an sg table from an existing sg table. A user + * would want to create a temporary sg table which is a clone of an + * existing table. This cloned sg table is released by sg_free_table. + * + * Returns: + * 0 on success, negative error on failure + */ +int sg_clone_table(struct sg_table *sgt_in, struct sg_table *sgt_out, + gfp_t gfp_mask) +{ + struct scatterlist *s, *s_out; + unsigned int i; + int ret; + + ret = sg_alloc_table(sgt_out, sgt_in->orig_nents, gfp_mask); + if (unlikely(ret)) + return ret; + + s_out = sgt_out->sgl; + for_each_sg(sgt_in->sgl, s, sgt_in->orig_nents, i) { + sg_set_page(s_out, sg_page(s), s->length, s->offset); + s_out = sg_next(s_out); + } + + return 0; +} +EXPORT_SYMBOL(sg_clone_table); + /** * sg_alloc_table_from_pages - Allocate and initialize an sg table from * an array of pages
The function dma_get_sgtable will allocate a sg table internally so it is not necessary to allocate a sg table before it. The unnecessary 'sg_alloc_table' call is removed.
Signed-off-by: Prathyush K prathyush.k@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 8 +------- 1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c index b98da30..d9307bd 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c @@ -40,21 +40,15 @@ static struct sg_table *exynos_get_sgt(struct drm_device *drm_dev, if (!sgt) goto out;
- ret = sg_alloc_table(sgt, buf->sgt->nents, GFP_KERNEL); - if (ret) - goto err_free_sgt; - ret = dma_get_sgtable(drm_dev->dev, sgt, buf->kvaddr, buf->dma_addr, buf->size); if (ret < 0) { DRM_ERROR("failed to get sgtable.\n"); - goto err_free_table; + goto err_free_sgt; }
return sgt;
-err_free_table: - sg_free_table(sgt); err_free_sgt: kfree(sgt); sgt = NULL;
2012/11/7 Prathyush K prathyush.k@samsung.com
The function dma_get_sgtable will allocate a sg table internally so it is not necessary to allocate a sg table before it. The unnecessary 'sg_alloc_table' call is removed.
Signed-off-by: Prathyush K prathyush.k@samsung.com
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 8 +------- 1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c index b98da30..d9307bd 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c @@ -40,21 +40,15 @@ static struct sg_table *exynos_get_sgt(struct drm_device *drm_dev, if (!sgt) goto out;
ret = sg_alloc_table(sgt, buf->sgt->nents, GFP_KERNEL);
if (ret)
goto err_free_sgt;
It's good catch. Right, I missed it. dma_get_sgtable function also calls sg_alloc_table.
ret = dma_get_sgtable(drm_dev->dev, sgt, buf->kvaddr, buf->dma_addr, buf->size); if (ret < 0) { DRM_ERROR("failed to get sgtable.\n");
goto err_free_table;
goto err_free_sgt; } return sgt;
-err_free_table:
sg_free_table(sgt);
err_free_sgt: kfree(sgt); sgt = NULL; -- 1.7.0.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Applied. And it seems like that another one needs some review So will let me pick it up later after review.
Thanks, Inki Dae
2012/11/8 Inki Dae inki.dae@samsung.com
2012/11/7 Prathyush K prathyush.k@samsung.com
The function dma_get_sgtable will allocate a sg table internally so it is not necessary to allocate a sg table before it. The unnecessary 'sg_alloc_table' call is removed.
Signed-off-by: Prathyush K prathyush.k@samsung.com
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 8 +------- 1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c index b98da30..d9307bd 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c @@ -40,21 +40,15 @@ static struct sg_table *exynos_get_sgt(struct drm_device *drm_dev, if (!sgt) goto out;
ret = sg_alloc_table(sgt, buf->sgt->nents, GFP_KERNEL);
if (ret)
goto err_free_sgt;
It's good catch. Right, I missed it. dma_get_sgtable function also calls sg_alloc_table.
ret = dma_get_sgtable(drm_dev->dev, sgt, buf->kvaddr, buf->dma_addr, buf->size); if (ret < 0) { DRM_ERROR("failed to get sgtable.\n");
goto err_free_table;
goto err_free_sgt; } return sgt;
-err_free_table:
sg_free_table(sgt);
err_free_sgt: kfree(sgt); sgt = NULL; -- 1.7.0.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
During map_dma_buf, a new sgt needs to be created before being mapped to another device's address space. Currently, this sgt is created from the pages of the gem buffer everytime by calling dma_get_sgtable. This will be time consuming if the map/unmap calls are done repeatedly for very large buffers.
Instead, we can just clone the sgt which was already created during buffer allocation and create the new sgt. This is done by calling 'sg_clone_table'. This will be much faster than creating from pages.
Signed-off-by: Prathyush K prathyush.k@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c index d9307bd..56efdd5 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c @@ -40,10 +40,14 @@ static struct sg_table *exynos_get_sgt(struct drm_device *drm_dev, if (!sgt) goto out;
- ret = dma_get_sgtable(drm_dev->dev, sgt, buf->kvaddr, - buf->dma_addr, buf->size); + if (!buf->sgt) { + DRM_ERROR("sgt is null.\n"); + goto err_free_sgt; + } + + ret = sg_clone_table(buf->sgt, sgt, GFP_KERNEL); if (ret < 0) { - DRM_ERROR("failed to get sgtable.\n"); + DRM_ERROR("failed to clone sgtable.\n"); goto err_free_sgt; }
2012/11/7 Prathyush K prathyush.k@samsung.com
During map_dma_buf, a new sgt needs to be created before being mapped to another device's address space. Currently, this sgt is created from the pages of the gem buffer everytime by calling dma_get_sgtable. This will be time consuming if the map/unmap calls are done repeatedly for very large buffers.
Instead, we can just clone the sgt which was already created during buffer allocation and create the new sgt. This is done by calling 'sg_clone_table'. This will be much faster than creating from pages.
Signed-off-by: Prathyush K prathyush.k@samsung.com
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c index d9307bd..56efdd5 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c @@ -40,10 +40,14 @@ static struct sg_table *exynos_get_sgt(struct drm_device *drm_dev, if (!sgt) goto out;
ret = dma_get_sgtable(drm_dev->dev, sgt, buf->kvaddr,
buf->dma_addr, buf->size);
if (!buf->sgt) {
DRM_ERROR("sgt is null.\n");
goto err_free_sgt;
}
ret = sg_clone_table(buf->sgt, sgt, GFP_KERNEL);
I think it's good idea but may need review enough. Let me sleep on it.
Thanks, Inki Dae
if (ret < 0) {
DRM_ERROR("failed to get sgtable.\n");
DRM_ERROR("failed to clone sgtable.\n"); goto err_free_sgt; }
-- 1.7.0.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org