Dear All,
During the Exynos DRM GEM rework and fixing the issues in the. drm_prime_sg_to_page_addr_arrays() function [1] I've noticed that most drivers in DRM framework incorrectly use nents and orig_nents entries of the struct sg_table.
In case of the most DMA-mapping implementations exchanging those two entries or using nents for all loops on the scatterlist is harmless, because they both have the same value. There exists however a DMA-mapping implementations, for which such incorrect usage breaks things. The nents returned by dma_map_sg() might be lower than the nents passed as its parameter and this is perfectly fine. DMA framework or IOMMU is allowed to join consecutive chunks while mapping if such operation is supported by the underlying HW (bus, bridge, IOMMU, etc). Example of the case where dma_map_sg() might return 1 'DMA' chunk for the 4 'physical' pages is described here [2]
The DMA-mapping framework documentation [3] states that dma_map_sg() returns the numer of the created entries in the DMA address space. However the subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be called with the original number of entries passed to dma_map_sg. The common pattern in DRM drivers were to assign the dma_map_sg() return value to sg_table->nents and use that value for the subsequent calls to dma_sync_sg_* or dma_unmap_sg functions. Also the code iterated over nents times to access the pages stored in the processed scatterlist, while it should use orig_nents as the numer of the page entries.
I've tried to identify all such incorrect usage of sg_table->nents and this is a result of my research. It looks that the incorrect pattern has been copied over the many drivers mainly in the DRM subsystem. Too bad in most cases it even worked correctly if the system used a simple, linear DMA-mapping implementation, for which swapping nents and orig_nents doesn't make any difference. To avoid similar issues in the future, I've introduced a common wrappers for DMA-mapping calls, which operate directly on the sg_table objects. I've also added wrappers for iterating over the scatterlists stored in the sg_table objects and applied them where possible. This, together with some common DRM prime helpers, allowed me to almost get rid of all nents/orig_nents usage in the drivers. I hope that such change makes the code robust, easier to follow and copy/paste safe.
The biggest TODO is DRM/i915 driver and I don't feel brave enough to fix it fully. The driver creatively uses sg_table->orig_nents to store the size of the allocate scatterlist and ignores the number of the entries returned by dma_map_sg function. In this patchset I only fixed the sg_table objects exported by dmabuf related functions. I hope that I didn't break anything there.
Patches are based on top of Linux next-20200618. The required changes to DMA-mapping framework has been already merged to v5.8-rc1.
If possible I would like ask for merging most of the patches via DRM tree.
Best regards, Marek Szyprowski
References:
[1] https://lkml.org/lkml/2020/3/27/555 [2] https://lkml.org/lkml/2020/3/29/65 [3] Documentation/DMA-API-HOWTO.txt [4] https://lore.kernel.org/linux-iommu/20200512121931.GD20393@lst.de/T/#ma18c95...
Changelog:
v6: - rebased onto Linux next-20200618, which is based on v5.8-rc1; fixed conflicts
v5: https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprowski@samsu... - fixed some minor style issues and typos - fixed lack of the attrs argument in ion, dmabuf, rapidio, fastrpc and vfio patches
v4: https://lore.kernel.org/linux-iommu/20200512121931.GD20393@lst.de/T/ - added for_each_sgtable_* wrappers and applied where possible - added drm_prime_get_contiguous_size() and applied where possible - applied drm_prime_sg_to_page_addr_arrays() where possible to remove page extraction from sg_table objects - added documentation for the introduced wrappers - improved patches description a bit
v3: https://lore.kernel.org/dri-devel/20200505083926.28503-1-m.szyprowski@samsun... - introduce dma_*_sgtable_* wrappers and use them in all patches
v2: https://lore.kernel.org/linux-iommu/c01c9766-9778-fd1f-f36e-2dc7bd376ba4@arm... - dropped most of the changes to drm/i915 - added fixes for rcar-du, xen, media and ion - fixed a few issues pointed by kbuild test robot - added wide cc: list for each patch
v1: https://lore.kernel.org/linux-iommu/c01c9766-9778-fd1f-f36e-2dc7bd376ba4@arm... - initial version
Patch summary:
Marek Szyprowski (36): drm: prime: add common helper to check scatterlist contiguity drm: prime: use sgtable iterators in drm_prime_sg_to_page_addr_arrays() drm: core: fix common struct sg_table related issues drm: amdgpu: fix common struct sg_table related issues drm: armada: fix common struct sg_table related issues drm: etnaviv: fix common struct sg_table related issues drm: exynos: use common helper for a scatterlist contiguity check drm: exynos: fix common struct sg_table related issues drm: i915: fix common struct sg_table related issues drm: lima: fix common struct sg_table related issues drm: mediatek: use common helper for a scatterlist contiguity check drm: mediatek: use common helper for extracting pages array drm: msm: fix common struct sg_table related issues drm: omapdrm: use common helper for extracting pages array drm: omapdrm: fix common struct sg_table related issues drm: panfrost: fix common struct sg_table related issues drm: radeon: fix common struct sg_table related issues drm: rockchip: use common helper for a scatterlist contiguity check drm: rockchip: fix common struct sg_table related issues drm: tegra: fix common struct sg_table related issues drm: v3d: fix common struct sg_table related issues drm: virtio: fix common struct sg_table related issues drm: vmwgfx: fix common struct sg_table related issues xen: gntdev: fix common struct sg_table related issues drm: host1x: fix common struct sg_table related issues drm: rcar-du: fix common struct sg_table related issues dmabuf: fix common struct sg_table related issues staging: ion: remove dead code staging: ion: fix common struct sg_table related issues staging: tegra-vde: fix common struct sg_table related issues misc: fastrpc: fix common struct sg_table related issues rapidio: fix common struct sg_table related issues samples: vfio-mdev/mbochs: fix common struct sg_table related issues media: pci: fix common ALSA DMA-mapping related codes videobuf2: use sgtable-based scatterlist wrappers drm: xen: fix common struct sg_table related issues
drivers/dma-buf/heaps/heap-helpers.c | 13 ++- drivers/dma-buf/udmabuf.c | 7 +- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 9 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 8 +- drivers/gpu/drm/armada/armada_gem.c | 12 +-- drivers/gpu/drm/drm_cache.c | 2 +- drivers/gpu/drm/drm_gem_cma_helper.c | 23 +---- drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +-- drivers/gpu/drm/drm_prime.c | 86 ++++++++++--------- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 12 ++- drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 13 +-- drivers/gpu/drm/exynos/exynos_drm_g2d.c | 10 +-- drivers/gpu/drm/exynos/exynos_drm_gem.c | 23 +---- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 11 +-- .../gpu/drm/i915/gem/selftests/mock_dmabuf.c | 7 +- drivers/gpu/drm/lima/lima_gem.c | 11 ++- drivers/gpu/drm/lima/lima_vm.c | 5 +- drivers/gpu/drm/mediatek/mtk_drm_gem.c | 37 ++------ drivers/gpu/drm/msm/msm_gem.c | 13 ++- drivers/gpu/drm/msm/msm_gpummu.c | 14 ++- drivers/gpu/drm/msm/msm_iommu.c | 2 +- drivers/gpu/drm/omapdrm/omap_gem.c | 20 ++--- drivers/gpu/drm/panfrost/panfrost_gem.c | 4 +- drivers/gpu/drm/panfrost/panfrost_mmu.c | 7 +- drivers/gpu/drm/radeon/radeon_ttm.c | 11 ++- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 +- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 42 +++------ drivers/gpu/drm/tegra/gem.c | 27 +++--- drivers/gpu/drm/tegra/plane.c | 15 ++-- drivers/gpu/drm/v3d/v3d_mmu.c | 17 ++-- drivers/gpu/drm/virtio/virtgpu_object.c | 36 ++++---- drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 17 +--- drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +- drivers/gpu/host1x/job.c | 22 ++--- .../common/videobuf2/videobuf2-dma-contig.c | 41 ++++----- .../media/common/videobuf2/videobuf2-dma-sg.c | 32 +++---- .../common/videobuf2/videobuf2-vmalloc.c | 12 +-- drivers/media/pci/cx23885/cx23885-alsa.c | 2 +- drivers/media/pci/cx25821/cx25821-alsa.c | 2 +- drivers/media/pci/cx88/cx88-alsa.c | 2 +- drivers/media/pci/saa7134/saa7134-alsa.c | 2 +- drivers/media/platform/vsp1/vsp1_drm.c | 8 +- drivers/misc/fastrpc.c | 4 +- drivers/rapidio/devices/rio_mport_cdev.c | 8 +- drivers/staging/android/ion/ion.c | 25 +++--- drivers/staging/android/ion/ion.h | 1 - drivers/staging/android/ion/ion_heap.c | 53 +++--------- drivers/staging/android/ion/ion_system_heap.c | 2 +- drivers/staging/media/tegra-vde/iommu.c | 4 +- drivers/xen/gntdev-dmabuf.c | 13 ++- include/drm/drm_prime.h | 2 + samples/vfio-mdev/mbochs.c | 3 +- 54 files changed, 311 insertions(+), 478 deletions(-)
It is a common operation done by DRM drivers to check the contiguity of the DMA-mapped buffer described by a scatterlist in the sg_table object. Let's add a common helper for this operation.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/drm_gem_cma_helper.c | 23 +++-------------------- drivers/gpu/drm/drm_prime.c | 26 ++++++++++++++++++++++++++ include/drm/drm_prime.h | 2 ++ 3 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 06a5b9ee1fe0..41566a15dabd 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -471,26 +471,9 @@ drm_gem_cma_prime_import_sg_table(struct drm_device *dev, { struct drm_gem_cma_object *cma_obj;
- if (sgt->nents != 1) { - /* check if the entries in the sg_table are contiguous */ - dma_addr_t next_addr = sg_dma_address(sgt->sgl); - struct scatterlist *s; - unsigned int i; - - for_each_sg(sgt->sgl, s, sgt->nents, i) { - /* - * sg_dma_address(s) is only valid for entries - * that have sg_dma_len(s) != 0 - */ - if (!sg_dma_len(s)) - continue; - - if (sg_dma_address(s) != next_addr) - return ERR_PTR(-EINVAL); - - next_addr = sg_dma_address(s) + sg_dma_len(s); - } - } + /* check if the entries in the sg_table are contiguous */ + if (drm_prime_get_contiguous_size(sgt) < attach->dmabuf->size) + return ERR_PTR(-EINVAL);
/* Create a CMA GEM buffer. */ cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index bbfc713bfdc3..0784969894c1 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -825,6 +825,32 @@ struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_page } EXPORT_SYMBOL(drm_prime_pages_to_sg);
+/** + * drm_prime_get_contiguous_size - returns the contiguous size of the buffer + * @sgt: sg_table describing the buffer to check + * + * This helper calculates the contiguous size in the DMA address space + * of the the buffer described by the provided sg_table. + * + * This is useful for implementing + * &drm_gem_object_funcs.gem_prime_import_sg_table. + */ +unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt) +{ + dma_addr_t expected = sg_dma_address(sgt->sgl); + struct sg_dma_page_iter dma_iter; + unsigned long size = 0; + + for_each_sgtable_dma_page(sgt, &dma_iter, 0) { + if (sg_page_iter_dma_address(&dma_iter) != expected) + break; + expected += PAGE_SIZE; + size += PAGE_SIZE; + } + return size; +} +EXPORT_SYMBOL(drm_prime_get_contiguous_size); + /** * drm_gem_prime_export - helper library implementation of the export callback * @obj: GEM object to export diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h index 9af7422b44cf..47ef11614627 100644 --- a/include/drm/drm_prime.h +++ b/include/drm/drm_prime.h @@ -92,6 +92,8 @@ struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_page struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj, int flags);
+unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt); + /* helper functions for importing */ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev, struct dma_buf *dma_buf,
On 2020-06-18 16:39, Marek Szyprowski wrote:
It is a common operation done by DRM drivers to check the contiguity of the DMA-mapped buffer described by a scatterlist in the sg_table object. Let's add a common helper for this operation.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/drm_gem_cma_helper.c | 23 +++-------------------- drivers/gpu/drm/drm_prime.c | 26 ++++++++++++++++++++++++++ include/drm/drm_prime.h | 2 ++ 3 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 06a5b9ee1fe0..41566a15dabd 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -471,26 +471,9 @@ drm_gem_cma_prime_import_sg_table(struct drm_device *dev, { struct drm_gem_cma_object *cma_obj;
- if (sgt->nents != 1) {
/* check if the entries in the sg_table are contiguous */dma_addr_t next_addr = sg_dma_address(sgt->sgl);struct scatterlist *s;unsigned int i;for_each_sg(sgt->sgl, s, sgt->nents, i) {/** sg_dma_address(s) is only valid for entries* that have sg_dma_len(s) != 0*/if (!sg_dma_len(s))continue;if (sg_dma_address(s) != next_addr)return ERR_PTR(-EINVAL);next_addr = sg_dma_address(s) + sg_dma_len(s);}- }
/* check if the entries in the sg_table are contiguous */
if (drm_prime_get_contiguous_size(sgt) < attach->dmabuf->size)
return ERR_PTR(-EINVAL);/* Create a CMA GEM buffer. */ cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index bbfc713bfdc3..0784969894c1 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -825,6 +825,32 @@ struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_page } EXPORT_SYMBOL(drm_prime_pages_to_sg);
+/**
- drm_prime_get_contiguous_size - returns the contiguous size of the buffer
- @sgt: sg_table describing the buffer to check
- This helper calculates the contiguous size in the DMA address space
- of the the buffer described by the provided sg_table.
- This is useful for implementing
- &drm_gem_object_funcs.gem_prime_import_sg_table.
- */
+unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt) +{
- dma_addr_t expected = sg_dma_address(sgt->sgl);
- struct sg_dma_page_iter dma_iter;
- unsigned long size = 0;
- for_each_sgtable_dma_page(sgt, &dma_iter, 0) {
if (sg_page_iter_dma_address(&dma_iter) != expected)break;expected += PAGE_SIZE;size += PAGE_SIZE;
Hmm, in many cases this is likely to be far less efficient than simply using for_each_sgtable_dma() and sg_dma_len() equivalent to the original implementation, and there doesn't seem to be any good reason for that. Plus AFAICS it could potentially let false-positives through if someone were to pass in a table with non-page-aligned lengths (I assume that's expected never to happen, but still...)
Robin.
- }
- return size;
+} +EXPORT_SYMBOL(drm_prime_get_contiguous_size);
- /**
- drm_gem_prime_export - helper library implementation of the export callback
- @obj: GEM object to export
diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h index 9af7422b44cf..47ef11614627 100644 --- a/include/drm/drm_prime.h +++ b/include/drm/drm_prime.h @@ -92,6 +92,8 @@ struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_page struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj, int flags);
+unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt);
- /* helper functions for importing */ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev, struct dma_buf *dma_buf,
Replace the current hand-crafted code for extracting pages and DMA addresses from the given scatterlist by the much more robust code based on the generic scatterlist iterators and recently introduced sg_table-based wrappers. The resulting code is simple and easy to understand, so the comment describing the old code is no longer needed.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/drm_prime.c | 47 +++++++++++-------------------------- 1 file changed, 14 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0784969894c1..22953ee1e2ba 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -985,45 +985,26 @@ EXPORT_SYMBOL(drm_gem_prime_import); int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, dma_addr_t *addrs, int max_entries) { - unsigned count; - struct scatterlist *sg; - struct page *page; - u32 page_len, page_index; - dma_addr_t addr; - u32 dma_len, dma_index; + struct sg_dma_page_iter dma_iter; + struct sg_page_iter page_iter; + struct page **p = pages; + dma_addr_t *a = addrs;
- /* - * Scatterlist elements contains both pages and DMA addresses, but - * one shoud not assume 1:1 relation between them. The sg->length is - * the size of the physical memory chunk described by the sg->page, - * while sg_dma_len(sg) is the size of the DMA (IO virtual) chunk - * described by the sg_dma_address(sg). - */ - page_index = 0; - dma_index = 0; - for_each_sg(sgt->sgl, sg, sgt->nents, count) { - page_len = sg->length; - page = sg_page(sg); - dma_len = sg_dma_len(sg); - addr = sg_dma_address(sg); - - while (pages && page_len > 0) { - if (WARN_ON(page_index >= max_entries)) + if (pages) { + for_each_sgtable_page(sgt, &page_iter, 0) { + if (p - pages >= max_entries) return -1; - pages[page_index] = page; - page++; - page_len -= PAGE_SIZE; - page_index++; + *p++ = sg_page_iter_page(&page_iter); } - while (addrs && dma_len > 0) { - if (WARN_ON(dma_index >= max_entries)) + } + if (addrs) { + for_each_sgtable_dma_page(sgt, &dma_iter, 0) { + if (a - addrs >= max_entries) return -1; - addrs[dma_index] = addr; - addr += PAGE_SIZE; - dma_len -= PAGE_SIZE; - dma_index++; + *a++ = sg_page_iter_dma_address(&dma_iter); } } + return 0; } EXPORT_SYMBOL(drm_prime_sg_to_page_addr_arrays);
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/drm_cache.c | 2 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +++++++++----- drivers/gpu/drm/drm_prime.c | 11 ++++++----- 3 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c index 03e01b000f7a..0fe3c496002a 100644 --- a/drivers/gpu/drm/drm_cache.c +++ b/drivers/gpu/drm/drm_cache.c @@ -127,7 +127,7 @@ drm_clflush_sg(struct sg_table *st) struct sg_page_iter sg_iter;
mb(); /*CLFLUSH is ordered only by using memory barriers*/ - for_each_sg_page(st->sgl, &sg_iter, st->nents, 0) + for_each_sgtable_page(st, &sg_iter, 0) drm_clflush_page(sg_page_iter_page(&sg_iter)); mb(); /*Make sure that all cache line entry is flushed*/
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 4b7cfbac4daa..47d8211221f2 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -126,8 +126,8 @@ void drm_gem_shmem_free_object(struct drm_gem_object *obj) drm_prime_gem_destroy(obj, shmem->sgt); } else { if (shmem->sgt) { - dma_unmap_sg(obj->dev->dev, shmem->sgt->sgl, - shmem->sgt->nents, DMA_BIDIRECTIONAL); + dma_unmap_sgtable(obj->dev->dev, shmem->sgt, + DMA_BIDIRECTIONAL, 0); sg_free_table(shmem->sgt); kfree(shmem->sgt); } @@ -424,8 +424,7 @@ void drm_gem_shmem_purge_locked(struct drm_gem_object *obj)
WARN_ON(!drm_gem_shmem_is_purgeable(shmem));
- dma_unmap_sg(obj->dev->dev, shmem->sgt->sgl, - shmem->sgt->nents, DMA_BIDIRECTIONAL); + dma_unmap_sgtable(obj->dev->dev, shmem->sgt, DMA_BIDIRECTIONAL, 0); sg_free_table(shmem->sgt); kfree(shmem->sgt); shmem->sgt = NULL; @@ -697,12 +696,17 @@ struct sg_table *drm_gem_shmem_get_pages_sgt(struct drm_gem_object *obj) goto err_put_pages; } /* Map the pages for use by the h/w. */ - dma_map_sg(obj->dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL); + ret = dma_map_sgtable(obj->dev->dev, sgt, DMA_BIDIRECTIONAL, 0); + if (ret) + goto err_free_sgt;
shmem->sgt = sgt;
return sgt;
+err_free_sgt: + sg_free_table(sgt); + kfree(sgt); err_put_pages: drm_gem_shmem_put_pages(shmem); return ERR_PTR(ret); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 22953ee1e2ba..dc2efa8a8dd3 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -617,6 +617,7 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, { struct drm_gem_object *obj = attach->dmabuf->priv; struct sg_table *sgt; + int ret;
if (WARN_ON(dir == DMA_NONE)) return ERR_PTR(-EINVAL); @@ -626,11 +627,12 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, else sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
- if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir, - DMA_ATTR_SKIP_CPU_SYNC)) { + ret = dma_map_sgtable(attach->dev, sgt, dir, + DMA_ATTR_SKIP_CPU_SYNC); + if (ret) { sg_free_table(sgt); kfree(sgt); - sgt = ERR_PTR(-ENOMEM); + sgt = ERR_PTR(ret); }
return sgt; @@ -652,8 +654,7 @@ void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach, if (!sgt) return;
- dma_unmap_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir, - DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(attach->dev, sgt, dir, DMA_ATTR_SKIP_CPU_SYNC); sg_free_table(sgt); kfree(sgt); }
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Reviewed-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 6 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 9 +++------ drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 8 ++++---- 3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c index 43d8ed7dbd00..519ce4427fce 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c @@ -307,8 +307,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct dma_buf_attachment *attach, if (IS_ERR(sgt)) return sgt;
- if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir, - DMA_ATTR_SKIP_CPU_SYNC)) + if (dma_map_sgtable(attach->dev, sgt, dir, + DMA_ATTR_SKIP_CPU_SYNC)) goto error_free; break;
@@ -349,7 +349,7 @@ static void amdgpu_dma_buf_unmap(struct dma_buf_attachment *attach, struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
if (sgt->sgl->page_link) { - dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir); + dma_unmap_sgtable(attach->dev, sgt, dir, 0); sg_free_table(sgt); kfree(sgt); } else { diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 5129a996e941..97fb73e5a6ae 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1025,7 +1025,6 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm) { struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev); struct amdgpu_ttm_tt *gtt = (void *)ttm; - unsigned nents; int r;
int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY); @@ -1040,9 +1039,8 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm) goto release_sg;
/* Map SG to device */ - r = -ENOMEM; - nents = dma_map_sg(adev->dev, ttm->sg->sgl, ttm->sg->nents, direction); - if (nents == 0) + r = dma_map_sgtable(adev->dev, ttm->sg, direction, 0); + if (r) goto release_sg;
/* convert SG to linear array of pages and dma addresses */ @@ -1073,8 +1071,7 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_tt *ttm) return;
/* unmap the pages mapped to the device */ - dma_unmap_sg(adev->dev, ttm->sg->sgl, ttm->sg->nents, direction); - + dma_unmap_sgtable(adev->dev, ttm->sg, direction, 0); sg_free_table(ttm->sg);
#if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c index d399e5893170..75495a7898b9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c @@ -477,11 +477,11 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev, if (r) goto error_free;
- for_each_sg((*sgt)->sgl, sg, num_entries, i) + for_each_sgtable_sg(*sgt, sg, i) sg->length = 0;
node = mem->mm_node; - for_each_sg((*sgt)->sgl, sg, num_entries, i) { + for_each_sgtable_sg(*sgt, sg, i) { phys_addr_t phys = (node->start << PAGE_SHIFT) + adev->gmc.aper_base; size_t size = node->size << PAGE_SHIFT; @@ -501,7 +501,7 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev, return 0;
error_unmap: - for_each_sg((*sgt)->sgl, sg, num_entries, i) { + for_each_sgtable_sg(*sgt, sg, i) { if (!sg->length) continue;
@@ -532,7 +532,7 @@ void amdgpu_vram_mgr_free_sgt(struct amdgpu_device *adev, struct scatterlist *sg; int i;
- for_each_sg(sgt->sgl, sg, sgt->nents, i) + for_each_sgtable_sg(sgt, sg, i) dma_unmap_resource(dev, sg->dma_address, sg->length, dir, DMA_ATTR_SKIP_CPU_SYNC);
Hi Marek,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20200618] [also build test WARNING on v5.8-rc1] [cannot apply to linuxtv-media/master staging/staging-testing drm-exynos/exynos-drm-next drm-intel/for-linux-next linus/master v5.8-rc1 v5.7 v5.7-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Marek-Szyprowski/DRM-fix-struct-sg_... base: ce2cc8efd7a40cbd17841add878cb691d0ce0bba config: alpha-allyesconfig (attached as .config) compiler: alpha-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>, old ones prefixed by <<):
In file included from include/linux/dma-mapping.h:11, from drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:25: drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c: In function 'amdgpu_vram_mgr_alloc_sgt': include/linux/scatterlist.h:158:17: error: '*sgt' is a pointer; did you mean to use '->'? 158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i) | ^~ include/linux/scatterlist.h:152:22: note: in definition of macro 'for_each_sg' 152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg)) | ^~~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:480:2: note: in expansion of macro 'for_each_sgtable_sg'
480 | for_each_sgtable_sg(*sgt, sg, i) | ^~~~~~~~~~~~~~~~~~~ include/linux/scatterlist.h:158:31: error: '*sgt' is a pointer; did you mean to use '->'? 158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i) | ^~ include/linux/scatterlist.h:152:38: note: in definition of macro 'for_each_sg' 152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg)) | ^~
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:480:2: note: in expansion of macro 'for_each_sgtable_sg'
480 | for_each_sgtable_sg(*sgt, sg, i) | ^~~~~~~~~~~~~~~~~~~ include/linux/scatterlist.h:158:17: error: '*sgt' is a pointer; did you mean to use '->'? 158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i) | ^~ include/linux/scatterlist.h:152:22: note: in definition of macro 'for_each_sg' 152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg)) | ^~~~~~ drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:484:2: note: in expansion of macro 'for_each_sgtable_sg' 484 | for_each_sgtable_sg(*sgt, sg, i) { | ^~~~~~~~~~~~~~~~~~~ include/linux/scatterlist.h:158:31: error: '*sgt' is a pointer; did you mean to use '->'? 158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i) | ^~ include/linux/scatterlist.h:152:38: note: in definition of macro 'for_each_sg' 152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg)) | ^~ drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:484:2: note: in expansion of macro 'for_each_sgtable_sg' 484 | for_each_sgtable_sg(*sgt, sg, i) { | ^~~~~~~~~~~~~~~~~~~ include/linux/scatterlist.h:158:17: error: '*sgt' is a pointer; did you mean to use '->'? 158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i) | ^~ include/linux/scatterlist.h:152:22: note: in definition of macro 'for_each_sg' 152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg)) | ^~~~~~ drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:504:2: note: in expansion of macro 'for_each_sgtable_sg' 504 | for_each_sgtable_sg(*sgt, sg, i) { | ^~~~~~~~~~~~~~~~~~~ include/linux/scatterlist.h:158:31: error: '*sgt' is a pointer; did you mean to use '->'? 158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i) | ^~ include/linux/scatterlist.h:152:38: note: in definition of macro 'for_each_sg' 152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg)) | ^~ drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:504:2: note: in expansion of macro 'for_each_sgtable_sg' 504 | for_each_sgtable_sg(*sgt, sg, i) { | ^~~~~~~~~~~~~~~~~~~ In file included from drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:26: At top level: drivers/gpu/drm/amd/amdgpu/amdgpu.h:190:18: warning: 'sched_policy' defined but not used [-Wunused-const-variable=] 190 | static const int sched_policy = KFD_SCHED_POLICY_HWS; | ^~~~~~~~~~~~ In file included from drivers/gpu/drm/amd/amdgpu/../display/dc/dc_types.h:33, from drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:30, from drivers/gpu/drm/amd/amdgpu/../include/dm_pp_interface.h:26, from drivers/gpu/drm/amd/amdgpu/amdgpu.h:65, from drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:26: drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:76:32: warning: 'dc_fixpt_ln2_div_2' defined but not used [-Wunused-const-variable=] 76 | static const struct fixed31_32 dc_fixpt_ln2_div_2 = { 1488522236LL }; | ^~~~~~~~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:75:32: warning: 'dc_fixpt_ln2' defined but not used [-Wunused-const-variable=] 75 | static const struct fixed31_32 dc_fixpt_ln2 = { 2977044471LL }; | ^~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:74:32: warning: 'dc_fixpt_e' defined but not used [-Wunused-const-variable=] 74 | static const struct fixed31_32 dc_fixpt_e = { 11674931555LL }; | ^~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:73:32: warning: 'dc_fixpt_two_pi' defined but not used [-Wunused-const-variable=] 73 | static const struct fixed31_32 dc_fixpt_two_pi = { 26986075409LL }; | ^~~~~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:72:32: warning: 'dc_fixpt_pi' defined but not used [-Wunused-const-variable=] 72 | static const struct fixed31_32 dc_fixpt_pi = { 13493037705LL }; | ^~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:67:32: warning: 'dc_fixpt_zero' defined but not used [-Wunused-const-variable=] 67 | static const struct fixed31_32 dc_fixpt_zero = { 0 }; | ^~~~~~~~~~~~~
vim +/for_each_sgtable_sg +480 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
444 445 /** 446 * amdgpu_vram_mgr_alloc_sgt - allocate and fill a sg table 447 * 448 * @adev: amdgpu device pointer 449 * @mem: TTM memory object 450 * @dev: the other device 451 * @dir: dma direction 452 * @sgt: resulting sg table 453 * 454 * Allocate and fill a sg table from a VRAM allocation. 455 */ 456 int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev, 457 struct ttm_mem_reg *mem, 458 struct device *dev, 459 enum dma_data_direction dir, 460 struct sg_table **sgt) 461 { 462 struct drm_mm_node *node; 463 struct scatterlist *sg; 464 int num_entries = 0; 465 unsigned int pages; 466 int i, r; 467 468 *sgt = kmalloc(sizeof(*sg), GFP_KERNEL); 469 if (!*sgt) 470 return -ENOMEM; 471 472 for (pages = mem->num_pages, node = mem->mm_node; 473 pages; pages -= node->size, ++node) 474 ++num_entries; 475 476 r = sg_alloc_table(*sgt, num_entries, GFP_KERNEL); 477 if (r) 478 goto error_free; 479
480 for_each_sgtable_sg(*sgt, sg, i)
481 sg->length = 0; 482 483 node = mem->mm_node; 484 for_each_sgtable_sg(*sgt, sg, i) { 485 phys_addr_t phys = (node->start << PAGE_SHIFT) + 486 adev->gmc.aper_base; 487 size_t size = node->size << PAGE_SHIFT; 488 dma_addr_t addr; 489 490 ++node; 491 addr = dma_map_resource(dev, phys, size, dir, 492 DMA_ATTR_SKIP_CPU_SYNC); 493 r = dma_mapping_error(dev, addr); 494 if (r) 495 goto error_unmap; 496 497 sg_set_page(sg, NULL, size, 0); 498 sg_dma_address(sg) = addr; 499 sg_dma_len(sg) = size; 500 } 501 return 0; 502 503 error_unmap: 504 for_each_sgtable_sg(*sgt, sg, i) { 505 if (!sg->length) 506 continue; 507 508 dma_unmap_resource(dev, sg->dma_address, 509 sg->length, dir, 510 DMA_ATTR_SKIP_CPU_SYNC); 511 } 512 sg_free_table(*sgt); 513 514 error_free: 515 kfree(*sgt); 516 return r; 517 } 518
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/armada/armada_gem.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c index 8005614d2e6b..bedd8937d8a1 100644 --- a/drivers/gpu/drm/armada/armada_gem.c +++ b/drivers/gpu/drm/armada/armada_gem.c @@ -395,7 +395,7 @@ armada_gem_prime_map_dma_buf(struct dma_buf_attachment *attach,
mapping = dobj->obj.filp->f_mapping;
- for_each_sg(sgt->sgl, sg, count, i) { + for_each_sgtable_sg(sgt, sg, i) { struct page *page;
page = shmem_read_mapping_page(mapping, i); @@ -407,8 +407,8 @@ armada_gem_prime_map_dma_buf(struct dma_buf_attachment *attach, sg_set_page(sg, page, PAGE_SIZE, 0); }
- if (dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir) == 0) { - num = sgt->nents; + if (dma_map_sgtable(attach->dev, sgt, dir, 0)) { + num = count; goto release; } } else if (dobj->page) { @@ -418,7 +418,7 @@ armada_gem_prime_map_dma_buf(struct dma_buf_attachment *attach,
sg_set_page(sgt->sgl, dobj->page, dobj->obj.size, 0);
- if (dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir) == 0) + if (dma_map_sgtable(attach->dev, sgt, dir, 0)) goto free_table; } else if (dobj->linear) { /* Single contiguous physical region - no struct page */ @@ -449,11 +449,11 @@ static void armada_gem_prime_unmap_dma_buf(struct dma_buf_attachment *attach, int i;
if (!dobj->linear) - dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir); + dma_unmap_sgtable(attach->dev, sgt, dir, 0);
if (dobj->obj.filp) { struct scatterlist *sg; - for_each_sg(sgt->sgl, sg, sgt->nents, i) + for_each_sgtable_sg(sgt, sg, i) put_page(sg_page(sg)); }
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 12 +++++------- drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 13 +++---------- 2 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c index f5e5bb8ba953..9f4613f7e255 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c @@ -27,7 +27,7 @@ static void etnaviv_gem_scatter_map(struct etnaviv_gem_object *etnaviv_obj) * because display controller, GPU, etc. are not coherent. */ if (etnaviv_obj->flags & ETNA_BO_CACHE_MASK) - dma_map_sg(dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL); + dma_map_sgtable(dev->dev, sgt, DMA_BIDIRECTIONAL, 0); }
static void etnaviv_gem_scatterlist_unmap(struct etnaviv_gem_object *etnaviv_obj) @@ -51,7 +51,7 @@ static void etnaviv_gem_scatterlist_unmap(struct etnaviv_gem_object *etnaviv_obj * discard those writes. */ if (etnaviv_obj->flags & ETNA_BO_CACHE_MASK) - dma_unmap_sg(dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL); + dma_unmap_sgtable(dev->dev, sgt, DMA_BIDIRECTIONAL, 0); }
/* called with etnaviv_obj->lock held */ @@ -404,9 +404,8 @@ int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op, }
if (etnaviv_obj->flags & ETNA_BO_CACHED) { - dma_sync_sg_for_cpu(dev->dev, etnaviv_obj->sgt->sgl, - etnaviv_obj->sgt->nents, - etnaviv_op_to_dma_dir(op)); + dma_sync_sgtable_for_cpu(dev->dev, etnaviv_obj->sgt, + etnaviv_op_to_dma_dir(op)); etnaviv_obj->last_cpu_prep_op = op; }
@@ -421,8 +420,7 @@ int etnaviv_gem_cpu_fini(struct drm_gem_object *obj) if (etnaviv_obj->flags & ETNA_BO_CACHED) { /* fini without a prep is almost certainly a userspace error */ WARN_ON(etnaviv_obj->last_cpu_prep_op == 0); - dma_sync_sg_for_device(dev->dev, etnaviv_obj->sgt->sgl, - etnaviv_obj->sgt->nents, + dma_sync_sgtable_for_device(dev->dev, etnaviv_obj->sgt, etnaviv_op_to_dma_dir(etnaviv_obj->last_cpu_prep_op)); etnaviv_obj->last_cpu_prep_op = 0; } diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c index 3607d348c298..13b100553a0b 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c @@ -79,7 +79,7 @@ static int etnaviv_iommu_map(struct etnaviv_iommu_context *context, u32 iova, if (!context || !sgt) return -EINVAL;
- for_each_sg(sgt->sgl, sg, sgt->nents, i) { + for_each_sgtable_dma_sg(sgt, sg, i) { u32 pa = sg_dma_address(sg) - sg->offset; size_t bytes = sg_dma_len(sg) + sg->offset;
@@ -95,14 +95,7 @@ static int etnaviv_iommu_map(struct etnaviv_iommu_context *context, u32 iova, return 0;
fail: - da = iova; - - for_each_sg(sgt->sgl, sg, i, j) { - size_t bytes = sg_dma_len(sg) + sg->offset; - - etnaviv_context_unmap(context, da, bytes); - da += bytes; - } + etnaviv_context_unmap(context, iova, da - iova); return ret; }
@@ -113,7 +106,7 @@ static void etnaviv_iommu_unmap(struct etnaviv_iommu_context *context, u32 iova, unsigned int da = iova; int i;
- for_each_sg(sgt->sgl, sg, sgt->nents, i) { + for_each_sgtable_dma_sg(sgt, sg, i) { size_t bytes = sg_dma_len(sg) + sg->offset;
etnaviv_context_unmap(context, da, bytes);
Use common helper for checking the contiguity of the imported dma-buf.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_gem.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index efa476858db5..1716a023bca0 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c @@ -431,27 +431,10 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev, { struct exynos_drm_gem *exynos_gem;
- if (sgt->nents < 1) + /* check if the entries in the sg_table are contiguous */ + if (drm_prime_get_contiguous_size(sgt) < attach->dmabuf->size) { + DRM_ERROR("buffer chunks must be mapped contiguously"); return ERR_PTR(-EINVAL); - - /* - * Check if the provided buffer has been mapped as contiguous - * into DMA address space. - */ - if (sgt->nents > 1) { - dma_addr_t next_addr = sg_dma_address(sgt->sgl); - struct scatterlist *s; - unsigned int i; - - for_each_sg(sgt->sgl, s, sgt->nents, i) { - if (!sg_dma_len(s)) - break; - if (sg_dma_address(s) != next_addr) { - DRM_ERROR("buffer chunks must be mapped contiguously"); - return ERR_PTR(-EINVAL); - } - next_addr = sg_dma_address(s) + sg_dma_len(s); - } }
exynos_gem = exynos_drm_gem_init(dev, attach->dmabuf->size);
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_g2d.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c index fcee33a43aca..7014a8cd971a 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c @@ -395,8 +395,8 @@ static void g2d_userptr_put_dma_addr(struct g2d_data *g2d, return;
out: - dma_unmap_sg(to_dma_dev(g2d->drm_dev), g2d_userptr->sgt->sgl, - g2d_userptr->sgt->nents, DMA_BIDIRECTIONAL); + dma_unmap_sgtable(to_dma_dev(g2d->drm_dev), g2d_userptr->sgt, + DMA_BIDIRECTIONAL, 0);
pages = frame_vector_pages(g2d_userptr->vec); if (!IS_ERR(pages)) { @@ -511,10 +511,10 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct g2d_data *g2d,
g2d_userptr->sgt = sgt;
- if (!dma_map_sg(to_dma_dev(g2d->drm_dev), sgt->sgl, sgt->nents, - DMA_BIDIRECTIONAL)) { + ret = dma_map_sgtable(to_dma_dev(g2d->drm_dev), sgt, + DMA_BIDIRECTIONAL, 0); + if (ret) { DRM_DEV_ERROR(g2d->dev, "failed to map sgt with dma region.\n"); - ret = -ENOMEM; goto err_sg_free_table; }
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
This driver creatively uses sg_table->orig_nents to store the size of the allocated scatterlist and ignores the number of the entries returned by dma_map_sg function. The sg_table->orig_nents is (mis)used to properly free the (over)allocated scatterlist.
This patch only introduces the common DMA-mapping wrappers operating directly on the struct sg_table objects to the dmabuf related functions, so the other drivers, which might share buffers with i915 could rely on the properly set nents and orig_nents values.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 11 +++-------- drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c | 7 +++---- 2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index 2679380159fc..8a988592715b 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -48,12 +48,9 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme src = sg_next(src); }
- if (!dma_map_sg_attrs(attachment->dev, - st->sgl, st->nents, dir, - DMA_ATTR_SKIP_CPU_SYNC)) { - ret = -ENOMEM; + ret = dma_map_sgtable(attachment->dev, st, dir, DMA_ATTR_SKIP_CPU_SYNC); + if (ret) goto err_free_sg; - }
return st;
@@ -73,9 +70,7 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment, { struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf);
- dma_unmap_sg_attrs(attachment->dev, - sg->sgl, sg->nents, dir, - DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(attachment->dev, sg, dir, DMA_ATTR_SKIP_CPU_SYNC); sg_free_table(sg); kfree(sg);
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c index debaf7b18ab5..be30b27e2926 100644 --- a/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c @@ -28,10 +28,9 @@ static struct sg_table *mock_map_dma_buf(struct dma_buf_attachment *attachment, sg = sg_next(sg); }
- if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir)) { - err = -ENOMEM; + err = dma_map_sgtable(attachment->dev, st, dir, 0); + if (err) goto err_st; - }
return st;
@@ -46,7 +45,7 @@ static void mock_unmap_dma_buf(struct dma_buf_attachment *attachment, struct sg_table *st, enum dma_data_direction dir) { - dma_unmap_sg(attachment->dev, st->sgl, st->nents, dir); + dma_unmap_sgtable(attachment->dev, st, dir, 0); sg_free_table(st); kfree(st); }
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Reviewed-by: Qiang Yu yuq825@gmail.com --- drivers/gpu/drm/lima/lima_gem.c | 11 ++++++++--- drivers/gpu/drm/lima/lima_vm.c | 5 ++--- 2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c index 155f2b4b4030..11223fe348df 100644 --- a/drivers/gpu/drm/lima/lima_gem.c +++ b/drivers/gpu/drm/lima/lima_gem.c @@ -69,8 +69,7 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm) return ret;
if (bo->base.sgt) { - dma_unmap_sg(dev, bo->base.sgt->sgl, - bo->base.sgt->nents, DMA_BIDIRECTIONAL); + dma_unmap_sgtable(dev, bo->base.sgt, DMA_BIDIRECTIONAL, 0); sg_free_table(bo->base.sgt); } else { bo->base.sgt = kmalloc(sizeof(*bo->base.sgt), GFP_KERNEL); @@ -80,7 +79,13 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm) } }
- dma_map_sg(dev, sgt.sgl, sgt.nents, DMA_BIDIRECTIONAL); + ret = dma_map_sgtable(dev, &sgt, DMA_BIDIRECTIONAL, 0); + if (ret) { + sg_free_table(&sgt); + kfree(bo->base.sgt); + bo->base.sgt = NULL; + return ret; + }
*bo->base.sgt = sgt;
diff --git a/drivers/gpu/drm/lima/lima_vm.c b/drivers/gpu/drm/lima/lima_vm.c index 5b92fb82674a..2b2739adc7f5 100644 --- a/drivers/gpu/drm/lima/lima_vm.c +++ b/drivers/gpu/drm/lima/lima_vm.c @@ -124,7 +124,7 @@ int lima_vm_bo_add(struct lima_vm *vm, struct lima_bo *bo, bool create) if (err) goto err_out1;
- for_each_sg_dma_page(bo->base.sgt->sgl, &sg_iter, bo->base.sgt->nents, 0) { + for_each_sgtable_dma_page(bo->base.sgt, &sg_iter, 0) { err = lima_vm_map_page(vm, sg_page_iter_dma_address(&sg_iter), bo_va->node.start + offset); if (err) @@ -298,8 +298,7 @@ int lima_vm_map_bo(struct lima_vm *vm, struct lima_bo *bo, int pageoff) mutex_lock(&vm->lock);
base = bo_va->node.start + (pageoff << PAGE_SHIFT); - for_each_sg_dma_page(bo->base.sgt->sgl, &sg_iter, - bo->base.sgt->nents, pageoff) { + for_each_sgtable_dma_page(bo->base.sgt, &sg_iter, pageoff) { err = lima_vm_map_page(vm, sg_page_iter_dma_address(&sg_iter), base + offset); if (err)
Use common helper for checking the contiguity of the imported dma-buf and do this check before allocating resources, so the error path is simpler.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/mediatek/mtk_drm_gem.c | 28 ++++++-------------------- 1 file changed, 6 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c b/drivers/gpu/drm/mediatek/mtk_drm_gem.c index 6190cc3b7b0d..3654ec732029 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c @@ -212,37 +212,21 @@ struct drm_gem_object *mtk_gem_prime_import_sg_table(struct drm_device *dev, struct dma_buf_attachment *attach, struct sg_table *sg) { struct mtk_drm_gem_obj *mtk_gem; - int ret; - struct scatterlist *s; - unsigned int i; - dma_addr_t expected;
- mtk_gem = mtk_drm_gem_init(dev, attach->dmabuf->size); + /* check if the entries in the sg_table are contiguous */ + if (drm_prime_get_contiguous_size(sg) < attach->dmabuf->size) { + DRM_ERROR("sg_table is not contiguous"); + return ERR_PTR(-EINVAL); + }
+ mtk_gem = mtk_drm_gem_init(dev, attach->dmabuf->size); if (IS_ERR(mtk_gem)) return ERR_CAST(mtk_gem);
- expected = sg_dma_address(sg->sgl); - for_each_sg(sg->sgl, s, sg->nents, i) { - if (!sg_dma_len(s)) - break; - - if (sg_dma_address(s) != expected) { - DRM_ERROR("sg_table is not contiguous"); - ret = -EINVAL; - goto err_gem_free; - } - expected = sg_dma_address(s) + sg_dma_len(s); - } - mtk_gem->dma_addr = sg_dma_address(sg->sgl); mtk_gem->sg = sg;
return &mtk_gem->base; - -err_gem_free: - kfree(mtk_gem); - return ERR_PTR(ret); }
void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
Use common helper for converting a sg_table object into struct page pointer array.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/mediatek/mtk_drm_gem.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c b/drivers/gpu/drm/mediatek/mtk_drm_gem.c index 3654ec732029..0583e557ad37 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c @@ -233,9 +233,7 @@ void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj) { struct mtk_drm_gem_obj *mtk_gem = to_mtk_gem_obj(obj); struct sg_table *sgt; - struct sg_page_iter iter; unsigned int npages; - unsigned int i = 0;
if (mtk_gem->kvaddr) return mtk_gem->kvaddr; @@ -249,11 +247,8 @@ void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj) if (!mtk_gem->pages) goto out;
- for_each_sg_page(sgt->sgl, &iter, sgt->orig_nents, 0) { - mtk_gem->pages[i++] = sg_page_iter_page(&iter); - if (i > npages) - break; - } + drm_prime_sg_to_page_addr_arrays(sgt, mtk_gem->pages, NULL, npages); + mtk_gem->kvaddr = vmap(mtk_gem->pages, npages, VM_MAP, pgprot_writecombine(PAGE_KERNEL));
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/msm/msm_gem.c | 13 +++++-------- drivers/gpu/drm/msm/msm_gpummu.c | 14 ++++++-------- drivers/gpu/drm/msm/msm_iommu.c | 2 +- 3 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 38b0c0e1f83e..e0d5fd36ea8f 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -53,11 +53,10 @@ static void sync_for_device(struct msm_gem_object *msm_obj) struct device *dev = msm_obj->base.dev->dev;
if (get_dma_ops(dev) && IS_ENABLED(CONFIG_ARM64)) { - dma_sync_sg_for_device(dev, msm_obj->sgt->sgl, - msm_obj->sgt->nents, DMA_BIDIRECTIONAL); + dma_sync_sgtable_for_device(dev, msm_obj->sgt, + DMA_BIDIRECTIONAL); } else { - dma_map_sg(dev, msm_obj->sgt->sgl, - msm_obj->sgt->nents, DMA_BIDIRECTIONAL); + dma_map_sgtable(dev, msm_obj->sgt, DMA_BIDIRECTIONAL, 0); } }
@@ -66,11 +65,9 @@ static void sync_for_cpu(struct msm_gem_object *msm_obj) struct device *dev = msm_obj->base.dev->dev;
if (get_dma_ops(dev) && IS_ENABLED(CONFIG_ARM64)) { - dma_sync_sg_for_cpu(dev, msm_obj->sgt->sgl, - msm_obj->sgt->nents, DMA_BIDIRECTIONAL); + dma_sync_sgtable_for_cpu(dev, msm_obj->sgt, DMA_BIDIRECTIONAL); } else { - dma_unmap_sg(dev, msm_obj->sgt->sgl, - msm_obj->sgt->nents, DMA_BIDIRECTIONAL); + dma_unmap_sgtable(dev, msm_obj->sgt, DMA_BIDIRECTIONAL, 0); } }
diff --git a/drivers/gpu/drm/msm/msm_gpummu.c b/drivers/gpu/drm/msm/msm_gpummu.c index 310a31b05faa..319f06c28235 100644 --- a/drivers/gpu/drm/msm/msm_gpummu.c +++ b/drivers/gpu/drm/msm/msm_gpummu.c @@ -30,21 +30,19 @@ static int msm_gpummu_map(struct msm_mmu *mmu, uint64_t iova, { struct msm_gpummu *gpummu = to_msm_gpummu(mmu); unsigned idx = (iova - GPUMMU_VA_START) / GPUMMU_PAGE_SIZE; - struct scatterlist *sg; + struct sg_dma_page_iter dma_iter; unsigned prot_bits = 0; - unsigned i, j;
if (prot & IOMMU_WRITE) prot_bits |= 1; if (prot & IOMMU_READ) prot_bits |= 2;
- for_each_sg(sgt->sgl, sg, sgt->nents, i) { - dma_addr_t addr = sg->dma_address; - for (j = 0; j < sg->length / GPUMMU_PAGE_SIZE; j++, idx++) { - gpummu->table[idx] = addr | prot_bits; - addr += GPUMMU_PAGE_SIZE; - } + for_each_sgtable_dma_page(sgt, &dma_iter, 0) { + dma_addr_t addr = sg_page_iter_dma_address(&dma_iter); + + BUILD_BUG_ON(GPUMMU_PAGE_SIZE != PAGE_SIZE); + gpummu->table[idx++] = addr | prot_bits; }
/* we can improve by deferring flush for multiple map() */ diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c index 3a381a9674c9..6c31e65834c6 100644 --- a/drivers/gpu/drm/msm/msm_iommu.c +++ b/drivers/gpu/drm/msm/msm_iommu.c @@ -36,7 +36,7 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova, struct msm_iommu *iommu = to_msm_iommu(mmu); size_t ret;
- ret = iommu_map_sg(iommu->domain, iova, sgt->sgl, sgt->nents, prot); + ret = iommu_map_sgtable(iommu->domain, iova, sgt, prot); WARN_ON(!ret);
return (ret == len) ? 0 : -EINVAL;
Use common helper for converting a sg_table object into struct page pointer array.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/omapdrm/omap_gem.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c index d0d12d5dd76c..ff0c4b0c3fd0 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem.c +++ b/drivers/gpu/drm/omapdrm/omap_gem.c @@ -1297,10 +1297,9 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size, omap_obj->dma_addr = sg_dma_address(sgt->sgl); } else { /* Create pages list from sgt */ - struct sg_page_iter iter; struct page **pages; unsigned int npages; - unsigned int i = 0; + unsigned int ret;
npages = DIV_ROUND_UP(size, PAGE_SIZE); pages = kcalloc(npages, sizeof(*pages), GFP_KERNEL); @@ -1311,14 +1310,9 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size, }
omap_obj->pages = pages; - - for_each_sg_page(sgt->sgl, &iter, sgt->orig_nents, 0) { - pages[i++] = sg_page_iter_page(&iter); - if (i > npages) - break; - } - - if (WARN_ON(i != npages)) { + ret = drm_prime_sg_to_page_addr_arrays(sgt, pages, NULL, + npages); + if (WARN_ON(ret)) { omap_gem_free_object(obj); obj = ERR_PTR(-ENOMEM); goto done;
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
Fix the code to refer to proper nents or orig_nents entries. This driver checks for a buffer contiguity in DMA address space, so it should test sg_table->nents entry.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/omapdrm/omap_gem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c index ff0c4b0c3fd0..a7a9a0afe2b6 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem.c +++ b/drivers/gpu/drm/omapdrm/omap_gem.c @@ -48,7 +48,7 @@ struct omap_gem_object { * OMAP_BO_MEM_DMA_API flag set) * * - buffers imported from dmabuf (with the OMAP_BO_MEM_DMABUF flag set) - * if they are physically contiguous (when sgt->orig_nents == 1) + * if they are physically contiguous (when sgt->nents == 1) * * - buffers mapped through the TILER when dma_addr_cnt is not zero, in * which case the DMA address points to the TILER aperture @@ -1279,7 +1279,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size, union omap_gem_size gsize;
/* Without a DMM only physically contiguous buffers can be supported. */ - if (sgt->orig_nents != 1 && !priv->has_dmm) + if (sgt->nents != 1 && !priv->has_dmm) return ERR_PTR(-EINVAL);
gsize.bytes = PAGE_ALIGN(size); @@ -1293,7 +1293,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,
omap_obj->sgt = sgt;
- if (sgt->orig_nents == 1) { + if (sgt->nents == 1) { omap_obj->dma_addr = sg_dma_address(sgt->sgl); } else { /* Create pages list from sgt */
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Reviewed-by: Steven Price steven.price@arm.com Reviewed-by: Rob Herring robh@kernel.org --- drivers/gpu/drm/panfrost/panfrost_gem.c | 4 ++-- drivers/gpu/drm/panfrost/panfrost_mmu.c | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c index ac5d0aa80276..ba8450ea04d0 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem.c +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c @@ -41,8 +41,8 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj)
for (i = 0; i < n_sgt; i++) { if (bo->sgts[i].sgl) { - dma_unmap_sg(pfdev->dev, bo->sgts[i].sgl, - bo->sgts[i].nents, DMA_BIDIRECTIONAL); + dma_unmap_sgtable(pfdev->dev, &bo->sgts[i], + DMA_BIDIRECTIONAL, 0); sg_free_table(&bo->sgts[i]); } } diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index 0a339c6fbfaa..fd294f6a7d3b 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -253,7 +253,7 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, struct io_pgtable_ops *ops = mmu->pgtbl_ops; u64 start_iova = iova;
- for_each_sg(sgt->sgl, sgl, sgt->nents, count) { + for_each_sgtable_dma_sg(sgt, sgl, count) { unsigned long paddr = sg_dma_address(sgl); size_t len = sg_dma_len(sgl);
@@ -517,10 +517,9 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, if (ret) goto err_pages;
- if (!dma_map_sg(pfdev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL)) { - ret = -EINVAL; + ret = dma_map_sgtable(pfdev->dev, sgt, DMA_BIDIRECTIONAL, 0); + if (ret) goto err_map; - }
mmu_map_sg(pfdev, bomapping->mmu, addr, IOMMU_WRITE | IOMMU_READ | IOMMU_NOEXEC, sgt);
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Reviewed-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/radeon/radeon_ttm.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 5d50c9edbe80..0e3eb0d22831 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -481,7 +481,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm) { struct radeon_device *rdev = radeon_get_rdev(ttm->bdev); struct radeon_ttm_tt *gtt = (void *)ttm; - unsigned pinned = 0, nents; + unsigned pinned = 0; int r;
int write = !(gtt->userflags & RADEON_GEM_USERPTR_READONLY); @@ -521,9 +521,8 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm) if (r) goto release_sg;
- r = -ENOMEM; - nents = dma_map_sg(rdev->dev, ttm->sg->sgl, ttm->sg->nents, direction); - if (nents == 0) + r = dma_map_sgtable(rdev->dev, ttm->sg, direction, 0); + if (r) goto release_sg;
drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages, @@ -554,9 +553,9 @@ static void radeon_ttm_tt_unpin_userptr(struct ttm_tt *ttm) return;
/* free the sg table and pages again */ - dma_unmap_sg(rdev->dev, ttm->sg->sgl, ttm->sg->nents, direction); + dma_unmap_sgtable(rdev->dev, ttm->sg, direction, 0);
- for_each_sg_page(ttm->sg->sgl, &sg_iter, ttm->sg->nents, 0) { + for_each_sgtable_page(ttm->sg, &sg_iter, 0) { struct page *page = sg_page_iter_page(&sg_iter); if (!(gtt->userflags & RADEON_GEM_USERPTR_READONLY)) set_page_dirty(page);
Use common helper for checking the contiguity of the imported dma-buf.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c index b9275ba7c5a5..2970e534e2bb 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c @@ -460,23 +460,6 @@ struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj) return sgt; }
-static unsigned long rockchip_sg_get_contiguous_size(struct sg_table *sgt, - int count) -{ - struct scatterlist *s; - dma_addr_t expected = sg_dma_address(sgt->sgl); - unsigned int i; - unsigned long size = 0; - - for_each_sg(sgt->sgl, s, count, i) { - if (sg_dma_address(s) != expected) - break; - expected = sg_dma_address(s) + sg_dma_len(s); - size += sg_dma_len(s); - } - return size; -} - static int rockchip_gem_iommu_map_sg(struct drm_device *drm, struct dma_buf_attachment *attach, @@ -498,7 +481,7 @@ rockchip_gem_dma_map_sg(struct drm_device *drm, if (!count) return -EINVAL;
- if (rockchip_sg_get_contiguous_size(sg, count) < attach->dmabuf->size) { + if (drm_prime_get_contiguous_size(sg) < attach->dmabuf->size) { DRM_ERROR("failed to map sg_table to contiguous linear address.\n"); dma_unmap_sg(drm->dev, sg->sgl, sg->nents, DMA_BIDIRECTIONAL);
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 23 +++++++++------------ 1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c index 2970e534e2bb..cb50f2ba2e46 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c @@ -36,8 +36,8 @@ static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
rk_obj->dma_addr = rk_obj->mm.start;
- ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl, - rk_obj->sgt->nents, prot); + ret = iommu_map_sgtable(private->domain, rk_obj->dma_addr, rk_obj->sgt, + prot); if (ret < rk_obj->base.size) { DRM_ERROR("failed to map buffer: size=%zd request_size=%zd\n", ret, rk_obj->base.size); @@ -98,11 +98,10 @@ static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj) * TODO: Replace this by drm_clflush_sg() once it can be implemented * without relying on symbols that are not exported. */ - for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i) + for_each_sgtable_sg(rk_obj->sgt, s, i) sg_dma_address(s) = sg_phys(s);
- dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents, - DMA_TO_DEVICE); + dma_sync_sgtable_for_device(drm->dev, rk_obj->sgt, DMA_TO_DEVICE);
return 0;
@@ -350,8 +349,8 @@ void rockchip_gem_free_object(struct drm_gem_object *obj) if (private->domain) { rockchip_gem_iommu_unmap(rk_obj); } else { - dma_unmap_sg(drm->dev, rk_obj->sgt->sgl, - rk_obj->sgt->nents, DMA_BIDIRECTIONAL); + dma_unmap_sgtable(drm->dev, rk_obj->sgt, + DMA_BIDIRECTIONAL, 0); } drm_prime_gem_destroy(obj, rk_obj->sgt); } else { @@ -476,15 +475,13 @@ rockchip_gem_dma_map_sg(struct drm_device *drm, struct sg_table *sg, struct rockchip_gem_object *rk_obj) { - int count = dma_map_sg(drm->dev, sg->sgl, sg->nents, - DMA_BIDIRECTIONAL); - if (!count) - return -EINVAL; + int err = dma_map_sgtable(drm->dev, sg, DMA_BIDIRECTIONAL, 0); + if (err) + return err;
if (drm_prime_get_contiguous_size(sg) < attach->dmabuf->size) { DRM_ERROR("failed to map sg_table to contiguous linear address.\n"); - dma_unmap_sg(drm->dev, sg->sgl, sg->nents, - DMA_BIDIRECTIONAL); + dma_unmap_sgtable(drm->dev, sg, DMA_BIDIRECTIONAL, 0); return -EINVAL; }
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/tegra/gem.c | 27 ++++++++++----------------- drivers/gpu/drm/tegra/plane.c | 15 +++++---------- 2 files changed, 15 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c index 723df142a981..01d94befab11 100644 --- a/drivers/gpu/drm/tegra/gem.c +++ b/drivers/gpu/drm/tegra/gem.c @@ -98,8 +98,8 @@ static struct sg_table *tegra_bo_pin(struct device *dev, struct host1x_bo *bo, * the SG table needs to be copied to avoid overwriting any * other potential users of the original SG table. */ - err = sg_alloc_table_from_sg(sgt, obj->sgt->sgl, obj->sgt->nents, - GFP_KERNEL); + err = sg_alloc_table_from_sg(sgt, obj->sgt->sgl, + obj->sgt->orig_nents, GFP_KERNEL); if (err < 0) goto free; } else { @@ -196,8 +196,7 @@ static int tegra_bo_iommu_map(struct tegra_drm *tegra, struct tegra_bo *bo)
bo->iova = bo->mm->start;
- bo->size = iommu_map_sg(tegra->domain, bo->iova, bo->sgt->sgl, - bo->sgt->nents, prot); + bo->size = iommu_map_sgtable(tegra->domain, bo->iova, bo->sgt, prot); if (!bo->size) { dev_err(tegra->drm->dev, "failed to map buffer\n"); err = -ENOMEM; @@ -264,8 +263,7 @@ static struct tegra_bo *tegra_bo_alloc_object(struct drm_device *drm, static void tegra_bo_free(struct drm_device *drm, struct tegra_bo *bo) { if (bo->pages) { - dma_unmap_sg(drm->dev, bo->sgt->sgl, bo->sgt->nents, - DMA_FROM_DEVICE); + dma_unmap_sgtable(drm->dev, bo->sgt, DMA_FROM_DEVICE, 0); drm_gem_put_pages(&bo->gem, bo->pages, true, true); sg_free_table(bo->sgt); kfree(bo->sgt); @@ -290,12 +288,9 @@ static int tegra_bo_get_pages(struct drm_device *drm, struct tegra_bo *bo) goto put_pages; }
- err = dma_map_sg(drm->dev, bo->sgt->sgl, bo->sgt->nents, - DMA_FROM_DEVICE); - if (err == 0) { - err = -EFAULT; + err = dma_map_sgtable(drm->dev, bo->sgt, DMA_FROM_DEVICE, 0); + if (err) goto free_sgt; - }
return 0;
@@ -571,7 +566,7 @@ tegra_gem_prime_map_dma_buf(struct dma_buf_attachment *attach, goto free; }
- if (dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir) == 0) + if (dma_map_sgtable(attach->dev, sgt, dir, 0)) goto free;
return sgt; @@ -590,7 +585,7 @@ static void tegra_gem_prime_unmap_dma_buf(struct dma_buf_attachment *attach, struct tegra_bo *bo = to_tegra_bo(gem);
if (bo->pages) - dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir); + dma_unmap_sgtable(attach->dev, sgt, dir, 0);
sg_free_table(sgt); kfree(sgt); @@ -609,8 +604,7 @@ static int tegra_gem_prime_begin_cpu_access(struct dma_buf *buf, struct drm_device *drm = gem->dev;
if (bo->pages) - dma_sync_sg_for_cpu(drm->dev, bo->sgt->sgl, bo->sgt->nents, - DMA_FROM_DEVICE); + dma_sync_sgtable_for_cpu(drm->dev, bo->sgt, DMA_FROM_DEVICE);
return 0; } @@ -623,8 +617,7 @@ static int tegra_gem_prime_end_cpu_access(struct dma_buf *buf, struct drm_device *drm = gem->dev;
if (bo->pages) - dma_sync_sg_for_device(drm->dev, bo->sgt->sgl, bo->sgt->nents, - DMA_TO_DEVICE); + dma_sync_sgtable_for_device(drm->dev, bo->sgt, DMA_TO_DEVICE);
return 0; } diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c index 9ccfb56e9b01..0d2ef1662a39 100644 --- a/drivers/gpu/drm/tegra/plane.c +++ b/drivers/gpu/drm/tegra/plane.c @@ -130,12 +130,9 @@ static int tegra_dc_pin(struct tegra_dc *dc, struct tegra_plane_state *state) }
if (sgt) { - err = dma_map_sg(dc->dev, sgt->sgl, sgt->nents, - DMA_TO_DEVICE); - if (err == 0) { - err = -ENOMEM; + err = dma_map_sgtable(dc->dev, sgt, DMA_TO_DEVICE, 0); + if (err) goto unpin; - }
/* * The display controller needs contiguous memory, so @@ -143,7 +140,7 @@ static int tegra_dc_pin(struct tegra_dc *dc, struct tegra_plane_state *state) * map its SG table to a single contiguous chunk of * I/O virtual memory. */ - if (err > 1) { + if (sgt->nents > 1) { err = -EINVAL; goto unpin; } @@ -165,8 +162,7 @@ static int tegra_dc_pin(struct tegra_dc *dc, struct tegra_plane_state *state) struct sg_table *sgt = state->sgt[i];
if (sgt) - dma_unmap_sg(dc->dev, sgt->sgl, sgt->nents, - DMA_TO_DEVICE); + dma_unmap_sgtable(dc->dev, sgt, DMA_TO_DEVICE, 0);
host1x_bo_unpin(dc->dev, &bo->base, sgt); state->iova[i] = DMA_MAPPING_ERROR; @@ -185,8 +181,7 @@ static void tegra_dc_unpin(struct tegra_dc *dc, struct tegra_plane_state *state) struct sg_table *sgt = state->sgt[i];
if (sgt) - dma_unmap_sg(dc->dev, sgt->sgl, sgt->nents, - DMA_TO_DEVICE); + dma_unmap_sgtable(dc->dev, sgt, DMA_TO_DEVICE, 0);
host1x_bo_unpin(dc->dev, &bo->base, sgt); state->iova[i] = DMA_MAPPING_ERROR;
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Reviewed-by: Eric Anholt eric@anholt.net --- drivers/gpu/drm/v3d/v3d_mmu.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_mmu.c b/drivers/gpu/drm/v3d/v3d_mmu.c index 3b81ea28c0bb..175c2578ad73 100644 --- a/drivers/gpu/drm/v3d/v3d_mmu.c +++ b/drivers/gpu/drm/v3d/v3d_mmu.c @@ -90,19 +90,16 @@ void v3d_mmu_insert_ptes(struct v3d_bo *bo) struct v3d_dev *v3d = to_v3d_dev(shmem_obj->base.dev); u32 page = bo->node.start; u32 page_prot = V3D_PTE_WRITEABLE | V3D_PTE_VALID; - unsigned int count; - struct scatterlist *sgl; + struct sg_dma_page_iter dma_iter;
- for_each_sg(shmem_obj->sgt->sgl, sgl, shmem_obj->sgt->nents, count) { - u32 page_address = sg_dma_address(sgl) >> V3D_MMU_PAGE_SHIFT; + for_each_sgtable_dma_page(shmem_obj->sgt, &dma_iter, 0) { + dma_addr_t dma_addr = sg_page_iter_dma_address(&dma_iter); + u32 page_address = dma_addr >> V3D_MMU_PAGE_SHIFT; u32 pte = page_prot | page_address; - u32 i;
- BUG_ON(page_address + (sg_dma_len(sgl) >> V3D_MMU_PAGE_SHIFT) >= - BIT(24)); - - for (i = 0; i < sg_dma_len(sgl) >> V3D_MMU_PAGE_SHIFT; i++) - v3d->pt[page++] = pte + i; + BUILD_BUG_ON(V3D_MMU_PAGE_SHIFT != PAGE_SIZE); + BUG_ON(page_address + 1 >= BIT(24)); + v3d->pt[page++] = pte; }
WARN_ON_ONCE(page - bo->node.start !=
Hi Marek,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20200618] [also build test WARNING on v5.8-rc1] [cannot apply to linuxtv-media/master staging/staging-testing drm-exynos/exynos-drm-next drm-intel/for-linux-next linus/master v5.8-rc1 v5.7 v5.7-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Marek-Szyprowski/DRM-fix-struct-sg_... base: ce2cc8efd7a40cbd17841add878cb691d0ce0bba config: alpha-allyesconfig (attached as .config) compiler: alpha-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>, old ones prefixed by <<):
In file included from include/linux/kernel.h:11, from include/linux/delay.h:22, from drivers/gpu/drm/v3d/v3d_drv.h:4, from drivers/gpu/drm/v3d/v3d_mmu.c:21: drivers/gpu/drm/v3d/v3d_mmu.c: In function 'v3d_mmu_insert_ptes': include/linux/compiler.h:339:38: error: call to '__compiletime_assert_254' declared with attribute error: BUILD_BUG_ON failed: V3D_MMU_PAGE_SHIFT != PAGE_SIZE 339 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^ include/linux/compiler.h:320:4: note: in definition of macro '__compiletime_assert' 320 | prefix ## suffix(); | ^~~~~~ include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert' 339 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^~~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert' 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) | ^~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:50:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' 50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) | ^~~~~~~~~~~~~~~~
drivers/gpu/drm/v3d/v3d_mmu.c:100:3: note: in expansion of macro 'BUILD_BUG_ON'
100 | BUILD_BUG_ON(V3D_MMU_PAGE_SHIFT != PAGE_SIZE); | ^~~~~~~~~~~~
vim +/BUILD_BUG_ON +100 drivers/gpu/drm/v3d/v3d_mmu.c
86 87 void v3d_mmu_insert_ptes(struct v3d_bo *bo) 88 { 89 struct drm_gem_shmem_object *shmem_obj = &bo->base; 90 struct v3d_dev *v3d = to_v3d_dev(shmem_obj->base.dev); 91 u32 page = bo->node.start; 92 u32 page_prot = V3D_PTE_WRITEABLE | V3D_PTE_VALID; 93 struct sg_dma_page_iter dma_iter; 94 95 for_each_sgtable_dma_page(shmem_obj->sgt, &dma_iter, 0) { 96 dma_addr_t dma_addr = sg_page_iter_dma_address(&dma_iter); 97 u32 page_address = dma_addr >> V3D_MMU_PAGE_SHIFT; 98 u32 pte = page_prot | page_address; 99
100 BUILD_BUG_ON(V3D_MMU_PAGE_SHIFT != PAGE_SIZE);
101 BUG_ON(page_address + 1 >= BIT(24)); 102 v3d->pt[page++] = pte; 103 } 104 105 WARN_ON_ONCE(page - bo->node.start != 106 shmem_obj->base.size >> V3D_MMU_PAGE_SHIFT); 107 108 if (v3d_mmu_flush_all(v3d)) 109 dev_err(v3d->drm.dev, "MMU flush timeout\n"); 110 } 111
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Acked-by: Gerd Hoffmann kraxel@redhat.com --- drivers/gpu/drm/virtio/virtgpu_object.c | 36 ++++++++++++++----------- drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++++----- 2 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index 346cef5ce251..c3b9e3cf786e 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -72,9 +72,8 @@ void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo)
if (shmem->pages) { if (shmem->mapped) { - dma_unmap_sg(vgdev->vdev->dev.parent, - shmem->pages->sgl, shmem->mapped, - DMA_TO_DEVICE); + dma_unmap_sgtable(vgdev->vdev->dev.parent, + shmem->pages, DMA_TO_DEVICE, 0); shmem->mapped = 0; }
@@ -157,13 +156,13 @@ static int virtio_gpu_object_shmem_init(struct virtio_gpu_device *vgdev, }
if (use_dma_api) { - shmem->mapped = dma_map_sg(vgdev->vdev->dev.parent, - shmem->pages->sgl, - shmem->pages->nents, - DMA_TO_DEVICE); - *nents = shmem->mapped; + ret = dma_map_sgtable(vgdev->vdev->dev.parent, + shmem->pages, DMA_TO_DEVICE, 0); + if (ret) + return ret; + *nents = shmem->mapped = shmem->pages->nents; } else { - *nents = shmem->pages->nents; + *nents = shmem->pages->orig_nents; }
*ents = kmalloc_array(*nents, sizeof(struct virtio_gpu_mem_entry), @@ -173,13 +172,20 @@ static int virtio_gpu_object_shmem_init(struct virtio_gpu_device *vgdev, return -ENOMEM; }
- for_each_sg(shmem->pages->sgl, sg, *nents, si) { - (*ents)[si].addr = cpu_to_le64(use_dma_api - ? sg_dma_address(sg) - : sg_phys(sg)); - (*ents)[si].length = cpu_to_le32(sg->length); - (*ents)[si].padding = 0; + if (use_dma_api) { + for_each_sgtable_dma_sg(shmem->pages, sg, si) { + (*ents)[si].addr = cpu_to_le64(sg_dma_address(sg)); + (*ents)[si].length = cpu_to_le32(sg_dma_len(sg)); + (*ents)[si].padding = 0; + } + } else { + for_each_sgtable_sg(shmem->pages, sg, si) { + (*ents)[si].addr = cpu_to_le64(sg_phys(sg)); + (*ents)[si].length = cpu_to_le32(sg->length); + (*ents)[si].padding = 0; + } } + return 0; }
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 9e663a5d9952..e5765db85c20 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -302,7 +302,7 @@ static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size, int *sg_ents) return NULL; }
- for_each_sg(sgt->sgl, sg, *sg_ents, i) { + for_each_sgtable_sg(sgt, sg, i) { pg = vmalloc_to_page(data); if (!pg) { sg_free_table(sgt); @@ -603,9 +603,8 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
if (use_dma_api) - dma_sync_sg_for_device(vgdev->vdev->dev.parent, - shmem->pages->sgl, shmem->pages->nents, - DMA_TO_DEVICE); + dma_sync_sgtable_for_device(vgdev->vdev->dev.parent, + shmem->pages, DMA_TO_DEVICE);
cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p)); memset(cmd_p, 0, sizeof(*cmd_p)); @@ -1019,9 +1018,8 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev, struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
if (use_dma_api) - dma_sync_sg_for_device(vgdev->vdev->dev.parent, - shmem->pages->sgl, shmem->pages->nents, - DMA_TO_DEVICE); + dma_sync_sgtable_for_device(vgdev->vdev->dev.parent, + shmem->pages, DMA_TO_DEVICE);
cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p)); memset(cmd_p, 0, sizeof(*cmd_p));
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Acked-by: Roland Scheidegger sroland@vmware.com --- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c index bf0bc4697959..49ed6add6969 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c @@ -362,8 +362,7 @@ static void vmw_ttm_unmap_from_dma(struct vmw_ttm_tt *vmw_tt) { struct device *dev = vmw_tt->dev_priv->dev->dev;
- dma_unmap_sg(dev, vmw_tt->sgt.sgl, vmw_tt->sgt.nents, - DMA_BIDIRECTIONAL); + dma_unmap_sgtable(dev, vmw_tt->sgt, DMA_BIDIRECTIONAL, 0); vmw_tt->sgt.nents = vmw_tt->sgt.orig_nents; }
@@ -383,16 +382,8 @@ static void vmw_ttm_unmap_from_dma(struct vmw_ttm_tt *vmw_tt) static int vmw_ttm_map_for_dma(struct vmw_ttm_tt *vmw_tt) { struct device *dev = vmw_tt->dev_priv->dev->dev; - int ret; - - ret = dma_map_sg(dev, vmw_tt->sgt.sgl, vmw_tt->sgt.orig_nents, - DMA_BIDIRECTIONAL); - if (unlikely(ret == 0)) - return -ENOMEM;
- vmw_tt->sgt.nents = ret; - - return 0; + return dma_map_sgtable(dev, vmw_tt->sgt, DMA_BIDIRECTIONAL, 0); }
/** @@ -449,10 +440,10 @@ static int vmw_ttm_map_dma(struct vmw_ttm_tt *vmw_tt) if (unlikely(ret != 0)) goto out_sg_alloc_fail;
- if (vsgt->num_pages > vmw_tt->sgt.nents) { + if (vsgt->num_pages > vmw_tt->sgt.orig_nents) { uint64_t over_alloc = sgl_size * (vsgt->num_pages - - vmw_tt->sgt.nents); + vmw_tt->sgt.orig_nents);
ttm_mem_global_free(glob, over_alloc); vmw_tt->sg_alloc_size -= over_alloc;
Hi Marek,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20200618] [also build test WARNING on v5.8-rc1] [cannot apply to linuxtv-media/master staging/staging-testing drm-exynos/exynos-drm-next drm-intel/for-linux-next linus/master v5.8-rc1 v5.7 v5.7-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Marek-Szyprowski/DRM-fix-struct-sg_... base: ce2cc8efd7a40cbd17841add878cb691d0ce0bba config: x86_64-randconfig-s021-20200618 (attached as .config) compiler: gcc-9 (Debian 9.3.0-13) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-rc1-10-gc17b1b06-dirty # save the attached .config to linux build tree make W=1 C=1 ARCH=x86_64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
sparse warnings: (new ones prefixed by >>)
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c:365:38: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected struct sg_table *sgt @@ got struct sg_table sgt @@ drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c:365:38: sparse: expected struct sg_table *sgt drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c:365:38: sparse: got struct sg_table sgt
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c:386:43: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected struct sg_table *sgt @@ got struct sg_table sgt @@ drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c:386:43: sparse: expected struct sg_table *sgt drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c:386:43: sparse: got struct sg_table sgt
vim +365 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
352 353 /** 354 * vmw_ttm_unmap_from_dma - unmap device addresses previsouly mapped for 355 * TTM pages 356 * 357 * @vmw_tt: Pointer to a struct vmw_ttm_backend 358 * 359 * Used to free dma mappings previously mapped by vmw_ttm_map_for_dma. 360 */ 361 static void vmw_ttm_unmap_from_dma(struct vmw_ttm_tt *vmw_tt) 362 { 363 struct device *dev = vmw_tt->dev_priv->dev->dev; 364
365 dma_unmap_sgtable(dev, vmw_tt->sgt, DMA_BIDIRECTIONAL, 0);
366 vmw_tt->sgt.nents = vmw_tt->sgt.orig_nents; 367 } 368
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Marek,
I love your patch! Yet something to improve:
[auto build test ERROR on next-20200618] [also build test ERROR on v5.8-rc1] [cannot apply to linuxtv-media/master staging/staging-testing drm-exynos/exynos-drm-next drm-intel/for-linux-next linus/master v5.8-rc1 v5.7 v5.7-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Marek-Szyprowski/DRM-fix-struct-sg_... base: ce2cc8efd7a40cbd17841add878cb691d0ce0bba config: x86_64-rhel-7.6 (attached as .config) compiler: gcc-9 (Debian 9.3.0-13) 9.3.0 reproduce (this is a W=1 build): # save the attached .config to linux build tree make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All error/warnings (new ones prefixed by >>):
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c: In function 'vmw_ttm_unmap_from_dma':
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c:365:31: error: incompatible type for argument 2 of 'dma_unmap_sgtable'
365 | dma_unmap_sgtable(dev, vmw_tt->sgt, DMA_BIDIRECTIONAL, 0); | ~~~~~~^~~~~ | | | struct sg_table In file included from include/linux/dma-buf.h:20, from drivers/gpu/drm/vmwgfx/ttm_object.h:40, from drivers/gpu/drm/vmwgfx/ttm_lock.h:55, from drivers/gpu/drm/vmwgfx/vmwgfx_drv.h:44, from drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c:28: include/linux/dma-mapping.h:651:75: note: expected 'struct sg_table *' but argument is of type 'struct sg_table' 651 | static inline void dma_unmap_sgtable(struct device *dev, struct sg_table *sgt, | ~~~~~~~~~~~~~~~~~^~~ drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c: In function 'vmw_ttm_map_for_dma':
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c:386:36: error: incompatible type for argument 2 of 'dma_map_sgtable'
386 | return dma_map_sgtable(dev, vmw_tt->sgt, DMA_BIDIRECTIONAL, 0); | ~~~~~~^~~~~ | | | struct sg_table In file included from include/linux/dma-buf.h:20, from drivers/gpu/drm/vmwgfx/ttm_object.h:40, from drivers/gpu/drm/vmwgfx/ttm_lock.h:55, from drivers/gpu/drm/vmwgfx/vmwgfx_drv.h:44, from drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c:28: include/linux/dma-mapping.h:628:72: note: expected 'struct sg_table *' but argument is of type 'struct sg_table' 628 | static inline int dma_map_sgtable(struct device *dev, struct sg_table *sgt, | ~~~~~~~~~~~~~~~~~^~~
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c:387:1: warning: control reaches end of non-void function [-Wreturn-type]
387 | } | ^
vim +/dma_unmap_sgtable +365 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
352 353 /** 354 * vmw_ttm_unmap_from_dma - unmap device addresses previsouly mapped for 355 * TTM pages 356 * 357 * @vmw_tt: Pointer to a struct vmw_ttm_backend 358 * 359 * Used to free dma mappings previously mapped by vmw_ttm_map_for_dma. 360 */ 361 static void vmw_ttm_unmap_from_dma(struct vmw_ttm_tt *vmw_tt) 362 { 363 struct device *dev = vmw_tt->dev_priv->dev->dev; 364
365 dma_unmap_sgtable(dev, vmw_tt->sgt, DMA_BIDIRECTIONAL, 0);
366 vmw_tt->sgt.nents = vmw_tt->sgt.orig_nents; 367 } 368 369 /** 370 * vmw_ttm_map_for_dma - map TTM pages to get device addresses 371 * 372 * @vmw_tt: Pointer to a struct vmw_ttm_backend 373 * 374 * This function is used to get device addresses from the kernel DMA layer. 375 * However, it's violating the DMA API in that when this operation has been 376 * performed, it's illegal for the CPU to write to the pages without first 377 * unmapping the DMA mappings, or calling dma_sync_sg_for_cpu(). It is 378 * therefore only legal to call this function if we know that the function 379 * dma_sync_sg_for_cpu() is a NOP, and dma_sync_sg_for_device() is at most 380 * a CPU write buffer flush. 381 */ 382 static int vmw_ttm_map_for_dma(struct vmw_ttm_tt *vmw_tt) 383 { 384 struct device *dev = vmw_tt->dev_priv->dev->dev; 385
386 return dma_map_sgtable(dev, vmw_tt->sgt, DMA_BIDIRECTIONAL, 0); 387 }
388
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Acked-by: Juergen Gross jgross@suse.com --- drivers/xen/gntdev-dmabuf.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c index 75d3bb948bf3..ba6cad871568 100644 --- a/drivers/xen/gntdev-dmabuf.c +++ b/drivers/xen/gntdev-dmabuf.c @@ -247,10 +247,9 @@ static void dmabuf_exp_ops_detach(struct dma_buf *dma_buf,
if (sgt) { if (gntdev_dmabuf_attach->dir != DMA_NONE) - dma_unmap_sg_attrs(attach->dev, sgt->sgl, - sgt->nents, - gntdev_dmabuf_attach->dir, - DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(attach->dev, sgt, + gntdev_dmabuf_attach->dir, + DMA_ATTR_SKIP_CPU_SYNC); sg_free_table(sgt); }
@@ -288,8 +287,8 @@ dmabuf_exp_ops_map_dma_buf(struct dma_buf_attachment *attach, sgt = dmabuf_pages_to_sgt(gntdev_dmabuf->pages, gntdev_dmabuf->nr_pages); if (!IS_ERR(sgt)) { - if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir, - DMA_ATTR_SKIP_CPU_SYNC)) { + if (dma_map_sgtable(attach->dev, sgt, dir, + DMA_ATTR_SKIP_CPU_SYNC)) { sg_free_table(sgt); kfree(sgt); sgt = ERR_PTR(-ENOMEM); @@ -625,7 +624,7 @@ dmabuf_imp_to_refs(struct gntdev_dmabuf_priv *priv, struct device *dev,
/* Now convert sgt to array of pages and check for page validity. */ i = 0; - for_each_sg_page(sgt->sgl, &sg_iter, sgt->nents, 0) { + for_each_sgtable_page(sgt, &sg_iter, 0) { struct page *page = sg_page_iter_page(&sg_iter); /* * Check if page is valid: this can happen if we are given
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/host1x/job.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c index a10643aa89aa..4832b57f10c4 100644 --- a/drivers/gpu/host1x/job.c +++ b/drivers/gpu/host1x/job.c @@ -166,11 +166,9 @@ static unsigned int pin_job(struct host1x *host, struct host1x_job *job) goto unpin; }
- err = dma_map_sg(dev, sgt->sgl, sgt->nents, dir); - if (!err) { - err = -ENOMEM; + err = dma_map_sgtable(dev, sgt, dir, 0); + if (err) goto unpin; - }
job->unpins[job->num_unpins].dev = dev; job->unpins[job->num_unpins].dir = dir; @@ -217,7 +215,7 @@ static unsigned int pin_job(struct host1x *host, struct host1x_job *job) }
if (!IS_ENABLED(CONFIG_TEGRA_HOST1X_FIREWALL) && host->domain) { - for_each_sg(sgt->sgl, sg, sgt->nents, j) + for_each_sgtable_sg(sgt, sg, j) gather_size += sg->length; gather_size = iova_align(&host->iova, gather_size);
@@ -229,9 +227,9 @@ static unsigned int pin_job(struct host1x *host, struct host1x_job *job) goto unpin; }
- err = iommu_map_sg(host->domain, + err = iommu_map_sgtable(host->domain, iova_dma_addr(&host->iova, alloc), - sgt->sgl, sgt->nents, IOMMU_READ); + sgt, IOMMU_READ); if (err == 0) { __free_iova(&host->iova, alloc); err = -EINVAL; @@ -241,12 +239,9 @@ static unsigned int pin_job(struct host1x *host, struct host1x_job *job) job->unpins[job->num_unpins].size = gather_size; phys_addr = iova_dma_addr(&host->iova, alloc); } else if (sgt) { - err = dma_map_sg(host->dev, sgt->sgl, sgt->nents, - DMA_TO_DEVICE); - if (!err) { - err = -ENOMEM; + err = dma_map_sgtable(host->dev, sgt, DMA_TO_DEVICE, 0); + if (err) goto unpin; - }
job->unpins[job->num_unpins].dir = DMA_TO_DEVICE; job->unpins[job->num_unpins].dev = host->dev; @@ -647,8 +642,7 @@ void host1x_job_unpin(struct host1x_job *job) }
if (unpin->dev && sgt) - dma_unmap_sg(unpin->dev, sgt->sgl, sgt->nents, - unpin->dir); + dma_unmap_sgtable(unpin->dev, sgt, unpin->dir, 0);
host1x_bo_unpin(dev, unpin->bo, sgt); host1x_bo_put(unpin->bo);
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
dma_map_sgtable() function returns zero or an error code, so adjust the return value check for the vsp1_du_map_sg() function.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 +-- drivers/media/platform/vsp1/vsp1_drm.c | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index f1a81c9b184d..a27bff999649 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -197,9 +197,8 @@ int rcar_du_vsp_map_fb(struct rcar_du_vsp *vsp, struct drm_framebuffer *fb, goto fail;
ret = vsp1_du_map_sg(vsp->vsp, sgt); - if (!ret) { + if (ret) { sg_free_table(sgt); - ret = -ENOMEM; goto fail; } } diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index a4a45d68a6ef..86d5e3f4b1ff 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -912,8 +912,8 @@ int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt) * skip cache sync. This will need to be revisited when support for * non-coherent buffers will be added to the DU driver. */ - return dma_map_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents, - DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); + return dma_map_sgtable(vsp1->bus_master, sgt, DMA_TO_DEVICE, + DMA_ATTR_SKIP_CPU_SYNC); } EXPORT_SYMBOL_GPL(vsp1_du_map_sg);
@@ -921,8 +921,8 @@ void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt) { struct vsp1_device *vsp1 = dev_get_drvdata(dev);
- dma_unmap_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents, - DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(vsp1->bus_master, sgt, DMA_TO_DEVICE, + DMA_ATTR_SKIP_CPU_SYNC); } EXPORT_SYMBOL_GPL(vsp1_du_unmap_sg);
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Acked-by: Gerd Hoffmann kraxel@redhat.com --- drivers/dma-buf/heaps/heap-helpers.c | 13 ++++++------- drivers/dma-buf/udmabuf.c | 7 +++---- 2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/dma-buf/heaps/heap-helpers.c b/drivers/dma-buf/heaps/heap-helpers.c index 9f964ca3f59c..d0696cf937af 100644 --- a/drivers/dma-buf/heaps/heap-helpers.c +++ b/drivers/dma-buf/heaps/heap-helpers.c @@ -140,13 +140,12 @@ struct sg_table *dma_heap_map_dma_buf(struct dma_buf_attachment *attachment, enum dma_data_direction direction) { struct dma_heaps_attachment *a = attachment->priv; - struct sg_table *table; - - table = &a->table; + struct sg_table *table = &a->table; + int ret;
- if (!dma_map_sg(attachment->dev, table->sgl, table->nents, - direction)) - table = ERR_PTR(-ENOMEM); + ret = dma_map_sgtable(attachment->dev, table, direction, 0); + if (ret) + table = ERR_PTR(ret); return table; }
@@ -154,7 +153,7 @@ static void dma_heap_unmap_dma_buf(struct dma_buf_attachment *attachment, struct sg_table *table, enum dma_data_direction direction) { - dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction); + dma_unmap_sgtable(attachment->dev, table, direction, 0); }
static vm_fault_t dma_heap_vm_fault(struct vm_fault *vmf) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index acb26c627d27..89e293bd9252 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -63,10 +63,9 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, GFP_KERNEL); if (ret < 0) goto err; - if (!dma_map_sg(dev, sg->sgl, sg->nents, direction)) { - ret = -EINVAL; + ret = dma_map_sgtable(dev, sg, direction, 0); + if (ret < 0) goto err; - } return sg;
err: @@ -78,7 +77,7 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, static void put_sg_table(struct device *dev, struct sg_table *sg, enum dma_data_direction direction) { - dma_unmap_sg(dev, sg->sgl, sg->nents, direction); + dma_unmap_sgtable(dev, sg, direction, 0); sg_free_table(sg); kfree(sg); }
ion_heap_pages_zero() function is not used at all, so remove it to simplify the ion_heap_sglist_zero() function later.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/staging/android/ion/ion.h | 1 - drivers/staging/android/ion/ion_heap.c | 9 --------- 2 files changed, 10 deletions(-)
diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h index 74914a266e25..c199e88afc6c 100644 --- a/drivers/staging/android/ion/ion.h +++ b/drivers/staging/android/ion/ion.h @@ -177,7 +177,6 @@ void ion_heap_unmap_kernel(struct ion_heap *heap, struct ion_buffer *buffer); int ion_heap_map_user(struct ion_heap *heap, struct ion_buffer *buffer, struct vm_area_struct *vma); int ion_heap_buffer_zero(struct ion_buffer *buffer); -int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot);
/** * ion_heap_init_shrinker diff --git a/drivers/staging/android/ion/ion_heap.c b/drivers/staging/android/ion/ion_heap.c index 0755b11348ed..9c23b2382a1e 100644 --- a/drivers/staging/android/ion/ion_heap.c +++ b/drivers/staging/android/ion/ion_heap.c @@ -145,15 +145,6 @@ int ion_heap_buffer_zero(struct ion_buffer *buffer) return ion_heap_sglist_zero(table->sgl, table->nents, pgprot); }
-int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot) -{ - struct scatterlist sg; - - sg_init_table(&sg, 1); - sg_set_page(&sg, page, size, 0); - return ion_heap_sglist_zero(&sg, 1, pgprot); -} - void ion_heap_freelist_add(struct ion_heap *heap, struct ion_buffer *buffer) { spin_lock(&heap->free_lock);
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/staging/android/ion/ion.c | 25 +++++------ drivers/staging/android/ion/ion_heap.c | 44 ++++++------------- drivers/staging/android/ion/ion_system_heap.c | 2 +- 3 files changed, 25 insertions(+), 46 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 38b51eace4f9..3c9f09506ffa 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -147,14 +147,14 @@ static struct sg_table *dup_sg_table(struct sg_table *table) if (!new_table) return ERR_PTR(-ENOMEM);
- ret = sg_alloc_table(new_table, table->nents, GFP_KERNEL); + ret = sg_alloc_table(new_table, table->orig_nents, GFP_KERNEL); if (ret) { kfree(new_table); return ERR_PTR(-ENOMEM); }
new_sg = new_table->sgl; - for_each_sg(table->sgl, sg, table->nents, i) { + for_each_sgtable_sg(table, sg, i) { memcpy(new_sg, sg, sizeof(*sg)); new_sg->dma_address = 0; new_sg = sg_next(new_sg); @@ -224,12 +224,13 @@ static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment, { struct ion_dma_buf_attachment *a = attachment->priv; struct sg_table *table; + int ret;
table = a->table;
- if (!dma_map_sg(attachment->dev, table->sgl, table->nents, - direction)) - return ERR_PTR(-ENOMEM); + ret = dma_map_sgtable(attachment->dev, table, direction, 0); + if (ret) + return ERR_PTR(ret);
return table; } @@ -238,7 +239,7 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment, struct sg_table *table, enum dma_data_direction direction) { - dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction); + dma_unmap_sgtable(attachment->dev, table, direction, 0); }
static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) @@ -296,10 +297,8 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, }
mutex_lock(&buffer->lock); - list_for_each_entry(a, &buffer->attachments, list) { - dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents, - direction); - } + list_for_each_entry(a, &buffer->attachments, list) + dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
unlock: mutex_unlock(&buffer->lock); @@ -319,10 +318,8 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf, }
mutex_lock(&buffer->lock); - list_for_each_entry(a, &buffer->attachments, list) { - dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents, - direction); - } + list_for_each_entry(a, &buffer->attachments, list) + dma_sync_sgtable_for_device(a->dev, a->table, direction); mutex_unlock(&buffer->lock);
return 0; diff --git a/drivers/staging/android/ion/ion_heap.c b/drivers/staging/android/ion/ion_heap.c index 9c23b2382a1e..79f27949edda 100644 --- a/drivers/staging/android/ion/ion_heap.c +++ b/drivers/staging/android/ion/ion_heap.c @@ -20,8 +20,7 @@ void *ion_heap_map_kernel(struct ion_heap *heap, struct ion_buffer *buffer) { - struct scatterlist *sg; - int i, j; + struct sg_page_iter piter; void *vaddr; pgprot_t pgprot; struct sg_table *table = buffer->sg_table; @@ -38,14 +37,11 @@ void *ion_heap_map_kernel(struct ion_heap *heap, else pgprot = pgprot_writecombine(PAGE_KERNEL);
- for_each_sg(table->sgl, sg, table->nents, i) { - int npages_this_entry = PAGE_ALIGN(sg->length) / PAGE_SIZE; - struct page *page = sg_page(sg); - - BUG_ON(i >= npages); - for (j = 0; j < npages_this_entry; j++) - *(tmp++) = page++; + for_each_sgtable_page(table, &piter, 0) { + BUG_ON(tmp - pages >= npages); + *tmp++ = sg_page_iter_page(&piter); } + vaddr = vmap(pages, npages, VM_MAP, pgprot); vfree(pages);
@@ -64,32 +60,19 @@ void ion_heap_unmap_kernel(struct ion_heap *heap, int ion_heap_map_user(struct ion_heap *heap, struct ion_buffer *buffer, struct vm_area_struct *vma) { + struct sg_page_iter piter; struct sg_table *table = buffer->sg_table; unsigned long addr = vma->vm_start; - unsigned long offset = vma->vm_pgoff * PAGE_SIZE; - struct scatterlist *sg; - int i; int ret;
- for_each_sg(table->sgl, sg, table->nents, i) { - struct page *page = sg_page(sg); - unsigned long remainder = vma->vm_end - addr; - unsigned long len = sg->length; + for_each_sgtable_page(table, &piter, vma->vm_pgoff) { + struct page *page = sg_page_iter_page(&piter);
- if (offset >= sg->length) { - offset -= sg->length; - continue; - } else if (offset) { - page += offset / PAGE_SIZE; - len = sg->length - offset; - offset = 0; - } - len = min(len, remainder); - ret = remap_pfn_range(vma, addr, page_to_pfn(page), len, + ret = remap_pfn_range(vma, addr, page_to_pfn(page), PAGE_SIZE, vma->vm_page_prot); if (ret) return ret; - addr += len; + addr += PAGE_SIZE; if (addr >= vma->vm_end) return 0; } @@ -109,15 +92,14 @@ static int ion_heap_clear_pages(struct page **pages, int num, pgprot_t pgprot) return 0; }
-static int ion_heap_sglist_zero(struct scatterlist *sgl, unsigned int nents, - pgprot_t pgprot) +static int ion_heap_sglist_zero(struct sg_table *sgt, pgprot_t pgprot) { int p = 0; int ret = 0; struct sg_page_iter piter; struct page *pages[32];
- for_each_sg_page(sgl, &piter, nents, 0) { + for_each_sgtable_page(sgt, &piter, 0) { pages[p++] = sg_page_iter_page(&piter); if (p == ARRAY_SIZE(pages)) { ret = ion_heap_clear_pages(pages, p, pgprot); @@ -142,7 +124,7 @@ int ion_heap_buffer_zero(struct ion_buffer *buffer) else pgprot = pgprot_writecombine(PAGE_KERNEL);
- return ion_heap_sglist_zero(table->sgl, table->nents, pgprot); + return ion_heap_sglist_zero(table, pgprot); }
void ion_heap_freelist_add(struct ion_heap *heap, struct ion_buffer *buffer) diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c index b83a1d16bd89..eac0632ab4e8 100644 --- a/drivers/staging/android/ion/ion_system_heap.c +++ b/drivers/staging/android/ion/ion_system_heap.c @@ -162,7 +162,7 @@ static void ion_system_heap_free(struct ion_buffer *buffer) if (!(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) ion_heap_buffer_zero(buffer);
- for_each_sg(table->sgl, sg, table->nents, i) + for_each_sgtable_sg(table, sg, i) free_buffer_page(sys_heap, buffer, sg_page(sg)); sg_free_table(table); kfree(table);
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Reviewed-by: Dmitry Osipenko digetx@gmail.com --- drivers/staging/media/tegra-vde/iommu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/tegra-vde/iommu.c b/drivers/staging/media/tegra-vde/iommu.c index 6af863d92123..adf8dc7ee25c 100644 --- a/drivers/staging/media/tegra-vde/iommu.c +++ b/drivers/staging/media/tegra-vde/iommu.c @@ -36,8 +36,8 @@ int tegra_vde_iommu_map(struct tegra_vde *vde,
addr = iova_dma_addr(&vde->iova, iova);
- size = iommu_map_sg(vde->domain, addr, sgt->sgl, sgt->nents, - IOMMU_READ | IOMMU_WRITE); + size = iommu_map_sgtable(vde->domain, addr, sgt, + IOMMU_READ | IOMMU_WRITE); if (!size) { __free_iova(&vde->iova, iova); return -ENXIO;
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/misc/fastrpc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 7939c55daceb..9d6867749316 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -518,7 +518,7 @@ fastrpc_map_dma_buf(struct dma_buf_attachment *attachment,
table = &a->sgt;
- if (!dma_map_sg(attachment->dev, table->sgl, table->nents, dir)) + if (!dma_map_sgtable(attachment->dev, table, dir, 0)) return ERR_PTR(-ENOMEM);
return table; @@ -528,7 +528,7 @@ static void fastrpc_unmap_dma_buf(struct dma_buf_attachment *attach, struct sg_table *table, enum dma_data_direction dir) { - dma_unmap_sg(attach->dev, table->sgl, table->nents, dir); + dma_unmap_sgtable(attach->dev, table, dir, 0); }
static void fastrpc_release(struct dma_buf *dmabuf)
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/rapidio/devices/rio_mport_cdev.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c index 451608e960a1..a33cc1b6beb2 100644 --- a/drivers/rapidio/devices/rio_mport_cdev.c +++ b/drivers/rapidio/devices/rio_mport_cdev.c @@ -573,8 +573,7 @@ static void dma_req_free(struct kref *ref) refcount); struct mport_cdev_priv *priv = req->priv;
- dma_unmap_sg(req->dmach->device->dev, - req->sgt.sgl, req->sgt.nents, req->dir); + dma_unmap_sgtable(req->dmach->device->dev, req->sgt, req->dir, 0); sg_free_table(&req->sgt); if (req->page_list) { unpin_user_pages(req->page_list, req->nr_pages); @@ -930,9 +929,8 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode, xfer->offset, xfer->length); }
- nents = dma_map_sg(chan->device->dev, - req->sgt.sgl, req->sgt.nents, dir); - if (nents == 0) { + ret = dma_map_sgtable(chan->device->dev, req->sgt, dir, 0); + if (ret) { rmcd_error("Failed to map SG list"); ret = -EFAULT; goto err_pg;
Hi Marek,
I love your patch! Yet something to improve:
[auto build test ERROR on next-20200618] [also build test ERROR on v5.8-rc1] [cannot apply to linuxtv-media/master staging/staging-testing drm-exynos/exynos-drm-next drm-intel/for-linux-next linus/master v5.8-rc1 v5.7 v5.7-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Marek-Szyprowski/DRM-fix-struct-sg_... base: ce2cc8efd7a40cbd17841add878cb691d0ce0bba config: alpha-allyesconfig (attached as .config) compiler: alpha-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
In file included from include/linux/dma-mapping.h:11, from drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:25: drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c: In function 'amdgpu_vram_mgr_alloc_sgt':
include/linux/scatterlist.h:158:17: error: '*sgt' is a pointer; did you mean to use '->'?
158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i) | ^~ include/linux/scatterlist.h:152:22: note: in definition of macro 'for_each_sg' 152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg)) | ^~~~~~ drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:480:2: note: in expansion of macro 'for_each_sgtable_sg' 480 | for_each_sgtable_sg(*sgt, sg, i) | ^~~~~~~~~~~~~~~~~~~ include/linux/scatterlist.h:158:31: error: '*sgt' is a pointer; did you mean to use '->'? 158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i) | ^~ include/linux/scatterlist.h:152:38: note: in definition of macro 'for_each_sg' 152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg)) | ^~ drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:480:2: note: in expansion of macro 'for_each_sgtable_sg' 480 | for_each_sgtable_sg(*sgt, sg, i) | ^~~~~~~~~~~~~~~~~~~
include/linux/scatterlist.h:158:17: error: '*sgt' is a pointer; did you mean to use '->'?
158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i) | ^~ include/linux/scatterlist.h:152:22: note: in definition of macro 'for_each_sg' 152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg)) | ^~~~~~ drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:484:2: note: in expansion of macro 'for_each_sgtable_sg' 484 | for_each_sgtable_sg(*sgt, sg, i) { | ^~~~~~~~~~~~~~~~~~~ include/linux/scatterlist.h:158:31: error: '*sgt' is a pointer; did you mean to use '->'? 158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i) | ^~ include/linux/scatterlist.h:152:38: note: in definition of macro 'for_each_sg' 152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg)) | ^~ drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:484:2: note: in expansion of macro 'for_each_sgtable_sg' 484 | for_each_sgtable_sg(*sgt, sg, i) { | ^~~~~~~~~~~~~~~~~~~
include/linux/scatterlist.h:158:17: error: '*sgt' is a pointer; did you mean to use '->'?
158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i) | ^~ include/linux/scatterlist.h:152:22: note: in definition of macro 'for_each_sg' 152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg)) | ^~~~~~ drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:504:2: note: in expansion of macro 'for_each_sgtable_sg' 504 | for_each_sgtable_sg(*sgt, sg, i) { | ^~~~~~~~~~~~~~~~~~~ include/linux/scatterlist.h:158:31: error: '*sgt' is a pointer; did you mean to use '->'? 158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i) | ^~ include/linux/scatterlist.h:152:38: note: in definition of macro 'for_each_sg' 152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg)) | ^~ drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:504:2: note: in expansion of macro 'for_each_sgtable_sg' 504 | for_each_sgtable_sg(*sgt, sg, i) { | ^~~~~~~~~~~~~~~~~~~ In file included from drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:26: At top level: drivers/gpu/drm/amd/amdgpu/amdgpu.h:190:18: warning: 'sched_policy' defined but not used [-Wunused-const-variable=] 190 | static const int sched_policy = KFD_SCHED_POLICY_HWS; | ^~~~~~~~~~~~ In file included from drivers/gpu/drm/amd/amdgpu/../display/dc/dc_types.h:33, from drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:30, from drivers/gpu/drm/amd/amdgpu/../include/dm_pp_interface.h:26, from drivers/gpu/drm/amd/amdgpu/amdgpu.h:65, from drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:26: drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:76:32: warning: 'dc_fixpt_ln2_div_2' defined but not used [-Wunused-const-variable=] 76 | static const struct fixed31_32 dc_fixpt_ln2_div_2 = { 1488522236LL }; | ^~~~~~~~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:75:32: warning: 'dc_fixpt_ln2' defined but not used [-Wunused-const-variable=] 75 | static const struct fixed31_32 dc_fixpt_ln2 = { 2977044471LL }; | ^~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:74:32: warning: 'dc_fixpt_e' defined but not used [-Wunused-const-variable=] 74 | static const struct fixed31_32 dc_fixpt_e = { 11674931555LL }; | ^~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:73:32: warning: 'dc_fixpt_two_pi' defined but not used [-Wunused-const-variable=] 73 | static const struct fixed31_32 dc_fixpt_two_pi = { 26986075409LL }; | ^~~~~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:72:32: warning: 'dc_fixpt_pi' defined but not used [-Wunused-const-variable=] 72 | static const struct fixed31_32 dc_fixpt_pi = { 13493037705LL }; | ^~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:67:32: warning: 'dc_fixpt_zero' defined but not used [-Wunused-const-variable=] 67 | static const struct fixed31_32 dc_fixpt_zero = { 0 }; | ^~~~~~~~~~~~~ -- In file included from include/linux/kernel.h:11, from include/linux/delay.h:22, from drivers/gpu/drm/v3d/v3d_drv.h:4, from drivers/gpu/drm/v3d/v3d_mmu.c:21: drivers/gpu/drm/v3d/v3d_mmu.c: In function 'v3d_mmu_insert_ptes':
include/linux/compiler.h:339:38: error: call to '__compiletime_assert_254' declared with attribute error: BUILD_BUG_ON failed: V3D_MMU_PAGE_SHIFT != PAGE_SIZE
339 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^ include/linux/compiler.h:320:4: note: in definition of macro '__compiletime_assert' 320 | prefix ## suffix(); \ | ^~~~~~ include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert' 339 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^~~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert' 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) | ^~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:50:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' 50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) | ^~~~~~~~~~~~~~~~ drivers/gpu/drm/v3d/v3d_mmu.c:100:3: note: in expansion of macro 'BUILD_BUG_ON' 100 | BUILD_BUG_ON(V3D_MMU_PAGE_SHIFT != PAGE_SIZE); | ^~~~~~~~~~~~ -- drivers/rapidio/devices/rio_mport_cdev.c: In function 'dma_req_free':
drivers/rapidio/devices/rio_mport_cdev.c:576:48: error: incompatible type for argument 2 of 'dma_unmap_sgtable'
576 | dma_unmap_sgtable(req->dmach->device->dev, req->sgt, req->dir, 0); | ~~~^~~~~ | | | struct sg_table In file included from drivers/rapidio/devices/rio_mport_cdev.c:32: include/linux/dma-mapping.h:651:75: note: expected 'struct sg_table *' but argument is of type 'struct sg_table' 651 | static inline void dma_unmap_sgtable(struct device *dev, struct sg_table *sgt, | ~~~~~~~~~~~~~~~~~^~~ drivers/rapidio/devices/rio_mport_cdev.c: In function 'rio_dma_transfer':
drivers/rapidio/devices/rio_mport_cdev.c:932:46: error: incompatible type for argument 2 of 'dma_map_sgtable'
932 | ret = dma_map_sgtable(chan->device->dev, req->sgt, dir, 0); | ~~~^~~~~ | | | struct sg_table In file included from drivers/rapidio/devices/rio_mport_cdev.c:32: include/linux/dma-mapping.h:628:72: note: expected 'struct sg_table *' but argument is of type 'struct sg_table' 628 | static inline int dma_map_sgtable(struct device *dev, struct sg_table *sgt, | ~~~~~~~~~~~~~~~~~^~~
vim +/dma_unmap_sgtable +576 drivers/rapidio/devices/rio_mport_cdev.c
569 570 static void dma_req_free(struct kref *ref) 571 { 572 struct mport_dma_req *req = container_of(ref, struct mport_dma_req, 573 refcount); 574 struct mport_cdev_priv *priv = req->priv; 575
576 dma_unmap_sgtable(req->dmach->device->dev, req->sgt, req->dir, 0);
577 sg_free_table(&req->sgt); 578 if (req->page_list) { 579 unpin_user_pages(req->page_list, req->nr_pages); 580 kfree(req->page_list); 581 } 582 583 if (req->map) { 584 mutex_lock(&req->map->md->buf_mutex); 585 kref_put(&req->map->ref, mport_release_mapping); 586 mutex_unlock(&req->map->md->buf_mutex); 587 } 588 589 kref_put(&priv->dma_ref, mport_release_dma); 590 591 kfree(req); 592 } 593
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe.
While touching this code, also add missing call to dma_unmap_sgtable.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- samples/vfio-mdev/mbochs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c index 3cc5e5921682..e03068917273 100644 --- a/samples/vfio-mdev/mbochs.c +++ b/samples/vfio-mdev/mbochs.c @@ -846,7 +846,7 @@ static struct sg_table *mbochs_map_dmabuf(struct dma_buf_attachment *at, if (sg_alloc_table_from_pages(sg, dmabuf->pages, dmabuf->pagecount, 0, dmabuf->mode.size, GFP_KERNEL) < 0) goto err2; - if (!dma_map_sg(at->dev, sg->sgl, sg->nents, direction)) + if (dma_map_sgtable(at->dev, sg, direction, 0)) goto err3;
return sg; @@ -868,6 +868,7 @@ static void mbochs_unmap_dmabuf(struct dma_buf_attachment *at,
dev_dbg(dev, "%s: %d\n", __func__, dmabuf->id);
+ dma_unmap_sgtable(at->dev, sg, direction, 0); sg_free_table(sg); kfree(sg); }
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the numer of the created entries in the DMA address space. However the subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be called with the original number of entries passed to dma_map_sg. The sg_table->nents in turn holds the result of the dma_map_sg call as stated in include/linux/scatterlist.h. Adapt the code to obey those rules.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/media/pci/cx23885/cx23885-alsa.c | 2 +- drivers/media/pci/cx25821/cx25821-alsa.c | 2 +- drivers/media/pci/cx88/cx88-alsa.c | 2 +- drivers/media/pci/saa7134/saa7134-alsa.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/pci/cx23885/cx23885-alsa.c b/drivers/media/pci/cx23885/cx23885-alsa.c index df44ed7393a0..3f366e4e4685 100644 --- a/drivers/media/pci/cx23885/cx23885-alsa.c +++ b/drivers/media/pci/cx23885/cx23885-alsa.c @@ -129,7 +129,7 @@ static int cx23885_alsa_dma_unmap(struct cx23885_audio_dev *dev) if (!buf->sglen) return 0;
- dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->sglen, PCI_DMA_FROMDEVICE); + dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->nr_pages, PCI_DMA_FROMDEVICE); buf->sglen = 0; return 0; } diff --git a/drivers/media/pci/cx25821/cx25821-alsa.c b/drivers/media/pci/cx25821/cx25821-alsa.c index 301616426d8a..c40304d33776 100644 --- a/drivers/media/pci/cx25821/cx25821-alsa.c +++ b/drivers/media/pci/cx25821/cx25821-alsa.c @@ -193,7 +193,7 @@ static int cx25821_alsa_dma_unmap(struct cx25821_audio_dev *dev) if (!buf->sglen) return 0;
- dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->sglen, PCI_DMA_FROMDEVICE); + dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->nr_pages, PCI_DMA_FROMDEVICE); buf->sglen = 0; return 0; } diff --git a/drivers/media/pci/cx88/cx88-alsa.c b/drivers/media/pci/cx88/cx88-alsa.c index 7d7aceecc985..3c6fe6ceb0b7 100644 --- a/drivers/media/pci/cx88/cx88-alsa.c +++ b/drivers/media/pci/cx88/cx88-alsa.c @@ -332,7 +332,7 @@ static int cx88_alsa_dma_unmap(struct cx88_audio_dev *dev) if (!buf->sglen) return 0;
- dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->sglen, + dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->nr_pages, PCI_DMA_FROMDEVICE); buf->sglen = 0; return 0; diff --git a/drivers/media/pci/saa7134/saa7134-alsa.c b/drivers/media/pci/saa7134/saa7134-alsa.c index 544ca57eee75..398c47ff473d 100644 --- a/drivers/media/pci/saa7134/saa7134-alsa.c +++ b/drivers/media/pci/saa7134/saa7134-alsa.c @@ -313,7 +313,7 @@ static int saa7134_alsa_dma_unmap(struct saa7134_dev *dev) if (!dma->sglen) return 0;
- dma_unmap_sg(&dev->pci->dev, dma->sglist, dma->sglen, PCI_DMA_FROMDEVICE); + dma_unmap_sg(&dev->pci->dev, dma->sglist, dma->nr_pages, PCI_DMA_FROMDEVICE); dma->sglen = 0; return 0; }
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@samsung.com --- .../common/videobuf2/videobuf2-dma-contig.c | 41 ++++++++----------- .../media/common/videobuf2/videobuf2-dma-sg.c | 32 ++++++--------- .../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 f4b4a7c135eb..ba01a8692d88 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; } return size; } @@ -99,8 +98,7 @@ static void vb2_dc_prepare(void *buf_priv) if (!sgt || buf->db_attach) return;
- dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir); + dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir); }
static void vb2_dc_finish(void *buf_priv) @@ -112,7 +110,7 @@ static void vb2_dc_finish(void *buf_priv) if (!sgt || buf->db_attach) return;
- dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir); + dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir); }
/*********************************************/ @@ -273,8 +271,8 @@ static void vb2_dc_dmabuf_ops_detach(struct dma_buf *dbuf, * memory locations do not require any explicit cache * maintenance prior or after being used by the device. */ - dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents, - attach->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC); sg_free_table(sgt); kfree(attach); db_attach->priv = NULL; @@ -299,8 +297,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
/* release any previous cache */ if (attach->dma_dir != DMA_NONE) { - dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents, - attach->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC); attach->dma_dir = DMA_NONE; }
@@ -308,9 +306,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map( * mapping to the client with new direction, no cache sync * required see comment in vb2_dc_dmabuf_ops_detach() */ - sgt->nents = dma_map_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents, - dma_dir, DMA_ATTR_SKIP_CPU_SYNC); - if (!sgt->nents) { + if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, + DMA_ATTR_SKIP_CPU_SYNC)) { pr_err("failed to map scatterlist\n"); mutex_unlock(lock); return ERR_PTR(-EIO); @@ -423,8 +420,8 @@ static void vb2_dc_put_userptr(void *buf_priv) * No need to sync to CPU, it's already synced to the CPU * since the finish() memop will have been called before this. */ - dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC); pages = frame_vector_pages(buf->vec); /* sgt should exist only if vector contains pages... */ BUG_ON(IS_ERR(pages)); @@ -521,9 +518,8 @@ static void *vb2_dc_get_userptr(struct device *dev, 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, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); - if (sgt->nents <= 0) { + if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC)) { pr_err("failed to map scatterlist\n"); ret = -EIO; goto fail_sgt_init; @@ -545,8 +541,7 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr, return buf;
fail_map_sg: - dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
fail_sgt_init: sg_free_table(sgt); diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c index 92072a08af25..6ddf953efa11 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c @@ -142,9 +142,8 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs, * 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, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); - if (!sgt->nents) + if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC)) { goto fail_map;
buf->handler.refcount = &buf->refcount; @@ -180,8 +179,8 @@ static void vb2_dma_sg_put(void *buf_priv) if (refcount_dec_and_test(&buf->refcount)) { dprintk(1, "%s: Freeing buffer of %d pages\n", __func__, buf->num_pages); - dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC); if (buf->vaddr) vm_unmap_ram(buf->vaddr, buf->num_pages); sg_free_table(buf->dma_sgt); @@ -202,8 +201,7 @@ static void vb2_dma_sg_prepare(void *buf_priv) if (buf->db_attach) return;
- dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir); + dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir); }
static void vb2_dma_sg_finish(void *buf_priv) @@ -215,7 +213,7 @@ static void vb2_dma_sg_finish(void *buf_priv) if (buf->db_attach) return;
- dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir); + dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir); }
static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr, @@ -258,9 +256,8 @@ static void *vb2_dma_sg_get_userptr(struct device *dev, 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, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); - if (!sgt->nents) + if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC)) { goto userptr_fail_map;
return buf; @@ -286,8 +283,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
dprintk(1, "%s: Releasing userspace buffer of %d pages\n", __func__, buf->num_pages); - dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir, - DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); if (buf->vaddr) vm_unmap_ram(buf->vaddr, buf->num_pages); sg_free_table(buf->dma_sgt); @@ -410,8 +406,7 @@ static void vb2_dma_sg_dmabuf_ops_detach(struct dma_buf *dbuf,
/* release the scatterlist cache */ if (attach->dma_dir != DMA_NONE) - dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, - attach->dma_dir); + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir); sg_free_table(sgt); kfree(attach); db_attach->priv = NULL; @@ -436,15 +431,12 @@ static struct sg_table *vb2_dma_sg_dmabuf_ops_map(
/* release any previous cache */ if (attach->dma_dir != DMA_NONE) { - dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, - attach->dma_dir); + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir); attach->dma_dir = DMA_NONE; }
/* mapping to the client with new direction */ - sgt->nents = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, - dma_dir); - if (!sgt->nents) { + if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, 0)) { pr_err("failed to map scatterlist\n"); mutex_unlock(lock); return ERR_PTR(-EIO); diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c index c66fda4a65e4..bf5ac63a5742 100644 --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c @@ -229,7 +229,7 @@ static int vb2_vmalloc_dmabuf_ops_attach(struct dma_buf *dbuf, kfree(attach); return ret; } - for_each_sg(sgt->sgl, sg, sgt->nents, i) { + for_each_sgtable_sg(sgt, sg, i) { struct page *page = vmalloc_to_page(vaddr);
if (!page) { @@ -259,8 +259,7 @@ static void vb2_vmalloc_dmabuf_ops_detach(struct dma_buf *dbuf,
/* release the scatterlist cache */ if (attach->dma_dir != DMA_NONE) - dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, - attach->dma_dir); + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir, 0); sg_free_table(sgt); kfree(attach); db_attach->priv = NULL; @@ -285,15 +284,12 @@ static struct sg_table *vb2_vmalloc_dmabuf_ops_map(
/* release any previous cache */ if (attach->dma_dir != DMA_NONE) { - dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, - attach->dma_dir); + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir, 0); attach->dma_dir = DMA_NONE; }
/* mapping to the client with new direction */ - sgt->nents = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, - dma_dir); - if (!sgt->nents) { + if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, 0)) { pr_err("failed to map scatterlist\n"); mutex_unlock(lock); return ERR_PTR(-EIO);
Hi Marek,
I love your patch! Yet something to improve:
[auto build test ERROR on next-20200618] [also build test ERROR on v5.8-rc1] [cannot apply to linuxtv-media/master staging/staging-testing drm-exynos/exynos-drm-next drm-intel/for-linux-next linus/master v5.8-rc1 v5.7 v5.7-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Marek-Szyprowski/DRM-fix-struct-sg_... base: ce2cc8efd7a40cbd17841add878cb691d0ce0bba config: alpha-allyesconfig (attached as .config) compiler: alpha-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All error/warnings (new ones prefixed by >>):
drivers/media/common/videobuf2/videobuf2-dma-sg.c: In function 'vb2_dma_sg_alloc':
drivers/media/common/videobuf2/videobuf2-dma-sg.c:173:13: error: invalid storage class for function 'vb2_dma_sg_put'
173 | static void vb2_dma_sg_put(void *buf_priv) | ^~~~~~~~~~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:173:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
173 | static void vb2_dma_sg_put(void *buf_priv) | ^~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:195:13: error: invalid storage class for function 'vb2_dma_sg_prepare'
195 | static void vb2_dma_sg_prepare(void *buf_priv) | ^~~~~~~~~~~~~~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:207:13: error: invalid storage class for function 'vb2_dma_sg_finish'
207 | static void vb2_dma_sg_finish(void *buf_priv) | ^~~~~~~~~~~~~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:219:14: error: invalid storage class for function 'vb2_dma_sg_get_userptr'
219 | static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr, | ^~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c: In function 'vb2_dma_sg_get_userptr':
drivers/media/common/videobuf2/videobuf2-dma-sg.c:278:13: error: invalid storage class for function 'vb2_dma_sg_put_userptr'
278 | static void vb2_dma_sg_put_userptr(void *buf_priv) | ^~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:278:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] 278 | static void vb2_dma_sg_put_userptr(void *buf_priv) | ^~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:298:14: error: invalid storage class for function 'vb2_dma_sg_vaddr'
298 | static void *vb2_dma_sg_vaddr(void *buf_priv) | ^~~~~~~~~~~~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:315:21: error: invalid storage class for function 'vb2_dma_sg_num_users'
315 | static unsigned int vb2_dma_sg_num_users(void *buf_priv) | ^~~~~~~~~~~~~~~~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:322:12: error: invalid storage class for function 'vb2_dma_sg_mmap'
322 | static int vb2_dma_sg_mmap(void *buf_priv, struct vm_area_struct *vma) | ^~~~~~~~~~~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:358:12: error: invalid storage class for function 'vb2_dma_sg_dmabuf_ops_attach'
358 | static int vb2_dma_sg_dmabuf_ops_attach(struct dma_buf *dbuf, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:396:13: error: invalid storage class for function 'vb2_dma_sg_dmabuf_ops_detach'
396 | static void vb2_dma_sg_dmabuf_ops_detach(struct dma_buf *dbuf, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c: In function 'vb2_dma_sg_dmabuf_ops_detach':
drivers/media/common/videobuf2/videobuf2-dma-sg.c:409:3: error: too few arguments to function 'dma_unmap_sgtable'
409 | dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir); | ^~~~~~~~~~~~~~~~~ In file included from include/linux/dma-buf.h:20, from include/media/videobuf2-core.h:18, from include/media/videobuf2-v4l2.h:16, from drivers/media/common/videobuf2/videobuf2-dma-sg.c:21: include/linux/dma-mapping.h:651:20: note: declared here 651 | static inline void dma_unmap_sgtable(struct device *dev, struct sg_table *sgt, | ^~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c: In function 'vb2_dma_sg_get_userptr':
drivers/media/common/videobuf2/videobuf2-dma-sg.c:415:25: error: invalid storage class for function 'vb2_dma_sg_dmabuf_ops_map'
415 | static struct sg_table *vb2_dma_sg_dmabuf_ops_map( | ^~~~~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c: In function 'vb2_dma_sg_dmabuf_ops_map': drivers/media/common/videobuf2/videobuf2-dma-sg.c:434:3: error: too few arguments to function 'dma_unmap_sgtable' 434 | dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir); | ^~~~~~~~~~~~~~~~~ In file included from include/linux/dma-buf.h:20, from include/media/videobuf2-core.h:18, from include/media/videobuf2-v4l2.h:16, from drivers/media/common/videobuf2/videobuf2-dma-sg.c:21: include/linux/dma-mapping.h:651:20: note: declared here 651 | static inline void dma_unmap_sgtable(struct device *dev, struct sg_table *sgt, | ^~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c: In function 'vb2_dma_sg_get_userptr':
drivers/media/common/videobuf2/videobuf2-dma-sg.c:452:13: error: invalid storage class for function 'vb2_dma_sg_dmabuf_ops_unmap'
452 | static void vb2_dma_sg_dmabuf_ops_unmap(struct dma_buf_attachment *db_attach, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:458:13: error: invalid storage class for function 'vb2_dma_sg_dmabuf_ops_release'
458 | static void vb2_dma_sg_dmabuf_ops_release(struct dma_buf *dbuf) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:464:14: error: invalid storage class for function 'vb2_dma_sg_dmabuf_ops_vmap'
464 | static void *vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf) | ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:471:12: error: invalid storage class for function 'vb2_dma_sg_dmabuf_ops_mmap'
471 | static int vb2_dma_sg_dmabuf_ops_mmap(struct dma_buf *dbuf, | ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:478:12: error: initializer element is not constant
478 | .attach = vb2_dma_sg_dmabuf_ops_attach, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:478:12: note: (near initialization for 'vb2_dma_sg_dmabuf_ops.attach') drivers/media/common/videobuf2/videobuf2-dma-sg.c:479:12: error: initializer element is not constant 479 | .detach = vb2_dma_sg_dmabuf_ops_detach, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:479:12: note: (near initialization for 'vb2_dma_sg_dmabuf_ops.detach') drivers/media/common/videobuf2/videobuf2-dma-sg.c:480:17: error: initializer element is not constant 480 | .map_dma_buf = vb2_dma_sg_dmabuf_ops_map, | ^~~~~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:480:17: note: (near initialization for 'vb2_dma_sg_dmabuf_ops.map_dma_buf') drivers/media/common/videobuf2/videobuf2-dma-sg.c:481:19: error: initializer element is not constant 481 | .unmap_dma_buf = vb2_dma_sg_dmabuf_ops_unmap, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:481:19: note: (near initialization for 'vb2_dma_sg_dmabuf_ops.unmap_dma_buf') drivers/media/common/videobuf2/videobuf2-dma-sg.c:482:10: error: initializer element is not constant 482 | .vmap = vb2_dma_sg_dmabuf_ops_vmap, | ^~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:482:10: note: (near initialization for 'vb2_dma_sg_dmabuf_ops.vmap') drivers/media/common/videobuf2/videobuf2-dma-sg.c:483:10: error: initializer element is not constant 483 | .mmap = vb2_dma_sg_dmabuf_ops_mmap, | ^~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:483:10: note: (near initialization for 'vb2_dma_sg_dmabuf_ops.mmap') drivers/media/common/videobuf2/videobuf2-dma-sg.c:484:13: error: initializer element is not constant 484 | .release = vb2_dma_sg_dmabuf_ops_release, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:484:13: note: (near initialization for 'vb2_dma_sg_dmabuf_ops.release')
drivers/media/common/videobuf2/videobuf2-dma-sg.c:487:24: error: invalid storage class for function 'vb2_dma_sg_get_dmabuf'
487 | static struct dma_buf *vb2_dma_sg_get_dmabuf(void *buf_priv, unsigned long flags) | ^~~~~~~~~~~~~~~~~~~~~
drivers/media/common/videobuf2/videobuf2-dma-sg.c:515:12: error: invalid storage class for function 'vb2_dma_sg_map_dmabuf'
515 | static int vb2_dma_sg_map_dmabuf(void *mem_priv) | ^~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:543:13: error: invalid storage class for function 'vb2_dma_sg_unmap_dmabuf' 543 | static void vb2_dma_sg_unmap_dmabuf(void *mem_priv) | ^~~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:567:13: error: invalid storage class for function 'vb2_dma_sg_detach_dmabuf' 567 | static void vb2_dma_sg_detach_dmabuf(void *mem_priv) | ^~~~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:580:14: error: invalid storage class for function 'vb2_dma_sg_attach_dmabuf' 580 | static void *vb2_dma_sg_attach_dmabuf(struct device *dev, struct dma_buf *dbuf, | ^~~~~~~~~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:612:14: error: invalid storage class for function 'vb2_dma_sg_cookie' 612 | static void *vb2_dma_sg_cookie(void *buf_priv) | ^~~~~~~~~~~~~~~~~ In file included from include/linux/linkage.h:7, from include/linux/kernel.h:8, from include/linux/list.h:9, from include/linux/module.h:12, from drivers/media/common/videobuf2/videobuf2-dma-sg.c:13: drivers/media/common/videobuf2/videobuf2-dma-sg.c:636:19: error: extern declaration of 'vb2_dma_sg_memops' follows declaration with no linkage 636 | EXPORT_SYMBOL_GPL(vb2_dma_sg_memops); | ^~~~~~~~~~~~~~~~~ include/linux/export.h:98:21: note: in definition of macro '___EXPORT_SYMBOL' 98 | extern typeof(sym) sym; \ | ^~~ include/linux/export.h:155:34: note: in expansion of macro '__EXPORT_SYMBOL' 155 | #define _EXPORT_SYMBOL(sym, sec) __EXPORT_SYMBOL(sym, sec, "") | ^~~~~~~~~~~~~~~ include/linux/export.h:159:33: note: in expansion of macro '_EXPORT_SYMBOL' 159 | #define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "_gpl") | ^~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:636:1: note: in expansion of macro 'EXPORT_SYMBOL_GPL' 636 | EXPORT_SYMBOL_GPL(vb2_dma_sg_memops); | ^~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:619:26: note: previous definition of 'vb2_dma_sg_memops' was here 619 | const struct vb2_mem_ops vb2_dma_sg_memops = { | ^~~~~~~~~~~~~~~~~ In file included from include/linux/linkage.h:7, from include/linux/kernel.h:8, from include/linux/list.h:9, from include/linux/module.h:12, from drivers/media/common/videobuf2/videobuf2-dma-sg.c:13: include/linux/export.h:67:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] 67 | static const struct kernel_symbol __ksymtab_##sym \ | ^~~~~~ include/linux/export.h:108:2: note: in expansion of macro '__KSYMTAB_ENTRY' 108 | __KSYMTAB_ENTRY(sym, sec) | ^~~~~~~~~~~~~~~ include/linux/export.h:147:39: note: in expansion of macro '___EXPORT_SYMBOL' 147 | #define __EXPORT_SYMBOL(sym, sec, ns) ___EXPORT_SYMBOL(sym, sec, ns) | ^~~~~~~~~~~~~~~~ include/linux/export.h:155:34: note: in expansion of macro '__EXPORT_SYMBOL' 155 | #define _EXPORT_SYMBOL(sym, sec) __EXPORT_SYMBOL(sym, sec, "") | ^~~~~~~~~~~~~~~ include/linux/export.h:159:33: note: in expansion of macro '_EXPORT_SYMBOL' 159 | #define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "_gpl") | ^~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:636:1: note: in expansion of macro 'EXPORT_SYMBOL_GPL' 636 | EXPORT_SYMBOL_GPL(vb2_dma_sg_memops); | ^~~~~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:640:1: error: expected declaration or statement at end of input 640 | MODULE_LICENSE("GPL"); | ^~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c: In function 'vb2_dma_sg_alloc': drivers/media/common/videobuf2/videobuf2-dma-sg.c:640:1: error: expected declaration or statement at end of input drivers/media/common/videobuf2/videobuf2-dma-sg.c: At top level: drivers/media/common/videobuf2/videobuf2-dma-sg.c:56:13: warning: 'vb2_dma_sg_put' used but never defined 56 | static void vb2_dma_sg_put(void *buf_priv); | ^~~~~~~~~~~~~~ drivers/media/common/videobuf2/videobuf2-dma-sg.c:219:14: warning: 'vb2_dma_sg_get_userptr' defined but not used [-Wunused-function] 219 | static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr, | ^~~~~~~~~~~~~~~~~~~~~~
vim +/vb2_dma_sg_put +173 drivers/media/common/videobuf2/videobuf2-dma-sg.c
5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 55 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 @56 static void vb2_dma_sg_put(void *buf_priv); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 57 df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 58 static int vb2_dma_sg_alloc_compacted(struct vb2_dma_sg_buf *buf, df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 59 gfp_t gfp_flags) df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 60 { df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 61 unsigned int last_page = 0; 14f28f5cea9e399 drivers/media/common/videobuf2/videobuf2-dma-sg.c Sakari Ailus 2018-12-12 62 unsigned long size = buf->size; df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 63 df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 64 while (size > 0) { df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 65 struct page *pages; df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 66 int order; df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 67 int i; df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 68 df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 69 order = get_order(size); 4b129dc907e9b95 drivers/media/common/videobuf2/videobuf2-dma-sg.c Mauro Carvalho Chehab 2019-02-18 70 /* Don't over allocate*/ df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 71 if ((PAGE_SIZE << order) > size) df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 72 order--; df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 73 df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 74 pages = NULL; df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 75 while (!pages) { df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 76 pages = alloc_pages(GFP_KERNEL | __GFP_ZERO | df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 77 __GFP_NOWARN | gfp_flags, order); df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 78 if (pages) df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 79 break; df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 80 df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 81 if (order == 0) { df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 82 while (last_page--) df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 83 __free_page(buf->pages[last_page]); df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 84 return -ENOMEM; df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 85 } df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 86 order--; df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 87 } df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 88 df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 89 split_page(pages, order); 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 90 for (i = 0; i < (1 << order); i++) 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 91 buf->pages[last_page++] = &pages[i]; df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 92 df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 93 size -= PAGE_SIZE << order; df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 94 } df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 95 df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 96 return 0; df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 97 } df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 98 00085f1efa387a8 drivers/media/v4l2-core/videobuf2-dma-sg.c Krzysztof Kozlowski 2016-08-03 99 static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs, d16e832da23edff drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-04-15 100 unsigned long size, enum dma_data_direction dma_dir, d16e832da23edff drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-04-15 101 gfp_t gfp_flags) 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 102 { 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 103 struct vb2_dma_sg_buf *buf; d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 104 struct sg_table *sgt; df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 105 int ret; 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 106 int num_pages; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 107 0ff657b0f6120cb drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-07-21 108 if (WARN_ON(!dev)) 0ff657b0f6120cb drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-07-21 109 return ERR_PTR(-EINVAL); 0ff657b0f6120cb drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-07-21 110 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 111 buf = kzalloc(sizeof *buf, GFP_KERNEL); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 112 if (!buf) 0ff657b0f6120cb drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-07-21 113 return ERR_PTR(-ENOMEM); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 114 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 115 buf->vaddr = NULL; d935c57e8fb6902 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 116 buf->dma_dir = dma_dir; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 117 buf->offset = 0; 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 118 buf->size = size; 7f8414594e47552 drivers/media/v4l2-core/videobuf2-dma-sg.c Mauro Carvalho Chehab 2013-04-19 119 /* size is already page aligned */ 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 120 buf->num_pages = size >> PAGE_SHIFT; e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 121 buf->dma_sgt = &buf->sg_table; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 122 758d90e161382c1 drivers/media/v4l2-core/videobuf2-dma-sg.c Tomasz Figa 2017-06-19 123 buf->pages = kvmalloc_array(buf->num_pages, sizeof(struct page *), 758d90e161382c1 drivers/media/v4l2-core/videobuf2-dma-sg.c Tomasz Figa 2017-06-19 124 GFP_KERNEL | __GFP_ZERO); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 125 if (!buf->pages) 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 126 goto fail_pages_array_alloc; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 127 df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 128 ret = vb2_dma_sg_alloc_compacted(buf, gfp_flags); df23728118cd0f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 129 if (ret) 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 130 goto fail_pages_alloc; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 131 e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 132 ret = sg_alloc_table_from_pages(buf->dma_sgt, buf->pages, 47bc59c52b005f5 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-08-01 133 buf->num_pages, 0, size, GFP_KERNEL); 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 134 if (ret) 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 135 goto fail_table_alloc; 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 136 0c3a14c177aa85a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 137 /* Prevent the device from being released while the buffer is used */ 36c0f8b32c4bd4f drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-04-15 138 buf->dev = get_device(dev); d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 139 d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 140 sgt = &buf->sg_table; 251a79f8f5adfd8 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 141 /* 251a79f8f5adfd8 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 142 * No need to sync to the device, this will happen later when the 251a79f8f5adfd8 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 143 * prepare() memop is called. 251a79f8f5adfd8 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 144 */ b8bd9d592f68e3c drivers/media/common/videobuf2/videobuf2-dma-sg.c Marek Szyprowski 2020-06-18 145 if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir, b8bd9d592f68e3c drivers/media/common/videobuf2/videobuf2-dma-sg.c Marek Szyprowski 2020-06-18 146 DMA_ATTR_SKIP_CPU_SYNC)) { d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 147 goto fail_map; d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 148 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 149 buf->handler.refcount = &buf->refcount; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 150 buf->handler.put = vb2_dma_sg_put; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 151 buf->handler.arg = buf; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 152 6c4bb65d0be8f34 drivers/media/v4l2-core/videobuf2-dma-sg.c Elena Reshetova 2017-03-06 153 refcount_set(&buf->refcount, 1); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 154 ffdc78efe1a8a01 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2013-03-02 155 dprintk(1, "%s: Allocated buffer of %d pages\n", 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 156 __func__, buf->num_pages); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 157 return buf; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 158 d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 159 fail_map: d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 160 put_device(buf->dev); e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 161 sg_free_table(buf->dma_sgt); 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 162 fail_table_alloc: 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 163 num_pages = buf->num_pages; 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 164 while (num_pages--) 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 165 __free_page(buf->pages[num_pages]); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 166 fail_pages_alloc: 758d90e161382c1 drivers/media/v4l2-core/videobuf2-dma-sg.c Tomasz Figa 2017-06-19 167 kvfree(buf->pages); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 168 fail_pages_array_alloc: 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 169 kfree(buf); 0ff657b0f6120cb drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-07-21 170 return ERR_PTR(-ENOMEM); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 171 } 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 172 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 @173 static void vb2_dma_sg_put(void *buf_priv) 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 174 { 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 175 struct vb2_dma_sg_buf *buf = buf_priv; d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 176 struct sg_table *sgt = &buf->sg_table; 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 177 int i = buf->num_pages; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 178 6c4bb65d0be8f34 drivers/media/v4l2-core/videobuf2-dma-sg.c Elena Reshetova 2017-03-06 179 if (refcount_dec_and_test(&buf->refcount)) { ffdc78efe1a8a01 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2013-03-02 180 dprintk(1, "%s: Freeing buffer of %d pages\n", __func__, 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 181 buf->num_pages); b8bd9d592f68e3c drivers/media/common/videobuf2/videobuf2-dma-sg.c Marek Szyprowski 2020-06-18 182 dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir, b8bd9d592f68e3c drivers/media/common/videobuf2/videobuf2-dma-sg.c Marek Szyprowski 2020-06-18 183 DMA_ATTR_SKIP_CPU_SYNC); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 184 if (buf->vaddr) 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 185 vm_unmap_ram(buf->vaddr, buf->num_pages); e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 186 sg_free_table(buf->dma_sgt); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 187 while (--i >= 0) 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 188 __free_page(buf->pages[i]); 758d90e161382c1 drivers/media/v4l2-core/videobuf2-dma-sg.c Tomasz Figa 2017-06-19 189 kvfree(buf->pages); 0c3a14c177aa85a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 190 put_device(buf->dev); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 191 kfree(buf); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 192 } 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 193 } 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 194 d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 @195 static void vb2_dma_sg_prepare(void *buf_priv) d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 196 { d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 197 struct vb2_dma_sg_buf *buf = buf_priv; e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 198 struct sg_table *sgt = buf->dma_sgt; e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 199 e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 200 /* DMABUF exporter will flush the cache for us */ e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 201 if (buf->db_attach) e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 202 return; d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 203 b8bd9d592f68e3c drivers/media/common/videobuf2/videobuf2-dma-sg.c Marek Szyprowski 2020-06-18 204 dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir); d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 205 } d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 206 d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 @207 static void vb2_dma_sg_finish(void *buf_priv) d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 208 { d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 209 struct vb2_dma_sg_buf *buf = buf_priv; e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 210 struct sg_table *sgt = buf->dma_sgt; e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 211 e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 212 /* DMABUF exporter will flush the cache for us */ e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 213 if (buf->db_attach) e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 214 return; d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 215 b8bd9d592f68e3c drivers/media/common/videobuf2/videobuf2-dma-sg.c Marek Szyprowski 2020-06-18 216 dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir); d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 217 } d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 218 36c0f8b32c4bd4f drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-04-15 @219 static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr, cd474037c4a9a9c drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 220 unsigned long size, cd474037c4a9a9c drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 221 enum dma_data_direction dma_dir) 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 222 { 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 223 struct vb2_dma_sg_buf *buf; d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 224 struct sg_table *sgt; 3336c24f25ec932 drivers/media/v4l2-core/videobuf2-dma-sg.c Jan Kara 2015-07-13 225 struct frame_vector *vec; 251a79f8f5adfd8 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 226 10791829eb52d57 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-07-21 227 if (WARN_ON(!dev)) 10791829eb52d57 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-07-21 228 return ERR_PTR(-EINVAL); 10791829eb52d57 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-07-21 229 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 230 buf = kzalloc(sizeof *buf, GFP_KERNEL); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 231 if (!buf) 0ff657b0f6120cb drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-07-21 232 return ERR_PTR(-ENOMEM); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 233 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 234 buf->vaddr = NULL; 36c0f8b32c4bd4f drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-04-15 235 buf->dev = dev; cd474037c4a9a9c drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 236 buf->dma_dir = dma_dir; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 237 buf->offset = vaddr & ~PAGE_MASK; 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 238 buf->size = size; e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 239 buf->dma_sgt = &buf->sg_table; 707947247e9517b drivers/media/common/videobuf2/videobuf2-dma-sg.c Hans Verkuil 2019-04-04 240 vec = vb2_create_framevec(vaddr, size); 3336c24f25ec932 drivers/media/v4l2-core/videobuf2-dma-sg.c Jan Kara 2015-07-13 241 if (IS_ERR(vec)) 3336c24f25ec932 drivers/media/v4l2-core/videobuf2-dma-sg.c Jan Kara 2015-07-13 242 goto userptr_fail_pfnvec; 3336c24f25ec932 drivers/media/v4l2-core/videobuf2-dma-sg.c Jan Kara 2015-07-13 243 buf->vec = vec; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 244 3336c24f25ec932 drivers/media/v4l2-core/videobuf2-dma-sg.c Jan Kara 2015-07-13 245 buf->pages = frame_vector_pages(vec); 3336c24f25ec932 drivers/media/v4l2-core/videobuf2-dma-sg.c Jan Kara 2015-07-13 246 if (IS_ERR(buf->pages)) 3336c24f25ec932 drivers/media/v4l2-core/videobuf2-dma-sg.c Jan Kara 2015-07-13 247 goto userptr_fail_sgtable; 3336c24f25ec932 drivers/media/v4l2-core/videobuf2-dma-sg.c Jan Kara 2015-07-13 248 buf->num_pages = frame_vector_count(vec); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 249 e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 250 if (sg_alloc_table_from_pages(buf->dma_sgt, buf->pages, 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 251 buf->num_pages, buf->offset, size, 0)) 3336c24f25ec932 drivers/media/v4l2-core/videobuf2-dma-sg.c Jan Kara 2015-07-13 252 goto userptr_fail_sgtable; 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 253 d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 254 sgt = &buf->sg_table; 251a79f8f5adfd8 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 255 /* 251a79f8f5adfd8 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 256 * No need to sync to the device, this will happen later when the 251a79f8f5adfd8 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 257 * prepare() memop is called. 251a79f8f5adfd8 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 258 */ b8bd9d592f68e3c drivers/media/common/videobuf2/videobuf2-dma-sg.c Marek Szyprowski 2020-06-18 259 if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir, b8bd9d592f68e3c drivers/media/common/videobuf2/videobuf2-dma-sg.c Marek Szyprowski 2020-06-18 260 DMA_ATTR_SKIP_CPU_SYNC)) { d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 261 goto userptr_fail_map; 6a5d77cbf26078e drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2015-04-29 262 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 263 return buf; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 264 d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 265 userptr_fail_map: d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 266 sg_free_table(&buf->sg_table); 3336c24f25ec932 drivers/media/v4l2-core/videobuf2-dma-sg.c Jan Kara 2015-07-13 267 userptr_fail_sgtable: 3336c24f25ec932 drivers/media/v4l2-core/videobuf2-dma-sg.c Jan Kara 2015-07-13 268 vb2_destroy_framevec(vec); 3336c24f25ec932 drivers/media/v4l2-core/videobuf2-dma-sg.c Jan Kara 2015-07-13 269 userptr_fail_pfnvec: 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 270 kfree(buf); 0ff657b0f6120cb drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2016-07-21 271 return ERR_PTR(-ENOMEM); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 272 } 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 273 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 274 /* 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 275 * @put_userptr: inform the allocator that a USERPTR buffer will no longer 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 276 * be used 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 277 */ 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 @278 static void vb2_dma_sg_put_userptr(void *buf_priv) 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 279 { 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 280 struct vb2_dma_sg_buf *buf = buf_priv; d790b7eda953df4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-24 281 struct sg_table *sgt = &buf->sg_table; 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 282 int i = buf->num_pages; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 283 ffdc78efe1a8a01 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2013-03-02 284 dprintk(1, "%s: Releasing userspace buffer of %d pages\n", 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 285 __func__, buf->num_pages); b8bd9d592f68e3c drivers/media/common/videobuf2/videobuf2-dma-sg.c Marek Szyprowski 2020-06-18 286 dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 287 if (buf->vaddr) 223012475968fb8 drivers/media/v4l2-core/videobuf2-dma-sg.c Ricardo Ribalda 2013-08-02 288 vm_unmap_ram(buf->vaddr, buf->num_pages); e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 289 sg_free_table(buf->dma_sgt); 5b6f9abe5a49df8 drivers/media/v4l2-core/videobuf2-dma-sg.c Stanimir Varbanov 2017-08-21 290 if (buf->dma_dir == DMA_FROM_DEVICE || 5b6f9abe5a49df8 drivers/media/v4l2-core/videobuf2-dma-sg.c Stanimir Varbanov 2017-08-21 291 buf->dma_dir == DMA_BIDIRECTIONAL) c0cb76589c77b9a drivers/media/v4l2-core/videobuf2-dma-sg.c Stanimir Varbanov 2017-08-29 292 while (--i >= 0) 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 293 set_page_dirty_lock(buf->pages[i]); 3336c24f25ec932 drivers/media/v4l2-core/videobuf2-dma-sg.c Jan Kara 2015-07-13 294 vb2_destroy_framevec(buf->vec); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 295 kfree(buf); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 296 } 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 297 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 @298 static void *vb2_dma_sg_vaddr(void *buf_priv) 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 299 { 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 300 struct vb2_dma_sg_buf *buf = buf_priv; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 301 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 302 BUG_ON(!buf); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 303 e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 304 if (!buf->vaddr) { e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 305 if (buf->db_attach) e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 306 buf->vaddr = dma_buf_vmap(buf->db_attach->dmabuf); e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 307 else d4efd79a81abc70 drivers/media/common/videobuf2/videobuf2-dma-sg.c Christoph Hellwig 2020-06-01 308 buf->vaddr = vm_map_ram(buf->pages, buf->num_pages, -1); e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 309 } 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 310 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 311 /* add offset in case userptr is not page-aligned */ e078b79d8aa70a4 drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 312 return buf->vaddr ? buf->vaddr + buf->offset : NULL; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 313 } 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 314 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 @315 static unsigned int vb2_dma_sg_num_users(void *buf_priv) 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 316 { 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 317 struct vb2_dma_sg_buf *buf = buf_priv; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 318 6c4bb65d0be8f34 drivers/media/v4l2-core/videobuf2-dma-sg.c Elena Reshetova 2017-03-06 319 return refcount_read(&buf->refcount); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 320 } 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 321 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 @322 static int vb2_dma_sg_mmap(void *buf_priv, struct vm_area_struct *vma) 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 323 { 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 324 struct vb2_dma_sg_buf *buf = buf_priv; a17ae14766935aa drivers/media/common/videobuf2/videobuf2-dma-sg.c Souptick Joarder 2019-05-13 325 int err; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 326 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 327 if (!buf) { 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 328 printk(KERN_ERR "No memory to map\n"); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 329 return -EINVAL; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 330 } 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 331 a17ae14766935aa drivers/media/common/videobuf2/videobuf2-dma-sg.c Souptick Joarder 2019-05-13 332 err = vm_map_pages(vma, buf->pages, buf->num_pages); a17ae14766935aa drivers/media/common/videobuf2/videobuf2-dma-sg.c Souptick Joarder 2019-05-13 333 if (err) { a17ae14766935aa drivers/media/common/videobuf2/videobuf2-dma-sg.c Souptick Joarder 2019-05-13 334 printk(KERN_ERR "Remapping memory, error: %d\n", err); a17ae14766935aa drivers/media/common/videobuf2/videobuf2-dma-sg.c Souptick Joarder 2019-05-13 335 return err; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 336 } 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 337 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 338 /* 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 339 * Use common vm_area operations to track buffer refcount. 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 340 */ 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 341 vma->vm_private_data = &buf->handler; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 342 vma->vm_ops = &vb2_common_vm_ops; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 343 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 344 vma->vm_ops->open(vma); 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 345 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 346 return 0; 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 347 } 5ba3f757f0592ca drivers/media/video/videobuf2-dma-sg.c Andrzej Pietrasiewicz 2010-11-29 348 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 349 /*********************************************/ 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 350 /* DMABUF ops for exporters */ 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 351 /*********************************************/ 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 352 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 353 struct vb2_dma_sg_attachment { 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 354 struct sg_table sgt; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 355 enum dma_data_direction dma_dir; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 356 }; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 357 a19741e5e5a9f1f drivers/media/common/videobuf2/videobuf2-dma-sg.c Christian König 2018-05-28 @358 static int vb2_dma_sg_dmabuf_ops_attach(struct dma_buf *dbuf, 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 359 struct dma_buf_attachment *dbuf_attach) 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 360 { 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 361 struct vb2_dma_sg_attachment *attach; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 362 unsigned int i; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 363 struct scatterlist *rd, *wr; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 364 struct sg_table *sgt; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 365 struct vb2_dma_sg_buf *buf = dbuf->priv; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 366 int ret; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 367 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 368 attach = kzalloc(sizeof(*attach), GFP_KERNEL); 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 369 if (!attach) 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 370 return -ENOMEM; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 371 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 372 sgt = &attach->sgt; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 373 /* Copy the buf->base_sgt scatter list to the attachment, as we can't 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 374 * map the same scatter list to multiple attachments at the same time. 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 375 */ 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 376 ret = sg_alloc_table(sgt, buf->dma_sgt->orig_nents, GFP_KERNEL); 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 377 if (ret) { 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 378 kfree(attach); 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 379 return -ENOMEM; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 380 } 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 381 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 382 rd = buf->dma_sgt->sgl; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 383 wr = sgt->sgl; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 384 for (i = 0; i < sgt->orig_nents; ++i) { 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 385 sg_set_page(wr, sg_page(rd), rd->length, rd->offset); 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 386 rd = sg_next(rd); 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 387 wr = sg_next(wr); 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 388 } 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 389 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 390 attach->dma_dir = DMA_NONE; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 391 dbuf_attach->priv = attach; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 392 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 393 return 0; 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 394 } 041c7b6ac74ee7a drivers/media/v4l2-core/videobuf2-dma-sg.c Hans Verkuil 2014-11-18 395
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On 2020-06-18 16:39, Marek Szyprowski wrote:
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@samsung.com
.../common/videobuf2/videobuf2-dma-contig.c | 41 ++++++++----------- .../media/common/videobuf2/videobuf2-dma-sg.c | 32 ++++++--------- .../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 f4b4a7c135eb..ba01a8692d88 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;
Same comment as for the DRM version. In fact, given that it's the same function with the same purpose, might it be worth hoisting out as a generic helper for the sg_table API itself?
} return size; }
[...]
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c index 92072a08af25..6ddf953efa11 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c @@ -142,9 +142,8 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs, * 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,
buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);- if (!sgt->nents)
- if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir,
DMA_ATTR_SKIP_CPU_SYNC)) {
As 0-day's explosions of nonsense imply, there's a rogue bracket here...
goto fail_map;buf->handler.refcount = &buf->refcount; @@ -180,8 +179,8 @@ static void vb2_dma_sg_put(void *buf_priv) if (refcount_dec_and_test(&buf->refcount)) { dprintk(1, "%s: Freeing buffer of %d pages\n", __func__, buf->num_pages);
dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir,DMA_ATTR_SKIP_CPU_SYNC);@@ -202,8 +201,7 @@ static void vb2_dma_sg_prepare(void *buf_priv) if (buf->db_attach) return;
- dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
buf->dma_dir);
dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir); }
static void vb2_dma_sg_finish(void *buf_priv)
@@ -215,7 +213,7 @@ static void vb2_dma_sg_finish(void *buf_priv) if (buf->db_attach) return;
- dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir); }
static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr,
@@ -258,9 +256,8 @@ static void *vb2_dma_sg_get_userptr(struct device *dev, 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,
buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);- if (!sgt->nents)
- if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir,
DMA_ATTR_SKIP_CPU_SYNC)) {
... and here.
Robin.
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg().
struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry).
It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function.
Fix the code to refer to proper nents or orig_nents entries. This driver reports the number of the pages in the imported scatterlist, so it should refer to sg_table->orig_nents entry.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Acked-by: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com --- drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c index f0b85e094111..ba4bdc5590ea 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c @@ -215,7 +215,7 @@ xen_drm_front_gem_import_sg_table(struct drm_device *dev, return ERR_PTR(ret);
DRM_DEBUG("Imported buffer of size %zu with nents %u\n", - size, sgt->nents); + size, sgt->orig_nents);
return &xen_obj->base; }
dri-devel@lists.freedesktop.org
-
kernel test robot -
Marek Szyprowski -
Robin Murphy