>-----Original Message----- >From: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> >Sent: Tuesday, May 12, 2020 4:34 PM >To: Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>; dri- >devel@xxxxxxxxxxxxxxxxxxxxx; iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; linaro-mm- >sig@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx >Cc: Pawel Osciak <pawel@xxxxxxxxxx>; Bartlomiej Zolnierkiewicz ><b.zolnierkie@xxxxxxxxxxx>; David Airlie <airlied@xxxxxxxx>; linux- >media@xxxxxxxxxxxxxxx; Hans Verkuil <hverkuil-cisco@xxxxxxxxx>; Mauro >Carvalho Chehab <mchehab@xxxxxxxxxx>; Robin Murphy ><robin.murphy@xxxxxxx>; Christoph Hellwig <hch@xxxxxx>; linux-arm- >kernel@xxxxxxxxxxxxxxxxxxx >Subject: Re: [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist >wrappers > >Hi Michael, > >On 12.05.2020 19:52, Ruhl, Michael J wrote: >>> -----Original Message----- >>> From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of >>> Marek Szyprowski >>> Sent: Tuesday, May 12, 2020 5:01 AM >>> To: dri-devel@xxxxxxxxxxxxxxxxxxxxx; iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; >>> linaro-mm-sig@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx >>> Cc: Pawel Osciak <pawel@xxxxxxxxxx>; Bartlomiej Zolnierkiewicz >>> <b.zolnierkie@xxxxxxxxxxx>; David Airlie <airlied@xxxxxxxx>; linux- >>> media@xxxxxxxxxxxxxxx; Hans Verkuil <hverkuil-cisco@xxxxxxxxx>; Mauro >>> Carvalho Chehab <mchehab@xxxxxxxxxx>; Robin Murphy >>> <robin.murphy@xxxxxxx>; Christoph Hellwig <hch@xxxxxx>; linux-arm- >>> kernel@xxxxxxxxxxxxxxxxxxx; Marek Szyprowski >>> <m.szyprowski@xxxxxxxxxxx> >>> Subject: [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist >wrappers >>> >>> Use recently introduced common wrappers operating directly on the struct >>> sg_table objects and scatterlist page iterators to make the code a bit >>> more compact, robust, easier to follow and copy/paste safe. >>> >>> No functional change, because the code already properly did all the >>> scaterlist related calls. >>> >>> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> >>> --- >>> For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents >>> vs. orig_nents misuse' thread: >>> https://lore.kernel.org/dri-devel/20200512085710.14688-1- >>> m.szyprowski@xxxxxxxxxxx/T/ >>> --- >>> .../media/common/videobuf2/videobuf2-dma-contig.c | 41 ++++++++++- >--- >>> -------- >>> drivers/media/common/videobuf2/videobuf2-dma-sg.c | 32 +++++++----- >--- >>> -- >>> drivers/media/common/videobuf2/videobuf2-vmalloc.c | 12 +++---- >>> 3 files changed, 34 insertions(+), 51 deletions(-) >>> >>> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c >>> b/drivers/media/common/videobuf2/videobuf2-dma-contig.c >>> index d3a3ee5..bf31a9d 100644 >>> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c >>> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c >>> @@ -48,16 +48,15 @@ struct vb2_dc_buf { >>> >>> static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt) >>> { >>> - struct scatterlist *s; >>> dma_addr_t expected = sg_dma_address(sgt->sgl); >>> - unsigned int i; >>> + struct sg_dma_page_iter dma_iter; >>> unsigned long size = 0; >>> >>> - for_each_sg(sgt->sgl, s, sgt->nents, i) { >>> - if (sg_dma_address(s) != expected) >>> + for_each_sgtable_dma_page(sgt, &dma_iter, 0) { >>> + if (sg_page_iter_dma_address(&dma_iter) != expected) >>> break; >>> - expected = sg_dma_address(s) + sg_dma_len(s); >>> - size += sg_dma_len(s); >>> + expected += PAGE_SIZE; >>> + size += PAGE_SIZE; >> This code in drm_prime_t_contiguous_size and here. I seem to remember >seeing >> the same pattern in other drivers. >> >> Would it worthwhile to make this a helper as well? >I think I've identified such patterns in all DRM drivers and replaced >with a common helper. So far I have no idea where to put such helper to >make it available for media/videobuf2, so those a few lines are indeed >duplicated here. I was thinking of drivers outside of DRM/media. Specifically RDMA. However, looking at that code, I see that my memory was a little off. It is working with continuous pages, but not finding the size. >> Also, isn't the sg_dma_len() the actual length of the chunk we are looking >at? >> >> If its I not PAGE_SIZE (ie. dma chunk is 4 * PAGE_SIZE?), does your >loop/calculation still work? > >scaterlist page iterators (for_each_sg_page/for_each_sg_dma_page and >their sgtable variants) always operates on PAGE_SIZE units. They >correctly handle larger sg_dma_len(). Ahh, ok, I see. Thank you! Mike > >Best regards >-- >Marek Szyprowski, PhD >Samsung R&D Institute Poland
