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

[Martin Peres]

On 10/08/15 14:25, Martin Peres wrote:

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 <ch...@chris-wilson.co.uk>
Cc: Kenneth Graunke <kenn...@whitecape.org>
Cc: Martin Peres <martin.pe...@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.

Forgot to add my R-b:
Reviewed-by: Martin Peres <martin.pe...@linux.intel.com>

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.

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev