From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 01/25] drm/i915: Add ring_notify mechanism Date: Wed, 18 Jun 2014 22:06:02 +0200 Message-ID: <20140618200602.GU5821@phenom.ffwll.local> References: <1403114338-30513-1-git-send-email-ville.syrjala@linux.intel.com> <1403114338-30513-2-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-we0-f179.google.com (mail-we0-f179.google.com [74.125.82.179]) by gabe.freedesktop.org (Postfix) with ESMTP id DCBD76E13C for ; Wed, 18 Jun 2014 13:06:09 -0700 (PDT) Received: by mail-we0-f179.google.com with SMTP id w62so1374236wes.24 for ; Wed, 18 Jun 2014 13:06:09 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1403114338-30513-2-git-send-email-ville.syrjala@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: ville.syrjala@linux.intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Jun 18, 2014 at 08:58:34PM +0300, ville.syrjala@linux.intel.com wro= te: > From: Ville Syrj=E4l=E4 > = > Proveide a ring notify mechanism where you can ask for a callback when a > specific seqno has been passed. > = > Can be used for FBC and mmio flips at least. > = > Signed-off-by: Ville Syrj=E4l=E4 Hm, most of the callbacks I can image need some process context anyway due to at least grabbing a mutex or something similar. And we support lockless seqno waits. So with that I'd lean towards offloading this into work items and just doing a lockless wait in the overall sequence since doing full callback-based programming is hard. But I haven't looked at the users at all here tbh. -Daniel > --- > drivers/gpu/drm/i915/i915_gem.c | 10 ++++ > drivers/gpu/drm/i915/i915_irq.c | 4 ++ > drivers/gpu/drm/i915/intel_ringbuffer.c | 89 +++++++++++++++++++++++++++= ++++++ > drivers/gpu/drm/i915/intel_ringbuffer.h | 17 +++++++ > 4 files changed, 120 insertions(+) > = > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_= gem.c > index d857f58..a5e62cb 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2521,6 +2521,8 @@ static void i915_gem_reset_ring_status(struct drm_i= 915_private *dev_priv, > static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_pri= v, > struct intel_engine_cs *ring) > { > + struct intel_ring_notify *notify, *next; > + > while (!list_empty(&ring->active_list)) { > struct drm_i915_gem_object *obj; > = > @@ -2552,6 +2554,14 @@ static void i915_gem_reset_ring_cleanup(struct drm= _i915_private *dev_priv, > kfree(ring->preallocated_lazy_request); > ring->preallocated_lazy_request =3D NULL; > ring->outstanding_lazy_seqno =3D 0; > + > + spin_lock_irq(&ring->lock); > + list_for_each_entry_safe(notify, next, &ring->notify_list, list) { > + intel_ring_notify_complete(notify); > + /* FIXME should we notify at reset? */ > + notify->notify(notify); > + } > + spin_unlock_irq(&ring->lock); > } > = > void i915_gem_restore_fences(struct drm_device *dev) > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_= irq.c > index 1c1ec22..218f011 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1221,6 +1221,10 @@ static void notify_ring(struct drm_device *dev, > if (drm_core_check_feature(dev, DRIVER_MODESET)) > intel_notify_mmio_flip(ring); > = > + spin_lock(&ring->lock); > + intel_ring_notify_check(ring); > + spin_unlock(&ring->lock); > + > wake_up_all(&ring->irq_queue); > i915_queue_hangcheck(dev); > } > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i9= 15/intel_ringbuffer.c > index b96edaf..31321ae 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1447,6 +1447,9 @@ static int intel_init_ring_buffer(struct drm_device= *dev, > = > init_waitqueue_head(&ring->irq_queue); > = > + INIT_LIST_HEAD(&ring->notify_list); > + spin_lock_init(&ring->lock); > + > if (I915_NEED_GFX_HWS(dev)) { > ret =3D init_status_page(ring); > if (ret) > @@ -2407,3 +2410,89 @@ intel_stop_ring_buffer(struct intel_engine_cs *rin= g) > = > stop_ring(ring); > } > + > +void intel_ring_notify_complete(struct intel_ring_notify *notify) > +{ > + struct intel_engine_cs *ring =3D notify->ring; > + > + ring->irq_put(ring); > + list_del(¬ify->list); > + notify->ring =3D NULL; > +} > + > +void intel_ring_notify_check(struct intel_engine_cs *ring) > +{ > + struct intel_ring_notify *notify, *next; > + u32 seqno; > + > + assert_spin_locked(&ring->lock); > + > + if (list_empty(&ring->notify_list)) > + return; > + > + seqno =3D ring->get_seqno(ring, false); > + > + list_for_each_entry_safe(notify, next, &ring->notify_list, list) { > + if (i915_seqno_passed(seqno, notify->seqno)) { > + intel_ring_notify_complete(notify); > + notify->notify(notify); > + } > + } > +} > + > +int intel_ring_notify_add(struct intel_engine_cs *ring, > + struct intel_ring_notify *notify) > +{ > + unsigned long irqflags; > + int ret; > + > + lockdep_assert_held(&ring->dev->struct_mutex); > + > + if (WARN_ON(notify->ring !=3D NULL || notify->seqno =3D=3D 0)) > + return -EINVAL; > + > + if (i915_seqno_passed(ring->get_seqno(ring, true), notify->seqno)) > + goto notify_immediately; > + > + ret =3D i915_gem_check_olr(ring, notify->seqno); > + if (ret) > + return ret; > + > + if (WARN_ON(!ring->irq_get(ring))) > + goto notify_immediately; > + > + spin_lock_irqsave(&ring->lock, irqflags); > + notify->ring =3D ring; > + list_add_tail(¬ify->list, &ring->notify_list); > + /* check again in case we just missed it */ > + intel_ring_notify_check(ring); > + spin_unlock_irqrestore(&ring->lock, irqflags); > + > + return 0; > + > + notify_immediately: > + spin_lock_irqsave(&ring->lock, irqflags); > + notify->notify(notify); > + spin_unlock_irqrestore(&ring->lock, irqflags); > + > + return 0; > +} > + > +bool intel_ring_notify_pending(struct intel_ring_notify *notify) > +{ > + return notify->ring !=3D NULL; > +} > + > +void intel_ring_notify_cancel(struct intel_ring_notify *notify) > +{ > + struct intel_engine_cs *ring =3D ACCESS_ONCE(notify->ring); > + unsigned long irqflags; > + > + if (!ring) > + return; > + > + spin_lock_irqsave(&ring->lock, irqflags); > + if (notify->ring) > + intel_ring_notify_complete(notify); > + spin_unlock_irqrestore(&ring->lock, irqflags); > +} > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i9= 15/intel_ringbuffer.h > index e72017b..273abf3 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -79,6 +79,13 @@ struct intel_ringbuffer { > u32 last_retired_head; > }; > = > +struct intel_ring_notify { > + void (*notify)(struct intel_ring_notify *notify); > + struct intel_engine_cs *ring; > + struct list_head list; > + u32 seqno; > +}; > + > struct intel_engine_cs { > const char *name; > enum intel_ring_id { > @@ -217,6 +224,9 @@ struct intel_engine_cs { > * to encode the command length in the header). > */ > u32 (*get_cmd_length_mask)(u32 cmd_header); > + > + struct list_head notify_list; > + spinlock_t lock; > }; > = > static inline bool > @@ -335,6 +345,13 @@ static inline void i915_trace_irq_get(struct intel_e= ngine_cs *ring, u32 seqno) > ring->trace_irq_seqno =3D seqno; > } > = > +void intel_ring_notify_complete(struct intel_ring_notify *notify); > +void intel_ring_notify_check(struct intel_engine_cs *ring); > +int __must_check intel_ring_notify_add(struct intel_engine_cs *ring, > + struct intel_ring_notify *notify); > +bool intel_ring_notify_pending(struct intel_ring_notify *notify); > +void intel_ring_notify_cancel(struct intel_ring_notify *notify); > + > /* DRI warts */ > int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 si= ze); > = > -- = > 1.8.5.5 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch