On Wed, May 10, 2017 at 05:49:26PM +0200, Michal Wajdeczko wrote: > On Wed, May 10, 2017 at 05:32:48PM +0200, Daniel Vetter wrote: > > On Wed, May 10, 2017 at 05:31:02PM +0200, Michal Wajdeczko wrote: > > > On Wed, May 10, 2017 at 05:07:01PM +0200, Daniel Vetter wrote: > > > > The unconditionally fallback to the blocking wait_for resulted in > > > > impressive fireworks at boot-up on my snb here. Make sure if we set > > > > the slow timeout to 0 that we never ever sleep. The tail of the > > > > callchain was > > > > > > > > intel_wait_for_register > > > > -> __intel_wait_for_register_fw > > > > -> usleep_range > > > > -> BOOM > > > > > > > > It blew up in intel_crt_detect load detection code on the > > > > ADPA_CRT_HOTPLUG_FORCE_TRIGGER in the ADPA register. > > > > > > > > > > Hmm, by reading the code, it looks that call stack should be like this: > > > > > > -> intel_wait_for_register(..., timeout_ms=1000) > > > -> __intel_wait_for_register_fw(..., fast_us=2, slow_ms=0, NULL); > > > -> wait_for(..., MS=1000) > > > -> _wait_for(..., US=1000*1000, W=1000) > > > -> usleep_range(W, 2*W) > > > > > > so the slow_timeout_ms will be 0 in __intel_wait_for_register_fw() > > > > > > Are you sure that fix below is in right place? > > > > The wait_for is _within the __intel_wait_for_register_fw. I've left out > > the macros because those don't show up in the bt. We do _not_ blow up on > > the wait_for after the __intel_wait_for_register_fw call in > > intel_wait_for_register. > > Ok, so the correct call stack is > > -> intel_wait_for_register(..., timeout_ms=1000) > -> __intel_wait_for_register_fw(..., fast_us=2, slow_ms=0, NULL); > -> wait_for(..., MS=0) > -> _wait_for(..., US=0, W=1000) > -> usleep_range(W, 2*W) > > so maybe we should just fix the wait_for/_wait_for macros and do not attempt > to sleep when timeout is zero ? It's rather unexpected that even with with > timeout MS=0 we will still call usleep_range(1000us, 2000us) In this case, it was clearly incorrect to do a wait_for_pass at all. In general, those wait_for macros are already complicated enough and the callers of wait_for() must reasonably expect it to sleep and so a seperate dance for timeout==0 seems unjustified. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
