Unnamed repository; edit this file 'description' to name the repository.
 help / color / mirror / Atom feed
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Dmitry Osipenko <digetx@gmail.com>
Cc: dri-devel@lists.freedesktop.org,
	iommu@lists.linux-foundation.org, linaro-mm-sig@lists.linaro.org,
	linux-kernel@vger.kernel.org, Christoph Hellwig <hch@lst.de>,
	Robin Murphy <robin.murphy@arm.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	linux-media@vger.kernel.org, linux-tegra@vger.kernel.org,
Subject: Re: [PATCH v7 31/36] staging: tegra-vde: fix common struct sg_table related issues
Date: Tue, 30 Jun 2020 12:07:26 +0200
Message-ID: <559970b6-e80f-90ec-7fb0-1fab742d99de@samsung.com> (raw)
In-Reply-To: <20200621070015.0cf833ab@dimatab>

On 21.06.2020 06:00, Dmitry Osipenko wrote:
> В Fri, 19 Jun 2020 12:36:31 +0200
> Marek Szyprowski <m.szyprowski@samsung.com> пишет:
>> 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,
>> +	size = iommu_map_sgtable(vde->domain, addr, sgt,
>>   	if (!size) {
>>   		__free_iova(&vde->iova, iova);
>>   		return -ENXIO;
> Ahh, I saw the build failure report. You're changing the DMA API in
> this series, while DMA API isn't used by this driver, it uses IOMMU
> API. Hence there is no need to touch this code. Similar problem in the
> host1x driver patch.

The issue is caused by the lack of iommu_map_sgtable() stub when no 
IOMMU support is configured. I've posted a patch for this:


The patch for this driver is fine, we have to wait until the above fix 
gets merged and then it can be applied during the next release cycle.

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

  reply	other threads:[~2020-06-30 10:07 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200619103636.11974-1-m.szyprowski@samsung.com>
     [not found] ` <CGME20200619103711eucas1p188e07cb9aaad13d39238aac4fe84b10c@eucas1p1.samsung.com>
2020-06-19 10:36   ` [PATCH v7 27/36] drm: rcar-du: " Marek Szyprowski
     [not found] ` <CGME20200619103714eucas1p18db6efd1a380fc0bdb16174ee85036fa@eucas1p1.samsung.com>
2020-06-19 10:36   ` [PATCH v7 31/36] staging: tegra-vde: " Marek Szyprowski
2020-06-20 21:10     ` kernel test robot
2020-06-21  4:00     ` Dmitry Osipenko
2020-06-30 10:07       ` Marek Szyprowski [this message]
2020-07-01  1:45         ` Dmitry Osipenko
     [not found] ` <CGME20200619103717eucas1p23b82366794c92cc70c0492e4ca29c4a1@eucas1p2.samsung.com>
2020-06-19 10:36   ` [PATCH v7 35/36] media: pci: fix common ALSA DMA-mapping related codes Marek Szyprowski
     [not found] ` <CGME20200619103718eucas1p11cd577b435672197f48bfcba2d06bc18@eucas1p1.samsung.com>
2020-06-19 10:36   ` [PATCH v7 36/36] videobuf2: use sgtable-based scatterlist wrappers Marek Szyprowski
     [not found]     ` <CGME20200630104527eucas1p13f19c24ff053556fb1fa7dc72be14c77@eucas1p1.samsung.com>
2020-06-30 10:45       ` [PATCH v8] " Marek Szyprowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=559970b6-e80f-90ec-7fb0-1fab742d99de@samsung.com \
    --to=m.szyprowski@samsung.com \
    --cc=airlied@linux.ie \
    --cc=b.zolnierkie@samsung.com \
    --cc=daniel@ffwll.ch \
    --cc=devel@driverdev.osuosl.org \
    --cc=digetx@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jonathanh@nvidia.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=thierry.reding@gmail.com \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Unnamed repository; edit this file 'description' to name the repository.

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://archive.lwn.net:8080/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ http://archive.lwn.net:8080/linux-media \
		linux-media@vger.kernel.org lwn-linux-media@archive.lwn.net
	public-inbox-index linux-media

Example config snippet for mirrors.
Newsgroup available over NNTP:

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git