[Intel-gfx] [PATCH] drm/i915: Use hash tables for the command parser

[Volkin, Bradley D]

On Thu, May 08, 2014 at 02:56:05AM -0700, 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?
> 
> I know it won't happen almost ever so it's a minor point really. I just 
> dislike actively hosing the whole system if it is avoidable.

Hi Tvrtko,

I agree that a BUG_ON might be harsh here. I suppose we could log an
error and disable the command parser. Most command buffers would
still go through fine but HW parsing would reject some that the SW
parser might otherwise allow. That could be a bit odd if we ever did
get a failure - apps/functionality that worked the last time I booted
suddenly don't this time. The issue would be in the log though.

I don't have a strong preference on this. Whatever people prefer.

Thanks,
Brad

> 
> Regards,
> 
> Tvrtko