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