[Intel-gfx] [PATCH 1/6] dma-buf: add dynamic DMA-buf handling v12

[Christian König]

Am 25.06.19 um 18:05 schrieb Daniel Vetter:
> On Tue, Jun 25, 2019 at 02:46:49PM +0200, Christian König wrote:
>> On the exporter side we add optional explicit pinning callbacks. If those
>> callbacks are implemented the framework no longer caches sg tables and the
>> map/unmap callbacks are always called with the lock of the reservation object
>> held.
>>
>> On the importer side we add an optional invalidate callback. This callback is
>> used by the exporter to inform the importers that their mappings should be
>> destroyed as soon as possible.
>>
>> This allows the exporter to provide the mappings without the need to pin
>> the backing store.
>>
>> v2: don't try to invalidate mappings when the callback is NULL,
>>      lock the reservation obj while using the attachments,
>>      add helper to set the callback
>> v3: move flag for invalidation support into the DMA-buf,
>>      use new attach_info structure to set the callback
>> v4: use importer_priv field instead of mangling exporter priv.
>> v5: drop invalidation_supported flag
>> v6: squash together with pin/unpin changes
>> v7: pin/unpin takes an attachment now
>> v8: nuke dma_buf_attachment_(map|unmap)_locked,
>>      everything is now handled backward compatible
>> v9: always cache when export/importer don't agree on dynamic handling
>> v10: minimal style cleanup
>> v11: drop automatically re-entry avoidance
>> v12: rename callback to move_notify
>>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
> One thing I've forgotten, just stumbled over ttm_bo->moving. For pinned
> buffer sharing that's not needed, and I think for dynamic buffer sharing
> it's also not going to be the primary requirement. But I think there's two
> reasons we should maybe look into moving that from ttm_bo to resv_obj:

That is already part of the resv_obj. The difference is that radeon is 
overwriting the one in the resv_obj during CS while amdgpu isn't.

So for amdgpu we keep an extra copy in ttm_bo->moving to keep the page 
fault handler from unnecessary waiting for a fence in radeon.

>
> - You sound like you want to use this a lot more, even internally in
>    amdgpu. For that I do think the sepearate dma_fence just to make sure
>    the buffer is accessible will be needed in resv_obj.
>
> - Once we have ->moving I think there's some good chances to extract a bit
>    of the eviction/pipeline bo move boilerplate from ttm, and maybe use it
>    in other drivers. i915 could already make use of this in upstream, since
>    we already pipeline get_pages and clflush of buffers. Ofc once we have
>    vram support, even more useful.

I actually indeed wanted to add more stuff to the reservation object 
implementation, like finally cleaning up the distinction of readers/writers.

And cleaning up the fence removal hack we have in the KFD for freed up 
BOs. That would also allow for getting rid of this in the long term.

Christian.

>
> And doing that slight semantic change is much easier once we only have a
> few dynamic exporters/importers. And since it's a pure opt-in optimization
> (you can always fall back to the exclusive fence) it should be easy to
> roll out.
>
> Thoughts about moving ttm_bo->moving to resv_obj? Ofc strictly only as a
> follow up. Plus maybe with a clearer name :-)
>
> Cheers, Daniel
>