Hi, Thanks for the patch. The main comment I have is about being precise that this is about DSPr2 and not r1. This has some consequences:
- I think a consistent string "dspr2" should be used to refer to this functionality. Ie., the files should be called pixman-mips-dspr2.[ch], the functions and variables should have "dspr2" in their names, and not "dsp" etc. The SSE2 implementation used to use "sse" and "sse2" interchangably and it resulted in total confusion. - The runtime CPU detection seems to check whether the "dsp" is found in /proc/cpuinfo. However, from reading the kernel source, it looks like "dsp" only indicates that rev. 1 of the DSP ASE is available. I also think the search function may be buggy. Consider what happens when given the string "ddsp" > + while (EOF != (k = fgetc(f))) { > + if (k == *what) { > + ++what; > + while ((*what != '\0') && (*what == fgetc(f))) { > + ++what; > + } > + if (*what == '\0') { > + fclose(f); > + return TRUE; > + } else { > + what = search_string; > + } > + } > + } Consider what it does when given the string "ddsp". The "k == *what will be true, and so the second fgetc(f) will eat the second 'd'. Since *what is not \0 at this point, it will be reset to "dsp", but the file is now at the "sp" point. Ie., it will miss the "dsp" string in "ddsp". Personally, I think I'd do it as just a loop using fgets() into some reasonable-sized buffer, and then checking each line with strstr(). But the bigger problem is that the kernel apparently doesn't offer any way to detect dspr2 that I can tell. Also, there are some coding style issues. Please see the file CODING_STYLE. In particular, - Use /* ... */ for comments, not // - Don't use braces with single-line statements - Put braces on their own line - Format functions with their return type on its own line Finally, feel free to send all the patches that you think are ready to be reviewed. There is no need to wait for each one to be reviewed. Note that git send-email <last commit-id before your patches> is convenient for this. Søren _______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman