Re: [PATCH 09/10] drm/i915: wait render timeout ioctl

Daniel Vetter
2012-04-22T10:10:39Z

On Sun, Apr 22, 2012 at 10:48:01AM +0100, Chris Wilson wrote:
> On Fri, 20 Apr 2012 18:23:31 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > +int
> > +i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> > +{
> > + struct drm_i915_gem_wait *args = data;
> > + struct drm_i915_gem_object *obj;
> > + struct intel_ring_buffer *ring;
> > + long timeout;
> > + u32 seqno = 0;
> > + int ret = 0;
> > +
> > + ret = i915_mutex_lock_interruptible(dev);
> > + if (ret)
> > + return ret;
> > +
> > + obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->bo_handle));
> > + if (&obj->base == NULL) {
> > + mutex_unlock(&dev->struct_mutex);
> > + return -ENOENT;
> > + }
> > +
> > + timeout = args->timeout_ns;
>
> We discussed on IRC whether or not we needed to flush here. For the
> drm_intel_gem_bo_wait_rendering() use-case, we do need to be able to first
> queue a flush.
>
> if (args->flags & I915_WAIT_FINISH)
> i915_gem_object_flush_gpu_write_domain(obj);
>
> Borrowing the terminology from glFlush/glFinish. This becomes moot if
> the flushing list is ever vanquished, but still a useful distinction and
> required today.

Yeah, I agree that emitting any required flushes (akin to the busy ioctl)
makes sense here. While I bikeshed, the mutex_lock can be moved after
drm_gem_object_lookup - we only need the mutex when dropping the reference
(and hence the unref_unlocked variant).
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48