On Saturday, June 14, 2014 05:41:37 PM Matt Turner wrote: > On Sat, Jun 14, 2014 at 12:54 PM, Kenneth Graunke <kenn...@whitecape.org> wrote: > >> - uint16_t compacted, uncompacted = 0; > >> - > >> - uncompacted |= (src->bits2.ud >> 13) & 0xfff; > >> + uint16_t compacted; > >> + uint16_t uncompacted = /* 12b */ > >> + (brw_inst_src0_vstride(brw, src) << 8) | /* 4b */ > >> + (brw_inst_src0_width(brw, src) << 5) | /* 3b */ > >> + (brw_inst_src0_hstride(brw, src) << 3) | /* 2b */ > > > > One thing that's a little funny here...we pull out hstride/width/vstride, > > which makes sense for align1 mode...but presumably this function is also used > > for align16 mode, where we instead have src0_da16_swiz_x etc. > > > > But, it's the same bits, so this ought to work. I'm not objecting, it's > > just...a little funny at first glance. > > > > I don't think it's worth adding conditionals, but would it be worth adding a > > comment saying basically /* this also works for align16 mode because they > > share the same bits */? > > > > Whatever you decide is fine. This patch is: > > Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> > > I'm glad you mentioned this. I think I want to modify these to use > brw_inst_bits/brw_inst_set_bits instead. I know they're different on > Broadwell, so we'll have slightly more C, but I think the benefits > outweight that.
Yeah, that seems like a good plan to me. --Ken
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
