From mboxrd@z Thu Jan 1 00:00:00 1970 From: Naresh Kumar Kachhi Subject: Re: [RFC 6/6] FOR_UPSTREAM [VPG]: drm/i915: call init_runtime_pm before gem_init Date: Fri, 24 Jan 2014 20:41:21 +0530 Message-ID: <52E28299.4020002@intel.com> References: <1390392262-30937-1-git-send-email-naresh.kumar.kachhi@intel.com> <1390392262-30937-7-git-send-email-naresh.kumar.kachhi@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0421481105==" Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 8B532FAF75 for ; Fri, 24 Jan 2014 07:12:37 -0800 (PST) In-Reply-To: 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 Graphics Development , "Satyanantha, Rama Gopal M" List-Id: intel-gfx@lists.freedesktop.org This is a multi-part message in MIME format. --===============0421481105== Content-Type: multipart/alternative; boundary="------------020701040507000101040900" This is a multi-part message in MIME format. --------------020701040507000101040900 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit On 01/22/2014 07:14 PM, Paulo Zanoni wrote: > 2014/1/22: >> From: Naresh Kumar Kachhi >> >> with current code intel_runtime_pm_gpu_idle is getting called >> even before runtime_pm is initialized. Moving runtime_pm_init >> before i915_gem_init >> >> Following is the call stack, note: by this time >> runtime_pm was not initialized >> >> intel_runtime_pm_gpu_idle+0x37/0x90 >> i915_gem_retire_requests+0x8d/0xa0 >> i915_gem_init_seqno+0x48/0x90 >> i915_gem_set_seqno+0x2a/0x70 >> i915_gem_init_hw+0x19c/0x300 >> ? >> i915_gem_context_init+0x123/0x220 >> i915_gem_init+0x57/0x1a0 >> i915_driver_load+0xbf4/0xd50 > And why exactly is this a problem? Delaying intel_init_runtime_pm is a > nice thing because we don't want our driver trying to runtime suspend > while it's still being loaded. pm.gpu_idle will be false (dev_priv is zeroed) until the init_runtime_pm is called. In the call stack mentioned above we might see a call to intel_runtime_pm_gpu_idle. If runtime pm is not initialized by now, pm.gpu_idle will be false and we will do a runtime_put, without a matching runtime_get. However in you patch http://lists.freedesktop.org/archives/intel-gfx/2013-December/037729.html, i saw you have moved the initializing gpu_idle in intel_pm_setup, which will fix this problem. We will not need this patch with your new patches. only concern in this patch is that we are scattering the pm initialization across two function (runtime_pm_init and pm_setup). >> Signed-off-by: Naresh Kumar Kachhi >> --- >> drivers/gpu/drm/i915/i915_dma.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c >> index b5af745..85162da 100644 >> --- a/drivers/gpu/drm/i915/i915_dma.c >> +++ b/drivers/gpu/drm/i915/i915_dma.c >> @@ -1511,6 +1511,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) >> mutex_init(&dev_priv->dpio_lock); >> mutex_init(&dev_priv->modeset_restore_lock); >> >> + intel_init_runtime_pm(dev_priv); >> + >> intel_pm_setup(dev); >> >> intel_display_crc_init(dev); >> @@ -1674,8 +1676,6 @@ 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: >> -- >> 1.8.1.2 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > --------------020701040507000101040900 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit
On 01/22/2014 07:14 PM, Paulo Zanoni wrote:
2014/1/22  <naresh.kumar.kachhi@intel.com>:

From: Naresh Kumar Kachhi <naresh.kumar.kachhi@intel.com>

with current code intel_runtime_pm_gpu_idle is getting called
even before runtime_pm is initialized. Moving runtime_pm_init
before i915_gem_init

Following is the call stack, note: by this time
runtime_pm was not initialized

intel_runtime_pm_gpu_idle+0x37/0x90
i915_gem_retire_requests+0x8d/0xa0
i915_gem_init_seqno+0x48/0x90
i915_gem_set_seqno+0x2a/0x70
i915_gem_init_hw+0x19c/0x300
?
i915_gem_context_init+0x123/0x220
i915_gem_init+0x57/0x1a0
i915_driver_load+0xbf4/0xd50

And why exactly is this a problem? Delaying intel_init_runtime_pm is a
nice thing because we don't want our driver trying to runtime suspend
while it's still being loaded.

pm.gpu_idle will be false (dev_priv is zeroed) until the init_runtime_pm is called. 
In the call stack mentioned above we might see a call to intel_runtime_pm_gpu_idle. 
If runtime pm is not initialized by now, pm.gpu_idle will be false and we will do a
runtime_put, without a matching runtime_get.
However in you patch 
http://lists.freedesktop.org/archives/intel-gfx/2013-December/037729.html, i saw
you have moved the initializing gpu_idle in intel_pm_setup, which will fix this problem.
We will not need this patch with your new patches.
only concern in this patch is that we are scattering the pm initialization across two
function (runtime_pm_init and pm_setup).


Signed-off-by: Naresh Kumar Kachhi <naresh.kumar.kachhi@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b5af745..85162da 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1511,6 +1511,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
        mutex_init(&dev_priv->dpio_lock);
        mutex_init(&dev_priv->modeset_restore_lock);

+       intel_init_runtime_pm(dev_priv);
+
        intel_pm_setup(dev);

        intel_display_crc_init(dev);
@@ -1674,8 +1676,6 @@ 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:
--
1.8.1.2

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

--------------020701040507000101040900-- --===============0421481105== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============0421481105==--