[Mesa-dev] [PATCH 2/3] i965: Keep track of the per-thread scratch allocation in brw_stage_state.

[Francisco Jerez]

Kenneth Graunke <kenneth at whitecape.org> writes:

> On Saturday, June 11, 2016 2:50:27 PM PDT Francisco Jerez wrote:
>> diff --git a/src/mesa/drivers/dri/i965/brw_program.c b/src/mesa/drivers/dri/i965/brw_program.c
>> index 792f81b..9f822d2 100644
>> --- a/src/mesa/drivers/dri/i965/brw_program.c
>> +++ b/src/mesa/drivers/dri/i965/brw_program.c
>> @@ -345,6 +345,23 @@ brw_get_scratch_bo(struct brw_context *brw,
>>     }
>>  }
>>  
>> +/**
>> + * Reserve enough scratch space for the given stage to hold \p per_thread_size
>> + * bytes times the given \p thread_count.
>> + */
>> +void
>> +brw_alloc_stage_scratch(struct brw_context *brw,
>> +                        struct brw_stage_state *stage_state,
>> +                        unsigned per_thread_size,
>> +                        unsigned thread_count)
>> +{
>> +   if (stage_state->per_thread_scratch < per_thread_size) {
>> +      stage_state->per_thread_scratch = per_thread_size;
>> +      brw_get_scratch_bo(brw, &stage_state->scratch_bo,
>> +                         per_thread_size * thread_count);
>> +   }
>> +}
>> +
>>  void brwInitFragProgFuncs( struct dd_function_table *functions )
>>  {
>>     assert(functions->ProgramStringNotify == _tnl_program_string);
>
> There may still be an obscure bug here: brw_get_scratch_bo() skips
> allocating a new BO if old_bo->size >= size.  If old_bo->size were
> actually the size we requested, then that would be fine, as we only
> request a new BO when the size actually increases.
>
> I'm concerned that libdrm's buffer cache may round up bo->size to
> the next bucket size, or a page or something.  I don't remember the
> details...it might be fine.
>
> Still, I don't think the redundant check buys us anything.  How about
> deleting brw_get_scratch_bo() and calling unreference/bo_alloc directly
> from brw_alloc_stage_scratch()?  Seems simpler if nothing else.
>
Sure, I'll change it to call drm_intel_bo_alloc/unreference manually in
order to avoid the redundant check.  That still doesn't allow me to
remove brw_get_scratch_bo() though because there is one remaining use in
brw_wm_surface_state.c for the SNB multisample null render target
workaround (which has nothing to do with the shader scratch space so
brw_alloc_stage_scratch() wouldn't work).

> With that changed, the series is:
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>
Thanks!

> Thanks so much for tracking this down!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160612/834dfca8/attachment.sig>