* [PATCH v3 1/3] drm/prime: Iterate SG DMA addresses separately
@ 2018-04-30 13:54 Robin Murphy
2018-04-30 13:54 ` [PATCH v3 2/3] drm/amdgpu: Allow dma_map_sg() coalescing Robin Murphy
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Robin Murphy @ 2018-04-30 13:54 UTC (permalink / raw)
To: amd-gfx, dri-devel; +Cc: okaya, alexander.deucher, christian.koenig
For dma_map_sg(), DMA API implementations are free to merge consecutive
segments into a single DMA mapping if conditions are suitable, thus the
resulting DMA addresses which drm_prime_sg_to_page_addr_arrays()
iterates over may be packed into fewer entries than sgt->nents implies.
The current implementation does not account for this, meaning that its
callers either have to reject the 0 < count < nents case or risk getting
bogus DMA addresses beyond the first segment. Fortunately this is quite
easy to handle without having to rejig structures to also store the
mapped count, since the total DMA length should still be equal to the
total buffer length. All we need is a second scatterlist cursor to
iterate through the DMA addresses independently of the page addresses.
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Move dma_len == 0 logic earlier to avoid iterating dma_sg too far
drivers/gpu/drm/drm_prime.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 7856a9b3f8a8..3e74c84d0baf 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -933,16 +933,24 @@ 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 scatterlist *sg, *dma_sg;
struct page *page;
- u32 len, index;
+ u32 len, dma_len, index;
dma_addr_t addr;
index = 0;
+ dma_sg = sgt->sgl;
+ dma_len = sg_dma_len(dma_sg);
+ addr = sg_dma_address(dma_sg);
for_each_sg(sgt->sgl, sg, sgt->nents, count) {
len = sg->length;
page = sg_page(sg);
- addr = sg_dma_address(sg);
+
+ if (addrs && dma_len == 0) {
+ dma_sg = sg_next(dma_sg);
+ dma_len = sg_dma_len(dma_sg);
+ addr = sg_dma_address(dma_sg);
+ }
while (len > 0) {
if (WARN_ON(index >= max_entries))
@@ -955,6 +963,7 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages,
page++;
addr += PAGE_SIZE;
len -= PAGE_SIZE;
+ dma_len -= PAGE_SIZE;
index++;
}
}
--
2.17.0.dirty
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3 2/3] drm/amdgpu: Allow dma_map_sg() coalescing
2018-04-30 13:54 [PATCH v3 1/3] drm/prime: Iterate SG DMA addresses separately Robin Murphy
@ 2018-04-30 13:54 ` Robin Murphy
2018-04-30 13:54 ` [PATCH v3 3/3] drm/radeon: " Robin Murphy
2018-04-30 17:59 ` [PATCH v3 1/3] drm/prime: Iterate SG DMA addresses separately Sinan Kaya
2 siblings, 0 replies; 5+ messages in thread
From: Robin Murphy @ 2018-04-30 13:54 UTC (permalink / raw)
To: amd-gfx, dri-devel; +Cc: okaya, alexander.deucher, christian.koenig
The amdgpu driver doesn't appear to directly use the scatterlist mapped
by amdgpu_ttm_tt_pin_userptr(), it merely hands it off to
drm_prime_sg_to_page_addr_arrays() to generate the dma_address array
which it actually cares about. Now that the latter can cope with
dma_map_sg() coalescing dma-contiguous segments such that it returns
0 < count < nents, we can relax the current count == nents check to
only consider genuine failure as other drivers do.
Reported-by: Sinan Kaya <okaya@codeaurora.org>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: No change
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 205da3ff9cd0..f81e96a4242f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -813,7 +813,7 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm)
r = -ENOMEM;
nents = dma_map_sg(adev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
- if (nents != ttm->sg->nents)
+ if (nents == 0)
goto release_sg;
drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
--
2.17.0.dirty
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3 3/3] drm/radeon: Allow dma_map_sg() coalescing
2018-04-30 13:54 [PATCH v3 1/3] drm/prime: Iterate SG DMA addresses separately Robin Murphy
2018-04-30 13:54 ` [PATCH v3 2/3] drm/amdgpu: Allow dma_map_sg() coalescing Robin Murphy
@ 2018-04-30 13:54 ` Robin Murphy
2018-04-30 17:59 ` [PATCH v3 1/3] drm/prime: Iterate SG DMA addresses separately Sinan Kaya
2 siblings, 0 replies; 5+ messages in thread
From: Robin Murphy @ 2018-04-30 13:54 UTC (permalink / raw)
To: amd-gfx, dri-devel; +Cc: okaya, alexander.deucher, christian.koenig
Much like amdgpu, the radeon driver doesn't appear to directly use the
scatterlist mapped by radeon_ttm_tt_pin_userptr(), it merely hands it
off to drm_prime_sg_to_page_addr_arrays() to generate the dma_address
array which it actually cares about. Now that the latter can cope with
dma_map_sg() coalescing dma-contiguous segments such that it returns
0 < count < nents, we can relax the current count == nents check to
only consider genuine failure as other drivers do.
Suggested-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: No change
drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 8689fcca051c..7c099192c7fa 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -585,7 +585,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm)
r = -ENOMEM;
nents = dma_map_sg(rdev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
- if (nents != ttm->sg->nents)
+ if (nents == 0)
goto release_sg;
drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
--
2.17.0.dirty
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/3] drm/prime: Iterate SG DMA addresses separately
2018-04-30 13:54 [PATCH v3 1/3] drm/prime: Iterate SG DMA addresses separately Robin Murphy
2018-04-30 13:54 ` [PATCH v3 2/3] drm/amdgpu: Allow dma_map_sg() coalescing Robin Murphy
2018-04-30 13:54 ` [PATCH v3 3/3] drm/radeon: " Robin Murphy
@ 2018-04-30 17:59 ` Sinan Kaya
2018-05-25 13:33 ` Robin Murphy
2 siblings, 1 reply; 5+ messages in thread
From: Sinan Kaya @ 2018-04-30 17:59 UTC (permalink / raw)
To: Robin Murphy, amd-gfx, dri-devel; +Cc: alexander.deucher, christian.koenig
On 4/30/2018 9:54 AM, Robin Murphy wrote:
> For dma_map_sg(), DMA API implementations are free to merge consecutive
> segments into a single DMA mapping if conditions are suitable, thus the
> resulting DMA addresses which drm_prime_sg_to_page_addr_arrays()
> iterates over may be packed into fewer entries than sgt->nents implies.
>
> The current implementation does not account for this, meaning that its
> callers either have to reject the 0 < count < nents case or risk getting
> bogus DMA addresses beyond the first segment. Fortunately this is quite
> easy to handle without having to rejig structures to also store the
> mapped count, since the total DMA length should still be equal to the
> total buffer length. All we need is a second scatterlist cursor to
> iterate through the DMA addresses independently of the page addresses.
>
> Reviewed-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
Much better
Tested-by: Sinan Kaya <okaya@codeauora.org>
for the first two patches. (1/3 and 2/3)
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/3] drm/prime: Iterate SG DMA addresses separately
2018-04-30 17:59 ` [PATCH v3 1/3] drm/prime: Iterate SG DMA addresses separately Sinan Kaya
@ 2018-05-25 13:33 ` Robin Murphy
0 siblings, 0 replies; 5+ messages in thread
From: Robin Murphy @ 2018-05-25 13:33 UTC (permalink / raw)
To: Sinan Kaya, amd-gfx, dri-devel; +Cc: alexander.deucher, christian.koenig
On 30/04/18 18:59, Sinan Kaya wrote:
> On 4/30/2018 9:54 AM, Robin Murphy wrote:
>> For dma_map_sg(), DMA API implementations are free to merge consecutive
>> segments into a single DMA mapping if conditions are suitable, thus the
>> resulting DMA addresses which drm_prime_sg_to_page_addr_arrays()
>> iterates over may be packed into fewer entries than sgt->nents implies.
>>
>> The current implementation does not account for this, meaning that its
>> callers either have to reject the 0 < count < nents case or risk getting
>> bogus DMA addresses beyond the first segment. Fortunately this is quite
>> easy to handle without having to rejig structures to also store the
>> mapped count, since the total DMA length should still be equal to the
>> total buffer length. All we need is a second scatterlist cursor to
>> iterate through the DMA addresses independently of the page addresses.
>>
>> Reviewed-by: Christian König <christian.koenig@amd.com>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>
> Much better
>
> Tested-by: Sinan Kaya <okaya@codeauora.org>
>
> for the first two patches. (1/3 and 2/3)
Cheers Sinan.
Alex, Christian, David; is the AMD GPU tree the right target for these
patches, or is there a wider audience I should consider resending them
to? (before I forget about them again...)
Thanks,
Robin.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-25 13:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-30 13:54 [PATCH v3 1/3] drm/prime: Iterate SG DMA addresses separately Robin Murphy
2018-04-30 13:54 ` [PATCH v3 2/3] drm/amdgpu: Allow dma_map_sg() coalescing Robin Murphy
2018-04-30 13:54 ` [PATCH v3 3/3] drm/radeon: " Robin Murphy
2018-04-30 17:59 ` [PATCH v3 1/3] drm/prime: Iterate SG DMA addresses separately Sinan Kaya
2018-05-25 13:33 ` Robin Murphy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox