Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751657AbcDOS7F (ORCPT ); Fri, 15 Apr 2016 14:59:05 -0400 Received: from mail-pa0-f44.google.com ([209.85.220.44]:33025 "EHLO mail-pa0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751443AbcDOS7D (ORCPT ); Fri, 15 Apr 2016 14:59:03 -0400 Date: Fri, 15 Apr 2016 11:59:00 -0700 From: Gustavo Padovan To: 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 Subject: Re: [RFC 5/8] drm/fence: add fence to drm_pending_event Message-ID: <20160415185900.GD23954@joana> 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> <20160415080948.GS2510@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160415080948.GS2510@phenom.ffwll.local> 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: 4170 Lines: 125 2016-04-15 Daniel Vetter : > 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. Not yet. I'll check for test cases. > > > --- > > 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? Not sure now. But I needed this to prevent a crash, I don't have logs of it anymore, I'll check this again. > > > + 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 ...). Okay. Looking into that. Gustavo