On Tue, May 01, 2018 at 02:34:15PM -0700, Jason Ekstrand wrote:
> On Wed, Mar 7, 2018 at 5:08 AM, Pohjolainen, Topi <
> topi.pohjolai...@gmail.com> wrote:
>
> > On Fri, Jan 26, 2018 at 05:59:55PM -0800, Jason Ekstrand wrote:
> > > By making use of the NIR helper for uint vector casts, we should now be
> > > able to bitcast between any two uint formats so long as their channels
> > > are in RGBA order (possibly with channels missing). In order to do this
> > > we need to rework the key a bit to pass the actual formats instead of
> > > just the number of bits in each.
> > > ---
> > > src/intel/blorp/blorp_blit.c | 105 ++++++++++++++++++++++++++++++
> > +++++--------
> > > src/intel/blorp/blorp_priv.h | 13 +++---
> > > 2 files changed, 95 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/src/intel/blorp/blorp_blit.c b/src/intel/blorp/blorp_blit.c
> > > index ea0687f..321b3e3 100644
> > > --- a/src/intel/blorp/blorp_blit.c
> > > +++ b/src/intel/blorp/blorp_blit.c
> > > @@ -871,17 +871,71 @@ bit_cast_color(struct nir_builder *b, nir_ssa_def
> > *color,
> > > {
> > > assert(key->texture_data_type == nir_type_uint);
> > >
> > > - /* We don't actually know how many source channels we have and NIR
> > will
> > > - * assert if the number of destination channels ends up being more
> > than 4.
> > > - * Choose the largest number of source channels that won't over-fill
> > a
> > > - * destination vec4.
> > > - */
> > > - const unsigned src_channels =
> > > - MIN2(4, (4 * key->dst_bpc) / key->src_bpc);
> > > - color = nir_channels(b, color, (1 << src_channels) - 1);
> > > + if (key->src_format == key->dst_format)
> > > + return color;
> > >
> > > - color = nir_format_bitcast_uint_vec_unmasked(b, color, key->src_bpc,
> > > - key->dst_bpc);
> > > + const struct isl_format_layout *src_fmtl =
> > > + isl_format_get_layout(key->src_format);
> > > + const struct isl_format_layout *dst_fmtl =
> > > + isl_format_get_layout(key->dst_format);
> > > +
> > > + /* They must be uint formats with the same bit size */
> > > + assert(src_fmtl->bpb == dst_fmtl->bpb);
> > > + assert(src_fmtl->channels.r.type == ISL_UINT);
> > > + assert(dst_fmtl->channels.r.type == ISL_UINT);
> > > +
> > > + /* They must be in regular color formats (no luminance or alpha) */
> > > + assert(src_fmtl->channels.r.bits > 0);
> > > + assert(dst_fmtl->channels.r.bits > 0);
> > > +
> > > + /* They must be in RGBA order (possibly with channels missing) */
> > > + assert(src_fmtl->channels.r.start_bit == 0);
> > > + assert(dst_fmtl->channels.r.start_bit == 0);
> > > +
> > > + if (src_fmtl->bpb <= 32) {
> > > + const unsigned src_channels =
> > > + isl_format_get_num_channels(key->src_format);
> > > + const unsigned src_bits[4] = {
> > > + src_fmtl->channels.r.bits,
> > > + src_fmtl->channels.g.bits,
> > > + src_fmtl->channels.b.bits,
> > > + src_fmtl->channels.a.bits,
> > > + };
> > > + const unsigned dst_channels =
> > > + isl_format_get_num_channels(key->dst_format);
> > > + const unsigned dst_bits[4] = {
> > > + dst_fmtl->channels.r.bits,
> > > + dst_fmtl->channels.g.bits,
> > > + dst_fmtl->channels.b.bits,
> > > + dst_fmtl->channels.a.bits,
> > > + };
> > > + nir_ssa_def *packed =
> > > + nir_format_pack_uint_unmasked(b, color, src_bits,
> > src_channels);
> > > + color = nir_format_unpack_uint(b, packed, dst_bits, dst_channels);
> >
> > I tried to think why nir_format_bitcast_uint_vec_unmasked() can't handle
> > these cases (src_fmtl->bpb <= 32). At this point it still does, right?
> > Using
> > nir_format_pack_uint_unmasked()/nir_format_unpack_uint() already here is
> > preparing for cases where src or dst is ISL_FORMAT_R11G11B10_FLOAT and
> > which
> > nir_format_bitcast_uint_vec_unmasked() can't handle anymore?
> >
>
> Correct. nir_format_bitcast_uint_vec_unmasked can handle any cast from one
> format with a uniform channel size to another so long as the channel sizes
> are at least 8 bits. However, for 1010102 and similar, it falls over.
> Maybe I should say that in the commit message.
Thanks, with that:
Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
>
>
> > > + } else {
> > > + const unsigned src_bpc = src_fmtl->channels.r.bits;
> > > + const unsigned dst_bpc = dst_fmtl->channels.r.bits;
> > > +
> > > + assert(src_fmtl->channels.g.bits == 0 ||
> > > + src_fmtl->channels.g.bits == src_fmtl->channels.r.bits);
> > > + assert(src_fmtl->channels.b.bits == 0 ||
> > > + src_fmtl->channels.b.bits == src_fmtl->channels.r.bits);
> > > + assert(src_fmtl->channels.a.bits == 0 ||
> > > + src_fmtl->channels.a.bits == src_fmtl->channels.r.bits);
> > > + assert(dst_fmtl->channels.g.bits == 0 ||
> > > + dst_fmtl->channels.g.bits == dst_fmtl->channels.r.bits);
> > > + assert(dst_fmtl->channels.b.bits == 0 ||
> > > + dst_fmtl->channels.b.bits == dst_fmtl->channels.r.bits);
> > > + assert(dst_fmtl->channels.a.bits == 0 ||
> > > + dst_fmtl->channels.a.bits == dst_fmtl->channels.r.bits);
> > > +
> > > + /* Restrict to only the channels we actually have */
> > > + const unsigned src_channels =
> > > + isl_format_get_num_channels(key->src_format);
> > > + color = nir_channels(b, color, (1 << src_channels) - 1);
> > > +
> > > + color = nir_format_bitcast_uint_vec_unmasked(b, color, src_bpc,
> > dst_bpc);
> > > + }
> > >
> > > /* Blorp likes to assume that colors are vec4s */
> > > nir_ssa_def *u = nir_ssa_undef(b, 1, 32);
> > > @@ -1321,14 +1375,13 @@ brw_blorp_build_nir_shader(struct blorp_context
> > *blorp, void *mem_ctx,
> > > nir_type_int);
> > > }
> > >
> > > - if (key->dst_bpc != key->src_bpc) {
> > > + if (key->format_bit_cast) {
> > > assert(isl_swizzle_is_identity(key->src_swizzle));
> > > assert(isl_swizzle_is_identity(key->dst_swizzle));
> > > color = bit_cast_color(&b, color, key);
> > > - }
> > > -
> > > - if (key->dst_format)
> > > + } else if (key->dst_format) {
> > > color = convert_color(&b, color, key);
> > > + }
> > >
> > > if (key->dst_rgb) {
> > > /* The destination image is bound as a red texture three times as
> > wide
> > > @@ -2522,10 +2575,26 @@ blorp_copy(struct blorp_batch *batch,
> > > params.dst.view.format, packed);
> > > }
> > >
> > > - wm_prog_key.src_bpc =
> > > - isl_format_get_layout(params.src.view.format)->channels.r.bits;
> > > - wm_prog_key.dst_bpc =
> > > - isl_format_get_layout(params.dst.view.format)->channels.r.bits;
> > > + if (params.src.view.format != params.dst.view.format) {
> > > + enum isl_format src_cast_format = params.src.view.format;
> > > + enum isl_format dst_cast_format = params.dst.view.format;
> > > +
> > > + /* The BLORP bitcast code gets confused by RGB formats. Just
> > treat them
> > > + * as RGBA and then everything will be happy. This is perfectly
> > safe
> > > + * because BLORP likes to treat things as if they have vec4
> > colors all
> > > + * the time anyway.
> > > + */
> > > + if (isl_format_is_rgb(src_cast_format))
> > > + src_cast_format = isl_format_rgb_to_rgba(src_cast_format);
> > > + if (isl_format_is_rgb(dst_cast_format))
> > > + dst_cast_format = isl_format_rgb_to_rgba(dst_cast_format);
> > > +
> > > + if (src_cast_format != dst_cast_format) {
> > > + wm_prog_key.format_bit_cast = true;
> > > + wm_prog_key.src_format = src_cast_format;
> > > + wm_prog_key.dst_format = dst_cast_format;
> > > + }
> > > + }
> > >
> > > if (src_fmtl->bw > 1 || src_fmtl->bh > 1) {
> > > blorp_surf_convert_to_uncompressed(batch->blorp->isl_dev,
> > ¶ms.src,
> > > diff --git a/src/intel/blorp/blorp_priv.h b/src/intel/blorp/blorp_priv.h
> > > index 291c0a9..2013c53 100644
> > > --- a/src/intel/blorp/blorp_priv.h
> > > +++ b/src/intel/blorp/blorp_priv.h
> > > @@ -245,8 +245,11 @@ struct brw_blorp_blit_prog_key
> > > /* The swizzle to apply to the source in the shader */
> > > struct isl_swizzle src_swizzle;
> > >
> > > - /* Number of bits per channel in the source image. */
> > > - uint8_t src_bpc;
> > > + /* The format of the source if format-specific workarounds are needed
> > > + * and 0 (ISL_FORMAT_R32G32B32A32_FLOAT) if the destination is
> > natively
> > > + * renderable.
> > > + */
> > > + enum isl_format src_format;
> > >
> > > /* True if the source requires normalized coordinates */
> > > bool src_coords_normalized;
> > > @@ -268,15 +271,15 @@ struct brw_blorp_blit_prog_key
> > > /* The swizzle to apply to the destination in the shader */
> > > struct isl_swizzle dst_swizzle;
> > >
> > > - /* Number of bits per channel in the destination image. */
> > > - uint8_t dst_bpc;
> > > -
> > > /* The format of the destination if format-specific workarounds are
> > needed
> > > * and 0 (ISL_FORMAT_R32G32B32A32_FLOAT) if the destination is
> > natively
> > > * renderable.
> > > */
> > > enum isl_format dst_format;
> > >
> > > + /* Whether or not the format workarounds are a bitcast operation */
> > > + bool format_bit_cast;
> > > +
> > > /* Type of the data to be read from the texture (one of
> > > * nir_type_(int|uint|float)).
> > > */
> > > --
> > > 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
