[PATCH] drm/i915/selftests: Use preemption timeout on cleanup

Janusz Krzysztofik janusz.krzysztofik at linux.intel.com
Mon Dec 16 18:06:01 UTC 2024


Hi Andi,

Thanks for review.

On Monday, 16 December 2024 14:26:58 CET Andi Shyti wrote:
> Hi Janusz,
> 
> ...
> 
> >  	for_each_gt(gt, i915, i) {
> > +		struct intel_engine_cs *engine;
> > +		unsigned long timeout_ms = 0;
> > +		unsigned int id;
> > +
> >  		if (intel_gt_is_wedged(gt))
> >  			ret = -EIO;
> >  
> > +		for_each_engine(engine, gt, id) {
> > +			if (engine->props.preempt_timeout_ms > 
timeout_ms)
> > +				timeout_ms = engine-
>props.preempt_timeout_ms;
> > +		}
> 
> 
> the brackets are not really required here.

OK, I was not sure if for_each_if used inside for_each_engine is supposed to 
resolve potential issues with potentially confusing if nesting, but from your 
comment I conclude it does.  I'll fix it.

> 
> > +
> >  		cond_resched();
> >  
> > -		if (intel_gt_wait_for_idle(gt, HZ * 3) == -ETIME) {
> > +		if (intel_gt_wait_for_idle(gt, HZ * timeout_ms / 500) == -
ETIME) {
> 
> where is this 500 coming from?

/ 1000 would convert it to seconds as needed, and / 500 used instead was 
supposed to to mean that we are willing to wait for preempt_timeout_ms * 2.  
Sorry for that shortcut.  Would you like me to provide a clarifying comment, 
or maybe better use explicit 2 * preempt_timeout / 1000 ?

Thanks,
Janusz

> 
> Thanks,
> Andi
> 
> >  			pr_err("%pS timed out, cancelling all further 
testing.\n",
> >  			       __builtin_return_address(0));
> >  
> 






More information about the Intel-gfx mailing list