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

[Martin Peres]

On 06/07/15 13:33, 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.)
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   src/mesa/drivers/dri/i965/brw_pipe_control.c | 6 +++---
>   src/mesa/drivers/dri/i965/intel_screen.c     | 4 ++++
>   src/mesa/drivers/dri/i965/intel_screen.h     | 1 +
>   3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.c b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> index 7ee3cb6..05e14cd 100644
> --- a/src/mesa/drivers/dri/i965/brw_pipe_control.c
> +++ b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> @@ -341,12 +341,12 @@ brw_init_pipe_control(struct brw_context *brw,
>       * 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);
> +   brw->workaround_bo = brw->intelScreen->workaround_bo;
>      if (brw->workaround_bo == NULL)
>         return -ENOMEM;
>   
> +   drm_intel_bo_reference(brw->workaround_bo);
> +
>      brw->pipe_controls_since_last_cs_stall = 0;
>   
>      return 0;
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index 839a984..cd8e6eb 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -961,6 +961,7 @@ intelDestroyScreen(__DRIscreen * sPriv)
>   {
>      struct intel_screen *intelScreen = sPriv->driverPrivate;
>   
> +   drm_intel_bo_unreference(intelScreen->workaround_bo);

I guess this is OK because after the last context got destroyed the 
screen would get destroyed?

In any case, I don't get the point of reference counting when we 
obviously only need to free the buffer when the screen gets freed.
>      dri_bufmgr_destroy(intelScreen->bufmgr);
>      driDestroyOptionInfo(&intelScreen->optionCache);
>   
> @@ -1096,6 +1097,9 @@ intel_init_bufmgr(struct intel_screen *intelScreen)
>         return false;
>      }
>   
> +   intelScreen->workaround_bo =
> +      drm_intel_bo_alloc(intelScreen->bufmgr, "pipe_control w/a", 4096, 4096);
> +
>      return true;
>   }
>   
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h
> index 941e0fc..e55fddb 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.h
> +++ b/src/mesa/drivers/dri/i965/intel_screen.h
> @@ -60,6 +60,7 @@ struct intel_screen
>      bool has_context_reset_notification;
>   
>      dri_bufmgr *bufmgr;
> +   drm_intel_bo *workaround_bo;
>   
>      /**
>       * A unique ID for shader programs.