<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Sat, Apr 23, 2016 at 8:11 AM, Pohjolainen, Topi <<a href="mailto:topi.pohjolainen@intel.com" target="_blank">topi.pohjolainen@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">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)); </div></div>I suppose you want to play it safe and keep it the way it was. In practise though this would suffice: <a href="http://src.mt" rel="noreferrer" target="_blank">src.mt</a> = NULL; <a href="http://dst.mt" rel="noreferrer" target="_blank">dst.mt</a> = NULL; <a href="http://depth.mt" rel="noreferrer" target="_blank">depth.mt</a> = 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. </blockquote><div> </div><div>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. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > 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, &params.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, &params.src, src_mt, src_level, > + src_layer, src_format, false); > + brw_blorp_surface_info_init(brw, &params.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, &params.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, &params.dst, mt, > + 0 /* level */, 0 /* layer */, format, true); > > brw_get_resolve_rect(brw, mt, &params.x0, &params.y0, > &params.x1, &params.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-><a href="http://src.mt" rel="noreferrer" target="_blank">src.mt</a>) { > - const brw_blorp_surface_info *surface = &params->src; > + const struct brw_blorp_surface_info *surface = &params->src; > intel_mipmap_tree *mt = surface->mt; > > /* Textures are always sampled as 2D. */ > -- > 2.5.0.400.gff86faf > </div></div>> _______________________________________________ > 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" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>