Re: [Intel-gfx] [PATCH 21/26] drm: use new iterator in drm_gem_plane_helper_prepare_fb v2

Daniel Vetter
2021-09-17T14:55:56Z

On Fri, Sep 17, 2021 at 02:35:08PM +0200, Christian König wrote:
> Makes the handling a bit more complex, but avoids the use of
> dma_resv_get_excl_unlocked().
>
> v2: add missing rcu_read_lock()/unlock()
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/drm_gem_atomic_helper.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c
> index e570398abd78..d8f9c6432544 100644
> --- a/drivers/gpu/drm/drm_gem_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
> @@ -143,6 +143,7 @@
> */
> 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;
>
> @@ -150,9 +151,18 @@ int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane, struct drm_plane_st
> return 0;
>
> obj = drm_gem_fb_get_obj(state->fb, 0);
> - fence = dma_resv_get_excl_unlocked(obj->resv);
> - drm_atomic_set_fence_for_plane(state, fence);
> + rcu_read_lock();
> + dma_resv_iter_begin(&cursor, obj->resv, false);
> + dma_resv_for_each_fence_unlocked(&cursor, fence) {
> + rcu_read_unlock();
> + /* TODO: We only use the first write fence here */
> + drm_atomic_set_fence_for_plane(state, fence);

Yeah I wonder whether we should/need to collate them all together. But I
guesss whomever hits that first with their funny multi-plane yuv or
whatever gets to do that. Or I'm not clear on what exactly your TODO here
means?

> + return 0;
> + }
> + dma_resv_iter_end(&cursor);
> + rcu_read_unlock();

Imo we should do full dma_resv_lock here. atomic helpers are designed to
allow this, and it simplifies things. Also it really doesn't matter for
atomic, we should be able to do 60fps*a few planes easily :-)
-Daniel

>
> + drm_atomic_set_fence_for_plane(state, NULL);
> return 0;
> }
> EXPORT_SYMBOL_GPL(drm_gem_plane_helper_prepare_fb);
> --
> 2.25.1
>

--
Daniel Vetter
Software Engineer, Intel Corporation