Commit 2cd9a689 authored by Imre Deak's avatar Imre Deak
Browse files

drm/i915: Refactor intel_display_set_init_power() logic 



The device global init_power_on flag is somewhat arbitrary and makes
debugging power refcounting problems difficult. Instead arrange things
so that all display power domain get has a corresponding put call. After
this change we have the following sequences:

driver loading:
intel_power_domains_init_hw();
<other init steps>
intel_power_domains_enable();

driver unloading:
intel_power_domains_disable();
<other uninit steps>
intel_power_domains_fini_hw();

system suspend:
intel_power_domains_disable();
<other suspend steps>
intel_power_domains_suspend();

system resume:
intel_power_domains_resume();
<other resume steps>
intel_power_domains_enable();

at other times while the driver is loaded:
intel_display_power_get();
...
intel_display_power_put();

Suggested-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20180816123757.3286-2-imre.deak@intel.com
parent 07d80572

drivers/gpu/drm/i915/i915_drv.c

+30 −35
Original line number Diff line number Diff line
@@ -1282,6 +1282,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) 
	if (INTEL_INFO(dev_priv)->num_pipes)

		drm_kms_helper_poll_init(dev);



	intel_power_domains_enable(dev_priv);

	intel_runtime_pm_enable(dev_priv);

}
@@ -1292,6 +1293,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) 
static void i915_driver_unregister(struct drm_i915_private *dev_priv)

{

	intel_runtime_pm_disable(dev_priv);

	intel_power_domains_disable(dev_priv);



	intel_fbdev_unregister(dev_priv);

	intel_audio_deinit(dev_priv);
@@ -1374,7 +1376,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent) 
	if (ret < 0)

		goto out_pci_disable;



	intel_runtime_pm_get(dev_priv);

	disable_rpm_wakeref_asserts(dev_priv);



	ret = i915_driver_init_mmio(dev_priv);

	if (ret < 0)
@@ -1404,7 +1406,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent) 


	intel_init_ipc(dev_priv);



	intel_runtime_pm_put(dev_priv);

	enable_rpm_wakeref_asserts(dev_priv);



	i915_welcome_messages(dev_priv);
@@ -1415,7 +1417,7 @@ out_cleanup_hw: 
out_cleanup_mmio:

	i915_driver_cleanup_mmio(dev_priv);

out_runtime_pm_put:

	intel_runtime_pm_put(dev_priv);

	enable_rpm_wakeref_asserts(dev_priv);

	i915_driver_cleanup_early(dev_priv);

out_pci_disable:

	pci_disable_device(pdev);
@@ -1433,7 +1435,7 @@ void i915_driver_unload(struct drm_device *dev) 
	struct drm_i915_private *dev_priv = to_i915(dev);

	struct pci_dev *pdev = dev_priv->drm.pdev;



	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);

	disable_rpm_wakeref_asserts(dev_priv);



	i915_driver_unregister(dev_priv);
@@ -1465,7 +1467,8 @@ void i915_driver_unload(struct drm_device *dev) 
	i915_driver_cleanup_hw(dev_priv);

	i915_driver_cleanup_mmio(dev_priv);



	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);

	enable_rpm_wakeref_asserts(dev_priv);



	WARN_ON(atomic_read(&dev_priv->runtime_pm.wakeref_count));

}
@@ -1575,7 +1578,7 @@ static int i915_drm_suspend(struct drm_device *dev) 


	/* We do a lot of poking in a lot of registers, make sure they work

	 * properly. */

	intel_display_set_init_power(dev_priv, true);

	intel_power_domains_disable(dev_priv);



	drm_kms_helper_poll_disable(dev);
@@ -1612,6 +1615,18 @@ static int i915_drm_suspend(struct drm_device *dev) 
	return 0;

}



static enum i915_drm_suspend_mode

get_suspend_mode(struct drm_i915_private *dev_priv, bool hibernate)

{

	if (hibernate)

		return I915_DRM_SUSPEND_HIBERNATE;



	if (suspend_to_idle(dev_priv))

		return I915_DRM_SUSPEND_IDLE;



	return I915_DRM_SUSPEND_MEM;

}



static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)

{

	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -1622,21 +1637,10 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) 


	i915_gem_suspend_late(dev_priv);



	intel_display_set_init_power(dev_priv, false);

	intel_uncore_suspend(dev_priv);



	/*

	 * In case of firmware assisted context save/restore don't manually

	 * deinit the power domains. This also means the CSR/DMC firmware will

	 * stay active, it will power down any HW resources as required and

	 * also enable deeper system power states that would be blocked if the

	 * firmware was inactive.

	 */

	if (IS_GEN9_LP(dev_priv) || hibernation || !suspend_to_idle(dev_priv) ||

	    dev_priv->csr.dmc_payload == NULL) {

		intel_power_domains_suspend(dev_priv);

		dev_priv->power_domains_suspended = true;

	}

	intel_power_domains_suspend(dev_priv,

				    get_suspend_mode(dev_priv, hibernation));



	ret = 0;

	if (IS_GEN9_LP(dev_priv))
@@ -1648,10 +1652,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) 


	if (ret) {

		DRM_ERROR("Suspend complete failed: %d\n", ret);

		if (dev_priv->power_domains_suspended) {

			intel_power_domains_init_hw(dev_priv, true);

			dev_priv->power_domains_suspended = false;

		}

		intel_power_domains_resume(dev_priv);



		goto out;

	}
@@ -1768,6 +1769,8 @@ static int i915_drm_resume(struct drm_device *dev) 


	intel_opregion_notify_adapter(dev_priv, PCI_D0);



	intel_power_domains_enable(dev_priv);



	enable_rpm_wakeref_asserts(dev_priv);



	return 0;
@@ -1802,7 +1805,7 @@ static int i915_drm_resume_early(struct drm_device *dev) 
	ret = pci_set_power_state(pdev, PCI_D0);

	if (ret) {

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

		goto out;

		return ret;

	}



	/*
@@ -1818,10 +1821,8 @@ static int i915_drm_resume_early(struct drm_device *dev) 
	 * depend on the device enable refcount we can't anyway depend on them

	 * disabling/enabling the device.

	 */

	if (pci_enable_device(pdev)) {

		ret = -EIO;

		goto out;

	}

	if (pci_enable_device(pdev))

		return -EIO;



	pci_set_master(pdev);
@@ -1844,18 +1845,12 @@ static int i915_drm_resume_early(struct drm_device *dev) 


	intel_uncore_sanitize(dev_priv);



	if (dev_priv->power_domains_suspended)

		intel_power_domains_init_hw(dev_priv, true);

	else

		intel_display_set_init_power(dev_priv, true);

	intel_power_domains_resume(dev_priv);



	intel_engines_sanitize(dev_priv);



	enable_rpm_wakeref_asserts(dev_priv);



out:

	dev_priv->power_domains_suspended = false;



	return ret;

}

drivers/gpu/drm/i915/i915_drv.h

+1 −1
Original line number Diff line number Diff line
@@ -935,8 +935,8 @@ struct i915_power_domains { 
	 * Power wells needed for initialization at driver init and suspend

	 * time are on. They are kept on until after the first modeset.

	 */

	bool init_power_on;

	bool initializing;

	bool display_core_suspended;

	int power_well_count;



	struct mutex lock;

drivers/gpu/drm/i915/intel_display.c

+4 −1
Original line number Diff line number Diff line
@@ -15846,6 +15846,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev, 
	struct intel_encoder *encoder;

	int i;



	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);



	intel_early_display_was(dev_priv);

	intel_modeset_readout_hw_state(dev);
@@ -15900,7 +15902,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev, 
		if (WARN_ON(put_domains))

			modeset_put_power_domains(dev_priv, put_domains);

	}

	intel_display_set_init_power(dev_priv, false);



	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);



	intel_power_domains_verify_state(dev_priv);

drivers/gpu/drm/i915/intel_drv.h

+12 −3
Original line number Diff line number Diff line
@@ -1954,7 +1954,18 @@ int intel_power_domains_init(struct drm_i915_private *); 
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);

void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);

void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv);

void intel_power_domains_suspend(struct drm_i915_private *dev_priv);

void intel_power_domains_enable(struct drm_i915_private *dev_priv);

void intel_power_domains_disable(struct drm_i915_private *dev_priv);



enum i915_drm_suspend_mode {

	I915_DRM_SUSPEND_IDLE,

	I915_DRM_SUSPEND_MEM,

	I915_DRM_SUSPEND_HIBERNATE,

};



void intel_power_domains_suspend(struct drm_i915_private *dev_priv,

				 enum i915_drm_suspend_mode);

void intel_power_domains_resume(struct drm_i915_private *dev_priv);

void intel_power_domains_verify_state(struct drm_i915_private *dev_priv);

void bxt_display_core_init(struct drm_i915_private *dev_priv, bool resume);

void bxt_display_core_uninit(struct drm_i915_private *dev_priv);
@@ -2037,8 +2048,6 @@ bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv); 
void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv);

void intel_runtime_pm_put(struct drm_i915_private *dev_priv);



void intel_display_set_init_power(struct drm_i915_private *dev, bool enable);



void chv_phy_powergate_lanes(struct intel_encoder *encoder,

			     bool override, unsigned int mask);

bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy,

drivers/gpu/drm/i915/intel_runtime_pm.c

+95 −38
Original line number Diff line number Diff line
@@ -257,30 +257,6 @@ bool intel_display_power_is_enabled(struct drm_i915_private *dev_priv, 
	return ret;

}



/**

 * intel_display_set_init_power - set the initial power domain state

 * @dev_priv: i915 device instance

 * @enable: whether to enable or disable the initial power domain state

 *

 * For simplicity our driver load/unload and system suspend/resume code assumes

 * that all power domains are always enabled. This functions controls the state

 * of this little hack. While the initial power domain state is enabled runtime

 * pm is effectively disabled.

 */

void intel_display_set_init_power(struct drm_i915_private *dev_priv,

				  bool enable)

{

	if (dev_priv->power_domains.init_power_on == enable)

		return;



	if (enable)

		intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);

	else

		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);



	dev_priv->power_domains.init_power_on = enable;

}



/*

 * Starting with Haswell, we have a "Power Down Well" that can be turned off

 * when not needed anymore. We have 4 registers that can request the power well
@@ -3750,6 +3726,10 @@ static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv) 
 * domains (and not in the INIT domain) are referenced or disabled during the

 * modeset state HW readout. After that the reference count of each power well

 * must match its HW enabled state, see intel_power_domains_verify_state().

 *

 * It will return with power domains disabled (to be enabled later by

 * intel_power_domains_enable()) and must be paired with

 * intel_power_domains_fini_hw().

 */

void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)

{
@@ -3775,8 +3755,13 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume) 
		mutex_unlock(&power_domains->lock);

	}



	/* For now, we need the power well to be always enabled. */

	intel_display_set_init_power(dev_priv, true);

	/*

	 * Keep all power wells enabled for any dependent HW access during

	 * initialization and to make sure we keep BIOS enabled display HW

	 * resources powered until display HW readout is complete. We drop

	 * this reference in intel_power_domains_enable().

	 */

	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);

	/* Disable power support if the user asked so. */

	if (!i915_modparams.disable_power_well)

		intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
@@ -3790,16 +3775,13 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume) 
 *

 * De-initializes the display power domain HW state. It also ensures that the

 * device stays powered up so that the driver can be reloaded.

 *

 * It must be called with power domains already disabled (after a call to

 * intel_power_domains_disable()) and must be paired with

 * intel_power_domains_init_hw().

 */

void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv)

{

	/*

	 * The i915.ko module is still not prepared to be loaded when

	 * the power well is not enabled, so just enable it in case

	 * we're going to unload/reload.

	 */

	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);



	/* Keep the power well enabled, but cancel its rpm wakeref. */

	intel_runtime_pm_put(dev_priv);
@@ -3808,18 +3790,67 @@ void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv) 
		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);

}



/**

 * intel_power_domains_enable - enable toggling of display power wells

 * @dev_priv: i915 device instance

 *

 * Enable the ondemand enabling/disabling of the display power wells. Note that

 * power wells not belonging to POWER_DOMAIN_INIT are allowed to be toggled

 * only at specific points of the display modeset sequence, thus they are not

 * affected by the intel_power_domains_enable()/disable() calls. The purpose

 * of these function is to keep the rest of power wells enabled until the end

 * of display HW readout (which will acquire the power references reflecting

 * the current HW state).

 */

void intel_power_domains_enable(struct drm_i915_private *dev_priv)

{

	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);

}



/**

 * intel_power_domains_disable - disable toggling of display power wells

 * @dev_priv: i915 device instance

 *

 * Disable the ondemand enabling/disabling of the display power wells. See

 * intel_power_domains_enable() for which power wells this call controls.

 */

void intel_power_domains_disable(struct drm_i915_private *dev_priv)

{

	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);

}



/**

 * intel_power_domains_suspend - suspend power domain state

 * @dev_priv: i915 device instance

 * @suspend_mode: specifies the target suspend state (idle, mem, hibernation)

 *

 * This function prepares the hardware power domain state before entering

 * system suspend. It must be paired with intel_power_domains_init_hw().

 * system suspend.

 *

 * It must be called with power domains already disabled (after a call to

 * intel_power_domains_disable()) and paired with intel_power_domains_resume().

 */

void intel_power_domains_suspend(struct drm_i915_private *dev_priv)

void intel_power_domains_suspend(struct drm_i915_private *dev_priv,

				 enum i915_drm_suspend_mode suspend_mode)

{

	struct i915_power_domains *power_domains = &dev_priv->power_domains;



	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);



	/*

	 * In case of firmware assisted context save/restore don't manually

	 * deinit the power domains. This also means the CSR/DMC firmware will

	 * stay active, it will power down any HW resources as required and

	 * also enable deeper system power states that would be blocked if the

	 * firmware was inactive.

	 */

	if (!IS_GEN9_LP(dev_priv) && suspend_mode == I915_DRM_SUSPEND_IDLE &&

	    dev_priv->csr.dmc_payload != NULL)

		return;



	/*

	 * Even if power well support was disabled we still want to disable

	 * power wells while we are system suspended.

	 * power wells if power domains must be deinitialized for suspend.

	 */

	if (!i915_modparams.disable_power_well)

		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
@@ -3832,6 +3863,32 @@ void intel_power_domains_suspend(struct drm_i915_private *dev_priv) 
		skl_display_core_uninit(dev_priv);

	else if (IS_GEN9_LP(dev_priv))

		bxt_display_core_uninit(dev_priv);



	power_domains->display_core_suspended = true;

}



/**

 * intel_power_domains_resume - resume power domain state

 * @dev_priv: i915 device instance

 *

 * This function resume the hardware power domain state during system resume.

 *

 * It will return with power domain support disabled (to be enabled later by

 * intel_power_domains_enable()) and must be paired with

 * intel_power_domains_suspend().

 */

void intel_power_domains_resume(struct drm_i915_private *dev_priv)

{

	struct i915_power_domains *power_domains = &dev_priv->power_domains;



	if (power_domains->display_core_suspended) {

		intel_power_domains_init_hw(dev_priv, true);

		power_domains->display_core_suspended = false;



		return;

	}



	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);

}



static void intel_power_domains_dump_info(struct drm_i915_private *dev_priv)
@@ -4030,8 +4087,8 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv) 
 * This function enables runtime pm at the end of the driver load sequence.

 *

 * Note that this function does currently not enable runtime pm for the

 * subordinate display power domains. That is only done on the first modeset

 * using intel_display_set_init_power().

 * subordinate display power domains. That is done by

 * intel_power_domains_enable().

 */

void intel_runtime_pm_enable(struct drm_i915_private *dev_priv)

{

CRA Git | Maintained and supported by SUSTech CRA and CCSE