From: Jerome Glisse jglisse@redhat.com
Previously we were calling back move_notify in error path when the bo is returned to it's original position or when destroy the bo. When destroying the bo set the new mem placement as NULL when calling back in the driver.
Updating nouveau to deal with NULL placement properly.
v2: reserve the object before calling move_notify in bo destroy path at that point ttm should be the only piece of code interacting with the object so atomic_set is safe here. v3: callback move notify only once the bo is in its new position call move notify want swaping out the buffer
Reviewed-by: Jerome Glisse jglisse@redhat.com --- drivers/gpu/drm/nouveau/nouveau_bo.c | 4 ++-- drivers/gpu/drm/ttm/ttm_bo.c | 17 +++++++++++++---- 2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 857bca4..f12dd0f 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -815,10 +815,10 @@ nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, struct ttm_mem_reg *new_mem) struct nouveau_vma *vma;
list_for_each_entry(vma, &nvbo->vma_list, head) { - if (new_mem->mem_type == TTM_PL_VRAM) { + if (new_mem && new_mem->mem_type == TTM_PL_VRAM) { nouveau_vm_map(vma, new_mem->mm_node); } else - if (new_mem->mem_type == TTM_PL_TT && + if (new_mem && new_mem->mem_type == TTM_PL_TT && nvbo->page_shift == vma->vm->spg_shift) { nouveau_vm_map_sg(vma, 0, new_mem-> num_pages << PAGE_SHIFT, diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index de7ad99..0c1d821 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -147,6 +147,12 @@ static void ttm_bo_release_list(struct kref *list_kref) BUG_ON(!list_empty(&bo->lru)); BUG_ON(!list_empty(&bo->ddestroy));
+ /* force bo to reserved, at this point we should be the only owner */ + atomic_set(&bo->reserved, 1); + if (bdev->driver->move_notify) + bdev->driver->move_notify(bo, NULL); + atomic_set(&bo->reserved, 0); + if (bo->ttm) ttm_tt_destroy(bo->ttm); atomic_dec(&bo->glob->bo_count); @@ -404,9 +410,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, } }
- if (bdev->driver->move_notify) - bdev->driver->move_notify(bo, mem); - if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) && !(new_man->flags & TTM_MEMTYPE_FLAG_FIXED)) ret = ttm_bo_move_ttm(bo, evict, no_wait_reserve, no_wait_gpu, mem); @@ -419,6 +422,9 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, if (ret) goto out_err;
+ if (bdev->driver->move_notify) + bdev->driver->move_notify(bo, mem); + moved: if (bo->evicted) { ret = bdev->driver->invalidate_caches(bdev, bo->mem.placement); @@ -1872,9 +1878,12 @@ static int ttm_bo_swapout(struct ttm_mem_shrink *shrink) if (bo->bdev->driver->swap_notify) bo->bdev->driver->swap_notify(bo);
+ if (bo->bdev->driver->move_notify) + bo->bdev->driver->move_notify(bo, NULL); + ret = ttm_tt_swapout(bo->ttm, bo->persistent_swap_storage); -out:
+out: /** * * Unreserve without putting on LRU to avoid swapping out an
On 11/19/2011 12:32 AM, j.glisse@gmail.com wrote:
From: Jerome Glissejglisse@redhat.com
Previously we were calling back move_notify in error path when the bo is returned to it's original position or when destroy the bo. When destroying the bo set the new mem placement as NULL when calling back in the driver.
Updating nouveau to deal with NULL placement properly.
v2: reserve the object before calling move_notify in bo destroy path at that point ttm should be the only piece of code interacting with the object so atomic_set is safe here. v3: callback move notify only once the bo is in its new position call move notify want swaping out the buffer
Reviewed-by: Jerome Glissejglisse@redhat.com
drivers/gpu/drm/nouveau/nouveau_bo.c | 4 ++-- drivers/gpu/drm/ttm/ttm_bo.c | 17 +++++++++++++---- 2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 857bca4..f12dd0f 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -815,10 +815,10 @@ nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, struct ttm_mem_reg *new_mem) struct nouveau_vma *vma;
list_for_each_entry(vma,&nvbo->vma_list, head) {
if (new_mem->mem_type == TTM_PL_VRAM) {
if (new_mem&& new_mem->mem_type == TTM_PL_VRAM) { nouveau_vm_map(vma, new_mem->mm_node);
if (new_mem->mem_type == TTM_PL_TT&&
if (new_mem&& new_mem->mem_type == TTM_PL_TT&&diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index de7ad99..0c1d821 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -147,6 +147,12 @@ static void ttm_bo_release_list(struct kref *list_kref) BUG_ON(!list_empty(&bo->lru)); BUG_ON(!list_empty(&bo->ddestroy));
- /* force bo to reserved, at this point we should be the only owner */
- atomic_set(&bo->reserved, 1);
- if (bdev->driver->move_notify)
bdev->driver->move_notify(bo, NULL);- atomic_set(&bo->reserved, 0);
We can actually do this from the top of ttm_bo_cleanup_memtype_use(). Then we should catch all current and future use-cases and you wouldn't need the fake reserving, because at that point, we're already reserved.
- if (bo->ttm) ttm_tt_destroy(bo->ttm); atomic_dec(&bo->glob->bo_count);
@@ -404,9 +410,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, } }
- if (bdev->driver->move_notify)
bdev->driver->move_notify(bo, mem);- if (!(old_man->flags& TTM_MEMTYPE_FLAG_FIXED)&& !(new_man->flags& TTM_MEMTYPE_FLAG_FIXED)) ret = ttm_bo_move_ttm(bo, evict, no_wait_reserve, no_wait_gpu, mem);
@@ -419,6 +422,9 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, if (ret) goto out_err;
- if (bdev->driver->move_notify)
bdev->driver->move_notify(bo, mem);
moved: if (bo->evicted) { ret = bdev->driver->invalidate_caches(bdev, bo->mem.placement); @@ -1872,9 +1878,12 @@ static int ttm_bo_swapout(struct ttm_mem_shrink *shrink) if (bo->bdev->driver->swap_notify) bo->bdev->driver->swap_notify(bo);
- if (bo->bdev->driver->move_notify)
bo->bdev->driver->move_notify(bo, NULL);
Hmm. On second thought, we could use swap_notify() for this, I missed we already had that and that's what vmwgfx once used for exactly the same purpose.
ret = ttm_tt_swapout(bo->ttm, bo->persistent_swap_storage); -out:
+out:
Whitespace.
/Thomas
On Sat, Nov 19, 2011 at 3:45 PM, Thomas Hellstrom thellstrom@vmware.com wrote:
On 11/19/2011 12:32 AM, j.glisse@gmail.com wrote:
From: Jerome Glissejglisse@redhat.com
Previously we were calling back move_notify in error path when the bo is returned to it's original position or when destroy the bo. When destroying the bo set the new mem placement as NULL when calling back in the driver.
Updating nouveau to deal with NULL placement properly.
v2: reserve the object before calling move_notify in bo destroy path at that point ttm should be the only piece of code interacting with the object so atomic_set is safe here. v3: callback move notify only once the bo is in its new position call move notify want swaping out the buffer
Reviewed-by: Jerome Glissejglisse@redhat.com
drivers/gpu/drm/nouveau/nouveau_bo.c | 4 ++-- drivers/gpu/drm/ttm/ttm_bo.c | 17 +++++++++++++---- 2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 857bca4..f12dd0f 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -815,10 +815,10 @@ nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, struct ttm_mem_reg *new_mem) struct nouveau_vma *vma;
list_for_each_entry(vma,&nvbo->vma_list, head) {
- if (new_mem->mem_type == TTM_PL_VRAM) {
- if (new_mem&& new_mem->mem_type == TTM_PL_VRAM) {
nouveau_vm_map(vma, new_mem->mm_node); } else
- if (new_mem->mem_type == TTM_PL_TT&&
- if (new_mem&& new_mem->mem_type == TTM_PL_TT&&
nvbo->page_shift == vma->vm->spg_shift) { nouveau_vm_map_sg(vma, 0, new_mem-> num_pages<< PAGE_SHIFT, diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index de7ad99..0c1d821 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -147,6 +147,12 @@ static void ttm_bo_release_list(struct kref *list_kref) BUG_ON(!list_empty(&bo->lru)); BUG_ON(!list_empty(&bo->ddestroy));
- /* force bo to reserved, at this point we should be the only owner
*/
- atomic_set(&bo->reserved, 1);
- if (bdev->driver->move_notify)
- bdev->driver->move_notify(bo, NULL);
- atomic_set(&bo->reserved, 0);
We can actually do this from the top of ttm_bo_cleanup_memtype_use(). Then we should catch all current and future use-cases and you wouldn't need the fake reserving, because at that point, we're already reserved.
if (bo->ttm) ttm_tt_destroy(bo->ttm); atomic_dec(&bo->glob->bo_count); @@ -404,9 +410,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, } }
- if (bdev->driver->move_notify)
- bdev->driver->move_notify(bo, mem);
if (!(old_man->flags& TTM_MEMTYPE_FLAG_FIXED)&& !(new_man->flags& TTM_MEMTYPE_FLAG_FIXED)) ret = ttm_bo_move_ttm(bo, evict, no_wait_reserve, no_wait_gpu, mem); @@ -419,6 +422,9 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, if (ret) goto out_err;
- if (bdev->driver->move_notify)
- bdev->driver->move_notify(bo, mem);
moved: if (bo->evicted) { ret = bdev->driver->invalidate_caches(bdev, bo->mem.placement); @@ -1872,9 +1878,12 @@ static int ttm_bo_swapout(struct ttm_mem_shrink *shrink) if (bo->bdev->driver->swap_notify) bo->bdev->driver->swap_notify(bo);
- if (bo->bdev->driver->move_notify)
- bo->bdev->driver->move_notify(bo, NULL);
Hmm. On second thought, we could use swap_notify() for this, I missed we already had that and that's what vmwgfx once used for exactly the same purpose.
ret = ttm_tt_swapout(bo->ttm, bo->persistent_swap_storage); -out:
+out:
Whitespace.
/Thomas
Attached updated patch
Cheers, Jerome
Updated patch
Reviewed-by: Thomas Hellstrom thellstrom@vmware.com
On 11/20/2011 10:02 PM, Jerome Glisse wrote:
On Sat, Nov 19, 2011 at 3:45 PM, Thomas Hellstromthellstrom@vmware.com wrote:
On 11/19/2011 12:32 AM, j.glisse@gmail.com wrote:
From: Jerome Glissejglisse@redhat.com
Previously we were calling back move_notify in error path when the bo is returned to it's original position or when destroy the bo. When destroying the bo set the new mem placement as NULL when calling back in the driver.
Updating nouveau to deal with NULL placement properly.
v2: reserve the object before calling move_notify in bo destroy path at that point ttm should be the only piece of code interacting with the object so atomic_set is safe here. v3: callback move notify only once the bo is in its new position call move notify want swaping out the buffer
Reviewed-by: Jerome Glissejglisse@redhat.com
drivers/gpu/drm/nouveau/nouveau_bo.c | 4 ++-- drivers/gpu/drm/ttm/ttm_bo.c | 17 +++++++++++++---- 2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 857bca4..f12dd0f 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -815,10 +815,10 @@ nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, struct ttm_mem_reg *new_mem) struct nouveau_vma *vma;
list_for_each_entry(vma,&nvbo->vma_list, head) {
if (new_mem->mem_type == TTM_PL_VRAM) {
if (new_mem&& new_mem->mem_type == TTM_PL_VRAM) { nouveau_vm_map(vma, new_mem->mm_node); } else
if (new_mem->mem_type == TTM_PL_TT&&
if (new_mem&& new_mem->mem_type == TTM_PL_TT&& nvbo->page_shift == vma->vm->spg_shift) { nouveau_vm_map_sg(vma, 0, new_mem-> num_pages<< PAGE_SHIFT,diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index de7ad99..0c1d821 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -147,6 +147,12 @@ static void ttm_bo_release_list(struct kref *list_kref) BUG_ON(!list_empty(&bo->lru)); BUG_ON(!list_empty(&bo->ddestroy));
/* force bo to reserved, at this point we should be the only owner*/
atomic_set(&bo->reserved, 1);if (bdev->driver->move_notify)bdev->driver->move_notify(bo, NULL);atomic_set(&bo->reserved, 0);We can actually do this from the top of ttm_bo_cleanup_memtype_use(). Then we should catch all current and future use-cases and you wouldn't need the fake reserving, because at that point, we're already reserved.
if (bo->ttm) ttm_tt_destroy(bo->ttm); atomic_dec(&bo->glob->bo_count);@@ -404,9 +410,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, } }
if (bdev->driver->move_notify)bdev->driver->move_notify(bo, mem);if (!(old_man->flags& TTM_MEMTYPE_FLAG_FIXED)&& !(new_man->flags& TTM_MEMTYPE_FLAG_FIXED)) ret = ttm_bo_move_ttm(bo, evict, no_wait_reserve,no_wait_gpu, mem); @@ -419,6 +422,9 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, if (ret) goto out_err;
if (bdev->driver->move_notify)bdev->driver->move_notify(bo, mem);moved: if (bo->evicted) { ret = bdev->driver->invalidate_caches(bdev, bo->mem.placement); @@ -1872,9 +1878,12 @@ static int ttm_bo_swapout(struct ttm_mem_shrink *shrink) if (bo->bdev->driver->swap_notify) bo->bdev->driver->swap_notify(bo);
if (bo->bdev->driver->move_notify)bo->bdev->driver->move_notify(bo, NULL);Hmm. On second thought, we could use swap_notify() for this, I missed we already had that and that's what vmwgfx once used for exactly the same purpose.
ret = ttm_tt_swapout(bo->ttm, bo->persistent_swap_storage);-out:
+out:
Whitespace.
/Thomas
Attached updated patch
Cheers, Jerome
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
j.glisse＠gmail.com -
Jerome Glisse
-
Thomas Hellstrom