On Jul 26, 2016 10:39 PM, "Pohjolainen, Topi" <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> wrote: > > On Tue, Jul 26, 2016 at 03:02:06PM -0700, Jason Ekstrand wrote: > > Eventually, this will be the actual view that gets passed into isl to > > create the surface state. For now, we just use it for the format and the > > swizzle. > > --- > > src/mesa/drivers/dri/i965/brw_blorp.c | 38 +++++++++++++++++++-------- > > src/mesa/drivers/dri/i965/brw_blorp.h | 16 ++--------- > > src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 34 ++++++++++++++++++++---- > > src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 2 +- > > src/mesa/drivers/dri/i965/gen8_blorp.c | 29 ++++---------------- > > 5 files changed, 64 insertions(+), 55 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c > > index 8f7690c..ef256a7 100644 > > --- a/src/mesa/drivers/dri/i965/brw_blorp.c > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c > > @@ -43,9 +43,11 @@ brw_blorp_surface_info_init(struct brw_context *brw, > > * using INTEL_MSAA_LAYOUT_UMS or INTEL_MSAA_LAYOUT_CMS, then it had better > > * be a multiple of num_samples. > > */ > > + unsigned layer_multiplier = 1; > > if (mt->msaa_layout == INTEL_MSAA_LAYOUT_UMS || > > mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS) { > > assert(mt->num_samples <= 1 || layer % mt->num_samples == 0); > > + layer_multiplier = MAX2(mt->num_samples, 1); > > } > > > > intel_miptree_check_level_layer(mt, level, layer); > > @@ -61,13 +63,27 @@ brw_blorp_surface_info_init(struct brw_context *brw, > > info->aux_usage = ISL_AUX_USAGE_NONE; > > } > > > > + info->view = (struct isl_view) { > > + .usage = is_render_target ? ISL_SURF_USAGE_RENDER_TARGET_BIT : > > + ISL_SURF_USAGE_TEXTURE_BIT, > > + .format = ISL_FORMAT_UNSUPPORTED, /* Set later */ > > + .base_level = level, > > + .levels = 1, > > + .base_array_layer = layer / layer_multiplier, > > + .array_len = 1, > > + .channel_select = { > > + ISL_CHANNEL_SELECT_RED, > > + ISL_CHANNEL_SELECT_GREEN, > > + ISL_CHANNEL_SELECT_BLUE, > > + ISL_CHANNEL_SELECT_ALPHA, > > + }, > > + }; > > + > > 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); > > > > - info->swizzle = SWIZZLE_XYZW; > > - > > if (format == MESA_FORMAT_NONE) > > format = mt->format; > > > > @@ -75,8 +91,8 @@ brw_blorp_surface_info_init(struct brw_context *brw, > > case MESA_FORMAT_S_UINT8: > > assert(info->surf.tiling == ISL_TILING_W); > > /* Prior to Broadwell, we can't render to R8_UINT */ > > - info->brw_surfaceformat = brw->gen >= 8 ? BRW_SURFACEFORMAT_R8_UINT : > > - BRW_SURFACEFORMAT_R8_UNORM; > > + info->view.format = brw->gen >= 8 ? BRW_SURFACEFORMAT_R8_UINT : > > + BRW_SURFACEFORMAT_R8_UNORM; > > Should we use ISL_FORMAT_ instead? Or at least the cast. Assigning an enum > with another always looks bad unless it is clear they happen to have exact > same values. Somewhere in the series that gets cleaned up. Eventually we need to just kill BRW_SURFACEFORMAT all together. > > break; > > case MESA_FORMAT_Z24_UNORM_X8_UINT: > > /* It would make sense to use BRW_SURFACEFORMAT_R24_UNORM_X8_TYPELESS > > @@ -89,20 +105,20 @@ brw_blorp_surface_info_init(struct brw_context *brw, > > * pattern as long as we copy the right amount of data, so just map it > > * as 8-bit BGRA. > > */ > > - info->brw_surfaceformat = BRW_SURFACEFORMAT_B8G8R8A8_UNORM; > > + info->view.format = BRW_SURFACEFORMAT_B8G8R8A8_UNORM; > > break; > > case MESA_FORMAT_Z_FLOAT32: > > - info->brw_surfaceformat = BRW_SURFACEFORMAT_R32_FLOAT; > > + info->view.format = BRW_SURFACEFORMAT_R32_FLOAT; > > break; > > case MESA_FORMAT_Z_UNORM16: > > - info->brw_surfaceformat = BRW_SURFACEFORMAT_R16_UNORM; > > + info->view.format = BRW_SURFACEFORMAT_R16_UNORM; > > break; > > default: { > > if (is_render_target) { > > assert(brw->format_supported_as_render_target[format]); > > - info->brw_surfaceformat = brw->render_target_format[format]; > > + info->view.format = brw->render_target_format[format]; > > Perhaps use the cast such as you do further down in the patch: > > info->view.format = > (enum isl_format)brw->render_target_format[format]; The reason I do further down is because it's C++. Again, this will get cleaned up eventually... > > } else { > > - info->brw_surfaceformat = brw_format_for_mesa_format(format); > > + info->view.format = brw_format_for_mesa_format(format); > > } > > break; > > } > > @@ -111,7 +127,7 @@ brw_blorp_surface_info_init(struct brw_context *brw, > > uint32_t x_offset, y_offset; > > intel_miptree_get_image_offset(mt, level, layer, &x_offset, &y_offset); > > > > - uint8_t bs = isl_format_get_layout(info->brw_surfaceformat)->bpb / 8; > > + uint8_t bs = isl_format_get_layout(info->view.format)->bpb / 8; > > isl_tiling_get_intratile_offset_el(&brw->isl_dev, info->surf.tiling, bs, > > info->surf.row_pitch, x_offset, y_offset, > > &info->bo_offset, > > @@ -287,7 +303,7 @@ brw_blorp_emit_surface_state(struct brw_context *brw, > > } > > > > struct isl_view view = { > > - .format = surface->brw_surfaceformat, > > + .format = surface->view.format, > > .base_level = 0, > > .levels = 1, > > .base_array_layer = 0, > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h b/src/mesa/drivers/dri/i965/brw_blorp.h > > index e591f41..185406e 100644 > > --- a/src/mesa/drivers/dri/i965/brw_blorp.h > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.h > > @@ -76,6 +76,8 @@ struct brw_blorp_surface_info > > struct isl_surf aux_surf; > > enum isl_aux_usage aux_usage; > > > > + struct isl_view view; > > + > > /** > > * The miplevel to use. > > */ > > @@ -106,20 +108,6 @@ struct brw_blorp_surface_info > > > > uint32_t bo_offset; > > uint32_t tile_x_sa, tile_y_sa; > > - > > - /** > > - * Format that should be used when setting up the surface state for this > > - * surface. Should correspond to one of the BRW_SURFACEFORMAT_* enums. > > - */ > > - uint32_t brw_surfaceformat; > > - > > - /** > > - * In order to support cases where RGBA format is backing client requested > > - * RGB, one needs to have means to force alpha channel to one when user > > - * requested RGB surface is used as blit source. This is possible by > > - * setting source swizzle for the texture surface. > > - */ > > - int swizzle; > > }; > > > > void > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp > > index fc0aada..32450aa 100644 > > --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp > > @@ -1575,6 +1575,25 @@ get_isl_msaa_layout(unsigned samples, enum intel_msaa_layout layout) > > } > > > > /** > > + * Convert an swizzle enumeration (i.e. SWIZZLE_X) to one of the Gen7.5+ > > + * "Shader Channel Select" enumerations (i.e. HSW_SCS_RED). The mappings are > > + * > > + * SWIZZLE_X, SWIZZLE_Y, SWIZZLE_Z, SWIZZLE_W, SWIZZLE_ZERO, SWIZZLE_ONE > > + * 0 1 2 3 4 5 > > + * 4 5 6 7 0 1 > > + * SCS_RED, SCS_GREEN, SCS_BLUE, SCS_ALPHA, SCS_ZERO, SCS_ONE > > + * > > + * which is simply adding 4 then modding by 8 (or anding with 7). > > + * > > + * We then may need to apply workarounds for textureGather hardware bugs. > > + */ > > +static enum isl_channel_select > > +swizzle_to_scs(GLenum swizzle) > > +{ > > + return (enum isl_channel_select)((swizzle + 4) & 7); > > +} > > + > > +/** > > * 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 > > * the physical layer holding sample 0. So, for example, if > > @@ -1651,8 +1670,10 @@ brw_blorp_blit_miptrees(struct brw_context *brw, > > _mesa_get_srgb_format_linear(src_mt->format) == > > _mesa_get_srgb_format_linear(dst_mt->format)) { > > assert(brw->format_supported_as_render_target[dst_mt->format]); > > - params.dst.brw_surfaceformat = brw->render_target_format[dst_mt->format]; > > - params.src.brw_surfaceformat = brw_format_for_mesa_format(dst_mt->format); > > + params.dst.view.format = > > + (enum isl_format)brw->render_target_format[dst_mt->format]; > > + params.src.view.format = > > + (enum isl_format)brw_format_for_mesa_format(dst_mt->format); > > } > > > > /* When doing a multisample resolve of a GL_LUMINANCE32F or GL_INTENSITY32F > > @@ -1667,8 +1688,8 @@ brw_blorp_blit_miptrees(struct brw_context *brw, > > if (brw->gen == 6 && > > params.src.surf.samples > 1 && params.dst.surf.samples <= 1 && > > src_mt->format == dst_mt->format && > > - params.dst.brw_surfaceformat == BRW_SURFACEFORMAT_R32_FLOAT) { > > - params.src.brw_surfaceformat = params.dst.brw_surfaceformat; > > + params.dst.view.format == ISL_FORMAT_R32_FLOAT) { > > + params.src.view.format = params.dst.view.format; > > } > > > > struct brw_blorp_blit_prog_key wm_prog_key; > > @@ -1951,7 +1972,10 @@ brw_blorp_blit_miptrees(struct brw_context *brw, > > > > brw_blorp_get_blit_kernel(brw, &params, &wm_prog_key); > > > > - params.src.swizzle = src_swizzle; > > + for (unsigned i = 0; i < 4; i++) { > > + params.src.view.channel_select[i] = > > + swizzle_to_scs(GET_SWZ(src_swizzle, i)); > > + } > > > > brw_blorp_exec(brw, &params); > > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp > > index 1e00719..625bace 100644 > > --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp > > @@ -216,7 +216,7 @@ do_single_blorp_clear(struct brw_context *brw, struct gl_framebuffer *fb, > > layer, format, true); > > > > /* Override the surface format according to the context's sRGB rules. */ > > - params.dst.brw_surfaceformat = brw->render_target_format[format]; > > + params.dst.view.format = (enum isl_format)brw->render_target_format[format]; > > > > const char *clear_type; > > if (is_fast_clear) > > diff --git a/src/mesa/drivers/dri/i965/gen8_blorp.c b/src/mesa/drivers/dri/i965/gen8_blorp.c > > index 99adebe..806895c 100644 > > --- a/src/mesa/drivers/dri/i965/gen8_blorp.c > > +++ b/src/mesa/drivers/dri/i965/gen8_blorp.c > > @@ -475,25 +475,6 @@ gen8_blorp_emit_depth_stencil_state(struct brw_context *brw, > > ADVANCE_BATCH(); > > } > > > > -/** > > - * Convert an swizzle enumeration (i.e. SWIZZLE_X) to one of the Gen7.5+ > > - * "Shader Channel Select" enumerations (i.e. HSW_SCS_RED). The mappings are > > - * > > - * SWIZZLE_X, SWIZZLE_Y, SWIZZLE_Z, SWIZZLE_W, SWIZZLE_ZERO, SWIZZLE_ONE > > - * 0 1 2 3 4 5 > > - * 4 5 6 7 0 1 > > - * SCS_RED, SCS_GREEN, SCS_BLUE, SCS_ALPHA, SCS_ZERO, SCS_ONE > > - * > > - * which is simply adding 4 then modding by 8 (or anding with 7). > > - * > > - * We then may need to apply workarounds for textureGather hardware bugs. > > - */ > > -static unsigned > > -swizzle_to_scs(GLenum swizzle) > > -{ > > - return (swizzle + 4) & 7; > > -} > > - > > static uint32_t > > gen8_blorp_emit_surface_states(struct brw_context *brw, > > const struct brw_blorp_params *params) > > @@ -533,16 +514,16 @@ gen8_blorp_emit_surface_states(struct brw_context *brw, > > surface->layer / layer_divider : 0; > > > > struct isl_view view = { > > - .format = surface->brw_surfaceformat, > > + .format = surface->view.format, > > .base_level = surface->level, > > .levels = mt->last_level - surface->level + 1, > > .base_array_layer = layer, > > .array_len = depth - layer, > > .channel_select = { > > - swizzle_to_scs(GET_SWZ(surface->swizzle, 0)), > > - swizzle_to_scs(GET_SWZ(surface->swizzle, 1)), > > - swizzle_to_scs(GET_SWZ(surface->swizzle, 2)), > > - swizzle_to_scs(GET_SWZ(surface->swizzle, 3)), > > + surface->view.channel_select[0], > > + surface->view.channel_select[1], > > + surface->view.channel_select[2], > > + surface->view.channel_select[3], > > }, > > .usage = ISL_SURF_USAGE_TEXTURE_BIT, > > }; > > -- > > 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>