Re: [PATCH 3/7] drm/vc4: Mimic drm_atomic_helper_commit() behavior

[Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]

On Thu, 15 Jun 2017 16:33:11 -0700
Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> wrote:

> Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> writes:
> 
> > [ Unknown signature status ]
> > Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> writes:
> >  
> >> On Tue, 06 Jun 2017 13:27:09 -0700
> >> Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> wrote:
> >>  
> >>> Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> writes:
> >>>   
> >>> > The VC4 KMS driver is implementing its own ->atomic_commit() but there
> >>> > are a few generic helpers we can use instead of open-coding the logic.
> >>> >
> >>> > Signed-off-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> >>> > ---
> >>> >  drivers/gpu/drm/vc4/vc4_kms.c | 38 ++++++++++++--------------------------
> >>> >  1 file changed, 12 insertions(+), 26 deletions(-)
> >>> >
> >>> > diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> >>> > index ad7925a9e0ea..f229abc0991b 100644
> >>> > --- a/drivers/gpu/drm/vc4/vc4_kms.c
> >>> > +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> >>> > @@ -42,6 +42,10 @@ vc4_atomic_complete_commit(struct vc4_commit *c)
> >>> >  	struct drm_device *dev = state->dev;
> >>> >  	struct vc4_dev *vc4 = to_vc4_dev(dev);
> >>> >  
> >>> > +	drm_atomic_helper_wait_for_fences(dev, state, false);
> >>> > +
> >>> > +	drm_atomic_helper_wait_for_dependencies(state);    
> >>> 
> >>> With this wait_for_fences() addition and the reservation stuff that
> >>> landed, I think we can rip out the "seqno cb" in vc4, and just use
> >>> drm_atomic_helper_commit() and drm_atomic_hepler_commit_tail().  Do you
> >>> see anything missing, with that?  
> >>
> >> I can't tell. I haven't dig enough to understand what this seqno cb was
> >> used for :-), but Daniel was suggesting the same thing. I'll try to
> >> better understand what seqno cb does and if it's all safe to get rid of
> >> it and use the standard helpers.  
> >
> > The seqno cb was the thing for stalling the modeset until V3D was done
> > rendering to the planes.  The wait_for_fences() does the same thing
> > using generic dmabuf reservations, so the seqno cb isn't needed any
> > more.  
> 
> Oh, we've got another user of the seqno cb (async flips), anyway, so we
> can't delete it quite yet.  I've pushed this patch since it's a clear
> improvement (less code, waits for fences from other devices if any).

Ok. Did you fix the leak before applying?

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html