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

[Kenneth Graunke]

On Monday, July 06, 2015 11:33:08 AM 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;

Checking for out-of-memory conditions in code that doesn't actually
allocate anything looks funky now.  I'd be inclined just to drop the
-ENOMEM path and make this a void function.

Alternatively, you could just fold this into the brw_context setup and
ditch the functions altogether.  Up to you.

>  
> +   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);
>     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);
> +

Seems a little funny to put this in intel_init_bufmgr, since it's not
setting up libdrm - why not put it in the caller?

>     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.
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150706/771d60a8/attachment.sig>