| Message ID | 1389775119-26051-1-git-send-email-eric@anholt.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/15/2014 12:38 AM, Eric Anholt wrote: > I've seen a number of apps spending unreasonable amounts of time in > drm_intel_bo_busy during the buffer mapping process. > > We can't track idleness in general, in the case of buffers shared > across processes. But this should significantly reduce our overhead > for checking for busy on things like VBOs. > > Improves (unoptimized) glamor x11perf -f8text by 0.243334% +/- > 0.161498% (n=1549), which has formerly been spending about .5% of its > time hitting the kernel for drm_intel_gem_bo_busy(). > --- > > I've still got a patch outstanding on the list for valgrind-cleaning > the modesetting paths. Since we're probably rolling a release soon, > that might be nice to get in. > > intel/intel_bufmgr_gem.c | 23 ++++++++++++++++++++++- > 1 file changed, 22 insertions(+), 1 deletion(-) Nice, makes sense...if they submit a batch referencing the buffer, it might be busy, so we need to check...if not, and it isn't shared, it's definitely idle, so skip the check. I like it. Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Dear Eric, thank you for the patch. I noticed one typo. Am Mittwoch, den 15.01.2014, 00:38 -0800 schrieb Eric Anholt: > I've seen a number of apps spending unreasonable amounts of time in > drm_intel_bo_busy during the buffer mapping process. > > We can't track idleness in general, in the case of buffers shared > across processes. But this should significantly reduce our overhead > for checking for busy on things like VBOs. > > Improves (unoptimized) glamor x11perf -f8text by 0.243334% +/- > 0.161498% (n=1549), which has formerly been spending about .5% of its > time hitting the kernel for drm_intel_gem_bo_busy(). > --- > > I've still got a patch outstanding on the list for valgrind-cleaning > the modesetting paths. Since we're probably rolling a release soon, > that might be nice to get in. > > intel/intel_bufmgr_gem.c | 23 ++++++++++++++++++++++- > 1 file changed, 22 insertions(+), 1 deletion(-) > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > index 75e95e6..27ad576 100644 > --- a/intel/intel_bufmgr_gem.c > +++ b/intel/intel_bufmgr_gem.c > @@ -212,6 +212,15 @@ struct _drm_intel_bo_gem { > bool reusable; > > /** > + * Boolean of whether the GPU is definitely not accessing the buffer. > + * > + * This is only valid when reusable, since non-reusable > + * buffers are those that have been shared wth other w*i*th > + * processes, so we don't know their state. > + */ > + bool idle; > + > + /** > * Size in bytes of this buffer and its relocation descendents. > * > * Used to avoid costly tree walking in The rest looks fine. Thanks, Paul
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 75e95e6..27ad576 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -212,6 +212,15 @@ struct _drm_intel_bo_gem { bool reusable; /** + * Boolean of whether the GPU is definitely not accessing the buffer. + * + * This is only valid when reusable, since non-reusable + * buffers are those that have been shared wth other + * processes, so we don't know their state. + */ + bool idle; + + /** * Size in bytes of this buffer and its relocation descendents. * * Used to avoid costly tree walking in @@ -567,11 +576,19 @@ drm_intel_gem_bo_busy(drm_intel_bo *bo) struct drm_i915_gem_busy busy; int ret; + if (bo_gem->reusable && bo_gem->idle) + return false; + VG_CLEAR(busy); busy.handle = bo_gem->gem_handle; ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy); - + if (ret == 0) { + bo_gem->idle = !busy.busy; + return busy.busy; + } else { + return false; + } return (ret == 0 && busy.busy); } @@ -2242,6 +2259,8 @@ drm_intel_gem_bo_exec(drm_intel_bo *bo, int used, drm_intel_bo *bo = bufmgr_gem->exec_bos[i]; drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; + bo_gem->idle = false; + /* Disconnect the buffer from the validate list */ bo_gem->validate_index = -1; bufmgr_gem->exec_bos[i] = NULL; @@ -2337,6 +2356,8 @@ skip_execution: drm_intel_bo *bo = bufmgr_gem->exec_bos[i]; drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *)bo; + bo_gem->idle = false; + /* Disconnect the buffer from the validate list */ bo_gem->validate_index = -1; bufmgr_gem->exec_bos[i] = NULL;