Re: [PATCH 2/2] drm/amdgpu: Allow dma_map_sg() coalescing

2018-04-12 Thread Christian König 

Am 12.04.2018 um 19:53 schrieb Robin Murphy:

On 11/04/18 19:28, Christian König wrote:

Am 11.04.2018 um 19:11 schrieb Robin Murphy:

Now that drm_prime_sg_to_page_addr_arrays() understands the case where
dma_map_sg() has coalesced segments and returns 0 < count < nents, we
can relax the check to only consider genuine failure.


That pattern is repeated in pretty much all drivers using TTM.


AFAICS from a look through drivers/gpu/ only 3 drivers consider the 
actual value of what dma_map_sg() returns - others only handle the 
failure case of 0, and some don't check it at all - and of those it's 
only amdgpu and radeon barfing on it being different from nents 
(vmwgfx appears to just stash it to use slightly incorrectly later).


So you would need to fix all of them, but as I said I don't think 
that this approach is a good idea.


We essentially wanted to get rid of the dma_address array in the mid 
term and that change goes into the exactly opposite direction.


But this patch bears no intention of making any fundamental change to 
the existing behaviour of this particular driver, it simply permits 
said behaviour to work at all on more systems than it currently does. 
I would consider that entirely orthogonal to future TTM-wide 
development :/


That's a really good point. Please try to fix radeon as well and maybe 
ping the vmwgfx maintainers.


With that in place I'm perfectly ok with going ahead with that approach.

Christian.



Robin.



Regards,
Christian.



Signed-off-by: Robin Murphy 
---
  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,




___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/amdgpu: Allow dma_map_sg() coalescing

2018-04-12 Thread Robin Murphy 

On 11/04/18 19:28, Christian König wrote:

Am 11.04.2018 um 19:11 schrieb Robin Murphy:

Now that drm_prime_sg_to_page_addr_arrays() understands the case where
dma_map_sg() has coalesced segments and returns 0 < count < nents, we
can relax the check to only consider genuine failure.


That pattern is repeated in pretty much all drivers using TTM.


AFAICS from a look through drivers/gpu/ only 3 drivers consider the 
actual value of what dma_map_sg() returns - others only handle the 
failure case of 0, and some don't check it at all - and of those it's 
only amdgpu and radeon barfing on it being different from nents (vmwgfx 
appears to just stash it to use slightly incorrectly later).


So you would need to fix all of them, but as I said I don't think that 
this approach is a good idea.


We essentially wanted to get rid of the dma_address array in the mid 
term and that change goes into the exactly opposite direction.


But this patch bears no intention of making any fundamental change to 
the existing behaviour of this particular driver, it simply permits said 
behaviour to work at all on more systems than it currently does. I would 
consider that entirely orthogonal to future TTM-wide development :/


Robin.



Regards,
Christian.



Signed-off-by: Robin Murphy 
---
  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,



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/amdgpu: Allow dma_map_sg() coalescing

2018-04-11 Thread Christian König 

Am 11.04.2018 um 19:11 schrieb Robin Murphy:

Now that drm_prime_sg_to_page_addr_arrays() understands the case where
dma_map_sg() has coalesced segments and returns 0 < count < nents, we
can relax the check to only consider genuine failure.


That pattern is repeated in pretty much all drivers using TTM.

So you would need to fix all of them, but as I said I don't think that 
this approach is a good idea.


We essentially wanted to get rid of the dma_address array in the mid 
term and that change goes into the exactly opposite direction.


Regards,
Christian.



Signed-off-by: Robin Murphy 
---
  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,


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/amdgpu: Allow dma_map_sg() coalescing

2018-04-11 Thread Robin Murphy 
Now that drm_prime_sg_to_page_addr_arrays() understands the case where
dma_map_sg() has coalesced segments and returns 0 < count < nents, we
can relax the check to only consider genuine failure.

Signed-off-by: Robin Murphy 
---
 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.16.1.dirty

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel