From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Hellstrom Subject: Re: [PATCH] drm/ttm: callback move_notify any time bo placement change v3 Date: Sat, 19 Nov 2011 21:45:13 +0100 Message-ID: <4EC81559.9080002@vmware.com> References: <1321659155-18532-1-git-send-email-j.glisse@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from smtp-outbound-2.vmware.com (smtp-outbound-2.vmware.com [65.115.85.73]) by gabe.freedesktop.org (Postfix) with ESMTP id 8E93C9EED4 for ; Sat, 19 Nov 2011 12:47:58 -0800 (PST) In-Reply-To: <1321659155-18532-1-git-send-email-j.glisse@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: j.glisse@gmail.com Cc: Jerome Glisse , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 11/19/2011 12:32 AM, j.glisse@gmail.com wrote: > From: Jerome Glisse > > 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 > --- > 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