Re: [PATCH 02/12] dma-buf: add explicit buffer pinning v2

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 30.04.19 um 15:59 schrieb Daniel Vetter:
> On Tue, Apr 30, 2019 at 03:42:22PM +0200, Christian König wrote:
>> Am 29.04.19 um 10:40 schrieb Daniel Vetter:
>>> On Fri, Apr 26, 2019 at 02:36:28PM +0200, Christian König wrote:
>>>> [SNIP]
>>>> +/**
>>>> + * dma_buf_pin - Lock down the DMA-buf
>>>> + *
>>>> + * @dmabuf:	[in]	DMA-buf to lock down.
>>>> + *
>>>> + * Returns:
>>>> + * 0 on success, negative error code on failure.
>>>> + */
>>>> +int dma_buf_pin(struct dma_buf *dmabuf)
>>> I think this should be on the attachment, not on the buffer. Or is the
>>> idea that a pin is for the entire buffer, and all subsequent
>>> dma_buf_map_attachment must work for all attachments? I think this matters
>>> for sufficiently contrived p2p scenarios.
>> This is indeed for the DMA-buf, cause we are pinning the underlying backing
>> store and not just one attachment.
> You can't move the buffer either way, so from that point there's no
> difference. I more meant from an account/api point of view of whether it's
> ok to pin a buffer you haven't even attached to yet. From the code it
> seems like first you always want to attach, hence it would make sense to
> put the pin/unpin onto the attachment. That might also help with
> debugging, we could check whether everyone balances their pin/unpin
> correctly (instead of just counting for the overall dma-buf).
>
> There's also a slight semantic difference between a pin on an attachment
> (which would mean this attachment is always going to be mappable and wont
> move around), whereas a pin on the entire dma-buf means the entire dma-buf
> and all it's attachment must always be mappable. Otoh, global pin is
> probably easier, you just need to check all current attachments again
> whether they still work or whether you might need to move the buffer
> around to a more suitable place (e.g. if you not all can do p2p it needs
> to go into system ram before it's pinned).
>
> For the backing storage you only want one per-bo ->pinned_count, that's
> clear, my suggestion was more about whether having more information about
> who's pinning is useful. Exporters can always just ignore that added
> information.
>
>> Key point is I want to call this in the exporter itself in the long run.
>> E.g. that the exporter stops working with its internal special handling
>> functions and uses this one instead.
> Hm, not exactly following why the exporter needs to call
> dma_buf_pin/unpin, instead of whatever is used to implement that. Or do
> you mean that you want a ->pinned_count in dma_buf itself, so that there's
> only one book-keeping for this?

Yes, exactly that is one of the final goals of this.

We could of course use the attachment here, but that would disqualify 
the exporter calling this directly without an attachment.

Regards,
Christian.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel