On Jul 28, 2016 7:59 AM, "Pohjolainen, Topi" <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> wrote: > > On Tue, Jul 26, 2016 at 03:02:20PM -0700, Jason Ekstrand wrote: > > Since the dawn of time, blorp has used offsets directly to get at different > > mip levels and array slices of surfaces. This isn't really necessary since > > we can just use the base level/layer provided in the surface state. While > > it may have simplified blorp's original design, we haven't been using the > > blorp path for surface state on gen8 thanks to render compression and > > there's really no good need for it most of the time. This commit restricts > > such surface munging to the cases of fake W-tiling and fake interleaved > > multisampling. > > --- > > src/mesa/drivers/dri/i965/brw_blorp.c | 74 ++--------- > > src/mesa/drivers/dri/i965/brw_blorp.h | 6 + > > src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 185 +++++++++++++++++++-------- > > 3 files changed, 152 insertions(+), 113 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c > > index 64e507a..215f765 100644 > > --- a/src/mesa/drivers/dri/i965/brw_blorp.c > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c > > @@ -90,7 +90,7 @@ get_image_offset_sa_gen6_stencil(const struct isl_surf *surf, > > *y_offset_sa = y; > > } > > > > -static void > > +void > > I noticed that this is now only used in brw_blorp_blit.cpp, should we just > move it there instead? It gets deleted entirely later. > > blorp_get_image_offset_sa(struct isl_device *dev, const struct isl_surf *surf, > > uint32_t level, uint32_t layer, > > uint32_t *x_offset_sa, > > @@ -100,60 +100,11 @@ blorp_get_image_offset_sa(struct isl_device *dev, const struct isl_surf *surf, > > get_image_offset_sa_gen6_stencil(surf, level, layer, > > x_offset_sa, y_offset_sa); > > } else { > > - /* Using base_array_layer for Z in 3-D surfaces is a bit abusive, but it > > - * will go away soon enough. > > - */ > > - uint32_t z = 0; > > - if (surf->dim == ISL_SURF_DIM_3D) { > > - z = layer; > > - layer = 0; > > - } > > - > > - isl_surf_get_image_offset_sa(surf, level, layer, z, > > + isl_surf_get_image_offset_sa(surf, level, layer, 0, > > x_offset_sa, y_offset_sa); > > } > > } > > > > -static void > > -surf_apply_level_layer_offsets(struct isl_device *dev, struct isl_surf *surf, > > - struct isl_view *view, uint32_t *byte_offset, > > - uint32_t *tile_x_sa, uint32_t *tile_y_sa) > > -{ > > - /* This only makes sense for a single level and array slice */ > > - assert(view->levels == 1 && view->array_len == 1); > > - > > - uint32_t x_offset_sa, y_offset_sa; > > - blorp_get_image_offset_sa(dev, surf, view->base_level, > > - view->base_array_layer, > > - &x_offset_sa, &y_offset_sa); > > - > > - isl_tiling_get_intratile_offset_sa(dev, surf->tiling, view->format, > > - surf->row_pitch, x_offset_sa, y_offset_sa, > > - byte_offset, tile_x_sa, tile_y_sa); > > - > > - /* Now that that's done, we have a very bare 2-D surface */ > > - surf->dim = ISL_SURF_DIM_2D; > > - surf->dim_layout = ISL_DIM_LAYOUT_GEN4_2D; > > - > > - surf->logical_level0_px.width = > > - minify(surf->logical_level0_px.width, view->base_level); > > - surf->logical_level0_px.height = > > - minify(surf->logical_level0_px.height, view->base_level); > > - surf->logical_level0_px.depth = 1; > > - surf->logical_level0_px.array_len = 1; > > - surf->levels = 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); > > - > > - /* TODO: surf->physcal_level0_extent_sa? */ > > I was wondering about this in the introducing patch... > > > - > > - view->base_level = 0; > > - view->base_array_layer = 0; > > -} > > - > > void > > brw_blorp_surface_info_init(struct brw_context *brw, > > struct brw_blorp_surface_info *info, > > @@ -191,8 +142,6 @@ brw_blorp_surface_info_init(struct brw_context *brw, > > .format = ISL_FORMAT_UNSUPPORTED, /* Set later */ > > .base_level = level, > > This still prevents surf_convert_to_single_slice() from skipping the munging, > I think. Shouldn't we drop it also? I'm confused. If it las a level other than zero then we need the munging. The only reason to skip is because there are cases where we call convert_two_single_slice twice. > > .levels = 1, > > - .base_array_layer = layer / layer_multiplier, > > - .array_len = 1, > > .channel_select = { > > ISL_CHANNEL_SELECT_RED, > > ISL_CHANNEL_SELECT_GREEN, > > @@ -201,12 +150,21 @@ brw_blorp_surface_info_init(struct brw_context *brw, > > }, > > }; > > > > - if (brw->gen >= 8 && !is_render_target && info->surf.dim == ISL_SURF_DIM_3D) { > > - /* On gen8+ we use actual 3-D textures so we need to pass the layer > > - * through to the sampler. > > + if (!is_render_target && > > + (info->surf.dim == ISL_SURF_DIM_3D || > > + info->surf.msaa_layout == ISL_MSAA_LAYOUT_ARRAY)) { > > + /* 3-D textures don't support base_array layer and neither do 2-D > > + * multisampled textures on IVB so we need to pass it through the > > + * sampler in those cases. These are also two cases where we are > > + * guaranteed that we won't be doing any funny surface hacks. > > */ > > + info->view.base_array_layer = 0; > > + info->view.array_len = MAX2(info->surf.logical_level0_px.depth, > > + info->surf.logical_level0_px.array_len); > > info->z_offset = layer / layer_multiplier; > > } else { > > + info->view.base_array_layer = layer / layer_multiplier; > > + info->view.array_len = 1; > > info->z_offset = 0; > > } > > > > @@ -252,10 +210,6 @@ brw_blorp_surface_info_init(struct brw_context *brw, > > break; > > } > > } > > - > > - surf_apply_level_layer_offsets(&brw->isl_dev, &info->surf, &info->view, > > - &info->bo_offset, > > - &info->tile_x_sa, &info->tile_y_sa); > > } > > > > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h b/src/mesa/drivers/dri/i965/brw_blorp.h > > index ec12dfe..706d53e 100644 > > --- a/src/mesa/drivers/dri/i965/brw_blorp.h > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.h > > @@ -338,6 +338,12 @@ brw_blorp_compile_nir_shader(struct brw_context *brw, struct nir_shader *nir, > > struct brw_blorp_prog_data *prog_data, > > unsigned *program_size); > > > > +void > > +blorp_get_image_offset_sa(struct isl_device *dev, const struct isl_surf *surf, > > + uint32_t level, uint32_t layer, > > + uint32_t *x_offset_sa, > > + uint32_t *y_offset_sa); > > + > > uint32_t > > brw_blorp_emit_surface_state(struct brw_context *brw, > > const struct brw_blorp_surface_info *surface, > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp > > index a35cdb3..007c061 100644 > > --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp > > @@ -1586,6 +1586,115 @@ swizzle_to_scs(GLenum swizzle) > > return (enum isl_channel_select)((swizzle + 4) & 7); > > } > > > > +static void > > +surf_convert_to_single_slice(struct brw_context *brw, > > + struct brw_blorp_surface_info *info) > > +{ > > + /* This only makes sense for a single level and array slice */ > > + assert(info->view.levels == 1 && info->view.array_len == 1); > > + > > + /* Just bail if we have nothing to do. */ > > + if (info->surf.dim == ISL_SURF_DIM_2D && > > + info->view.base_level == 0 && info->view.base_array_layer == 0 && > > + info->surf.levels == 0 && info->surf.logical_level0_px.array_len == 0) > > + return; > > + > > + uint32_t x_offset_sa, y_offset_sa; > > + blorp_get_image_offset_sa(&brw->isl_dev, &info->surf, info->view.base_level, > > + info->view.base_array_layer, > > + &x_offset_sa, &y_offset_sa); > > + > > + isl_tiling_get_intratile_offset_sa(&brw->isl_dev, info->surf.tiling, > > + info->view.format, info->surf.row_pitch, > > + x_offset_sa, y_offset_sa, > > + &info->bo_offset, > > + &info->tile_x_sa, &info->tile_y_sa); > > + > > + /* TODO: Once this file gets converted to C, we shouls just use designated > > + * initializers. > > + */ > > + struct isl_surf_init_info init_info = isl_surf_init_info(); > > + > > + init_info.dim = ISL_SURF_DIM_2D; > > + init_info.format = ISL_FORMAT_R8_UINT; > > + init_info.width = > > + minify(info->surf.logical_level0_px.width, info->view.base_level); > > + init_info.height = > > + minify(info->surf.logical_level0_px.height, info->view.base_level); > > + init_info.depth = 1; > > + init_info.levels = 1; > > + init_info.array_len = 1; > > + init_info.samples = info->surf.samples; > > + init_info.min_pitch = info->surf.row_pitch; > > + init_info.usage = info->surf.usage; > > + init_info.tiling_flags = 1 << info->surf.tiling; > > + > > + isl_surf_init_s(&brw->isl_dev, &info->surf, &init_info); > > + assert(info->surf.row_pitch == init_info.min_pitch); > > + > > + /* The view is also different now. */ > > + info->view.base_level = 0; > > + info->view.levels = 1; > > + info->view.base_array_layer = 0; > > + info->view.array_len = 1; > > +} > > + > > +static void > > +surf_fake_interleaved_msaa(struct brw_context *brw, > > + struct brw_blorp_surface_info *info) > > +{ > > + assert(info->surf.msaa_layout == ISL_MSAA_LAYOUT_INTERLEAVED); > > + > > + /* First, we need to convert it to a simple 1-level 1-layer 2-D surface */ > > + surf_convert_to_single_slice(brw, info); > > + > > + info->surf.logical_level0_px = info->surf.phys_level0_sa; > > + info->surf.samples = 1; > > + info->surf.msaa_layout = ISL_MSAA_LAYOUT_NONE; > > +} > > + > > +static void > > +surf_retile_w_to_y(struct brw_context *brw, > > + struct brw_blorp_surface_info *info) > > +{ > > + assert(info->surf.tiling == ISL_TILING_W); > > + > > + /* First, we need to convert it to a simple 1-level 1-layer 2-D surface */ > > + surf_convert_to_single_slice(brw, info); > > + > > + /* On gen7+, we don't have interleaved multisampling for color render > > + * targets so we have to fake it. > > + * > > + * TODO: Are we sure we don't also need to fake it on gen6? > > + */ > > + if (brw->gen > 6 && info->surf.msaa_layout == ISL_MSAA_LAYOUT_INTERLEAVED) { > > + info->surf.logical_level0_px = info->surf.phys_level0_sa; > > + info->surf.samples = 1; > > + info->surf.msaa_layout = ISL_MSAA_LAYOUT_NONE; > > + } > > + > > + if (brw->gen == 6) { > > + /* Gen6 stencil buffers have a very large alignment coming in from the > > + * miptree. It's out-of-bounds for what the surface state can handle. > > + * Since we have a single layer and level, it doesn't really matter as > > + * long as we don't pass a bogus value into isl_surf_fill_state(). > > + */ > > + info->surf.image_alignment_el = isl_extent3d(4, 2, 1); > > + } > > + > > + /* Now that we've converted everything to a simple 2-D surface with only > > + * one miplevel, we can go about retiling it. > > + */ > > + const unsigned x_align = 8, y_align = info->surf.samples != 0 ? 8 : 4; > > + info->surf.tiling = ISL_TILING_Y0; > > + info->surf.logical_level0_px.width = > > + ALIGN(info->surf.logical_level0_px.width, x_align) * 2; > > + info->surf.logical_level0_px.height = > > + ALIGN(info->surf.logical_level0_px.height, y_align) / 2; > > + info->tile_x_sa *= 2; > > + info->tile_y_sa /= 2; > > +} > > + > > /** > > * Note: if the src (or dst) is a 2D multisample array texture on Gen7+ using > > * INTEL_MSAA_LAYOUT_UMS or INTEL_MSAA_LAYOUT_CMS, src_layer (dst_layer) is > > @@ -1782,7 +1891,10 @@ brw_blorp_blit_miptrees(struct brw_context *brw, > > /* For some texture types, we need to pass the layer through the sampler. */ > > params.wm_inputs.src_z = params.src.z_offset; > > > > - if (brw->gen > 6 && dst_mt->msaa_layout == INTEL_MSAA_LAYOUT_IMS) { > > + if (brw->gen > 6 && > > + params.dst.surf.msaa_layout == ISL_MSAA_LAYOUT_INTERLEAVED) { > > + assert(params.dst.surf.samples > 1); > > + > > /* We must expand the rectangle we send through the rendering pipeline, > > * to account for the fact that we are mapping the destination region as > > * single-sampled when it is in fact multisampled. We must also align > > @@ -1795,71 +1907,41 @@ brw_blorp_blit_miptrees(struct brw_context *brw, > > * If it's UMS, then we have no choice but to set up the rendering > > * pipeline as multisampled. > > */ > > - assert(params.dst.surf.msaa_layout = ISL_MSAA_LAYOUT_INTERLEAVED); > > switch (params.dst.surf.samples) { > > case 2: > > params.x0 = ROUND_DOWN_TO(params.x0 * 2, 4); > > params.y0 = ROUND_DOWN_TO(params.y0, 4); > > params.x1 = ALIGN(params.x1 * 2, 4); > > params.y1 = ALIGN(params.y1, 4); > > - params.dst.surf.logical_level0_px.width *= 2; > > break; > > case 4: > > params.x0 = ROUND_DOWN_TO(params.x0 * 2, 4); > > params.y0 = ROUND_DOWN_TO(params.y0 * 2, 4); > > params.x1 = ALIGN(params.x1 * 2, 4); > > params.y1 = ALIGN(params.y1 * 2, 4); > > - params.dst.surf.logical_level0_px.width *= 2; > > - params.dst.surf.logical_level0_px.height *= 2; > > break; > > case 8: > > params.x0 = ROUND_DOWN_TO(params.x0 * 4, 8); > > params.y0 = ROUND_DOWN_TO(params.y0 * 2, 4); > > params.x1 = ALIGN(params.x1 * 4, 8); > > params.y1 = ALIGN(params.y1 * 2, 4); > > - params.dst.surf.logical_level0_px.width *= 4; > > - params.dst.surf.logical_level0_px.height *= 2; > > break; > > case 16: > > params.x0 = ROUND_DOWN_TO(params.x0 * 4, 8); > > params.y0 = ROUND_DOWN_TO(params.y0 * 4, 8); > > params.x1 = ALIGN(params.x1 * 4, 8); > > params.y1 = ALIGN(params.y1 * 4, 8); > > - params.dst.surf.logical_level0_px.width *= 4; > > - params.dst.surf.logical_level0_px.height *= 4; > > break; > > default: > > unreachable("Unrecognized sample count in brw_blorp_blit_params ctor"); > > } > > > > - /* Gen7's rendering hardware only supports the IMS layout for depth and > > - * stencil render targets. Blorp always maps its destination surface as > > - * a color render target (even if it's actually a depth or stencil > > - * buffer). So if the destination is IMS, we'll have to map it as a > > - * single-sampled texture and interleave the samples ourselves. > > - */ > > - params.dst.surf.samples = 1; > > - params.dst.surf.msaa_layout = ISL_MSAA_LAYOUT_NONE; > > + surf_fake_interleaved_msaa(brw, &params.dst); > > > > wm_prog_key.use_kill = true; > > } > > > > if (params.dst.surf.tiling == ISL_TILING_W) { > > - /* We need to fake W-tiling with Y-tiling */ > > - params.dst.surf.tiling = ISL_TILING_Y0; > > - > > - wm_prog_key.dst_tiled_w = true; > > This is just moved further down, right? I believe so yes > > - > > - if (params.dst.surf.samples > 1) { > > - /* If the destination surface is a W-tiled multisampled stencil > > - * buffer that we're mapping as Y tiled, then we need to arrange for > > - * the WM program to run once per sample rather than once per pixel, > > - * because the memory layout of related samples doesn't match between > > - * W and Y tiling. > > - */ > > - wm_prog_key.persample_msaa_dispatch = true; > > - } > > - > > /* We must modify the rectangle we send through the rendering pipeline > > * (and the size and x/y offset of the destination surface), to account > > * for the fact that we are mapping it as Y-tiled when it is in fact > > @@ -1911,39 +1993,36 @@ brw_blorp_blit_miptrees(struct brw_context *brw, > > params.y0 = ROUND_DOWN_TO(params.y0, y_align) / 2; > > params.x1 = ALIGN(params.x1, x_align) * 2; > > params.y1 = ALIGN(params.y1, y_align) / 2; > > - params.dst.surf.logical_level0_px.width = > > - ALIGN(params.dst.surf.logical_level0_px.width, x_align) * 2; > > - params.dst.surf.logical_level0_px.height = > > - ALIGN(params.dst.surf.logical_level0_px.height, y_align) / 2; > > - params.dst.tile_x_sa *= 2; > > - params.dst.tile_y_sa /= 2; > > + > > + /* Retile the surface to Y-tiled */ > > + surf_retile_w_to_y(brw, &params.dst); > > + > > + wm_prog_key.dst_tiled_w = true; > > wm_prog_key.use_kill = true; > > + > > + if (params.dst.surf.samples > 1) { > > + /* If the destination surface is a W-tiled multisampled stencil > > + * buffer that we're mapping as Y tiled, then we need to arrange for > > + * the WM program to run once per sample rather than once per pixel, > > + * because the memory layout of related samples doesn't match between > > + * W and Y tiling. > > + */ > > + wm_prog_key.persample_msaa_dispatch = true; > > + } > > } > > > > if (brw->gen < 8 && params.src.surf.tiling == ISL_TILING_W) { > > /* On Haswell and earlier, we have to fake W-tiled sources as Y-tiled. > > * Broadwell adds support for sampling from stencil. > > - */ > > - params.src.surf.tiling = ISL_TILING_Y0; > > - > > - wm_prog_key.src_tiled_w = true; > > Same as this? Yes > > - > > - /* We must modify the size and x/y offset of the source surface to > > - * account for the fact that we are mapping it as Y-tiled when it is in > > - * fact W tiled. > > * > > * See the comments above concerning x/y offset alignment for the > > * destination surface. > > * > > * TODO: what if this makes the texture size too large? > > */ > > - const unsigned x_align = 8, y_align = params.src.surf.samples != 0 ? 8 : 4; > > - params.src.surf.logical_level0_px.width = > > - ALIGN(params.src.surf.logical_level0_px.width, x_align) * 2; > > - params.src.surf.logical_level0_px.height = > > - ALIGN(params.src.surf.logical_level0_px.height, y_align) / 2; > > - params.src.tile_x_sa *= 2; > > - params.src.tile_y_sa /= 2; > > + surf_retile_w_to_y(brw, &params.src); > > + > > + wm_prog_key.src_tiled_w = true; > > } > > > > /* tex_samples and rt_samples are the sample counts that are set up in > > -- > > 2.5.0.400.gff86faf > > > > _______________________________________________ > > mesa-dev mailing list > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>