Re: [RFC 3/6] drm/i915: add enable_runtime_pm option

[Imre Deak <imre.deak@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4582 bytes --]

On Tue, 2013-10-22 at 17:30 -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> And leave it off by default. We have way too many driver entry points,
> we can't assume this will work without regressions without tons of
> testing first. This option allows people to test and fix the problems.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 4 ++--
>  drivers/gpu/drm/i915/i915_drv.c | 8 ++++++--
>  drivers/gpu/drm/i915/i915_drv.h | 1 +
>  drivers/gpu/drm/i915/intel_pm.c | 4 ++--
>  4 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 6aa044e..dd4f424 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1458,7 +1458,7 @@ static void i915_init_runtime_pm(struct drm_i915_private *dev_priv)
>  
>  	dev_priv->pm.suspended = false;
>  
> -	if (!HAS_RUNTIME_PM(dev))
> +	if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
>  		return;
>  
>  	pm_runtime_set_active(device);
> @@ -1475,7 +1475,7 @@ static void i915_fini_runtime_pm(struct drm_i915_private *dev_priv)
>  	struct drm_device *dev = dev_priv->dev;
>  	struct device *device = &dev->pdev->dev;
>  
> -	if (!HAS_RUNTIME_PM(dev))
> +	if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
>  		return;
>  
>  	/* Make sure we're not suspended first. */
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a999a3f..c75b78f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -154,6 +154,10 @@ module_param_named(prefault_disable, i915_prefault_disable, bool, 0600);
>  MODULE_PARM_DESC(prefault_disable,
>  		"Disable page prefaulting for pread/pwrite/reloc (default:false). For developers only.");
>  
> +int i915_enable_runtime_pm __read_mostly = 0;
> +module_param_named(enable_runtime_pm, i915_enable_runtime_pm, int, 0600);
> +MODULE_PARM_DESC(enable_runtime_pm, "Enable runtime PM on supported platforms (default: disabled)");
> +
>  static struct drm_driver driver;
>  extern int intel_agp_enabled;
>  
> @@ -890,7 +894,7 @@ static int i915_runtime_suspend(struct device *device)
>  	struct drm_device *dev = pci_get_drvdata(pdev);
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	WARN_ON(!HAS_RUNTIME_PM(dev));
> +	WARN_ON(!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm);
>  
>  	DRM_DEBUG_KMS("Suspending device\n");
>  
> @@ -909,7 +913,7 @@ static int i915_runtime_resume(struct device *device)
>  	struct drm_device *dev = pci_get_drvdata(pdev);
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	WARN_ON(!HAS_RUNTIME_PM(dev));
> +	WARN_ON(!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm);

This could a problem if someone disables runtime_pm through sysfs while
the device is suspended. One solution would be just to do a get/put in
the handler of i915_enable_runtime_pm and not check for it afterwards.

--Imre

>  
>  	DRM_DEBUG_KMS("Resuming device\n");
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 74f2b5d..73ebb9e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1831,6 +1831,7 @@ extern bool i915_fastboot __read_mostly;
>  extern int i915_enable_pc8 __read_mostly;
>  extern int i915_pc8_timeout __read_mostly;
>  extern bool i915_prefault_disable __read_mostly;
> +extern int i915_enable_runtime_pm __read_mostly;
>  
>  extern int i915_suspend(struct drm_device *dev, pm_message_t state);
>  extern int i915_resume(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a973f35..617e934 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5785,7 +5785,7 @@ void intel_runtime_pm_get(struct drm_i915_private *dev_priv)
>  	struct drm_device *dev = dev_priv->dev;
>  	struct device *device = &dev->pdev->dev;
>  
> -	if (!HAS_RUNTIME_PM(dev))
> +	if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
>  		return;
>  
>  	pm_runtime_mark_last_busy(device);
> @@ -5798,7 +5798,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv)
>  	struct drm_device *dev = dev_priv->dev;
>  	struct device *device = &dev->pdev->dev;
>  
> -	if (!HAS_RUNTIME_PM(dev))
> +	if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
>  		return;
>  
>  	pm_runtime_mark_last_busy(device);


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx