Various TTM cleanups/fixes

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

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

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stdenis@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(&bo->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(&bo->lru));
 
 		man = &bdev->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(&bdev->wq,
 				      ((HZ / 100) < 1) ? 1 : HZ / 100);
-	}
 }
 
 static void ttm_bo_release(struct kref *kref)
-- 
2.14.3

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

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

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stdenis@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(&bo->ttm_resv);
 	mutex_destroy(&bo->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(&bo->kref);
 	kref_init(&bo->list_kref);
-- 
2.14.3

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

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

[Felix Kuehling]

On 2018-01-26 01:29 PM, Tom St Denis wrote:
> Signed-off-by: Tom St Denis <tom.stdenis@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);
> +};

Stray semicolon.

> +
>  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(&bo->ttm_resv);
>  	mutex_destroy(&bo->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;

This could be written shorter as "!destroy ? ttm_bo_default_destroy :
destroy", or even "destroy ? destroy : ttm_bo_default_destroy".

Regards,
  Felix

>  
>  	kref_init(&bo->kref);
>  	kref_init(&bo->list_kref);

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

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

[Ville Syrjälä]

On Fri, Jan 26, 2018 at 02:23:39PM -0500, Felix Kuehling wrote:
> On 2018-01-26 01:29 PM, Tom St Denis wrote:
> > Signed-off-by: Tom St Denis <tom.stdenis@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);
> > +};
> 
> Stray semicolon.
> 
> > +
> >  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(&bo->ttm_resv);
> >  	mutex_destroy(&bo->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;
> 
> This could be written shorter as "!destroy ? ttm_bo_default_destroy :
> destroy", or even "destroy ? destroy : ttm_bo_default_destroy".

Or even 'destroy ?: ttm_bo_default_destroy' if you think
the gcc extension is cool enough. It is used elsewhere in
the kernel.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stdenis@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", &val);
 	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 == &ttm_page_pool_max)
+	if (attr == &ttm_page_pool_max) {
 		m->options.max_size = val;
-	else if (attr == &ttm_page_pool_small)
+	} else if (attr == &ttm_page_pool_small) {
 		m->options.small = val;
-	else if (attr == &ttm_page_pool_alloc_size) {
+	} else if (attr == &ttm_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

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

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

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stdenis@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(&page, 1);
-			if (ret) {
+			if (set_pages_array_wb(&page, 1)) {
 				pr_err("%s: Failed to set %d pages to wb!\n",
 				       pool->dev_name, 1);
 			}
-- 
2.14.3

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

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

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stdenis@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, &dev->dma_pools, pools) {
-		if (pool->type != type)
-			continue;
-		found = pool;
-		break;
+		if (pool->type == type)
+			return pool;
 	}
-	return found;
+	return NULL;
 }
 
 /*
-- 
2.14.3

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

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

[Felix Kuehling]

On 2018-01-26 01:29 PM, Tom St Denis wrote:
> Signed-off-by: Tom St Denis <tom.stdenis@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, &dev->dma_pools, pools) {
> -		if (pool->type != type)
> -			continue;
> -		found = pool;
> -		break;
> +		if (pool->type == type)
> +			return pool;
>  	}

Now you could also remove the braces around the loop body, because it's
only a single statement.

Regards,
  Felix

> -	return found;
> +	return NULL;
>  }
>  
>  /*

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

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

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stdenis@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

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

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

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stdenis@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

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

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

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stdenis@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 =
 		&bdev->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(&bo->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, &ctx)) {
-			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

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

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

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stdenis@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(&bo->resv->lock,
-							       ticket);
-		} else if (ret == -EDEADLK) {
-			ww_mutex_lock_slow(&bo->resv->lock, ticket);
-			ret = 0;
+		if (ret == -EDEADLK) {
+			if (intr) {
+				ret = ww_mutex_lock_slow_interruptible(&bo->resv->lock,
+								       ticket);
+			} else {
+				ww_mutex_lock_slow(&bo->resv->lock, ticket);
+				ret = 0;
+			}
 		}
 
 		if (!ret && entry->shared)
-- 
2.14.3

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

Re: Various TTM cleanups/fixes

[Christian König]

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.koenig@amd.com>.

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-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

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.koenig@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-gfx@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 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.stdenis@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(&ttm_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 04/12] drm/ttm: Fix indentation in ttm_tt_swapout()

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stdenis@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 11/12] drm/ttm: Fix 'buf' pointer update in ttm_bo_vm_access_kmap()

[Tom St Denis]

Signed-off-by: Tom St Denis <tom.stdenis@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

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

[Felix Kuehling]

On 2018-01-26 01:29 PM, Tom St Denis wrote:
> Signed-off-by: Tom St Denis <tom.stdenis@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;

I'd put this right before the bytes_left -= bytes.

Also, buf is a void *. That makes pointer arithmetic on it somewhat
undefined. From what I can find, GCC supports it as an extension. See
also
https://stackoverflow.com/questions/3523145/pointer-arithmetic-for-void-pointer-in-c.

If it compiles without a warning, I guess it's OK as is.

Regards,
  Felix

>  	} while (bytes_left);
>  
>  	return len;

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