On 10/13/2013 08:33 PM, Ian Romanick wrote:
On 10/13/2013 01:50 PM, Frank Henigman wrote:
On Fri, Oct 11, 2013 at 10:00 PM, Chad Versace
<chad.vers...@linux.intel.com> wrote:
On 10/11/2013 10:17 AM, Courtney Goeltzenleuchter wrote:
Support all levels of a supported texture format.
---
src/mesa/drivers/dri/i965/intel_tex_subimage.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_tex_subimage.c
b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
index 4aec05d..5e46760 100644
--- a/src/mesa/drivers/dri/i965/intel_tex_subimage.c
+++ b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
@@ -541,14 +541,13 @@ intel_texsubimage_tiled_memcpy(struct gl_context *
ctx,
uint32_t cpp;
mem_copy_fn mem_copy = NULL;
- /* This fastpath is restricted to specific texture types: level 0 of
+ /* This fastpath is restricted to specific texture types:
* a 2D BGRA, RGBA, L8 or A8 texture. It could be generalized to
support
* more types.
*/
if (!brw->has_llc ||
type != GL_UNSIGNED_BYTE ||
texImage->TexObject->Target != GL_TEXTURE_2D ||
- texImage->Level != 0 ||
pixels == NULL ||
_mesa_is_bufferobj(packing->BufferObj) ||
packing->Alignment > 4 ||
@@ -616,6 +615,16 @@ intel_texsubimage_tiled_memcpy(struct gl_context *
ctx,
DBG("%s: level=%d offset=(%d,%d) (w,h)=(%d,%d)\n",
__FUNCTION__, texImage->Level, xoffset, yoffset, width, height);
+ /* Adjust x and y offset based on miplevel
+ */
+ if (texImage->Level) {
+ GLuint xlevel, ylevel;
+ intel_miptree_get_image_offset(image->mt, texImage->Level, 0,
+ &xlevel, &ylevel);
+ xoffset += xlevel;
+ yoffset += ylevel;
+ }
+
linear_to_tiled(
xoffset * cpp, (xoffset + width) * cpp,
yoffset, yoffset + height,
Usually when we commit performance patches like this, we state in the
commit message what the observed relative performance gain.
What gain did you see? Hardware? Benchmark? Kernel version? How many
runs?
We could quote from my patch, as this is just opening more paths into that code.
Or do you think this calls for different testing?
I think what Chad is asking is whether there's some information like
"Improves load time of application XYZ 12.3+4.5%" or similar.
In the past, we've had problems with patches that just make vague claims
of "improves performance" when we later find critical bugs in those
patches... can we just revert the code, or is it going to run the
performance of... something?
For reference, see commit 329cd6a9b and this thread from mesa-dev:
http://lists.freedesktop.org/archives/mesa-dev/2013-June/040811.html
Ian read my mind correctly. The commit message should say "Improves XYZ of
application ABC by 10.3+-1.2%", as well as state the hardware at a minimum,
and kernel version too if you're feeling gracious.
In the future, if someone discover that this patch introduces a bug, the commit
message's performance claim will prevent that someone from simply reverting the
code.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
