Re: [Intel-gfx] [PATCH] Revert drm/i915: fix infinite loop at gen6_update_ring_freq
2014-04-23 4:05 GMT-03:00 Daniel Vetter dan...@ffwll.ch: On Tue, Apr 22, 2014 at 06:25:12PM -0300, Paulo Zanoni wrote: 2014-04-11 6:02 GMT-03:00 Daniel Vetter dan...@ffwll.ch: On Thu, Apr 10, 2014 at 10:52:26AM -0700, Ben Widawsky wrote: On Thu, Apr 10, 2014 at 10:50:43AM -0700, Ben Widawsky wrote: On Thu, Apr 10, 2014 at 09:04:47AM +0200, Daniel Vetter wrote: This reverts commit 4b28a1f3ef55a3b0b68dbab1fe6dbaf18e186710. This patch duct-tapes over some issue in the current bdw rps patches which must wait with enabling rc6/rps until the very first batch has been submitted by userspace. But those patches aren't merged yet, and for upstream we need to have an in-kernel emission of the very first batch. I shouldn't have merged this patch so let's revert it again. I said this on the mailing last before you merged the patch. 20140402050338.gb13...@bwidawsk.net 20140402145813.GV7225@phenom.ffwll.local will explain things. There's now a regression report pointing to the revert: https://bugs.freedesktop.org/show_bug.cgi?id=77565 . What is the proposed solution now? Just WARN and still avoid the infinite loop? Or keep the infinite loop and leave the bug open? Disable BDW runtime PM? I've thought that we can only hit this with the as-yet unmerged rc6 patches on bdw, so I'm really confused why this blows up now? In any case I've thought Imre has stumbled over a similar issue on byt and he has a fix to prevent runtime pm until the delayed rps init has run. I've assigned the bug to him. Still confused why this suddenly blew up ... Sorry for the delayed response. The bug is very simple: since we did not enable RC6, by the time we run gen6_update_ring_freq(), the RPS limits will all be zero. The loop decrements a variable until it reaches a point where it is smaller than the other. But since the other variable is zero, the loop won't end since we can't be smaller than zero on the unsigned world, no matter how much we decrement it. This can probably be reproduced on non-BDW machines too, with RC6 disabled. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert drm/i915: fix infinite loop at gen6_update_ring_freq
On Mon, Apr 28, 2014 at 8:14 PM, Paulo Zanoni przan...@gmail.com wrote: This can probably be reproduced on non-BDW machines too, with RC6 disabled. If I understand Imre's patch correctly the bug is that we didn't have rc6 on bdw, but the sanitize function didn't make this clear leading to bugs. If my understanding is wrong the I need to drop Imre's patch again. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert drm/i915: fix infinite loop at gen6_update_ring_freq
On Mon, 2014-04-28 at 21:23 +0200, Daniel Vetter wrote: On Mon, Apr 28, 2014 at 8:14 PM, Paulo Zanoni przan...@gmail.com wrote: This can probably be reproduced on non-BDW machines too, with RC6 disabled. If I understand Imre's patch correctly the bug is that we didn't have rc6 on bdw, but the sanitize function didn't make this clear leading to bugs. Yes, that's correct. For runtime PM we require RC6 to be enabled, and we use intel_enable_rc6() to check for this. Before patch [1] intel_enable_rc6() reported incorrectly on BDW that RC6 is enabled. --Imre [1] http://lists.freedesktop.org/archives/intel-gfx/2014-April/044354.html If my understanding is wrong the I need to drop Imre's patch again. -Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert drm/i915: fix infinite loop at gen6_update_ring_freq
On Tue, Apr 22, 2014 at 06:25:12PM -0300, Paulo Zanoni wrote: 2014-04-11 6:02 GMT-03:00 Daniel Vetter dan...@ffwll.ch: On Thu, Apr 10, 2014 at 10:52:26AM -0700, Ben Widawsky wrote: On Thu, Apr 10, 2014 at 10:50:43AM -0700, Ben Widawsky wrote: On Thu, Apr 10, 2014 at 09:04:47AM +0200, Daniel Vetter wrote: This reverts commit 4b28a1f3ef55a3b0b68dbab1fe6dbaf18e186710. This patch duct-tapes over some issue in the current bdw rps patches which must wait with enabling rc6/rps until the very first batch has been submitted by userspace. But those patches aren't merged yet, and for upstream we need to have an in-kernel emission of the very first batch. I shouldn't have merged this patch so let's revert it again. I said this on the mailing last before you merged the patch. 20140402050338.gb13...@bwidawsk.net 20140402145813.GV7225@phenom.ffwll.local will explain things. There's now a regression report pointing to the revert: https://bugs.freedesktop.org/show_bug.cgi?id=77565 . What is the proposed solution now? Just WARN and still avoid the infinite loop? Or keep the infinite loop and leave the bug open? Disable BDW runtime PM? I've thought that we can only hit this with the as-yet unmerged rc6 patches on bdw, so I'm really confused why this blows up now? In any case I've thought Imre has stumbled over a similar issue on byt and he has a fix to prevent runtime pm until the delayed rps init has run. I've assigned the bug to him. Still confused why this suddenly blew up ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert drm/i915: fix infinite loop at gen6_update_ring_freq
2014-04-11 6:02 GMT-03:00 Daniel Vetter dan...@ffwll.ch: On Thu, Apr 10, 2014 at 10:52:26AM -0700, Ben Widawsky wrote: On Thu, Apr 10, 2014 at 10:50:43AM -0700, Ben Widawsky wrote: On Thu, Apr 10, 2014 at 09:04:47AM +0200, Daniel Vetter wrote: This reverts commit 4b28a1f3ef55a3b0b68dbab1fe6dbaf18e186710. This patch duct-tapes over some issue in the current bdw rps patches which must wait with enabling rc6/rps until the very first batch has been submitted by userspace. But those patches aren't merged yet, and for upstream we need to have an in-kernel emission of the very first batch. I shouldn't have merged this patch so let's revert it again. I said this on the mailing last before you merged the patch. 20140402050338.gb13...@bwidawsk.net 20140402145813.GV7225@phenom.ffwll.local will explain things. There's now a regression report pointing to the revert: https://bugs.freedesktop.org/show_bug.cgi?id=77565 . What is the proposed solution now? Just WARN and still avoid the infinite loop? Or keep the infinite loop and leave the bug open? Disable BDW runtime PM? Thanks, Paulo -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] Revert drm/i915: fix infinite loop at gen6_update_ring_freq
This reverts commit 4b28a1f3ef55a3b0b68dbab1fe6dbaf18e186710.
This patch duct-tapes over some issue in the current bdw rps patches
which must wait with enabling rc6/rps until the very first batch has
been submitted by userspace.
But those patches aren't merged yet, and for upstream we need to have
an in-kernel emission of the very first batch. I shouldn't have
merged this patch so let's revert it again.
Also Imre noticed that even when rps is set up normally there's a
small window (due to the 1s delay of the async rps init work) where we
could runtime suspend already and blow up all over the place. Imre has
a proper fix to block runtime pm until the rps init work has
successfully completed.
Cc: Paulo Zanoni paulo.r.zan...@intel.com
Cc: Imre Deak imre.d...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_pm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8531cf6e2774..dc7adadbb945 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3522,8 +3522,7 @@ void gen6_update_ring_freq(struct drm_device *dev)
* to use for memory access. We do this by specifying the IA frequency
* the PCU should use as a reference to determine the ring frequency.
*/
- for (gpu_freq = dev_priv-rps.max_freq_softlimit;
-gpu_freq = dev_priv-rps.min_freq_softlimit gpu_freq != 0;
+ for (gpu_freq = dev_priv-rps.max_freq_softlimit; gpu_freq =
dev_priv-rps.min_freq_softlimit;
gpu_freq--) {
int diff = dev_priv-rps.max_freq_softlimit - gpu_freq;
unsigned int ia_freq = 0, ring_freq = 0;
--
1.8.5.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert drm/i915: fix infinite loop at gen6_update_ring_freq
On Thu, Apr 10, 2014 at 09:04:47AM +0200, Daniel Vetter wrote:
This reverts commit 4b28a1f3ef55a3b0b68dbab1fe6dbaf18e186710.
This patch duct-tapes over some issue in the current bdw rps patches
which must wait with enabling rc6/rps until the very first batch has
been submitted by userspace.
But those patches aren't merged yet, and for upstream we need to have
an in-kernel emission of the very first batch. I shouldn't have
merged this patch so let's revert it again.
I said this on the mailing last before you merged the patch.
Also Imre noticed that even when rps is set up normally there's a
small window (due to the 1s delay of the async rps init work) where we
could runtime suspend already and blow up all over the place. Imre has
a proper fix to block runtime pm until the rps init work has
successfully completed.
Cc: Paulo Zanoni paulo.r.zan...@intel.com
Cc: Imre Deak imre.d...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_pm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8531cf6e2774..dc7adadbb945 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3522,8 +3522,7 @@ void gen6_update_ring_freq(struct drm_device *dev)
* to use for memory access. We do this by specifying the IA frequency
* the PCU should use as a reference to determine the ring frequency.
*/
- for (gpu_freq = dev_priv-rps.max_freq_softlimit;
- gpu_freq = dev_priv-rps.min_freq_softlimit gpu_freq != 0;
+ for (gpu_freq = dev_priv-rps.max_freq_softlimit; gpu_freq =
dev_priv-rps.min_freq_softlimit;
gpu_freq--) {
int diff = dev_priv-rps.max_freq_softlimit - gpu_freq;
unsigned int ia_freq = 0, ring_freq = 0;
--
1.8.5.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert drm/i915: fix infinite loop at gen6_update_ring_freq
On Thu, Apr 10, 2014 at 10:50:43AM -0700, Ben Widawsky wrote:
On Thu, Apr 10, 2014 at 09:04:47AM +0200, Daniel Vetter wrote:
This reverts commit 4b28a1f3ef55a3b0b68dbab1fe6dbaf18e186710.
This patch duct-tapes over some issue in the current bdw rps patches
which must wait with enabling rc6/rps until the very first batch has
been submitted by userspace.
But those patches aren't merged yet, and for upstream we need to have
an in-kernel emission of the very first batch. I shouldn't have
merged this patch so let's revert it again.
I said this on the mailing last before you merged the patch.
20140402050338.gb13...@bwidawsk.net
Also Imre noticed that even when rps is set up normally there's a
small window (due to the 1s delay of the async rps init work) where we
could runtime suspend already and blow up all over the place. Imre has
a proper fix to block runtime pm until the rps init work has
successfully completed.
Cc: Paulo Zanoni paulo.r.zan...@intel.com
Cc: Imre Deak imre.d...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_pm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 8531cf6e2774..dc7adadbb945 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3522,8 +3522,7 @@ void gen6_update_ring_freq(struct drm_device *dev)
* to use for memory access. We do this by specifying the IA frequency
* the PCU should use as a reference to determine the ring frequency.
*/
- for (gpu_freq = dev_priv-rps.max_freq_softlimit;
-gpu_freq = dev_priv-rps.min_freq_softlimit gpu_freq != 0;
+ for (gpu_freq = dev_priv-rps.max_freq_softlimit; gpu_freq =
dev_priv-rps.min_freq_softlimit;
gpu_freq--) {
int diff = dev_priv-rps.max_freq_softlimit - gpu_freq;
unsigned int ia_freq = 0, ring_freq = 0;
--
1.8.5.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
