[Mesa-dev] [PATCH 2/3] i965: Make the param pointer arrays for the WM dynamically sized.

[Eric Anholt]

Kenneth Graunke <kenneth at whitecape.org> writes:

> On 08/27/2012 10:49 AM, Eric Anholt wrote:
>> Saves 26.5MB of wasted memory allocation in the l4d2 demo.
>> ---
>>  src/mesa/drivers/dri/i965/brw_context.h     |   15 ++++++++---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp        |    2 --
>>  src/mesa/drivers/dri/i965/brw_state_cache.c |    7 +++++
>>  src/mesa/drivers/dri/i965/brw_wm.c          |   38 +++++++++++++++++++++++++++
>>  src/mesa/drivers/dri/i965/brw_wm.h          |    1 +
>>  5 files changed, 58 insertions(+), 5 deletions(-)
>> 
>> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
>> index 15ef0a3..c1cd500 100644
>> --- a/src/mesa/drivers/dri/i965/brw_context.h
>> +++ b/src/mesa/drivers/dri/i965/brw_context.h

>> +typedef void (*cache_aux_free_func)(const void *aux);
>>  
>>  struct brw_cache {
>>     struct brw_context *brw;
>> @@ -629,6 +630,14 @@ struct brw_cache {
>>  
>>     uint32_t next_offset;
>>     bool bo_used_by_gpu;
>> +
>> +   /**
>> +    * Functions used in determining whether the prog_data for a new cache item
>> +    * matches an existing cache item.  This is sometimes done by memcmp, but
>> +    * for some cache ids that would mean statically sizing arrays instead of
>> +    * using nice pointers.
>> +    */
>
> This comment makes no sense to me.  Your newly introduced function is a
> helper to free arrays.  I don't see any changes to how key comparisons
> are done (memcmp or something else).  I'm kind of scared that this
> comment implies that memcmp doesn't work but I don't see a change to not
> use memcmp.

Sorry about that.  I should have reread before sending out.  This series
started out with adding prog_data compare funcs for the
upload_using_copy path, and having to change prog_datas to be ralloced
and passed in to be owned by the cache so that the allocs weren't
leaked.  That turned out to be a ton of churn, , and I went and replaced
my compare funcs with just free funcs because the code looked similar.

We still need the need comparison changes, and I'll add them back in.

>> +
>> +   /* Allocate the references to the uniforms that will end up in the
>> +    * prog_data associated with the compiled program, and which will be freed
>> +    * by the state cache.
>> +    */
>> +   if (fs) {
>> +      int param_count = fs->num_uniform_components;
>> +      /* The backend also sometimes adds params for texture size. */
>> +      param_count += 2 * BRW_MAX_TEX_UNIT;
>> +
>> +      c->prog_data.param = rzalloc_array(c, const float *, param_count);
>> +      c->prog_data.pull_param = rzalloc_array(c, const float *, param_count);
>> +   } else {
>> +      /* brw_wm_pass0.c will also add references to 0.0 and 1.0 which are
>> +       * uploaded as push parameters.
>> +       */
>> +      int param_count = (fp->program.Base.Parameters->NumParameters + 2) * 4;
>> +      /* The old backend also uploads constants using push parameters, so we
>> +       * can't just rely on the NumParameters set up by .
>> +       */
>> +      c->prog_data.param = rzalloc_array(c, const float *, param_count);
>
> Your sentence here is unfinished, but it sounds like you're saying we
> need to include constants, and I sure don't see that in the code (other
> than 0.0 and 1.0).  Don't you need to solve that?
>
> A totally lame solution would be to use MAX_UNIFORMS for the count when
> using the old backend.  That gives the benefit for anything not using
> ARB programs...

Constants was an early theory, and it turned out it was constants in a
way, except it was just the static 0.0 and 1.0.  I'll fix the comment.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120828/887ebb6a/attachment.pgp>