[PATCH] intel: Track whether a buffer is idle to avoid trips to the kernel.

[PATCH] intel: Track whether a buffer is idle to avoid trips to the kernel.

[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
+	 * 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;
-- 
1.8.5.2

Re: [PATCH] intel: Track whether a buffer is idle to avoid trips to the kernel.

[Kenneth Graunke]

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>

Re: [PATCH] intel: Track whether a buffer is idle to avoid trips to the kernel.

[Paul Menzel]

[-- Attachment #1.1: Type: text/plain, Size: 1675 bytes --]

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

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel