[PATCH] drm/ttm: callback move_notify any time bo placement change v3

[Jerome Glisse]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-ttm-callback-move_notify-any-time-bo-placement-c.patch
Type: application/octet-stream
Size: 3037 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20111120/a32e587e/attachment.obj>