[Mesa-dev] [PATCH] i965: Fast texture upload now supports all levels

[Ian Romanick]

On 10/13/2013 01:50 PM, Frank Henigman wrote:
> On Fri, Oct 11, 2013 at 10:00 PM, Chad Versace
> <chad.versace at 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

> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev