On Tue, Aug 06, 2013 at 04:19:00PM +0200, Daniel Vetter wrote: > Thinking about this some more we already cancel the rps work items > (with the exception of the vlv_work) in intel_disable_gt_powersave, > which is called already from i915_save_state. So I think we just need > to add the cancel_work for vlv_work at the right spot. > > For the hotplug work item we also have the > dev_priv->enable_hotplug_processing state changes (required to get the > ordering right on the resume side of things). I'd prefer if the > hotplug cancel_work_sync call is right next to this (maybe as part of > a irq disable function?). > > That leaves the catchall for unpin work and similar stuff. I think > that suits best as part of i915_gem_idle next to cancelling the > retire_work handler - with unpin am slightly afraid that if it runs > later on while gem is already quiescent it'll create havoc. > > So roughly three patches, with the hotplug one for -fixes/stable since > we have real reports about it. What do you think? The ordering did bother me. However, I like the paranoid safety that one big barrier before teardown offers. It does wonders stating that after this point there shall be no outstanding work. We can do the fine grained shutdown, and convert this barrier into a sanity check just by using WARN_ON(cancel_delayed_work_sync()). -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
