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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
