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

[Courtney Goeltzenleuchter]

Does anyone know of a test that measures frame 0 time? Or texture upload
speed?

For Smokin' Guns, I tried measuring the overall time, but an improved frame
0 time has difficulty standing out of a 2607 frame test.

I may have to create something. Suggestions for an appropriate framework?

Thanks,
 Courtney


On Mon, Oct 14, 2013 at 8:32 AM, Chad Versace
<chad.versace at linux.intel.com>wrote:

> 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.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<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.
>
>


-- 
Courtney Goeltzenleuchter
LunarG
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131014/5d7835ad/attachment.html>