From: "Christian König" <ckoenig.leichtzumerken@gmail.com> To: Chris Wilson <chris@chris-wilson.co.uk>, linaro-mm-sig@lists.linaro.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 1/5] dma-buf: add optional invalidate_mappings callback v2 Date: Fri, 16 Mar 2018 15:22:32 +0100 [thread overview] Message-ID: <21879456-db47-589c-b5e2-dfe8333d9e4c@gmail.com> (raw) In-Reply-To: <152120831102.25315.4326885184264378830@mail.alporthouse.com> Am 16.03.2018 um 14:51 schrieb Chris Wilson: > Quoting Christian König (2018-03-16 13:20:45) >> @@ -326,6 +338,29 @@ struct dma_buf_attachment { >> struct device *dev; >> struct list_head node; >> void *priv; >> + >> + /** >> + * @invalidate_mappings: >> + * >> + * Optional callback provided by the importer of the attachment which >> + * must be set before mappings are created. >> + * >> + * If provided the exporter can avoid pinning the backing store while >> + * mappings exists. > Hmm, no I don't think it avoids the pinning issue entirely. As it stands, > the importer doesn't have a page refcount and so they all rely on the > exporter keeping the dmabuf pages pinned while attached. What can happen > is that given the invalidate cb, the importers can revoke their > attachments, letting the exporter recover the pages/sg, and then start > again from scratch. Yes, exactly. The wording is just not 100% precise and I haven't found something better so far. > That also neatly answers what happens if not all importers provide an > invalidate cb, or fail, the dmabuf remains pinned and the exporter must > retreat. Yes, exactly as well. As soon as at least one importer says "I can't do this", we must fallback to the old behavior. >> + * >> + * The function is called with the lock of the reservation object >> + * associated with the dma_buf held and the mapping function must be >> + * called with this lock held as well. This makes sure that no mapping >> + * is created concurrently with an ongoing invalidation. >> + * >> + * After the callback all existing mappings are still valid until all >> + * fences in the dma_bufs reservation object are signaled, but should be >> + * destroyed by the importer as soon as possible. >> + * >> + * New mappings can be created immediately, but can't be used before the >> + * exclusive fence in the dma_bufs reservation object is signaled. >> + */ >> + void (*invalidate_mappings)(struct dma_buf_attachment *attach); > The intent is that the invalidate is synchronous and immediate, while > locked? We are looking at recursing back into the dma_buf functions to > remove each attachment from the invalidate cb (as well as waiting for > dma), won't that cause some nasty issues? No, with this idea invalidation is asynchronous. Already discussed that with Daniel as well and YES Daniel also already pointed out that I need to better document this. When the exporter calls invalidate_mappings() it just means that all importers can no longer use their sg tables for new submissions, old ones stay active. The existing sg tables are guaranteed to stay valid until all fences in the reservation object have signaled and the importer should also delay it's call to call dma_buf_unmap_attachment() until all the fences have signaled. When the importer has new work to do, e.g. wants to attach a new fence to the reservation object, it must grab a new sg table for that. The importer also needs to make sure that all new work touching the dma-buf doesn't start before the exclusive fence in the reservation object signals. This allows for full grown pipelining, e.g. the exporter can say I need to move the buffer for some operation. Then let the move operation wait for all existing fences in the reservation object and install the fence of the move operation as exclusive fence. The importer can then immediately grab a new sg table for the new location of the buffer and use it to prepare the next operation. Regards, Christian. > -Chris > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: "Christian König" <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> To: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>, linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw@public.gmane.org, linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Subject: Re: [PATCH 1/5] dma-buf: add optional invalidate_mappings callback v2 Date: Fri, 16 Mar 2018 15:22:32 +0100 [thread overview] Message-ID: <21879456-db47-589c-b5e2-dfe8333d9e4c@gmail.com> (raw) In-Reply-To: <152120831102.25315.4326885184264378830-M6iVdVfohj6unts5RBS2dVaTQe2KTcn/@public.gmane.org> Am 16.03.2018 um 14:51 schrieb Chris Wilson: > Quoting Christian König (2018-03-16 13:20:45) >> @@ -326,6 +338,29 @@ struct dma_buf_attachment { >> struct device *dev; >> struct list_head node; >> void *priv; >> + >> + /** >> + * @invalidate_mappings: >> + * >> + * Optional callback provided by the importer of the attachment which >> + * must be set before mappings are created. >> + * >> + * If provided the exporter can avoid pinning the backing store while >> + * mappings exists. > Hmm, no I don't think it avoids the pinning issue entirely. As it stands, > the importer doesn't have a page refcount and so they all rely on the > exporter keeping the dmabuf pages pinned while attached. What can happen > is that given the invalidate cb, the importers can revoke their > attachments, letting the exporter recover the pages/sg, and then start > again from scratch. Yes, exactly. The wording is just not 100% precise and I haven't found something better so far. > That also neatly answers what happens if not all importers provide an > invalidate cb, or fail, the dmabuf remains pinned and the exporter must > retreat. Yes, exactly as well. As soon as at least one importer says "I can't do this", we must fallback to the old behavior. >> + * >> + * The function is called with the lock of the reservation object >> + * associated with the dma_buf held and the mapping function must be >> + * called with this lock held as well. This makes sure that no mapping >> + * is created concurrently with an ongoing invalidation. >> + * >> + * After the callback all existing mappings are still valid until all >> + * fences in the dma_bufs reservation object are signaled, but should be >> + * destroyed by the importer as soon as possible. >> + * >> + * New mappings can be created immediately, but can't be used before the >> + * exclusive fence in the dma_bufs reservation object is signaled. >> + */ >> + void (*invalidate_mappings)(struct dma_buf_attachment *attach); > The intent is that the invalidate is synchronous and immediate, while > locked? We are looking at recursing back into the dma_buf functions to > remove each attachment from the invalidate cb (as well as waiting for > dma), won't that cause some nasty issues? No, with this idea invalidation is asynchronous. Already discussed that with Daniel as well and YES Daniel also already pointed out that I need to better document this. When the exporter calls invalidate_mappings() it just means that all importers can no longer use their sg tables for new submissions, old ones stay active. The existing sg tables are guaranteed to stay valid until all fences in the reservation object have signaled and the importer should also delay it's call to call dma_buf_unmap_attachment() until all the fences have signaled. When the importer has new work to do, e.g. wants to attach a new fence to the reservation object, it must grab a new sg table for that. The importer also needs to make sure that all new work touching the dma-buf doesn't start before the exclusive fence in the reservation object signals. This allows for full grown pipelining, e.g. the exporter can say I need to move the buffer for some operation. Then let the move operation wait for all existing fences in the reservation object and install the fence of the move operation as exclusive fence. The importer can then immediately grab a new sg table for the new location of the buffer and use it to prepare the next operation. Regards, Christian. > -Chris > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2018-03-16 14:22 UTC|newest] Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-03-16 13:20 RFC: unpinned DMA-buf exporting v2 Christian König 2018-03-16 13:20 ` Christian König 2018-03-16 13:20 ` [PATCH 1/5] dma-buf: add optional invalidate_mappings callback v2 Christian König 2018-03-16 13:20 ` Christian König 2018-03-16 13:51 ` Chris Wilson 2018-03-16 13:51 ` Chris Wilson 2018-03-16 14:22 ` Christian König [this message] 2018-03-16 14:22 ` Christian König 2018-03-19 15:53 ` Chris Wilson 2018-03-19 15:53 ` Chris Wilson 2018-03-19 16:23 ` Christian König 2018-03-19 16:23 ` Christian König 2018-03-20 7:44 ` [Linaro-mm-sig] " Daniel Vetter 2018-03-20 7:44 ` Daniel Vetter 2018-03-20 10:54 ` Christian König 2018-03-20 10:54 ` Christian König 2018-03-20 14:08 ` Daniel Vetter 2018-03-20 14:08 ` Daniel Vetter 2018-03-20 17:47 ` Christian König 2018-03-20 17:47 ` Christian König 2018-03-21 8:18 ` Daniel Vetter 2018-03-21 8:18 ` Daniel Vetter 2018-03-21 9:34 ` Christian König 2018-03-21 9:34 ` Christian König 2018-03-22 7:14 ` Daniel Vetter 2018-03-22 7:14 ` Daniel Vetter 2018-03-22 9:37 ` Christian König 2018-03-22 9:37 ` Christian König 2018-03-26 7:51 ` Daniel Vetter 2018-03-26 7:51 ` Daniel Vetter 2018-03-21 8:28 ` Daniel Vetter 2018-03-21 8:28 ` Daniel Vetter 2018-03-21 11:54 ` Christian König 2018-03-21 11:54 ` Christian König 2018-03-22 7:18 ` Daniel Vetter 2018-03-22 7:18 ` Daniel Vetter 2018-03-22 9:58 ` Christian König 2018-03-22 9:58 ` Christian König 2018-03-26 8:01 ` Daniel Vetter 2018-03-26 8:01 ` Daniel Vetter 2018-03-26 15:42 ` Jerome Glisse 2018-03-26 15:42 ` Jerome Glisse 2018-03-27 7:35 ` Christian König 2018-03-27 7:35 ` Christian König 2018-03-27 7:53 ` Daniel Vetter 2018-03-27 7:53 ` Daniel Vetter 2018-03-27 8:06 ` Christian König 2018-03-27 8:06 ` Christian König 2018-03-27 8:27 ` Daniel Vetter 2018-03-27 8:27 ` Daniel Vetter 2018-03-19 14:04 ` Daniel Vetter 2018-03-19 14:04 ` Daniel Vetter 2018-03-16 13:20 ` [PATCH 2/5] drm/ttm: keep a reference to transfer pipelined BOs Christian König 2018-03-16 13:20 ` Christian König 2018-03-27 3:32 ` He, Roger 2018-03-27 3:32 ` He, Roger 2018-03-16 13:20 ` [PATCH 3/5] drm/ttm: remove the backing store if no placement is given Christian König 2018-03-16 13:20 ` Christian König 2018-03-16 13:20 ` [PATCH 4/5] drm/amdgpu: add independent DMA-buf export v2 Christian König 2018-03-16 13:20 ` Christian König 2018-03-16 13:20 ` [PATCH 5/5] drm/amdgpu: add independent DMA-buf import v2 Christian König 2018-03-16 13:20 ` Christian König 2018-03-19 14:09 ` RFC: unpinned DMA-buf exporting v2 Daniel Vetter 2018-03-19 14:09 ` Daniel Vetter
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=21879456-db47-589c-b5e2-dfe8333d9e4c@gmail.com \ --to=ckoenig.leichtzumerken@gmail.com \ --cc=amd-gfx@lists.freedesktop.org \ --cc=chris@chris-wilson.co.uk \ --cc=christian.koenig@amd.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=linaro-mm-sig@lists.linaro.org \ --cc=linux-media@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.