diff mbox series

[v3,1/4] drm/ttm: Create pinned list

Message ID 20210827203910.5565-2-andrey.grodzovsky@amd.com (mailing list archive)
State New, archived
Headers show
Series Various fixes to pass libdrm hotunplug tests | expand

Commit Message

Andrey Grodzovsky Aug. 27, 2021, 8:39 p.m. UTC
This list will be used to capture all non VRAM BOs not
on LRU so when device is hot unplugged we can iterate
the list and unmap DMA mappings before device is removed.

v2: Reanme function to ttm_bo_move_to_pinned
v3: Move the pinned list to ttm device

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
Suggested-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c     | 18 ++++++++++++++----
 drivers/gpu/drm/ttm/ttm_device.c |  1 +
 include/drm/ttm/ttm_device.h     |  1 +
 3 files changed, 16 insertions(+), 4 deletions(-)

Comments

Christian König Aug. 30, 2021, 8:58 a.m. UTC | #1
Am 27.08.21 um 22:39 schrieb Andrey Grodzovsky:
> This list will be used to capture all non VRAM BOs not
> on LRU so when device is hot unplugged we can iterate
> the list and unmap DMA mappings before device is removed.
>
> v2: Reanme function to ttm_bo_move_to_pinned
> v3: Move the pinned list to ttm device

As far as I can see there is not list_del() remaining. So this won't 
work correctly.

I suggest to rather rebase on top of the stuff I'm working on for a 
while to move the LRU into the resource instead.

Just send out the latest patch set of this with you in CC.

Christian.

>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> Suggested-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_bo.c     | 18 ++++++++++++++----
>   drivers/gpu/drm/ttm/ttm_device.c |  1 +
>   include/drm/ttm/ttm_device.h     |  1 +
>   3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 1b950b45cf4b..1fedd0eb67ba 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -69,7 +69,17 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
>   	}
>   }
>   
> -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
> +static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object *bo)
> +{
> +	struct ttm_device *bdev = bo->bdev;
> +
> +	list_move_tail(&bo->lru, &bdev->pinned);
> +
> +	if (bdev->funcs->del_from_lru_notify)
> +		bdev->funcs->del_from_lru_notify(bo);
> +}
> +
> +static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>   {
>   	struct ttm_device *bdev = bo->bdev;
>   
> @@ -98,7 +108,7 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
>   		dma_resv_assert_held(bo->base.resv);
>   
>   	if (bo->pin_count) {
> -		ttm_bo_del_from_lru(bo);
> +		ttm_bo_move_to_pinned(bo);
>   		return;
>   	}
>   
> @@ -339,7 +349,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
>   		return ret;
>   	}
>   
> -	ttm_bo_del_from_lru(bo);
> +	ttm_bo_move_to_pinned(bo);
>   	list_del_init(&bo->ddestroy);
>   	spin_unlock(&bo->bdev->lru_lock);
>   	ttm_bo_cleanup_memtype_use(bo);
> @@ -1154,7 +1164,7 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
>   		return 0;
>   	}
>   
> -	ttm_bo_del_from_lru(bo);
> +	ttm_bo_move_to_pinned(bo);
>   	/* TODO: Cleanup the locking */
>   	spin_unlock(&bo->bdev->lru_lock);
>   
> diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
> index 5f31acec3ad7..530a9c36be37 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, struct ttm_device_funcs *funcs,
>   	INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
>   	spin_lock_init(&bdev->lru_lock);
>   	INIT_LIST_HEAD(&bdev->ddestroy);
> +	INIT_LIST_HEAD(&bdev->pinned);
>   	bdev->dev_mapping = mapping;
>   	mutex_lock(&ttm_global_mutex);
>   	list_add_tail(&bdev->device_list, &glob->device_list);
> diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
> index cd592f8e941b..03fb44d061e0 100644
> --- a/include/drm/ttm/ttm_device.h
> +++ b/include/drm/ttm/ttm_device.h
> @@ -265,6 +265,7 @@ struct ttm_device {
>   	 */
>   	spinlock_t lru_lock;
>   	struct list_head ddestroy;
> +	struct list_head pinned;
>   
>   	/*
>   	 * Protected by load / firstopen / lastclose /unload sync.
Andrey Grodzovsky Aug. 30, 2021, 2:16 p.m. UTC | #2
On 2021-08-30 4:58 a.m., Christian König wrote:
> Am 27.08.21 um 22:39 schrieb Andrey Grodzovsky:
>> This list will be used to capture all non VRAM BOs not
>> on LRU so when device is hot unplugged we can iterate
>> the list and unmap DMA mappings before device is removed.
>>
>> v2: Reanme function to ttm_bo_move_to_pinned
>> v3: Move the pinned list to ttm device
>
> As far as I can see there is not list_del() remaining. So this won't 
> work correctly.


It's in ttm_bo_release, there was no code change there hence it's not 
captured in the patch.

Andrey


>
> I suggest to rather rebase on top of the stuff I'm working on for a 
> while to move the LRU into the resource instead.
>
> Just send out the latest patch set of this with you in CC.
>
> Christian.
>
>>
>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>> Suggested-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo.c     | 18 ++++++++++++++----
>>   drivers/gpu/drm/ttm/ttm_device.c |  1 +
>>   include/drm/ttm/ttm_device.h     |  1 +
>>   3 files changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 1b950b45cf4b..1fedd0eb67ba 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -69,7 +69,17 @@ static void ttm_bo_mem_space_debug(struct 
>> ttm_buffer_object *bo,
>>       }
>>   }
>>   -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>> +static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object *bo)
>> +{
>> +    struct ttm_device *bdev = bo->bdev;
>> +
>> +    list_move_tail(&bo->lru, &bdev->pinned);
>> +
>> +    if (bdev->funcs->del_from_lru_notify)
>> +        bdev->funcs->del_from_lru_notify(bo);
>> +}
>> +
>> +static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>>   {
>>       struct ttm_device *bdev = bo->bdev;
>>   @@ -98,7 +108,7 @@ void ttm_bo_move_to_lru_tail(struct 
>> ttm_buffer_object *bo,
>>           dma_resv_assert_held(bo->base.resv);
>>         if (bo->pin_count) {
>> -        ttm_bo_del_from_lru(bo);
>> +        ttm_bo_move_to_pinned(bo);
>>           return;
>>       }
>>   @@ -339,7 +349,7 @@ static int ttm_bo_cleanup_refs(struct 
>> ttm_buffer_object *bo,
>>           return ret;
>>       }
>>   -    ttm_bo_del_from_lru(bo);
>> +    ttm_bo_move_to_pinned(bo);
>>       list_del_init(&bo->ddestroy);
>>       spin_unlock(&bo->bdev->lru_lock);
>>       ttm_bo_cleanup_memtype_use(bo);
>> @@ -1154,7 +1164,7 @@ int ttm_bo_swapout(struct ttm_buffer_object 
>> *bo, struct ttm_operation_ctx *ctx,
>>           return 0;
>>       }
>>   -    ttm_bo_del_from_lru(bo);
>> +    ttm_bo_move_to_pinned(bo);
>>       /* TODO: Cleanup the locking */
>>       spin_unlock(&bo->bdev->lru_lock);
>>   diff --git a/drivers/gpu/drm/ttm/ttm_device.c 
>> b/drivers/gpu/drm/ttm/ttm_device.c
>> index 5f31acec3ad7..530a9c36be37 100644
>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>> @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, 
>> struct ttm_device_funcs *funcs,
>>       INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
>>       spin_lock_init(&bdev->lru_lock);
>>       INIT_LIST_HEAD(&bdev->ddestroy);
>> +    INIT_LIST_HEAD(&bdev->pinned);
>>       bdev->dev_mapping = mapping;
>>       mutex_lock(&ttm_global_mutex);
>>       list_add_tail(&bdev->device_list, &glob->device_list);
>> diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
>> index cd592f8e941b..03fb44d061e0 100644
>> --- a/include/drm/ttm/ttm_device.h
>> +++ b/include/drm/ttm/ttm_device.h
>> @@ -265,6 +265,7 @@ struct ttm_device {
>>        */
>>       spinlock_t lru_lock;
>>       struct list_head ddestroy;
>> +    struct list_head pinned;
>>         /*
>>        * Protected by load / firstopen / lastclose /unload sync.
>
Christian König Aug. 30, 2021, 4:51 p.m. UTC | #3
Am 30.08.21 um 16:16 schrieb Andrey Grodzovsky:
>
> On 2021-08-30 4:58 a.m., Christian König wrote:
>> Am 27.08.21 um 22:39 schrieb Andrey Grodzovsky:
>>> This list will be used to capture all non VRAM BOs not
>>> on LRU so when device is hot unplugged we can iterate
>>> the list and unmap DMA mappings before device is removed.
>>>
>>> v2: Reanme function to ttm_bo_move_to_pinned
>>> v3: Move the pinned list to ttm device
>>
>> As far as I can see there is not list_del() remaining. So this won't 
>> work correctly.
>
>
> It's in ttm_bo_release, there was no code change there hence it's not 
> captured in the patch.

Ah! So you keep the logic as is there. Sorry totally missed that.

In this case the patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>

Can you push this to drm-misc-next?

Thanks,
Christian.

>
> Andrey
>
>
>>
>> I suggest to rather rebase on top of the stuff I'm working on for a 
>> while to move the LRU into the resource instead.
>>
>> Just send out the latest patch set of this with you in CC.
>>
>> Christian.
>>
>>>
>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>> Suggested-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   drivers/gpu/drm/ttm/ttm_bo.c     | 18 ++++++++++++++----
>>>   drivers/gpu/drm/ttm/ttm_device.c |  1 +
>>>   include/drm/ttm/ttm_device.h     |  1 +
>>>   3 files changed, 16 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
>>> b/drivers/gpu/drm/ttm/ttm_bo.c
>>> index 1b950b45cf4b..1fedd0eb67ba 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>> @@ -69,7 +69,17 @@ static void ttm_bo_mem_space_debug(struct 
>>> ttm_buffer_object *bo,
>>>       }
>>>   }
>>>   -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>>> +static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object *bo)
>>> +{
>>> +    struct ttm_device *bdev = bo->bdev;
>>> +
>>> +    list_move_tail(&bo->lru, &bdev->pinned);
>>> +
>>> +    if (bdev->funcs->del_from_lru_notify)
>>> +        bdev->funcs->del_from_lru_notify(bo);
>>> +}
>>> +
>>> +static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>>>   {
>>>       struct ttm_device *bdev = bo->bdev;
>>>   @@ -98,7 +108,7 @@ void ttm_bo_move_to_lru_tail(struct 
>>> ttm_buffer_object *bo,
>>>           dma_resv_assert_held(bo->base.resv);
>>>         if (bo->pin_count) {
>>> -        ttm_bo_del_from_lru(bo);
>>> +        ttm_bo_move_to_pinned(bo);
>>>           return;
>>>       }
>>>   @@ -339,7 +349,7 @@ static int ttm_bo_cleanup_refs(struct 
>>> ttm_buffer_object *bo,
>>>           return ret;
>>>       }
>>>   -    ttm_bo_del_from_lru(bo);
>>> +    ttm_bo_move_to_pinned(bo);
>>>       list_del_init(&bo->ddestroy);
>>>       spin_unlock(&bo->bdev->lru_lock);
>>>       ttm_bo_cleanup_memtype_use(bo);
>>> @@ -1154,7 +1164,7 @@ int ttm_bo_swapout(struct ttm_buffer_object 
>>> *bo, struct ttm_operation_ctx *ctx,
>>>           return 0;
>>>       }
>>>   -    ttm_bo_del_from_lru(bo);
>>> +    ttm_bo_move_to_pinned(bo);
>>>       /* TODO: Cleanup the locking */
>>>       spin_unlock(&bo->bdev->lru_lock);
>>>   diff --git a/drivers/gpu/drm/ttm/ttm_device.c 
>>> b/drivers/gpu/drm/ttm/ttm_device.c
>>> index 5f31acec3ad7..530a9c36be37 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>>> @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, 
>>> struct ttm_device_funcs *funcs,
>>>       INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
>>>       spin_lock_init(&bdev->lru_lock);
>>>       INIT_LIST_HEAD(&bdev->ddestroy);
>>> +    INIT_LIST_HEAD(&bdev->pinned);
>>>       bdev->dev_mapping = mapping;
>>>       mutex_lock(&ttm_global_mutex);
>>>       list_add_tail(&bdev->device_list, &glob->device_list);
>>> diff --git a/include/drm/ttm/ttm_device.h 
>>> b/include/drm/ttm/ttm_device.h
>>> index cd592f8e941b..03fb44d061e0 100644
>>> --- a/include/drm/ttm/ttm_device.h
>>> +++ b/include/drm/ttm/ttm_device.h
>>> @@ -265,6 +265,7 @@ struct ttm_device {
>>>        */
>>>       spinlock_t lru_lock;
>>>       struct list_head ddestroy;
>>> +    struct list_head pinned;
>>>         /*
>>>        * Protected by load / firstopen / lastclose /unload sync.
>>
Andrey Grodzovsky Aug. 30, 2021, 5:02 p.m. UTC | #4
On 2021-08-30 12:51 p.m., Christian König wrote:
> Am 30.08.21 um 16:16 schrieb Andrey Grodzovsky:
>>
>> On 2021-08-30 4:58 a.m., Christian König wrote:
>>> Am 27.08.21 um 22:39 schrieb Andrey Grodzovsky:
>>>> This list will be used to capture all non VRAM BOs not
>>>> on LRU so when device is hot unplugged we can iterate
>>>> the list and unmap DMA mappings before device is removed.
>>>>
>>>> v2: Reanme function to ttm_bo_move_to_pinned
>>>> v3: Move the pinned list to ttm device
>>>
>>> As far as I can see there is not list_del() remaining. So this won't 
>>> work correctly.
>>
>>
>> It's in ttm_bo_release, there was no code change there hence it's not 
>> captured in the patch.
>
> Ah! So you keep the logic as is there. Sorry totally missed that.
>
> In this case the patch is Reviewed-by: Christian König 
> <christian.koenig@amd.com>
>
> Can you push this to drm-misc-next?
>
> Thanks,
> Christian.


I think It's supposed to go on top of your changes you mention here 
which are not pushed yet.
I will need to apply all the patches on top of yours and retest (I was 
doing everything in amd-staging-drm-next)
until now.

Andrey


>
>>
>> Andrey
>>
>>
>>>
>>> I suggest to rather rebase on top of the stuff I'm working on for a 
>>> while to move the LRU into the resource instead.
>>>
>>> Just send out the latest patch set of this with you in CC.
>>>
>>> Christian.
>>>
>>>>
>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>> Suggested-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/ttm/ttm_bo.c     | 18 ++++++++++++++----
>>>>   drivers/gpu/drm/ttm/ttm_device.c |  1 +
>>>>   include/drm/ttm/ttm_device.h     |  1 +
>>>>   3 files changed, 16 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
>>>> b/drivers/gpu/drm/ttm/ttm_bo.c
>>>> index 1b950b45cf4b..1fedd0eb67ba 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>> @@ -69,7 +69,17 @@ static void ttm_bo_mem_space_debug(struct 
>>>> ttm_buffer_object *bo,
>>>>       }
>>>>   }
>>>>   -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>>>> +static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object 
>>>> *bo)
>>>> +{
>>>> +    struct ttm_device *bdev = bo->bdev;
>>>> +
>>>> +    list_move_tail(&bo->lru, &bdev->pinned);
>>>> +
>>>> +    if (bdev->funcs->del_from_lru_notify)
>>>> +        bdev->funcs->del_from_lru_notify(bo);
>>>> +}
>>>> +
>>>> +static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>>>>   {
>>>>       struct ttm_device *bdev = bo->bdev;
>>>>   @@ -98,7 +108,7 @@ void ttm_bo_move_to_lru_tail(struct 
>>>> ttm_buffer_object *bo,
>>>>           dma_resv_assert_held(bo->base.resv);
>>>>         if (bo->pin_count) {
>>>> -        ttm_bo_del_from_lru(bo);
>>>> +        ttm_bo_move_to_pinned(bo);
>>>>           return;
>>>>       }
>>>>   @@ -339,7 +349,7 @@ static int ttm_bo_cleanup_refs(struct 
>>>> ttm_buffer_object *bo,
>>>>           return ret;
>>>>       }
>>>>   -    ttm_bo_del_from_lru(bo);
>>>> +    ttm_bo_move_to_pinned(bo);
>>>>       list_del_init(&bo->ddestroy);
>>>>       spin_unlock(&bo->bdev->lru_lock);
>>>>       ttm_bo_cleanup_memtype_use(bo);
>>>> @@ -1154,7 +1164,7 @@ int ttm_bo_swapout(struct ttm_buffer_object 
>>>> *bo, struct ttm_operation_ctx *ctx,
>>>>           return 0;
>>>>       }
>>>>   -    ttm_bo_del_from_lru(bo);
>>>> +    ttm_bo_move_to_pinned(bo);
>>>>       /* TODO: Cleanup the locking */
>>>>       spin_unlock(&bo->bdev->lru_lock);
>>>>   diff --git a/drivers/gpu/drm/ttm/ttm_device.c 
>>>> b/drivers/gpu/drm/ttm/ttm_device.c
>>>> index 5f31acec3ad7..530a9c36be37 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>>>> @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, 
>>>> struct ttm_device_funcs *funcs,
>>>>       INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
>>>>       spin_lock_init(&bdev->lru_lock);
>>>>       INIT_LIST_HEAD(&bdev->ddestroy);
>>>> +    INIT_LIST_HEAD(&bdev->pinned);
>>>>       bdev->dev_mapping = mapping;
>>>>       mutex_lock(&ttm_global_mutex);
>>>>       list_add_tail(&bdev->device_list, &glob->device_list);
>>>> diff --git a/include/drm/ttm/ttm_device.h 
>>>> b/include/drm/ttm/ttm_device.h
>>>> index cd592f8e941b..03fb44d061e0 100644
>>>> --- a/include/drm/ttm/ttm_device.h
>>>> +++ b/include/drm/ttm/ttm_device.h
>>>> @@ -265,6 +265,7 @@ struct ttm_device {
>>>>        */
>>>>       spinlock_t lru_lock;
>>>>       struct list_head ddestroy;
>>>> +    struct list_head pinned;
>>>>         /*
>>>>        * Protected by load / firstopen / lastclose /unload sync.
>>>
>
Christian König Aug. 30, 2021, 5:05 p.m. UTC | #5
Am 30.08.21 um 19:02 schrieb Andrey Grodzovsky:
>
> On 2021-08-30 12:51 p.m., Christian König wrote:
>> Am 30.08.21 um 16:16 schrieb Andrey Grodzovsky:
>>>
>>> On 2021-08-30 4:58 a.m., Christian König wrote:
>>>> Am 27.08.21 um 22:39 schrieb Andrey Grodzovsky:
>>>>> This list will be used to capture all non VRAM BOs not
>>>>> on LRU so when device is hot unplugged we can iterate
>>>>> the list and unmap DMA mappings before device is removed.
>>>>>
>>>>> v2: Reanme function to ttm_bo_move_to_pinned
>>>>> v3: Move the pinned list to ttm device
>>>>
>>>> As far as I can see there is not list_del() remaining. So this 
>>>> won't work correctly.
>>>
>>>
>>> It's in ttm_bo_release, there was no code change there hence it's 
>>> not captured in the patch.
>>
>> Ah! So you keep the logic as is there. Sorry totally missed that.
>>
>> In this case the patch is Reviewed-by: Christian König 
>> <christian.koenig@amd.com>
>>
>> Can you push this to drm-misc-next?
>>
>> Thanks,
>> Christian.
>
>
> I think It's supposed to go on top of your changes you mention here 
> which are not pushed yet.
> I will need to apply all the patches on top of yours and retest (I was 
> doing everything in amd-staging-drm-next)
> until now.

Works for me as well. Alternatively you can just push this patch here to 
drm-misc-next so that I can rebase on top and merge the rest through 
amd-staging-drm-next.

The patch pushed to drm-misc-next should automatically fall out when 
Alex rebases his stuff on upstream the next time.

Christian.

>
> Andrey
>
>
>>
>>>
>>> Andrey
>>>
>>>
>>>>
>>>> I suggest to rather rebase on top of the stuff I'm working on for a 
>>>> while to move the LRU into the resource instead.
>>>>
>>>> Just send out the latest patch set of this with you in CC.
>>>>
>>>> Christian.
>>>>
>>>>>
>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>>> Suggested-by: Christian König <christian.koenig@amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/ttm/ttm_bo.c     | 18 ++++++++++++++----
>>>>>   drivers/gpu/drm/ttm/ttm_device.c |  1 +
>>>>>   include/drm/ttm/ttm_device.h     |  1 +
>>>>>   3 files changed, 16 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
>>>>> b/drivers/gpu/drm/ttm/ttm_bo.c
>>>>> index 1b950b45cf4b..1fedd0eb67ba 100644
>>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>>> @@ -69,7 +69,17 @@ static void ttm_bo_mem_space_debug(struct 
>>>>> ttm_buffer_object *bo,
>>>>>       }
>>>>>   }
>>>>>   -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>>>>> +static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object 
>>>>> *bo)
>>>>> +{
>>>>> +    struct ttm_device *bdev = bo->bdev;
>>>>> +
>>>>> +    list_move_tail(&bo->lru, &bdev->pinned);
>>>>> +
>>>>> +    if (bdev->funcs->del_from_lru_notify)
>>>>> +        bdev->funcs->del_from_lru_notify(bo);
>>>>> +}
>>>>> +
>>>>> +static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>>>>>   {
>>>>>       struct ttm_device *bdev = bo->bdev;
>>>>>   @@ -98,7 +108,7 @@ void ttm_bo_move_to_lru_tail(struct 
>>>>> ttm_buffer_object *bo,
>>>>>           dma_resv_assert_held(bo->base.resv);
>>>>>         if (bo->pin_count) {
>>>>> -        ttm_bo_del_from_lru(bo);
>>>>> +        ttm_bo_move_to_pinned(bo);
>>>>>           return;
>>>>>       }
>>>>>   @@ -339,7 +349,7 @@ static int ttm_bo_cleanup_refs(struct 
>>>>> ttm_buffer_object *bo,
>>>>>           return ret;
>>>>>       }
>>>>>   -    ttm_bo_del_from_lru(bo);
>>>>> +    ttm_bo_move_to_pinned(bo);
>>>>>       list_del_init(&bo->ddestroy);
>>>>>       spin_unlock(&bo->bdev->lru_lock);
>>>>>       ttm_bo_cleanup_memtype_use(bo);
>>>>> @@ -1154,7 +1164,7 @@ int ttm_bo_swapout(struct ttm_buffer_object 
>>>>> *bo, struct ttm_operation_ctx *ctx,
>>>>>           return 0;
>>>>>       }
>>>>>   -    ttm_bo_del_from_lru(bo);
>>>>> +    ttm_bo_move_to_pinned(bo);
>>>>>       /* TODO: Cleanup the locking */
>>>>>       spin_unlock(&bo->bdev->lru_lock);
>>>>>   diff --git a/drivers/gpu/drm/ttm/ttm_device.c 
>>>>> b/drivers/gpu/drm/ttm/ttm_device.c
>>>>> index 5f31acec3ad7..530a9c36be37 100644
>>>>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>>>>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>>>>> @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, 
>>>>> struct ttm_device_funcs *funcs,
>>>>>       INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
>>>>>       spin_lock_init(&bdev->lru_lock);
>>>>>       INIT_LIST_HEAD(&bdev->ddestroy);
>>>>> +    INIT_LIST_HEAD(&bdev->pinned);
>>>>>       bdev->dev_mapping = mapping;
>>>>>       mutex_lock(&ttm_global_mutex);
>>>>>       list_add_tail(&bdev->device_list, &glob->device_list);
>>>>> diff --git a/include/drm/ttm/ttm_device.h 
>>>>> b/include/drm/ttm/ttm_device.h
>>>>> index cd592f8e941b..03fb44d061e0 100644
>>>>> --- a/include/drm/ttm/ttm_device.h
>>>>> +++ b/include/drm/ttm/ttm_device.h
>>>>> @@ -265,6 +265,7 @@ struct ttm_device {
>>>>>        */
>>>>>       spinlock_t lru_lock;
>>>>>       struct list_head ddestroy;
>>>>> +    struct list_head pinned;
>>>>>         /*
>>>>>        * Protected by load / firstopen / lastclose /unload sync.
>>>>
>>
Andrey Grodzovsky Aug. 30, 2021, 5:15 p.m. UTC | #6
On 2021-08-30 1:05 p.m., Christian König wrote:
>
>
> Am 30.08.21 um 19:02 schrieb Andrey Grodzovsky:
>>
>> On 2021-08-30 12:51 p.m., Christian König wrote:
>>> Am 30.08.21 um 16:16 schrieb Andrey Grodzovsky:
>>>>
>>>> On 2021-08-30 4:58 a.m., Christian König wrote:
>>>>> Am 27.08.21 um 22:39 schrieb Andrey Grodzovsky:
>>>>>> This list will be used to capture all non VRAM BOs not
>>>>>> on LRU so when device is hot unplugged we can iterate
>>>>>> the list and unmap DMA mappings before device is removed.
>>>>>>
>>>>>> v2: Reanme function to ttm_bo_move_to_pinned
>>>>>> v3: Move the pinned list to ttm device
>>>>>
>>>>> As far as I can see there is not list_del() remaining. So this 
>>>>> won't work correctly.
>>>>
>>>>
>>>> It's in ttm_bo_release, there was no code change there hence it's 
>>>> not captured in the patch.
>>>
>>> Ah! So you keep the logic as is there. Sorry totally missed that.
>>>
>>> In this case the patch is Reviewed-by: Christian König 
>>> <christian.koenig@amd.com>
>>>
>>> Can you push this to drm-misc-next?
>>>
>>> Thanks,
>>> Christian.
>>
>>
>> I think It's supposed to go on top of your changes you mention here 
>> which are not pushed yet.
>> I will need to apply all the patches on top of yours and retest (I 
>> was doing everything in amd-staging-drm-next)
>> until now.
>
> Works for me as well. Alternatively you can just push this patch here 
> to drm-misc-next so that I can rebase on top and merge the rest 
> through amd-staging-drm-next.
>
> The patch pushed to drm-misc-next should automatically fall out when 
> Alex rebases his stuff on upstream the next time.
>
> Christian.


So i can both push this specific patch to drm-misc-next and also push 
the entire 4 patch series to amd-stagin-drm-next (after rest of the 
patches RBed)?

Andrey


>
>>
>> Andrey
>>
>>
>>>
>>>>
>>>> Andrey
>>>>
>>>>
>>>>>
>>>>> I suggest to rather rebase on top of the stuff I'm working on for 
>>>>> a while to move the LRU into the resource instead.
>>>>>
>>>>> Just send out the latest patch set of this with you in CC.
>>>>>
>>>>> Christian.
>>>>>
>>>>>>
>>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>>>> Suggested-by: Christian König <christian.koenig@amd.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/ttm/ttm_bo.c     | 18 ++++++++++++++----
>>>>>>   drivers/gpu/drm/ttm/ttm_device.c |  1 +
>>>>>>   include/drm/ttm/ttm_device.h     |  1 +
>>>>>>   3 files changed, 16 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
>>>>>> b/drivers/gpu/drm/ttm/ttm_bo.c
>>>>>> index 1b950b45cf4b..1fedd0eb67ba 100644
>>>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>>>> @@ -69,7 +69,17 @@ static void ttm_bo_mem_space_debug(struct 
>>>>>> ttm_buffer_object *bo,
>>>>>>       }
>>>>>>   }
>>>>>>   -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>>>>>> +static inline void ttm_bo_move_to_pinned(struct 
>>>>>> ttm_buffer_object *bo)
>>>>>> +{
>>>>>> +    struct ttm_device *bdev = bo->bdev;
>>>>>> +
>>>>>> +    list_move_tail(&bo->lru, &bdev->pinned);
>>>>>> +
>>>>>> +    if (bdev->funcs->del_from_lru_notify)
>>>>>> +        bdev->funcs->del_from_lru_notify(bo);
>>>>>> +}
>>>>>> +
>>>>>> +static inline void ttm_bo_del_from_lru(struct ttm_buffer_object 
>>>>>> *bo)
>>>>>>   {
>>>>>>       struct ttm_device *bdev = bo->bdev;
>>>>>>   @@ -98,7 +108,7 @@ void ttm_bo_move_to_lru_tail(struct 
>>>>>> ttm_buffer_object *bo,
>>>>>>           dma_resv_assert_held(bo->base.resv);
>>>>>>         if (bo->pin_count) {
>>>>>> -        ttm_bo_del_from_lru(bo);
>>>>>> +        ttm_bo_move_to_pinned(bo);
>>>>>>           return;
>>>>>>       }
>>>>>>   @@ -339,7 +349,7 @@ static int ttm_bo_cleanup_refs(struct 
>>>>>> ttm_buffer_object *bo,
>>>>>>           return ret;
>>>>>>       }
>>>>>>   -    ttm_bo_del_from_lru(bo);
>>>>>> +    ttm_bo_move_to_pinned(bo);
>>>>>>       list_del_init(&bo->ddestroy);
>>>>>>       spin_unlock(&bo->bdev->lru_lock);
>>>>>>       ttm_bo_cleanup_memtype_use(bo);
>>>>>> @@ -1154,7 +1164,7 @@ int ttm_bo_swapout(struct ttm_buffer_object 
>>>>>> *bo, struct ttm_operation_ctx *ctx,
>>>>>>           return 0;
>>>>>>       }
>>>>>>   -    ttm_bo_del_from_lru(bo);
>>>>>> +    ttm_bo_move_to_pinned(bo);
>>>>>>       /* TODO: Cleanup the locking */
>>>>>>       spin_unlock(&bo->bdev->lru_lock);
>>>>>>   diff --git a/drivers/gpu/drm/ttm/ttm_device.c 
>>>>>> b/drivers/gpu/drm/ttm/ttm_device.c
>>>>>> index 5f31acec3ad7..530a9c36be37 100644
>>>>>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>>>>>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>>>>>> @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, 
>>>>>> struct ttm_device_funcs *funcs,
>>>>>>       INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
>>>>>>       spin_lock_init(&bdev->lru_lock);
>>>>>>       INIT_LIST_HEAD(&bdev->ddestroy);
>>>>>> +    INIT_LIST_HEAD(&bdev->pinned);
>>>>>>       bdev->dev_mapping = mapping;
>>>>>>       mutex_lock(&ttm_global_mutex);
>>>>>>       list_add_tail(&bdev->device_list, &glob->device_list);
>>>>>> diff --git a/include/drm/ttm/ttm_device.h 
>>>>>> b/include/drm/ttm/ttm_device.h
>>>>>> index cd592f8e941b..03fb44d061e0 100644
>>>>>> --- a/include/drm/ttm/ttm_device.h
>>>>>> +++ b/include/drm/ttm/ttm_device.h
>>>>>> @@ -265,6 +265,7 @@ struct ttm_device {
>>>>>>        */
>>>>>>       spinlock_t lru_lock;
>>>>>>       struct list_head ddestroy;
>>>>>> +    struct list_head pinned;
>>>>>>         /*
>>>>>>        * Protected by load / firstopen / lastclose /unload sync.
>>>>>
>>>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 1b950b45cf4b..1fedd0eb67ba 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -69,7 +69,17 @@  static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
 	}
 }
 
-static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
+static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object *bo)
+{
+	struct ttm_device *bdev = bo->bdev;
+
+	list_move_tail(&bo->lru, &bdev->pinned);
+
+	if (bdev->funcs->del_from_lru_notify)
+		bdev->funcs->del_from_lru_notify(bo);
+}
+
+static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
 {
 	struct ttm_device *bdev = bo->bdev;
 
@@ -98,7 +108,7 @@  void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
 		dma_resv_assert_held(bo->base.resv);
 
 	if (bo->pin_count) {
-		ttm_bo_del_from_lru(bo);
+		ttm_bo_move_to_pinned(bo);
 		return;
 	}
 
@@ -339,7 +349,7 @@  static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
 		return ret;
 	}
 
-	ttm_bo_del_from_lru(bo);
+	ttm_bo_move_to_pinned(bo);
 	list_del_init(&bo->ddestroy);
 	spin_unlock(&bo->bdev->lru_lock);
 	ttm_bo_cleanup_memtype_use(bo);
@@ -1154,7 +1164,7 @@  int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
 		return 0;
 	}
 
-	ttm_bo_del_from_lru(bo);
+	ttm_bo_move_to_pinned(bo);
 	/* TODO: Cleanup the locking */
 	spin_unlock(&bo->bdev->lru_lock);
 
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index 5f31acec3ad7..530a9c36be37 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -208,6 +208,7 @@  int ttm_device_init(struct ttm_device *bdev, struct ttm_device_funcs *funcs,
 	INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue);
 	spin_lock_init(&bdev->lru_lock);
 	INIT_LIST_HEAD(&bdev->ddestroy);
+	INIT_LIST_HEAD(&bdev->pinned);
 	bdev->dev_mapping = mapping;
 	mutex_lock(&ttm_global_mutex);
 	list_add_tail(&bdev->device_list, &glob->device_list);
diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
index cd592f8e941b..03fb44d061e0 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -265,6 +265,7 @@  struct ttm_device {
 	 */
 	spinlock_t lru_lock;
 	struct list_head ddestroy;
+	struct list_head pinned;
 
 	/*
 	 * Protected by load / firstopen / lastclose /unload sync.