Updated patch Reviewed-by: Thomas Hellstrom <thellstrom at vmware.com>
On 11/20/2011 10:02 PM, Jerome Glisse wrote: > On Sat, Nov 19, 2011 at 3:45 PM, Thomas Hellstrom<thellstrom at vmware.com> > wrote: > >> On 11/19/2011 12:32 AM, j.glisse at gmail.com wrote: >> >>> From: Jerome Glisse<jglisse at 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 at 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 at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >