* [PATCH v7 07/36] drm: exynos: use common helper for a scatterlist contiguity check [not found] ` <CGME20200619103657eucas1p24bff92408adbd4715130fb47595a6187@eucas1p2.samsung.com> @ 2020-06-19 10:36 ` Marek Szyprowski 2020-07-07 9:35 ` Andrzej Hajda 2020-07-14 0:28 ` Inki Dae 0 siblings, 2 replies; 8+ messages in thread From: Marek Szyprowski @ 2020-06-19 10:36 UTC (permalink / raw) To: dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: Marek Szyprowski, Christoph Hellwig, Robin Murphy, Bartlomiej Zolnierkiewicz, linux-arm-kernel, David Airlie, Daniel Vetter, Inki Dae, Seung-Woo Kim, linux-samsung-soc 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); -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v7 07/36] drm: exynos: use common helper for a scatterlist contiguity check 2020-06-19 10:36 ` [PATCH v7 07/36] drm: exynos: use common helper for a scatterlist contiguity check Marek Szyprowski @ 2020-07-07 9:35 ` Andrzej Hajda 2020-07-07 15:04 ` Andrzej Hajda 2020-07-14 0:28 ` Inki Dae 1 sibling, 1 reply; 8+ messages in thread From: Andrzej Hajda @ 2020-07-07 9:35 UTC (permalink / raw) To: Marek Szyprowski, dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, David Airlie, Seung-Woo Kim, Robin Murphy, Christoph Hellwig, linux-arm-kernel Hi, On 19.06.2020 12:36, Marek Szyprowski wrote: > 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); > - } > } Reviewed-by <a.hajda@samsung.com> Regards Andrzej > > exynos_gem = exynos_drm_gem_init(dev, attach->dmabuf->size); ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v7 07/36] drm: exynos: use common helper for a scatterlist contiguity check 2020-07-07 9:35 ` Andrzej Hajda @ 2020-07-07 15:04 ` Andrzej Hajda 0 siblings, 0 replies; 8+ messages in thread From: Andrzej Hajda @ 2020-07-07 15:04 UTC (permalink / raw) To: Marek Szyprowski, dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, David Airlie, Seung-Woo Kim, Robin Murphy, Christoph Hellwig, linux-arm-kernel On 07.07.2020 11:35, Andrzej Hajda wrote: > Hi, > > On 19.06.2020 12:36, Marek Szyprowski wrote: >> Use common helper for checking the contiguity of the imported dma-buf. >> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Just fixing my signature :) Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> Regards Andrzej ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v7 07/36] drm: exynos: use common helper for a scatterlist contiguity check 2020-06-19 10:36 ` [PATCH v7 07/36] drm: exynos: use common helper for a scatterlist contiguity check Marek Szyprowski 2020-07-07 9:35 ` Andrzej Hajda @ 2020-07-14 0:28 ` Inki Dae 1 sibling, 0 replies; 8+ messages in thread From: Inki Dae @ 2020-07-14 0:28 UTC (permalink / raw) To: Marek Szyprowski, dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: Christoph Hellwig, Robin Murphy, Bartlomiej Zolnierkiewicz, linux-arm-kernel, David Airlie, Daniel Vetter, Seung-Woo Kim, linux-samsung-soc 20. 6. 19. 오후 7:36에 Marek Szyprowski 이(가) 쓴 글: > Use common helper for checking the contiguity of the imported dma-buf. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Acked-by : Inki Dae <inki.dae@samsung.com> Thanks, Inki Dae > --- > 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); > ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CGME20200619103658eucas1p1c3236e2de2798c2d8c02279a9263e9a9@eucas1p1.samsung.com>]
* [PATCH v7 08/36] drm: exynos: fix common struct sg_table related issues [not found] ` <CGME20200619103658eucas1p1c3236e2de2798c2d8c02279a9263e9a9@eucas1p1.samsung.com> @ 2020-06-19 10:36 ` Marek Szyprowski 2020-07-07 9:40 ` Andrzej Hajda 2020-07-14 0:27 ` Inki Dae 0 siblings, 2 replies; 8+ messages in thread From: Marek Szyprowski @ 2020-06-19 10:36 UTC (permalink / raw) To: dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: Marek Szyprowski, Christoph Hellwig, Robin Murphy, Bartlomiej Zolnierkiewicz, linux-arm-kernel, David Airlie, Daniel Vetter, Inki Dae, Seung-Woo Kim, linux-samsung-soc 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; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v7 08/36] drm: exynos: fix common struct sg_table related issues 2020-06-19 10:36 ` [PATCH v7 08/36] drm: exynos: fix common struct sg_table related issues Marek Szyprowski @ 2020-07-07 9:40 ` Andrzej Hajda 2020-07-07 15:05 ` Andrzej Hajda 2020-07-14 0:27 ` Inki Dae 1 sibling, 1 reply; 8+ messages in thread From: Andrzej Hajda @ 2020-07-07 9:40 UTC (permalink / raw) To: Marek Szyprowski, dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, David Airlie, Seung-Woo Kim, Robin Murphy, Christoph Hellwig, linux-arm-kernel On 19.06.2020 12:36, Marek Szyprowski wrote: > 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 <a.hajda@samsung.com> Regards Andrzej > --- > 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; > } > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v7 08/36] drm: exynos: fix common struct sg_table related issues 2020-07-07 9:40 ` Andrzej Hajda @ 2020-07-07 15:05 ` Andrzej Hajda 0 siblings, 0 replies; 8+ messages in thread From: Andrzej Hajda @ 2020-07-07 15:05 UTC (permalink / raw) To: Marek Szyprowski, dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: linux-samsung-soc, Bartlomiej Zolnierkiewicz, David Airlie, Seung-Woo Kim, Robin Murphy, Christoph Hellwig, linux-arm-kernel On 07.07.2020 11:40, Andrzej Hajda wrote: > On 19.06.2020 12:36, Marek Szyprowski wrote: >> 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> > Just fixing my signature :) Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> Regards Andrzej ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v7 08/36] drm: exynos: fix common struct sg_table related issues 2020-06-19 10:36 ` [PATCH v7 08/36] drm: exynos: fix common struct sg_table related issues Marek Szyprowski 2020-07-07 9:40 ` Andrzej Hajda @ 2020-07-14 0:27 ` Inki Dae 1 sibling, 0 replies; 8+ messages in thread From: Inki Dae @ 2020-07-14 0:27 UTC (permalink / raw) To: Marek Szyprowski, dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: Christoph Hellwig, Robin Murphy, Bartlomiej Zolnierkiewicz, linux-arm-kernel, David Airlie, Daniel Vetter, Seung-Woo Kim, linux-samsung-soc 20. 6. 19. 오후 7:36에 Marek Szyprowski 이(가) 쓴 글: > 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 : Inki Dae <inki.dae@samsung.com> Thanks, Inki Dae > --- > 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; > } > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-07-14 0:22 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200619103636.11974-1-m.szyprowski@samsung.com> [not found] ` <CGME20200619103657eucas1p24bff92408adbd4715130fb47595a6187@eucas1p2.samsung.com> 2020-06-19 10:36 ` [PATCH v7 07/36] drm: exynos: use common helper for a scatterlist contiguity check Marek Szyprowski 2020-07-07 9:35 ` Andrzej Hajda 2020-07-07 15:04 ` Andrzej Hajda 2020-07-14 0:28 ` Inki Dae [not found] ` <CGME20200619103658eucas1p1c3236e2de2798c2d8c02279a9263e9a9@eucas1p1.samsung.com> 2020-06-19 10:36 ` [PATCH v7 08/36] drm: exynos: fix common struct sg_table related issues Marek Szyprowski 2020-07-07 9:40 ` Andrzej Hajda 2020-07-07 15:05 ` Andrzej Hajda 2020-07-14 0:27 ` Inki Dae
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).