2010/12/16 Lucas Stach <[email protected]>: > Hi Michel, > > had a short look on this, as far as i can tell at the time the patch > looks fairly good and we really need this checks. > > Some small notes: > > 1. In loops with fixed iterations the WAIT_RING should be called before > the loop, not inside. I think it is ok if we waste a few pushbuf entries > to avoid calling libdrm too often. > > See for example nvfx_push_vbo and nvfx_render_prim. >
Calling WAIT_RING is almost a NOOP since most of the function is inlined in the short path (more likely to happen, else the buffer is too really small). So wasting is few pushbuffer entries is ok for me, but since it is almost a NOOP, I see it as a bad practice. I mean, if you group the atomical RING command together, one can hardly make a misstake. On IRC I was advised to change most of the WAIT_RING to BEGIN_RING, that as quite some sence to me. Even if I guess the subchannel was binded explicitly for performance reasons or because it's the only subchannel to support the 3D objects (I didn't checked, but I guess it's a pure convention), lets be in the safe way, if one day we remove the explicit bind. While I agree it as some cost to call BEGIN_RING, it still sounds quite resonable, since it is inlined. > > 2. You decreased the size of the checks in several places. We should > review all of them before pushing this changes. I remember some of them > had some offset for some crude reason. I will review them by this > weekend if no one does it earlier. > Review them ;) I'm quite confident since I used one game (using quake engine) for around 45 minutes before quitting cleanly, while in the past it was crashing randomly ine less then 1 minutes when huge vertex loads happened ;) _______________________________________________ Nouveau mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/nouveau
