Hi Laurent,
On 04/17/2012 02:43 AM, Laurent Pinchart wrote:
> Hi Tomasz,
>
> Thanks for the patch.
>
> On Friday 13 April 2012 17:47:50 Tomasz Stanislawski wrote:
>> From: Andrzej Pietrasiewicz <andrzej.p at samsung.com>
>>
>> This patch introduces usage of dma_map_sg to map memory behind
>> a userspace pointer to a device as dma-contiguous mapping.
>>
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p at samsung.com>
>> Signed-off-by: Marek Szyprowski <m.szyprowski at samsung.com>
>> [bugfixing]
>> Signed-off-by: Kamil Debski <k.debski at samsung.com>
>> [bugfixing]
>> Signed-off-by: Tomasz Stanislawski <t.stanislaws at samsung.com>
>> [add sglist subroutines/code refactoring]
>> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
>> ---
>> drivers/media/video/videobuf2-dma-contig.c | 287 +++++++++++++++++++++++--
>> 1 files changed, 270 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/media/video/videobuf2-dma-contig.c
>> b/drivers/media/video/videobuf2-dma-contig.c index 476e536..3a1e314 100644
>> --- a/drivers/media/video/videobuf2-dma-contig.c
>> +++ b/drivers/media/video/videobuf2-dma-contig.c
>
> [snip]
>
I found out that the function below may be useful for DMABUF
exporters and importers. I found that its code in
'[PATCH 1/4] [RFC] drm/exynos: DMABUF: Added support for exporting non-contig
buffers'
by Prathyush K.
Therefore I posted a patch on linux-media:
'[PATCH] scatterlist: add sg_alloc_table_by_pages function'.
For now I will keep the function below (with your fixes). If
the patch above gets merged then the function will be removed.
Regards,
Tomasz Stanislawski
>> +static struct sg_table *vb2_dc_pages_to_sgt(struct page **pages,
>> + unsigned int n_pages, unsigned long offset, unsigned long size)
>> +{
>> + struct sg_table *sgt;
>> + unsigned int chunks;
>> + unsigned int i;
>> + unsigned int cur_page;
>> + int ret;
>> + struct scatterlist *s;
>> + unsigned int offset_end = n_pages * PAGE_SIZE - size;
>
> Shouldn't offset_end be equal to n_page * PAGE_SIZE - size - offset ?
>
>> + sgt = kzalloc(sizeof *sgt, GFP_KERNEL);
>> + if (!sgt)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + /* compute number of chunks */
>> + chunks = 1;
>> + for (i = 1; i < n_pages; ++i)
>> + if (pages[i] != pages[i - 1] + 1)
>> + ++chunks;
>> +
>> + ret = sg_alloc_table(sgt, chunks, GFP_KERNEL);
>> + if (ret) {
>> + kfree(sgt);
>> + return ERR_PTR(-ENOMEM);
>> + }
>> +
>> + /* merging chunks and putting them into the scatterlist */
>> + cur_page = 0;
>> + for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
>> + size_t size = PAGE_SIZE;
>
> This will shadow the size parameter, it's a bit confusing. You could rename
> it
> chunk_size.
>
>> + unsigned int j;
>> +
>> + for (j = cur_page + 1; j < n_pages; ++j) {
>> + if (pages[j] != pages[j - 1] + 1)
>> + break;
>> + size += PAGE_SIZE;
>> + }
>
>> + /* cut offset if chunk starts at the first page */
>> + if (cur_page == 0)
>> + size -= offset;
>> + /* cut offset_end if chunk ends at the last page */
>> + if (j == n_pages)
>> + size -= offset_end;
>> +
>> + sg_set_page(s, pages[cur_page], size, offset);
>> + offset = 0;
>
> What about just
>
> chunk_size -= offset;
> sg_set_page(s, pages[cur_page], min(size, chunk_size), offset);
> size -= chunk_size;
> offset = 0;
>
> You could then remove the offset_end calculation above.
>
>> + cur_page = j;
>> + }
>> +
>> + return sgt;
>> +}
>
