| Message ID | 20211123142111.3885-20-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/26] drm/amdgpu: partially revert "svm bo enable_signal call condition" | expand |
On Tue, Nov 23, 2021 at 03:21:04PM +0100, Christian König wrote: > Use dma_resv_get_singleton() here to eventually get more than one write > fence as single fence. Yeah this is nice, atomic commit helpers not supporting multiple write fences was really my main worry in this entire endeavour. Otherwise looks all rather reasonable. I'll try to find some review bandwidth, but would be really if you can volunteer others too (especially making sure ttm drivers set the KERNEL fences correctly in all cases). -Daniel > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/drm_gem_atomic_helper.c | 18 +++++++----------- > 1 file changed, 7 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c > index c3189afe10cb..9338ddb7edff 100644 > --- a/drivers/gpu/drm/drm_gem_atomic_helper.c > +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c > @@ -143,25 +143,21 @@ > */ > int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) > { > - struct dma_resv_iter cursor; > struct drm_gem_object *obj; > struct dma_fence *fence; > + int ret; > > if (!state->fb) > return 0; > > obj = drm_gem_fb_get_obj(state->fb, 0); > - dma_resv_iter_begin(&cursor, obj->resv, false); > - dma_resv_for_each_fence_unlocked(&cursor, fence) { > - /* TODO: Currently there should be only one write fence, so this > - * here works fine. But drm_atomic_set_fence_for_plane() should > - * be changed to be able to handle more fences in general for > - * multiple BOs per fb anyway. */ > - dma_fence_get(fence); > - break; > - } > - dma_resv_iter_end(&cursor); > + ret = dma_resv_get_singleton(obj->resv, false, &fence); > + if (ret) > + return ret; > > + /* TODO: drm_atomic_set_fence_for_plane() should be changed to be able > + * to handle more fences in general for multiple BOs per fb. > + */ > drm_atomic_set_fence_for_plane(state, fence); > return 0; > } > -- > 2.25.1 >
Am 25.11.21 um 16:47 schrieb Daniel Vetter: > On Tue, Nov 23, 2021 at 03:21:04PM +0100, Christian König wrote: >> Use dma_resv_get_singleton() here to eventually get more than one write >> fence as single fence. > Yeah this is nice, atomic commit helpers not supporting multiple write > fences was really my main worry in this entire endeavour. Otherwise looks > all rather reasonable. > > I'll try to find some review bandwidth, but would be really if you can > volunteer others too (especially making sure ttm drivers set the KERNEL > fences correctly in all cases). Maybe I should split that up into switching over to adding the enum and then switching to kernel/bookkeep(previously other) for some use cases. It would be good if I could get an rb on the trivial driver cleanups first. I can send those out individually if that helps. Thanks, Christian. > -Daniel > > >> Signed-off-by: Christian König <christian.koenig@amd.com> >> --- >> drivers/gpu/drm/drm_gem_atomic_helper.c | 18 +++++++----------- >> 1 file changed, 7 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c >> index c3189afe10cb..9338ddb7edff 100644 >> --- a/drivers/gpu/drm/drm_gem_atomic_helper.c >> +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c >> @@ -143,25 +143,21 @@ >> */ >> int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) >> { >> - struct dma_resv_iter cursor; >> struct drm_gem_object *obj; >> struct dma_fence *fence; >> + int ret; >> >> if (!state->fb) >> return 0; >> >> obj = drm_gem_fb_get_obj(state->fb, 0); >> - dma_resv_iter_begin(&cursor, obj->resv, false); >> - dma_resv_for_each_fence_unlocked(&cursor, fence) { >> - /* TODO: Currently there should be only one write fence, so this >> - * here works fine. But drm_atomic_set_fence_for_plane() should >> - * be changed to be able to handle more fences in general for >> - * multiple BOs per fb anyway. */ >> - dma_fence_get(fence); >> - break; >> - } >> - dma_resv_iter_end(&cursor); >> + ret = dma_resv_get_singleton(obj->resv, false, &fence); >> + if (ret) >> + return ret; >> >> + /* TODO: drm_atomic_set_fence_for_plane() should be changed to be able >> + * to handle more fences in general for multiple BOs per fb. >> + */ >> drm_atomic_set_fence_for_plane(state, fence); >> return 0; >> } >> -- >> 2.25.1 >>
On Fri, Nov 26, 2021 at 11:30:19AM +0100, Christian König wrote: > Am 25.11.21 um 16:47 schrieb Daniel Vetter: > > On Tue, Nov 23, 2021 at 03:21:04PM +0100, Christian König wrote: > > > Use dma_resv_get_singleton() here to eventually get more than one write > > > fence as single fence. > > Yeah this is nice, atomic commit helpers not supporting multiple write > > fences was really my main worry in this entire endeavour. Otherwise looks > > all rather reasonable. > > > > I'll try to find some review bandwidth, but would be really if you can > > volunteer others too (especially making sure ttm drivers set the KERNEL > > fences correctly in all cases). > > Maybe I should split that up into switching over to adding the enum and then > switching to kernel/bookkeep(previously other) for some use cases. > > It would be good if I could get an rb on the trivial driver cleanups first. > I can send those out individually if that helps. Yeah some of the conversion patches might make sense to be split a bit more. Especially when there's functional changes hiding, but I tought you've split them out? But didn't read them in detail. Either way for stuff like this I think it's always best to split the mechanical stuff from the concept intro/docs and functional changes, except when it's really very obvious what's going on and just as small patch. -Daniel > > Thanks, > Christian. > > > -Daniel > > > > > > > Signed-off-by: Christian König <christian.koenig@amd.com> > > > --- > > > drivers/gpu/drm/drm_gem_atomic_helper.c | 18 +++++++----------- > > > 1 file changed, 7 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c > > > index c3189afe10cb..9338ddb7edff 100644 > > > --- a/drivers/gpu/drm/drm_gem_atomic_helper.c > > > +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c > > > @@ -143,25 +143,21 @@ > > > */ > > > int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) > > > { > > > - struct dma_resv_iter cursor; > > > struct drm_gem_object *obj; > > > struct dma_fence *fence; > > > + int ret; > > > if (!state->fb) > > > return 0; > > > obj = drm_gem_fb_get_obj(state->fb, 0); > > > - dma_resv_iter_begin(&cursor, obj->resv, false); > > > - dma_resv_for_each_fence_unlocked(&cursor, fence) { > > > - /* TODO: Currently there should be only one write fence, so this > > > - * here works fine. But drm_atomic_set_fence_for_plane() should > > > - * be changed to be able to handle more fences in general for > > > - * multiple BOs per fb anyway. */ > > > - dma_fence_get(fence); > > > - break; > > > - } > > > - dma_resv_iter_end(&cursor); > > > + ret = dma_resv_get_singleton(obj->resv, false, &fence); > > > + if (ret) > > > + return ret; > > > + /* TODO: drm_atomic_set_fence_for_plane() should be changed to be able > > > + * to handle more fences in general for multiple BOs per fb. > > > + */ > > > drm_atomic_set_fence_for_plane(state, fence); > > > return 0; > > > } > > > -- > > > 2.25.1 > > > >
diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c index c3189afe10cb..9338ddb7edff 100644 --- a/drivers/gpu/drm/drm_gem_atomic_helper.c +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c @@ -143,25 +143,21 @@ */ int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) { - struct dma_resv_iter cursor; struct drm_gem_object *obj; struct dma_fence *fence; + int ret; if (!state->fb) return 0; obj = drm_gem_fb_get_obj(state->fb, 0); - dma_resv_iter_begin(&cursor, obj->resv, false); - dma_resv_for_each_fence_unlocked(&cursor, fence) { - /* TODO: Currently there should be only one write fence, so this - * here works fine. But drm_atomic_set_fence_for_plane() should - * be changed to be able to handle more fences in general for - * multiple BOs per fb anyway. */ - dma_fence_get(fence); - break; - } - dma_resv_iter_end(&cursor); + ret = dma_resv_get_singleton(obj->resv, false, &fence); + if (ret) + return ret; + /* TODO: drm_atomic_set_fence_for_plane() should be changed to be able + * to handle more fences in general for multiple BOs per fb. + */ drm_atomic_set_fence_for_plane(state, fence); return 0; }
Use dma_resv_get_singleton() here to eventually get more than one write fence as single fence. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/drm_gem_atomic_helper.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)