On Sat, Apr 23, 2016 at 8:11 AM, Pohjolainen, Topi <
topi.pohjolai...@intel.com> wrote:
> On Fri, Apr 22, 2016 at 04:19:14PM -0700, Jason Ekstrand wrote:
> > This commit is mostly mechanical except that it changes where we set the
> > swizzle. Previously, the blorp_surface_info constructor defaulted the
> > swizzle to SWIZZLE_XYZW. Now, we memset to zero and fill out the swizzle
> > when we setup the rest of the struct.
> > ---
> > src/mesa/drivers/dri/i965/brw_blorp.cpp | 77
> ++++++++++++---------------
> > src/mesa/drivers/dri/i965/brw_blorp.h | 32 +++++------
> > src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 6 ++-
> > src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 6 ++-
> > src/mesa/drivers/dri/i965/gen6_blorp.cpp | 4 +-
> > src/mesa/drivers/dri/i965/gen7_blorp.cpp | 6 +--
> > src/mesa/drivers/dri/i965/gen8_blorp.cpp | 7 +--
> > 7 files changed, 69 insertions(+), 69 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp
> b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > index e501818..cd37922 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > @@ -30,22 +30,9 @@
> >
> > #define FILE_DEBUG_FLAG DEBUG_BLORP
> >
> > -brw_blorp_surface_info::brw_blorp_surface_info()
> > - : mt(NULL),
> > - level(0),
> > - layer(0),
> > - width(0),
> > - height(0),
> > - x_offset(0),
> > - y_offset(0),
> > - map_stencil_as_y_tiled(false),
> > - num_samples(0),
> > - swizzle(SWIZZLE_XYZW)
> > -{
> > -}
> > -
> > void
> > -brw_blorp_surface_info::set(struct brw_context *brw,
> > +brw_blorp_surface_info_init(struct brw_context *brw,
> > + struct brw_blorp_surface_info *info,
> > struct intel_mipmap_tree *mt,
> > unsigned int level, unsigned int layer,
> > mesa_format format, bool is_render_target)
> > @@ -61,18 +48,20 @@ brw_blorp_surface_info::set(struct brw_context *brw,
> >
> > intel_miptree_check_level_layer(mt, level, layer);
> >
> > - this->mt = mt;
> > - this->level = level;
> > - this->layer = layer;
> > - this->width = minify(mt->physical_width0, level - mt->first_level);
> > - this->height = minify(mt->physical_height0, level - mt->first_level);
> > + info->mt = mt;
> > + info->level = level;
> > + info->layer = layer;
> > + info->width = minify(mt->physical_width0, level - mt->first_level);
> > + info->height = minify(mt->physical_height0, level - mt->first_level);
> >
> > - intel_miptree_get_image_offset(mt, level, layer, &x_offset,
> &y_offset);
> > + intel_miptree_get_image_offset(mt, level, layer,
> > + &info->x_offset, &info->y_offset);
> >
> > - this->num_samples = mt->num_samples;
> > - this->array_layout = mt->array_layout;
> > - this->map_stencil_as_y_tiled = false;
> > - this->msaa_layout = mt->msaa_layout;
> > + info->num_samples = mt->num_samples;
> > + info->array_layout = mt->array_layout;
> > + info->map_stencil_as_y_tiled = false;
> > + info->msaa_layout = mt->msaa_layout;
> > + info->swizzle = SWIZZLE_XYZW;
> >
> > if (format == MESA_FORMAT_NONE)
> > format = mt->format;
> > @@ -83,8 +72,8 @@ brw_blorp_surface_info::set(struct brw_context *brw,
> > * up for W tiling, so we'll need to use Y tiling and have the WM
> > * program swizzle the coordinates.
> > */
> > - this->map_stencil_as_y_tiled = true;
> > - this->brw_surfaceformat = brw->gen >= 8 ?
> BRW_SURFACEFORMAT_R8_UINT :
> > + info->map_stencil_as_y_tiled = true;
> > + info->brw_surfaceformat = brw->gen >= 8 ?
> BRW_SURFACEFORMAT_R8_UINT :
> >
> BRW_SURFACEFORMAT_R8_UNORM;
> > break;
> > case MESA_FORMAT_Z24_UNORM_X8_UINT:
> > @@ -98,20 +87,20 @@ brw_blorp_surface_info::set(struct brw_context *brw,
> > * pattern as long as we copy the right amount of data, so just
> map it
> > * as 8-bit BGRA.
> > */
> > - this->brw_surfaceformat = BRW_SURFACEFORMAT_B8G8R8A8_UNORM;
> > + info->brw_surfaceformat = BRW_SURFACEFORMAT_B8G8R8A8_UNORM;
> > break;
> > case MESA_FORMAT_Z_FLOAT32:
> > - this->brw_surfaceformat = BRW_SURFACEFORMAT_R32_FLOAT;
> > + info->brw_surfaceformat = BRW_SURFACEFORMAT_R32_FLOAT;
> > break;
> > case MESA_FORMAT_Z_UNORM16:
> > - this->brw_surfaceformat = BRW_SURFACEFORMAT_R16_UNORM;
> > + info->brw_surfaceformat = BRW_SURFACEFORMAT_R16_UNORM;
> > break;
> > default: {
> > if (is_render_target) {
> > assert(brw->format_supported_as_render_target[format]);
> > - this->brw_surfaceformat = brw->render_target_format[format];
> > + info->brw_surfaceformat = brw->render_target_format[format];
> > } else {
> > - this->brw_surfaceformat = brw_format_for_mesa_format(format);
> > + info->brw_surfaceformat = brw_format_for_mesa_format(format);
> > }
> > break;
> > }
> > @@ -127,21 +116,21 @@ brw_blorp_surface_info::set(struct brw_context
> *brw,
> > * directly from the adjusted offsets.
> > */
> > uint32_t
> > -brw_blorp_surface_info::compute_tile_offsets(uint32_t *tile_x,
> > - uint32_t *tile_y) const
> > +brw_blorp_compute_tile_offsets(const struct brw_blorp_surface_info
> *info,
> > + uint32_t *tile_x, uint32_t *tile_y)
> > {
> > uint32_t mask_x, mask_y;
> >
> > - intel_get_tile_masks(mt->tiling, mt->tr_mode, mt->cpp,
> > - map_stencil_as_y_tiled,
> > + intel_get_tile_masks(info->mt->tiling, info->mt->tr_mode,
> info->mt->cpp,
> > + info->map_stencil_as_y_tiled,
> > &mask_x, &mask_y);
> >
> > - *tile_x = x_offset & mask_x;
> > - *tile_y = y_offset & mask_y;
> > + *tile_x = info->x_offset & mask_x;
> > + *tile_y = info->y_offset & mask_y;
> >
> > - return intel_miptree_get_aligned_offset(mt, x_offset & ~mask_x,
> > - y_offset & ~mask_y,
> > - map_stencil_as_y_tiled);
> > + return intel_miptree_get_aligned_offset(info->mt, info->x_offset &
> ~mask_x,
> > + info->y_offset & ~mask_y,
> > +
> info->map_stencil_as_y_tiled);
> > }
> >
> >
> > @@ -159,6 +148,9 @@ brw_blorp_params::brw_blorp_params()
> > wm_prog_kernel(BRW_BLORP_NO_WM_PROG),
> > wm_prog_data(NULL)
> > {
> > + memset(&src, 0, sizeof(src));
> > + memset(&dst, 0, sizeof(dst));
> > + memset(&depth, 0, sizeof(depth));
>
> I suppose you want to play it safe and keep it the way it was. In practise
> though this would suffice:
>
> src.mt = NULL;
> dst.mt = NULL;
> depth.mt = NULL;
>
> Any logic trying to access the surface checks for the existence of the
> miptree
> pointer first - if it isn't there, the surface is regarded unused.
>
Right. A few patches later, we end up just mesetting all of
brw_blorp_params so unless we want to change that I don't think it makes a
difference.
> > color_write_disable[0] = false;
> > color_write_disable[1] = false;
> > color_write_disable[2] = false;
> > @@ -304,7 +296,8 @@ gen6_blorp_hiz_exec(struct brw_context *brw, struct
> intel_mipmap_tree *mt,
> >
> > params.hiz_op = op;
> >
> > - params.depth.set(brw, mt, level, layer, mt->format, true);
> > + brw_blorp_surface_info_init(brw, ¶ms.depth, mt, level, layer,
> > + mt->format, true);
> >
> > /* Align the rectangle primitive to 8x4 pixels.
> > *
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h
> b/src/mesa/drivers/dri/i965/brw_blorp.h
> > index e08693f..be5485c 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.h
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
> > @@ -69,18 +69,8 @@ enum {
> > BRW_BLORP_NUM_BINDING_TABLE_ENTRIES
> > };
> >
> > -class brw_blorp_surface_info
> > +struct brw_blorp_surface_info
> > {
> > -public:
> > - brw_blorp_surface_info();
> > -
> > - void set(struct brw_context *brw,
> > - struct intel_mipmap_tree *mt,
> > - unsigned int level, unsigned int layer,
> > - mesa_format format, bool is_render_target);
> > -
> > - uint32_t compute_tile_offsets(uint32_t *tile_x, uint32_t *tile_y)
> const;
> > -
> > struct intel_mipmap_tree *mt;
> >
> > /**
> > @@ -161,7 +151,7 @@ public:
> > * For MSAA surfaces, MSAA layout that should be used when setting
> up the
> > * surface state for this surface.
> > */
> > - intel_msaa_layout msaa_layout;
> > + enum intel_msaa_layout msaa_layout;
> >
> > /**
> > * In order to support cases where RGBA format is backing client
> requested
> > @@ -172,6 +162,18 @@ public:
> > int swizzle;
> > };
> >
> > +void
> > +brw_blorp_surface_info_init(struct brw_context *brw,
> > + struct brw_blorp_surface_info *info,
> > + struct intel_mipmap_tree *mt,
> > + unsigned int level, unsigned int layer,
> > + mesa_format format, bool is_render_target);
> > +
> > +uint32_t
> > +brw_blorp_compute_tile_offsets(const struct brw_blorp_surface_info
> *info,
> > + uint32_t *tile_x, uint32_t *tile_y);
> > +
> > +
> >
> > struct brw_blorp_coord_transform_params
> > {
> > @@ -230,10 +232,10 @@ public:
> > uint32_t y0;
> > uint32_t x1;
> > uint32_t y1;
> > - brw_blorp_surface_info depth;
> > + struct brw_blorp_surface_info depth;
> > uint32_t depth_format;
> > - brw_blorp_surface_info src;
> > - brw_blorp_surface_info dst;
> > + struct brw_blorp_surface_info src;
> > + struct brw_blorp_surface_info dst;
> > enum gen6_hiz_op hiz_op;
> > unsigned fast_clear_op;
> > bool color_write_disable[4];
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> > index 56f6ced..00e9e3c 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> > @@ -1897,8 +1897,10 @@ brw_blorp_blit_miptrees(struct brw_context *brw,
> >
> > brw_blorp_params params;
> >
> > - params.src.set(brw, src_mt, src_level, src_layer, src_format, false);
> > - params.dst.set(brw, dst_mt, dst_level, dst_layer, dst_format, true);
> > + brw_blorp_surface_info_init(brw, ¶ms.src, src_mt, src_level,
> > + src_layer, src_format, false);
> > + brw_blorp_surface_info_init(brw, ¶ms.dst, dst_mt, dst_level,
> > + dst_layer, dst_format, true);
> >
> > /* Even though we do multisample resolves at the time of the blit,
> OpenGL
> > * specification defines them as if they happen at the time of
> rendering,
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> > index b16102c..3c97ad6 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> > @@ -213,7 +213,8 @@ do_single_blorp_clear(struct brw_context *brw,
> struct gl_framebuffer *fb,
> > if (!encode_srgb && _mesa_get_format_color_encoding(format) ==
> GL_SRGB)
> > format = _mesa_get_srgb_format_linear(format);
> >
> > - params.dst.set(brw, irb->mt, irb->mt_level, layer, format, true);
> > + brw_blorp_surface_info_init(brw, ¶ms.dst, irb->mt, irb->mt_level,
> > + layer, format, true);
> >
> > /* Override the surface format according to the context's sRGB
> rules. */
> > params.dst.brw_surfaceformat = brw->render_target_format[format];
> > @@ -381,7 +382,8 @@ brw_blorp_resolve_color(struct brw_context *brw,
> struct intel_mipmap_tree *mt)
> >
> > brw_blorp_params params;
> >
> > - params.dst.set(brw, mt, 0 /* level */, 0 /* layer */, format, true);
> > + brw_blorp_surface_info_init(brw, ¶ms.dst, mt,
> > + 0 /* level */, 0 /* layer */, format,
> true);
> >
> > brw_get_resolve_rect(brw, mt, ¶ms.x0, ¶ms.y0,
> > ¶ms.x1, ¶ms.y1);
> > diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> > index 2d2195e..d4f271d 100644
> > --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> > @@ -372,7 +372,7 @@ gen6_blorp_emit_wm_constants(struct brw_context *brw,
> > static uint32_t
> > gen6_blorp_emit_surface_state(struct brw_context *brw,
> > const brw_blorp_params *params,
> > - const brw_blorp_surface_info *surface,
> > + const struct brw_blorp_surface_info
> *surface,
> > uint32_t read_domains, uint32_t
> write_domain)
> > {
> > uint32_t wm_surf_offset;
> > @@ -399,7 +399,7 @@ gen6_blorp_emit_surface_state(struct brw_context
> *brw,
> > surface->brw_surfaceformat << BRW_SURFACE_FORMAT_SHIFT);
> >
> > /* reloc */
> > - surf[1] = (surface->compute_tile_offsets(&tile_x, &tile_y) +
> > + surf[1] = (brw_blorp_compute_tile_offsets(surface, &tile_x, &tile_y)
> +
> > mt->bo->offset64);
> >
> > surf[2] = (0 << BRW_SURFACE_LOD_SHIFT |
> > diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > index 52fb89c..80545c2 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > @@ -160,7 +160,7 @@ gen7_blorp_emit_depth_stencil_state_pointers(struct
> brw_context *brw,
> > */
> > static uint32_t
> > gen7_blorp_emit_surface_state(struct brw_context *brw,
> > - const brw_blorp_surface_info *surface,
> > + const struct brw_blorp_surface_info
> *surface,
> > uint32_t read_domains, uint32_t
> write_domain,
> > bool is_render_target)
> > {
> > @@ -198,8 +198,8 @@ gen7_blorp_emit_surface_state(struct brw_context
> *brw,
> > surf[0] |= GEN7_SURFACE_ARYSPC_FULL;
> >
> > /* reloc */
> > - surf[1] =
> > - surface->compute_tile_offsets(&tile_x, &tile_y) +
> mt->bo->offset64;
> > + surf[1] = brw_blorp_compute_tile_offsets(surface, &tile_x, &tile_y) +
> > + mt->bo->offset64;
> >
> > /* Note that the low bits of these fields are missing, so
> > * there's the possibility of getting in trouble.
> > diff --git a/src/mesa/drivers/dri/i965/gen8_blorp.cpp
> b/src/mesa/drivers/dri/i965/gen8_blorp.cpp
> > index d94a17a..fd3bfa1 100644
> > --- a/src/mesa/drivers/dri/i965/gen8_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/gen8_blorp.cpp
> > @@ -39,7 +39,7 @@
> > */
> > static uint32_t
> > gen8_blorp_emit_surface_state(struct brw_context *brw,
> > - const brw_blorp_surface_info *surface,
> > + const struct brw_blorp_surface_info
> *surface,
> > uint32_t read_domains, uint32_t
> write_domain,
> > bool is_render_target)
> > {
> > @@ -90,7 +90,8 @@ gen8_blorp_emit_surface_state(struct brw_context *brw,
> >
> > /* reloc */
> > *((uint64_t *)&surf[8]) =
> > - surface->compute_tile_offsets(&tile_x, &tile_y) +
> mt->bo->offset64;
> > + brw_blorp_compute_tile_offsets(surface, &tile_x, &tile_y) +
> > + mt->bo->offset64;
> >
> > /* Note that the low bits of these fields are missing, so there's the
> > * possibility of getting in trouble.
> > @@ -597,7 +598,7 @@ gen8_blorp_emit_surface_states(struct brw_context
> *brw,
> > I915_GEM_DOMAIN_RENDER,
> > true /* is_render_target */);
> > if (params->src.mt) {
> > - const brw_blorp_surface_info *surface = ¶ms->src;
> > + const struct brw_blorp_surface_info *surface = ¶ms->src;
> > intel_mipmap_tree *mt = surface->mt;
> >
> > /* Textures are always sampled as 2D. */
> > --
> > 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
