From: Daniel Vetter <daniel@ffwll.ch>
To: "Christian König" <christian.koenig@amd.com>
Cc: daniel.vetter@ffwll.ch, dri-devel@lists.freedesktop.org,
DMA-resvusage@phenom.ffwll.local
Subject: Re: [PATCH 03/16] dma-buf: specify usage while adding fences to dma_resv obj v6
Date: Thu, 7 Apr 2022 11:26:57 +0200 [thread overview]
Message-ID: <Yk6uYXZq8BDalPXA@phenom.ffwll.local> (raw)
In-Reply-To: <0e34ce06-c962-b1f1-d2a9-5dedbd19ff32@amd.com>
On Thu, Apr 07, 2022 at 10:01:52AM +0200, Christian König wrote:
> Am 06.04.22 um 14:35 schrieb Daniel Vetter:
> > On Wed, Apr 06, 2022 at 02:32:22PM +0200, Daniel Vetter wrote:
> > > On Wed, Apr 06, 2022 at 09:51:19AM +0200, Christian König wrote:
> > > > [SNIP]
> > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > > > index 53f7c78628a4..98bb5c9239de 100644
> > > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > > > @@ -202,14 +202,10 @@ static void submit_attach_object_fences(struct etnaviv_gem_submit *submit)
> > > > for (i = 0; i < submit->nr_bos; i++) {
> > > > struct drm_gem_object *obj = &submit->bos[i].obj->base;
> > > > + bool write = submit->bos[i].flags & ETNA_SUBMIT_BO_WRITE;
> > > > - if (submit->bos[i].flags & ETNA_SUBMIT_BO_WRITE)
> > > > - dma_resv_add_excl_fence(obj->resv,
> > > > - submit->out_fence);
> > > > - else
> > > > - dma_resv_add_shared_fence(obj->resv,
> > > > - submit->out_fence);
> > > > -
> > > > + dma_resv_add_fence(obj->resv, submit->out_fence, write ?
> > > > + DMA_RESV_USAGE_WRITE : DMA_RESV_USAGE_READ);
> > > Iirc I had some suggestions to use dma_resv_usage_rw here and above. Do
> > > these happen in later patches? There's also a few more of these later on.
>
> That won't work. dma_resv_usage_rw() returns the usage as necessary for
> dependencies. In other words write return DMA_RESV_USAGE_READ and read
> return DMA_RESV_USAGE_WRITE.
Hm right, that's a bit annoying due to the asymetry in dependencies and
adding fences.
>
> What we could do is to add a dma_resv_add_fence_rw() wrapper which does the
> necessary ?: in a central place.
I'm not sure it's overkill, but what about something like this:
enum drm_sync_mode {
DRM_SYNC_NO_IMPLICIT,
DRM_SYNC_WRITE,
DRM_SYNC_READ,
}
And then two functions, on in the drm/sched which replaces the current
add_implicit_dependencies, and the other which would be in the glorious
future eu utils shared between ttm and gem drivers, which adds the fence
with the right usage. And they would take care of the right mapping in
each case.
And then all we'd still have in driver code is mapping from random
bonghits driver flags to drm_sync_mode, and all the confusion would be in
shared code. And see above, at least for me it's confusing as heck :-)
>
> > > > diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
> > > > index e0a11ee0e86d..cb3bfccc930f 100644
> > > > --- a/drivers/gpu/drm/lima/lima_gem.c
> > > > +++ b/drivers/gpu/drm/lima/lima_gem.c
> > > > @@ -367,7 +367,7 @@ int lima_gem_submit(struct drm_file *file, struct lima_submit *submit)
> > > > if (submit->bos[i].flags & LIMA_SUBMIT_BO_WRITE)
> > > > dma_resv_add_excl_fence(lima_bo_resv(bos[i]), fence);
> > > > else
> > > > - dma_resv_add_shared_fence(lima_bo_resv(bos[i]), fence);
> > > > + dma_resv_add_fence(lima_bo_resv(bos[i]), fence);
> > Correction on the r-b, I'm still pretty sure that this won't compile at
> > all.
>
> Grrr, I've forgot to add CONFIG_OF to my compile build config.
>
> BTW: Do we have a tool for compile test coverage of patches? E.g. something
> which figures out if a build created an o file for each c file a patch
> touched?
Just regrets when I screw up :-/
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
next prev parent reply other threads:[~2022-04-07 9:27 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-06 7:51 Christian König
2022-04-06 7:51 ` [PATCH 01/16] dma-buf/drivers: make reserving a shared slot mandatory v4 Christian König
2022-04-06 12:21 ` Daniel Vetter
2022-04-06 7:51 ` [PATCH 02/16] dma-buf: add enum dma_resv_usage v4 Christian König
2022-04-06 7:51 ` [PATCH 03/16] dma-buf: specify usage while adding fences to dma_resv obj v6 Christian König
2022-04-06 12:32 ` Daniel Vetter
2022-04-06 12:35 ` Daniel Vetter
2022-04-07 8:01 ` Christian König
2022-04-07 9:26 ` Daniel Vetter [this message]
2022-04-06 7:51 ` [PATCH 04/16] dma-buf & drm/amdgpu: remove dma_resv workaround Christian König
2022-04-06 12:39 ` Daniel Vetter
2022-04-06 7:51 ` [PATCH 05/16] dma-buf: add DMA_RESV_USAGE_KERNEL v3 Christian König
2022-04-06 12:41 ` Daniel Vetter
2022-04-06 7:51 ` [PATCH 06/16] drm/amdgpu: use DMA_RESV_USAGE_KERNEL Christian König
2022-04-06 12:42 ` Daniel Vetter
2022-04-06 14:54 ` Christian König
2022-04-06 7:51 ` [PATCH 07/16] drm/radeon: " Christian König
2022-04-06 12:43 ` Daniel Vetter
2022-04-06 7:51 ` [PATCH 08/16] drm/etnaviv: always wait for kernel fences Christian König
2022-04-06 12:46 ` Daniel Vetter
2022-04-06 7:51 ` [PATCH 09/16] drm/nouveau: only wait for kernel fences in nouveau_bo_vm_cleanup Christian König
2022-04-06 12:47 ` Daniel Vetter
2022-04-06 7:51 ` [PATCH 10/16] RDMA: use DMA_RESV_USAGE_KERNEL Christian König
2022-04-06 12:48 ` Daniel Vetter
2022-04-06 7:51 ` [PATCH 11/16] dma-buf: add DMA_RESV_USAGE_BOOKKEEP v3 Christian König
2022-04-06 7:51 ` [PATCH 12/16] drm/amdgpu: use DMA_RESV_USAGE_BOOKKEEP Christian König
2022-04-06 7:51 ` [PATCH 13/16] dma-buf: wait for map to complete for static attachments Christian König
2022-04-06 7:51 ` [PATCH 14/16] drm/i915: drop bo->moving dependency Christian König
2022-04-06 7:51 ` [Intel-gfx] " Christian König
2022-04-06 13:24 ` Matthew Auld
2022-04-06 13:24 ` Matthew Auld
2022-04-06 7:51 ` [PATCH 15/16] drm/ttm: remove bo->moving Christian König
2022-04-06 12:52 ` Daniel Vetter
2022-04-06 7:51 ` [PATCH 16/16] dma-buf: drop seq count based update Christian König
2022-04-06 13:00 ` Daniel Vetter
2022-04-06 12:59 ` 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=Yk6uYXZq8BDalPXA@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=DMA-resvusage@phenom.ffwll.local \
--cc=christian.koenig@amd.com \
--cc=daniel.vetter@ffwll.ch \
--cc=dri-devel@lists.freedesktop.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: link
Be 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.