From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH 05/19] drm/i915: add initial Runtime PM functions Date: Fri, 29 Nov 2013 15:05:54 +0100 Message-ID: References: <1385048853-1579-6-git-send-email-przanoni@gmail.com> <1385583030-2533-1-git-send-email-przanoni@gmail.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by gabe.freedesktop.org (Postfix) with ESMTP id 7075BFB006 for ; Fri, 29 Nov 2013 06:05:55 -0800 (PST) In-Reply-To: <1385583030-2533-1-git-send-email-przanoni@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Paulo Zanoni Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org At Wed, 27 Nov 2013 18:10:30 -0200, Paulo Zanoni wrote: > > From: Paulo Zanoni > > 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 > --- > 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 > #include > #include > +#include > +#include > > #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. > + 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