[Nouveau] mesa: Patch that fix/add missing WAIT_RING calls

Michel Hermier michel.hermier at gmail.com
Fri Dec 17 01:23:12 PST 2010


2010/12/16 Lucas Stach <dev at lynxeye.de>:
> 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 ;)


More information about the Nouveau mailing list