Re: [PATCH 1/6] drm/ttm: Add unampping of the entire device address space

["Koenig, Christian" <Christian.Koenig@amd.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3124 bytes --]



Am 09.06.2020 18:37 schrieb "Grodzovsky, Andrey" <Andrey.Grodzovsky@amd.com>:

On 6/5/20 2:40 PM, Christian König wrote:
> Am 05.06.20 um 16:29 schrieb Andrey Grodzovsky:
>>
>> On 5/11/20 2:45 AM, Christian König wrote:
>>> Am 09.05.20 um 20:51 schrieb Andrey Grodzovsky:
>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/ttm/ttm_bo.c    | 22 +++++++++++++++++++++-
>>>>   include/drm/ttm/ttm_bo_driver.h |  2 ++
>>>>   2 files changed, 23 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
>>>> b/drivers/gpu/drm/ttm/ttm_bo.c
>>>> index c5b516f..eae61cc 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>> @@ -1750,9 +1750,29 @@ void ttm_bo_unmap_virtual(struct
>>>> ttm_buffer_object *bo)
>>>>       ttm_bo_unmap_virtual_locked(bo);
>>>>       ttm_mem_io_unlock(man);
>>>>   }
>>>> +EXPORT_SYMBOL(ttm_bo_unmap_virtual);
>>>>   +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev)
>>>> +{
>>>> +    struct ttm_mem_type_manager *man;
>>>> +    int i;
>>>>   -EXPORT_SYMBOL(ttm_bo_unmap_virtual);
>>>
>>>> +    for (i = 0; i < TTM_NUM_MEM_TYPES; i++) {
>>>> +        man = &bdev->man[i];
>>>> +        if (man->has_type && man->use_type)
>>>> +            ttm_mem_io_lock(man, false);
>>>> +    }
>>>
>>> You should drop that it will just result in a deadlock warning for
>>> Nouveau and has no effect at all.
>>>
>>> Apart from that looks good to me,
>>> Christian.
>>
>>
>> As I am considering to re-include this in V2 of the patchsets, can
>> you clarify please why this will have no effect at all ?
>
> The locks are exclusive for Nouveau to allocate/free the io address
> space.
>
> Since we don't do this here we don't need the locks.
>
> Christian.


So basically calling unmap_mapping_range doesn't require any extra
locking around it and whatever locks are taken within the function
should be enough ?


I think so, yes.

Christian.


Andrey


>
>>
>> Andrey
>>
>>
>>>
>>>> +
>>>> +    unmap_mapping_range(bdev->dev_mapping, 0, 0 , 1);
>>>> +    /*TODO What about ttm_mem_io_free_vm(bo) ? */
>>>> +
>>>> +    for (i = TTM_NUM_MEM_TYPES - 1; i >= 0; i--) {
>>>> +        man = &bdev->man[i];
>>>> +        if (man->has_type && man->use_type)
>>>> +            ttm_mem_io_unlock(man);
>>>> +    }
>>>> +}
>>>> +EXPORT_SYMBOL(ttm_bo_unmap_virtual_address_space);
>>>>     int ttm_bo_wait(struct ttm_buffer_object *bo,
>>>>           bool interruptible, bool no_wait)
>>>> diff --git a/include/drm/ttm/ttm_bo_driver.h
>>>> b/include/drm/ttm/ttm_bo_driver.h
>>>> index c9e0fd0..3133463 100644
>>>> --- a/include/drm/ttm/ttm_bo_driver.h
>>>> +++ b/include/drm/ttm/ttm_bo_driver.h
>>>> @@ -600,6 +600,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>>>>    */
>>>>   void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo);
>>>>   +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device
>>>> *bdev);
>>>> +
>>>>   /**
>>>>    * ttm_bo_unmap_virtual
>>>>    *
>>>
>


[-- Attachment #1.2: Type: text/html, Size: 6441 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx