[Mesa-dev] [PATCH v2 5/5] i965/gen6: Initial implementation of MSAA.

[Ian Romanick]

On 05/14/2012 02:13 PM, Paul Berry wrote:
> Just to update the list: I had an in-person meeting with Eric, Ian, and
> Ken today to discuss the design of the patch series and decide what to
> do about it.  We agreed to go ahead and push the patches to master as
> is, and to make further improvements as time goes on.
>
> Specific improvements that were discussed:
>
> - I seem to be the only person who thinks it makes sense for the
> brw_blorp_params class to contain an exec() function.  In the interest
> of avoiding a rebasing nightmare, I will go ahead with the patches as
> they are, and submitt a follow up patch which replaces
> brw_blorp_params::exec() with a global function brw_blorp_exec(const
> brw_blorp_params *).

Part of the issue was that "exec" isn't a terrific name, and exec'ing a 
set of parameters doesn't have implicit meaning.  My preference was for 
a function, similar to what you mention, called brw_blorp_do_blit or 
similar.

If I see foo->exec() or exec(foo) in a pipe of code, I have to go look 
at the declaration of foo to know what's going on.  If I see 
do_some_action(foo) it's a bit more obvious.

> - Gen7 has some new instructions to facilitate bit twiddling (bfe, bfi1,
> and bfi2), and it seems like many of the messier blorp operations could
> be streamlined using them.  We agreed that this work shouldn't block the
> patch series.  I'll make a note to revisit this as a performance
> optimization after MSAA functionality is complete.
> - It would be nice to unit test some of this code.  I'm in support of
> this, and I will start working on unit tests as a future patch series.
>
> Also, I was admonished for not splitting up the patch "i965:
> Parameterize HiZ code to prepare for adding blitting." into two patches,
> one to do the refactoring and one to rename things, since that would
> have made it easier to review the code.  I apologize about this.  We
> also decided, however, that since the only real benefit of splitting up
> the patch would have been to make review easier, there's not much point
> in going to extra trouble to split this patch now that review has occurred.
>
> It's now about 2pm PDT.  Unless new issues are raised, I plan to push
> this patch series around 2pm PDT tomorrow.
>
> Paul