[PATCH 1/4] drm/vram-helper: cleanup drm_gem_vram_bo_driver_move_notify

[Thomas Zimmermann]

Hi

Am 11.02.21 um 16:05 schrieb Christian König:
> 
> 
> Am 11.02.21 um 15:52 schrieb Thomas Zimmermann:
>> Hi
>>
>> Am 11.02.21 um 14:16 schrieb Christian König:
>>> Swapping bo->mem was completely unecessary. Cleanup the function which
>>> is just a leftover from a TTM cleanup.
>>
>> Yes this was introduced in a recent cleanup effort. Can you explain 
>> what the code intends to do? It seems as if it tries to "re-unmap the 
>> BO" if the move_memcpy fails.
>>
>> If the move_memcpy fails now, it seems like we can live without 
>> reverting that call to drm_gem_vram_bo_driver_move_notify(). (?)
> 
> I think so, but I'm not 100% sure either.
> 
> The swap() -> notify -> swap() was just how TTM did it and that was 
> moved into the drivers.
> 
> I'm now just trying to remove all the hard write accesses to bo->mem 
> from drivers and stumbled over this here.

We already have a vmap count of zero; so unmapping the BO pages is fine 
at any time. The next caller of vmap will simple instantiate a new mapping.

Let me give this patch a test run tomorrow, but it seems correct.

Best regards
Thomas

> 
> Thanks for the comments,
> Christian.
> 
>>
>> Best regards
>> Thomas
>>
>>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>> ---
>>>   drivers/gpu/drm/drm_gem_vram_helper.c | 18 ++++--------------
>>>   1 file changed, 4 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
>>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>>> index a0992f0b8afd..0c2233ee6029 100644
>>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>>> @@ -564,9 +564,7 @@ static void 
>>> drm_gem_vram_bo_driver_evict_flags(struct drm_gem_vram_object *gbo,
>>>       *pl = gbo->placement;
>>>   }
>>>   -static void drm_gem_vram_bo_driver_move_notify(struct 
>>> drm_gem_vram_object *gbo,
>>> -                           bool evict,
>>> -                           struct ttm_resource *new_mem)
>>> +static void drm_gem_vram_bo_driver_move_notify(struct 
>>> drm_gem_vram_object *gbo)
>>>   {
>>>       struct ttm_buffer_object *bo = &gbo->bo;
>>>       struct drm_device *dev = bo->base.dev;
>>> @@ -582,16 +580,8 @@ static int drm_gem_vram_bo_driver_move(struct 
>>> drm_gem_vram_object *gbo,
>>>                          struct ttm_operation_ctx *ctx,
>>>                          struct ttm_resource *new_mem)
>>>   {
>>> -    int ret;
>>> -
>>> -    drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
>>> -    ret = ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
>>> -    if (ret) {
>>> -        swap(*new_mem, gbo->bo.mem);
>>> -        drm_gem_vram_bo_driver_move_notify(gbo, false, new_mem);
>>> -        swap(*new_mem, gbo->bo.mem);
>>> -    }
>>> -    return ret;
>>> +    drm_gem_vram_bo_driver_move_notify(gbo);
>>> +    return ttm_bo_move_memcpy(&gbo->bo, ctx, new_mem);
>>>   }
>>>     /*
>>> @@ -947,7 +937,7 @@ static void bo_driver_delete_mem_notify(struct 
>>> ttm_buffer_object *bo)
>>>         gbo = drm_gem_vram_of_bo(bo);
>>>   -    drm_gem_vram_bo_driver_move_notify(gbo, false, NULL);
>>> +    drm_gem_vram_bo_driver_move_notify(gbo);
>>>   }
>>>     static int bo_driver_move(struct ttm_buffer_object *bo,
>>>
>>
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20210211/1416b526/attachment.sig>