Comment 47 for bug 1174495

Revision history for this message
In , D. Hugh Redelmeier (hugh-mimosa) wrote :

Thanks, Neils, for the patch in comment #38.

Warning: the following comments are made with no understanding of X code.

"writting" should be "writing"

What is 0x000fffff? I would find the code clearer if this constant were given a name (RADEON_MAX_DMA?).

Why code
  nfill = (size / 0x000fffff) + !!(size % 0x000fffff);
instead of the generally more efficient
  nfill = (size+ 0x000fffff - 1) / 0x000fffff;
My guess: to avoid overflow. But if that is the case, then nfill should be uint64_t (like size), not just unsigned (which might be only 32 bits). Any case where the second statement might have an overflow problem would also be a problem with nfill being only unsigned 32.

If you know size is not 0, this is even better:
  nfill = ((size - 1) / 0x000fffff) + 1;
If size can be 0, it is probably worth an early-out test to avoid bothering the GPU anyway:
  if (size == 0) return 0;

I find code is easier to read if the scope of a variable is minimized. So I'd make the assignment to fsize also its declaration.

Have you created a patch to get these functions called in place of the currently broken code?