Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753076AbcDOIJ6 (ORCPT ); Fri, 15 Apr 2016 04:09:58 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:33178 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750924AbcDOIJx (ORCPT ); Fri, 15 Apr 2016 04:09:53 -0400 Date: Fri, 15 Apr 2016 10:09:48 +0200 From: Daniel Vetter To: Gustavo Padovan Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Daniel Stone , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Riley Andrews , Daniel Vetter , Rob Clark , Greg Hackmann , John Harrison , laurent.pinchart@ideasonboard.com, seanpaul@google.com, marcheu@google.com, m.chehab@samsung.com, Maarten Lankhorst , Gustavo Padovan Subject: Re: [RFC 5/8] drm/fence: add fence to drm_pending_event Message-ID: <20160415080948.GS2510@phenom.ffwll.local> Mail-Followup-To: Gustavo Padovan , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Daniel Stone , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Riley Andrews , Rob Clark , Greg Hackmann , John Harrison , laurent.pinchart@ideasonboard.com, seanpaul@google.com, marcheu@google.com, m.chehab@samsung.com, Maarten Lankhorst , Gustavo Padovan References: <1460683781-22535-1-git-send-email-gustavo@padovan.org> <1460683781-22535-6-git-send-email-gustavo@padovan.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1460683781-22535-6-git-send-email-gustavo@padovan.org> X-Operating-System: Linux phenom 4.4.0-1-amd64 User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4324 Lines: 138 On Thu, Apr 14, 2016 at 06:29:38PM -0700, Gustavo Padovan wrote: > From: Gustavo Padovan > > Now a drm_pending_event can either send a real drm_event or signal a > fence, or both. It allow us to signal via fences when the buffer is > displayed on the screen. Which in turn means that the previous buffer > is not in use anymore and can be freed or sent back to another driver > for processing. > > Signed-off-by: Gustavo Padovan Do you have atomic igt testcase that exercise all the combinations of drm_event and in/out-fences and make sure it all keeps working? Tomeu already converted that over to be a generic testcase. > --- > drivers/gpu/drm/drm_atomic.c | 19 +++++++++++++------ > drivers/gpu/drm/drm_fops.c | 5 +++-- > drivers/gpu/drm/drm_irq.c | 7 +++++++ > include/drm/drmP.h | 1 + > 4 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 6702502..0b95526 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1431,7 +1431,8 @@ EXPORT_SYMBOL(drm_atomic_async_commit); > */ > > static struct drm_pending_vblank_event *create_vblank_event( > - struct drm_device *dev, struct drm_file *file_priv, uint64_t user_data) > + struct drm_device *dev, struct drm_file *file_priv, > + struct fence *fence, uint64_t user_data) > { > struct drm_pending_vblank_event *e = NULL; > int ret; > @@ -1444,12 +1445,17 @@ static struct drm_pending_vblank_event *create_vblank_event( > e->event.base.length = sizeof(e->event); > e->event.user_data = user_data; > > - ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base); > - if (ret) { > - kfree(e); > - return NULL; > + if (file_priv) { > + ret = drm_event_reserve_init(dev, file_priv, &e->base, > + &e->event.base); > + if (ret) { > + kfree(e); > + return NULL; > + } > } > > + e->base.fence = fence; > + > return e; > } > > @@ -1676,7 +1682,8 @@ retry: > for_each_crtc_in_state(state, crtc, crtc_state, i) { > struct drm_pending_vblank_event *e; > > - e = create_vblank_event(dev, file_priv, arg->user_data); > + e = create_vblank_event(dev, file_priv, NULL, > + arg->user_data); > if (!e) { > ret = -ENOMEM; > goto out; > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c > index aeef58e..38def49 100644 > --- a/drivers/gpu/drm/drm_fops.c > +++ b/drivers/gpu/drm/drm_fops.c > @@ -801,8 +801,9 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) > { > assert_spin_locked(&dev->event_lock); > > - if (!e->file_priv) { > - e->destroy(e); > + if (!e->file_priv || !e->event) { This would be a bug: e->file_priv != NULL iff e->event != NULL. How did this happen? > + if (e->destroy) > + e->destroy(e); > return; > } > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index 3c1a6f1..0c5d7cb 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -41,6 +41,7 @@ > > #include > #include > +#include > > /* Access macro for slots in vblank timestamp ringbuffer. */ > #define vblanktimestamp(dev, pipe, count) \ > @@ -1124,6 +1125,12 @@ void drm_send_vblank_event(struct drm_device *dev, unsigned int pipe, > now = get_drm_timestamp(); > } > e->pipe = pipe; > + > + if (e->base.fence) { > + fence_signal(e->base.fence); > + fence_put(e->base.fence); > + } I'd put this into drm_send_event_locked even. In case we send out fences for other events too (e.g. exynos uses drm_event for rendering ...). -Daniel > + > send_vblank_event(dev, e, seq, &now); > } > EXPORT_SYMBOL(drm_send_vblank_event); > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 3c8422c..8f83c2a 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -282,6 +282,7 @@ struct drm_ioctl_desc { > /* Event queued up for userspace to read */ > struct drm_pending_event { > struct drm_event *event; > + struct fence *fence; > struct list_head link; > struct list_head pending_link; > struct drm_file *file_priv; > -- > 2.5.5 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch