From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [RFC 3/6] drm/i915: add enable_runtime_pm option Date: Mon, 4 Nov 2013 23:36:06 +0200 Message-ID: <20131104213606.GA13047@intel.com> References: <1382470214-1597-1-git-send-email-przanoni@gmail.com> <1382470214-1597-4-git-send-email-przanoni@gmail.com> <1382966859.5775.76.camel@intelbox> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id A28A4FB1CE for ; Mon, 4 Nov 2013 13:36:10 -0800 (PST) Content-Disposition: inline In-Reply-To: <1382966859.5775.76.camel@intelbox> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Imre Deak Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Mon, Oct 28, 2013 at 03:27:39PM +0200, Imre Deak wrote: > On Tue, 2013-10-22 at 17:30 -0200, Paulo Zanoni wrote: > > From: Paulo Zanoni > > = > > 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 > > --- > > 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/i91= 5_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 =3D 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 =3D dev_priv->dev; > > struct device *device =3D &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/i91= 5_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). Fo= r developers only."); > > = > > +int i915_enable_runtime_pm __read_mostly =3D 0; > > +module_param_named(enable_runtime_pm, i915_enable_runtime_pm, int, 060= 0); > > +MODULE_PARM_DESC(enable_runtime_pm, "Enable runtime PM on supported pl= atforms (default: disabled)"); > > + > > static struct drm_driver driver; > > extern int intel_agp_enabled; > > = > > @@ -890,7 +894,7 @@ static int i915_runtime_suspend(struct device *devi= ce) > > struct drm_device *dev =3D pci_get_drvdata(pdev); > > struct drm_i915_private *dev_priv =3D 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 *devic= e) > > struct drm_device *dev =3D pci_get_drvdata(pdev); > > struct drm_i915_private *dev_priv =3D 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. There's already a standard interface for enabling/disabling runtime PM under sysfs. Why do we want this custom one? -- = Ville Syrj=E4l=E4 Intel OTC