[Mesa-dev] [PATCH 11/31] i965/blorp/genX: Add a blorp_surface_reloc helper

[Pohjolainen, Topi]

On Tue, Aug 23, 2016 at 04:42:38PM -0700, Jason Ekstrand wrote:
>    On Tue, Aug 23, 2016 at 7:06 AM, Pohjolainen, Topi
>    <[1]topi.pohjolainen at gmail.com> wrote:
> 
>    On Fri, Aug 19, 2016 at 09:55:48AM -0700, Jason Ekstrand wrote:
>    > ---
>    >Â  src/mesa/drivers/dri/i965/genX_blorp_exec.c | 38
>    ++++++++++++++++-------------
>    >Â  1 file changed, 21 insertions(+), 17 deletions(-)
>    >
>    > diff --git a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
>    b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
>    > index 32a7445..f226255 100644
>    > --- a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
>    > +++ b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
>    > @@ -61,6 +61,23 @@ blorp_emit_reloc(struct brw_context *brw, void
>    *location,
>    >Â  Â  Â }
>    >Â  }
>    >
>    > +static void
>    > +blorp_surface_reloc(struct brw_context *brw, uint32_t ss_offset,
>    > +Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  struct blorp_address address, uint32_t
>    delta)
>    > +{
>    > +Â  Â drm_intel_bo_emit_reloc(brw->[2]batch.bo, ss_offset,
>    > +Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â address.buffer,
>    address.offset + delta,
>    > +Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â address.read_domains,
>    address.write_domain);
>    > +
>    > +Â  Â uint64_t reloc_val = address.buffer->offset64 + address.offset
>    + delta;
>    > +Â  Â void *reloc_ptr = (void *)brw->batch.map + ss_offset;
>    > +#if GEN_GEN >= 8
>    > +Â  Â *(uint64_t *)reloc_ptr = reloc_val;
>    > +#else
>    > +Â  Â *(uint32_t *)reloc_ptr = reloc_val;
>    > +#endif
>    > +}
>    > +
>    >Â  static void *
>    >Â  blorp_alloc_dynamic_state(struct blorp_context *blorp,
>    >Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  enum aub_state_struct_type
>    type,
>    > @@ -951,24 +968,15 @@ blorp_emit_surface_state(struct brw_context
>    *brw,
>    >
>    >Â  Â  Â const uint32_t mocs =
>    >Â  Â  Â  Â  is_render_target ? brw->blorp.mocs.rb :
>    brw->blorp.mocs.tex;
>    > -Â  Â uint64_t aux_bo_offset =
>    > -Â  Â  Â  surface->aux_addr.buffer ? surface->aux_addr.buffer->
>    offset64 : 0;
>    >
>    >Â  Â  Â isl_surf_fill_state(&brw->isl_dev, dw, .surf = &surf, .view =
>    &surface->view,
>    > -Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â .address =
>    surface->addr.buffer->offset64 + surface->addr.offset,
>    >Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â .aux_surf = &surface->aux_surf,
>    .aux_usage = aux_usage,
>    > -Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â .aux_address = aux_bo_offset +
>    surface->aux_addr.offset,
> 
>      Should you have dropped ".address" and ".aux_address" already in the
>      previous
>      patch?
>      Otherwise the patch makes sense.
> 
>    No.  One of the big changes here is that the blorp_surface_reloc
>    helper writes the relocated value into the buffer.  Previously, we
>    were depending on isl to write the value for us and
>    drm_intel_bo_emit_reloc only added the relocation to the list.
>    Maybe that needs to go in the commit message?

Ok. Thanks for explaining it - my confusion went all the way back to patch
10. I totally missed the fact that the value given for
drm_intel_bo_emit_reloc() now gets calculated directly instead of using the
value set for the surface state (and subtracting the buffer offset).

Having that in the commit message sounds really good.