lkml.org 
[lkml]   [2015]   [Feb]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/3] media/videobuf2-dma-contig: Fix handling of sg_table structure
Hello,

On 2015-02-09 17:14, Ricardo Ribalda Delgado wrote:
> when sg_alloc_table_from_pages() does not fail it returns a sg_table
> structure with nents and nents_orig initialized to the same value.
>
> dma_map_sg returns the dma_map_sg returns the number of areas mapped
> by the hardware, which could be different than the areas given as an input.
> The output must be saved to nent.
> Unfortunately nent differs in sign to the output of dma_map_sg, so an
> intermediate value must be used.
>
> The output of dma_map, should be used to transverse the scatter list.
>
> dma_unmap_sg needs the value passed to dma_map_sg (nents_orig).
>
> sg_free_tables uses also orig_nent.
>
> This patch fix the file to follow this paradigm.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> drivers/media/v4l2-core/videobuf2-dma-contig.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> index b481d20..c7e4bdd 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -56,7 +56,7 @@ static void vb2_dc_sgt_foreach_page(struct sg_table *sgt,
> struct scatterlist *s;
> unsigned int i;
>
> - for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
> + for_each_sg(sgt->sgl, s, sgt->nents, i) {
> struct page *page = sg_page(s);
> unsigned int n_pages = PAGE_ALIGN(s->offset + s->length)
> >> PAGE_SHIFT;

This code iterates over memory pages added to the scatter list not the
dma chunks,
so orig_nents must be used. This change is not needed.

> @@ -260,7 +260,7 @@ static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
>
> rd = buf->sgt_base->sgl;
> wr = sgt->sgl;
> - for (i = 0; i < sgt->orig_nents; ++i) {
> + for (i = 0; i < sgt->nents; ++i) {
> sg_set_page(wr, sg_page(rd), rd->length, rd->offset);
> rd = sg_next(rd);
> wr = sg_next(wr);

Same comment as for videobuf2-dma-sg.c patch.

> @@ -324,6 +324,7 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
> mutex_unlock(lock);
> return ERR_PTR(-EIO);
> }
> + sgt->nents = ret;
>
> attach->dma_dir = dma_dir;
>
> @@ -669,13 +670,14 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
> * No need to sync to the device, this will happen later when the
> * prepare() memop is called.
> */
> - sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
> + ret = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
> buf->dma_dir, &attrs);
> - if (sgt->nents <= 0) {
> + if (ret <= 0) {
> pr_err("failed to map scatterlist\n");
> ret = -EIO;
> goto fail_sgt_init;
> }
> + sgt->nents = ret;

This one is okay, although the check for error could be simplified to a
check
for zero value.

>
> contig_size = vb2_dc_get_contiguous_size(sgt);
> if (contig_size < size) {

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland



\
 
 \ /
  Last update: 2015-02-11 09:21    [W:0.062 / U:0.828 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site