[Mesa-dev] [PATCH 04/70] i965: Share the workaround bo between all contexts

[Martin Peres]

On 07/08/15 23:13, Chris Wilson wrote:
> Since the workaround bo is used strictly as a write-only buffer, we need
> only allocate one per screen and use the same one from all contexts.
>
> (The caveat here is during extension initialisation, where we write into
> and read back register values from the buffer, but that is performed only
> once for the first context - and baring synchronisation issues should not
> be a problem. Safer would be to move that also to the screen.)
>
> v2: Give the workaround bo its own init function and don't piggy back
> intel_bufmgr_init() since it is not that related.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Martin Peres <martin.peres at linux.intel.com>
> ---
>   src/mesa/drivers/dri/i965/brw_context.c      |  7 +------
>   src/mesa/drivers/dri/i965/brw_context.h      |  4 ++--
>   src/mesa/drivers/dri/i965/brw_pipe_control.c | 13 ++++---------
>   src/mesa/drivers/dri/i965/intel_screen.c     | 15 +++++++++++++++
>   src/mesa/drivers/dri/i965/intel_screen.h     |  1 +
>   5 files changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index efcd91a..ac744d7 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -819,12 +819,7 @@ brwCreateContext(gl_api api,
>         }
>      }
>   
> -   if (brw_init_pipe_control(brw, devinfo)) {
> -      *dri_ctx_error = __DRI_CTX_ERROR_NO_MEMORY;
> -      intelDestroyContext(driContextPriv);
> -      return false;
> -   }
> -
> +   brw_init_pipe_control(brw, devinfo);
>      brw_init_state(brw);
>   
>      intelInitExtensions(ctx);
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index ffdf821..166b852 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -2016,8 +2016,8 @@ gen9_use_linear_1d_layout(const struct brw_context *brw,
>                             const struct intel_mipmap_tree *mt);
>   
>   /* brw_pipe_control.c */
> -int brw_init_pipe_control(struct brw_context *brw,
> -			  const struct brw_device_info *info);
> +void brw_init_pipe_control(struct brw_context *brw,
> +                           const struct brw_device_info *info);
>   void brw_fini_pipe_control(struct brw_context *brw);
>   
>   void brw_emit_pipe_control_flush(struct brw_context *brw, uint32_t flags);
> diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.c b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> index 7ee3cb6..872bfe8 100644
> --- a/src/mesa/drivers/dri/i965/brw_pipe_control.c
> +++ b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> @@ -330,26 +330,21 @@ brw_emit_mi_flush(struct brw_context *brw)
>      brw_render_cache_set_clear(brw);
>   }
>   
> -int
> +void
>   brw_init_pipe_control(struct brw_context *brw,
>                         const struct brw_device_info *devinfo)
>   {
>      if (devinfo->gen < 6)
> -      return 0;
> +      return;
>   
>      /* We can't just use brw_state_batch to get a chunk of space for
>       * the gen6 workaround because it involves actually writing to
>       * the buffer, and the kernel doesn't let us write to the batch.
>       */
> -   brw->workaround_bo = drm_intel_bo_alloc(brw->bufmgr,
> -                                           "pipe_control workaround",
> -                                           4096, 4096);
> -   if (brw->workaround_bo == NULL)
> -      return -ENOMEM;
> +   brw->workaround_bo = brw->intelScreen->workaround_bo;
> +   drm_intel_bo_reference(brw->workaround_bo);

Why do you need reference counting here? Why not simply destroy the 
buffer when the screen is destroyed? I would assume that a screen could 
not be destroyed until all its associated contexts are gone, right?

In any case, it seems to improve performance by a few percent for 
Synmark OglBatch7.

>   
>      brw->pipe_controls_since_last_cs_stall = 0;
> -
> -   return 0;
>   }
>   
>   void
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index 147fa1e..61f1dbe 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -967,6 +967,7 @@ intelDestroyScreen(__DRIscreen * sPriv)
>   {
>      struct intel_screen *intelScreen = sPriv->driverPrivate;
>   
> +   drm_intel_bo_unreference(intelScreen->workaround_bo);
>      dri_bufmgr_destroy(intelScreen->bufmgr);
>      driDestroyOptionInfo(&intelScreen->optionCache);
>   
> @@ -1106,6 +1107,17 @@ intel_init_bufmgr(struct intel_screen *intelScreen)
>   }
>   
>   static bool
> +intel_init_workaround_bo(struct intel_screen *intelScreen)
> +{
> +   /* A small scratch bo shared by all contexts, primarily used
> +    * for doing PIPECONTROL serialisation writes that are discarded.
> +    */
> +   intelScreen->workaround_bo =
> +      drm_intel_bo_alloc(intelScreen->bufmgr, "pipe_control w/a", 4096, 4096);
> +   return intelScreen->workaround_bo != NULL;
> +}
> +
> +static bool
>   intel_detect_swizzling(struct intel_screen *screen)
>   {
>      drm_intel_bo *buffer;
> @@ -1417,6 +1429,9 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
>      if (!intel_init_bufmgr(intelScreen))
>          return false;
>   
> +   if (!intel_init_workaround_bo(intelScreen))
> +       return false;
> +
>      intelScreen->deviceID = drm_intel_bufmgr_gem_get_devid(intelScreen->bufmgr);
>      intelScreen->devinfo = brw_get_device_info(intelScreen->deviceID,
>                                                 brw_get_revision(psp->fd));
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h
> index 0bae95e..76179fc 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.h
> +++ b/src/mesa/drivers/dri/i965/intel_screen.h
> @@ -65,6 +65,7 @@ struct intel_screen
>      bool has_context_reset_notification : 1;
>   
>      dri_bufmgr *bufmgr;
> +   drm_intel_bo *workaround_bo;
>   
>      /**
>       * A unique ID for shader programs.