On ma, 2016-04-18 at 09:52 +0100, Chris Wilson wrote: > On Mon, Apr 18, 2016 at 11:37:05AM +0300, Imre Deak wrote: > > I don't know. I guess you mean that enabling the device first seems > > like the first logical step. To me enabling power is the first > > logical > > step and enabling the device itself is the second. > > It is not clear exactly, but what is clear is that > pci_enable_device() > sets the power state of this device only after doing so for the > bridge > hierachy. That bridge enabling in pci_enable_deivce() won't make a difference since the suspend/resume order is already defined so that bridge devices are suspended last and resumed first. Otoh, the current order is to call pci_set_power_state() first and then pci_enable_device() both for our device and all the rest of PCI drivers/devices that I checked so far. Note that we can't even change this order during the suspend/resume phase (as opposed to the freeze/thaw phase) since then it's the PCI core that imposes the order already. So if I would change this now as you suggest, we would have a different order during the suspend/resume and the freeze/thaw phases. > The order in this patch is inconsistent. I attribute this inconsistency to the sloppiness of the PCI API. I don't want to change the order in this fix, but I can follow up with a patch that removes the pci_disable_device()/pci_enable_device() from our suspend/resume hooks. We can't anyway depend on these doing anything, since they are dependent on the device enable refcount which is out of reach of the driver. I can also add now a code comment about this. --Imre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
