Re: [Intel-gfx] [PATCH] intel: add a timed wait function

[Ben Widawsky <ben@bwidawsk.net>]

On Wed, 30 May 2012 10:41:20 -0700
Eric Anholt <eric@anholt.net> wrote:

> On Sun, 27 May 2012 13:16:54 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> > index b776d2f..695a449 100644
> > --- a/intel/intel_bufmgr_gem.c
> > +++ b/intel/intel_bufmgr_gem.c
> > @@ -1478,6 +1478,32 @@ drm_intel_gem_bo_wait_rendering(drm_intel_bo *bo)
> >  	drm_intel_gem_bo_start_gtt_access(bo, 1);
> >  }
> >  
> > +int drm_intel_gem_bo_wait(drm_intel_bo *bo, uint64_t *timeout_ns)
> > +{
> > +	drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)
> > bo->bufmgr;
> > +	drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
> > +	struct drm_i915_gem_wait wait;
> > +	int ret;
> > +
> > +	if (!timeout_ns)
> > +		return -EINVAL;
> 
> At least for the GL case, timeout of 0 ns wants to turn into
> GL_TIMEOUT_EXPIRED or GL_ALREADY_SIGNALED.  -EINVAL doesn't sound like
> translating into either of those -- are you thinking that GL will
> special case 0 ns to not call this function?

Well, it's timeout of NULL, not 0. 0 should do what you want. I can turn
NULL into 0 just as easily, if you want?

> 
> > +
> > +	wait.bo_handle = bo_gem->gem_handle;
> > +	wait.timeout_ns = *timeout_ns;
> > +	wait.flags = 0;
> > +	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_WAIT, &wait);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (wait.timeout_ns == 0) {
> > +		DBG("Wait timed out on buffer %d\n",
> > bo_gem->gem_handle);
> > +		*timeout_ns = 0;
> > +	} else
> > +		*timeout_ns = wait.timeout_ns;
> > +
> > +	return ret;
> > +}
> 
> Do we see any consumers wanting the unslept time?  GL doesn't care, and
> not passing a pointer would be more convenient for the caller.

That is how I originally had it, but Daniel Vetter requested otherwise.
I don't care either way. This interacts with your earlier comment as
well.

> 
> I guess GL_ALREADY_SIGNALED handling will be done using a check for
> bo_busy() before calling this.

It shouldn't have to.

I think the outcome is either, drop the return time, or convert NULLs to
0, and everything should be fine, right?


-- 
Ben Widawsky, Intel Open Source Technology Center