2013/11/29 Takashi Iwai <tiwai@xxxxxxx>:
> At Wed, 27 Nov 2013 18:10:30 -0200,
> Paulo Zanoni wrote:
>>
>> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>>
>> This patch adds the initial infrastructure to allow a Runtime PM
>> implementation that sets the device to its D3 state. The patch just
>> adds the necessary callbacks and the initial infrastructure.
>>
>> We still don't have any platform that actually uses this
>> infrastructure, we still don't call get/put in all the places we need
>> to, and we don't have any function to save/restore the state of the
>> registers. This is not a problem since no platform uses the code added
>> by this patch. We have a few people simultaneously working on runtime
>> PM, so this initial code could help everybody make their plans.
>>
>> V2: - Move some functions to intel_pm.c
>> - Remove useless pm_runtime_allow() call at init
>> - Remove useless pm_runtime_mark_last_busy() call at get
>> - Use pm_runtime_get_sync() instead of 2 calls
>> - Add a WARN to check if we're really awake
>>
>> V3: - Rebase.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/i915_dma.c | 6 ++++
>> drivers/gpu/drm/i915/i915_drv.c | 42 ++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/i915_drv.h | 7 +++++
>> drivers/gpu/drm/i915/intel_drv.h | 4 +++
>> drivers/gpu/drm/i915/intel_pm.c | 56 +++++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_uncore.c | 9 ++++++
>> 6 files changed, 124 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index 89e4cf1..4cdc1ee 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -42,6 +42,8 @@
>> #include <linux/vga_switcheroo.h>
>> #include <linux/slab.h>
>> #include <acpi/video.h>
>> +#include <linux/pm.h>
>> +#include <linux/pm_runtime.h>
>>
>> #define LP_RING(d) (&((struct drm_i915_private *)(d))->ring[RCS])
>>
>> @@ -1663,6 +1665,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>> if (IS_GEN5(dev))
>> intel_gpu_ips_init(dev_priv);
>>
>> + intel_init_runtime_pm(dev_priv);
>> +
>> return 0;
>>
>> out_power_well:
>> @@ -1702,6 +1706,8 @@ int i915_driver_unload(struct drm_device *dev)
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> int ret;
>>
>> + intel_fini_runtime_pm(dev_priv);
>> +
>> intel_gpu_ips_teardown();
>>
>> /* The i915.ko module is still not prepared to be loaded when
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 0ec0fb3..d5310a0 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -502,6 +502,8 @@ static int i915_drm_freeze(struct drm_device *dev)
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> struct drm_crtc *crtc;
>>
>> + intel_runtime_pm_get(dev_priv);
>> +
>> /* ignore lid events during suspend */
>> mutex_lock(&dev_priv->modeset_restore_lock);
>> dev_priv->modeset_restore = MODESET_SUSPENDED;
>> @@ -688,6 +690,8 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
>> mutex_lock(&dev_priv->modeset_restore_lock);
>> dev_priv->modeset_restore = MODESET_DONE;
>> mutex_unlock(&dev_priv->modeset_restore_lock);
>> +
>> + intel_runtime_pm_put(dev_priv);
>> return error;
>> }
>>
>> @@ -902,6 +906,42 @@ static int i915_pm_poweroff(struct device *dev)
>> return i915_drm_freeze(drm_dev);
>> }
>>
>> +static int i915_runtime_suspend(struct device *device)
>> +{
>> + struct pci_dev *pdev = to_pci_dev(device);
>> + struct drm_device *dev = pci_get_drvdata(pdev);
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>> + WARN_ON(!HAS_RUNTIME_PM(dev));
>
> It'd be better to add runtime_idle callback for this kind of checks.
> It's called always before actually doing runtime PM, and you can
> return -EBUSY if the runtime PM isn't available.
It's a WARN we never expect to hit, so I don't think it's worth
creating a new function just for a check like that.
>
>
>> + DRM_DEBUG_KMS("Suspending device\n");
>> +
>> + dev_priv->pm.suspended = true;
>> +
>> + pci_save_state(pdev);
>> + pci_set_power_state(pdev, PCI_D3cold);
>
> Why do you need these calls? PCI runtime PM core should already
> handle the power state change by itself.
>
>
> Takashi
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
