On Friday, June 9, 2017 6:01:33 AM PDT 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.
I'm not crazy about exposing __brw_bo_busy and brw_bo_busy, with slightly
different semantics. Why not just make brw_bo_busy do:
if (bo->idle && bo->reusable)
return false;
/* otherwise query the kernel */
These days, it appears that bo->reusable is false for any buffers that
have been imported/exported via dmabuf or flink, and true otherwise.
(We might want to rename it to bo->foreign or such.)
With that change, brw_bo_busy should bypass the ioctl for most BOs, but
would still work for foreign BOs, without the caller having to worry
about it.
> 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 | 17 +++++------------
> src/mesa/drivers/dri/i965/brw_bufmgr.h | 33 ++++++++++++++++++++++++++++++---
> 2 files changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> index 67c15878d0..01590a0b0a 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> @@ -194,21 +194,14 @@ brw_bo_reference(struct brw_bo *bo)
> }
>
> int
> -brw_bo_busy(struct brw_bo *bo)
> +__brw_bo_busy(struct brw_bo *bo)
> {
> - struct brw_bufmgr *bufmgr = bo->bufmgr;
> - struct drm_i915_gem_busy busy;
> - int ret;
> + struct drm_i915_gem_busy busy = { bo->gem_handle };
>
> - memclear(busy);
> - busy.handle = bo->gem_handle;
> + drmIoctl(bo->bufmgr->fd, DRM_IOCTL_I915_GEM_BUSY, &busy);
>
> - ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_BUSY, &busy);
> - if (ret == 0) {
> - bo->idle = !busy.busy;
> - return busy.busy;
> - }
> - return false;
> + bo->idle = !busy.busy;
> + return busy.busy;
> }
>
> int
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h
> b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> index 70cc2bbc6c..3a397be695 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> @@ -240,11 +240,38 @@ int brw_bo_get_tiling(struct brw_bo *bo, uint32_t
> *tiling_mode,
> */
> int brw_bo_flink(struct brw_bo *bo, uint32_t *name);
>
> +int __brw_bo_busy(struct brw_bo *bo);
> +
> /**
> - * Returns 1 if mapping the buffer for write could cause the process
> - * to block, due to the object being active in the GPU.
> + * Returns 0 if mapping the buffer is not in active use by the gpu.
> + * If non-zero, any mapping for for write could cause the process
> + * to block, due to the object being active in the GPU. If the lower
> + * 16 bits are zero, then we can map for read without stalling.
> + *
> + * The last-known busy status of the brw_bo is checked first. This may be
> + * stale if the brw_bo has been exported to a foriegn process. If used on an
> + * exported bo, call __brw_bo_busy() directly to bypass the local check.
> */
> -int brw_bo_busy(struct brw_bo *bo);
> +static inline int brw_bo_busy(struct brw_bo *bo)
> +{
> + if (bo->idle) /* Note this may be stale if the bo is exported */
> + return 0;
> +
> + return __brw_bo_busy(bo);
> +}
I'd rather keep this as a boolean result, rather than an integer with
certain bits having particular meanings. Bonus points for changing the
return type to "bool".
> +
> +/**
> + * Returns true if mapping the buffer for read will cause the process to
> + * block (i.e. the buffer is still being writen). Note that when it
> + * returns false, the buffer may still be concurrently read by the GPU.
> + */
> +static inline int brw_bo_write_busy(struct brw_bo *bo)
> +{
> + if (bo->idle) /* Note this may be stale if the bo is exported */
> + return 0;
> +
> + return __brw_bo_busy(bo) & 0xffff;
> +}
>
> /**
> * Specify the volatility of the buffer.
This seems like a nice helper.
--Ken
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
