On Fri, Sep 25, 2015 at 03:29:40PM +0300, Ville Syrjälä wrote: > On Fri, Sep 25, 2015 at 08:09:57AM +0200, Egbert Eich wrote: > > +void intel_hpd_uninit(struct drm_i915_private *dev_priv) > > +{ > > + struct drm_device *dev = dev_priv->dev; > > + int i; > > + > > + cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work); > > What if another hpd happens here? > > Oh and we already have the intel_hpd_cancel_work() function. Sounds like > we need to rethink how that works rather than rolling another function > to do the same thing. > > There's also the matter of VLV/CHV where hpd init gets called from the > display power well enable hook. I was thinking that could race with this > stuff, but since this is only called from the suspend I suppose we > shouldn't be re-enabling the display power well at that time. At least on big core we have checks for intel_display_power_is_enabled in the irq postinstall hooks and corresponding post_enable code in the power well code to make sure you can enable interrupts/power wells in any order and it'll work. Not sure this is a reasonable design assumption any more, but otoh fixing that will likely mean we need to fix the power well handling in our driver load and suspend/resume code. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx