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

[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?

/Thomas


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