On 10/02/2018 11:06 AM, Chris Wilson wrote:
> If we know the bo is idle (that is we have no submitted a command buffer
> referencing this bo since the last query) we can skip asking the kernel.
> Note this may report a false negative if the target is being shared
> between processes (exported via dmabuf or flink). To allow the caller
> control over using the last known flag, the query is split into two.
>
> v2: Check against external bo before trusting our own tracking.
>
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Kenneth Graunke <kenn...@whitecape.org>
> Cc: Matt Turner <matts...@gmail.com>
> ---
> src/mesa/drivers/dri/i965/brw_bufmgr.c | 40 ++++++++++++++++++++------
> src/mesa/drivers/dri/i965/brw_bufmgr.h | 11 +++++--
> 2 files changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> index f1675b191c1..d9e8453787c 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> @@ -444,18 +444,40 @@ vma_free(struct brw_bufmgr *bufmgr,
> }
> }
>
> -int
> +static int
> +__brw_bo_busy(struct brw_bo *bo)
> +{
> + struct drm_i915_gem_busy busy = { bo->gem_handle };
I think it makes the code more readable to keep ".handle = ..."
> +
> + if (bo->idle && !bo->external)
> + return 0;
> +
> + /* If we hit an error here, it means that bo->gem_handle is invalid.
> + * Treat it as being idle (busy.busy is left as 0) and move along.
> + */
> + drmIoctl(bo->bufmgr->fd, DRM_IOCTL_I915_GEM_BUSY, &busy);
> +
> + bo->idle = !busy.busy;
> + return busy.busy;
> +}
> +
> +bool
> brw_bo_busy(struct brw_bo *bo)
> {
> - struct brw_bufmgr *bufmgr = bo->bufmgr;
> - struct drm_i915_gem_busy busy = { .handle = bo->gem_handle };
> + return __brw_bo_busy(bo);
> +}
>
> - int ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_BUSY, &busy);
> - if (ret == 0) {
> - bo->idle = !busy.busy;
> - return busy.busy;
> - }
> - return false;
> +bool
> +brw_bo_map_busy(struct brw_bo *bo, unsigned flags)
> +{
> + unsigned mask;
> +
> + if (flags & MAP_WRITE)
> + mask = ~0u;
> + else
> + mask = 0xffff;
In other places we would do this as
const unsigned mask = (flags & MAP_WRITE) ? ~0u : 0xffff;
Unless, of course, a later patch adds more choices.
> +
> + return __brw_bo_busy(bo) & mask;
> }
>
> int
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h
> b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> index 32fc7a553c9..e1f46b091ce 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> @@ -323,10 +323,17 @@ int brw_bo_get_tiling(struct brw_bo *bo, uint32_t
> *tiling_mode,
> int brw_bo_flink(struct brw_bo *bo, uint32_t *name);
>
> /**
> - * Returns 1 if mapping the buffer for write could cause the process
> + * Returns false if mapping the buffer is not in active use by the gpu.
> + * If it returns true, any mapping for for write could cause the process
for for^W
With those nits fixed, this patch is
Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
> * to block, due to the object being active in the GPU.
> */
> -int brw_bo_busy(struct brw_bo *bo);
> +bool brw_bo_busy(struct brw_bo *bo);
> +
> +/**
> + * Returns true if mapping the buffer for the set of flags (i.e. MAP_READ or
> + * MAP_WRITE) will cause the process to block.
> + */
> +bool brw_bo_map_busy(struct brw_bo *bo, unsigned flags);
>
> /**
> * Specify the volatility of the buffer.
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
