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

Martin Peres martin.peres at linux.intel.com
Mon Aug 10 04:26:29 PDT 2015



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 <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.

Forgot to add my R-b:
Reviewed-by: Martin Peres <martin.peres at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev



More information about the mesa-dev mailing list