Re: [PATCH 1/2] drm/ttm: set ttm_buffer_object pointer as null after it's freed

[Tom St Denis]

On 2018-09-10 9:04 a.m., Christian König wrote:

Hi Tom,

I'm talking about adding new printks to figure out what the heck is 
going wrong here.


Thanks,
Christian.


Hi Christian,

Sure, if you want to send me a simple patch that adds more printk I'll 
gladly give it a try (doubly so since my workstation depends on our 
staging tree to work properly...).


Tom



Am 10.09.2018 um 14:59 schrieb Tom St Denis:

Hi Christian,

Are you adding new traces or turning on existing ones?  Would you like 
me to try them out in my setup?


Tom

On 2018-09-10 8:49 a.m., Christian König wrote:

Am 10.09.2018 um 14:05 schrieb Huang Rui:

On Mon, Sep 10, 2018 at 05:25:48PM +0800, Koenig, Christian wrote:

Am 10.09.2018 um 11:23 schrieb Huang Rui:

On Mon, Sep 10, 2018 at 11:00:04AM +0200, Christian König wrote:

Hi Ray,

well those patches doesn't make sense, the pointer is only local to
the function.

You're right.
I narrowed it with gdb dump from ttm_bo_bulk_move_lru_tail+0x2b, the
use-after-free should be in below codes:

man = >tt[i].first->bdev->man[TTM_PL_TT];
ttm_bo_bulk_move_helper(>tt[i], >lru[i], false);

Is there a case, when orignal bo is destroyed in the bulk pos, but it
doesn't update pos->first pointer, then we still use it during the 
bulk

moving?

Only when a per VM BO is freed or the VM destroyed.

The first case should now be handled by "drm/amdgpu: set bulk_moveable
to false when a per VM is released" and when we use a destroyed VM we
would see other problems as well.


If a VM instance is teared down, all BOs which belong that VM should be
removed from LRU. But how can we submit cmd based on a destroyed VM? 
You

know, we do the bulk move at last step of submission.


Well exactly that's the point this can't happen :)

Otherwise we would crash because of using freed up memory much 
earlier in the command submission.


The best idea I have to track this down further is to add some 
trace_printk in ttm_bo_bulk_move_helper and amdgpu_bo_destroy and see 
why and when we are actually using a destroyed BO.


Christian.




Thanks,
Ray

BTW: Just pushed this commit to the repository, should show up any 
second.


Christian.


Thanks,
Ray


Regards,
Christian.

Am 10.09.2018 um 10:57 schrieb Huang Rui:

It avoids to be refered again after freed.

Signed-off-by: Huang Rui 
Cc: Christian König 
Cc: Tom StDenis 
---
   drivers/gpu/drm/ttm/ttm_bo.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
b/drivers/gpu/drm/ttm/ttm_bo.c

index 138c989..d3ef5f8 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -54,6 +54,7 @@ static struct attribute ttm_bo_count = {
   static void ttm_bo_default_destroy(struct ttm_buffer_object *bo)
   {
   kfree(bo);
+    bo = NULL;
   }
   static inline int ttm_mem_type_from_place(const struct 
ttm_place *place,

___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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




___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




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

Re: [PATCH 1/2] drm/ttm: set ttm_buffer_object pointer as null after it's freed

[Tom St Denis]

Hi Christian,

Are you adding new traces or turning on existing ones?  Would you like 
me to try them out in my setup?


Tom

On 2018-09-10 8:49 a.m., Christian König wrote:

Am 10.09.2018 um 14:05 schrieb Huang Rui:

On Mon, Sep 10, 2018 at 05:25:48PM +0800, Koenig, Christian wrote:

Am 10.09.2018 um 11:23 schrieb Huang Rui:

On Mon, Sep 10, 2018 at 11:00:04AM +0200, Christian König wrote:

Hi Ray,

well those patches doesn't make sense, the pointer is only local to
the function.

You're right.
I narrowed it with gdb dump from ttm_bo_bulk_move_lru_tail+0x2b, the
use-after-free should be in below codes:

man = >tt[i].first->bdev->man[TTM_PL_TT];
ttm_bo_bulk_move_helper(>tt[i], >lru[i], false);

Is there a case, when orignal bo is destroyed in the bulk pos, but it
doesn't update pos->first pointer, then we still use it during the bulk
moving?

Only when a per VM BO is freed or the VM destroyed.

The first case should now be handled by "drm/amdgpu: set bulk_moveable
to false when a per VM is released" and when we use a destroyed VM we
would see other problems as well.


If a VM instance is teared down, all BOs which belong that VM should be
removed from LRU. But how can we submit cmd based on a destroyed VM? You
know, we do the bulk move at last step of submission.


Well exactly that's the point this can't happen :)

Otherwise we would crash because of using freed up memory much earlier 
in the command submission.


The best idea I have to track this down further is to add some 
trace_printk in ttm_bo_bulk_move_helper and amdgpu_bo_destroy and see 
why and when we are actually using a destroyed BO.


Christian.




Thanks,
Ray

BTW: Just pushed this commit to the repository, should show up any 
second.


Christian.


Thanks,
Ray


Regards,
Christian.

Am 10.09.2018 um 10:57 schrieb Huang Rui:

It avoids to be refered again after freed.

Signed-off-by: Huang Rui 
Cc: Christian König 
Cc: Tom StDenis 
---
   drivers/gpu/drm/ttm/ttm_bo.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
b/drivers/gpu/drm/ttm/ttm_bo.c

index 138c989..d3ef5f8 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -54,6 +54,7 @@ static struct attribute ttm_bo_count = {
   static void ttm_bo_default_destroy(struct ttm_buffer_object *bo)
   {
   kfree(bo);
+    bo = NULL;
   }
   static inline int ttm_mem_type_from_place(const struct 
ttm_place *place,

___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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




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

[PATCH] drm/amdgpu: replace iova debugfs file with iomem (v3)

[Tom St Denis]

This allows access to pages allocated through the driver with optional
IOMMU mapping.

v2: Fix number of bytes copied and add write method
v3: drop check for kmap return

Original-by: Christian König <christian.koe...@amd.com>
Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 102 +---
 1 file changed, 81 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index b372d8d650a5..1338c908056f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,98 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {
 
 #endif
 
-static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,
-  size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+size_t size, loff_t *pos)
 {
struct amdgpu_device *adev = file_inode(f)->i_private;
-   int r;
-   uint64_t phys;
struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
 
-   // always return 8 bytes
-   if (size != 8)
-   return -EINVAL;
+   dom = iommu_get_domain_for_dev(adev->dev);
 
-   // only accept page addresses
-   if (*pos & 0xFFF)
-   return -EINVAL;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
+
+   bytes = bytes < size ? bytes : size;
+
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
+
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   r = copy_to_user(buf, ptr, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;
+
+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
+}
+
+static ssize_t amdgpu_iomem_write(struct file *f, const char __user *buf,
+size_t size, loff_t *pos)
+{
+   struct amdgpu_device *adev = file_inode(f)->i_private;
+   struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
 
dom = iommu_get_domain_for_dev(adev->dev);
-   if (dom)
-   phys = iommu_iova_to_phys(dom, *pos);
-   else
-   phys = *pos;
 
-   r = copy_to_user(buf, , 8);
-   if (r)
-   return -EFAULT;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
+
+   bytes = bytes < size ? bytes : size;
 
-   return 8;
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
+
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   r = copy_from_user(ptr, buf, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;
+
+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
 }
 
-static const struct file_operations amdgpu_ttm_iova_fops = {
+static const struct file_operations amdgpu_ttm_iomem_fops = {
.owner = THIS_MODULE,
-   .read = amdgpu_iova_to_phys_read,
+   .read = amdgpu_iomem_read,
+   .write = amdgpu_iomem_write,
.llseek = default_llseek
 };
 
@@ -1973,7 +2033,7 @@ static const struct {
 #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
{ "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT },
 #endif
-   { "amdgpu_iova", _ttm_iova_fops, TTM_PL_SYSTEM },
+   { "amdgpu_iomem", _ttm_iomem_fops, TTM_PL_SYSTEM },
 };
 
 #endif
-- 
2.14.3

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

Re: [PATCH v3 00/13] gpu: drm: amd: remove unused headers

[Tom St Denis]

This will break umr since we source the headers from the kernel.  The 
kernel might not use them but the different IP blocks have deltas that 
umr is aware of.


One might argue that we should then publish the headers somewhere else 
that is public but the kernel is our vehicle right now.


Thoughts Alex/Christian?

Tom


On 14/02/18 09:46 AM, Corentin Labbe wrote:

Hello

This patchset remove several headers which are not used by any source
file.

Regards

Change since v1:
- splited in multiple patchs

Change since v2
- Use --irreversible-delete for format-patch

Corentin Labbe (13):
   drm/amd/include: remove unused asic_reg/oss headers
   drm/amd/include: remove unused asic_reg/bif headers
   drm/amd/include: remove unused asic_reg/dce headers
   drm/amd/include: remove unused asic_reg/gca headers
   drm/amd/include: remove unused asic_reg/gmc headers
   drm/amd/include: remove unused asic_reg/smu headers
   drm/amd/include: remove unused asic_reg/umc headers
   drm/amd/include: remove unused asic_reg/uvd headers
   drm/amd/include: remove unused asic_reg/vce headers
   drm/amd/include: remove unused asic_reg/sdma headers
   drm/amd/include: remove unused asic_reg/nbif headers
   drm/amd/include: remove unused displayobject.h header
   drm/amd/powerplay: remove unused headers

  .../drm/amd/include/asic_reg/bif/bif_5_0_enum.h|  1198 --
  .../drm/amd/include/asic_reg/bif/bif_5_1_enum.h|  1068 -
  .../drm/amd/include/asic_reg/dce/dce_11_2_enum.h   |  6813 --
  .../drm/amd/include/asic_reg/dce/dce_8_0_enum.h|  1117 -
  .../gpu/drm/amd/include/asic_reg/gca/gfx_8_1_d.h   |  2791 ---
  .../drm/amd/include/asic_reg/gca/gfx_8_1_enum.h|  6808 --
  .../drm/amd/include/asic_reg/gca/gfx_8_1_sh_mask.h | 21368 ---
  .../drm/amd/include/asic_reg/gmc/gmc_8_1_enum.h|  1198 --
  .../drm/amd/include/asic_reg/gmc/gmc_8_2_enum.h|  1068 -
  .../amd/include/asic_reg/nbif/nbif_6_1_sh_mask.h   | 10281 -
  .../drm/amd/include/asic_reg/oss/oss_2_4_enum.h|  1340 --
  .../drm/amd/include/asic_reg/oss/oss_3_0_1_enum.h  |  1464 --
  .../drm/amd/include/asic_reg/oss/oss_3_0_enum.h|  1497 --
  .../amd/include/asic_reg/sdma0/sdma0_4_0_default.h |   286 -
  .../amd/include/asic_reg/sdma1/sdma1_4_0_default.h |   282 -
  .../gpu/drm/amd/include/asic_reg/smu/smu_6_0_d.h   |   148 -
  .../drm/amd/include/asic_reg/smu/smu_6_0_sh_mask.h |   715 -
  .../gpu/drm/amd/include/asic_reg/smu/smu_7_1_0_d.h |  1344 --
  .../drm/amd/include/asic_reg/smu/smu_7_1_0_enum.h  |  1191 --
  .../amd/include/asic_reg/smu/smu_7_1_0_sh_mask.h   |  5648 -
  .../drm/amd/include/asic_reg/smu/smu_7_1_1_enum.h  |  1205 --
  .../drm/amd/include/asic_reg/smu/smu_7_1_2_enum.h  |  1246 --
  .../drm/amd/include/asic_reg/smu/smu_7_1_3_enum.h  |  1282 --
  .../drm/amd/include/asic_reg/smu/smu_8_0_enum.h|  1072 -
  .../drm/amd/include/asic_reg/umc/umc_6_0_default.h |31 -
  .../drm/amd/include/asic_reg/umc/umc_6_0_offset.h  |52 -
  .../drm/amd/include/asic_reg/uvd/uvd_4_0_sh_mask.h |   795 -
  .../drm/amd/include/asic_reg/uvd/uvd_5_0_enum.h|  1211 --
  .../drm/amd/include/asic_reg/uvd/uvd_6_0_enum.h|  1081 -
  .../gpu/drm/amd/include/asic_reg/vce/vce_1_0_d.h   |64 -
  .../drm/amd/include/asic_reg/vce/vce_1_0_sh_mask.h |99 -
  drivers/gpu/drm/amd/include/displayobject.h|   249 -
  .../gpu/drm/amd/powerplay/inc/polaris10_ppsmc.h|   412 -
  drivers/gpu/drm/amd/powerplay/inc/pp_feature.h |67 -
  34 files changed, 76491 deletions(-)
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/bif/bif_5_0_enum.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/bif/bif_5_1_enum.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/dce/dce_11_2_enum.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/dce/dce_8_0_enum.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/gca/gfx_8_1_d.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/gca/gfx_8_1_enum.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/gca/gfx_8_1_sh_mask.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/gmc/gmc_8_1_enum.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/gmc/gmc_8_2_enum.h
  delete mode 100644 
drivers/gpu/drm/amd/include/asic_reg/nbif/nbif_6_1_sh_mask.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/oss/oss_2_4_enum.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/oss/oss_3_0_1_enum.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/oss/oss_3_0_enum.h
  delete mode 100644 
drivers/gpu/drm/amd/include/asic_reg/sdma0/sdma0_4_0_default.h
  delete mode 100644 
drivers/gpu/drm/amd/include/asic_reg/sdma1/sdma1_4_0_default.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/smu/smu_6_0_d.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/smu/smu_6_0_sh_mask.h
  delete mode 100644 drivers/gpu/drm/amd/include/asic_reg/smu/smu_7_1_0_d.h
  delete mode 100644 

Re: [PATCH] drm/amdgpu: replace iova debugfs file with iomem (v2)

[Tom St Denis]

On 12/02/18 12:16 PM, Christian König wrote:

Am 12.02.2018 um 17:40 schrieb Tom St Denis:

On 09/02/18 01:44 PM, Christian König wrote:

Am 09.02.2018 um 19:19 schrieb Tom St Denis:

On 09/02/18 01:17 PM, Christian König wrote:

Am 09.02.2018 um 18:28 schrieb Tom St Denis:

On 09/02/18 12:27 PM, Tom St Denis wrote:

From: Christian König <ckoenig.leichtzumer...@gmail.com>


Oops, I'll remove this from the commit message before pushing :-)

I did give you credit below though.


The patch before this one isn't merged yet because I'm still not 
sure if that works under all circumstances or not.


Could you give it some extended testing? Especially if it work with 
eviction.


I supposed there is a race on the kmap'ed memory which is why I 
added a ptr check.  Not perfect but since it's a debugfs entry 
probably better than it needs to be.


I think you can drop that, kmap can only return NULL on 32bit systems 
when we ran out of vmap area and then you can basically call panic() 
as well.


The question is if setting page->mapping during allocation has some 
undesired side effect.



Haven't noticed anything with piglit or other FOSS GL games.

Is there a specific test you'd like me to consider?


Try to restrict VRAM/GTT/memory in general and run something which needs 
a lot of CPU access to BOs. When it affects anything then the CPU page 
table unmap of BOs during eviction.



Limiting vram to 256M seems to be fine for both my iGPU and dGPU 
(thought it drops heaven from 40fps to about 3fps hehehe).


Trying to play with gttsize=256M seems to have broken things I get:


[  165.080976] [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* 
amdgpu_cs_list_validate(validated) failed.
[  165.081163] [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory 
for command submission!


and it seems heaven is blocked.  There are fences being submitted (and 
signalled) but nothing is being drawn.


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

Re: [PATCH] drm/amdgpu: replace iova debugfs file with iomem (v2)

[Tom St Denis]

On 09/02/18 01:44 PM, Christian König wrote:

Am 09.02.2018 um 19:19 schrieb Tom St Denis:

On 09/02/18 01:17 PM, Christian König wrote:

Am 09.02.2018 um 18:28 schrieb Tom St Denis:

On 09/02/18 12:27 PM, Tom St Denis wrote:

From: Christian König <ckoenig.leichtzumer...@gmail.com>


Oops, I'll remove this from the commit message before pushing :-)

I did give you credit below though.


The patch before this one isn't merged yet because I'm still not sure 
if that works under all circumstances or not.


Could you give it some extended testing? Especially if it work with 
eviction.


I supposed there is a race on the kmap'ed memory which is why I added 
a ptr check.  Not perfect but since it's a debugfs entry probably 
better than it needs to be.


I think you can drop that, kmap can only return NULL on 32bit systems 
when we ran out of vmap area and then you can basically call panic() as 
well.


The question is if setting page->mapping during allocation has some 
undesired side effect.



Haven't noticed anything with piglit or other FOSS GL games.

Is there a specific test you'd like me to consider?

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

[PATCH] drm/amdgpu: replace iova debugfs file with iomem (v3)

[Tom St Denis]

From: Christian König <ckoenig.leichtzumer...@gmail.com>

This allows access to pages allocated through the driver with optional
IOMMU mapping.

v2: Fix number of bytes copied and add write method
v3: drop check for kmap return

Original-by: Christian König <christian.koe...@amd.com>
Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 102 +---
 1 file changed, 81 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index b372d8d650a5..1338c908056f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,98 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {
 
 #endif
 
-static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,
-  size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+size_t size, loff_t *pos)
 {
struct amdgpu_device *adev = file_inode(f)->i_private;
-   int r;
-   uint64_t phys;
struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
 
-   // always return 8 bytes
-   if (size != 8)
-   return -EINVAL;
+   dom = iommu_get_domain_for_dev(adev->dev);
 
-   // only accept page addresses
-   if (*pos & 0xFFF)
-   return -EINVAL;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
+
+   bytes = bytes < size ? bytes : size;
+
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
+
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   r = copy_to_user(buf, ptr, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;
+
+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
+}
+
+static ssize_t amdgpu_iomem_write(struct file *f, const char __user *buf,
+size_t size, loff_t *pos)
+{
+   struct amdgpu_device *adev = file_inode(f)->i_private;
+   struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
 
dom = iommu_get_domain_for_dev(adev->dev);
-   if (dom)
-   phys = iommu_iova_to_phys(dom, *pos);
-   else
-   phys = *pos;
 
-   r = copy_to_user(buf, , 8);
-   if (r)
-   return -EFAULT;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
+
+   bytes = bytes < size ? bytes : size;
 
-   return 8;
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
+
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   r = copy_from_user(ptr, buf, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;
+
+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
 }
 
-static const struct file_operations amdgpu_ttm_iova_fops = {
+static const struct file_operations amdgpu_ttm_iomem_fops = {
.owner = THIS_MODULE,
-   .read = amdgpu_iova_to_phys_read,
+   .read = amdgpu_iomem_read,
+   .write = amdgpu_iomem_write,
.llseek = default_llseek
 };
 
@@ -1973,7 +2033,7 @@ static const struct {
 #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
{ "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT },
 #endif
-   { "amdgpu_iova", _ttm_iova_fops, TTM_PL_SYSTEM },
+   { "amdgpu_iomem", _ttm_iomem_fops, TTM_PL_SYSTEM },
 };
 
 #endif
-- 
2.14.3

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

Re: [PATCH] drm/amdgpu: replace iova debugfs file with iomem (v2)

[Tom St Denis]

On 09/02/18 01:17 PM, Christian König wrote:

Am 09.02.2018 um 18:28 schrieb Tom St Denis:

On 09/02/18 12:27 PM, Tom St Denis wrote:

From: Christian König <ckoenig.leichtzumer...@gmail.com>


Oops, I'll remove this from the commit message before pushing :-)

I did give you credit below though.


The patch before this one isn't merged yet because I'm still not sure if 
that works under all circumstances or not.


Could you give it some extended testing? Especially if it work with 
eviction.


I supposed there is a race on the kmap'ed memory which is why I added a 
ptr check.  Not perfect but since it's a debugfs entry probably better 
than it needs to be.


I'll give it a try on some live traffic next.

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

Re: [PATCH] drm/amdgpu: replace iova debugfs file with iomem (v2)

[Tom St Denis]

On 09/02/18 12:27 PM, Tom St Denis wrote:

From: Christian König <ckoenig.leichtzumer...@gmail.com>


Oops, I'll remove this from the commit message before pushing :-)

I did give you credit below though.

Tom



This allows access to pages allocated through the driver with optional
IOMMU mapping.

v2: Fix number of bytes copied and add write method

Original-by: Christian König <christian.koe...@amd.com>
Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 110 ++--
  1 file changed, 89 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index b372d8d650a5..d6c56b001a2c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,106 @@ static const struct file_operations 
amdgpu_ttm_gtt_fops = {
  
  #endif
  
-static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,

-  size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+size_t size, loff_t *pos)
  {
struct amdgpu_device *adev = file_inode(f)->i_private;
-   int r;
-   uint64_t phys;
struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
  
-	// always return 8 bytes

-   if (size != 8)
-   return -EINVAL;
+   dom = iommu_get_domain_for_dev(adev->dev);
  
-	// only accept page addresses

-   if (*pos & 0xFFF)
-   return -EINVAL;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
+
+   bytes = bytes < size ? bytes : size;
+
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
+
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   if (ptr) {
+   r = copy_to_user(buf, ptr, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;
+   } else {
+   return -EFAULT;
+   }
+
+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
+}
+
+static ssize_t amdgpu_iomem_write(struct file *f, const char __user *buf,
+size_t size, loff_t *pos)
+{
+   struct amdgpu_device *adev = file_inode(f)->i_private;
+   struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
  
  	dom = iommu_get_domain_for_dev(adev->dev);

-   if (dom)
-   phys = iommu_iova_to_phys(dom, *pos);
-   else
-   phys = *pos;
  
-	r = copy_to_user(buf, , 8);

-   if (r)
-   return -EFAULT;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
+
+   bytes = bytes < size ? bytes : size;
+
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
  
-	return 8;

+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   if (ptr) {
+   r = copy_from_user(ptr, buf, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;
+   } else {
+   return -EFAULT;
+   }
+
+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
  }
  
-static const struct file_operations amdgpu_ttm_iova_fops = {

+static const struct file_operations amdgpu_ttm_iomem_fops = {
.owner = THIS_MODULE,
-   .read = amdgpu_iova_to_phys_read,
+   .read = amdgpu_iomem_read,
+   .write = amdgpu_iomem_write,
.llseek = default_llseek
  };
  
@@ -1973,7 +2041,7 @@ static const struct {

  #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
{ "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT },
  #endif
-   { "amdgpu_iova", _ttm_iova

[PATCH] drm/amdgpu: replace iova debugfs file with iomem (v2)

[Tom St Denis]

From: Christian König <ckoenig.leichtzumer...@gmail.com>

This allows access to pages allocated through the driver with optional
IOMMU mapping.

v2: Fix number of bytes copied and add write method

Original-by: Christian König <christian.koe...@amd.com>
Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 110 ++--
 1 file changed, 89 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index b372d8d650a5..d6c56b001a2c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,106 @@ static const struct file_operations 
amdgpu_ttm_gtt_fops = {
 
 #endif
 
-static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,
-  size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+size_t size, loff_t *pos)
 {
struct amdgpu_device *adev = file_inode(f)->i_private;
-   int r;
-   uint64_t phys;
struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
 
-   // always return 8 bytes
-   if (size != 8)
-   return -EINVAL;
+   dom = iommu_get_domain_for_dev(adev->dev);
 
-   // only accept page addresses
-   if (*pos & 0xFFF)
-   return -EINVAL;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
+
+   bytes = bytes < size ? bytes : size;
+
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
+
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   if (ptr) {
+   r = copy_to_user(buf, ptr, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;
+   } else {
+   return -EFAULT;
+   }
+
+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
+}
+
+static ssize_t amdgpu_iomem_write(struct file *f, const char __user *buf,
+size_t size, loff_t *pos)
+{
+   struct amdgpu_device *adev = file_inode(f)->i_private;
+   struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
 
dom = iommu_get_domain_for_dev(adev->dev);
-   if (dom)
-   phys = iommu_iova_to_phys(dom, *pos);
-   else
-   phys = *pos;
 
-   r = copy_to_user(buf, , 8);
-   if (r)
-   return -EFAULT;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
+
+   bytes = bytes < size ? bytes : size;
+
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
 
-   return 8;
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   if (ptr) {
+   r = copy_from_user(ptr, buf, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;
+   } else {
+   return -EFAULT;
+   }
+
+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
 }
 
-static const struct file_operations amdgpu_ttm_iova_fops = {
+static const struct file_operations amdgpu_ttm_iomem_fops = {
.owner = THIS_MODULE,
-   .read = amdgpu_iova_to_phys_read,
+   .read = amdgpu_iomem_read,
+   .write = amdgpu_iomem_write,
.llseek = default_llseek
 };
 
@@ -1973,7 +2041,7 @@ static const struct {
 #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
{ "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT },
 #endif
-   { "amdgpu_iova", _ttm_iova_fops, TTM_PL_SYSTEM },
+   { "amdgpu_iomem", _ttm_iomem_fops, TTM_PL_SYSTEM },
 };
 
 #endif
-- 
2.14.3

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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

[Tom St Denis]

On 09/02/18 09:56 AM, Christian König wrote:

Am 09.02.2018 um 15:51 schrieb Tom St Denis:

On 09/02/18 09:12 AM, Christian König wrote:
No, there is simply no need to initialize the system domain. What are 
the values of p->mapping and adev->mman.bdev.dev_mapping when they 
don't match? Maybe we are allocating memory before initializing 
adev->mman.bdev.dev_mapping.


In my test setup I'm running test 3 from libdrm (suite 1) with a pause 
before the unmap/free call.  So the IB should still be mapped.  Indeed 
the VM PTE decoding has the V bit set.


Or do you have more than one GPU in the system? E.g. APU+dGPU? Could 
it be that you read through the wrong device?


I found the issue:

while (size) {
    phys_addr_t addr = *pos & PAGE_MASK;
    loff_t off = *pos & ~PAGE_MASK;
    size_t bytes = PAGE_SIZE - off;

"bytes" should be limited by the 'size' parameter passed in.  What is 
happening instead is it's reading the entire PTB until it hits a V=0 
page and then returns an error and in the process is doing "fun 
things" to the user mode application (by copying more data than I 
asked for).


Ah, obvious problem.

Do you want to fix it or should I take a look? You wanted to add write 
support as well anyway IIRC.


Yup, I can tackle this this afternoon.

I'll take your read only patch and make it do both read/write (and fix 
the minor error).


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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

[Tom St Denis]

On 09/02/18 09:12 AM, Christian König wrote:
No, there is simply no need to initialize the system domain. What are 
the values of p->mapping and adev->mman.bdev.dev_mapping when they don't 
match? Maybe we are allocating memory before initializing 
adev->mman.bdev.dev_mapping.


In my test setup I'm running test 3 from libdrm (suite 1) with a pause 
before the unmap/free call.  So the IB should still be mapped.  Indeed 
the VM PTE decoding has the V bit set.


Or do you have more than one GPU in the system? E.g. APU+dGPU? Could it 
be that you read through the wrong device?


I found the issue:

while (size) {
phys_addr_t addr = *pos & PAGE_MASK;
loff_t off = *pos & ~PAGE_MASK;
size_t bytes = PAGE_SIZE - off;

"bytes" should be limited by the 'size' parameter passed in.  What is 
happening instead is it's reading the entire PTB until it hits a V=0 
page and then returns an error and in the process is doing "fun things" 
to the user mode application (by copying more data than I asked for).



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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

[Tom St Denis]

On 09/02/18 08:56 AM, Christian König wrote:

Am 09.02.2018 um 14:32 schrieb Tom St Denis:

On 02/02/18 02:09 PM, Christian König wrote:

[SNIP]
+    if (p->mapping != adev->mman.bdev.dev_mapping)
+    return -EPERM;


This comparison fails for both IOMMU and non-IOMMU devices in my 
carrizo+polaris10 box.


The address being read from is what the VM decodes to (checked with 
strace).


Have you applied the whole series? That patches before this one are 
necessary to initialize p->mapping when there isn't any userspace 
mapping for the page.



Yes, I have the entire 5 pages applied to a temp branch based on the tip 
of drm-next


$ git log --oneline HEAD~10..
405bc1dc85db (HEAD -> iomem) wip
a06d7a6f29e4 drm/amdgpu: replace iova debugfs file with iomem
d324c21f2c5e drm/ttm: set page mapping during allocation
9f440ee91c58 drm/radeon: remove extra TT unpopulated check
f55d505b0387 drm/amdgpu: remove extra TT unpopulated check
37d705119ea8 drm/ttm: add ttm_tt_populate wrapper
53af6035d04b (origin/amd-staging-drm-next, amd-staging-drm-next) 
drm/radeon: only enable swiotlb path when need v2


(the wip is me adding printks to see which error path is taken).

I don't see an init call for adev->mman.bdev.man[TTM_PL_SYSTEM] 
anywhere.  Maybe that's related?


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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

[Tom St Denis]

On 02/02/18 02:09 PM, Christian König wrote:

This allows access to pages allocated through the driver with optional
IOMMU mapping.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 -
  1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {
  
  #endif
  
-static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,

-  size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+size_t size, loff_t *pos)
  {
struct amdgpu_device *adev = file_inode(f)->i_private;
-   int r;
-   uint64_t phys;
struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
  
-	// always return 8 bytes

-   if (size != 8)
-   return -EINVAL;
+   dom = iommu_get_domain_for_dev(adev->dev);
  
-	// only accept page addresses

-   if (*pos & 0xFFF)
-   return -EINVAL;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
  
-	dom = iommu_get_domain_for_dev(adev->dev);

-   if (dom)
-   phys = iommu_iova_to_phys(dom, *pos);
-   else
-   phys = *pos;
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
  
-	r = copy_to_user(buf, , 8);

-   if (r)
-   return -EFAULT;
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;


This comparison fails for both IOMMU and non-IOMMU devices in my 
carrizo+polaris10 box.


The address being read from is what the VM decodes to (checked with strace).

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

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

[Tom St Denis]

Attached is a patch for umr{master} which should in theory support both 
iova and iomem.


I can add the write method if you want since ya it should be fairly 
simple to copy/pasta that up.


Cheers,
Tom

On 05/02/18 07:07 AM, Christian König wrote:

Well adding write support is trivial.

What I'm more concerned about is if setting page->mapping during 
allocation of the page could have any negative effect?


I of hand don't see any since the page isn't reclaimable directly 
anyway, but I'm not 100% sure of that.


Christian.

Am 05.02.2018 um 12:49 schrieb Tom St Denis:
Another thing that occurred to me is this will break write access to 
GPU bound memory.  Previously we relied on iova to translate the 
address and then /dev/mem or /dev/fmem to read/write it.  But since 
this is literally a read only method obviously there's no write support.


Tom


On 04/02/18 09:56 PM, He, Roger wrote:

Patch1 & 2 & 4,   Reviewed-by: Roger He <hongbo...@amd.com>
Patch 5:  Acked-by: Roger He <hongbo...@amd.com>

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On 
Behalf Of Christian K?nig

Sent: Saturday, February 03, 2018 3:10 AM
To: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Subject: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

This allows access to pages allocated through the driver with 
optional IOMMU mapping.


Signed-off-by: Christian König <christian.koe...@amd.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 
-

  1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c

index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations 
amdgpu_ttm_gtt_fops = {

    #endif
  -static ssize_t amdgpu_iova_to_phys_read(struct file *f, char 
__user *buf,

-   size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+ size_t size, loff_t *pos)
  {
  struct amdgpu_device *adev = file_inode(f)->i_private;
-    int r;
-    uint64_t phys;
  struct iommu_domain *dom;
+    ssize_t result = 0;
+    int r;
  -    // always return 8 bytes
-    if (size != 8)
-    return -EINVAL;
+    dom = iommu_get_domain_for_dev(adev->dev);
  -    // only accept page addresses
-    if (*pos & 0xFFF)
-    return -EINVAL;
+    while (size) {
+    phys_addr_t addr = *pos & PAGE_MASK;
+    loff_t off = *pos & ~PAGE_MASK;
+    size_t bytes = PAGE_SIZE - off;
+    unsigned long pfn;
+    struct page *p;
+    void *ptr;
  -    dom = iommu_get_domain_for_dev(adev->dev);
-    if (dom)
-    phys = iommu_iova_to_phys(dom, *pos);
-    else
-    phys = *pos;
+    addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
  -    r = copy_to_user(buf, , 8);
-    if (r)
-    return -EFAULT;
+    pfn = addr >> PAGE_SHIFT;
+    if (!pfn_valid(pfn))
+    return -EPERM;
+
+    p = pfn_to_page(pfn);
+    if (p->mapping != adev->mman.bdev.dev_mapping)
+    return -EPERM;
+
+    ptr = kmap(p);
+    r = copy_to_user(buf, ptr, bytes);
+    kunmap(p);
+    if (r)
+    return -EFAULT;
  -    return 8;
+    size -= bytes;
+    *pos += bytes;
+    result += bytes;
+    }
+
+    return result;
  }
  -static const struct file_operations amdgpu_ttm_iova_fops = {
+static const struct file_operations amdgpu_ttm_iomem_fops = {
  .owner = THIS_MODULE,
-    .read = amdgpu_iova_to_phys_read,
+    .read = amdgpu_iomem_read,
  .llseek = default_llseek
  };
  @@ -1973,7 +1986,7 @@ static const struct {  #ifdef 
CONFIG_DRM_AMDGPU_GART_DEBUGFS

  { "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT }, #endif
-    { "amdgpu_iova", _ttm_iova_fops, TTM_PL_SYSTEM },
+    { "amdgpu_iomem", _ttm_iomem_fops, TTM_PL_SYSTEM },
  };
    #endif
--
2.14.1

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







>From 67703a62763dfb2107bd503c5ae76414a50c50a4 Mon Sep 17 00:00:00 2001
From: Tom St Denis <tom.stde...@amd.com>
Date: Mon, 5 Feb 2018 08:53:40 -0500
Subject: [PATCH umr] add support for new iomem debugfs entry

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 src/lib/close_asic.c |  1 +
 src/lib/discover.c   |  3 +++
 src/lib/read_vram.c  | 29 +++--
 src/umr.h|  3 ++-
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/src/lib/close_asic.c b/src/lib

Re: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

[Tom St Denis]

Another thing that occurred to me is this will break write access to GPU 
bound memory.  Previously we relied on iova to translate the address and 
then /dev/mem or /dev/fmem to read/write it.  But since this is 
literally a read only method obviously there's no write support.


Tom


On 04/02/18 09:56 PM, He, Roger wrote:

Patch1 & 2 & 4,   Reviewed-by: Roger He 
Patch 5:  Acked-by: Roger He 

-Original Message-
From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
Christian K?nig
Sent: Saturday, February 03, 2018 3:10 AM
To: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Subject: [PATCH 5/5] drm/amdgpu: replace iova debugfs file with iomem

This allows access to pages allocated through the driver with optional IOMMU 
mapping.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 57 -
  1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 648c449aaa79..795ceaeb82d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1929,38 +1929,51 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {
  
  #endif
  
-static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,

-  size_t size, loff_t *pos)
+static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
+size_t size, loff_t *pos)
  {
struct amdgpu_device *adev = file_inode(f)->i_private;
-   int r;
-   uint64_t phys;
struct iommu_domain *dom;
+   ssize_t result = 0;
+   int r;
  
-	// always return 8 bytes

-   if (size != 8)
-   return -EINVAL;
+   dom = iommu_get_domain_for_dev(adev->dev);
  
-	// only accept page addresses

-   if (*pos & 0xFFF)
-   return -EINVAL;
+   while (size) {
+   phys_addr_t addr = *pos & PAGE_MASK;
+   loff_t off = *pos & ~PAGE_MASK;
+   size_t bytes = PAGE_SIZE - off;
+   unsigned long pfn;
+   struct page *p;
+   void *ptr;
  
-	dom = iommu_get_domain_for_dev(adev->dev);

-   if (dom)
-   phys = iommu_iova_to_phys(dom, *pos);
-   else
-   phys = *pos;
+   addr = dom ? iommu_iova_to_phys(dom, addr) : addr;
  
-	r = copy_to_user(buf, , 8);

-   if (r)
-   return -EFAULT;
+   pfn = addr >> PAGE_SHIFT;
+   if (!pfn_valid(pfn))
+   return -EPERM;
+
+   p = pfn_to_page(pfn);
+   if (p->mapping != adev->mman.bdev.dev_mapping)
+   return -EPERM;
+
+   ptr = kmap(p);
+   r = copy_to_user(buf, ptr, bytes);
+   kunmap(p);
+   if (r)
+   return -EFAULT;
  
-	return 8;

+   size -= bytes;
+   *pos += bytes;
+   result += bytes;
+   }
+
+   return result;
  }
  
-static const struct file_operations amdgpu_ttm_iova_fops = {

+static const struct file_operations amdgpu_ttm_iomem_fops = {
.owner = THIS_MODULE,
-   .read = amdgpu_iova_to_phys_read,
+   .read = amdgpu_iomem_read,
.llseek = default_llseek
  };
  
@@ -1973,7 +1986,7 @@ static const struct {  #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS

{ "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT },  #endif
-   { "amdgpu_iova", _ttm_iova_fops, TTM_PL_SYSTEM },
+   { "amdgpu_iomem", _ttm_iomem_fops, TTM_PL_SYSTEM },
  };
  
  #endif

--
2.14.1

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



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

[PATCH 08/12] drm/ttm: Remove unncessary retval from ttm_bo_vm_fault()

[Tom St Denis]

The dual ret/retval was more complex than need be.  Now
we drop the retval variable and assign the appropriate VM
codes to ret instead.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 28 +---
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 60fcef1593dd..716e724ac710 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -118,7 +118,6 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
int ret;
int i;
unsigned long address = vmf->address;
-   int retval = VM_FAULT_NOPAGE;
struct ttm_mem_type_manager *man =
>man[bo->mem.mem_type];
struct vm_area_struct cvma;
@@ -158,7 +157,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
 * (if at all) by redirecting mmap to the exporter.
 */
if (bo->ttm && (bo->ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-   retval = VM_FAULT_SIGBUS;
+   ret = VM_FAULT_SIGBUS;
goto out_unlock;
}
 
@@ -169,10 +168,10 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
break;
case -EBUSY:
case -ERESTARTSYS:
-   retval = VM_FAULT_NOPAGE;
+   ret = VM_FAULT_NOPAGE;
goto out_unlock;
default:
-   retval = VM_FAULT_SIGBUS;
+   ret = VM_FAULT_SIGBUS;
goto out_unlock;
}
}
@@ -183,12 +182,10 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
 */
ret = ttm_bo_vm_fault_idle(bo, vmf);
if (unlikely(ret != 0)) {
-   retval = ret;
-
-   if (retval == VM_FAULT_RETRY &&
+   if (ret == VM_FAULT_RETRY &&
!(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) {
/* The BO has already been unreserved. */
-   return retval;
+   return ret;
}
 
goto out_unlock;
@@ -196,12 +193,12 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
 
ret = ttm_mem_io_lock(man, true);
if (unlikely(ret != 0)) {
-   retval = VM_FAULT_NOPAGE;
+   ret = VM_FAULT_NOPAGE;
goto out_unlock;
}
ret = ttm_mem_io_reserve_vm(bo);
if (unlikely(ret != 0)) {
-   retval = VM_FAULT_SIGBUS;
+   ret = VM_FAULT_SIGBUS;
goto out_io_unlock;
}
 
@@ -211,7 +208,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
drm_vma_node_start(>vma_node);
 
if (unlikely(page_offset >= bo->num_pages)) {
-   retval = VM_FAULT_SIGBUS;
+   ret = VM_FAULT_SIGBUS;
goto out_io_unlock;
}
 
@@ -238,7 +235,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
 
/* Allocate all page at once, most common usage */
if (ttm->bdev->driver->ttm_tt_populate(ttm, )) {
-   retval = VM_FAULT_OOM;
+   ret = VM_FAULT_OOM;
goto out_io_unlock;
}
}
@@ -255,7 +252,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
} else {
page = ttm->pages[page_offset];
if (unlikely(!page && i == 0)) {
-   retval = VM_FAULT_OOM;
+   ret = VM_FAULT_OOM;
goto out_io_unlock;
} else if (unlikely(!page)) {
break;
@@ -280,7 +277,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
if (unlikely((ret == -EBUSY) || (ret != 0 && i > 0)))
break;
else if (unlikely(ret != 0)) {
-   retval =
+   ret =
(ret == -ENOMEM) ? VM_FAULT_OOM : VM_FAULT_SIGBUS;
goto out_io_unlock;
}
@@ -289,11 +286,12 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
if (unlikely(++page_offset >= page_last))
break;
}
+   ret = VM_FAULT_NOPAGE;
 out_io_unlock:
ttm_mem_io_unlock(man);
 out_unlock:
ttm_bo_unreserve(bo);
-   return retval;
+   return ret;
 }
 
 static void ttm_bo_vm_open(struct vm_area_struct *vma)
-- 
2.14.3

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

[PATCH 11/12] drm/ttm: Fix coding style in ttm_dma_pool_alloc_new_pages()

[Tom St Denis]

Add missing {} braces.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index 469e68e06be6..fcd16804c738 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -763,10 +763,9 @@ static int ttm_dma_pool_alloc_new_pages(struct dma_pool 
*pool,
return -ENOMEM;
}
 
-   if (count > 1) {
+   if (count > 1)
pr_debug("%s: (%s:%d) Getting %d pages\n",
 pool->dev_name, pool->name, current->pid, count);
-   }
 
for (i = 0, cpages = 0; i < count; ++i) {
dma_p = __ttm_dma_alloc_page(pool);
-- 
2.14.3

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

[PATCH 07/12] drm/ttm: Fix coding style in ttm_bo_move_memcpy()

[Tom St Denis]

Add missing {} braces.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 153de1bf0232..33ffe286f3a5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -402,8 +402,9 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
PAGE_KERNEL);
ret = ttm_copy_io_ttm_page(ttm, old_iomap, page,
   prot);
-   } else
+   } else {
ret = ttm_copy_io_page(new_iomap, old_iomap, page);
+   }
if (ret)
goto out1;
}
-- 
2.14.3

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

[PATCH 10/12] drm/ttm: Fix coding style in ttm_tt_swapout()

[Tom St Denis]

Add missing {} braces.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_tt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index e90d3ed6283f..95a77dab8cc9 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -352,8 +352,9 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file 
*persistent_swap_storage)
pr_err("Failed allocating swap storage\n");
return PTR_ERR(swap_storage);
}
-   } else
+   } else {
swap_storage = persistent_swap_storage;
+   }
 
swap_space = swap_storage->f_mapping;
 
-- 
2.14.3

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

[PATCH 12/12] drm/ttm: Simplify ttm_dma_page_put()

[Tom St Denis]

Remove redundant store of return code.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index fcd16804c738..b122f6eee94c 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -390,14 +390,12 @@ static void ttm_dma_page_put(struct dma_pool *pool, 
struct dma_page *d_page)
 {
struct page *page = d_page->p;
unsigned i, num_pages;
-   int ret;
 
/* Don't set WB on WB page pool. */
if (!(pool->type & IS_CACHED)) {
num_pages = pool->size / PAGE_SIZE;
for (i = 0; i < num_pages; ++i, ++page) {
-   ret = set_pages_array_wb(, 1);
-   if (ret) {
+   if (set_pages_array_wb(, 1)) {
pr_err("%s: Failed to set %d pages to wb!\n",
   pool->dev_name, 1);
}
-- 
2.14.3

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

[PATCH 05/12] drm/ttm: Fix coding style in ttm_pool_store()

[Tom St Denis]

Correct missing {} style.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index 9e90d0ebc773..647eb5f40ab9 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -210,6 +210,7 @@ static ssize_t ttm_pool_store(struct kobject *kobj, struct 
attribute *attr,
container_of(kobj, struct ttm_pool_manager, kobj);
int chars;
unsigned val;
+
chars = sscanf(buffer, "%u", );
if (chars == 0)
return size;
@@ -217,11 +218,11 @@ static ssize_t ttm_pool_store(struct kobject *kobj, 
struct attribute *attr,
/* Convert kb to number of pages */
val = val / (PAGE_SIZE >> 10);
 
-   if (attr == _page_pool_max)
+   if (attr == _page_pool_max) {
m->options.max_size = val;
-   else if (attr == _page_pool_small)
+   } else if (attr == _page_pool_small) {
m->options.small = val;
-   else if (attr == _page_pool_alloc_size) {
+   } else if (attr == _page_pool_alloc_size) {
if (val > NUM_PAGES_TO_ALLOC*8) {
pr_err("Setting allocation size to %lu is not allowed. 
Recommended size is %lu\n",
   NUM_PAGES_TO_ALLOC*(PAGE_SIZE >> 7),
-- 
2.14.3

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

[PATCH 09/12] drm/ttm: Simplify ttm_eu_reserve_buffers()

[Tom St Denis]

Hoist the comparison of the ret to -EDEADLK above
the two code paths to simplify the function.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_execbuf_util.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c 
b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
index 373ced0b2fc2..fa44f7b15285 100644
--- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
+++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
@@ -139,12 +139,14 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
 */
ttm_eu_backoff_reservation_reverse(list, entry);
 
-   if (ret == -EDEADLK && intr) {
-   ret = ww_mutex_lock_slow_interruptible(>resv->lock,
-  ticket);
-   } else if (ret == -EDEADLK) {
-   ww_mutex_lock_slow(>resv->lock, ticket);
-   ret = 0;
+   if (ret == -EDEADLK) {
+   if (intr) {
+   ret = 
ww_mutex_lock_slow_interruptible(>resv->lock,
+  ticket);
+   } else {
+   ww_mutex_lock_slow(>resv->lock, ticket);
+   ret = 0;
+   }
}
 
if (!ret && entry->shared)
-- 
2.14.3

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

[PATCH 06/12] drm/ttm: Simplify ttm_dma_find_pool() (v2)

[Tom St Denis]

Flip the logic of the comparison and remove
the redudant variable for the pool address.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>

(v2): Remove {} bracing.
---
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index 647eb5f40ab9..469e68e06be6 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -682,10 +682,10 @@ static struct dma_pool *ttm_dma_pool_init(struct device 
*dev, gfp_t flags,
 static struct dma_pool *ttm_dma_find_pool(struct device *dev,
  enum pool_type type)
 {
-   struct dma_pool *pool, *tmp, *found = NULL;
+   struct dma_pool *pool, *tmp;
 
if (type == IS_UNDEFINED)
-   return found;
+   return NULL;
 
/* NB: We iterate on the 'struct dev' which has no spinlock, but
 * it does have a kref which we have taken. The kref is taken during
@@ -698,13 +698,10 @@ static struct dma_pool *ttm_dma_find_pool(struct device 
*dev,
 * thing is at that point of time there are no pages associated with the
 * driver so this function will not be called.
 */
-   list_for_each_entry_safe(pool, tmp, >dma_pools, pools) {
-   if (pool->type != type)
-   continue;
-   found = pool;
-   break;
-   }
-   return found;
+   list_for_each_entry_safe(pool, tmp, >dma_pools, pools)
+   if (pool->type == type)
+   return pool;
+   return NULL;
 }
 
 /*
-- 
2.14.3

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

Various TTM cleanups (v2)

[Tom St Denis]

Various TTM cleanups (mostly no functional changes).

Notably patch #1 fixes a bug in the access_kmap() function.

The rest are either coding style fixes or simplifications.


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

[PATCH 02/12] drm/ttm: Fix coding style in ttm_bo.c

[Tom St Denis]

Correct indentation and {} brace style.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index d33a6bb742a1..8cf89da7030d 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -149,9 +149,8 @@ static void ttm_bo_release_list(struct kref *list_kref)
mutex_destroy(>wu_mutex);
if (bo->destroy)
bo->destroy(bo);
-   else {
+   else
kfree(bo);
-   }
ttm_mem_global_free(bdev->glob->mem_glob, acc_size);
 }
 
@@ -163,7 +162,6 @@ void ttm_bo_add_to_lru(struct ttm_buffer_object *bo)
reservation_object_assert_held(bo->resv);
 
if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
-
BUG_ON(!list_empty(>lru));
 
man = >man[bo->mem.mem_type];
@@ -614,10 +612,9 @@ static void ttm_bo_delayed_workqueue(struct work_struct 
*work)
struct ttm_bo_device *bdev =
container_of(work, struct ttm_bo_device, wq.work);
 
-   if (!ttm_bo_delayed_delete(bdev, false)) {
+   if (!ttm_bo_delayed_delete(bdev, false))
schedule_delayed_work(>wq,
  ((HZ / 100) < 1) ? 1 : HZ / 100);
-   }
 }
 
 static void ttm_bo_release(struct kref *kref)
-- 
2.14.3

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

[PATCH 01/12] drm/ttm: Fix 'buf' pointer update in ttm_bo_vm_access_kmap() (v2)

[Tom St Denis]

The buf pointer was not being incremented inside the loop
meaning the same block of data would be read or written
repeatedly.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>

(v2) Change 'buf' pointer to uint8_t* type
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 08a3c324242e..60fcef1593dd 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -316,7 +316,7 @@ static void ttm_bo_vm_close(struct vm_area_struct *vma)
 
 static int ttm_bo_vm_access_kmap(struct ttm_buffer_object *bo,
 unsigned long offset,
-void *buf, int len, int write)
+uint8_t *buf, int len, int write)
 {
unsigned long page = offset >> PAGE_SHIFT;
unsigned long bytes_left = len;
@@ -345,6 +345,7 @@ static int ttm_bo_vm_access_kmap(struct ttm_buffer_object 
*bo,
ttm_bo_kunmap();
 
page++;
+   buf += bytes;
bytes_left -= bytes;
offset = 0;
} while (bytes_left);
-- 
2.14.3

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

[PATCH 04/12] drm/ttm: Change ttm_tt page allocations to return errors

[Tom St Denis]

Explicitly return errors in ttm_tt_alloc_page_directory() and
ttm_dma_tt_alloc_page_directory() instead of relying on
further logic to detect errors.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_tt.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 9e4d43d68e91..e90d3ed6283f 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -50,19 +50,25 @@
 /**
  * Allocates storage for pointers to the pages that back the ttm.
  */
-static void ttm_tt_alloc_page_directory(struct ttm_tt *ttm)
+static int ttm_tt_alloc_page_directory(struct ttm_tt *ttm)
 {
ttm->pages = kvmalloc_array(ttm->num_pages, sizeof(void*),
GFP_KERNEL | __GFP_ZERO);
+   if (!ttm->pages)
+   return -ENOMEM;
+   return 0;
 }
 
-static void ttm_dma_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
+static int ttm_dma_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
 {
ttm->ttm.pages = kvmalloc_array(ttm->ttm.num_pages,
  sizeof(*ttm->ttm.pages) +
  sizeof(*ttm->dma_address),
  GFP_KERNEL | __GFP_ZERO);
+   if (!ttm->ttm.pages)
+   return -ENOMEM;
ttm->dma_address = (void *) (ttm->ttm.pages + ttm->ttm.num_pages);
+   return 0;
 }
 
 #ifdef CONFIG_X86
@@ -197,8 +203,7 @@ int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device 
*bdev,
ttm->state = tt_unpopulated;
ttm->swap_storage = NULL;
 
-   ttm_tt_alloc_page_directory(ttm);
-   if (!ttm->pages) {
+   if (ttm_tt_alloc_page_directory(ttm)) {
ttm_tt_destroy(ttm);
pr_err("Failed allocating page table\n");
return -ENOMEM;
@@ -230,8 +235,7 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct 
ttm_bo_device *bdev,
ttm->swap_storage = NULL;
 
INIT_LIST_HEAD(_dma->pages_list);
-   ttm_dma_tt_alloc_page_directory(ttm_dma);
-   if (!ttm->pages) {
+   if (ttm_dma_tt_alloc_page_directory(ttm_dma)) {
ttm_tt_destroy(ttm);
pr_err("Failed allocating page table\n");
return -ENOMEM;
-- 
2.14.3

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

[PATCH 03/12] drm/ttm: Add a default BO destructor to simplify code (v2)

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>

(v2): Remove stray ; noticed by Felix
---
 drivers/gpu/drm/ttm/ttm_bo.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 8cf89da7030d..d90b1cf10b27 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -49,6 +49,12 @@ static struct attribute ttm_bo_count = {
.mode = S_IRUGO
 };
 
+/* default destructor */
+static void ttm_bo_default_destroy(struct ttm_buffer_object *bo)
+{
+   kfree(bo);
+}
+
 static inline int ttm_mem_type_from_place(const struct ttm_place *place,
  uint32_t *mem_type)
 {
@@ -147,10 +153,7 @@ static void ttm_bo_release_list(struct kref *list_kref)
dma_fence_put(bo->moving);
reservation_object_fini(>ttm_resv);
mutex_destroy(>wu_mutex);
-   if (bo->destroy)
-   bo->destroy(bo);
-   else
-   kfree(bo);
+   bo->destroy(bo);
ttm_mem_global_free(bdev->glob->mem_glob, acc_size);
 }
 
@@ -1176,7 +1179,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
ttm_mem_global_free(mem_glob, acc_size);
return -EINVAL;
}
-   bo->destroy = destroy;
+   bo->destroy = destroy ? destroy : ttm_bo_default_destroy;
 
kref_init(>kref);
kref_init(>list_kref);
-- 
2.14.3

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

Re: Various TTM cleanups/fixes

[Tom St Denis]

On 26/01/18 01:38 PM, Christian König wrote:
Instead of "fix indentation" better write "fix coding style" and add 
some commit message to each patch. Something like "No functional 
change..." for the style changes should be ok.


Additional to that please move patch #11 to the top of the list and 
triple check in patch #10 that this is indeed safe.


With that done the series is Reviewed-by: Christian König 
<christian.koe...@amd.com>.


I'll do those changes on Monday and resubmit en masse.  This will give 
time for other dri/ttm folk to review and I can avoid too much churn if 
anyone else has issues.


I agree that #10 is a bit tricky because retval had a default value 
which hopefully I captured with the assignment towards the end of the 
function.  It just seemed kinda awkward to have ret and retval :-)


Thanks,
Tom



Regards,
Christian.

Am 26.01.2018 um 19:28 schrieb Tom St Denis:

This series includes mostly no-functional-changes to simplify
or cleanup various routines.

Patch #11 includes an fix to functional behaviour.

___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




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

[PATCH 09/12] drm/ttm: Fix indentation in ttm_bo_move_memcpy()

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 153de1bf0232..33ffe286f3a5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -402,8 +402,9 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
PAGE_KERNEL);
ret = ttm_copy_io_ttm_page(ttm, old_iomap, page,
   prot);
-   } else
+   } else {
ret = ttm_copy_io_page(new_iomap, old_iomap, page);
+   }
if (ret)
goto out1;
}
-- 
2.14.3

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

[PATCH 05/12] drm/ttm: Fix indentation in ttm_pool_store()

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index 9e90d0ebc773..647eb5f40ab9 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -210,6 +210,7 @@ static ssize_t ttm_pool_store(struct kobject *kobj, struct 
attribute *attr,
container_of(kobj, struct ttm_pool_manager, kobj);
int chars;
unsigned val;
+
chars = sscanf(buffer, "%u", );
if (chars == 0)
return size;
@@ -217,11 +218,11 @@ static ssize_t ttm_pool_store(struct kobject *kobj, 
struct attribute *attr,
/* Convert kb to number of pages */
val = val / (PAGE_SIZE >> 10);
 
-   if (attr == _page_pool_max)
+   if (attr == _page_pool_max) {
m->options.max_size = val;
-   else if (attr == _page_pool_small)
+   } else if (attr == _page_pool_small) {
m->options.small = val;
-   else if (attr == _page_pool_alloc_size) {
+   } else if (attr == _page_pool_alloc_size) {
if (val > NUM_PAGES_TO_ALLOC*8) {
pr_err("Setting allocation size to %lu is not allowed. 
Recommended size is %lu\n",
   NUM_PAGES_TO_ALLOC*(PAGE_SIZE >> 7),
-- 
2.14.3

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

[PATCH 11/12] drm/ttm: Fix 'buf' pointer update in ttm_bo_vm_access_kmap()

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 07b22f04b969..6311f8a481ea 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -345,6 +345,7 @@ static int ttm_bo_vm_access_kmap(struct ttm_buffer_object 
*bo,
page++;
bytes_left -= bytes;
offset = 0;
+   buf += bytes;
} while (bytes_left);
 
return len;
-- 
2.14.3

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

[PATCH 10/12] drm/ttm: Remove unncessary retval from ttm_bo_vm_fault()

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 28 +---
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 08a3c324242e..07b22f04b969 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -118,7 +118,6 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
int ret;
int i;
unsigned long address = vmf->address;
-   int retval = VM_FAULT_NOPAGE;
struct ttm_mem_type_manager *man =
>man[bo->mem.mem_type];
struct vm_area_struct cvma;
@@ -158,7 +157,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
 * (if at all) by redirecting mmap to the exporter.
 */
if (bo->ttm && (bo->ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-   retval = VM_FAULT_SIGBUS;
+   ret = VM_FAULT_SIGBUS;
goto out_unlock;
}
 
@@ -169,10 +168,10 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
break;
case -EBUSY:
case -ERESTARTSYS:
-   retval = VM_FAULT_NOPAGE;
+   ret = VM_FAULT_NOPAGE;
goto out_unlock;
default:
-   retval = VM_FAULT_SIGBUS;
+   ret = VM_FAULT_SIGBUS;
goto out_unlock;
}
}
@@ -183,12 +182,10 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
 */
ret = ttm_bo_vm_fault_idle(bo, vmf);
if (unlikely(ret != 0)) {
-   retval = ret;
-
-   if (retval == VM_FAULT_RETRY &&
+   if (ret == VM_FAULT_RETRY &&
!(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) {
/* The BO has already been unreserved. */
-   return retval;
+   return ret;
}
 
goto out_unlock;
@@ -196,12 +193,12 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
 
ret = ttm_mem_io_lock(man, true);
if (unlikely(ret != 0)) {
-   retval = VM_FAULT_NOPAGE;
+   ret = VM_FAULT_NOPAGE;
goto out_unlock;
}
ret = ttm_mem_io_reserve_vm(bo);
if (unlikely(ret != 0)) {
-   retval = VM_FAULT_SIGBUS;
+   ret = VM_FAULT_SIGBUS;
goto out_io_unlock;
}
 
@@ -211,7 +208,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
drm_vma_node_start(>vma_node);
 
if (unlikely(page_offset >= bo->num_pages)) {
-   retval = VM_FAULT_SIGBUS;
+   ret = VM_FAULT_SIGBUS;
goto out_io_unlock;
}
 
@@ -238,7 +235,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
 
/* Allocate all page at once, most common usage */
if (ttm->bdev->driver->ttm_tt_populate(ttm, )) {
-   retval = VM_FAULT_OOM;
+   ret = VM_FAULT_OOM;
goto out_io_unlock;
}
}
@@ -255,7 +252,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
} else {
page = ttm->pages[page_offset];
if (unlikely(!page && i == 0)) {
-   retval = VM_FAULT_OOM;
+   ret = VM_FAULT_OOM;
goto out_io_unlock;
} else if (unlikely(!page)) {
break;
@@ -280,7 +277,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
if (unlikely((ret == -EBUSY) || (ret != 0 && i > 0)))
break;
else if (unlikely(ret != 0)) {
-   retval =
+   ret =
(ret == -ENOMEM) ? VM_FAULT_OOM : VM_FAULT_SIGBUS;
goto out_io_unlock;
}
@@ -289,11 +286,12 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
if (unlikely(++page_offset >= page_last))
break;
}
+   ret = VM_FAULT_NOPAGE;
 out_io_unlock:
ttm_mem_io_unlock(man);
 out_unlock:
ttm_bo_unreserve(bo);
-   return retval;
+   return ret;
 }
 
 static void ttm_bo_vm_open(struct vm_area_struct *vma)
-- 
2.14.3

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

[PATCH 06/12] drm/ttm: Simplify ttm_dma_page_put()

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index 647eb5f40ab9..962838cfb1a3 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -390,14 +390,12 @@ static void ttm_dma_page_put(struct dma_pool *pool, 
struct dma_page *d_page)
 {
struct page *page = d_page->p;
unsigned i, num_pages;
-   int ret;
 
/* Don't set WB on WB page pool. */
if (!(pool->type & IS_CACHED)) {
num_pages = pool->size / PAGE_SIZE;
for (i = 0; i < num_pages; ++i, ++page) {
-   ret = set_pages_array_wb(, 1);
-   if (ret) {
+   if (set_pages_array_wb(, 1)) {
pr_err("%s: Failed to set %d pages to wb!\n",
   pool->dev_name, 1);
}
-- 
2.14.3

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

[PATCH 08/12] drm/ttm: Fix indentation in ttm_dma_pool_alloc_new_pages()

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index 579c4aedc17e..aa1ec35dc187 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -762,10 +762,9 @@ static int ttm_dma_pool_alloc_new_pages(struct dma_pool 
*pool,
return -ENOMEM;
}
 
-   if (count > 1) {
+   if (count > 1)
pr_debug("%s: (%s:%d) Getting %d pages\n",
 pool->dev_name, pool->name, current->pid, count);
-   }
 
for (i = 0, cpages = 0; i < count; ++i) {
dma_p = __ttm_dma_alloc_page(pool);
-- 
2.14.3

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

[PATCH 12/12] drm/ttm: Simplify ttm_eu_reserve_buffers()

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_execbuf_util.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c 
b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
index 373ced0b2fc2..fa44f7b15285 100644
--- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
+++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
@@ -139,12 +139,14 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
 */
ttm_eu_backoff_reservation_reverse(list, entry);
 
-   if (ret == -EDEADLK && intr) {
-   ret = ww_mutex_lock_slow_interruptible(>resv->lock,
-  ticket);
-   } else if (ret == -EDEADLK) {
-   ww_mutex_lock_slow(>resv->lock, ticket);
-   ret = 0;
+   if (ret == -EDEADLK) {
+   if (intr) {
+   ret = 
ww_mutex_lock_slow_interruptible(>resv->lock,
+  ticket);
+   } else {
+   ww_mutex_lock_slow(>resv->lock, ticket);
+   ret = 0;
+   }
}
 
if (!ret && entry->shared)
-- 
2.14.3

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

[PATCH 07/12] drm/ttm: Simplify ttm_dma_find_pool()

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index 962838cfb1a3..579c4aedc17e 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -680,10 +680,10 @@ static struct dma_pool *ttm_dma_pool_init(struct device 
*dev, gfp_t flags,
 static struct dma_pool *ttm_dma_find_pool(struct device *dev,
  enum pool_type type)
 {
-   struct dma_pool *pool, *tmp, *found = NULL;
+   struct dma_pool *pool, *tmp;
 
if (type == IS_UNDEFINED)
-   return found;
+   return NULL;
 
/* NB: We iterate on the 'struct dev' which has no spinlock, but
 * it does have a kref which we have taken. The kref is taken during
@@ -697,12 +697,10 @@ static struct dma_pool *ttm_dma_find_pool(struct device 
*dev,
 * driver so this function will not be called.
 */
list_for_each_entry_safe(pool, tmp, >dma_pools, pools) {
-   if (pool->type != type)
-   continue;
-   found = pool;
-   break;
+   if (pool->type == type)
+   return pool;
}
-   return found;
+   return NULL;
 }
 
 /*
-- 
2.14.3

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

[PATCH 04/12] drm/ttm: Fix indentation in ttm_tt_swapout()

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_tt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index e90d3ed6283f..95a77dab8cc9 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -352,8 +352,9 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file 
*persistent_swap_storage)
pr_err("Failed allocating swap storage\n");
return PTR_ERR(swap_storage);
}
-   } else
+   } else {
swap_storage = persistent_swap_storage;
+   }
 
swap_space = swap_storage->f_mapping;
 
-- 
2.14.3

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

[PATCH 01/12] drm/ttm: Clean up indentation in ttm_bo.c

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index d33a6bb742a1..8cf89da7030d 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -149,9 +149,8 @@ static void ttm_bo_release_list(struct kref *list_kref)
mutex_destroy(>wu_mutex);
if (bo->destroy)
bo->destroy(bo);
-   else {
+   else
kfree(bo);
-   }
ttm_mem_global_free(bdev->glob->mem_glob, acc_size);
 }
 
@@ -163,7 +162,6 @@ void ttm_bo_add_to_lru(struct ttm_buffer_object *bo)
reservation_object_assert_held(bo->resv);
 
if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
-
BUG_ON(!list_empty(>lru));
 
man = >man[bo->mem.mem_type];
@@ -614,10 +612,9 @@ static void ttm_bo_delayed_workqueue(struct work_struct 
*work)
struct ttm_bo_device *bdev =
container_of(work, struct ttm_bo_device, wq.work);
 
-   if (!ttm_bo_delayed_delete(bdev, false)) {
+   if (!ttm_bo_delayed_delete(bdev, false))
schedule_delayed_work(>wq,
  ((HZ / 100) < 1) ? 1 : HZ / 100);
-   }
 }
 
 static void ttm_bo_release(struct kref *kref)
-- 
2.14.3

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

[PATCH 02/12] drm/ttm: Add a default BO destructor to simplify code

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 8cf89da7030d..4e85c32fea26 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -49,6 +49,12 @@ static struct attribute ttm_bo_count = {
.mode = S_IRUGO
 };
 
+/* default destructor */
+static void ttm_bo_default_destroy(struct ttm_buffer_object *bo)
+{
+   kfree(bo);
+};
+
 static inline int ttm_mem_type_from_place(const struct ttm_place *place,
  uint32_t *mem_type)
 {
@@ -147,10 +153,7 @@ static void ttm_bo_release_list(struct kref *list_kref)
dma_fence_put(bo->moving);
reservation_object_fini(>ttm_resv);
mutex_destroy(>wu_mutex);
-   if (bo->destroy)
-   bo->destroy(bo);
-   else
-   kfree(bo);
+   bo->destroy(bo);
ttm_mem_global_free(bdev->glob->mem_glob, acc_size);
 }
 
@@ -1176,7 +1179,8 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
ttm_mem_global_free(mem_glob, acc_size);
return -EINVAL;
}
-   bo->destroy = destroy;
+   bo->destroy =
+   (destroy == NULL) ? ttm_bo_default_destroy : destroy;
 
kref_init(>kref);
kref_init(>list_kref);
-- 
2.14.3

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

[PATCH 03/12] drm/ttm: Change ttm_tt page allocations to return errors

[Tom St Denis]

Explicitly return errors in ttm_tt_alloc_page_directory() and
ttm_dma_tt_alloc_page_directory() instead of relying on
further logic to detect errors.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_tt.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 9e4d43d68e91..e90d3ed6283f 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -50,19 +50,25 @@
 /**
  * Allocates storage for pointers to the pages that back the ttm.
  */
-static void ttm_tt_alloc_page_directory(struct ttm_tt *ttm)
+static int ttm_tt_alloc_page_directory(struct ttm_tt *ttm)
 {
ttm->pages = kvmalloc_array(ttm->num_pages, sizeof(void*),
GFP_KERNEL | __GFP_ZERO);
+   if (!ttm->pages)
+   return -ENOMEM;
+   return 0;
 }
 
-static void ttm_dma_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
+static int ttm_dma_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
 {
ttm->ttm.pages = kvmalloc_array(ttm->ttm.num_pages,
  sizeof(*ttm->ttm.pages) +
  sizeof(*ttm->dma_address),
  GFP_KERNEL | __GFP_ZERO);
+   if (!ttm->ttm.pages)
+   return -ENOMEM;
ttm->dma_address = (void *) (ttm->ttm.pages + ttm->ttm.num_pages);
+   return 0;
 }
 
 #ifdef CONFIG_X86
@@ -197,8 +203,7 @@ int ttm_tt_init(struct ttm_tt *ttm, struct ttm_bo_device 
*bdev,
ttm->state = tt_unpopulated;
ttm->swap_storage = NULL;
 
-   ttm_tt_alloc_page_directory(ttm);
-   if (!ttm->pages) {
+   if (ttm_tt_alloc_page_directory(ttm)) {
ttm_tt_destroy(ttm);
pr_err("Failed allocating page table\n");
return -ENOMEM;
@@ -230,8 +235,7 @@ int ttm_dma_tt_init(struct ttm_dma_tt *ttm_dma, struct 
ttm_bo_device *bdev,
ttm->swap_storage = NULL;
 
INIT_LIST_HEAD(_dma->pages_list);
-   ttm_dma_tt_alloc_page_directory(ttm_dma);
-   if (!ttm->pages) {
+   if (ttm_dma_tt_alloc_page_directory(ttm_dma)) {
ttm_tt_destroy(ttm);
pr_err("Failed allocating page table\n");
return -ENOMEM;
-- 
2.14.3

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

Various TTM cleanups/fixes

[Tom St Denis]

This series includes mostly no-functional-changes to simplify
or cleanup various routines.

Patch #11 includes an fix to functional behaviour.

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

Re: 'buf' pointer in ttm_bo_vm_access_kmap()

[Tom St Denis]

On 26/01/18 09:38 AM, Christian König wrote:

Am 26.01.2018 um 15:31 schrieb Tom St Denis:

Hi all,

In the function ttm_bo_vm_access_kmap() it doesn't seem to me like the 
'buf' pointer is incremented.  That seems like a bug no?


Yeah, looks suspicious to me as well. But TTM questions should CC the 
dri list as well.


And in this particular case I think Felix was the author that function.



Hi Christian,

I'm authoring a set of mostly NFC patches for TTM and already crafted a 
fix for this in the process.


If the fix (which is literally "buf += bytes" at the end of the while 
loop) doesn't get NAK'ed I'll submit it as part of my cleanup patches.


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

Re: Hardware 3D acceleration doesn't work anymore with the latest git kernel

[Tom St Denis]

On 27/11/17 07:02 AM, Michel Dänzer wrote:

On 2017-11-27 12:50 PM, Christian König wrote:

Am 27.11.2017 um 12:02 schrieb Michel Dänzer:

On 2017-11-24 05:09 PM, Michel Dänzer wrote:

On 2017-11-24 03:29 PM, Christian Zigotzky wrote:

Hi All,

I bisected today and the first bad commit is:
a4dec819c8bba6365eb893a4ca88db4dd1210110 (drm/ttm: Add helper functions
to populate/map in one call (v2)) [1]

It can't really be that commit, since it just adds functions but doesn't
hook them up anywhere. Presumably it's commit
f7871fd19389c5f64f625a4389675d0740f0dfe4 ("drm/radeon: use new TTM
populate/dma map helper functions") instead, which makes the radeon
driver rely on ttm_populate_and_map_pages, which is just a stub
returning -ENOMEM when neither CONFIG_SWIOTLB nor CONFIG_INTEL_IOMMU is
enabled.

I can see two possible solutions:

1. Make ttm_populate_and_map_pages and ttm_unmap_and_unpopulate_pages
     work without SWIOTLB / INTEL_IOMMU as well.

2. Make the drivers work without ttm_populate_and_map_pages and
     ttm_unmap_and_unpopulate_pages again in that case.


Solution 1 would be preferable.

Which solution do you want to go for?


well, can somebody please explain to me why those patches actually
result in problems?


I thought I did above...

Commit f7871fd19389c5f64f625a4389675d0740f0dfe4 made the radeon driver
rely on ttm_populate_and_map_pages, which is implemented as:

static inline int ttm_populate_and_map_pages(struct device *dev, struct 
ttm_dma_tt *tt)
{
return -ENOMEM;
}

when neither CONFIG_SWIOTLB nor CONFIG_INTEL_IOMMU is enabled.
Previously, the driver worked fine without either of those enabled.




I think the issue is why would you have both disabled and we did run 
into a kbuild error when they weren't guarded (I don't remember exactly 
what though I'd have to go grep for it).


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

Re: [radeon-alex:upstream-4.14-drm-next-amd-dc-staging-chrome 4/16] drivers/gpu//drm/ttm/ttm_page_alloc.c:923:5: error: redefinition of 'ttm_populate_and_map_pages'

[Tom St Denis]

Is this:

commit 7a9667ae197460e6c9c3bb432fe68c708fce6259
Refs: v4.13-rc5-1195-g7a9667ae1974
Author: Tom St Denis <tom.stde...@amd.com>
AuthorDate: Tue Sep 5 07:30:59 2017 -0400
Commit: Alex Deucher <alexander.deuc...@amd.com>
CommitDate: Tue Sep 12 14:22:55 2017 -0400

drm/ttm: Fix configuration error around populate_and_map() functions

Fixed kbuild errors when IOMMU/SWIOTLB are disabled.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>
Signed-off-by: Alex Deucher <alexander.deuc...@amd.com>

Not part of this series because we literally went through this before :-)

Tom


On 13/11/17 05:25 PM, kbuild test robot wrote:

tree:   git://people.freedesktop.org/~agd5f/linux.git 
upstream-4.14-drm-next-amd-dc-staging-chrome
head:   4448b9a68413462529d018050cd246bc33957bd6
commit: ed285b98008b667978d7faf348a22000b8a1c6b9 [4/16] drm/ttm: Add helper 
functions to populate/map in one call (v2)
config: i386-randconfig-s0-201746 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
 git checkout ed285b98008b667978d7faf348a22000b8a1c6b9
 # save the attached .config to linux build tree
 make ARCH=i386

All errors (new ones prefixed by >>):


drivers/gpu//drm/ttm/ttm_page_alloc.c:923:5: error: redefinition of 
'ttm_populate_and_map_pages'

 int ttm_populate_and_map_pages(struct device *dev, struct ttm_dma_tt *tt)
 ^~
In file included from drivers/gpu//drm/ttm/ttm_page_alloc.c:49:0:
include/drm/ttm/ttm_page_alloc.h:120:19: note: previous definition of 
'ttm_populate_and_map_pages' was here
 static inline int ttm_populate_and_map_pages(struct device *dev, struct 
ttm_dma_tt *tt)
   ^~

drivers/gpu//drm/ttm/ttm_page_alloc.c:950:6: error: redefinition of 
'ttm_unmap_and_unpopulate_pages'

 void ttm_unmap_and_unpopulate_pages(struct device *dev, struct ttm_dma_tt 
*tt)
  ^~
In file included from drivers/gpu//drm/ttm/ttm_page_alloc.c:49:0:
include/drm/ttm/ttm_page_alloc.h:125:20: note: previous definition of 
'ttm_unmap_and_unpopulate_pages' was here
 static inline void ttm_unmap_and_unpopulate_pages(struct device *dev, 
struct ttm_dma_tt *tt)
^~

vim +/ttm_populate_and_map_pages +923 drivers/gpu//drm/ttm/ttm_page_alloc.c

922 
  > 923  int ttm_populate_and_map_pages(struct device *dev, struct 
ttm_dma_tt *tt)
924 {
925 unsigned i;
926 int r;
927 
928 r = ttm_pool_populate(>ttm);
929 if (r)
930 return r;
931 
932 for (i = 0; i < tt->ttm.num_pages; i++) {
933 tt->dma_address[i] = dma_map_page(dev, tt->ttm.pages[i],
934   0, PAGE_SIZE,
935   DMA_BIDIRECTIONAL);
936 if (dma_mapping_error(dev, tt->dma_address[i])) {
937 while (i--) {
938 dma_unmap_page(dev, tt->dma_address[i],
939PAGE_SIZE, 
DMA_BIDIRECTIONAL);
940 tt->dma_address[i] = 0;
941 }
942 ttm_pool_unpopulate(>ttm);
943 return -EFAULT;
944 }
945 }
946 return 0;
947 }
948 EXPORT_SYMBOL(ttm_populate_and_map_pages);
949 
  > 950  void ttm_unmap_and_unpopulate_pages(struct device *dev, struct 
ttm_dma_tt *tt)
951 {
952 unsigned i;
953 
954 for (i = 0; i < tt->ttm.num_pages; i++) {
955 if (tt->dma_address[i]) {
956 dma_unmap_page(dev, tt->dma_address[i],
957PAGE_SIZE, DMA_BIDIRECTIONAL);
958 }
959 }
960 ttm_pool_unpopulate(>ttm);
961 }
962 EXPORT_SYMBOL(ttm_unmap_and_unpopulate_pages);
963 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation



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

[PATCH] drm/ttm: Fix unused variables with huge page support

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index b6f16e73..95022473704b 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -723,7 +723,9 @@ static void ttm_put_pages(struct page **pages, unsigned 
npages, int flags,
  enum ttm_caching_state cstate)
 {
struct ttm_page_pool *pool = ttm_get_pool(flags, false, cstate);
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
struct ttm_page_pool *huge = ttm_get_pool(flags, true, cstate);
+#endif
unsigned long irq_flags;
unsigned i;
 
@@ -825,7 +827,9 @@ static int ttm_get_pages(struct page **pages, unsigned 
npages, int flags,
 enum ttm_caching_state cstate)
 {
struct ttm_page_pool *pool = ttm_get_pool(flags, false, cstate);
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
struct ttm_page_pool *huge = ttm_get_pool(flags, true, cstate);
+#endif
struct list_head plist;
struct page *p = NULL;
unsigned count;
@@ -834,7 +838,10 @@ static int ttm_get_pages(struct page **pages, unsigned 
npages, int flags,
/* No pool for cached pages */
if (pool == NULL) {
gfp_t gfp_flags = GFP_USER;
-   unsigned i, j;
+   unsigned i;
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+   unsigned j;
+#endif
 
/* set zero flag for page allocation if required */
if (flags & TTM_PAGE_FLAG_ZERO_ALLOC)
-- 
2.12.0

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

Re: [PATCH 4/4] drm/ttm: Remove TTM dma tracepoint since it's not required anymore

[Tom St Denis]

On 19/09/17 07:13 AM, Christian König wrote:

Am 18.09.2017 um 19:33 schrieb Tom St Denis:

Signed-off-by: Tom St Denis <tom.stde...@amd.com>


Mhm, I sometimes have good use for those. But just adding a printk at 
the right place does the job as well.


So patch is Reviewed-by: Christian König <christian.koe...@amd.com>.


Well if you want to keep them we should not apply patch #3 then since 
we're the only users of it :-)


I'm ok with dropping #3/#4 if you want (also less work for Alex since we 
won't have to prune that history out of the branch we submit upstream).


umr has already been ported over to the new iova file though so it won't 
be using the trace.


Cheers,
Tom




Regards,
Christian.


---
  drivers/gpu/drm/ttm/Makefile  |  2 +-
  drivers/gpu/drm/ttm/ttm_debug.c   | 74 
-
  drivers/gpu/drm/ttm/ttm_trace.h   | 87 
---

  drivers/gpu/drm/ttm/ttm_tracepoints.c | 45 --
  4 files changed, 1 insertion(+), 207 deletions(-)
  delete mode 100644 drivers/gpu/drm/ttm/ttm_debug.c
  delete mode 100644 drivers/gpu/drm/ttm/ttm_trace.h
  delete mode 100644 drivers/gpu/drm/ttm/ttm_tracepoints.c

diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile
index ab2bef1219e5..4d0c938ff4b2 100644
--- a/drivers/gpu/drm/ttm/Makefile
+++ b/drivers/gpu/drm/ttm/Makefile
@@ -4,7 +4,7 @@
  ttm-y := ttm_memory.o ttm_tt.o ttm_bo.o \
  ttm_bo_util.o ttm_bo_vm.o ttm_module.o \
  ttm_object.o ttm_lock.o ttm_execbuf_util.o ttm_page_alloc.o \
-    ttm_bo_manager.o ttm_page_alloc_dma.o ttm_debug.o ttm_tracepoints.o
+    ttm_bo_manager.o ttm_page_alloc_dma.o
  ttm-$(CONFIG_AGP) += ttm_agp_backend.o
  obj-$(CONFIG_DRM_TTM) += ttm.o
diff --git a/drivers/gpu/drm/ttm/ttm_debug.c 
b/drivers/gpu/drm/ttm/ttm_debug.c

deleted file mode 100644
index ef5f0d090154..
--- a/drivers/gpu/drm/ttm/ttm_debug.c
+++ /dev/null
@@ -1,74 +0,0 @@
-/** 


- *
- * Copyright (c) 2017 Advanced Micro Devices, Inc.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person 
obtaining a

- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial 
portions

- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT 
SHALL
- * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR 
ANY CLAIM,

- * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
- * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE 
OR THE

- * USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- 
**********/ 


-/*
- * Authors: Tom St Denis <tom.stde...@amd.com>
- */
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include "ttm_trace.h"
-
-void ttm_trace_dma_map(struct device *dev, struct ttm_dma_tt *tt)
-{
-    unsigned i;
-
-    if (unlikely(trace_ttm_dma_map_enabled())) {
-    for (i = 0; i < tt->ttm.num_pages; i++) {
-    trace_ttm_dma_map(
-    dev,
-    tt->ttm.pages[i],
-    tt->dma_address[i]);
-    }
-    }
-}
-EXPORT_SYMBOL(ttm_trace_dma_map);
-
-void ttm_trace_dma_unmap(struct device *dev, struct ttm_dma_tt *tt)
-{
-    unsigned i;
-
-    if (unlikely(trace_ttm_dma_unmap_enabled())) {
-    for (i = 0; i < tt->ttm.num_pages; i++) {
-    trace_ttm_dma_unmap(
-    dev,
-    tt->ttm.pages[i],
-    tt->dma_address[i]);
-    }
-    }
-}
-EXPORT_SYMBOL(ttm_trace_dma_unmap);
-
diff --git a/drivers/gpu/drm/ttm/ttm_trace.h 
b/drivers/gpu/drm/ttm/ttm_trace.h

deleted file mode 100644
index 715ce68b7b33..
--- a/drivers/gpu/drm/ttm/ttm_trace.h
+++ /dev/null
@@ -1,87 +0,0 @@
-/** 


- *
- * Copyright (c) 2017 Advanced Micro Devices, Inc.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person 
obtaining a

- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Soft

remove ttm trace and add iova debugfs (v2)

[Tom St Denis]

In this respin I add some changes per feedback and make the iova
entry have proper read/write methods which access pages mapped
by amdgpu.  So there is no need for /dev/mem or /dev/fmem anymore
when reading system memory.

Patches 3/4 are unchanged and remove the TTM trace from amdgpu 
and from TTM itself.


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

[PATCH 4/4] drm/ttm: Remove TTM dma tracepoint since it's not required anymore

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/Makefile  |  2 +-
 drivers/gpu/drm/ttm/ttm_debug.c   | 74 -
 drivers/gpu/drm/ttm/ttm_trace.h   | 87 ---
 drivers/gpu/drm/ttm/ttm_tracepoints.c | 45 --
 4 files changed, 1 insertion(+), 207 deletions(-)
 delete mode 100644 drivers/gpu/drm/ttm/ttm_debug.c
 delete mode 100644 drivers/gpu/drm/ttm/ttm_trace.h
 delete mode 100644 drivers/gpu/drm/ttm/ttm_tracepoints.c

diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile
index ab2bef1219e5..4d0c938ff4b2 100644
--- a/drivers/gpu/drm/ttm/Makefile
+++ b/drivers/gpu/drm/ttm/Makefile
@@ -4,7 +4,7 @@
 ttm-y := ttm_memory.o ttm_tt.o ttm_bo.o \
ttm_bo_util.o ttm_bo_vm.o ttm_module.o \
ttm_object.o ttm_lock.o ttm_execbuf_util.o ttm_page_alloc.o \
-   ttm_bo_manager.o ttm_page_alloc_dma.o ttm_debug.o ttm_tracepoints.o
+   ttm_bo_manager.o ttm_page_alloc_dma.o
 ttm-$(CONFIG_AGP) += ttm_agp_backend.o
 
 obj-$(CONFIG_DRM_TTM) += ttm.o
diff --git a/drivers/gpu/drm/ttm/ttm_debug.c b/drivers/gpu/drm/ttm/ttm_debug.c
deleted file mode 100644
index ef5f0d090154..
--- a/drivers/gpu/drm/ttm/ttm_debug.c
+++ /dev/null
@@ -1,74 +0,0 @@
-/**
- *
- * Copyright (c) 2017 Advanced Micro Devices, Inc.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
- * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
- * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
- * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
- * USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- **********/
-/*
- * Authors: Tom St Denis <tom.stde...@amd.com>
- */
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include "ttm_trace.h"
-
-void ttm_trace_dma_map(struct device *dev, struct ttm_dma_tt *tt)
-{
-   unsigned i;
-
-   if (unlikely(trace_ttm_dma_map_enabled())) {
-   for (i = 0; i < tt->ttm.num_pages; i++) {
-   trace_ttm_dma_map(
-   dev,
-   tt->ttm.pages[i],
-   tt->dma_address[i]);
-   }
-   }
-}
-EXPORT_SYMBOL(ttm_trace_dma_map);
-
-void ttm_trace_dma_unmap(struct device *dev, struct ttm_dma_tt *tt)
-{
-   unsigned i;
-
-   if (unlikely(trace_ttm_dma_unmap_enabled())) {
-   for (i = 0; i < tt->ttm.num_pages; i++) {
-   trace_ttm_dma_unmap(
-   dev,
-   tt->ttm.pages[i],
-   tt->dma_address[i]);
-   }
-   }
-}
-EXPORT_SYMBOL(ttm_trace_dma_unmap);
-
diff --git a/drivers/gpu/drm/ttm/ttm_trace.h b/drivers/gpu/drm/ttm/ttm_trace.h
deleted file mode 100644
index 715ce68b7b33..
--- a/drivers/gpu/drm/ttm/ttm_trace.h
+++ /dev/null
@@ -1,87 +0,0 @@
-/**
- *
- * Copyright (c) 2017 Advanced Micro Devices, Inc.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 

[PATCH 1/4] drm/amd/amdgpu: Fold TTM debugfs entries into array (v2)

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>

(v2): add domains and avoid strcmp
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 54 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 +--
 2 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 8ee16dfdb8af..50d20903de4f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1809,6 +1809,19 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {
 
 #endif
 
+
+
+static const struct {
+   char *name;
+   const struct file_operations *fops;
+   int domain;
+} ttm_debugfs_entries[] = {
+   { "amdgpu_vram", _ttm_vram_fops, TTM_PL_VRAM },
+#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
+   { "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT },
+#endif
+};
+
 #endif
 
 static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
@@ -1819,22 +1832,21 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device 
*adev)
struct drm_minor *minor = adev->ddev->primary;
struct dentry *ent, *root = minor->debugfs_root;
 
-   ent = debugfs_create_file("amdgpu_vram", S_IFREG | S_IRUGO, root,
- adev, _ttm_vram_fops);
-   if (IS_ERR(ent))
-   return PTR_ERR(ent);
-   i_size_write(ent->d_inode, adev->mc.mc_vram_size);
-   adev->mman.vram = ent;
-
-#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
-   ent = debugfs_create_file("amdgpu_gtt", S_IFREG | S_IRUGO, root,
- adev, _ttm_gtt_fops);
-   if (IS_ERR(ent))
-   return PTR_ERR(ent);
-   i_size_write(ent->d_inode, adev->mc.gart_size);
-   adev->mman.gtt = ent;
+   for (count = 0; count < ARRAY_SIZE(ttm_debugfs_entries); count++) {
+   ent = debugfs_create_file(
+   ttm_debugfs_entries[count].name,
+   S_IFREG | S_IRUGO, root,
+   adev,
+   ttm_debugfs_entries[count].fops);
+   if (IS_ERR(ent))
+   return PTR_ERR(ent);
+   if (ttm_debugfs_entries[count].domain == TTM_PL_VRAM)
+   i_size_write(ent->d_inode, adev->mc.mc_vram_size);
+   else if (ttm_debugfs_entries[count].domain == TTM_PL_TT)
+   i_size_write(ent->d_inode, adev->mc.gart_size);
+   adev->mman.debugfs_entries[count] = ent;
+   }
 
-#endif
count = ARRAY_SIZE(amdgpu_ttm_debugfs_list);
 
 #ifdef CONFIG_SWIOTLB
@@ -1844,7 +1856,6 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device 
*adev)
 
return amdgpu_debugfs_add_files(adev, amdgpu_ttm_debugfs_list, count);
 #else
-
return 0;
 #endif
 }
@@ -1852,14 +1863,9 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device 
*adev)
 static void amdgpu_ttm_debugfs_fini(struct amdgpu_device *adev)
 {
 #if defined(CONFIG_DEBUG_FS)
+   unsigned i;
 
-   debugfs_remove(adev->mman.vram);
-   adev->mman.vram = NULL;
-
-#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
-   debugfs_remove(adev->mman.gtt);
-   adev->mman.gtt = NULL;
-#endif
-
+   for (i = 0; i < ARRAY_SIZE(ttm_debugfs_entries); i++)
+   debugfs_remove(adev->mman.debugfs_entries[i]);
 #endif
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 64709e041d5b..7abae6867339 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -24,6 +24,7 @@
 #ifndef __AMDGPU_TTM_H__
 #define __AMDGPU_TTM_H__
 
+#include "amdgpu.h"
 #include "gpu_scheduler.h"
 
 #define AMDGPU_PL_GDS  (TTM_PL_PRIV + 0)
@@ -45,8 +46,7 @@ struct amdgpu_mman {
boolinitialized;
 
 #if defined(CONFIG_DEBUG_FS)
-   struct dentry   *vram;
-   struct dentry   *gtt;
+   struct dentry   *debugfs_entries[8];
 #endif
 
/* buffer handling */
-- 
2.12.0

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

[PATCH 3/4] drm/amd/amdgpu: remove usage of ttm trace

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 36 +++--
 1 file changed, 3 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 02ae32378e1c..b41d03226c26 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -34,7 +34,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -704,22 +703,6 @@ void amdgpu_ttm_tt_mark_user_pages(struct ttm_tt *ttm)
}
 }
 
-static void amdgpu_trace_dma_map(struct ttm_tt *ttm)
-{
-   struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
-   struct amdgpu_ttm_tt *gtt = (void *)ttm;
-
-   ttm_trace_dma_map(adev->dev, >ttm);
-}
-
-static void amdgpu_trace_dma_unmap(struct ttm_tt *ttm)
-{
-   struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
-   struct amdgpu_ttm_tt *gtt = (void *)ttm;
-
-   ttm_trace_dma_unmap(adev->dev, >ttm);
-}
-
 /* prepare the sg table with the user pages */
 static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm)
 {
@@ -746,8 +729,6 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm)
drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
 gtt->ttm.dma_address, ttm->num_pages);
 
-   amdgpu_trace_dma_map(ttm);
-
return 0;
 
 release_sg:
@@ -773,8 +754,6 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_tt *ttm)
 
amdgpu_ttm_tt_mark_user_pages(ttm);
 
-   amdgpu_trace_dma_unmap(ttm);
-
sg_free_table(ttm->sg);
 }
 
@@ -958,7 +937,6 @@ static int amdgpu_ttm_tt_populate(struct ttm_tt *ttm)
 {
struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
struct amdgpu_ttm_tt *gtt = (void *)ttm;
-   int r;
bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
 
if (ttm->state != tt_unpopulated)
@@ -978,22 +956,16 @@ static int amdgpu_ttm_tt_populate(struct ttm_tt *ttm)
drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
 gtt->ttm.dma_address, 
ttm->num_pages);
ttm->state = tt_unbound;
-   r = 0;
-   goto trace_mappings;
+   return 0;
}
 
 #ifdef CONFIG_SWIOTLB
if (swiotlb_nr_tbl()) {
-   r = ttm_dma_populate(>ttm, adev->dev);
-   goto trace_mappings;
+   return ttm_dma_populate(>ttm, adev->dev);
}
 #endif
 
-   r = ttm_populate_and_map_pages(adev->dev, >ttm);
-trace_mappings:
-   if (likely(!r))
-   amdgpu_trace_dma_map(ttm);
-   return r;
+   return ttm_populate_and_map_pages(adev->dev, >ttm);
 }
 
 static void amdgpu_ttm_tt_unpopulate(struct ttm_tt *ttm)
@@ -1014,8 +986,6 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_tt *ttm)
 
adev = amdgpu_ttm_adev(ttm->bdev);
 
-   amdgpu_trace_dma_unmap(ttm);
-
 #ifdef CONFIG_SWIOTLB
if (swiotlb_nr_tbl()) {
ttm_dma_unpopulate(>ttm, adev->dev);
-- 
2.12.0

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

[PATCH 2/4] drm/amd/amdgpu: add support for iova_to_phys to replace TTM trace (v3)

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>

(v2): Add domain to iova debugfs
(v3): Add true read/write methods to access system memory of pages
  mapped to the device
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 104 
 1 file changed, 104 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 50d20903de4f..02ae32378e1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -43,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "amdgpu.h"
 #include "amdgpu_trace.h"
 #include "bif/bif_4_1_d.h"
@@ -1810,6 +1811,108 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {
 #endif
 
 
+static void *transform_page(uint64_t phys)
+{
+   if (PageHighMem(pfn_to_page(PFN_DOWN(phys
+   return kmap(pfn_to_page(PFN_DOWN(phys)));
+   else
+   return __va(phys);
+}
+
+static void untransform_page(uint64_t phys)
+{
+   if (PageHighMem(pfn_to_page(PFN_DOWN(phys
+   return kunmap(pfn_to_page(PFN_DOWN(phys)));
+}
+
+static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,
+  size_t size, loff_t *pos)
+{
+   struct amdgpu_device *adev = file_inode(f)->i_private;
+   ssize_t result, n;
+   int r;
+   uint64_t phys;
+   void *ptr;
+
+   result = 0;
+   while (size) {
+   // get physical address and map
+   phys = iommu_iova_to_phys(iommu_get_domain_for_dev(adev->dev), 
*pos);
+
+   // copy upto one page
+   if (size > PAGE_SIZE)
+   n = PAGE_SIZE;
+   else
+   n = size;
+
+   // to end of the page
+   if (((*pos & (PAGE_SIZE - 1)) + n) >= PAGE_SIZE)
+   n = PAGE_SIZE - (*pos & (PAGE_SIZE - 1));
+
+   ptr = transform_page(phys);
+   if (!ptr)
+   return -EFAULT;
+
+   r = copy_to_user(buf, ptr, n);
+   untransform_page(phys);
+   if (r)
+   return -EFAULT;
+
+   *pos += n;
+   size -= n;
+   result += n;
+   }
+
+   return result;
+}
+
+static ssize_t amdgpu_iova_to_phys_write(struct file *f, const char __user 
*buf,
+  size_t size, loff_t *pos)
+{
+   struct amdgpu_device *adev = file_inode(f)->i_private;
+   ssize_t result, n;
+   int r;
+   uint64_t phys;
+   void *ptr;
+
+   result = 0;
+   while (size) {
+   // get physical address and map
+   phys = iommu_iova_to_phys(iommu_get_domain_for_dev(adev->dev), 
*pos);
+
+   // copy upto one page
+   if (size > PAGE_SIZE)
+   n = PAGE_SIZE;
+   else
+   n = size;
+
+   // to end of the page
+   if (((*pos & (PAGE_SIZE - 1)) + n) >= PAGE_SIZE)
+   n = PAGE_SIZE - (*pos & (PAGE_SIZE - 1));
+
+   ptr = transform_page(phys);
+   if (!ptr)
+   return -EFAULT;
+
+   r = copy_from_user(ptr, buf, n);
+   untransform_page(phys);
+   if (r)
+   return -EFAULT;
+
+   *pos += n;
+   size -= n;
+   result += n;
+   }
+
+   return result;
+}
+
+static const struct file_operations amdgpu_ttm_iova_fops = {
+   .owner = THIS_MODULE,
+   .read = amdgpu_iova_to_phys_read,
+   .write = amdgpu_iova_to_phys_write,
+   .llseek = default_llseek
+};
 
 static const struct {
char *name;
@@ -1820,6 +1923,7 @@ static const struct {
 #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
{ "amdgpu_gtt", _ttm_gtt_fops, TTM_PL_TT },
 #endif
+   { "amdgpu_iova", _ttm_iova_fops, TTM_PL_SYSTEM },
 };
 
 #endif
-- 
2.12.0

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

Re: [PATCH 1/4] drm/amd/amdgpu: Fold TTM debugfs entries into array

[Tom St Denis]

On 18/09/17 08:48 AM, Christian König wrote:

Am 18.09.2017 um 14:35 schrieb Tom St Denis:

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 53 
++---

  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 +--
  2 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c

index 8ee16dfdb8af..7848ffa99eb4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1809,6 +1809,18 @@ static const struct file_operations 
amdgpu_ttm_gtt_fops = {

  #endif
+
+
+static const struct {
+    char *name;
+    const struct file_operations *fops;
+} ttm_debugfs_entries[] = {
+    { "amdgpu_vram", _ttm_vram_fops },
+#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
+    { "amdgpu_gtt", _ttm_gtt_fops },
+#endif
+};
+
  #endif
  static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
@@ -1819,22 +1831,21 @@ static int amdgpu_ttm_debugfs_init(struct 
amdgpu_device *adev)

  struct drm_minor *minor = adev->ddev->primary;
  struct dentry *ent, *root = minor->debugfs_root;
-    ent = debugfs_create_file("amdgpu_vram", S_IFREG | S_IRUGO, root,
-  adev, _ttm_vram_fops);
-    if (IS_ERR(ent))
-    return PTR_ERR(ent);
-    i_size_write(ent->d_inode, adev->mc.mc_vram_size);
-    adev->mman.vram = ent;
-
-#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
-    ent = debugfs_create_file("amdgpu_gtt", S_IFREG | S_IRUGO, root,
-  adev, _ttm_gtt_fops);
-    if (IS_ERR(ent))
-    return PTR_ERR(ent);
-    i_size_write(ent->d_inode, adev->mc.gart_size);
-    adev->mman.gtt = ent;
+    for (count = 0; count < ARRAY_SIZE(ttm_debugfs_entries); count++) {
+    ent = debugfs_create_file(
+    ttm_debugfs_entries[count].name,
+    S_IFREG | S_IRUGO, root,
+    adev,
+    ttm_debugfs_entries[count].fops);
+    if (IS_ERR(ent))
+    return PTR_ERR(ent);
+    if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_vram"))
+    i_size_write(ent->d_inode, adev->mc.mc_vram_size);
+    else if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_gtt"))
+    i_size_write(ent->d_inode, adev->mc.gart_size);


Uff, string compare? That is screaming break me by typo.

Maybe but the domain type into the struct as well?

Apart from that looks good to me,



Sure, a quick grep didn't turn up any defines/enums for VRAM vs GTT 
though so just make some up?


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

Re: [PATCH 2/4] drm/amd/amdgpu: add support for iova_to_phys to replace TTM trace

[Tom St Denis]

On 18/09/17 08:52 AM, Christian König wrote:

Am 18.09.2017 um 14:35 schrieb Tom St Denis:

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 32 


  1 file changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c

index 7848ffa99eb4..b4c298925e2a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -43,6 +43,7 @@
  #include 
  #include 
  #include 
+#include 
  #include "amdgpu.h"
  #include "amdgpu_trace.h"
  #include "bif/bif_4_1_d.h"
@@ -1810,6 +1811,36 @@ static const struct file_operations 
amdgpu_ttm_gtt_fops = {

  #endif
+static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user 
*buf,

+   size_t size, loff_t *pos)
+{
+    struct amdgpu_device *adev = file_inode(f)->i_private;
+    int r;
+    uint64_t phys;
+
+    // always return 8 bytes
+    if (size != 8)
+    return -EINVAL;
+
+    // only accept page addresses
+    if (*pos & 0xFFF)
+    return -EINVAL;
+
+
+    phys = iommu_iova_to_phys(iommu_get_domain_for_dev(adev->dev), 
*pos);


Well how about adding directly read/write support for the page behind 
the address?


This way we won't need to fiddle with /dev/mem any more either.



Given the flak we got from requesting this from the iommu team I'm 
worried that might not be appreciated by the community (even though we 
maintain this part of the tree) and hurt our abilities to upstream.


I agree that adding a read/write method would be better though since we 
don't need config changes of /dev/fmem anymore.


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

[PATCH 4/4] drm/ttm: Remove TTM dma tracepoint since it's not required anymore

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/Makefile  |  2 +-
 drivers/gpu/drm/ttm/ttm_debug.c   | 74 -
 drivers/gpu/drm/ttm/ttm_trace.h   | 87 ---
 drivers/gpu/drm/ttm/ttm_tracepoints.c | 45 --
 4 files changed, 1 insertion(+), 207 deletions(-)
 delete mode 100644 drivers/gpu/drm/ttm/ttm_debug.c
 delete mode 100644 drivers/gpu/drm/ttm/ttm_trace.h
 delete mode 100644 drivers/gpu/drm/ttm/ttm_tracepoints.c

diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile
index ab2bef1219e5..4d0c938ff4b2 100644
--- a/drivers/gpu/drm/ttm/Makefile
+++ b/drivers/gpu/drm/ttm/Makefile
@@ -4,7 +4,7 @@
 ttm-y := ttm_memory.o ttm_tt.o ttm_bo.o \
ttm_bo_util.o ttm_bo_vm.o ttm_module.o \
ttm_object.o ttm_lock.o ttm_execbuf_util.o ttm_page_alloc.o \
-   ttm_bo_manager.o ttm_page_alloc_dma.o ttm_debug.o ttm_tracepoints.o
+   ttm_bo_manager.o ttm_page_alloc_dma.o
 ttm-$(CONFIG_AGP) += ttm_agp_backend.o
 
 obj-$(CONFIG_DRM_TTM) += ttm.o
diff --git a/drivers/gpu/drm/ttm/ttm_debug.c b/drivers/gpu/drm/ttm/ttm_debug.c
deleted file mode 100644
index ef5f0d090154..
--- a/drivers/gpu/drm/ttm/ttm_debug.c
+++ /dev/null
@@ -1,74 +0,0 @@
-/**
- *
- * Copyright (c) 2017 Advanced Micro Devices, Inc.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
- * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
- * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
- * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
- * USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- **********/
-/*
- * Authors: Tom St Denis <tom.stde...@amd.com>
- */
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include "ttm_trace.h"
-
-void ttm_trace_dma_map(struct device *dev, struct ttm_dma_tt *tt)
-{
-   unsigned i;
-
-   if (unlikely(trace_ttm_dma_map_enabled())) {
-   for (i = 0; i < tt->ttm.num_pages; i++) {
-   trace_ttm_dma_map(
-   dev,
-   tt->ttm.pages[i],
-   tt->dma_address[i]);
-   }
-   }
-}
-EXPORT_SYMBOL(ttm_trace_dma_map);
-
-void ttm_trace_dma_unmap(struct device *dev, struct ttm_dma_tt *tt)
-{
-   unsigned i;
-
-   if (unlikely(trace_ttm_dma_unmap_enabled())) {
-   for (i = 0; i < tt->ttm.num_pages; i++) {
-   trace_ttm_dma_unmap(
-   dev,
-   tt->ttm.pages[i],
-   tt->dma_address[i]);
-   }
-   }
-}
-EXPORT_SYMBOL(ttm_trace_dma_unmap);
-
diff --git a/drivers/gpu/drm/ttm/ttm_trace.h b/drivers/gpu/drm/ttm/ttm_trace.h
deleted file mode 100644
index 715ce68b7b33..
--- a/drivers/gpu/drm/ttm/ttm_trace.h
+++ /dev/null
@@ -1,87 +0,0 @@
-/**
- *
- * Copyright (c) 2017 Advanced Micro Devices, Inc.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 

[PATCH 2/4] drm/amd/amdgpu: add support for iova_to_phys to replace TTM trace

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 7848ffa99eb4..b4c298925e2a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -43,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "amdgpu.h"
 #include "amdgpu_trace.h"
 #include "bif/bif_4_1_d.h"
@@ -1810,6 +1811,36 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {
 #endif
 
 
+static ssize_t amdgpu_iova_to_phys_read(struct file *f, char __user *buf,
+  size_t size, loff_t *pos)
+{
+   struct amdgpu_device *adev = file_inode(f)->i_private;
+   int r;
+   uint64_t phys;
+
+   // always return 8 bytes 
+   if (size != 8)
+   return -EINVAL;
+
+   // only accept page addresses
+   if (*pos & 0xFFF)
+   return -EINVAL;
+
+
+   phys = iommu_iova_to_phys(iommu_get_domain_for_dev(adev->dev), *pos);
+
+   r = copy_to_user(buf, , 8);
+   if (r)
+   return -EFAULT;
+
+   return 8;
+}
+
+static const struct file_operations amdgpu_ttm_iova_fops = {
+   .owner = THIS_MODULE,
+   .read = amdgpu_iova_to_phys_read,
+   .llseek = default_llseek
+};
 
 static const struct {
char *name;
@@ -1819,6 +1850,7 @@ static const struct {
 #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
{ "amdgpu_gtt", _ttm_gtt_fops },
 #endif
+   { "amdgpu_iova", _ttm_iova_fops },
 };
 
 #endif
-- 
2.12.0

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

[PATCH 1/4] drm/amd/amdgpu: Fold TTM debugfs entries into array

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 53 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 +--
 2 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 8ee16dfdb8af..7848ffa99eb4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1809,6 +1809,18 @@ static const struct file_operations amdgpu_ttm_gtt_fops 
= {
 
 #endif
 
+
+
+static const struct {
+   char *name;
+   const struct file_operations *fops;
+} ttm_debugfs_entries[] = {
+   { "amdgpu_vram", _ttm_vram_fops },
+#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
+   { "amdgpu_gtt", _ttm_gtt_fops },
+#endif
+};
+
 #endif
 
 static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
@@ -1819,22 +1831,21 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device 
*adev)
struct drm_minor *minor = adev->ddev->primary;
struct dentry *ent, *root = minor->debugfs_root;
 
-   ent = debugfs_create_file("amdgpu_vram", S_IFREG | S_IRUGO, root,
- adev, _ttm_vram_fops);
-   if (IS_ERR(ent))
-   return PTR_ERR(ent);
-   i_size_write(ent->d_inode, adev->mc.mc_vram_size);
-   adev->mman.vram = ent;
-
-#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
-   ent = debugfs_create_file("amdgpu_gtt", S_IFREG | S_IRUGO, root,
- adev, _ttm_gtt_fops);
-   if (IS_ERR(ent))
-   return PTR_ERR(ent);
-   i_size_write(ent->d_inode, adev->mc.gart_size);
-   adev->mman.gtt = ent;
+   for (count = 0; count < ARRAY_SIZE(ttm_debugfs_entries); count++) {
+   ent = debugfs_create_file(
+   ttm_debugfs_entries[count].name,
+   S_IFREG | S_IRUGO, root,
+   adev,
+   ttm_debugfs_entries[count].fops);
+   if (IS_ERR(ent))
+   return PTR_ERR(ent);
+   if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_vram"))
+   i_size_write(ent->d_inode, adev->mc.mc_vram_size);
+   else if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_gtt"))
+   i_size_write(ent->d_inode, adev->mc.gart_size);
+   adev->mman.debugfs_entries[count] = ent;
+   }
 
-#endif
count = ARRAY_SIZE(amdgpu_ttm_debugfs_list);
 
 #ifdef CONFIG_SWIOTLB
@@ -1844,7 +1855,6 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device 
*adev)
 
return amdgpu_debugfs_add_files(adev, amdgpu_ttm_debugfs_list, count);
 #else
-
return 0;
 #endif
 }
@@ -1852,14 +1862,9 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device 
*adev)
 static void amdgpu_ttm_debugfs_fini(struct amdgpu_device *adev)
 {
 #if defined(CONFIG_DEBUG_FS)
+   unsigned i;
 
-   debugfs_remove(adev->mman.vram);
-   adev->mman.vram = NULL;
-
-#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
-   debugfs_remove(adev->mman.gtt);
-   adev->mman.gtt = NULL;
-#endif
-
+   for (i = 0; i < ARRAY_SIZE(ttm_debugfs_entries); i++)
+   debugfs_remove(adev->mman.debugfs_entries[i]);
 #endif
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 64709e041d5b..7abae6867339 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -24,6 +24,7 @@
 #ifndef __AMDGPU_TTM_H__
 #define __AMDGPU_TTM_H__
 
+#include "amdgpu.h"
 #include "gpu_scheduler.h"
 
 #define AMDGPU_PL_GDS  (TTM_PL_PRIV + 0)
@@ -45,8 +46,7 @@ struct amdgpu_mman {
boolinitialized;
 
 #if defined(CONFIG_DEBUG_FS)
-   struct dentry   *vram;
-   struct dentry   *gtt;
+   struct dentry   *debugfs_entries[8];
 #endif
 
/* buffer handling */
-- 
2.12.0

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

[PATCH 3/4] drm/amd/amdgpu: remove usage of ttm trace

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 36 +++--
 1 file changed, 3 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index b4c298925e2a..e0c37fe4d043 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -34,7 +34,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -704,22 +703,6 @@ void amdgpu_ttm_tt_mark_user_pages(struct ttm_tt *ttm)
}
 }
 
-static void amdgpu_trace_dma_map(struct ttm_tt *ttm)
-{
-   struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
-   struct amdgpu_ttm_tt *gtt = (void *)ttm;
-
-   ttm_trace_dma_map(adev->dev, >ttm);
-}
-
-static void amdgpu_trace_dma_unmap(struct ttm_tt *ttm)
-{
-   struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
-   struct amdgpu_ttm_tt *gtt = (void *)ttm;
-
-   ttm_trace_dma_unmap(adev->dev, >ttm);
-}
-
 /* prepare the sg table with the user pages */
 static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm)
 {
@@ -746,8 +729,6 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm)
drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
 gtt->ttm.dma_address, ttm->num_pages);
 
-   amdgpu_trace_dma_map(ttm);
-
return 0;
 
 release_sg:
@@ -773,8 +754,6 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_tt *ttm)
 
amdgpu_ttm_tt_mark_user_pages(ttm);
 
-   amdgpu_trace_dma_unmap(ttm);
-
sg_free_table(ttm->sg);
 }
 
@@ -958,7 +937,6 @@ static int amdgpu_ttm_tt_populate(struct ttm_tt *ttm)
 {
struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
struct amdgpu_ttm_tt *gtt = (void *)ttm;
-   int r;
bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
 
if (ttm->state != tt_unpopulated)
@@ -978,22 +956,16 @@ static int amdgpu_ttm_tt_populate(struct ttm_tt *ttm)
drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
 gtt->ttm.dma_address, 
ttm->num_pages);
ttm->state = tt_unbound;
-   r = 0;
-   goto trace_mappings;
+   return 0;
}
 
 #ifdef CONFIG_SWIOTLB
if (swiotlb_nr_tbl()) {
-   r = ttm_dma_populate(>ttm, adev->dev);
-   goto trace_mappings;
+   return ttm_dma_populate(>ttm, adev->dev);
}
 #endif
 
-   r = ttm_populate_and_map_pages(adev->dev, >ttm);
-trace_mappings:
-   if (likely(!r))
-   amdgpu_trace_dma_map(ttm);
-   return r;
+   return ttm_populate_and_map_pages(adev->dev, >ttm);
 }
 
 static void amdgpu_ttm_tt_unpopulate(struct ttm_tt *ttm)
@@ -1014,8 +986,6 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_tt *ttm)
 
adev = amdgpu_ttm_adev(ttm->bdev);
 
-   amdgpu_trace_dma_unmap(ttm);
-
 #ifdef CONFIG_SWIOTLB
if (swiotlb_nr_tbl()) {
ttm_dma_unpopulate(>ttm, adev->dev);
-- 
2.12.0

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

Remove TTM trace and use proper IOMMU api

[Tom St Denis]

These patches tidy up the amdgpu_ttm debugfs creation, add
an iova_to_phys interface and then remove the TTM trace from both
amdgpu and drm/ttm.


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

[PATCH] drm/ttm: Fix configuration error around populate_and_map() functions

[Tom St Denis]

Fixed kbuild errors when IOMMU/SWIOTLB are disabled.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 6a660d196d87..052e1f102113 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -920,6 +920,7 @@ void ttm_pool_unpopulate(struct ttm_tt *ttm)
 }
 EXPORT_SYMBOL(ttm_pool_unpopulate);
 
+#if defined(CONFIG_SWIOTLB) || defined(CONFIG_INTEL_IOMMU)
 int ttm_populate_and_map_pages(struct device *dev, struct ttm_dma_tt *tt)
 {
unsigned i;
@@ -960,6 +961,7 @@ void ttm_unmap_and_unpopulate_pages(struct device *dev, 
struct ttm_dma_tt *tt)
ttm_pool_unpopulate(>ttm);
 }
 EXPORT_SYMBOL(ttm_unmap_and_unpopulate_pages);
+#endif
 
 int ttm_page_alloc_debugfs(struct seq_file *m, void *data)
 {
-- 
2.12.0

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

Re: [PATCH] drm/ttm: Fix trace include path (v2)

[Tom St Denis]

On 01/09/17 01:48 PM, Thierry Reding wrote:

On Fri, Sep 01, 2017 at 12:14:23PM -0400, Tom St Denis wrote:

Signed-off-by: Tom St Denis <tom.stde...@amd.com>

(v2): Drop Makefile change too.
---
  drivers/gpu/drm/ttm/Makefile| 2 +-
  drivers/gpu/drm/ttm/ttm_trace.h | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Thierry Reding <tred...@nvidia.com>



Alex when you pull this into your fdo tree can you add these (and 
Christians R-b)?  I already pushed it to our internal staging :-)


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

[PATCH 1/2] drm/amd/amdgpu: Fix TRACE include path

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
index b1f97417241d..cef1a26deb2d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -417,5 +417,5 @@ TRACE_EVENT(amdgpu_ttm_bo_move,
 
 /* This part must be outside protection */
 #undef TRACE_INCLUDE_PATH
-#define TRACE_INCLUDE_PATH .
+#define TRACE_INCLUDE_PATH ../../drivers/gpu/drm/amd/amdgpu/
 #include 
-- 
2.12.0

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

[PATCH 2/2] drm/ttm: Fix trace include path

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_trace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_trace.h b/drivers/gpu/drm/ttm/ttm_trace.h
index 23279b9b8e64..715ce68b7b33 100644
--- a/drivers/gpu/drm/ttm/ttm_trace.h
+++ b/drivers/gpu/drm/ttm/ttm_trace.h
@@ -82,6 +82,6 @@ TRACE_EVENT(ttm_dma_unmap,
 
 /* This part must be outside protection */
 #undef TRACE_INCLUDE_PATH
-#define TRACE_INCLUDE_PATH .
+#define TRACE_INCLUDE_PATH ../../drivers/gpu/drm/ttm/
 #include 
 
-- 
2.12.0

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

[PATCH] drm/ttm: Fix trace include path (v2)

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stde...@amd.com>

(v2): Drop Makefile change too.
---
 drivers/gpu/drm/ttm/Makefile| 2 +-
 drivers/gpu/drm/ttm/ttm_trace.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile
index fd3da00c0bf2..f0549eab73e6 100644
--- a/drivers/gpu/drm/ttm/Makefile
+++ b/drivers/gpu/drm/ttm/Makefile
@@ -1,7 +1,7 @@
 #
 # Makefile for the drm device driver.  This driver provides support for the
 
-ccflags-y := -Iinclude/drm -I$(src)/.
+ccflags-y := -Iinclude/drm
 ttm-y := ttm_memory.o ttm_tt.o ttm_bo.o \
ttm_bo_util.o ttm_bo_vm.o ttm_module.o \
ttm_object.o ttm_lock.o ttm_execbuf_util.o ttm_page_alloc.o \
diff --git a/drivers/gpu/drm/ttm/ttm_trace.h b/drivers/gpu/drm/ttm/ttm_trace.h
index 23279b9b8e64..715ce68b7b33 100644
--- a/drivers/gpu/drm/ttm/ttm_trace.h
+++ b/drivers/gpu/drm/ttm/ttm_trace.h
@@ -82,6 +82,6 @@ TRACE_EVENT(ttm_dma_unmap,
 
 /* This part must be outside protection */
 #undef TRACE_INCLUDE_PATH
-#define TRACE_INCLUDE_PATH .
+#define TRACE_INCLUDE_PATH ../../drivers/gpu/drm/ttm/
 #include 
 
-- 
2.12.0

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

Re: [PATCH 2/2] drm/ttm: Fix trace include path

[Tom St Denis]

On 01/09/17 12:12 PM, Deucher, Alexander wrote:

-Original Message-
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
Of Tom St Denis
Sent: Friday, September 01, 2017 12:11 PM
To: amd-...@lists.freedesktop.org
Cc: StDenis, Tom; dri-devel@lists.freedesktop.org
Subject: [PATCH 2/2] drm/ttm: Fix trace include path

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
  drivers/gpu/drm/ttm/ttm_trace.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_trace.h
b/drivers/gpu/drm/ttm/ttm_trace.h
index 23279b9b8e64..715ce68b7b33 100644
--- a/drivers/gpu/drm/ttm/ttm_trace.h
+++ b/drivers/gpu/drm/ttm/ttm_trace.h
@@ -82,6 +82,6 @@ TRACE_EVENT(ttm_dma_unmap,

  /* This part must be outside protection */
  #undef TRACE_INCLUDE_PATH
-#define TRACE_INCLUDE_PATH .
+#define TRACE_INCLUDE_PATH ../../drivers/gpu/drm/ttm/
  #include 


I think you can drop the Makefile change as well.


Yup.

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

Re: [PATCH 1/6] drm: Use correct path to trace include

[Tom St Denis]

On 01/09/17 11:02 AM, Christian König wrote:

Am 01.09.2017 um 16:49 schrieb Thierry Reding:

From: Thierry Reding 

The header comment in include/trace/define_trace.h specifies that the
TRACE_INCLUDE_PATH needs to be relative to the define_trace.h header
rather than the trace file including it. Most instances get that wrong
and work around it by adding the $(src) directory to the include path.

While this works, it is preferable to refer to the correct path to the
trace file in the first place and avoid any workaround.

Signed-off-by: Thierry Reding 


Actually I've recently wondered how to correctly do this since we send 
out a TTM patch for 4.13 which most likely gets this wrong as well.


Thanks for pointing this out, patch #2 and #5 are Reviewed-by: Christian 
König 


The rest is Acked-by: Christian König .

Tom please check our TTM patch and if necessary provide a fix as well.


Hi Christian,

I'm sure we have it wrong and since I copied the TTM trace from the 
AMDGPU one I think that's wrong too.


I'll submit the necessary patch(es) shortly.

Cheers,
Tom



Thanks,
Christian.


---
  drivers/gpu/drm/Makefile    | 2 --
  drivers/gpu/drm/drm_trace.h | 2 +-
  2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index a8acc197dec3..f82d0faad690 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -44,8 +44,6 @@ drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += 
drm_dp_aux_dev.o

  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
  obj-$(CONFIG_DRM_DEBUG_MM_SELFTEST) += selftests/
-CFLAGS_drm_trace_points.o := -I$(src)
-
  obj-$(CONFIG_DRM)    += drm.o
  obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
  obj-$(CONFIG_DRM_ARM)    += arm/
diff --git a/drivers/gpu/drm/drm_trace.h b/drivers/gpu/drm/drm_trace.h
index 14c5a777682e..16c64d067e67 100644
--- a/drivers/gpu/drm/drm_trace.h
+++ b/drivers/gpu/drm/drm_trace.h
@@ -61,5 +61,5 @@ TRACE_EVENT(drm_vblank_event_delivered,
  /* This part must be outside protection */
  #undef TRACE_INCLUDE_PATH
-#define TRACE_INCLUDE_PATH .
+#define TRACE_INCLUDE_PATH ../../drivers/gpu/drm
  #include 





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

Re: [PATCH 2/2] drm/ttm: Remove needless 'extern' on functions in header.

[Tom St Denis]

On 24/08/17 07:53 AM, Christian König wrote:

Am 24.08.2017 um 12:48 schrieb Tom St Denis:

Minor tidy up.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>


Thanks and sorry that I thought you added this, I really need more sleep.

Patch is Reviewed-by: Christian König <christian.koe...@amd.com>.


No worries.  For a second there I thought I was writing patches too 
early for me :)


Should be an AMD rule that no patches before sun up in the summer... 
Alas in Winter here in Canada that'd cut productivity down to 20% hehehe.


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

[PATCH 2/2] drm/ttm: Remove needless 'extern' on functions in header.

[Tom St Denis]

Minor tidy up.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 include/drm/ttm/ttm_page_alloc.h | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/drm/ttm/ttm_page_alloc.h b/include/drm/ttm/ttm_page_alloc.h
index 4400c08169cd..19bdd907613c 100644
--- a/include/drm/ttm/ttm_page_alloc.h
+++ b/include/drm/ttm/ttm_page_alloc.h
@@ -47,7 +47,7 @@ void ttm_page_alloc_fini(void);
  *
  * Add backing pages to all of @ttm
  */
-extern int ttm_pool_populate(struct ttm_tt *ttm);
+int ttm_pool_populate(struct ttm_tt *ttm);
 
 /**
  * ttm_pool_unpopulate:
@@ -56,12 +56,12 @@ extern int ttm_pool_populate(struct ttm_tt *ttm);
  *
  * Free all pages of @ttm
  */
-extern void ttm_pool_unpopulate(struct ttm_tt *ttm);
+void ttm_pool_unpopulate(struct ttm_tt *ttm);
 
 /**
  * Output the state of pools to debugfs file
  */
-extern int ttm_page_alloc_debugfs(struct seq_file *m, void *data);
+int ttm_page_alloc_debugfs(struct seq_file *m, void *data);
 
 
 #if defined(CONFIG_SWIOTLB) || defined(CONFIG_INTEL_IOMMU)
@@ -78,10 +78,10 @@ void ttm_dma_page_alloc_fini(void);
 /**
  * Output the state of pools to debugfs file
  */
-extern int ttm_dma_page_alloc_debugfs(struct seq_file *m, void *data);
+int ttm_dma_page_alloc_debugfs(struct seq_file *m, void *data);
 
-extern int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev);
-extern void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev);
+int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev);
+void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev);
 
 
 /**
-- 
2.12.0

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

[PATCH 1/2] drm/ttm: Add dummy *populate_and_*map_pages() functions

[Tom St Denis]

On non IOTLB/IOMMU builds these functions would be undefined.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 include/drm/ttm/ttm_page_alloc.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/include/drm/ttm/ttm_page_alloc.h b/include/drm/ttm/ttm_page_alloc.h
index 8695918ea629..4400c08169cd 100644
--- a/include/drm/ttm/ttm_page_alloc.h
+++ b/include/drm/ttm/ttm_page_alloc.h
@@ -116,6 +116,16 @@ static inline void ttm_dma_unpopulate(struct ttm_dma_tt 
*ttm_dma,
  struct device *dev)
 {
 }
+
+static inline int ttm_populate_and_map_pages(struct device *dev, struct 
ttm_dma_tt *tt)
+{
+   return 0;
+}
+
+static inline void ttm_unmap_and_unpopulate_pages(struct device *dev, struct 
ttm_dma_tt *tt)
+{
+}
+
 #endif
 
 #endif
-- 
2.12.0

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

Re: [radeon-alex:drm-next-4.14-wip 39/44] drivers/gpu/drm/radeon/radeon_ttm.c:763:2: error: implicit declaration of function 'ttm_populate_and_map_pages'

[Tom St Denis]

On 24/08/17 02:43 AM, Christian König wrote:

The problem is here:

#if defined(CONFIG_SWIOTLB) || defined(CONFIG_INTEL_IOMMU)


extern int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device 
*dev);
extern void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct 
device *dev);


We have forgotten to provide dummies for non SWIOTLB/IOMMU systems and 
xtensa doesn't seem to have this.


And BTW please drop the "extern" keyword here, that is the default for 
functions anyway.


The functions I added don't have extern.  That being said I can write 
two patches one that adds dummy functions and one that removes the 
needless externs.


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

Re: [PATCH] drm/ttm: Add helper functions to populate/map in one call

[Tom St Denis]

ping?  Haven't seen any comments on amd-gfx or dri-devel.

Cheers,
Tom


On 18/08/17 10:07 AM, Tom St Denis wrote:

These functions replace a section of common code found
in radeon/amdgpu drivers (and possibly others) as part
of the ttm_tt_*populate() callbacks.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
  drivers/gpu/drm/ttm/ttm_page_alloc.c | 41 
  include/drm/ttm/ttm_page_alloc.h | 11 ++
  2 files changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 871599826773..6a660d196d87 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -920,6 +920,47 @@ void ttm_pool_unpopulate(struct ttm_tt *ttm)
  }
  EXPORT_SYMBOL(ttm_pool_unpopulate);
  
+int ttm_populate_and_map_pages(struct device *dev, struct ttm_dma_tt *tt)

+{
+   unsigned i;
+   int r;
+
+   r = ttm_pool_populate(>ttm);
+   if (r)
+   return r;
+
+   for (i = 0; i < tt->ttm.num_pages; i++) {
+   tt->dma_address[i] = dma_map_page(dev, tt->ttm.pages[i],
+ 0, PAGE_SIZE,
+ DMA_BIDIRECTIONAL);
+   if (dma_mapping_error(dev, tt->dma_address[i])) {
+   while (i--) {
+   dma_unmap_page(dev, tt->dma_address[i],
+  PAGE_SIZE, DMA_BIDIRECTIONAL);
+   tt->dma_address[i] = 0;
+   }
+   ttm_pool_unpopulate(>ttm);
+   return -EFAULT;
+   }
+   }
+   return 0;
+}
+EXPORT_SYMBOL(ttm_populate_and_map_pages);
+
+void ttm_unmap_and_unpopulate_pages(struct device *dev, struct ttm_dma_tt *tt)
+{
+   unsigned i;
+   
+   for (i = 0; i < tt->ttm.num_pages; i++) {
+   if (tt->dma_address[i]) {
+   dma_unmap_page(dev, tt->dma_address[i],
+  PAGE_SIZE, DMA_BIDIRECTIONAL);
+   }
+   }
+   ttm_pool_unpopulate(>ttm);
+}
+EXPORT_SYMBOL(ttm_unmap_and_unpopulate_pages);
+
  int ttm_page_alloc_debugfs(struct seq_file *m, void *data)
  {
struct ttm_page_pool *p;
diff --git a/include/drm/ttm/ttm_page_alloc.h b/include/drm/ttm/ttm_page_alloc.h
index 49a828425fa2..8695918ea629 100644
--- a/include/drm/ttm/ttm_page_alloc.h
+++ b/include/drm/ttm/ttm_page_alloc.h
@@ -83,6 +83,17 @@ extern int ttm_dma_page_alloc_debugfs(struct seq_file *m, 
void *data);
  extern int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev);
  extern void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device 
*dev);
  
+

+/**
+ * Populates and DMA maps pages to fullfil a ttm_dma_populate() request
+ */
+int ttm_populate_and_map_pages(struct device *dev, struct ttm_dma_tt *tt);
+
+/**
+ * Unpopulates and DMA unmaps pages as part of a
+ * ttm_dma_unpopulate() request */
+void ttm_unmap_and_unpopulate_pages(struct device *dev, struct ttm_dma_tt *tt);
+
  #else
  static inline int ttm_dma_page_alloc_init(struct ttm_mem_global *glob,
  unsigned max_pages)



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

[PATCH 2/2] drm/amd/amdgpu: Remove AMDGPU tracepoint and use new TTM tracepoint (v2)

[Tom St Denis]

Switches the AMDGPU driver over to the TTM tracepoint and removes
our old one.  Now you can enable traces before loading the module
and trace all mappings.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>

(v2): Use struct device instead of pci in trace.
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 56 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   | 21 ++--
 2 files changed, 3 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
index 1c88bd5e29ad..b1f97417241d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -14,62 +14,6 @@
 #define AMDGPU_JOB_GET_TIMELINE_NAME(job) \
 
job->base.s_fence->finished.ops->get_timeline_name(>base.s_fence->finished)
 
-TRACE_EVENT(amdgpu_ttm_tt_populate,
-   TP_PROTO(struct amdgpu_device *adev, uint64_t dma_address, uint64_t 
phys_address),
-   TP_ARGS(adev, dma_address, phys_address),
-   TP_STRUCT__entry(
-   __field(uint16_t, domain)
-   __field(uint8_t, bus)
-   __field(uint8_t, slot)
-   __field(uint8_t, func)
-   __field(uint64_t, dma)
-   __field(uint64_t, phys)
-   ),
-   TP_fast_assign(
-  __entry->domain = pci_domain_nr(adev->pdev->bus);
-  __entry->bus = adev->pdev->bus->number;
-  __entry->slot = PCI_SLOT(adev->pdev->devfn);
-  __entry->func = PCI_FUNC(adev->pdev->devfn);
-  __entry->dma = dma_address;
-  __entry->phys = phys_address;
-  ),
-   TP_printk("%04x:%02x:%02x.%x: 0x%llx => 0x%llx",
- (unsigned)__entry->domain,
- (unsigned)__entry->bus,
- (unsigned)__entry->slot,
- (unsigned)__entry->func,
- (unsigned long long)__entry->dma,
- (unsigned long long)__entry->phys)
-);
-
-TRACE_EVENT(amdgpu_ttm_tt_unpopulate,
-   TP_PROTO(struct amdgpu_device *adev, uint64_t dma_address, uint64_t 
phys_address),
-   TP_ARGS(adev, dma_address, phys_address),
-   TP_STRUCT__entry(
-   __field(uint16_t, domain)
-   __field(uint8_t, bus)
-   __field(uint8_t, slot)
-   __field(uint8_t, func)
-   __field(uint64_t, dma)
-   __field(uint64_t, phys)
-   ),
-   TP_fast_assign(
-  __entry->domain = pci_domain_nr(adev->pdev->bus);
-  __entry->bus = adev->pdev->bus->number;
-  __entry->slot = PCI_SLOT(adev->pdev->devfn);
-  __entry->func = PCI_FUNC(adev->pdev->devfn);
-  __entry->dma = dma_address;
-  __entry->phys = phys_address;
-  ),
-   TP_printk("%04x:%02x:%02x.%x: 0x%llx => 0x%llx",
- (unsigned)__entry->domain,
- (unsigned)__entry->bus,
- (unsigned)__entry->slot,
- (unsigned)__entry->func,
- (unsigned long long)__entry->dma,
- (unsigned long long)__entry->phys)
-);
-
 TRACE_EVENT(amdgpu_mm_rreg,
TP_PROTO(unsigned did, uint32_t reg, uint32_t value),
TP_ARGS(did, reg, value),
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 26665b4baf36..38d26a7d5d0d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -34,6 +34,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -667,32 +668,16 @@ static void amdgpu_trace_dma_map(struct ttm_tt *ttm)
 {
struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
struct amdgpu_ttm_tt *gtt = (void *)ttm;
-   unsigned i;
 
-   if (unlikely(trace_amdgpu_ttm_tt_populate_enabled())) {
-   for (i = 0; i < ttm->num_pages; i++) {
-   trace_amdgpu_ttm_tt_populate(
-   adev,
-   gtt->ttm.dma_address[i],
-   page_to_phys(ttm->pages[i]));
-   }
-   }
+   ttm_trace_dma_map(adev->dev, >tt

[PATCH 1/2] drm/ttm: Add DMA map/unmap tracepoint (v3)

[Tom St Denis]

Also exports two functions that vendor drivers can call
to trace DMA mappings.  This is meant to help translate
IOMMU mappings of bus addresses back to physical pages.

Used by the umr amdgpu debugger for instance.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>

(v2): Use dev_name() to get PCI path instead.
(v3): Use correct types for dma/phys addresses
---
 drivers/gpu/drm/ttm/Makefile  |  4 +-
 drivers/gpu/drm/ttm/ttm_debug.c   | 75 ++
 drivers/gpu/drm/ttm/ttm_trace.h   | 87 +++
 drivers/gpu/drm/ttm/ttm_tracepoints.c | 46 ++
 include/drm/ttm/ttm_debug.h   | 31 +
 5 files changed, 241 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/ttm/ttm_debug.c
 create mode 100644 drivers/gpu/drm/ttm/ttm_trace.h
 create mode 100644 drivers/gpu/drm/ttm/ttm_tracepoints.c
 create mode 100644 include/drm/ttm/ttm_debug.h

diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile
index f92325800f8a..fd3da00c0bf2 100644
--- a/drivers/gpu/drm/ttm/Makefile
+++ b/drivers/gpu/drm/ttm/Makefile
@@ -1,11 +1,11 @@
 #
 # Makefile for the drm device driver.  This driver provides support for the
 
-ccflags-y := -Iinclude/drm
+ccflags-y := -Iinclude/drm -I$(src)/.
 ttm-y := ttm_memory.o ttm_tt.o ttm_bo.o \
ttm_bo_util.o ttm_bo_vm.o ttm_module.o \
ttm_object.o ttm_lock.o ttm_execbuf_util.o ttm_page_alloc.o \
-   ttm_bo_manager.o ttm_page_alloc_dma.o
+   ttm_bo_manager.o ttm_page_alloc_dma.o ttm_debug.o ttm_tracepoints.o
 ttm-$(CONFIG_AGP) += ttm_agp_backend.o
 
 obj-$(CONFIG_DRM_TTM) += ttm.o
diff --git a/drivers/gpu/drm/ttm/ttm_debug.c b/drivers/gpu/drm/ttm/ttm_debug.c
new file mode 100644
index ..dd158c6ef90d
--- /dev/null
+++ b/drivers/gpu/drm/ttm/ttm_debug.c
@@ -0,0 +1,75 @@
+/**
+ *
+ * Copyright (c) 2017 Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+ * USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ **********/
+/*
+ * Authors: Tom St Denis <tom.stde...@amd.com>
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "ttm_trace.h"
+
+void ttm_trace_dma_map(struct device *dev, struct ttm_dma_tt *tt)
+{
+   unsigned i;
+
+   if (unlikely(trace_ttm_dma_map_enabled())) {
+   for (i = 0; i < tt->ttm.num_pages; i++) {
+   trace_ttm_dma_map(
+   dev,
+   tt->ttm.pages[i],
+   tt->dma_address[i]);
+   }
+   }
+}
+EXPORT_SYMBOL(ttm_trace_dma_map);
+
+void ttm_trace_dma_unmap(struct device *dev, struct ttm_dma_tt *tt)
+{
+   unsigned i;
+
+   if (unlikely(trace_ttm_dma_unmap_enabled())) {
+   for (i = 0; i < tt->ttm.num_pages; i++) {
+   trace_ttm_dma_unmap(
+   dev,
+   tt->ttm.pages[i],
+   tt->dma_address[i]);
+   }
+   }
+}
+EXPORT_SYMBOL(ttm_trace_dma_unmap);
+
diff --git a/drivers/gpu/drm/ttm/ttm_trace.h b/drivers/gpu/drm/ttm/ttm_trace.h
new file mode 100644
index ..23279b9b8e64
--- /dev/null
+++ b/drivers/gpu/drm/ttm/ttm_trace.h
@@ -0,0 +1,87 @@
+/**
+ *
+ * Copyright (c) 2017 Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this s

[PATCH] drm/ttm: Add helper functions to populate/map in one call

[Tom St Denis]

These functions replace a section of common code found
in radeon/amdgpu drivers (and possibly others) as part
of the ttm_tt_*populate() callbacks.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 41 
 include/drm/ttm/ttm_page_alloc.h | 11 ++
 2 files changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 871599826773..6a660d196d87 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -920,6 +920,47 @@ void ttm_pool_unpopulate(struct ttm_tt *ttm)
 }
 EXPORT_SYMBOL(ttm_pool_unpopulate);
 
+int ttm_populate_and_map_pages(struct device *dev, struct ttm_dma_tt *tt)
+{
+   unsigned i;
+   int r;
+
+   r = ttm_pool_populate(>ttm);
+   if (r)
+   return r;
+
+   for (i = 0; i < tt->ttm.num_pages; i++) {
+   tt->dma_address[i] = dma_map_page(dev, tt->ttm.pages[i],
+ 0, PAGE_SIZE,
+ DMA_BIDIRECTIONAL);
+   if (dma_mapping_error(dev, tt->dma_address[i])) {
+   while (i--) {
+   dma_unmap_page(dev, tt->dma_address[i],
+  PAGE_SIZE, DMA_BIDIRECTIONAL);
+   tt->dma_address[i] = 0;
+   }
+   ttm_pool_unpopulate(>ttm);
+   return -EFAULT;
+   }
+   }
+   return 0;
+}
+EXPORT_SYMBOL(ttm_populate_and_map_pages);
+
+void ttm_unmap_and_unpopulate_pages(struct device *dev, struct ttm_dma_tt *tt)
+{
+   unsigned i;
+   
+   for (i = 0; i < tt->ttm.num_pages; i++) {
+   if (tt->dma_address[i]) {
+   dma_unmap_page(dev, tt->dma_address[i],
+  PAGE_SIZE, DMA_BIDIRECTIONAL);
+   }
+   }
+   ttm_pool_unpopulate(>ttm);
+}
+EXPORT_SYMBOL(ttm_unmap_and_unpopulate_pages);
+
 int ttm_page_alloc_debugfs(struct seq_file *m, void *data)
 {
struct ttm_page_pool *p;
diff --git a/include/drm/ttm/ttm_page_alloc.h b/include/drm/ttm/ttm_page_alloc.h
index 49a828425fa2..8695918ea629 100644
--- a/include/drm/ttm/ttm_page_alloc.h
+++ b/include/drm/ttm/ttm_page_alloc.h
@@ -83,6 +83,17 @@ extern int ttm_dma_page_alloc_debugfs(struct seq_file *m, 
void *data);
 extern int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev);
 extern void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev);
 
+
+/**
+ * Populates and DMA maps pages to fullfil a ttm_dma_populate() request
+ */
+int ttm_populate_and_map_pages(struct device *dev, struct ttm_dma_tt *tt);
+
+/**
+ * Unpopulates and DMA unmaps pages as part of a
+ * ttm_dma_unpopulate() request */
+void ttm_unmap_and_unpopulate_pages(struct device *dev, struct ttm_dma_tt *tt);
+
 #else
 static inline int ttm_dma_page_alloc_init(struct ttm_mem_global *glob,
  unsigned max_pages)
-- 
2.12.0

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

[PATCH] drm/nouveau: use new TTM populate/DMA map function

[Tom St Denis]

Removes common code found in numerous vendor drivers and places
it higher up in the TTM tree.

Signed-off-by: Tom St Denis <tom.stde...@amd.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 37 ++--
 1 file changed, 2 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index e427f80344c4..6ad0ad53047a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1448,8 +1448,6 @@ nouveau_ttm_tt_populate(struct ttm_tt *ttm)
struct nvkm_device *device;
struct drm_device *dev;
struct device *pdev;
-   unsigned i;
-   int r;
bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
 
if (ttm->state != tt_unpopulated)
@@ -1480,30 +1478,7 @@ nouveau_ttm_tt_populate(struct ttm_tt *ttm)
}
 #endif
 
-   r = ttm_pool_populate(ttm);
-   if (r) {
-   return r;
-   }
-
-   for (i = 0; i < ttm->num_pages; i++) {
-   dma_addr_t addr;
-
-   addr = dma_map_page(pdev, ttm->pages[i], 0, PAGE_SIZE,
-   DMA_BIDIRECTIONAL);
-
-   if (dma_mapping_error(pdev, addr)) {
-   while (i--) {
-   dma_unmap_page(pdev, ttm_dma->dma_address[i],
-  PAGE_SIZE, DMA_BIDIRECTIONAL);
-   ttm_dma->dma_address[i] = 0;
-   }
-   ttm_pool_unpopulate(ttm);
-   return -EFAULT;
-   }
-
-   ttm_dma->dma_address[i] = addr;
-   }
-   return 0;
+   return ttm_populate_and_map_pages(pdev, ttm_dma);
 }
 
 static void
@@ -1514,7 +1489,6 @@ nouveau_ttm_tt_unpopulate(struct ttm_tt *ttm)
struct nvkm_device *device;
struct drm_device *dev;
struct device *pdev;
-   unsigned i;
bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
 
if (slave)
@@ -1539,14 +1513,7 @@ nouveau_ttm_tt_unpopulate(struct ttm_tt *ttm)
}
 #endif
 
-   for (i = 0; i < ttm->num_pages; i++) {
-   if (ttm_dma->dma_address[i]) {
-   dma_unmap_page(pdev, ttm_dma->dma_address[i], PAGE_SIZE,
-  DMA_BIDIRECTIONAL);
-   }
-   }
-
-   ttm_pool_unpopulate(ttm);
+   ttm_unmap_and_unpopulate_pages(pdev, ttm_dma);
 }
 
 void
-- 
2.12.0

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

Brightness control fails between 3.6.8 and 3.7.0-rc7 (Intel 915/Dell Vostro 3560)

[Tom St Denis]

Thanks Daniel so I'm taking back my bug report on the i915 and claiming this as 
a bug against the acpi_video stack.

What sort of information would the ACPI folk like from me to help diagnose this?

Here's a dmesg dump of all ACPI messages.

http://pastebin.com/b4XKe5Fk

Tom

- Original Message -
> From: "Daniel Vetter" 
> To: "Tom St Denis" 
> Cc: linux-kernel at vger.kernel.org, "intel-gfx"  lists.freedesktop.org>, "dri-devel"
> 
> Sent: Sunday, 2 December, 2012 9:42:39 AM
> Subject: Re: Brightness control fails between 3.6.8 and 3.7.0-rc7 (Intel 
> 915/Dell Vostro 3560)
> 
> On Sun, Dec 2, 2012 at 2:49 PM, Tom St Denis
>  wrote:
> > Ok so on v3.7-rc1 [and I suspect up] I can manually control the
> > brightness by echoing to
> >
> > /sys/class/backlight/intel_backlight/brightness
> >
> > But I still can't use the buttons to control it (it only goes one
> > tick down from max and stops there).  I'm using GNOME3 from a
> > x86_64 unstable debian install [up to date with latest].
> >
> > I just noticed that under v3.7 I have a new directory
> > /sys/class/backlight/acpi_video1 which is what GNOME is actually
> > controlling.  I can't disable ACPI video control so is there a
> > workaround?
> 
> Hah, acpi registered a broken backlight driver, which overwrites the
> intel backlight - for once it's not i915.ko's fault ;-) For a
> workaround either disable the acpi backlight with
> acpi_backlight=vendor (iirc) kernel option or teach the X driver that
> you want a different backlight driver in the xorg.conf (Option
> "Backlight" "intel_backlight" should do the trick).
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 

Brightness control fails between 3.6.8 and 3.7.0-rc7 (Intel 915/Dell Vostro 3560)

[Tom St Denis]

Ok so on v3.7-rc1 [and I suspect up] I can manually control the brightness by 
echoing to 

/sys/class/backlight/intel_backlight/brightness

But I still can't use the buttons to control it (it only goes one tick down 
from max and stops there).  I'm using GNOME3 from a x86_64 unstable debian 
install [up to date with latest].

I just noticed that under v3.7 I have a new directory 
/sys/class/backlight/acpi_video1 which is what GNOME is actually controlling.  
I can't disable ACPI video control so is there a workaround?

Tom



- Original Message -
> From: "Daniel Vetter" 
> To: "Tom St Denis" 
> Cc: linux-kernel at vger.kernel.org, "intel-gfx"  lists.freedesktop.org>, "dri-devel"
> 
> Sent: Sunday, 2 December, 2012 6:43:14 AM
> Subject: Re: Brightness control fails between 3.6.8 and 3.7.0-rc7 (Intel 
> 915/Dell Vostro 3560)
> 
> Including a bunch more lists to the cc.
> 
> On Sun, Dec 2, 2012 at 12:42 PM, Daniel Vetter
>  wrote:
> > On Sun, Dec 2, 2012 at 12:32 PM, Tom St Denis
> >  wrote:
> >> I've narrowed it down to sometime between v3.6.8 and v3.7-rc1.  So
> >> none of the v3.7 series will work with this GPU/panel.
> >
> > Hm, that's still a few hundred relevant commits intermingled with a
> > few thousand that likely don't matter ;-) Can you please attempt a
> > kernel bisect with git? For a nice howto check out
> > http://www.reactivated.net/weblog/archives/2006/01/using-git-bisect-to-find-buggy-kernel-patches/
> >
> > Also, can you please check which of the different backlight
> > controls
> > in /sys/class/backlight work on 3.6 (and whether any of them still
> > work on 3.7)?
> >
> > Thanks, Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 
> 
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 

Re: Brightness control fails between 3.6.8 and 3.7.0-rc7 (Intel 915/Dell Vostro 3560)

[Tom St Denis]

Ok so on v3.7-rc1 [and I suspect up] I can manually control the brightness by 
echoing to 

/sys/class/backlight/intel_backlight/brightness

But I still can't use the buttons to control it (it only goes one tick down 
from max and stops there).  I'm using GNOME3 from a x86_64 unstable debian 
install [up to date with latest].

I just noticed that under v3.7 I have a new directory 
/sys/class/backlight/acpi_video1 which is what GNOME is actually controlling.  
I can't disable ACPI video control so is there a workaround?

Tom



- Original Message -
 From: Daniel Vetter daniel.vet...@ffwll.ch
 To: Tom St Denis tstde...@elliptictech.com
 Cc: linux-ker...@vger.kernel.org, intel-gfx 
 intel-...@lists.freedesktop.org, dri-devel
 dri-devel@lists.freedesktop.org
 Sent: Sunday, 2 December, 2012 6:43:14 AM
 Subject: Re: Brightness control fails between 3.6.8 and 3.7.0-rc7 (Intel 
 915/Dell Vostro 3560)
 
 Including a bunch more lists to the cc.
 
 On Sun, Dec 2, 2012 at 12:42 PM, Daniel Vetter
 daniel.vet...@ffwll.ch wrote:
  On Sun, Dec 2, 2012 at 12:32 PM, Tom St Denis
  tstde...@elliptictech.com wrote:
  I've narrowed it down to sometime between v3.6.8 and v3.7-rc1.  So
  none of the v3.7 series will work with this GPU/panel.
 
  Hm, that's still a few hundred relevant commits intermingled with a
  few thousand that likely don't matter ;-) Can you please attempt a
  kernel bisect with git? For a nice howto check out
  http://www.reactivated.net/weblog/archives/2006/01/using-git-bisect-to-find-buggy-kernel-patches/
 
  Also, can you please check which of the different backlight
  controls
  in /sys/class/backlight work on 3.6 (and whether any of them still
  work on 3.7)?
 
  Thanks, Daniel
  --
  Daniel Vetter
  Software Engineer, Intel Corporation
  +41 (0) 79 365 57 48 - http://blog.ffwll.ch
 
 
 
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch
 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Brightness control fails between 3.6.8 and 3.7.0-rc7 (Intel 915/Dell Vostro 3560)

[Tom St Denis]

Thanks Daniel so I'm taking back my bug report on the i915 and claiming this as 
a bug against the acpi_video stack.

What sort of information would the ACPI folk like from me to help diagnose this?

Here's a dmesg dump of all ACPI messages.

http://pastebin.com/b4XKe5Fk

Tom

- Original Message -
 From: Daniel Vetter daniel.vet...@ffwll.ch
 To: Tom St Denis tstde...@elliptictech.com
 Cc: linux-ker...@vger.kernel.org, intel-gfx 
 intel-...@lists.freedesktop.org, dri-devel
 dri-devel@lists.freedesktop.org
 Sent: Sunday, 2 December, 2012 9:42:39 AM
 Subject: Re: Brightness control fails between 3.6.8 and 3.7.0-rc7 (Intel 
 915/Dell Vostro 3560)
 
 On Sun, Dec 2, 2012 at 2:49 PM, Tom St Denis
 tstde...@elliptictech.com wrote:
  Ok so on v3.7-rc1 [and I suspect up] I can manually control the
  brightness by echoing to
 
  /sys/class/backlight/intel_backlight/brightness
 
  But I still can't use the buttons to control it (it only goes one
  tick down from max and stops there).  I'm using GNOME3 from a
  x86_64 unstable debian install [up to date with latest].
 
  I just noticed that under v3.7 I have a new directory
  /sys/class/backlight/acpi_video1 which is what GNOME is actually
  controlling.  I can't disable ACPI video control so is there a
  workaround?
 
 Hah, acpi registered a broken backlight driver, which overwrites the
 intel backlight - for once it's not i915.ko's fault ;-) For a
 workaround either disable the acpi backlight with
 acpi_backlight=vendor (iirc) kernel option or teach the X driver that
 you want a different backlight driver in the xorg.conf (Option
 Backlight intel_backlight should do the trick).
 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch
 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel