On Mon, 23 May 2016, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > On Mon, May 23, 2016 at 12:12:30PM +0300, Jani Nikula wrote: >> > #define ACPI_EV_DISPLAY_SWITCH (1<<0) >> > @@ -814,11 +807,11 @@ void intel_opregion_fini(struct drm_device *dev) >> > if (!opregion->header) >> > return; >> > >> > + tasklet_kill(&dev_priv->opregion.asle_task); >> > + >> >> So what if you got a new asle interrupt right here? > > Before we call fini, we should have de-installed the irq and done > synchronize_irq, so we only have to worry about the residual task. > (At least that is what I expect!) I'd expect that too, but looks like i915_driver_unload -> i915_driver_unregister -> intel_opregion_fini happens *before*, not after i915_driver_unload -> intel_modeset_cleanup -> intel_irq_uninstall J. > >> > if (opregion->asle) >> > opregion->asle->ardy = ASLE_ARDY_NOT_READY; >> >> This is supposed to signal we're not ready to handle said interrupts >> anymore. Not that we should rely on it either. >> >> It wasn't pretty before, but I think this patch widens the window for a >> race. If you kept the *other* code as it were, and just changed the work >> to tasklets, I'd be willing to look in the other direction... > > Considering the recent discussion about the negatives of > tasklets/ksoftirqd, I think I was being too cavalier in this conversion, > and we should only think about using tasklet where the post-interrupt > latency is critical. > -Chris -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
