[Intel-gfx] [PATCH v2 02/38] drm/i915: Provide a hook for selftests

[Chris Wilson]

On Wed, Jan 25, 2017 at 01:50:01PM +0200, Joonas Lahtinen wrote:
> On to, 2017-01-19 at 11:41 +0000, Chris Wilson wrote:
> > @@ -499,7 +501,17 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  	if (vga_switcheroo_client_probe_defer(pdev))
> >  		return -EPROBE_DEFER;
> >  
> > -	return i915_driver_load(pdev, ent);
> > +	err = i915_driver_load(pdev, ent);
> > +	if (err)
> > +		return err;
> > +
> > +	err = i915_live_selftests(pdev);
> > +	if (err) {
> > +		i915_driver_unload(pci_get_drvdata(pdev));
> > +		return err > 0 ? -ENOTTY : err;
> 
> What's up with this?

What's up with what? We want to bail from the pci initialisation, so
need to return some error. ENOTTY is chosen as we don't (and I expect
should never) use it from the selftests and the internal routines used.

> >  static void i915_pci_remove(struct pci_dev *pdev)
> > @@ -521,6 +533,11 @@ static struct pci_driver i915_pci_driver = {
> >  static int __init i915_init(void)
> >  {
> >  	bool use_kms = true;
> > +	int err;
> > +
> > +	err = i915_mock_selftests();
> > +	if (err)
> > +		return err > 0 ? 0 : err;
> 
> Especially this, is this for skipping the device init completely?

Yes. 

> > +static int run_selftests(const char *name,
> > +			 struct selftest *st,
> > +			 unsigned int count,
> > +			 void *data)
> > +{
> > +	int err = 0;
> > +
> > +	while (!i915_selftest.random_seed)
> > +		i915_selftest.random_seed = get_random_int();
> 
> You know that in theory this might take an eternity. I'm not sure why
> zero is not OK after this point?

You wanted each run to be with a different seed!

The prng generator does produces 0 if state = { 0 }, but that is avoided
by prandom_seed_state().

> > +
> > +	i915_selftest.timeout_jiffies =
> > +		i915_selftest.timeout_ms ?
> > +		msecs_to_jiffies_timeout(i915_selftest.timeout_ms) :
> > +		MAX_SCHEDULE_TIMEOUT;
> 
> You had a default value for the variable too, I guess that's not needed
> now, and gets some bytes off .data.

I can move this into every user, if that's what you mean?
-chris

-- 
Chris Wilson, Intel Open Source Technology Centre