[Mesa-dev] [PATCH 12/33] intel/blorp: Stop using the X/YOffset field of RENDER_SURFACE_STATE
Pohjolainen, Topi
topi.pohjolainen at gmail.com
Sat Sep 3 17:11:23 UTC 2016
On Thu, Sep 01, 2016 at 08:28:38AM -0700, Jason Ekstrand wrote:
> On Sep 1, 2016 1:31 AM, "Pohjolainen, Topi"
> <[1]topi.pohjolainen at gmail.com> wrote:
> >
> > On Wed, Aug 31, 2016 at 02:22:31PM -0700, Jason Ekstrand wrote:
> > > While it can be useful, the field has substantial limtations. In
> > > particular, the bittom 2 or 3 bits is missing so your offset always
> has to
> >
> > bottom
> >
> > > be a multiple of 4 or 8. While surface alignments usually work out
> to make
> > > this ok, when you start trying to fake compressed surfaces as
> uncompressed
> > > (which we will want to do) this falls apart. The easiest solution
> is to
> > > simply align all offsets to a tile boundary and munge the regions
> we're
> > > copying to account for the intratile offset.
> > > ---
> > > src/intel/blorp/blorp_blit.c | 67
> ++++++++++++++++++++++++++++++++++++---
> > > src/intel/blorp/blorp_genX_exec.h | 4 +--
> > > src/intel/blorp/blorp_priv.h | 20 +++++++++++-
> > > 3 files changed, 82 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/src/intel/blorp/blorp_blit.c
> b/src/intel/blorp/blorp_blit.c
> > > index 2838a26..a8b38de 100644
> > > --- a/src/intel/blorp/blorp_blit.c
> > > +++ b/src/intel/blorp/blorp_blit.c
> > > @@ -49,6 +49,8 @@ struct brw_blorp_blit_vars {
> > > nir_variable *v_rect_grid;
> > > nir_variable *v_coord_transform;
> > > nir_variable *v_src_z;
> > > + nir_variable *v_src_offset;
> > > + nir_variable *v_dst_offset;
> > >
> > > /* gl_FragCoord */
> > > nir_variable *frag_coord;
> > > @@ -69,12 +71,16 @@ brw_blorp_blit_vars_init(nir_builder *b, struct
> brw_blorp_blit_vars *v,
> > > type, #name); \
> > > v->v_##name->data.interpolation = INTERP_MODE_FLAT; \
> > > v->v_##name->data.location = VARYING_SLOT_VAR0 + \
> > > - offsetof(struct brw_blorp_wm_inputs, name) / (4 *
> sizeof(float));
> > > + offsetof(struct brw_blorp_wm_inputs, name) / (4 *
> sizeof(float)); \
> > > + v->v_##name->data.location_frac = \
> > > + (offsetof(struct brw_blorp_wm_inputs, name) / sizeof(float))
> % 4;
> > >
> > > LOAD_INPUT(discard_rect, glsl_vec4_type())
> > > LOAD_INPUT(rect_grid, glsl_vec4_type())
> > > LOAD_INPUT(coord_transform, glsl_vec4_type())
> > > LOAD_INPUT(src_z, glsl_uint_type())
> > > + LOAD_INPUT(src_offset, glsl_vector_type(GLSL_TYPE_UINT, 2))
> > > + LOAD_INPUT(dst_offset, glsl_vector_type(GLSL_TYPE_UINT, 2))
> > >
> > > #undef LOAD_INPUT
> > >
> > > @@ -95,6 +101,10 @@ blorp_blit_get_frag_coords(nir_builder *b,
> > > {
> > > nir_ssa_def *coord = nir_f2i(b, nir_load_var(b,
> v->frag_coord));
> > >
> > > + /* Account for destination surface intratile offset */
After re-thinking the logic in my part and double checking with you,
I wondered if we liked to add a little helper for others:
/* Account for destination surface intratile offset.
* Transformation parameters giving translation from destination
* to source coordinates don't take into account possible
* intra-tile destination offset. Therefore it has to be first
* subtracted from the incoming coordinates. Vertices are set up
* based on coordinates containing the intra-tile offset.
*/
Anyway, patch itself:
Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > > + if (key->need_dst_offset)
> > > + coord = nir_isub(b, coord, nir_load_var(b,
> v->v_dst_offset));
> >
> > We need to subtract instead of add due to coordinate origin being
> upper-left,
> > right?
>
> Both src_offset and dst_offset are offsets into the surface where the
> part of the surface we care about is. Let's call them the physical and
> logical surfaces. gl_FragCoord is going to give the currently being
> rendered coordinate on the physical surface but we need it converted to
> a position on the logical surface before we go any further. When
> texturing, on the other hand, we get a logical position and need to
> covert it to a physical one. The conversion from logical to phi l
> physical of an addition and the other direction (which we're doing
> here) is a subtraction.
>
> > Otherwise this looks quite clear:
> >
> > Reviewed-by: Topi Pohjolainen <[2]topi.pohjolainen at intel.com>
> >
> > > +
> > > if (key->persample_msaa_dispatch) {
> > > return nir_vec3(b, nir_channel(b, coord, 0), nir_channel(b,
> coord, 1),
> > > nir_load_sample_id(b));
> > > @@ -1160,6 +1170,9 @@ brw_blorp_build_nir_shader(struct
> blorp_context *blorp,
> > > key->tex_layout);
> > > }
> > >
> > > + if (key->need_src_offset)
> > > + src_pos = nir_iadd(&b, src_pos, nir_load_var(&b,
> v.v_src_offset));
> > > +
> > > /* Now (X, Y, S) = decode_msaa(tex_samples,
> detile(tex_tiling, offset)).
> > > *
> > > * In other words: X, Y, and S now contain values which,
> when passed to
> > > @@ -1247,6 +1260,23 @@ brw_blorp_setup_coord_transform(struct
> brw_blorp_coord_transform *xform,
> > > }
> > > }
> > >
> > > +static inline void
> > > +surf_get_intratile_offset_px(struct brw_blorp_surface_info *info,
> > > + uint32_t *tile_x_px, uint32_t
> *tile_y_px)
> > > +{
> > > + if (info->surf.msaa_layout == ISL_MSAA_LAYOUT_INTERLEAVED) {
> > > + struct isl_extent2d px_size_sa =
> > > + isl_get_interleaved_msaa_px_size_sa(info->surf.samples);
> > > + assert(info->tile_x_sa % px_size_sa.width == 0);
> > > + assert(info->tile_y_sa % px_size_sa.height == 0);
> > > + *tile_x_px = info->tile_x_sa / px_size_sa.width;
> > > + *tile_y_px = info->tile_y_sa / px_size_sa.height;
> > > + } else {
> > > + *tile_x_px = info->tile_x_sa;
> > > + *tile_y_px = info->tile_y_sa;
> > > + }
> > > +}
> > > +
> > > static void
> > > surf_convert_to_single_slice(const struct isl_device *isl_dev,
> > > struct brw_blorp_surface_info *info)
> > > @@ -1283,6 +1313,14 @@ surf_convert_to_single_slice(const struct
> isl_device *isl_dev,
> > > &info->tile_x_sa,
> &info->tile_y_sa);
> > > info->addr.offset += byte_offset;
> > >
> > > + const uint32_t slice_width_px =
> > > + minify(info->surf.logical_level0_px.width,
> info->view.base_level);
> > > + const uint32_t slice_height_px =
> > > + minify(info->surf.logical_level0_px.height,
> info->view.base_level);
> > > +
> > > + uint32_t tile_x_px, tile_y_px;
> > > + surf_get_intratile_offset_px(info, &tile_x_px, &tile_y_px);
> > > +
> > > /* TODO: Once this file gets converted to C, we shouls just use
> designated
> > > * initializers.
> > > */
> > > @@ -1290,10 +1328,8 @@ surf_convert_to_single_slice(const struct
> isl_device *isl_dev,
> > >
> > > init_info.dim = ISL_SURF_DIM_2D;
> > > init_info.format = info->surf.format;
> > > - 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.width = slice_width_px + tile_x_px;
> > > + init_info.height = slice_height_px + tile_y_px;
> > > init_info.depth = 1;
> > > init_info.levels = 1;
> > > init_info.array_len = 1;
> > > @@ -1497,6 +1533,7 @@ blorp_blit(struct blorp_batch *batch,
> > > surf_fake_interleaved_msaa(batch->blorp->isl_dev,
> ¶ms.dst);
> > >
> > > wm_prog_key.use_kill = true;
> > > + wm_prog_key.need_dst_offset = true;
> > > }
> > >
> > > if (params.dst.surf.tiling == ISL_TILING_W) {
> > > @@ -1557,6 +1594,7 @@ blorp_blit(struct blorp_batch *batch,
> > >
> > > wm_prog_key.dst_tiled_w = true;
> > > wm_prog_key.use_kill = true;
> > > + wm_prog_key.need_dst_offset = true;
> > >
> > > if (params.dst.surf.samples > 1) {
> > > /* If the destination surface is a W-tiled multisampled
> stencil
> > > @@ -1581,6 +1619,7 @@ blorp_blit(struct blorp_batch *batch,
> > > surf_retile_w_to_y(batch->blorp->isl_dev, ¶ms.src);
> > >
> > > wm_prog_key.src_tiled_w = true;
> > > + wm_prog_key.need_src_offset = true;
> > > }
> > >
> > > /* tex_samples and rt_samples are the sample counts that are
> set up in
> > > @@ -1604,6 +1643,24 @@ blorp_blit(struct blorp_batch *batch,
> > > wm_prog_key.persample_msaa_dispatch = true;
> > > }
> > >
> > > + if (params.src.tile_x_sa || params.src.tile_y_sa) {
> > > + assert(wm_prog_key.need_src_offset);
> > > + surf_get_intratile_offset_px(¶ms.src,
> > > + ¶ms.wm_inputs.src_offset.x,
> > > +
> ¶ms.wm_inputs.src_offset.y);
> > > + }
> > > +
> > > + if (params.dst.tile_x_sa || params.dst.tile_y_sa) {
> > > + assert(wm_prog_key.need_dst_offset);
> > > + surf_get_intratile_offset_px(¶ms.dst,
> > > + ¶ms.wm_inputs.dst_offset.x,
> > > +
> ¶ms.wm_inputs.dst_offset.y);
> > > + params.x0 += params.wm_inputs.dst_offset.x;
> > > + params.y0 += params.wm_inputs.dst_offset.y;
> > > + params.x1 += params.wm_inputs.dst_offset.x;
> > > + params.y1 += params.wm_inputs.dst_offset.y;
> > > + }
> > > +
> > > brw_blorp_get_blit_kernel(batch->blorp, ¶ms, &wm_prog_key);
> > >
> > > params.src.view.swizzle = src_swizzle;
> > > diff --git a/src/intel/blorp/blorp_genX_exec.h
> b/src/intel/blorp/blorp_genX_exec.h
> > > index d049eb0..1129929 100644
> > > --- a/src/intel/blorp/blorp_genX_exec.h
> > > +++ b/src/intel/blorp/blorp_genX_exec.h
> > > @@ -908,9 +908,7 @@ blorp_emit_surface_state(struct blorp_batch
> *batch,
> > > isl_surf_fill_state(batch->blorp->isl_dev, state,
> > > .surf = &surf, .view = &surface->view,
> > > .aux_surf = &surface->aux_surf, .aux_usage
> = aux_usage,
> > > - .mocs = mocs, .clear_color =
> surface->clear_color,
> > > - .x_offset_sa = surface->tile_x_sa,
> > > - .y_offset_sa = surface->tile_y_sa);
> > > + .mocs = mocs, .clear_color =
> surface->clear_color);
> > >
> > > blorp_surface_reloc(batch, state_offset + ss_info.reloc_dw * 4,
> > > surface->addr, 0);
> > > diff --git a/src/intel/blorp/blorp_priv.h
> b/src/intel/blorp/blorp_priv.h
> > > index 33f197b..46ff272 100644
> > > --- a/src/intel/blorp/blorp_priv.h
> > > +++ b/src/intel/blorp/blorp_priv.h
> > > @@ -112,19 +112,27 @@ struct brw_blorp_rect_grid
> > > float pad[2];
> > > };
> > >
> > > +struct blorp_surf_offset {
> > > + uint32_t x;
> > > + uint32_t y;
> > > +};
> > > +
> > > struct brw_blorp_wm_inputs
> > > {
> > > struct brw_blorp_discard_rect discard_rect;
> > > struct brw_blorp_rect_grid rect_grid;
> > > struct brw_blorp_coord_transform coord_transform[2];
> > >
> > > + struct blorp_surf_offset src_offset;
> > > + struct blorp_surf_offset dst_offset;
> > > +
> > > /* Minimum layer setting works for all the textures types but
> texture_3d
> > > * for which the setting has no effect. Use the z-coordinate
> instead.
> > > */
> > > uint32_t src_z;
> > >
> > > /* Pad out to an integral number of registers */
> > > - uint32_t pad[3];
> > > + uint32_t pad[1];
> > > };
> > >
> > > struct brw_blorp_prog_data
> > > @@ -258,6 +266,16 @@ struct brw_blorp_blit_prog_key
> > > /* True for scaled blitting. */
> > > bool blit_scaled;
> > >
> > > + /* True if this blit operation may involve intratile offsets on
> the source.
> > > + * In this case, we need to add the offset before texturing.
> > > + */
> > > + bool need_src_offset;
> > > +
> > > + /* True if this blit operation may involve intratile offsets on
> the
> > > + * destination. In this case, we need to add the offset to
> gl_FragCoord.
> > > + */
> > > + bool need_dst_offset;
> > > +
> > > /* Scale factors between the pixel grid and the grid of
> samples. We're
> > > * using grid of samples for bilinear filetring in multisample
> scaled blits.
> > > */
> > > --
> > > 2.5.0.400.gff86faf
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > [3]mesa-dev at lists.freedesktop.org
> > > [4]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> References
>
> 1. mailto:topi.pohjolainen at gmail.com
> 2. mailto:topi.pohjolainen at intel.com
> 3. mailto:mesa-dev at lists.freedesktop.org
> 4. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list