| Message ID | 20211005113742.1101-27-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/28] dma-buf: add dma_resv_for_each_fence_unlocked v8 | expand |
On Tue, Oct 05, 2021 at 01:37:40PM +0200, Christian König wrote: > Makes the handling a bit more complex, but avoids the use of > dma_resv_get_excl_unlocked(). > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/nouveau/dispnv50/wndw.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > index 8d048bacd6f0..30712a681e2a 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > @@ -539,6 +539,8 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) > struct nouveau_bo *nvbo; > struct nv50_head_atom *asyh; > struct nv50_wndw_ctxdma *ctxdma; > + struct dma_resv_iter cursor; > + struct dma_fence *fence; > int ret; > > NV_ATOMIC(drm, "%s prepare: %p\n", plane->name, fb); > @@ -561,7 +563,13 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) > asyw->image.handle[0] = ctxdma->object.handle; > } > > - asyw->state.fence = dma_resv_get_excl_unlocked(nvbo->bo.base.resv); > + dma_resv_iter_begin(&cursor, nvbo->bo.base.resv, false); > + dma_resv_for_each_fence_unlocked(&cursor, fence) { > + /* TODO: We only use the first writer here */ Same thing as with the atomic core helper. This is actually broken, because for atomic we really do _not_ want to wait for any shared fences. Which this will do, if there's no exclusive fence attached. So upgrading my general concern on this and the atomic helper patch to a reject, since I think it's broken. -Daniel > + asyw->state.fence = dma_fence_get(fence); > + break; > + } > + dma_resv_iter_end(&cursor); > asyw->image.offset[0] = nvbo->offset; > > if (wndw->func->prepare) { > -- > 2.25.1 >
Am 13.10.21 um 16:29 schrieb Daniel Vetter: > On Tue, Oct 05, 2021 at 01:37:40PM +0200, Christian König wrote: >> Makes the handling a bit more complex, but avoids the use of >> dma_resv_get_excl_unlocked(). >> >> Signed-off-by: Christian König <christian.koenig@amd.com> >> --- >> drivers/gpu/drm/nouveau/dispnv50/wndw.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c >> index 8d048bacd6f0..30712a681e2a 100644 >> --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c >> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c >> @@ -539,6 +539,8 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) >> struct nouveau_bo *nvbo; >> struct nv50_head_atom *asyh; >> struct nv50_wndw_ctxdma *ctxdma; >> + struct dma_resv_iter cursor; >> + struct dma_fence *fence; >> int ret; >> >> NV_ATOMIC(drm, "%s prepare: %p\n", plane->name, fb); >> @@ -561,7 +563,13 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) >> asyw->image.handle[0] = ctxdma->object.handle; >> } >> >> - asyw->state.fence = dma_resv_get_excl_unlocked(nvbo->bo.base.resv); >> + dma_resv_iter_begin(&cursor, nvbo->bo.base.resv, false); >> + dma_resv_for_each_fence_unlocked(&cursor, fence) { >> + /* TODO: We only use the first writer here */ > Same thing as with the atomic core helper. This is actually broken, > because for atomic we really do _not_ want to wait for any shared fences. > Which this will do, if there's no exclusive fence attached. > > So upgrading my general concern on this and the atomic helper patch to a > reject, since I think it's broken. Since we simply had a misunderstanding with that could I get an rb for that now? Thanks, Christian. > -Daniel > >> + asyw->state.fence = dma_fence_get(fence); >> + break; >> + } >> + dma_resv_iter_end(&cursor); >> asyw->image.offset[0] = nvbo->offset; >> >> if (wndw->func->prepare) { >> -- >> 2.25.1 >>
On Fri, Oct 22, 2021 at 03:17:17PM +0200, Christian König wrote: > Am 13.10.21 um 16:29 schrieb Daniel Vetter: > > On Tue, Oct 05, 2021 at 01:37:40PM +0200, Christian König wrote: > > > Makes the handling a bit more complex, but avoids the use of > > > dma_resv_get_excl_unlocked(). > > > > > > Signed-off-by: Christian König <christian.koenig@amd.com> > > > --- > > > drivers/gpu/drm/nouveau/dispnv50/wndw.c | 10 +++++++++- > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > > > index 8d048bacd6f0..30712a681e2a 100644 > > > --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c > > > +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > > > @@ -539,6 +539,8 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) > > > struct nouveau_bo *nvbo; > > > struct nv50_head_atom *asyh; > > > struct nv50_wndw_ctxdma *ctxdma; > > > + struct dma_resv_iter cursor; > > > + struct dma_fence *fence; > > > int ret; > > > NV_ATOMIC(drm, "%s prepare: %p\n", plane->name, fb); > > > @@ -561,7 +563,13 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) > > > asyw->image.handle[0] = ctxdma->object.handle; > > > } > > > - asyw->state.fence = dma_resv_get_excl_unlocked(nvbo->bo.base.resv); > > > + dma_resv_iter_begin(&cursor, nvbo->bo.base.resv, false); > > > + dma_resv_for_each_fence_unlocked(&cursor, fence) { > > > + /* TODO: We only use the first writer here */ > > Same thing as with the atomic core helper. This is actually broken, > > because for atomic we really do _not_ want to wait for any shared fences. > > Which this will do, if there's no exclusive fence attached. > > > > So upgrading my general concern on this and the atomic helper patch to a > > reject, since I think it's broken. > > Since we simply had a misunderstanding with that could I get an rb for that > now? Oh sorry, I thought I've supplied that. As much a you still trust my r-b at least :-) Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Thanks, > Christian. > > > -Daniel > > > > > + asyw->state.fence = dma_fence_get(fence); > > > + break; > > > + } > > > + dma_resv_iter_end(&cursor); > > > asyw->image.offset[0] = nvbo->offset; > > > if (wndw->func->prepare) { > > > -- > > > 2.25.1 > > > >
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c index 8d048bacd6f0..30712a681e2a 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c @@ -539,6 +539,8 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) struct nouveau_bo *nvbo; struct nv50_head_atom *asyh; struct nv50_wndw_ctxdma *ctxdma; + struct dma_resv_iter cursor; + struct dma_fence *fence; int ret; NV_ATOMIC(drm, "%s prepare: %p\n", plane->name, fb); @@ -561,7 +563,13 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) asyw->image.handle[0] = ctxdma->object.handle; } - asyw->state.fence = dma_resv_get_excl_unlocked(nvbo->bo.base.resv); + dma_resv_iter_begin(&cursor, nvbo->bo.base.resv, false); + dma_resv_for_each_fence_unlocked(&cursor, fence) { + /* TODO: We only use the first writer here */ + asyw->state.fence = dma_fence_get(fence); + break; + } + dma_resv_iter_end(&cursor); asyw->image.offset[0] = nvbo->offset; if (wndw->func->prepare) {
Makes the handling a bit more complex, but avoids the use of dma_resv_get_excl_unlocked(). Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)