[Mesa-dev] [PATCH 2/2] RFC i965: Share the GTT between non-robust contexts

[Jason Ekstrand]

On Fri, Dec 15, 2017 at 5:42 AM, Chris Wilson <chris at chris-wilson.co.uk>
wrote:

> Every client (everyone instance that opens /dev/dri/card0 or the render
> nodes), receives a unique per-process GTT (where supported by the
> hardware, unfortunately that means only Broadwell and later). Every
> context created by each client, in turns receives its own unique ppGTT.
> This is overkill in terms of allocations and tracking, both in the
> kernel and in the hardware, as we could be sharing the per-client GTT
> amongst all of its contexts. The downside is that context segregation is
> reduced, a stray write from one context may affect another, and so we
> must honour any client requests that require robust segregation (e.g.
> ARB_robustness).
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  src/mesa/drivers/dri/i965/brw_bufmgr.c  | 23 +++++++++++++++++------
>  src/mesa/drivers/dri/i965/brw_bufmgr.h  |  2 +-
>  src/mesa/drivers/dri/i965/brw_context.c |  2 +-
>  3 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> index 52b5bf97a1..d8a9635f5d 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> @@ -1297,13 +1297,24 @@ init_cache_buckets(struct brw_bufmgr *bufmgr)
>  }
>
>  uint32_t
> -brw_create_hw_context(struct brw_bufmgr *bufmgr)
> +brw_create_hw_context(struct brw_bufmgr *bufmgr, unsigned int flags)
>  {
> -   struct drm_i915_gem_context_create create = { };
> -   int ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE,
> &create);
> -   if (ret != 0) {
> -      DBG("DRM_IOCTL_I915_GEM_CONTEXT_CREATE failed: %s\n",
> strerror(errno));
> -      return 0;
> +   struct local_i915_gem_context_create_v2 {
> +      uint32_t ctx_id; /* out */
> +      uint32_t flags;
> +#define I915_GEM_CONTEXT_SHARE_GTT 0x1
> +      uint32_t share_ctx;
>

So, we've left share_ctx as 0.  What does that mean?  Does that mean that
we share with some context that was implicitly created by opening
/dev/dri/card0?  Does it mean we share with the default context and
therefore other processes?  If some other component (say, vaapi) opens
/dev/dri/card0 but from the same process, are we now sharing with it?

I mean, as far as the kernel UABI bits go, it seems perfectly reasonable.
There are just a lot of crazy implications here and I'd like to understand
the subtle details better.

--Jason


> +      uint32_t pad;
> +   } create =  { };
> +#define LOCAL_IOCTL_I915_GEM_CONTEXT_CREATE      DRM_IOWR
> (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_CREATE, struct
> local_i915_gem_context_create_v2)
> +
> +   if (!(flags & __DRI_CTX_FLAG_ROBUST_BUFFER_ACCESS))
> +      create.flags |= I915_GEM_CONTEXT_SHARE_GTT;
> +
> +   if (drmIoctl(bufmgr->fd, LOCAL_IOCTL_I915_GEM_CONTEXT_CREATE,
> &create)) {
> +      create.flags = 0;
> +      if (drmIoctl(bufmgr->fd, LOCAL_IOCTL_I915_GEM_CONTEXT_CREATE,
> &create))
> +         return 0;
>     }
>
>     return create.ctx_id;
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h
> b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> index 0ae541cda0..f5191aff76 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> @@ -321,7 +321,7 @@ void brw_bufmgr_enable_reuse(struct brw_bufmgr
> *bufmgr);
>
>  int brw_bo_wait(struct brw_bo *bo, int64_t timeout_ns);
>
> -uint32_t brw_create_hw_context(struct brw_bufmgr *bufmgr);
> +uint32_t brw_create_hw_context(struct brw_bufmgr *bufmgr, unsigned int
> flags);
>
>  #define BRW_CONTEXT_LOW_PRIORITY ((I915_CONTEXT_MIN_USER_PRIORITY-1)/2)
>  #define BRW_CONTEXT_MEDIUM_PRIORITY (I915_CONTEXT_DEFAULT_PRIORITY)
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index 9e0f875b27..e374236b6b 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -986,7 +986,7 @@ brwCreateContext(gl_api api,
>      * This is required for transform feedback buffer offsets, query
> objects,
>      * and also allows us to reduce how much state we have to emit.
>      */
> -   brw->hw_ctx = brw_create_hw_context(brw->bufmgr);
> +   brw->hw_ctx = brw_create_hw_context(brw->bufmgr, ctx_config->flags);
>     if (!brw->hw_ctx && devinfo->gen >= 6) {
>        fprintf(stderr, "Failed to create hardware context.\n");
>        intelDestroyContext(driContextPriv);
> --
> 2.15.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171215/bbb95a28/attachment.html>