Matt Turner <matts...@gmail.com> writes: > On Tue, Nov 12, 2013 at 12:22 PM, Chad Versace > <chad.vers...@linux.intel.com> wrote: >> +mesa-dev >> >> >> On 11/12/2013 09:16 AM, Courtney Goeltzenleuchter wrote: >>> >>> On Mon, Nov 11, 2013 at 2:19 PM, Chad Versace >>> <chad.vers...@linux.intel.com>wrote: >>> >>>> On 11/07/2013 01:59 PM, Courtney Goeltzenleuchter wrote: >>>> >>>>> MESA_FORMAT_XRGB8888 is equivalent to MESA_FORMAT_ARGB8888 in terms >>>>> of storage on the device, so okay to use this optimized copy routine. >>>>> >>>>> This series builds on work from Frank Henigman to optimize the >>>>> process of uploading a texture to the GPU. This series adds support for >>>>> MESA_XRGB_8888 and full miptrees where were found to be common >>>>> activities >>>>> in the Smokin' Guns game. The issue was found while profiling the app >>>>> but that part is not benchmarked. Smokin-Guns uses mipmap textures with >>>>> an internal format of GL_RGB (MESA_XRGB_8888 in the driver). >>>>> >>>>> These changes need a performance tool to run against to show how they >>>>> improve execution performance for specific texture formats. Using this >>>>> benchmark I've measured the following improvement on my Ivybridge >>>>> Intel(R) Xeon(R) CPU E3-1225 V2 @ 3.20GHz. >>>>> >>>>> Using 1024x1024, RGBA 8888 source, mipmap >>>>> <<THIS PATCH>> >>>>> >>>> >>>> I don't understand. What do you mean by ``<<THIS PATCH>>``? That all >>>> these >>>> numbers were obtained with this patch? But that doesn't make sense, >>>> because >>>> these are before-and-after numbers. And it can't be just this patch, >>>> because >>>> these numbers are identical to the numbers quoted in patch 2. >>>> >>>> >>>> internal-format Before (MB/sec) XRGB (MB/sec) mipmap (MB/sec) >>>>> >>>>> GL_RGBA 628.15 627.15 615.90 >>>>> GL_RGB 265.95 456.35 611.53 >>>>> 512x512 >>>>> GL_RGBA 600.23 597.00 619.95 >>>>> GL_RGB 255.50 440.62 611.28 >>>>> 256x256 >>>>> GL_RGBA 489.08 487.80 587.42 >>>>> GL_RGB 229.03 376.63 585.00 >>>>> >>>>> Test shows similar pattern for 512x512 and 256x256. >>>>> >>>> >>>> The above table confuses me. There is a column named "Before", but no >>>> column >>>> named "After". There 'internal-format' exists in the same location as >>>> '512x512' >>>> and '256x256', but 'internal-format' is not a size. >>> >>> >>> >>> The first two rows were measured using a 1024x1024 texture. Then comes the >>> 512x512 two rows and finally the 256x256 measurements. >>> >>> How's this? >>> >>> <<THIS PATCH>> >>> 1024x1024 texture size >>> >>> internal-format Before (MB/sec) XRGB (MB/sec) mipmap (MB/sec) >>> GL_RGBA 628.15 627.15 615.90 >>> GL_RGB 265.95 456.35 611.53 >>> >>> 512x512 texture size >>> >>> internal-format Before (MB/sec) XRGB (MB/sec) mipmap (MB/sec) >>> GL_RGBA 600.23 597.00 619.95 >>> GL_RGB 255.50 440.62 611.28 >>> >>> 256x256 texture size >>> >>> internal-format Before (MB/sec) XRGB (MB/sec) mipmap (MB/sec) >>> GL_RGBA 489.08 487.80 587.42 >>> GL_RGB 229.03 376.63 585.00 >> >> >> >> This formatting makes more sense. >> >> The <<THIS PATCH>> thing still doesn't make sense out-of-context, though. >> If someone uses git-show or git-log to look at this patch, the presence of >> three columns >> and <<THIS PATCH>> is not self-explanatory. >> >> In short, each commit message should either be self-explaining in isolation, >> or refer to another patch for additional context. Otherwise, the commit >> message >> doesn't help where it supposed to help: when someone looks at your patch >> after >> you're no longer around. >> >> How about something like this instead? I think this table would make sense >> in >> the commit message for both your patches. >> >> Numbers are MB/sec. >> >> texture internal >> size format baseline patch1 patch2 >> -------------------------------------------- >> 1024x1024 GL_RGBA 628.15 627.15 615.90 > > Let's drop all of this multi-patch table nonsense and just say what > each patch does in its commit message.
Yes, please!
pgpb8eGMEIMO5.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
