[Intel-gfx] [PATCH] drm/i915: Use hash tables for the command parser
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Thu May 8 15:24:01 CEST 2014
On 05/08/2014 02:02 PM, Damien Lespiau wrote:
> On Thu, May 08, 2014 at 01:25:33PM +0100, Tvrtko Ursulin wrote:
>>
>> On 05/08/2014 12:44 PM, Damien Lespiau wrote:
>>> On Thu, May 08, 2014 at 10:56:05AM +0100, Tvrtko Ursulin wrote:
>>>>
>>>> Hi Brad,
>>>>
>>>> On 04/28/2014 04:22 PM, bradley.d.volkin at intel.com wrote:
>>>> [snip]
>>>>> - BUG_ON(!validate_cmds_sorted(ring));
>>>>> + BUG_ON(!validate_cmds_sorted(ring, cmd_tables, cmd_table_count));
>>>>> BUG_ON(!validate_regs_sorted(ring));
>>>>> +
>>>>> + BUG_ON(init_hash_table(ring, cmd_tables, cmd_table_count));
>>>>
>>>> Is a BUG_ON a bit harsh since the above fails only on ENOMEM condition?
>>>>
>>>> If the concern is not allowing any command execution if parser setup
>>>> has failed, it would be nicer to the system as whole to just keep
>>>> rejecting everything, but let the rest of the kernel live to enable
>>>> debug or whatever?
>>>
>>> Those number_of_cmds allocations are a bit awkward though, couldn't we
>>> just embed the hlist_node into the desciptor struct?
>>
>> Until Brad comes online, I think that is because command descriptors
>> to hash table entries are not 1-to-1.
>
> Ah, I guess the common cmds are part of several hash tables. We could at
> least turn the multiple allocations into one big allocation though.
Probably a problem since commands can be added dynamically.
Also from the memory use point of view, single allocations are 12/24
bytes so fit into 16/32 byte slabs. That's some wastage on the face of
it, but bunching them up would make... say common plus render commands
are ~30 in total, so 360/720 bytes. Which would waste 152/304 bytes (512
and 1024 slabs) vs. 120/240 bytes for individual allocations. So for 30
command array current approach is even better. :)
Maybe make a dedicated cache for struct cmd_node?
Tvrtko
More information about the Intel-gfx
mailing list