Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit dab9a266 authored by Imre Deak's avatar Imre Deak Committed by Jani Nikula
Browse files

drm/i915: Fix system resume if PCI device remained enabled 



During system resume we depended on pci_enable_device() also putting the
device into PCI D0 state. This won't work if the PCI device was already
enabled but still in D3 state. This is because pci_enable_device() is
refcounted and will not change the HW state if called with a non-zero
refcount. Leaving the device in D3 will make all subsequent device
accesses fail.

This didn't cause a problem most of the time, since we resumed with an
enable refcount of 0. But it fails at least after module reload because
after that we also happen to leak a PCI device enable reference: During
probing we call drm_get_pci_dev() which will enable the PCI device, but
during device removal drm_put_dev() won't disable it. This is a bug of
its own in DRM core, but without much harm as it only leaves the PCI
device enabled. Fixing it is also a bit more involved, due to DRM
mid-layering and because it affects non-i915 drivers too. The fix in
this patch is valid regardless of the problem in DRM core.

v2:
- Add a code comment about the relation of this fix to the freeze/thaw
  vs. the suspend/resume phases. (Ville)
- Add a code comment about the inconsistent ordering of set power state
  and device enable calls. (Chris)

CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
CC: Chris Wilson <chris@chris-wilson.co.uk>
CC: stable@vger.kernel.org
Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1460979954-14503-1-git-send-email-imre.deak@intel.com


(cherry picked from commit 44410cd0)
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
parent 7ac7d19f

drivers/gpu/drm/i915/i915_drv.c

+31 −1
Original line number Original line Diff line number Diff line
@@ -792,7 +792,7 @@ static int i915_drm_resume(struct drm_device *dev) 
static int i915_drm_resume_early(struct drm_device *dev)

static int i915_drm_resume_early(struct drm_device *dev)

{

{

	struct drm_i915_private *dev_priv = dev->dev_private;

	struct drm_i915_private *dev_priv = dev->dev_private;

	int ret = 0;

	int ret;





	/*

	/*

	 * We have a resume ordering issue with the snd-hda driver also

	 * We have a resume ordering issue with the snd-hda driver also
@@ -803,6 +803,36 @@ static int i915_drm_resume_early(struct drm_device *dev) 
	 * FIXME: This should be solved with a special hdmi sink device or

	 * FIXME: This should be solved with a special hdmi sink device or

	 * similar so that power domains can be employed.

	 * similar so that power domains can be employed.

	 */

	 */



	/*

	 * Note that we need to set the power state explicitly, since we

	 * powered off the device during freeze and the PCI core won't power

	 * it back up for us during thaw. Powering off the device during

	 * freeze is not a hard requirement though, and during the

	 * suspend/resume phases the PCI core makes sure we get here with the

	 * device powered on. So in case we change our freeze logic and keep

	 * the device powered we can also remove the following set power state

	 * call.

	 */

	ret = pci_set_power_state(dev->pdev, PCI_D0);

	if (ret) {

		DRM_ERROR("failed to set PCI D0 power state (%d)\n", ret);

		goto out;

	}



	/*

	 * Note that pci_enable_device() first enables any parent bridge

	 * device and only then sets the power state for this device. The

	 * bridge enabling is a nop though, since bridge devices are resumed

	 * first. The order of enabling power and enabling the device is

	 * imposed by the PCI core as described above, so here we preserve the

	 * same order for the freeze/thaw phases.

	 *

	 * TODO: eventually we should remove pci_disable_device() /

	 * pci_enable_enable_device() from suspend/resume. Due to how they

	 * depend on the device enable refcount we can't anyway depend on them

	 * disabling/enabling the device.

	 */

	if (pci_enable_device(dev->pdev)) {

	if (pci_enable_device(dev->pdev)) {

		ret = -EIO;

		ret = -EIO;

		goto out;

		goto out;