Re: [PATCH 05/19] drm/i915: add initial Runtime PM functions

Rodrigo Vivi
2013-11-29T13:33:52Z

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

(resent with correct smtp)

On Fri, Nov 29, 2013 at 10:54 AM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>
> On Wed, Nov 27, 2013 at 06:10:30PM -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> 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@intel.com>
>> ---
>> 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));
>> +
>> + DRM_DEBUG_KMS("Suspending device\n");
>> +
>> + dev_priv->pm.suspended = true;
>> +
>> + pci_save_state(pdev);
>> + pci_set_power_state(pdev, PCI_D3cold);
>> +
>> + return 0;
>> +}
>> +
>> +static int i915_runtime_resume(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));
>> +
>> + DRM_DEBUG_KMS("Resuming device\n");
>> +
>> + pci_set_power_state(pdev, PCI_D0);
>> + pci_restore_state(pdev);
>> +
>> + dev_priv->pm.suspended = false;
>> +
>> + return 0;
>> +}
>> +
>> static const struct dev_pm_ops i915_pm_ops = {
>> .suspend = i915_pm_suspend,
>> .resume = i915_pm_resume,
>> @@ -909,6 +949,8 @@ static const struct dev_pm_ops i915_pm_ops = {
>> .thaw = i915_pm_thaw,
>> .poweroff = i915_pm_poweroff,
>> .restore = i915_pm_resume,
>> + .runtime_suspend = i915_runtime_suspend,
>> + .runtime_resume = i915_runtime_resume,
>> };
>>
>> static const struct vm_operations_struct i915_gem_vm_ops = {
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 00a0217..309ac40 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1291,6 +1291,10 @@ struct i915_package_c8 {
>> } regsave;
>> };
>>
>> +struct i915_runtime_pm {
>> + bool suspended;
>> +};
>> +
>> enum intel_pipe_crc_source {
>> INTEL_PIPE_CRC_SOURCE_NONE,
>> INTEL_PIPE_CRC_SOURCE_PLANE1,
>> @@ -1521,6 +1525,8 @@ typedef struct drm_i915_private {
>>
>> struct i915_package_c8 pc8;
>>
>> + struct i915_runtime_pm pm;
>> +
>> /* Old dri1 support infrastructure, beware the dragons ya fools entering
>> * here! */
>> struct i915_dri1_state dri1;
>> @@ -1845,6 +1851,7 @@ struct drm_i915_file_private {
>> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
>> #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev))
>> #define HAS_PC8(dev) (IS_HASWELL(dev)) /* XXX HSW:ULX */
>> +#define HAS_RUNTIME_PM(dev) false
>>
>> #define INTEL_PCH_DEVICE_ID_MASK 0xff00
>> #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 0231281..5596498 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -856,6 +856,10 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv);
>> void gen6_rps_boost(struct drm_i915_private *dev_priv);
>> void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
>> void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
>> +void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
>> +void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
>> +void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
>> +void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
>> void ilk_wm_get_hw_state(struct drm_device *dev);
>>
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 1cb02b1..6374884 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -31,6 +31,7 @@
>> #include "../../../platform/x86/intel_ips.h"
>> #include <linux/module.h>
>> #include <drm/i915_powerwell.h>
>> +#include <linux/pm_runtime.h>
>>
>> /**
>> * RC6 is a special power stage which allows the GPU to enter an very
>> @@ -5953,6 +5954,61 @@ void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv)
>> hsw_enable_package_c8(dev_priv);
>> }
>>
>> +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))
>> + return;
>> +
>> + pm_runtime_get_sync(device);
>> + WARN(dev_priv->pm.suspended, "Device still suspended.\n");
>> +}
>> +
>> +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))
>> + return;
>> +
>> + pm_runtime_mark_last_busy(device);
>> + pm_runtime_put_autosuspend(device);
>> +}
>> +
>> +void intel_init_runtime_pm(struct drm_i915_private *dev_priv)
>> +{
>> + struct drm_device *dev = dev_priv->dev;
>> + struct device *device = &dev->pdev->dev;
>> +
>> + dev_priv->pm.suspended = false;
>> +
>> + if (!HAS_RUNTIME_PM(dev))
>> + return;
>> +
>> + pm_runtime_set_active(device);
>> + pm_runtime_enable(device);
>> +
>> + pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
>> + pm_runtime_mark_last_busy(device);
>> + pm_runtime_use_autosuspend(device);
>> +}
>> +
>> +void intel_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))
>> + return;
>> +
>> + /* Make sure we're not suspended first. */
>> + pm_runtime_get_sync(device);
>> + pm_runtime_disable(device);
>> +}
>> +
>> /* Set up chip specific power management-related functions */
>> void intel_init_pm(struct drm_device *dev)
>> {
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index 7619d87..00e5ced 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -433,6 +433,13 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
>> }
>> }
>>
>> +static void
>> +assert_device_not_suspended(struct drm_i915_private *dev_priv)
>> +{
>> + WARN(HAS_RUNTIME_PM(dev_priv->dev) && dev_priv->pm.suspended,
>> + "Device suspended\n");
>> +}
>> +
>> #define REG_READ_HEADER(x) \
>> unsigned long irqflags; \
>> u##x val = 0; \
>> @@ -561,6 +568,7 @@ gen6_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace
>> if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
>> __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
>> } \
>> + assert_device_not_suspended(dev_priv); \
>> __raw_i915_write##x(dev_priv, reg, val); \
>> if (unlikely(__fifo_ret)) { \
>> gen6_gt_check_fifodbg(dev_priv); \
>> @@ -576,6 +584,7 @@ hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace)
>> if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
>> __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
>> } \
>> + assert_device_not_suspended(dev_priv); \
>> hsw_unclaimed_reg_clear(dev_priv, reg); \
>> __raw_i915_write##x(dev_priv, reg, val); \
>> if (unlikely(__fifo_ret)) { \
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br