[Mesa-dev] [PATCH v3 13/34] i965/blorp: Add a generic ISL-based surface state emit path
Jason Ekstrand
jason at jlekstrand.net
Wed Jul 6 09:26:56 UTC 2016
On Jul 5, 2016 11:13 PM, "Pohjolainen, Topi" <topi.pohjolainen at intel.com>
wrote:
>
> On Tue, Jul 05, 2016 at 07:12:49AM -0700, Jason Ekstrand wrote:
> > On Jul 5, 2016 6:32 AM, "Pohjolainen, Topi"
> > <[1]topi.pohjolainen at intel.com> wrote:
> > >
> > > On Wed, Jun 29, 2016 at 05:14:51PM -0700, Jason Ekstrand wrote:
> > > > Reviewed-by: Topi Pohjolainen <[2]topi.pohjolainen at intel.com>
> > > >
> > > > v3: Stomp more surface state fields
> > > > ---
> > > > src/mesa/drivers/dri/i965/brw_blorp.c | 163
> > ++++++++++++++++++++++++++++++++++
> > > > src/mesa/drivers/dri/i965/brw_blorp.h | 6 ++
> > > > 2 files changed, 169 insertions(+)
> > > >
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> > b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > > index 9590968..275f10c 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > > @@ -240,6 +240,169 @@ brw_blorp_compile_nir_shader(struct
> > brw_context *brw, struct nir_shader *nir,
> > > > return program;
> > > > }
> > > >
> > > > +static enum isl_msaa_layout
> > > > +get_isl_msaa_layout(enum intel_msaa_layout layout)
> > > > +{
> > > > + switch (layout) {
> > > > + case INTEL_MSAA_LAYOUT_NONE:
> > > > + return ISL_MSAA_LAYOUT_NONE;
> > > > + case INTEL_MSAA_LAYOUT_IMS:
> > > > + return ISL_MSAA_LAYOUT_INTERLEAVED;
> > > > + case INTEL_MSAA_LAYOUT_UMS:
> > > > + case INTEL_MSAA_LAYOUT_CMS:
> > > > + return ISL_MSAA_LAYOUT_ARRAY;
> > > > + default:
> > > > + unreachable("Invalid MSAA layout");
> > > > + }
> > > > +}
> > > > +
> > > > +struct surface_state_info {
> > > > + unsigned num_dwords;
> > > > + unsigned ss_align; /* Required alignment of
> > RENDER_SURFACE_STATE in bytes */
> > > > + unsigned reloc_dw;
> > > > + unsigned aux_reloc_dw;
> > > > + unsigned tex_mocs;
> > > > + unsigned rb_mocs;
> > > > +};
> > > > +
> > > > +static const struct surface_state_info surface_state_infos[] = {
> > > > + [6] = {6, 32, 1, 0},
> > > > + [7] = {8, 32, 1, 6, GEN7_MOCS_L3, GEN7_MOCS_L3},
> > > > + [8] = {13, 64, 8, 10, BDW_MOCS_WB, BDW_MOCS_PTE},
> > > > + [9] = {16, 64, 8, 10, SKL_MOCS_WB, SKL_MOCS_PTE},
> > > > +};
> > > > +
> > > > +uint32_t
> > > > +brw_blorp_emit_surface_state(struct brw_context *brw,
> > > > + const struct brw_blorp_surface_info
> > *surface,
> > > > + uint32_t read_domains, uint32_t
> > write_domain,
> > > > + bool is_render_target)
> > > > +{
> > > > + /* TODO: This should go in the context */
> > > > + struct isl_device isl_dev;
> > > > + isl_device_init(&isl_dev, brw->intelScreen->devinfo,
> > brw->has_swizzling);
> > > > +
> > > > + const struct surface_state_info ss_info =
> > surface_state_infos[brw->gen];
> > > > +
> > > > + struct isl_surf surf;
> > > > + intel_miptree_get_isl_surf(brw, surface->mt, &surf);
> > > > +
> > > > + /* Stomp surface dimensions and tiling (if needed) with info
> > from blorp */
> > > > + surf.dim = ISL_SURF_DIM_2D;
> > > > + surf.dim_layout = ISL_DIM_LAYOUT_GEN4_2D;
> > > > + surf.msaa_layout = get_isl_msaa_layout(surface->msaa_layout);
> > > > + surf.logical_level0_px.width = surface->width;
> > > > + surf.logical_level0_px.height = surface->height;
> > > > + surf.logical_level0_px.depth = 1;
> > > > + surf.logical_level0_px.array_len = 1;
> > >
> > > You added these two lines which make sense. You didn't need them
> > before, did
> > > anything change or just assertions firing?
> >
> > The newly added X/YOffset assertions require this.
> >
> > > > + surf.levels = 1;
> > > > + surf.samples = MAX2(surface->num_samples, 1);
> > > > +
> > > > + /* Alignment doesn't matter since we have 1 miplevel and 1
> > array slice so
> > > > + * just pick something that works for everybody.
> > > > + */
> > > > + surf.image_alignment_el = isl_extent3d(4, 4, 1);
> > >
> > > Same here, looks sane but makes me womnder what made you add this?
> >
> > There are two cases where we can end up with an alignment which is
> > invalid for 3D surfaces: gen6 stencil and gen9 1D. In
> > Intel_mipmap_tree, gen9 1D surfaces have an invalid halign value and
we
> > just special-case gen9 1D to use 64 everywhere. When I started using
> > ISL for offset calculations in blorp, I needed image_alignment_el to
> > have the correct value of (64, 1, 1) so I had to add that to
> > miptree_get_isl_surf which meant we now had to undo it here.
>
> But this patch would still work without this, or am I misunderstanding? We
> still call brw_blorp_compute_tile_offsets() down below. In fact, I don't
> really understand why setting image_alignment_el in miptree_get_isl_surf()
> would help isl-based offset calculation either. That is going to replace
> the blorp specific call below, and we are overriding the calculation
> of miptree_get_isl_surf() here.
As part of the blorp transition, we need isl_surf_get_image_offset_el to
work. That requires that we have assignments. Another way to think about
this is just that this gets the resulting isl_surf closer to what isl
itself would produce.
> I suppose there is just too many things happening the same time for me to
keep
> track. Some preparing for future patches and some needed locally...
Yeah... This mess is tricky...
> >
> > > > +
> > > > + if (brw->gen == 6 && surface->num_samples > 1) {
> > > > + /* Since gen6 uses INTEL_MSAA_LAYOUT_IMS, width and height
> > are measured
> > > > + * in samples. But SURFACE_STATE wants them in pixels, so
> > we need to
> > > > + * divide them each by 2.
> > > > + */
> > > > + surf.logical_level0_px.width /= 2;
> > > > + surf.logical_level0_px.height /= 2;
> > > > + }
> > > > +
> > > > + if (brw->gen == 6 && surf.image_alignment_el.height > 4) {
> > > > + /* This can happen on stencil buffers on Sandy Bridge due
to
> > the
> > > > + * single-LOD work-around. It's fairly harmless as long
as
> > we don't
> > > > + * pass a bogus value into isl_surf_fill_state().
> > > > + */
> > > > + surf.image_alignment_el = isl_extent3d(4, 2, 1);
> > >
> > > Earlier you set this unconditionally to isl_extent3d(4, 4, 1),
isn't
> > that
> > > valid?
> >
> > Yes, I believe this can go now.
> >
> > > > + }
> > > > +
> > > > + if (surface->map_stencil_as_y_tiled) {
> > > > + /* We need to fake W-tiling with Y-tiling */
> > > > + surf.tiling = ISL_TILING_Y0;
> > > > + surf.row_pitch *= 2;
> > > > + }
> > > > +
> > > > + union isl_color_value clear_color = { .u32 = { 0, 0, 0, 0 }
};
> > > > +
> > > > + struct isl_surf *aux_surf = NULL, aux_surf_s;
> > > > + uint64_t aux_offset = 0;
> > > > + enum isl_aux_usage aux_usage = ISL_AUX_USAGE_NONE;
> > > > + if (surface->mt->mcs_mt) {
> > > > + /* We should probably to similar stomping to above but
most
> > of the aux
> > > > + * surf gets ignored when we fill out the surface state
> > anyway so
> > > > + * there's no point.
> > > > + */
> > > > + intel_miptree_get_aux_isl_surf(brw, surface->mt,
> > &aux_surf_s, &aux_usage);
> > > > + aux_surf = &aux_surf_s;
> > > > + assert(surface->mt->mcs_mt->offset == 0);
> > > > + aux_offset = surface->mt->mcs_mt->bo->offset64;
> > > > +
> > > > + /* We only really need a clear color if we also have an
> > auxiliary
> > > > + * surface. Without one, it does nothing.
> > > > + */
> > > > + clear_color = intel_miptree_get_isl_clear_color(brw,
> > surface->mt);
> > > > + }
> > > > +
> > > > + struct isl_view view = {
> > > > + .format = surface->brw_surfaceformat,
> > > > + .base_level = 0,
> > > > + .levels = 1,
> > > > + .base_array_layer = 0,
> > > > + .array_len = 1,
> > > > + .channel_select = {
> > > > + ISL_CHANNEL_SELECT_RED,
> > > > + ISL_CHANNEL_SELECT_GREEN,
> > > > + ISL_CHANNEL_SELECT_BLUE,
> > > > + ISL_CHANNEL_SELECT_ALPHA,
> > > > + },
> > > > + .usage = is_render_target ?
ISL_SURF_USAGE_RENDER_TARGET_BIT
> > :
> > > > + ISL_SURF_USAGE_TEXTURE_BIT,
> > > > + };
> > > > +
> > > > + uint32_t offset, tile_x, tile_y;
> > > > + offset = brw_blorp_compute_tile_offsets(surface, &tile_x,
> > &tile_y);
> > > > +
> > > > + uint32_t surf_offset;
> > > > + uint32_t *dw = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
> > > > + ss_info.num_dwords * 4,
> > ss_info.ss_align,
> > > > + &surf_offset);
> > > > +
> > > > + const uint32_t mocs = is_render_target ? ss_info.rb_mocs :
> > ss_info.tex_mocs;
> > > > +
> > > > + isl_surf_fill_state(&isl_dev, dw, .surf = &surf, .view =
&view,
> > > > + .address = surface->mt->bo->offset64 +
> > offset,
> > > > + .aux_surf = aux_surf, .aux_usage =
> > aux_usage,
> > > > + .aux_address = aux_offset,
> > > > + .mocs = mocs, .clear_color = clear_color,
> > > > + .x_offset_sa = tile_x, .y_offset_sa =
> > tile_y);
> > > > +
> > > > + /* Emit relocation to surface contents */
> > > > + drm_intel_bo_emit_reloc(brw->[3]batch.bo,
> > > > + surf_offset + ss_info.reloc_dw * 4,
> > > > + surface->mt->bo,
> > > > + dw[ss_info.reloc_dw] -
> > surface->mt->bo->offset64,
> > > > + read_domains, write_domain);
> > > > +
> > > > + if (aux_surf) {
> > > > + /* On gen7 and prior, the bottom 12 bits of the MCS base
> > address are
> > > > + * used to store other information. This should be ok,
> > however, because
> > > > + * surface buffer addresses are always 4K page alinged.
> > > > + */
> > > > + assert((aux_offset & 0xfff) == 0);
> > > > + drm_intel_bo_emit_reloc(brw->[4]batch.bo,
> > > > + surf_offset +
ss_info.aux_reloc_dw *
> > 4,
> > > > + surface->mt->mcs_mt->bo,
> > > > + dw[ss_info.aux_reloc_dw] & 0xfff,
> > > > + read_domains, write_domain);
> > > > + }
> > > > +
> > > > + return surf_offset;
> > > > +}
> > > > +
> > > > /**
> > > > * Perform a HiZ or depth resolve operation.
> > > > *
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h
> > b/src/mesa/drivers/dri/i965/brw_blorp.h
> > > > index 7ec5875..bacdecb 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_blorp.h
> > > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
> > > > @@ -372,6 +372,12 @@ brw_blorp_compile_nir_shader(struct
> > brw_context *brw, struct nir_shader *nir,
> > > > struct brw_blorp_prog_data
> > *prog_data,
> > > > unsigned *program_size);
> > > >
> > > > +uint32_t
> > > > +brw_blorp_emit_surface_state(struct brw_context *brw,
> > > > + const struct brw_blorp_surface_info
> > *surface,
> > > > + uint32_t read_domains, uint32_t
> > write_domain,
> > > > + bool is_render_target);
> > > > +
> > > > void
> > > > gen6_blorp_init(struct brw_context *brw);
> > > >
> > > > --
> > > > 2.5.0.400.gff86faf
> > > >
> > > > _______________________________________________
> > > > mesa-dev mailing list
> > > > [5]mesa-dev at lists.freedesktop.org
> > > > [6]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> > References
> >
> > 1. mailto:topi.pohjolainen at intel.com
> > 2. mailto:topi.pohjolainen at intel.com
> > 3. http://batch.bo/
> > 4. http://batch.bo/
> > 5. mailto:mesa-dev at lists.freedesktop.org
> > 6. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160706/8ee753c8/attachment-0001.html>
More information about the mesa-dev
mailing list