From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 1/2] drm/i915: Android sync points for i915 v2 Date: Wed, 3 Sep 2014 19:41:34 +0300 Message-ID: <20140903164134.GX4193@intel.com> References: <1409693561-1669-1-git-send-email-jbarnes@virtuousgeek.org> <1409693561-1669-2-git-send-email-jbarnes@virtuousgeek.org> <20140903070901.GK25238@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id A607789B7D for ; Wed, 3 Sep 2014 09:41:43 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140903070901.GK25238@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Jesse Barnes , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Sep 03, 2014 at 08:09:01AM +0100, Chris Wilson wrote: > On Tue, Sep 02, 2014 at 02:32:40PM -0700, Jesse Barnes wrote: > > +static int i915_fence_check(wait_queue_t *wait, unsigned mode, int fla= gs, > > + void *key) > > +{ > > + struct i915_fence *intel_fence =3D wait->private; > > + struct intel_engine_cs *ring =3D intel_fence->ring; > > + > > + if (!i915_seqno_passed(ring->get_seqno(ring, false), > > + intel_fence->seqno)) > > + return 0; > > + > > + fence_signal_locked(&intel_fence->base); > > + > > + __remove_wait_queue(&ring->irq_queue, wait); > > + fence_put(&intel_fence->base); > > + ring->irq_put(ring); > > + > > + return 0; > > +} > > + > > +static bool i915_fence_enable_signaling(struct fence *fence) > > +{ > > + struct i915_fence *intel_fence =3D to_intel_fence(fence); > > + struct intel_engine_cs *ring =3D intel_fence->ring; > > + struct drm_i915_private *dev_priv =3D ring->dev->dev_private; > > + wait_queue_t *wait =3D &intel_fence->wait; > > + > > + /* queue fence wait queue on irq queue and get fence */ > > + if (i915_seqno_passed(ring->get_seqno(ring, false), > > + intel_fence->seqno) || > > + i915_terminally_wedged(&dev_priv->gpu_error)) > > + return false; > > + > > + if (!ring->irq_get(ring)) > > + return false; > > + > > + wait->flags =3D 0; > > + wait->private =3D intel_fence; > > + wait->func =3D i915_fence_check; > > + > > + __add_wait_queue(&ring->irq_queue, wait); > > + fence_get(fence); > > + > > + return true; > > +} > = > This looks like it implements poll(). = > = > You should recheck i915_request_complete() after setting up the irq > waiter. Or does struct fence_ops handle that? Also looks quite a bit like my ring notify doohicky from: http://lists.freedesktop.org/archives/intel-gfx/2014-June/047623.html Except I kept the list in the driver so you would need to do only one get_seqno() per irq. Also if the list would be sorted (which it wasn't in my patch) it would prevent signalling the fences out of order. But maybe that's not really a problem for anyone. Hmm, so if the out of order thing isn't a problem maybe use the wait queue still but replace the wake_up() with __wake_up() so that the seqno can be passed in as the key. That's assuming people care about optimizing the seqno reads. -- = Ville Syrj=E4l=E4 Intel OTC