Re: [Mesa-dev] [PATCH 11/31] i965/blorp/genX: Add a blorp_surface_reloc helper
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.pohjolai...@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(>isl_dev, dw, .surf = , .view =
>>view,
>> -Ã Ã Ã Ã Ã Ã Ã Ã Ã Ã Ã Ã .address =
>surface->addr.buffer->offset64 + surface->addr.offset,
>>Ã Ã Ã Ã Ã Ã Ã Ã Ã Ã Ã Ã Ã .aux_surf =
> >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.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/31] i965/blorp/genX: Add a blorp_surface_reloc helper
On Tue, Aug 23, 2016 at 7:06 AM, Pohjolainen, Topi <
topi.pohjolai...@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->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(>isl_dev, dw, .surf = , .view =
> >view,
> > - .address = surface->addr.buffer->offset64 +
> surface->addr.offset,
> > .aux_surf = >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?
--Jason
> > .mocs = mocs, .clear_color =
> surface->clear_color,
> > .x_offset_sa = surface->tile_x_sa,
> > .y_offset_sa = surface->tile_y_sa);
> >
> > - /* Emit relocation to surface contents */
> > - drm_intel_bo_emit_reloc(brw->batch.bo,
> > - surf_offset + ss_info.reloc_dw * 4,
> > - surface->addr.buffer,
> > - dw[ss_info.reloc_dw] - surface->addr.buffer->
> offset64,
> > - surface->addr.read_domains,
> > - surface->addr.write_domain);
> > + blorp_surface_reloc(brw, surf_offset + ss_info.reloc_dw * 4,
> > + surface->addr, 0);
> >
> > if (aux_usage != ISL_AUX_USAGE_NONE) {
> >/* On gen7 and prior, the bottom 12 bits of the MCS base address
> are
> > @@ -976,12 +984,8 @@ blorp_emit_surface_state(struct brw_context *brw,
> > * surface buffer addresses are always 4K page alinged.
> > */
> >assert((surface->aux_addr.offset & 0xfff) == 0);
> > - drm_intel_bo_emit_reloc(brw->batch.bo,
> > - surf_offset + ss_info.aux_reloc_dw * 4,
> > - surface->aux_addr.buffer,
> > - dw[ss_info.aux_reloc_dw] & 0xfff,
> > - surface->aux_addr.read_domains,
> > - surface->aux_addr.write_domain);
> > + blorp_surface_reloc(brw, surf_offset + ss_info.aux_reloc_dw * 4,
> > + surface->aux_addr, dw[ss_info.aux_reloc_dw]);
> > }
> >
> > return surf_offset;
> > --
> > 2.5.0.400.gff86faf
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/31] i965/blorp/genX: Add a blorp_surface_reloc helper
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->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(>isl_dev, dw, .surf = , .view =
> >view,
> - .address = surface->addr.buffer->offset64 +
> surface->addr.offset,
> .aux_surf = >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.
> .mocs = mocs, .clear_color = surface->clear_color,
> .x_offset_sa = surface->tile_x_sa,
> .y_offset_sa = surface->tile_y_sa);
>
> - /* Emit relocation to surface contents */
> - drm_intel_bo_emit_reloc(brw->batch.bo,
> - surf_offset + ss_info.reloc_dw * 4,
> - surface->addr.buffer,
> - dw[ss_info.reloc_dw] -
> surface->addr.buffer->offset64,
> - surface->addr.read_domains,
> - surface->addr.write_domain);
> + blorp_surface_reloc(brw, surf_offset + ss_info.reloc_dw * 4,
> + surface->addr, 0);
>
> if (aux_usage != ISL_AUX_USAGE_NONE) {
>/* On gen7 and prior, the bottom 12 bits of the MCS base address are
> @@ -976,12 +984,8 @@ blorp_emit_surface_state(struct brw_context *brw,
> * surface buffer addresses are always 4K page alinged.
> */
>assert((surface->aux_addr.offset & 0xfff) == 0);
> - drm_intel_bo_emit_reloc(brw->batch.bo,
> - surf_offset + ss_info.aux_reloc_dw * 4,
> - surface->aux_addr.buffer,
> - dw[ss_info.aux_reloc_dw] & 0xfff,
> - surface->aux_addr.read_domains,
> - surface->aux_addr.write_domain);
> + blorp_surface_reloc(brw, surf_offset + ss_info.aux_reloc_dw * 4,
> + surface->aux_addr, dw[ss_info.aux_reloc_dw]);
> }
>
> return surf_offset;
> --
> 2.5.0.400.gff86faf
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 11/31] i965/blorp/genX: Add a blorp_surface_reloc helper
---
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->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(>isl_dev, dw, .surf = , .view =
>view,
- .address = surface->addr.buffer->offset64 +
surface->addr.offset,
.aux_surf = >aux_surf, .aux_usage = aux_usage,
- .aux_address = aux_bo_offset + surface->aux_addr.offset,
.mocs = mocs, .clear_color = surface->clear_color,
.x_offset_sa = surface->tile_x_sa,
.y_offset_sa = surface->tile_y_sa);
- /* Emit relocation to surface contents */
- drm_intel_bo_emit_reloc(brw->batch.bo,
- surf_offset + ss_info.reloc_dw * 4,
- surface->addr.buffer,
- dw[ss_info.reloc_dw] -
surface->addr.buffer->offset64,
- surface->addr.read_domains,
- surface->addr.write_domain);
+ blorp_surface_reloc(brw, surf_offset + ss_info.reloc_dw * 4,
+ surface->addr, 0);
if (aux_usage != ISL_AUX_USAGE_NONE) {
/* On gen7 and prior, the bottom 12 bits of the MCS base address are
@@ -976,12 +984,8 @@ blorp_emit_surface_state(struct brw_context *brw,
* surface buffer addresses are always 4K page alinged.
*/
assert((surface->aux_addr.offset & 0xfff) == 0);
- drm_intel_bo_emit_reloc(brw->batch.bo,
- surf_offset + ss_info.aux_reloc_dw * 4,
- surface->aux_addr.buffer,
- dw[ss_info.aux_reloc_dw] & 0xfff,
- surface->aux_addr.read_domains,
- surface->aux_addr.write_domain);
+ blorp_surface_reloc(brw, surf_offset + ss_info.aux_reloc_dw * 4,
+ surface->aux_addr, dw[ss_info.aux_reloc_dw]);
}
return surf_offset;
--
2.5.0.400.gff86faf
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
