[Mesa-dev] [PATCH 3/3] intel: wait render timeout implementation

[Ben Widawsky]

On Wed, 6 Jun 2012 11:33:21 +0200
Daniel Vetter <daniel at ffwll.ch> wrote:

> On Tue, Jun 05, 2012 at 03:43:07PM -0700, Ben Widawsky wrote:
> > int drm_intel_gem_bo_wait(drm_intel_bo *bo, uint64_t timeout_ns)
> > 
> > This should bump the libdrm version. We're waiting for context support
> > so we can do both features in one bump.
> > 
> > v2: don't return remaining timeout amount
> > use get param and fallback for older kernels
> > 
> > v3: only doing getparam at init
> > prototypes now have a signed input value
> > 
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> 
> Let's bikeshed this some more ;-)
> - I think a quick comment to say that negative timeouts result in a
>   infinite wait would be nice.
> - If we want to keep timeout=0 means polling, you need to add a special
>   case in the fallback to check for that and call the busy ioctl instead
>   (and remap the business value to the correct return value).
> - iirc drmIoctl doesn't return the kernels -ERRNO value in it's retval
>   (and shovels it into errno instead). I think undoing that suckiness
>   would be good, so that we return -ETIME if the timer expired.

All these are very good points which were lost when dropping the return
time in the v2 patch. I consider none of these bikesheds at all.

Thanks.
Ben

> 
> Yours, Daniel
> > ---
> >  intel/intel_bufmgr.h     |    1 +
> >  intel/intel_bufmgr_gem.c |   30 ++++++++++++++++++++++++++++++
> >  2 files changed, 31 insertions(+)
> > 
> > diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
> > index c197abc..fa6c4dd 100644
> > --- a/intel/intel_bufmgr.h
> > +++ b/intel/intel_bufmgr.h
> > @@ -184,6 +184,7 @@ int drm_intel_get_pipe_from_crtc_id(drm_intel_bufmgr *bufmgr, int crtc_id);
> >  
> >  int drm_intel_get_aperture_sizes(int fd, size_t *mappable, size_t *total);
> >  int drm_intel_bufmgr_gem_get_devid(drm_intel_bufmgr *bufmgr);
> > +int drm_intel_gem_bo_wait(drm_intel_bo *bo, int64_t timeout_ns);
> >  
> >  /* drm_intel_bufmgr_fake.c */
> >  drm_intel_bufmgr *drm_intel_bufmgr_fake_init(int fd,
> > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> > index b776d2f..048fca7 100644
> > --- a/intel/intel_bufmgr_gem.c
> > +++ b/intel/intel_bufmgr_gem.c
> > @@ -119,6 +119,7 @@ typedef struct _drm_intel_bufmgr_gem {
> >  	unsigned int has_blt : 1;
> >  	unsigned int has_relaxed_fencing : 1;
> >  	unsigned int has_llc : 1;
> > +	unsigned int has_wait_timeout : 1;
> >  	unsigned int bo_reuse : 1;
> >  	unsigned int no_exec : 1;
> >  	bool fenced_relocs;
> > @@ -1479,6 +1480,31 @@ drm_intel_gem_bo_wait_rendering(drm_intel_bo *bo)
> >  }
> >  
> >  /**
> > + * Same as drm_intel_gem_bo_wait_rendering except a timeout parameter allows the
> > + * operation to give up after a certain amount of time.
> > + *
> > + * A 0 return value implies that the wait was successful. Otherwise some
> > + * negative return value describes the error.
> > + */
> > +int drm_intel_gem_bo_wait(drm_intel_bo *bo, int64_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 (!bufmgr_gem->has_wait_timeout) {
> > +		drm_intel_gem_bo_wait_rendering(bo);
> > +		return 0;
> > +	}
> > +
> > +	wait.bo_handle = bo_gem->gem_handle;
> > +	wait.timeout_ns = timeout_ns;
> > +	wait.flags = 0;
> > +	return drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_WAIT, &wait);
> > +}
> > +
> > +/**
> >   * Sets the object to the GTT read and possibly write domain, used by the X
> >   * 2D driver in the absence of kernel support to do drm_intel_gem_bo_map_gtt().
> >   *
> > @@ -2898,6 +2924,10 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
> >  	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp);
> >  	bufmgr_gem->has_relaxed_fencing = ret == 0;
> >  
> > +	gp.param = I915_PARAM_HAS_WAIT_TIMEOUT;
> > +	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp);
> > +	bufmgr_gem->has_wait_timeout = ret == 0;
> > +
> >  	gp.param = I915_PARAM_HAS_LLC;
> >  	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp);
> >  	if (ret != 0) {
> > -- 
> > 1.7.10.3
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 



-- 
Ben Widawsky, Intel Open Source Technology Center