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

[Thomas Hellstrom <thellstrom@vmware.com>]

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 Hellstrom<thellstrom@vmware.com>  wrote:
>    
>> On 11/19/2011 12:32 AM, j.glisse@gmail.com wrote:
>>      
>>> 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);
>>>
>>>        
>> 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
>