[PATCH v2 09/15] drm/ttm, drm/amdgpu: Allow the driver some control over swapping

[Christian König]

Am 18.05.21 um 18:07 schrieb Thomas Hellström:
>
> On 5/18/21 5:42 PM, Christian König wrote:
>> Am 18.05.21 um 17:38 schrieb Thomas Hellström:
>>>
>>> On 5/18/21 5:28 PM, Christian König wrote:
>>>> Am 18.05.21 um 17:20 schrieb Thomas Hellström:
>>>>>
>>>>> On 5/18/21 5:18 PM, Christian König wrote:
>>>>>>
>>>>>>
>>>>>> Am 18.05.21 um 17:15 schrieb Thomas Hellström:
>>>>>>>
>>>>>>> On 5/18/21 10:26 AM, Thomas Hellström wrote:
>>>>>>>> We are calling the eviction_valuable driver callback at 
>>>>>>>> eviction time to
>>>>>>>> determine whether we actually can evict a buffer object.
>>>>>>>> The upcoming i915 TTM backend needs the same functionality for 
>>>>>>>> swapout,
>>>>>>>> and that might actually be beneficial to other drivers as well.
>>>>>>>>
>>>>>>>> Add an eviction_valuable call also in the swapout path. Try to 
>>>>>>>> keep the
>>>>>>>> current behaviour for all drivers by returning true if the 
>>>>>>>> buffer object
>>>>>>>> is already in the TTM_PL_SYSTEM placement. We change behaviour 
>>>>>>>> for the
>>>>>>>> case where a buffer object is in a TT backed placement when 
>>>>>>>> swapped out,
>>>>>>>> in which case the drivers normal eviction_valuable path is run.
>>>>>>>>
>>>>>>>> Finally export ttm_tt_unpopulate() and don't swap out bos
>>>>>>>> that are not populated. This allows a driver to purge a bo at
>>>>>>>> swapout time if its content is no longer valuable rather than to
>>>>>>>> have TTM swap the contents out.
>>>>>>>>
>>>>>>>> Cc: Christian König <christian.koenig at amd.com>
>>>>>>>> Signed-off-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>
>>>>>>>
>>>>>>> Christian,
>>>>>>>
>>>>>>> Here we have a ttm_tt_unpopulate() export as well at the end. I 
>>>>>>> figure you will push back on that one. What we really need is a 
>>>>>>> functionality to just drop the bo contents and end up in system 
>>>>>>> memory unpopulated. Should I perhaps add a utility function to 
>>>>>>> do that instead? like ttm_bo_purge()?
>>>>>>
>>>>>> We already have that. Just call ttm_bo_validate() without any 
>>>>>> place to put the buffer.
>>>>>>
>>>>>> See how ttm_bo_pipeline_gutting() is used.
>>>>>>
>>>>>> Christian.
>>>>>
>>>>> OK, so is that reentrant from the move() or swap_notify() callback.
>>>>
>>>> That sounds like a design bug to me since you should never need to 
>>>> do this.
>>>>
>>>> When you want to destroy the backing store of a buffer during 
>>>> eviction you should just do this by returning an empty placement 
>>>> from the evict_flags callback.
>>>
>>> So this is for the functionality where the user has indicated that 
>>> the contents is no longer of value, but the buffer itself
>>> is cached until evicted or swapped out for performance reasons. So 
>>> the above would work for eviction, but what about swapout. Could we 
>>> add some similar functionality there?
>>
>> Amdgpu has the same functionality and you don't need to handle swap 
>> at all.
>>
>> Just return from the evict_flags that you want to drop the backing 
>> store as soon as the BO leaves the GTT domain.
>
> Hmm, the pipeline_gutting function seems ok, but overly complex if the 
> bo is already idle, Am I allowed to optimize it slightly for the 
> latter case?

Yeah, sure. We just never hat that use case so far.

Christian.

>
> /Thomas
>
>
>>
>> Christian.
>>
>>>
>>> /Thomas
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> /Thomas
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Thomas
>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>