[Mesa-dev] [PATCH 11/19] i965: Convert brw_eu_compact.c to the new brw_inst API.

[Kenneth Graunke]

On Saturday, June 14, 2014 05:41:37 PM Matt Turner wrote:
> On Sat, Jun 14, 2014 at 12:54 PM, Kenneth Graunke <kenneth at 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 <kenneth at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140614/dc8a3a9d/attachment.sig>