Re: [Intel-gfx] [PATCH v2] drm/i915: Avoid GPU hang when coming out of S3 or S4
I agree Daniel. We need two patches here, 1) moving the enabling of the interrupts early on. 2) split the WA initialization into GT Display and move GT workarounds before ring init. Thanks Deepak -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Monday, May 4, 2015 8:25 PM To: Chris Wilson; Antoine, Peter; Deepak S; S, Deepak; intel-gfx@lists.freedesktop.org; Tian, YeX Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Avoid GPU hang when coming out of S3 or S4 On Wed, Apr 29, 2015 at 12:39:17PM +0100, Chris Wilson wrote: On Wed, Apr 29, 2015 at 02:07:19PM +0300, David Weinehall wrote: On Tue, Apr 28, 2015 at 03:46:46PM +0100, Chris Wilson wrote: On Tue, Apr 28, 2015 at 02:38:25PM +, Antoine, Peter wrote: So is the plan to push these patches and have follow-on work to cover the other paths? As this fixes the Bugzilla issue that has been raised. You've identified an issue, but I think your patch is incomplete. I've tried my best to go through the remaining similar-looking code, but the rest seems fine (I might've missed something though). The only thing I reacted on was that in intel_runtime_resume() the call to intel_init_pch_refclk() is conditional on IS_GEN6(), but none of the other invocations of intel_init_pch_refclk() are. The commit message doesn't seem to provide a sufficient explanation for why this is so. The explanation for moving init_hw() was that it setups clock_gating. If any in that path are required for enabling the rings, those should be move to init_render_ring() or the init_context. Yeah we've had this fun before. init_clock_gating is _not_ for GT workarounds, only for modeset workarounds. We might need to rename that hook to avoid getting bitten for eternity, but moving init_hw because of clock_gating to get the rings up and running is bogus. As Chris said we need to identify which bits need to get moved to the ring init or w/a bb code and do that (and in a separate patch from enabling the interrupts early enough like this one here does). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 v2] drm/i915: Avoid GPU hang when coming out of S3 or S4
Thanks Chirs for review, We moved Init_hw to initialize WA's before any BB submission. Init_hw calls init_clock_gating Thanks Deepak -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Tuesday, April 28, 2015 12:53 PM To: Antoine, Peter Cc: intel-gfx@lists.freedesktop.org; S, Deepak; Tian, YeX; Weinehall, David Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Avoid GPU hang when coming out of S3 or S4 On Tue, Apr 28, 2015 at 08:18:14AM +0100, Peter Antoine wrote: This patch fixed a timing issue that causes a GPU hang when a the system comes out of power saving. During pm_resume, We are submitting batchbuffers before enabling Interrupts this is causing us to miss the context switch interrupt, and in consequence intel_execlists_handle_ctx_events is not triggered. Ah. Thanks. How about a code comment? :) But that doesn't explain moving init_hw... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915: Read the CCK fuse register from CCK
On 11/11/2014 8:58 PM, Daniel Vetter wrote:
On Wed, Nov 12, 2014 at 08:40:47AM +0530, Deepak S wrote:
On Saturday 08 November 2014 01:03 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
When reading a CCK register we should obviously read it from CCK not
Punit. This problem has been present ever since this of code was
introduced in
commit 67c3bf6f55a97a0915a0f9ea07278a3073cc9601
Author: Deepak S deepa...@linux.intel.com
Date: Thu Jul 10 13:16:24 2014 +0530
drm/i915: populate mem_freq/cz_clock for chv
The problem was raised during review by Mika [1] but somehow slipped
through the cracks, and the patch got applied with the problem unfixed.
[1] http://lists.freedesktop.org/archives/intel-gfx/2014-July/048937.html
Cc: Deepak S deepa...@linux.intel.com
Cc: Mika Kuoppala mika.kuopp...@intel.com
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9285dee..befad36 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5253,7 +5253,10 @@ static void cherryview_init_gt_powersave(struct
drm_device *dev)
mutex_lock(dev_priv-rps.hw_lock);
- val = vlv_punit_read(dev_priv, CCK_FUSE_REG);
+ mutex_lock(dev_priv-dpio_lock);
+ val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
+ mutex_unlock(dev_priv-dpio_lock);
+
switch ((val 2) 0x7) {
case 0:
case 1:
Oops i missed the comment.
Reviewed-by: Deepak S deepa...@linux.intel.com
Queued for -next, thanks for the patch.
Deepak, can you please review the other patches in Ville's series here,
too?
Sure Daniel. Since CCK was a bug fix reviewed that first.
Other patches Just Started to review. I will send the comments. if any.
Thanks, Daniel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Wait old forcewake ack to clear on vlv
On 11/5/2014 2:23 PM, Ville Syrjälä wrote:
On Wed, Nov 05, 2014 at 10:18:46AM +0200, Mika Kuoppala wrote:
Don't rush into getting new fw until the clearing
of old one has been acked.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85684
Tested-by: lu hua huax...@intel.com
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
This is just a revert of
commit 5cb13c07dae73380d8b3ddc792740487b8742938
Author: Deepak S deepa...@linux.intel.com
Date: Thu Sep 18 18:51:50 2014 +0530
drm/i915/vlv: Remove check for Old Ack during forcewake
except you left the comment behind.
Would be nice to have some kind of real explanation what is going on
here. Deepak, any ideas?
Hi Ville,
This is strange :(. Expectation from HW team is not to wait for old
ack. Let me go thought my mail and I will send you the explanation i got
from HW team.
Sorry, I was working some other task and i will check this issue on my
system update the thread.
Thanks
Deepak
---
drivers/gpu/drm/i915/intel_uncore.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 9427641..5259b38 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -204,6 +204,10 @@ static void __vlv_force_wake_get(struct drm_i915_private
*dev_priv,
/* Check for Render Engine */
if (FORCEWAKE_RENDER fw_engine) {
+ if (wait_for_atomic((__raw_i915_read32(dev_priv, FORCEWAKE_ACK_VLV)
+FORCEWAKE_KERNEL) == 0,
FORCEWAKE_ACK_TIMEOUT_MS))
+ DRM_ERROR(Timed out: waiting for old Render ack to
clear.\n);
+
__raw_i915_write32(dev_priv, FORCEWAKE_VLV,
_MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
@@ -217,6 +221,11 @@ static void __vlv_force_wake_get(struct drm_i915_private *dev_priv,
/* Check for Media Engine */
if (FORCEWAKE_MEDIA fw_engine) {
+ if (wait_for_atomic((__raw_i915_read32(dev_priv, FORCEWAKE_ACK_MEDIA_VLV)
+FORCEWAKE_KERNEL) == 0,
FORCEWAKE_ACK_TIMEOUT_MS))
+ DRM_ERROR(Timed out: waiting for old media ack to
clear.\n);
+
+
__raw_i915_write32(dev_priv, FORCEWAKE_MEDIA_VLV,
_MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
--
1.9.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
Re: [Intel-gfx] [PATCH] drm/i915: Wait old forcewake ack to clear on vlv
On 11/5/2014 3:43 PM, Mika Kuoppala wrote:
Ville Syrjälä ville.syrj...@linux.intel.com writes:
On Wed, Nov 05, 2014 at 10:18:46AM +0200, Mika Kuoppala wrote:
Don't rush into getting new fw until the clearing
of old one has been acked.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85684
Tested-by: lu hua huax...@intel.com
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
This is just a revert of
commit 5cb13c07dae73380d8b3ddc792740487b8742938
Author: Deepak S deepa...@linux.intel.com
Date: Thu Sep 18 18:51:50 2014 +0530
drm/i915/vlv: Remove check for Old Ack during forcewake
except you left the comment behind.
I failed at archeology. There is even nice comment from Deepak on
top I managed to miss.
But after reading this comment and the workaround description, I
think that this WaRsDontPollForAckOnClearingFWBits is not valid for
our use case.
We use only one bit per engine, so our use case is not multithreaded
in that sense.
-Mika
I agree with you, in kernel we use only one bit for FW, but based on my
understanding of forcewake implementation back to back forcewake should
not cause any issue.
Don't you think adding old ack might be hiding some other issue? I think
we need to root cause further.
As of now we can revert the patch I will start root cause on my
machine on why the issue is happening.
Would be nice to have some kind of real explanation what is going on
here. Deepak, any ideas?
---
drivers/gpu/drm/i915/intel_uncore.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 9427641..5259b38 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -204,6 +204,10 @@ static void __vlv_force_wake_get(struct drm_i915_private
*dev_priv,
/* Check for Render Engine */
if (FORCEWAKE_RENDER fw_engine) {
+ if (wait_for_atomic((__raw_i915_read32(dev_priv, FORCEWAKE_ACK_VLV)
+FORCEWAKE_KERNEL) == 0,
FORCEWAKE_ACK_TIMEOUT_MS))
+ DRM_ERROR(Timed out: waiting for old Render ack to
clear.\n);
+
__raw_i915_write32(dev_priv, FORCEWAKE_VLV,
_MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
@@ -217,6 +221,11 @@ static void __vlv_force_wake_get(struct drm_i915_private *dev_priv,
/* Check for Media Engine */
if (FORCEWAKE_MEDIA fw_engine) {
+ if (wait_for_atomic((__raw_i915_read32(dev_priv, FORCEWAKE_ACK_MEDIA_VLV)
+FORCEWAKE_KERNEL) == 0,
FORCEWAKE_ACK_TIMEOUT_MS))
+ DRM_ERROR(Timed out: waiting for old media ack to
clear.\n);
+
+
__raw_i915_write32(dev_priv, FORCEWAKE_MEDIA_VLV,
_MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add cherryview specfic forcewake in execlists_elsp_write
Hmm Ok Daniel. Let me try and clean up the forcewake. :) Chris, most of the debugfs is already covered with runtime_get/put. If anything is missed we can add that Thanks Deepak -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Wednesday, September 10, 2014 9:21 PM To: Daniel Vetter Cc: Jesse Barnes; S, Deepak; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: add cherryview specfic forcewake in execlists_elsp_write On Wed, Sep 10, 2014 at 05:47:39PM +0200, Daniel Vetter wrote: Aside if someone gets bored and wants to apply some polish: And __ variant of force_wake_get/put which only asserts that the device isn't runtime suspended instead of doing the runtime_pm_get/put would be cute - we have one other user in the hsw/bdw rpm code already. The current force_wake_get/put functions would then just wrap those raw versions. I fail to see the reason why they take it at all. Conceptually, gen6_gt_force_wake_get() is just as lowlevel as the register access they wrap. The only one that breaks the rule is i915_debugfs.c and that seems easy enough to wrap with its own intel_runtime_pm_get/_put. And once you've finish with that you can then sort out the mess that is the vlv variants. And now skl continues in the cut'n'paste'n'paste vein. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle
I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to make sure both render and media wells and the gfx clock remain off, and I was also monitoring vnn via svid. While that was going on I just rewrote PUNIT_REG_GPU_FREQ_REQ to make Punit change the frequency, and sure enough it did, and svid showed me that vnn had also changed. So it appears there's no need to have the gfx clock on to change its frequency on this BYT. I wonder if this part of the workaround was only needed on older parts. Deepak, any ideas? Yes ville, this was added initial for older parts and force gfx clock was part of the workaround. We have not verified this on newer parts. Let me check with hw guys to see if workaround still exits and when this was fixed. Hi Ville, Got the confirmation from HW team, this WA as been fixed in latest stepping, we can remove the force gfx clock, mask and request only the min freq when we are idle. You will submit patch will fix or you want me to do it? Thanks Deepak ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle
On 6/13/2014 12:10 AM, Ville Syrjälä wrote: On Thu, Jun 12, 2014 at 11:45:03PM +0530, S, Deepak wrote: I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to make sure both render and media wells and the gfx clock remain off, and I was also monitoring vnn via svid. While that was going on I just rewrote PUNIT_REG_GPU_FREQ_REQ to make Punit change the frequency, and sure enough it did, and svid showed me that vnn had also changed. So it appears there's no need to have the gfx clock on to change its frequency on this BYT. I wonder if this part of the workaround was only needed on older parts. Deepak, any ideas? Yes ville, this was added initial for older parts and force gfx clock was part of the workaround. We have not verified this on newer parts. Let me check with hw guys to see if workaround still exits and when this was fixed. Hi Ville, Got the confirmation from HW team, this WA as been fixed in latest stepping, What's latest here? Did we ever ship any of the steppings that still need the gfx clock force? we can remove the force gfx clock, mask and request only the min freq when we are idle. You will submit patch will fix or you want me to do it? Go ahead if you have time. I'm already juggling too many things :) Ok. I will send the patch for review :) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle
On 6/11/2014 10:26 PM, Ville Syrjälä wrote: On Fri, Jun 06, 2014 at 08:03:20AM -0700, Jesse Barnes wrote: On Fri, 6 Jun 2014 11:29:24 +0300 Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Thu, Jun 05, 2014 at 01:49:34PM -0700, Jesse Barnes wrote: This may take awhile (~10ms), and we don't need to make noise about it. References: https://bugs.freedesktop.org/show_bug.cgi?id=75244 Tested-by: huax...@intel.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_pm.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ee27d74..4eebfd8 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3227,10 +3227,6 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, dev_priv-rps.min_freq_softlimit); - if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS)) -GENFREQSTATUS) == 0, 5)) - DRM_ERROR(timed out waiting for Punit\n); - As I recall the entire point of this was to make sure the frequency really drops before we allow turning off the clock. I'm not sure if we can trust that the frequency (and more importantly the voltage) will drop if we allow turning off the clock before the transition is complete. well, we may need to increase the timeout then... let's see if that helps with the bug. FYI I played around with my BYT a bit and it doesn't appear to require this force gfx clock on part of the workaround. I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to make sure both render and media wells and the gfx clock remain off, and I was also monitoring vnn via svid. While that was going on I just rewrote PUNIT_REG_GPU_FREQ_REQ to make Punit change the frequency, and sure enough it did, and svid showed me that vnn had also changed. So it appears there's no need to have the gfx clock on to change its frequency on this BYT. I wonder if this part of the workaround was only needed on older parts. Deepak, any ideas? Yes ville, this was added initial for older parts and force gfx clock was part of the workaround. We have not verified this on newer parts. Let me check with hw guys to see if workaround still exits and when this was fixed. Thanks Deepak ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/bdw: Implement a basic PM interrupt handler
On 4/16/2014 2:11 PM, Ville Syrjälä wrote:
On Tue, Apr 15, 2014 at 07:43:07PM -0700, Ben Widawsky wrote:
On Mon, Apr 14, 2014 at 10:55:53PM +0300, Ville Syrjälä wrote:
On Mon, Apr 14, 2014 at 10:41:14PM +0530, deepa...@intel.com wrote:
From: Ben Widawsky benjamin.widaw...@intel.com
Almost all of it is reusable from the existing code. The primary
difference is we need to do even less in the interrupt handler, since
interrupts are not shared in the same way.
The patch is mostly a copy-paste of the existing snb+ code, with updates
to the relevant parts requiring changes to the interrupt handling. As
such it /should/ be relatively trivial. It's highly likely that I missed
some places where I need a gen8 version of the PM interrupts, but it has
become invisible to me by now.
This patch could probably be split into adding the new functions,
followed by actually handling the interrupts. Since the code is
currently disabled (and broken) I think the patch stands better by
itself.
v2: Move the commit about not touching the ringbuffer interrupt to the
snb_* function where it belongs (Rodrigo)
v3: Rebased on Paulo's runtime PM changes
v4: Not well validated, but rebase on
commit 730488b2eddded4497f63f70867b1256cd9e117c
Author: Paulo Zanoni paulo.r.zan...@intel.com
Date: Fri Mar 7 20:12:32 2014 -0300
drm/i915: kill dev_priv-pm.regsave
v5: Rebased on latest code base. (Deepak)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Conflicts:
drivers/gpu/drm/i915/i915_irq.c
IIRC Daniel doesn't like these conflict markers. So should be dropped.
I like the conflict markers generally. Daniel can kill it if he likes,
but thanks for the input. I've killed it this time around, but I don't
plan on it for the future.
---
drivers/gpu/drm/i915/i915_irq.c | 81 +---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_pm.c | 38 ++-
4 files changed, 115 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7a4d3ae..96c459a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -248,6 +248,50 @@ static bool ivb_can_enable_err_int(struct drm_device *dev)
return true;
}
+/**
+ * bdw_update_pm_irq - update GT interrupt 2
+ * @dev_priv: driver private
+ * @interrupt_mask: mask of interrupt bits to update
+ * @enabled_irq_mask: mask of interrupt bits to enable
+ *
+ * Copied from the snb function, updated with relevant register offsets
+ */
+static void bdw_update_pm_irq(struct drm_i915_private *dev_priv,
+ uint32_t interrupt_mask,
+ uint32_t enabled_irq_mask)
+{
+ uint32_t new_val;
+
+ assert_spin_locked(dev_priv-irq_lock);
+
+ if (dev_priv-pm.irqs_disabled) {
+ WARN(1, IRQs disabled\n);
+ return;
+ }
+
+ new_val = dev_priv-pm_irq_mask;
+ new_val = ~interrupt_mask;
+ new_val |= (~enabled_irq_mask interrupt_mask);
+
+ if (new_val != dev_priv-pm_irq_mask) {
+ dev_priv-pm_irq_mask = new_val;
+ I915_WRITE(GEN8_GT_IMR(2), I915_READ(GEN8_GT_IMR(2)) |
+ dev_priv-pm_irq_mask);
+ POSTING_READ(GEN8_GT_IMR(2));
+ }
+}
+
+void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, mask);
+}
+
+void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, 0);
+}
+
+
Unnecessary empty line.
Got it, thanks.
static bool cpt_can_enable_serr_int(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -1126,13 +1170,17 @@ static void gen6_pm_rps_work(struct work_struct *work)
spin_lock_irq(dev_priv-irq_lock);
pm_iir = dev_priv-rps.pm_iir;
dev_priv-rps.pm_iir = 0;
- /* Make sure not to corrupt PMIMR state used by ringbuffer code */
- snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ if (IS_BROADWELL(dev_priv-dev))
+ bdw_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ else {
+ /* Make sure not to corrupt PMIMR state used by ringbuffer */
+ snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ /* Make sure we didn't queue anything we're not going to
+* process. */
+ WARN_ON(pm_iir ~dev_priv-pm_rps_events);
+ }
spin_unlock_irq(dev_priv-irq_lock);
- /* Make sure we didn't queue anything we're not going to process. */
- WARN_ON(pm_iir ~dev_priv-pm_rps_events);
Isn't this WARN equally valid for bdw?
So first, let me just mention, this has moved slightly in my latest
version of this patch, but it's still a valid question.
The answer is ambiguous actually. The
Re: [Intel-gfx] [PATCH 1/2] drm/i915/bdw: Implement a basic PM interrupt handler
On 4/16/2014 2:11 PM, Ville Syrjälä wrote:
On Tue, Apr 15, 2014 at 07:43:07PM -0700, Ben Widawsky wrote:
On Mon, Apr 14, 2014 at 10:55:53PM +0300, Ville Syrjälä wrote:
On Mon, Apr 14, 2014 at 10:41:14PM +0530, deepa...@intel.com wrote:
From: Ben Widawsky benjamin.widaw...@intel.com
Almost all of it is reusable from the existing code. The primary
difference is we need to do even less in the interrupt handler, since
interrupts are not shared in the same way.
The patch is mostly a copy-paste of the existing snb+ code, with updates
to the relevant parts requiring changes to the interrupt handling. As
such it /should/ be relatively trivial. It's highly likely that I missed
some places where I need a gen8 version of the PM interrupts, but it has
become invisible to me by now.
This patch could probably be split into adding the new functions,
followed by actually handling the interrupts. Since the code is
currently disabled (and broken) I think the patch stands better by
itself.
v2: Move the commit about not touching the ringbuffer interrupt to the
snb_* function where it belongs (Rodrigo)
v3: Rebased on Paulo's runtime PM changes
v4: Not well validated, but rebase on
commit 730488b2eddded4497f63f70867b1256cd9e117c
Author: Paulo Zanoni paulo.r.zan...@intel.com
Date: Fri Mar 7 20:12:32 2014 -0300
drm/i915: kill dev_priv-pm.regsave
v5: Rebased on latest code base. (Deepak)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Conflicts:
drivers/gpu/drm/i915/i915_irq.c
IIRC Daniel doesn't like these conflict markers. So should be dropped.
I like the conflict markers generally. Daniel can kill it if he likes,
but thanks for the input. I've killed it this time around, but I don't
plan on it for the future.
---
drivers/gpu/drm/i915/i915_irq.c | 81 +---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_pm.c | 38 ++-
4 files changed, 115 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7a4d3ae..96c459a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -248,6 +248,50 @@ static bool ivb_can_enable_err_int(struct drm_device *dev)
return true;
}
+/**
+ * bdw_update_pm_irq - update GT interrupt 2
+ * @dev_priv: driver private
+ * @interrupt_mask: mask of interrupt bits to update
+ * @enabled_irq_mask: mask of interrupt bits to enable
+ *
+ * Copied from the snb function, updated with relevant register offsets
+ */
+static void bdw_update_pm_irq(struct drm_i915_private *dev_priv,
+ uint32_t interrupt_mask,
+ uint32_t enabled_irq_mask)
+{
+ uint32_t new_val;
+
+ assert_spin_locked(dev_priv-irq_lock);
+
+ if (dev_priv-pm.irqs_disabled) {
+ WARN(1, IRQs disabled\n);
+ return;
+ }
+
+ new_val = dev_priv-pm_irq_mask;
+ new_val = ~interrupt_mask;
+ new_val |= (~enabled_irq_mask interrupt_mask);
+
+ if (new_val != dev_priv-pm_irq_mask) {
+ dev_priv-pm_irq_mask = new_val;
+ I915_WRITE(GEN8_GT_IMR(2), I915_READ(GEN8_GT_IMR(2)) |
+ dev_priv-pm_irq_mask);
+ POSTING_READ(GEN8_GT_IMR(2));
+ }
+}
+
+void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, mask);
+}
+
+void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, 0);
+}
+
+
Unnecessary empty line.
Got it, thanks.
static bool cpt_can_enable_serr_int(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -1126,13 +1170,17 @@ static void gen6_pm_rps_work(struct work_struct *work)
spin_lock_irq(dev_priv-irq_lock);
pm_iir = dev_priv-rps.pm_iir;
dev_priv-rps.pm_iir = 0;
- /* Make sure not to corrupt PMIMR state used by ringbuffer code */
- snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ if (IS_BROADWELL(dev_priv-dev))
+ bdw_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ else {
+ /* Make sure not to corrupt PMIMR state used by ringbuffer */
+ snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ /* Make sure we didn't queue anything we're not going to
+* process. */
+ WARN_ON(pm_iir ~dev_priv-pm_rps_events);
+ }
spin_unlock_irq(dev_priv-irq_lock);
- /* Make sure we didn't queue anything we're not going to process. */
- WARN_ON(pm_iir ~dev_priv-pm_rps_events);
Isn't this WARN equally valid for bdw?
So first, let me just mention, this has moved slightly in my latest
version of this patch, but it's still a valid question.
The answer is ambiguous actually. The
Re: [Intel-gfx] [PATCH 00/71] drm/i915/chv: Add Cherryview support
On 4/15/2014 9:46 PM, Ville Syrjälä wrote: On Tue, Apr 15, 2014 at 09:19:27PM +0530, S, Deepak wrote: On 4/10/2014 7:34 PM, Ville Syrjälä wrote: On Thu, Apr 10, 2014 at 04:41:10PM +0300, Jani Nikula wrote: On Thu, 10 Apr 2014, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Thu, Apr 10, 2014 at 09:31:39AM +0530, S, Deepak wrote: On 4/10/2014 1:30 AM, Daniel Vetter wrote: On Thu, Apr 10, 2014 at 12:42:42AM +0530, S, Deepak wrote: On 4/9/2014 10:23 PM, Daniel Vetter wrote: On Wed, Apr 09, 2014 at 06:05:27PM +0300, Ville Syrjälä wrote: On Wed, Apr 09, 2014 at 02:30:52PM +, S, Deepak wrote: Hi Ville, I am Ok with cleaning up and pushing the Code. Can you please tell me when we need to start pushing the code and branch to use (drm-intel-next)? Well you can consider it pushed now that it's in the open. The patches just need a bit of extra polish I think. Well, unless you're planning a full blown rewrite of the code ;) I guess you need to take into consideration whatever bdw rc6/rps patches are still in flight, but since you've been doing some review there I think you have a better idea than I do how things are progressing. I always work on top of nightly, so I guess that's a good choice :) Yes, -nightly is always the recommended branch to base upstream patches on. I'll sort out the conflict mess (or well, try to) if it doesn't apply to plain dinq or some other branch. drm-intel-next tends to be too outdated ;-) -Daniel Hi Daniel/Ville. Some of the patches are lined up for squashing right? So you want me to work on this patches to align to upstream code and resubmit it to same email thread? Hm, I expect this chv thread to become a bit mess really quickly tbh ;-) And since we don't have chv merged yet there's not really a baseline to do this on top. I guess the simplest approach would be for you to grab ville's chv tree, squash in the patches as discussed and then just starting on polishing your chv patches. Then as I pull in patches from this series you can drop them from yours. A bit messy, but I don't see any other approach really. Note that a pile of people are signed up to review this, so maybe hold off a bit until the review for your patches have been done. -Daniel Thanks Daniel. Ville can you please share your chv tree details? I rebased the lot and pushed here: git://gitorious.org/vsyrjala/linux.git chv_rebase /me being lazy, did you squash/reorder patches, i.e. do the patch # assignments [1] for review still apply? The numbers would get shifted around a bit due to two these two patches already getting merged: drm/i915/chv: IS_BROADWELL() should not be true for Cherryview drm/i915/chv: Add IS_CHERRYVIEW() macro And this patch got dropped as it no longer applies: drm/i915/chv: Add plane C support Apart from that no reordering/squashing. Jani. [1] http://mid.gmane.org/20140410110857.gw18...@intel.com -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center Hi Ville, Have you already squashed some of the RC6/turbo patches? Or you want me to do it as part of RC6/RPS rework patches submission. No, I didn't do it. If you can take care of it that'd be great. Ok, I will take care of squashing the patch. I will submit all rc6/rps patches after cleanup. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/71] drm/i915/chv: Add Cherryview support
Hi Ville, I am Ok with cleaning up and pushing the Code. Can you please tell me when we need to start pushing the code and branch to use (drm-intel-next)? Thanks Deepak -Original Message- From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] Sent: Wednesday, April 9, 2014 6:55 PM To: intel-gfx@lists.freedesktop.org Cc: S, Deepak; Deak, Imre Subject: Re: [PATCH 00/71] drm/i915/chv: Add Cherryview support As you may have noticed some of this stuf may need a bit of work still to fit in better with the more recent upstream changes. So I think we need a few people to do some work here to the the patches into a really good shape. So some division of labor would in order. I'm proposing the following: rc6/turbo - Deepak as he's written the code anyway interrupts - Imre since he worked on the VLV interrupts recently The rest of the patches should be fairly OK I think. So for the rest we probably just need reviews mostly, and maybe we want to squash some of the more trivial patches. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/71] drm/i915/chv: Add Cherryview support
Thanks Ville. I agree patches need polishing :). I might rewrite some of the patches based on testing :) Most of the BDW Rc6/Rps patches are in. I guess some of the interrupt related patch are pending. Thanks Deepak On 4/9/2014 8:35 PM, Ville Syrjälä wrote: On Wed, Apr 09, 2014 at 02:30:52PM +, S, Deepak wrote: Hi Ville, I am Ok with cleaning up and pushing the Code. Can you please tell me when we need to start pushing the code and branch to use (drm-intel-next)? Well you can consider it pushed now that it's in the open. The patches just need a bit of extra polish I think. Well, unless you're planning a full blown rewrite of the code ;) I guess you need to take into consideration whatever bdw rc6/rps patches are still in flight, but since you've been doing some review there I think you have a better idea than I do how things are progressing. I always work on top of nightly, so I guess that's a good choice :) Thanks Deepak -Original Message- From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] Sent: Wednesday, April 9, 2014 6:55 PM To: intel-gfx@lists.freedesktop.org Cc: S, Deepak; Deak, Imre Subject: Re: [PATCH 00/71] drm/i915/chv: Add Cherryview support As you may have noticed some of this stuf may need a bit of work still to fit in better with the more recent upstream changes. So I think we need a few people to do some work here to the the patches into a really good shape. So some division of labor would in order. I'm proposing the following: rc6/turbo - Deepak as he's written the code anyway interrupts - Imre since he worked on the VLV interrupts recently The rest of the patches should be fairly OK I think. So for the rest we probably just need reviews mostly, and maybe we want to squash some of the more trivial patches. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/71] drm/i915/chv: Add Cherryview support
On 4/9/2014 10:23 PM, Daniel Vetter wrote: On Wed, Apr 09, 2014 at 06:05:27PM +0300, Ville Syrjälä wrote: On Wed, Apr 09, 2014 at 02:30:52PM +, S, Deepak wrote: Hi Ville, I am Ok with cleaning up and pushing the Code. Can you please tell me when we need to start pushing the code and branch to use (drm-intel-next)? Well you can consider it pushed now that it's in the open. The patches just need a bit of extra polish I think. Well, unless you're planning a full blown rewrite of the code ;) I guess you need to take into consideration whatever bdw rc6/rps patches are still in flight, but since you've been doing some review there I think you have a better idea than I do how things are progressing. I always work on top of nightly, so I guess that's a good choice :) Yes, -nightly is always the recommended branch to base upstream patches on. I'll sort out the conflict mess (or well, try to) if it doesn't apply to plain dinq or some other branch. drm-intel-next tends to be too outdated ;-) -Daniel Hi Daniel/Ville. Some of the patches are lined up for squashing right? So you want me to work on this patches to align to upstream code and resubmit it to same email thread? Thanks Deepak ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/71] drm/i915/chv: Add Cherryview support
On 4/10/2014 1:30 AM, Daniel Vetter wrote: On Thu, Apr 10, 2014 at 12:42:42AM +0530, S, Deepak wrote: On 4/9/2014 10:23 PM, Daniel Vetter wrote: On Wed, Apr 09, 2014 at 06:05:27PM +0300, Ville Syrjälä wrote: On Wed, Apr 09, 2014 at 02:30:52PM +, S, Deepak wrote: Hi Ville, I am Ok with cleaning up and pushing the Code. Can you please tell me when we need to start pushing the code and branch to use (drm-intel-next)? Well you can consider it pushed now that it's in the open. The patches just need a bit of extra polish I think. Well, unless you're planning a full blown rewrite of the code ;) I guess you need to take into consideration whatever bdw rc6/rps patches are still in flight, but since you've been doing some review there I think you have a better idea than I do how things are progressing. I always work on top of nightly, so I guess that's a good choice :) Yes, -nightly is always the recommended branch to base upstream patches on. I'll sort out the conflict mess (or well, try to) if it doesn't apply to plain dinq or some other branch. drm-intel-next tends to be too outdated ;-) -Daniel Hi Daniel/Ville. Some of the patches are lined up for squashing right? So you want me to work on this patches to align to upstream code and resubmit it to same email thread? Hm, I expect this chv thread to become a bit mess really quickly tbh ;-) And since we don't have chv merged yet there's not really a baseline to do this on top. I guess the simplest approach would be for you to grab ville's chv tree, squash in the patches as discussed and then just starting on polishing your chv patches. Then as I pull in patches from this series you can drop them from yours. A bit messy, but I don't see any other approach really. Note that a pile of people are signed up to review this, so maybe hold off a bit until the review for your patches have been done. -Daniel Thanks Daniel. Ville can you please share your chv tree details? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] drm/i915: Bring UP Power Wells before disabling RC6.
On 4/8/2014 6:13 PM, Ville Syrjälä wrote:
On Mon, Apr 07, 2014 at 02:36:20PM -0700, Ben Widawsky wrote:
On Mon, Apr 07, 2014 at 05:01:46PM -0300, Rodrigo Vivi wrote:
From: Deepak S deepa...@intel.com
We need do forcewake before Disabling RC6, This is what the BIOS
expects while going into suspend.
v2: updated commit message. (Daniel)
Signed-off-by: Deepak S deepa...@intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/intel_pm.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 04af065..ad2ff99 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3198,8 +3198,14 @@ static void valleyview_disable_rps(struct drm_device
*dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
+ /* we're doing forcewake before Disabling RC6,
+* This what the BIOS expects when going into suspend */
+ gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
+
I915_WRITE(GEN6_RC_CONTROL, 0);
+ gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
+
gen6_disable_rps_interrupts(dev);
}
Isn't the forcewake done as part of I915_WRITE sufficient?
Writes don't do forcewake, nor is the register even part of the
VLV forcewake ranges.
I guess the rationale for this patche is still a bit vague. But if it's
really needed, I wonder whether we should do this same dance for !VLV
too? Do we have any GPU stuck in wrong power state after suspend type of
bugs still around?
One of suggestion form the HW team was to Bring the wells up before we
disable RC6 at run-time. We did see some issue when we enabled D0ix.
I think the is a good practice to make sure we bring-up the wells before
we disable RC6. At least this avoids the cases where wells are not up
before we can access the Next register after disable.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] drm/i915: Bring UP Power Wells before disabling RC6.
On 4/9/2014 9:43 AM, Ben Widawsky wrote:
On Tue, Apr 08, 2014 at 06:22:52PM +0530, S, Deepak wrote:
On 4/8/2014 6:13 PM, Ville Syrjälä wrote:
On Mon, Apr 07, 2014 at 02:36:20PM -0700, Ben Widawsky wrote:
On Mon, Apr 07, 2014 at 05:01:46PM -0300, Rodrigo Vivi wrote:
From: Deepak S deepa...@intel.com
We need do forcewake before Disabling RC6, This is what the BIOS
expects while going into suspend.
v2: updated commit message. (Daniel)
Signed-off-by: Deepak S deepa...@intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/intel_pm.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 04af065..ad2ff99 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3198,8 +3198,14 @@ static void valleyview_disable_rps(struct drm_device
*dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
+ /* we're doing forcewake before Disabling RC6,
+* This what the BIOS expects when going into suspend */
+ gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
+
I915_WRITE(GEN6_RC_CONTROL, 0);
+ gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
+
gen6_disable_rps_interrupts(dev);
}
Isn't the forcewake done as part of I915_WRITE sufficient?
Writes don't do forcewake, nor is the register even part of the
VLV forcewake ranges.
I guess the rationale for this patche is still a bit vague. But if it's
really needed, I wonder whether we should do this same dance for !VLV
too? Do we have any GPU stuck in wrong power state after suspend type of
bugs still around?
One of suggestion form the HW team was to Bring the wells up before we
disable RC6 at run-time. We did see some issue when we enabled D0ix.
I think the is a good practice to make sure we bring-up the wells before we
disable RC6. At least this avoids the cases where wells are not up before we
can access the Next register after disable.
Ville was totally right. I do think a POSTING_READ is still sufficient.
Don't care much either way.
If feel this patch is not adding any value. I OK dropping this patch.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/vlv: WA for Turbo and RC6 to work together.
On 3/13/2014 11:47 PM, Ville Syrjälä wrote:
On Thu, Mar 13, 2014 at 09:30:17PM +0530, deepa...@linux.intel.com wrote:
From: Deepak S deepa...@intel.com
With RC6 enabled, BYT has an HW issue in determining the right
Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide
on increasing/decreasing the freq. This logic will monitor C0
counters of render/media power-wells over EI period and takes
necessary action based on these values
v2: Refactor duplicate code. (Ville)
v3: Reformat the comments. (Ville)
Signed-off-by: Deepak S deepa...@linux.intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 17 +
drivers/gpu/drm/i915/i915_irq.c | 133
drivers/gpu/drm/i915/i915_reg.h | 13 +++-
drivers/gpu/drm/i915/intel_pm.c | 19 +++---
4 files changed, 173 insertions(+), 9 deletions(-)
snip
@@ -1564,6 +1683,16 @@ static void gen6_rps_irq_handler(struct drm_i915_private
*dev_priv, u32 pm_iir)
queue_work(dev_priv-wq, dev_priv-rps.work);
}
+ if (pm_iir GEN6_PM_RP_UP_EI_EXPIRED) {
+ spin_lock(dev_priv-irq_lock);
+ dev_priv-rps.pm_iir |= pm_iir GEN6_PM_RP_UP_EI_EXPIRED;
+ snb_disable_pm_irq(dev_priv, pm_iir GEN6_PM_RP_UP_EI_EXPIRED);
+ spin_unlock(dev_priv-irq_lock);
+ DRM_DEBUG_DRIVER(\nQueueing RPS Work - RC6 WA Turbo);
This debug message still seems rather pointless. Just drop it.
Oh actually isn't this entire block of code useless now that
pm_rps_events is used? The previous if block already checked
pm_rps_events which will include GEN6_PM_RP_UP_EI_EXPIRED on
VLV, so this code here will just repeat the work already done.
hmmm. I think i missed this in internal review. with pm_rps_events i
think this is redundant.
+
+ queue_work(dev_priv-wq, dev_priv-rps.work);
+ }
+
if (HAS_VEBOX(dev_priv-dev)) {
if (pm_iir PM_VEBOX_USER_INTERRUPT)
notify_ring(dev_priv-dev, dev_priv-ring[VECS]);
@@ -2989,6 +3118,10 @@ static void gen5_gt_irq_postinstall(struct drm_device
*dev)
pm_irqs |= PM_VEBOX_USER_INTERRUPT;
dev_priv-pm_irq_mask = 0x;
+
+ dev_priv-pm_irq_mask = ~dev_priv-pm_rps_events;
+ pm_irqs |= dev_priv-pm_rps_events;
What's this stuff doing here?
+
I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
I915_WRITE(GEN6_PMIMR, dev_priv-pm_irq_mask);
I915_WRITE(GEN6_PMIER, pm_irqs);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6174fda..d978b46 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -419,6 +419,7 @@ enum punit_power_well {
#define PUNIT_REG_GPU_FREQ_STS0xd8
#define GENFREQSTATUS (10)
#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ0xdc
+#define PUNIT_REG_CZ_TIMESTAMP 0xce
#define PUNIT_FUSE_BUS2 0xf6 /* bits 47:40 */
#define PUNIT_FUSE_BUS1 0xf5 /* bits 55:48 */
@@ -434,6 +435,11 @@ enum punit_power_well {
#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27
#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf800
+#define VLV_CZ_CLOCK_TO_MILLI_SEC 10
+#define VLV_RP_UP_EI_THRESHOLD 90
+#define VLV_RP_DOWN_EI_THRESHOLD 70
+#define VLV_INT_COUNT_FOR_DOWN_EI 5
+
/* vlv2 north clock has */
#define CCK_FUSE_REG 0x8
#define CCK_FUSE_HPLL_FREQ_MASK 0x3
@@ -4892,6 +4898,7 @@ enum punit_power_well {
#define VLV_GTLC_PW_STATUS 0x130094
#define VLV_GTLC_PW_RENDER_STATUS_MASK0x80
#define VLV_GTLC_PW_MEDIA_STATUS_MASK 0x20
+#define VLV_GTLC_SURVIVABILITY_REG 0x130098
#define FORCEWAKE_MT 0xa188 /* multi-threaded */
#define FORCEWAKE_KERNEL0x1
#define FORCEWAKE_USER 0x2
@@ -5019,13 +5026,17 @@ enum punit_power_well {
#define GEN6_GT_GFX_RC6_LOCKED0x138104
#define VLV_COUNTER_CONTROL 0x138104
+#define VLV_RC_COUNTER_CONTROL 0x00FF
I'd still like to see names for all the bits we frob, and I'd
still like to have some kind of an answer to the question whether
we really need to enable them all when the w/a is only interested
in the rc0 counters.
I did try with enabling only the rc0 counters, but the busyness
calculation was not right. Let me do some more investigation and get
back to you on this.
#define VLV_COUNT_RANGE_HIGH(115)
+#define VLV_MEDIA_RC0_COUNT_EN (15)
+#define VLV_RENDER_RC0_COUNT_EN (14)
#define VLV_MEDIA_RC6_COUNT_EN (11)
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Track the enabled PM interrupts in dev_priv.
On 3/13/2014 11:46 PM, Ville Syrjälä wrote: On Thu, Mar 13, 2014 at 09:30:16PM +0530, deepa...@linux.intel.com wrote: From: Deepak S deepa...@intel.com When we use different rps events for different platform or due to wa, we mgiht end up doing (vs) everywahere. Insted of this, Let's use a variable in dev_priv to track the enabled PM interrupts Signed-off-by: Deepak S deepa...@linux.intel.com --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 14 +++--- drivers/gpu/drm/i915/intel_pm.c | 14 +- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 70fbe90..d522313 100644 snip @@ -3311,6 +3311,8 @@ static void gen8_enable_rps(struct drm_device *dev) GEN6_RP_UP_BUSY_AVG | GEN6_RP_DOWN_IDLE_AVG); + dev_priv-pm_rps_events = GEN6_PM_RPS_EVENTS; + /* 6: Ring frequency + overclocking (our driver does this later */ gen6_set_rps(dev, (I915_READ(GEN6_GT_PERF_STATUS) 0xff00) 8); @@ -3430,6 +3432,7 @@ static void gen6_enable_rps(struct drm_device *dev) dev_priv-rps.power = HIGH_POWER; /* force a reset */ gen6_set_rps(dev_priv-dev, dev_priv-rps.min_delay); + dev_priv-pm_rps_events = GEN6_PM_RPS_EVENTS; gen6_enable_rps_interrupts(dev); rc6vids = 0; @@ -3688,6 +3691,7 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv-rps.rp_up_masked = false; dev_priv-rps.rp_down_masked = false; + dev_priv-pm_rps_events = GEN6_PM_RPS_EVENTS; gen6_enable_rps_interrupts(dev); gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL); I think we need to initialize pm_rps_events somewhere earlier since we depend on it already in irq postinstall. Othwewise the patch looks good. Adding it in functions intel_uncore_early_sanitize or pm_init as this gets executed before irq_install in driver_load? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: Add boot paramter to control rps boost at boot time.
On 3/13/2014 11:46 PM, Ville Syrjälä wrote:
On Thu, Mar 13, 2014 at 09:30:18PM +0530, deepa...@linux.intel.com wrote:
From: Deepak S deepa...@intel.com
We are adding a module paramter to control rps boost. By default, we
enable the boost for better performace. Based on the need (perf/power)
we can either enable/disable.
v2: Addressed rps default comment (Jani)
Signed-off-by: Deepak S deepa...@linux.intel.com
---
drivers/gpu/drm/i915/i915_drv.h| 1 +
drivers/gpu/drm/i915/i915_gem.c| 16 +++-
drivers/gpu/drm/i915/i915_params.c | 5 +
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 607042b..7808319 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2106,6 +2106,7 @@ struct i915_params {
int panel_use_ssc;
int vbt_sdvo_panel_type;
int enable_rc6;
+ int enable_rps_boost;
Should be bool like Jani said. And then it should be thrown somewhere
somewhere at the end of the structure next to the other bools.
I will address this.
int enable_fbc;
int enable_ppgtt;
int enable_psr;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 92b0b41..23a4700 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1002,6 +1002,17 @@ static bool can_wait_boost(struct drm_i915_file_private
*file_priv)
return !atomic_xchg(file_priv-rps_wait_boost, true);
}
+static int intel_enable_rps_boost(struct drm_device *dev)
+{
+ /* No RPS Boost before Ironlake */
This comment is still wrong. I'd just drop it, everyone should know what
the gen check below means.
Ok
+ if (INTEL_INFO(dev)-gen 6)
+ return 0;
+
+ /* Respect the kernel parameter if it is set */
This comment too seems rather obvious. I'd drop it as well.
Ok
+ return i915.enable_rps_boost;
+
+}
This function is still just a wrapper for i915.enable_rps_boost since
__wait_seqno() already does the gen check. You could just check
i915.enable_rps_boost directly in __wait_seqno(). The other option is
to just drop the gen check from __wait_seqno() and just let this
function take care of it. Hmm. Yeah that might be the nicest choice in
fact.
Agreed. Does not make sense to have multiple platform check's.
+
/**
* __wait_seqno - wait until execution of seqno has finished
* @ring: the ring expected to report seqno
@@ -1042,8 +1053,11 @@ static int __wait_seqno(struct intel_ring_buffer *ring,
u32 seqno,
timeout_expire = timeout ? jiffies +
timespec_to_jiffies_timeout(timeout) : 0;
- if (INTEL_INFO(dev)-gen = 6 can_wait_boost(file_priv)) {
+ if (INTEL_INFO(dev)-gen = 6 can_wait_boost(file_priv)
+ intel_enable_rps_boost(ring-dev)) {
Indentation is quite wrong. There's also trailing whitespace around
these parts. Please run patches through checkpatch.pl before submitting.
+
gen6_rps_boost(dev_priv);
+
if (file_priv)
mod_delayed_work(dev_priv-wq,
file_priv-mm.idle_work,
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index a66ffb6..2d207e3 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -34,6 +34,7 @@ struct i915_params i915 __read_mostly = {
.panel_use_ssc = -1,
.vbt_sdvo_panel_type = -1,
.enable_rc6 = -1,
+ .enable_rps_boost = 1,
true
.enable_fbc = -1,
.enable_hangcheck = true,
.enable_ppgtt = -1,
@@ -78,6 +79,10 @@ MODULE_PARM_DESC(enable_rc6,
For example, 3 would enable rc6 and deep rc6, and 7 would enable
everything.
default: -1 (use per-chip default));
+module_param_named(enable_rps_boost, i915.enable_rps_boost, int, 0600);
bool
+MODULE_PARM_DESC(enable_rps_boost,
+ Enable/Disable boost RPS frequency (default: enabled (1)));
default: true
+
module_param_named(enable_fbc, i915.enable_fbc, int, 0600);
MODULE_PARM_DESC(enable_fbc,
Enable frame buffer compression for power savings
--
1.8.4.2
___
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
Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: WA for Turbo and RC6 to work together.
On 3/5/2014 5:41 PM, Ville Syrjälä wrote:
On Mon, Mar 03, 2014 at 11:35:50AM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
With RC6 enabled, BYT has an HW issue in determining the right
Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide
on increasing/decreasing the freq. This logic will monitor C0
counters of render/media power-wells over EI period and takes
necessary action based on these values
v2: Refactor duplicate code. (ville)
Signed-off-by: Deepak S deepa...@intel.com
Did we reach some conclusion about this approach? It seemed to save
power in some workloads at least, but there's still the question
whether it ramps up the frquency fast enoguh to provide a good user
experience. Maybe we should make it optional even on VLV?
I have made this as a optional for the VLV. The boost logic is enabled
by default. If we need power savings then we can turn off the boost and
regular turbo logic will be enabled.
I will working on other options that Dainel suggested of identifying the
libva workload and controlling the boost at run time.
I will address the other comments and upload the patch for review.
---
drivers/gpu/drm/i915/i915_drv.h | 19 ++
drivers/gpu/drm/i915/i915_irq.c | 146 ++--
drivers/gpu/drm/i915/i915_reg.h | 15 +
drivers/gpu/drm/i915/intel_pm.c | 50 ++
4 files changed, 213 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 728b9c3..2baeeef 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -957,6 +957,12 @@ struct i915_suspend_saved_registers {
u32 savePCH_PORT_HOTPLUG;
};
+struct intel_rps_ei_calc {
+ u32 cz_ts_ei;
+ u32 render_ei_c0;
+ u32 media_ei_c0;
+};
+
struct intel_gen6_power_mgmt {
/* work and pm_iir are protected by dev_priv-irq_lock */
struct work_struct work;
@@ -969,10 +975,16 @@ struct intel_gen6_power_mgmt {
u8 rp1_delay;
u8 rp0_delay;
u8 hw_max;
+ u8 hw_min;
Some leftover still?
bool rp_up_masked;
bool rp_down_masked;
+ u32 cz_freq;
This too seems unused.
+ u32 ei_interrupt_count;
+
+ bool use_RC0_residency_for_turbo;
+
int last_adj;
enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
@@ -1531,6 +1543,13 @@ typedef struct drm_i915_private {
/* gen6+ rps state */
struct intel_gen6_power_mgmt rps;
+ /* rps wa up ei calculation */
+ struct intel_rps_ei_calc rps_up_ei;
+
+ /* rps wa down ei calculation */
+ struct intel_rps_ei_calc rps_down_ei;
+
+
/* ilk-only ips/rps state. Everything in here is protected by the global
* mchdev_lock in intel_pm.c */
struct intel_ilk_power_mgmt ips;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 56edff3..93b6ebf 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1023,6 +1023,120 @@ void gen6_set_pm_mask(struct drm_i915_private *dev_priv,
}
}
+static u32 vlv_c0_residency(struct drm_i915_private *dev_priv,
+ struct intel_rps_ei_calc *rps_ei)
+{
+ u32 cz_ts, cz_freq_khz;
+ u32 render_count, media_count;
+ u32 elapsed_render, elapsed_media, elapsed_time;
+ u32 residency = 0;
+
+ cz_ts = vlv_punit_read(dev_priv, PUNIT_REG_CZ_TIMESTAMP);
+ cz_freq_khz = DIV_ROUND_CLOSEST(dev_priv-mem_freq * 1000, 4);
+
+ render_count = I915_READ(VLV_RENDER_C0_COUNT_REG);
+ media_count = I915_READ(VLV_MEDIA_C0_COUNT_REG);
+
+ if (rps_ei-cz_ts_ei == 0) {
+ rps_ei-cz_ts_ei = cz_ts;
+ rps_ei-render_ei_c0 = render_count;
+ rps_ei-media_ei_c0 = media_count;
+
+ return dev_priv-rps.cur_delay;
+ }
+
+ elapsed_time = cz_ts - rps_ei-cz_ts_ei;
+ rps_ei-cz_ts_ei = cz_ts;
+
+ elapsed_render = render_count - rps_ei-render_ei_c0;
+ rps_ei-render_ei_c0 = render_count;
+
+ elapsed_media = media_count - rps_ei-media_ei_c0;
+ rps_ei-media_ei_c0 = media_count;
+
+ /* Convert all the counters into common unit of milli sec */
+ elapsed_time /= VLV_CZ_CLOCK_TO_MILLI_SEC;
+ elapsed_render /= cz_freq_khz;
+ elapsed_media /= cz_freq_khz;
+
+ /* Calculate overall C0 residency percentage only
+ * if elapsed time is non zero
+ */
Badly formatted comment.
+ if (elapsed_time) {
+ residency =
+ ((max(elapsed_render, elapsed_media) * 100)
+ / elapsed_time);
+ }
+
+ return residency;
+}
+
+
+/**
+ * vlv_calc_delay_from_C0_counters - Increase/Decrease freq based on GPU
+ * busy-ness calculated from C0 counters of render media power wells
+ * @dev_priv: DRM device private
+ *
+ */
Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: WA for Turbo and RC6 to work together.
Hi Ville,
Please review the patch and share the comments
Thanks
Deepak
On 3/3/2014 11:35 AM, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
With RC6 enabled, BYT has an HW issue in determining the right
Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide
on increasing/decreasing the freq. This logic will monitor C0
counters of render/media power-wells over EI period and takes
necessary action based on these values
v2: Refactor duplicate code. (ville)
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 19 ++
drivers/gpu/drm/i915/i915_irq.c | 146 ++--
drivers/gpu/drm/i915/i915_reg.h | 15 +
drivers/gpu/drm/i915/intel_pm.c | 50 ++
4 files changed, 213 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 728b9c3..2baeeef 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -957,6 +957,12 @@ struct i915_suspend_saved_registers {
u32 savePCH_PORT_HOTPLUG;
};
+struct intel_rps_ei_calc {
+ u32 cz_ts_ei;
+ u32 render_ei_c0;
+ u32 media_ei_c0;
+};
+
struct intel_gen6_power_mgmt {
/* work and pm_iir are protected by dev_priv-irq_lock */
struct work_struct work;
@@ -969,10 +975,16 @@ struct intel_gen6_power_mgmt {
u8 rp1_delay;
u8 rp0_delay;
u8 hw_max;
+ u8 hw_min;
bool rp_up_masked;
bool rp_down_masked;
+ u32 cz_freq;
+ u32 ei_interrupt_count;
+
+ bool use_RC0_residency_for_turbo;
+
int last_adj;
enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
@@ -1531,6 +1543,13 @@ typedef struct drm_i915_private {
/* gen6+ rps state */
struct intel_gen6_power_mgmt rps;
+ /* rps wa up ei calculation */
+ struct intel_rps_ei_calc rps_up_ei;
+
+ /* rps wa down ei calculation */
+ struct intel_rps_ei_calc rps_down_ei;
+
+
/* ilk-only ips/rps state. Everything in here is protected by the global
* mchdev_lock in intel_pm.c */
struct intel_ilk_power_mgmt ips;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 56edff3..93b6ebf 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1023,6 +1023,120 @@ void gen6_set_pm_mask(struct drm_i915_private *dev_priv,
}
}
+static u32 vlv_c0_residency(struct drm_i915_private *dev_priv,
+ struct intel_rps_ei_calc *rps_ei)
+{
+ u32 cz_ts, cz_freq_khz;
+ u32 render_count, media_count;
+ u32 elapsed_render, elapsed_media, elapsed_time;
+ u32 residency = 0;
+
+ cz_ts = vlv_punit_read(dev_priv, PUNIT_REG_CZ_TIMESTAMP);
+ cz_freq_khz = DIV_ROUND_CLOSEST(dev_priv-mem_freq * 1000, 4);
+
+ render_count = I915_READ(VLV_RENDER_C0_COUNT_REG);
+ media_count = I915_READ(VLV_MEDIA_C0_COUNT_REG);
+
+ if (rps_ei-cz_ts_ei == 0) {
+ rps_ei-cz_ts_ei = cz_ts;
+ rps_ei-render_ei_c0 = render_count;
+ rps_ei-media_ei_c0 = media_count;
+
+ return dev_priv-rps.cur_delay;
+ }
+
+ elapsed_time = cz_ts - rps_ei-cz_ts_ei;
+ rps_ei-cz_ts_ei = cz_ts;
+
+ elapsed_render = render_count - rps_ei-render_ei_c0;
+ rps_ei-render_ei_c0 = render_count;
+
+ elapsed_media = media_count - rps_ei-media_ei_c0;
+ rps_ei-media_ei_c0 = media_count;
+
+ /* Convert all the counters into common unit of milli sec */
+ elapsed_time /= VLV_CZ_CLOCK_TO_MILLI_SEC;
+ elapsed_render /= cz_freq_khz;
+ elapsed_media /= cz_freq_khz;
+
+ /* Calculate overall C0 residency percentage only
+ * if elapsed time is non zero
+ */
+ if (elapsed_time) {
+ residency =
+ ((max(elapsed_render, elapsed_media) * 100)
+ / elapsed_time);
+ }
+
+ return residency;
+}
+
+
+/**
+ * vlv_calc_delay_from_C0_counters - Increase/Decrease freq based on GPU
+ * busy-ness calculated from C0 counters of render media power wells
+ * @dev_priv: DRM device private
+ *
+ */
+static u32 vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv)
+{
+ u32 residency_C0_up = 0, residency_C0_down = 0;
+ u8 new_delay;
+
+ dev_priv-rps.ei_interrupt_count++;
+
+ WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock));
+
+
+ if (dev_priv-rps_up_ei.cz_ts_ei == 0) {
+ vlv_c0_residency(dev_priv, dev_priv-rps_up_ei);
+ vlv_c0_residency(dev_priv, dev_priv-rps_down_ei);
+ return dev_priv-rps.cur_delay;
+ }
+
+
+ /* To down throttle, C0 residency should be less than down threshold
+ * for continous EI intervals. So calculate down EI counters
+ * once in VLV_INT_COUNT_FOR_DOWN_EI
+ */
+ if
Re: [Intel-gfx] [PATCH 3/2] drm/i915: Streamline VLV forcewake handling
On 2/28/2014 1:37 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
It occured to me that when we're trying to wake up both render
and media wells on VLV, we might end up calling the low level
force_wake_get/put two times even though one call would be
enough. Make that happen by figuring out which wells really
need to be woken up based on the forcewake counts.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_uncore.c | 70 +++--
1 file changed, 29 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index dacb751..4119ddc 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -251,16 +251,16 @@ void vlv_force_wake_get(struct drm_i915_private *dev_priv,
unsigned long irqflags;
spin_lock_irqsave(dev_priv-uncore.lock, irqflags);
- if (FORCEWAKE_RENDER fw_engine) {
- if (dev_priv-uncore.fw_rendercount++ == 0)
- dev_priv-uncore.funcs.force_wake_get(dev_priv,
- FORCEWAKE_RENDER);
- }
- if (FORCEWAKE_MEDIA fw_engine) {
- if (dev_priv-uncore.fw_mediacount++ == 0)
- dev_priv-uncore.funcs.force_wake_get(dev_priv,
- FORCEWAKE_MEDIA);
- }
+
+ if (fw_engine FORCEWAKE_RENDER
+ dev_priv-uncore.fw_rendercount++ != 0)
+ fw_engine = ~FORCEWAKE_RENDER;
+ if (fw_engine FORCEWAKE_MEDIA
+ dev_priv-uncore.fw_mediacount++ != 0)
+ fw_engine = ~FORCEWAKE_MEDIA;
Should we add WARN_ON? I think it will help us if we have forcewake
count mismatch?
Other than this. Patch looks good.
Reviewed-by:Deepak S deepa...@intel.com
+ if (fw_engine)
+ dev_priv-uncore.funcs.force_wake_get(dev_priv, fw_engine);
spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
}
@@ -272,19 +272,15 @@ void vlv_force_wake_put(struct drm_i915_private *dev_priv,
spin_lock_irqsave(dev_priv-uncore.lock, irqflags);
- if (FORCEWAKE_RENDER fw_engine) {
- WARN_ON(dev_priv-uncore.fw_rendercount == 0);
- if (--dev_priv-uncore.fw_rendercount == 0)
- dev_priv-uncore.funcs.force_wake_put(dev_priv,
- FORCEWAKE_RENDER);
- }
+ if (fw_engine FORCEWAKE_RENDER
+ --dev_priv-uncore.fw_rendercount != 0)
+ fw_engine = ~FORCEWAKE_RENDER;
+ if (fw_engine FORCEWAKE_MEDIA
+ --dev_priv-uncore.fw_mediacount != 0)
+ fw_engine = ~FORCEWAKE_MEDIA;
- if (FORCEWAKE_MEDIA fw_engine) {
- WARN_ON(dev_priv-uncore.fw_mediacount == 0);
- if (--dev_priv-uncore.fw_mediacount == 0)
- dev_priv-uncore.funcs.force_wake_put(dev_priv,
- FORCEWAKE_MEDIA);
- }
+ if (fw_engine)
+ dev_priv-uncore.funcs.force_wake_put(dev_priv, fw_engine);
spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
}
@@ -502,27 +498,19 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t
reg, bool trace) { \
static u##x \
vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
unsigned fwengine = 0; \
- unsigned fwcount; \
REG_READ_HEADER(x); \
- if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) { \
- fwengine = FORCEWAKE_RENDER;\
- fwcount = dev_priv-uncore.fw_rendercount;\
- } \
- else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) { \
- fwengine = FORCEWAKE_MEDIA; \
- fwcount = dev_priv-uncore.fw_mediacount; \
+ if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) { \
+ if (dev_priv-uncore.fw_rendercount == 0) \
+ fwengine = FORCEWAKE_RENDER; \
+ } else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) { \
+ if (dev_priv-uncore.fw_mediacount == 0) \
+ fwengine = FORCEWAKE_MEDIA; \
} \
- if (fwengine != 0) {\
- if (fwcount == 0) \
- (dev_priv)-uncore.funcs.force_wake_get(dev_priv, \
- fwengine); \
- val = __raw_i915_read##x(dev_priv, reg); \
- if (fwcount == 0) \
- (dev_priv)-uncore.funcs.force_wake_put(dev_priv, \
- fwengine); \
- } else { \
- val = __raw_i915_read##x(dev_priv, reg); \
- } \
+ if (fwengine) \
+
Re: [Intel-gfx] Fwd: [PATCH 1/2] drm/i915: Fix VLV forcewake after reset
On Wed, Jan 29, 2014 at 9:55 AM, ville syrjala
-- Forwarded message --
From: ** ville.syrj...@linux.intel.com
mailto:ville.syrj...@linux.intel.com
Date: Mon, Feb 24, 2014 at 8:32 PM
Subject: [Intel-gfx] [PATCH 1/2] drm/i915: Fix VLV forcewake after reset
To: intel-gfx@lists.freedesktop.org mailto:intel-gfx@lists.freedesktop.org
From: Ville Syrjälä ville.syrj...@linux.intel.com
mailto:ville.syrj...@linux.intel.com
Use the render/media specific forcewake counts to properly restore the
forcewake status after a GPU reset on VLV.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
mailto:ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_uncore.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index c628414..09fa555 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -967,10 +967,22 @@ static int gen6_do_reset(struct drm_device *dev)
intel_uncore_forcewake_reset(dev);
/* If reset with a user forcewake, try to restore, otherwise
turn it off */
- if (dev_priv-uncore.forcewake_count)
- dev_priv-uncore.funcs.force_wake_get(dev_priv,
FORCEWAKE_ALL);
- else
- dev_priv-uncore.funcs.force_wake_put(dev_priv,
FORCEWAKE_ALL);
+ if (IS_VALLEYVIEW(dev)) {
+ if (dev_priv-uncore.fw_rendercount)
+ dev_priv-uncore.funcs.force_wake_get(dev_priv,
FORCEWAKE_RENDER);
+ else
+ dev_priv-uncore.funcs.force_wake_put(dev_priv,
FORCEWAKE_RENDER);
+
+ if (dev_priv-uncore.fw_mediacount)
+ dev_priv-uncore.funcs.force_wake_get(dev_priv,
FORCEWAKE_MEDIA);
+ else
+ dev_priv-uncore.funcs.force_wake_put(dev_priv,
FORCEWAKE_MEDIA);
+ } else {
+ if (dev_priv-uncore.forcewake_count)
+ dev_priv-uncore.funcs.force_wake_get(dev_priv,
FORCEWAKE_ALL);
+ else
+ dev_priv-uncore.funcs.force_wake_put(dev_priv,
FORCEWAKE_ALL);
+ }
/* Restore fifo count */
dev_priv-uncore.fifo_count = __raw_i915_read32(dev_priv,
GTFIFOCTL) GT_FIFO_FREE_ENTRIES_MASK;
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org mailto:Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by: Deepak S deepa...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Fwd: [PATCH 2/2] drm/i915: Drop the forcewake count inc/dec around register read on VLV
On Wed, Jan 29, 2014 at 9:55 AM, ville syrjala
-- Forwarded message --
From: ** ville.syrj...@linux.intel.com
mailto:ville.syrj...@linux.intel.com
Date: Mon, Feb 24, 2014 at 8:32 PM
Subject: [Intel-gfx] [PATCH 2/2] drm/i915: Drop the forcewake count
inc/dec around register read on VLV
To: intel-gfx@lists.freedesktop.org mailto:intel-gfx@lists.freedesktop.org
From: Ville Syrjälä ville.syrj...@linux.intel.com
mailto:ville.syrj...@linux.intel.com
VLV is the only platform where we increment/decrement the forcewake
count around register access. Drop the inc/dec on VLV to make the
forcewake code a bit more unified.
The inc/dec are not necessary since we hold the uncore lock around
the whole operation.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
mailto:ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_uncore.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 09fa555..dacb751 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -502,22 +502,22 @@ gen6_read##x(struct drm_i915_private *dev_priv,
off_t reg, bool trace) { \
static u##x \
vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
unsigned fwengine = 0; \
- unsigned *fwcount; \
+ unsigned fwcount; \
REG_READ_HEADER(x); \
if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) { \
fwengine = FORCEWAKE_RENDER;\
- fwcount = dev_priv-uncore.fw_rendercount;\
+ fwcount = dev_priv-uncore.fw_rendercount;\
} \
else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) { \
fwengine = FORCEWAKE_MEDIA; \
- fwcount = dev_priv-uncore.fw_mediacount; \
+ fwcount = dev_priv-uncore.fw_mediacount; \
} \
if (fwengine != 0) {\
- if ((*fwcount)++ == 0) \
+ if (fwcount == 0) \
(dev_priv)-uncore.funcs.force_wake_get(dev_priv, \
fwengine); \
val = __raw_i915_read##x(dev_priv, reg); \
- if (--(*fwcount) == 0) \
+ if (fwcount == 0) \
(dev_priv)-uncore.funcs.force_wake_put(dev_priv, \
fwengine); \
} else { \
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org mailto:Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by: Deepak S deepa...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/9] drm/i915: Clarify RC6 enabling
On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky benjamin.widaw...@intel.com mailto:benjamin.widaw...@intel.com wrote: At one time, we though all future platforms would have the deeper RC6 states. As it turned out, they killed it after Ivybridge, and began using other means to achieve the power savings (the stuff we need to get to PC7+). The enable function was left in a weird state of odd corner cases as a result. Since the future is now, and we also have some insight into what's currently the future, we have an opportunity to simplify, and future proof the function. NOTE: VLV will be addressed in a subsequent patch. This patch was trying not to change functionality. NOTE2: All callers sanitize the return value anyway, so this patch is simply to have the code make a bit more sense. Signed-off-by: Ben Widawsky b...@bwidawsk.net mailto:b...@bwidawsk.net --- drivers/gpu/drm/i915/intel_pm.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 53d64bb..bcbdac2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3161,14 +3161,10 @@ int intel_enable_rc6(const struct drm_device *dev) if (INTEL_INFO(dev)-gen == 5) return 0; - if (IS_HASWELL(dev)) - return INTEL_RC6_ENABLE; - - /* snb/ivb have more than one rc6 state. */ - if (INTEL_INFO(dev)-gen == 6) + if (IS_IVYBRIDGE(dev) || IS_VALLEYVIEW(dev)) + return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE); + else return INTEL_RC6_ENABLE; - - return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE); } static void gen6_enable_rps_interrupts(struct drm_device *dev) -- 1.8.5.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org mailto:Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx Reviewed-by: Deepak S deepa...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/9] drm/i915: Stop pretending VLV has rc6+
On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky benjamin.widaw...@intel.com mailto:benjamin.widaw...@intel.com wrote: It wasn't ever used by the caller anyway with the exception of what we show in sysfs. Signed-off-by: Ben Widawsky b...@bwidawsk.net mailto:b...@bwidawsk.net --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index bcbdac2..258241b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3161,7 +3161,7 @@ int intel_enable_rc6(const struct drm_device *dev) if (INTEL_INFO(dev)-gen == 5) return 0; - if (IS_IVYBRIDGE(dev) || IS_VALLEYVIEW(dev)) + if (IS_IVYBRIDGE(dev)) return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE); else return INTEL_RC6_ENABLE; -- 1.8.5.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org mailto:Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx I think, we can avoid else condition and return INTEL_RC6_ENABLE if the other platform checks fails? Reviewed-by: Deepak S deepa...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: Just print rc6 facts
On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky
benjamin.widaw...@intel.com mailto:benjamin.widaw...@intel.com wrote:
Everything can be overridden by module parameters, so don't confuse the
users that are using them.
We have RC6 turned on for all platforms which support it, but Ironlake,
so the need to explain the situation is no longer pressing.
Signed-off-by: Ben Widawsky b...@bwidawsk.net mailto:b...@bwidawsk.net
---
drivers/gpu/drm/i915/intel_pm.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 258241b..944b99c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3135,16 +3135,10 @@ static void valleyview_disable_rps(struct
drm_device *dev)
static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
{
- if (IS_GEN6(dev))
- DRM_DEBUG_DRIVER(Sandybridge: deep RC6 disabled\n);
-
- if (IS_HASWELL(dev))
- DRM_DEBUG_DRIVER(Haswell: only RC6 available\n);
-
DRM_INFO(Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n,
- (mode GEN6_RC_CTL_RC6_ENABLE) ? on : off,
- (mode GEN6_RC_CTL_RC6p_ENABLE) ? on : off,
- (mode GEN6_RC_CTL_RC6pp_ENABLE) ? on :
off);
+(mode GEN6_RC_CTL_RC6_ENABLE) ? on : off,
+(mode GEN6_RC_CTL_RC6p_ENABLE) ? on : off,
+(mode GEN6_RC_CTL_RC6pp_ENABLE) ? on : off);
}
int intel_enable_rc6(const struct drm_device *dev)
--
1.8.5.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org mailto:Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by: Deepak S deepa...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/9] drm/i915/bdw: Use centralized rc6 info print
On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky benjamin.widaw...@intel.com mailto:benjamin.widaw...@intel.com wrote: Signed-off-by: Ben Widawsky b...@bwidawsk.net mailto:b...@bwidawsk.net --- drivers/gpu/drm/i915/intel_pm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 944b99c..6acb429 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3215,10 +3215,10 @@ static void gen8_enable_rps(struct drm_device *dev) /* 3: Enable RC6 */ if (intel_enable_rc6(dev) INTEL_RC6_ENABLE) rc6_mask = GEN6_RC_CTL_RC6_ENABLE; - DRM_INFO(RC6 %s\n, (rc6_mask GEN6_RC_CTL_RC6_ENABLE) ? on : off); + intel_print_rc6_info(dev, rc6_mask); I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE | - GEN6_RC_CTL_EI_MODE(1) | - rc6_mask); + GEN6_RC_CTL_EI_MODE(1) | + rc6_mask); /* 4 Program defaults and thresholds for RPS*/ I915_WRITE(GEN6_RPNSWREQ, HSW_FREQUENCY(10)); /* Request 500 MHz */ -- 1.8.5.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org mailto:Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx Reviewed-by: Deepak S deepa...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/9] drm/i915/bdw: Set rp_state_caps
On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky
benjamin.widaw...@intel.com mailto:benjamin.widaw...@intel.com wrote:
Signed-off-by: Ben Widawsky b...@bwidawsk.net mailto:b...@bwidawsk.net
---
drivers/gpu/drm/i915/intel_pm.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 6acb429..ae59bd9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3184,6 +3184,17 @@ static void gen6_enable_rps_interrupts(struct
drm_device *dev)
I915_WRITE(GEN6_PMINTRMSK, ~enabled_intrs);
}
+static void parse_rp_state_cap(struct drm_i915_private *dev_priv,
u32 rp_state_cap)
+{
+ /* In units of 50MHz */
+ dev_priv-rps.hw_max = dev_priv-rps.max_delay =
rp_state_cap 0xff;
+ dev_priv-rps.min_delay = (rp_state_cap 16) 0xff;
+ dev_priv-rps.rp1_delay = (rp_state_cap 8) 0xff;
+ dev_priv-rps.rp0_delay = (rp_state_cap 0) 0xff;
+ dev_priv-rps.rpe_delay = dev_priv-rps.rp1_delay;
+ dev_priv-rps.cur_delay = 0;
+}
+
static void gen8_enable_rps(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -3202,6 +3213,7 @@ static void gen8_enable_rps(struct drm_device
*dev)
I915_WRITE(GEN6_RC_CONTROL, 0);
rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+ parse_rp_state_cap(dev_priv, rp_state_cap);
/* 2b: Program RC6 thresholds.*/
I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 16);
@@ -3288,13 +3300,7 @@ static void gen6_enable_rps(struct drm_device
*dev)
rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
- /* In units of 50MHz */
- dev_priv-rps.hw_max = dev_priv-rps.max_delay =
rp_state_cap 0xff;
- dev_priv-rps.min_delay = (rp_state_cap 16) 0xff;
- dev_priv-rps.rp1_delay = (rp_state_cap 8) 0xff;
- dev_priv-rps.rp0_delay = (rp_state_cap 0) 0xff;
- dev_priv-rps.rpe_delay = dev_priv-rps.rp1_delay;
- dev_priv-rps.cur_delay = 0;
+ parse_rp_state_cap(dev_priv, rp_state_cap);
/* disable the counters and set deterministic thresholds */
I915_WRITE(GEN6_RC_CONTROL, 0);
--
1.8.5.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org mailto:Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Looks fine
Reviewed-by: Deepak S deepa...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/9] drm/i915/bdw: RPS frequency bits are the same as HSW
On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky benjamin.widaw...@intel.com mailto:benjamin.widaw...@intel.com wrote: Signed-off-by: Ben Widawsky b...@bwidawsk.net mailto:b...@bwidawsk.net --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 34cc898..deaaaf2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3016,7 +3016,7 @@ void gen6_set_rps(struct drm_device *dev, u8 val) gen6_set_rps_thresholds(dev_priv, val); - if (IS_HASWELL(dev)) + if (IS_HASWELL(dev) || IS_BROADWELL(dev)) I915_WRITE(GEN6_RPNSWREQ, HSW_FREQUENCY(val)); else -- 1.8.5.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org mailto:Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx Looks fine Reviewed-by: Deepak S deepa...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/9] drm/i915/bdw: Implement a basic PM interrupt handler
On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky
benjamin.widaw...@intel.com mailto:benjamin.widaw...@intel.com wrote:
Almost all of it is reusable from the existing code. The primary
difference is we need to do even less in the interrupt handler, since
interrupts are not shared in the same way.
The patch is mostly a copy-paste of the existing snb+ code, with updates
to the relevant parts requiring changes to the interrupt handling. As
such it /should/ be relatively trivial. It's highly likely that I missed
some places where I need a gen8 version of the PM interrupts, but it has
become invisible to me by now.
This patch could probably be split into adding the new functions,
followed by actually handling the interrupts. Since the code is
currently disabled (and broken) I think the patch stands better by
itself.
Signed-off-by: Ben Widawsky b...@bwidawsk.net mailto:b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_irq.c | 80
++--
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 2 +
drivers/gpu/drm/i915/intel_pm.c | 39 +++-
4 files changed, 117 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 72ade87..ab0c7ac 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -214,6 +214,53 @@ static bool ivb_can_enable_err_int(struct
drm_device *dev)
return true;
}
+/**
+ * bdw_update_pm_irq - update GT interrupt 2
+ * @dev_priv: driver private
+ * @interrupt_mask: mask of interrupt bits to update
+ * @enabled_irq_mask: mask of interrupt bits to enable
+ *
+ * Copied from the snb function, updated with relevant register
offsets
+ */
+static void bdw_update_pm_irq(struct drm_i915_private *dev_priv,
+ uint32_t interrupt_mask,
+ uint32_t enabled_irq_mask)
+{
+ uint32_t new_val;
+
+ assert_spin_locked(dev_priv-irq_lock);
+
+ if (dev_priv-pc8.irqs_disabled) {
+ WARN(1, IRQs disabled\n);
+ dev_priv-pc8.regsave.gen6_pmimr = ~interrupt_mask;
+ dev_priv-pc8.regsave.gen6_pmimr |= (~enabled_irq_mask
+interrupt_mask);
+ return;
+ }
+
+ new_val = dev_priv-pm_irq_mask;
+ new_val = ~interrupt_mask;
+ new_val |= (~enabled_irq_mask interrupt_mask);
+
+ if (new_val != dev_priv-pm_irq_mask) {
+ dev_priv-pm_irq_mask = new_val;
+ I915_WRITE(GEN8_GT_IMR(2), I915_READ(GEN8_GT_IMR(2)) |
+ dev_priv-pm_irq_mask);
+ POSTING_READ(GEN8_GT_IMR(2));
+ }
+}
+
+void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t
mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, mask);
+}
+
+void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t
mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, 0);
+}
+
+
static bool cpt_can_enable_serr_int(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -997,11 +1044,15 @@ static void gen6_pm_rps_work(struct
work_struct *work)
pm_iir = dev_priv-rps.pm_iir;
dev_priv-rps.pm_iir = 0;
/* Make sure not to corrupt PMIMR state used by ringbuffer
code */
- snb_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
+ if (IS_BROADWELL(dev_priv-dev))
+ bdw_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
+ else {
+ /* Make sure we didn't queue anything we're not
going to process. */
+ snb_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
+ WARN_ON(pm_iir ~GEN6_PM_RPS_EVENTS);
+ }
spin_unlock_irq(dev_priv-irq_lock);
- /* Make sure we didn't queue anything we're not going to
process. */
- WARN_ON(pm_iir ~GEN6_PM_RPS_EVENTS);
if ((pm_iir GEN6_PM_RPS_EVENTS) == 0)
return;
@@ -1192,6 +1243,19 @@ static void snb_gt_irq_handler(struct
drm_device *dev,
ivybridge_parity_error_irq_handler(dev, gt_iir);
}
+static void gen8_rps_irq_handler(struct drm_i915_private *dev_priv,
u32 pm_iir)
+{
+ if ((pm_iir GEN6_PM_RPS_EVENTS) == 0)
+ return;
+
+ spin_lock(dev_priv-irq_lock);
+ dev_priv-rps.pm_iir |= pm_iir GEN6_PM_RPS_EVENTS;
+ bdw_disable_pm_irq(dev_priv, pm_iir GEN6_PM_RPS_EVENTS);
+ spin_unlock(dev_priv-irq_lock);
+
Re: [Intel-gfx] [PATCH 9/9] drm/i915/bdw: Enable RC6
On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky
benjamin.widaw...@intel.com mailto:benjamin.widaw...@intel.com wrote:
It is tested and looking fairly stable now, so turn it on. It wasn't
intentionally turned off originally :P
Signed-off-by: Ben Widawsky b...@bwidawsk.net mailto:b...@bwidawsk.net
---
drivers/gpu/drm/i915/intel_pm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index a5c9142..377bd7f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4428,7 +4428,7 @@ void intel_enable_gt_powersave(struct
drm_device *dev)
ironlake_enable_drps(dev);
ironlake_enable_rc6(dev);
intel_init_emon(dev);
- } else if (IS_GEN6(dev) || IS_GEN7(dev)) {
+ } else if (IS_GEN6(dev) || IS_GEN7(dev) || IS_BROADWELL(dev)) {
/*
* PCU communication is slow and this doesn't need
to be
* done at any specific time, so do this out of our
fast path
--
1.8.5.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org mailto:Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by: Deepak S deepa...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 1/2] drm/i915: Disable/Enable PM Intrrupts based on the current freq.
On 1/30/2014 1:00 AM, Daniel Vetter wrote:
On Wed, Jan 29, 2014 at 05:59:23PM +0200, Ville Syrjälä wrote:
On Mon, Jan 27, 2014 at 09:35:05PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
When current delay is already at max delay, Let's disable the PM UP
THRESHOLD INTRRUPTS, so that we will not get further interrupts until
current delay is less than max delay, Also request for the PM DOWN
THRESHOLD INTRRUPTS to indicate the decrease in clock freq. and
viceversa for PM DOWN THRESHOLD INTRRUPTS.
v2: Use bool variables (Daniel)
v3: Fix Interrupt masking bit (Deepak)
v4: Use existing symbolic constants in i915_reg.h (Daniel)
v5: Add pm interrupt mask after new_delay calculation (Ville)
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/i915_irq.c | 39 +++
drivers/gpu/drm/i915/intel_pm.c | 3 +++
3 files changed, 45 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 56c720b..f19de66 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -943,6 +943,9 @@ struct intel_gen6_power_mgmt {
u8 rp0_delay;
u8 hw_max;
+ bool rp_up_masked;
+ bool rp_down_masked;
+
int last_adj;
enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 01a8686..69a5214 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -972,6 +972,43 @@ static void notify_ring(struct drm_device *dev,
i915_queue_hangcheck(dev);
}
+static void gen6_set_pm_mask(struct drm_i915_private *dev_priv,
+ u32 pm_iir, int *new_delay)
Just a minor nit here. We don't modify new_delay in this function, so
passing by value would be better.
I've fixed this up and merged the patch. I've also polished the whitespace
a bit, please run patches through scripts/checkpatch.pl before submitting.
I usually don't all go whitespace-nazi about this, but generally the
suggestions result in more uniform and hence readable sources.
Otherwise the patch looks good to me. So if you change that, you can
add:
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Thanks, Daniel
Thanks Daniel.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 2/2] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.
On 1/27/2014 10:37 PM, Ville Syrjälä wrote:
On Mon, Jan 27, 2014 at 09:35:06PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
When we enter RC6 and GFX Clocks are off, the voltage remains higher
than Vmin. When we try to set the freq to RPn, it might fail since the
Gfx clocks are down. So to fix this in Gfx idle, Bring the GFX clock up
and set the freq to RPn then move GFx down.
v2: remove vlv_update_rps_cur_delay function. Update commit message (Daniel)
v3: Fix the timeout during wait for gfx clock (Jesse)
v4: addressed comments on set freq and punit wait (Ville)
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 4
drivers/gpu/drm/i915/intel_pm.c | 53 -
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 242f540..feaa83b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4944,6 +4944,10 @@
GEN6_PM_RP_DOWN_THRESHOLD | \
GEN6_PM_RP_DOWN_TIMEOUT)
+#define VLV_GTLC_SURVIVABILITY_REG 0x130098
+#define VLV_GFX_CLK_STATUS_BIT (13)
+#define VLV_GFX_CLK_FORCE_ON_BIT (12)
+
#define GEN6_GT_GFX_RC6_LOCKED0x138104
#define VLV_COUNTER_CONTROL 0x138104
#define VLV_COUNT_RANGE_HIGH(115)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c6a07c9..84e20d0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3035,6 +3035,56 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
trace_intel_gpu_freq_change(val * 50);
}
+/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+ *
+ * * If Gfx is Idle, then
+ * 1. Mask Turbo interrupts
+ * 2. Bring up Gfx clock
+ * 3. Change the freq to Rpn and wait till P-Unit updates freq
+ * 4. Clear the Force GFX CLK ON bit so that Gfx can down
+ * 5. Unmask Turbo interrupts
+*/
+static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
+{
+ /*
+* When we are idle. Drop to min voltage state.
+*/
+
+ if (dev_priv-rps.cur_delay = dev_priv-rps.min_delay)
+ return;
If we're already at min freq I guess there's a good chance we're at the
min voltage too. But I'm not sure that's really guaranteed by anything.
Maybe it's enough. If not then I guess we should track whether we've
already called this function w/o going to higher voltage in between.
If we are already in min_freq we will just return right? Only if we have
crossed the min_delay and if the gpu is idle we are setting is freq back
to min.
+
+ /* Mask turbo interrupt so that they will not come in between */
+ I915_WRITE(GEN6_PMINTRMSK, 0x);
+
+ /* Bring up the Gfx clock */
+ I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG) |
+ VLV_GFX_CLK_FORCE_ON_BIT);
+
+ if (wait_for_atomic(((VLV_GFX_CLK_STATUS_BIT
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 5)) {
+ DRM_ERROR(GFX_CLK_ON request timed out\n);
+ return;
+ }
+
+ vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
dev_priv-rps.min_delay);
We should update cur_delay to reflect this.
I will fix this issue.
+
+ if (wait_for_atomic(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
+GENFREQSTATUS) == 0, 5))
+ DRM_DEBUG_DRIVER(timed out waiting for Punit\n);
+
+ /* Release the Gfx clock */
+ I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG)
+ ~VLV_GFX_CLK_FORCE_ON_BIT);
+
+ /* Unmask Turbo interrupts */
+ I915_WRITE(GEN6_PMINTRMSK, ~(GEN6_PM_RPS_EVENTS |
+ GEN6_PM_RP_UP_EI_EXPIRED));
Wouldn't that confuse the interrupt masking logic you just introduced
in the previous patch?
So looks to me like pretending we got a down threshold interrupt here
is all that's needed to keep things in sync. So somehting like:
gen6_set_pm_mask(dev_priv, GEN6_PM_RP_DOWN_THRESHOLD, min_delay);
+}
+
+
+
void gen6_rps_idle(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = dev_priv-dev;
@@ -3042,7 +3092,7 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
mutex_lock(dev_priv-rps.hw_lock);
if (dev_priv-rps.enabled) {
if (IS_VALLEYVIEW(dev))
- valleyview_set_rps(dev_priv-dev,
dev_priv-rps.min_delay);
+ vlv_set_rps_idle(dev_priv);
else
gen6_set_rps(dev_priv-dev, dev_priv-rps.min_delay);
dev_priv-rps.last_adj =
Re: [Intel-gfx] [PATCH v4 2/2] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.
On 1/27/2014 10:22 PM, Daniel Vetter wrote:
On Mon, Jan 27, 2014 at 09:35:06PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
When we enter RC6 and GFX Clocks are off, the voltage remains higher
than Vmin. When we try to set the freq to RPn, it might fail since the
Gfx clocks are down. So to fix this in Gfx idle, Bring the GFX clock up
and set the freq to RPn then move GFx down.
v2: remove vlv_update_rps_cur_delay function. Update commit message (Daniel)
v3: Fix the timeout during wait for gfx clock (Jesse)
v4: addressed comments on set freq and punit wait (Ville)
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 4
drivers/gpu/drm/i915/intel_pm.c | 53 -
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 242f540..feaa83b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4944,6 +4944,10 @@
GEN6_PM_RP_DOWN_THRESHOLD | \
GEN6_PM_RP_DOWN_TIMEOUT)
+#define VLV_GTLC_SURVIVABILITY_REG 0x130098
+#define VLV_GFX_CLK_STATUS_BIT (13)
+#define VLV_GFX_CLK_FORCE_ON_BIT (12)
+
#define GEN6_GT_GFX_RC6_LOCKED0x138104
#define VLV_COUNTER_CONTROL 0x138104
#define VLV_COUNT_RANGE_HIGH(115)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c6a07c9..84e20d0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3035,6 +3035,56 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
trace_intel_gpu_freq_change(val * 50);
}
+/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+ *
+ * * If Gfx is Idle, then
+ * 1. Mask Turbo interrupts
+ * 2. Bring up Gfx clock
+ * 3. Change the freq to Rpn and wait till P-Unit updates freq
+ * 4. Clear the Force GFX CLK ON bit so that Gfx can down
+ * 5. Unmask Turbo interrupts
+*/
+static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
+{
+ /*
+* When we are idle. Drop to min voltage state.
+*/
+
+ if (dev_priv-rps.cur_delay = dev_priv-rps.min_delay)
+ return;
+
+ /* Mask turbo interrupt so that they will not come in between */
+ I915_WRITE(GEN6_PMINTRMSK, 0x);
+
+ /* Bring up the Gfx clock */
+ I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG) |
+ VLV_GFX_CLK_FORCE_ON_BIT);
+
+ if (wait_for_atomic(((VLV_GFX_CLK_STATUS_BIT
Do we really need an atomic register busy loop here? Afaics this is only
called from process context, so the normal wait_for macro should be good
enough ...
-Daniel
I agree, the reason why i did the _atomic as we observed delay in
scheduling the wait_for and we ended up spending lot of time here.
I will wait for other comments, If i dont get any, I will address this
comment and submit the patch.
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 5)) {
+ DRM_ERROR(GFX_CLK_ON request timed out\n);
+ return;
+ }
+
+ vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
dev_priv-rps.min_delay);
+
+ if (wait_for_atomic(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
+GENFREQSTATUS) == 0, 5))
+ DRM_DEBUG_DRIVER(timed out waiting for Punit\n);
+
+ /* Release the Gfx clock */
+ I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG)
+ ~VLV_GFX_CLK_FORCE_ON_BIT);
+
+ /* Unmask Turbo interrupts */
+ I915_WRITE(GEN6_PMINTRMSK, ~(GEN6_PM_RPS_EVENTS |
+ GEN6_PM_RP_UP_EI_EXPIRED));
+}
+
+
+
void gen6_rps_idle(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = dev_priv-dev;
@@ -3042,7 +3092,7 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
mutex_lock(dev_priv-rps.hw_lock);
if (dev_priv-rps.enabled) {
if (IS_VALLEYVIEW(dev))
- valleyview_set_rps(dev_priv-dev,
dev_priv-rps.min_delay);
+ vlv_set_rps_idle(dev_priv);
else
gen6_set_rps(dev_priv-dev, dev_priv-rps.min_delay);
dev_priv-rps.last_adj = 0;
@@ -4273,6 +4323,7 @@ void intel_gpu_ips_teardown(void)
i915_mch_dev = NULL;
spin_unlock_irq(mchdev_lock);
}
+
static void intel_init_emon(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
--
1.8.5.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [PATCH v4 2/2] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.
On 1/29/2014 1:03 AM, Daniel Vetter wrote:
On Tue, Jan 28, 2014 at 08:02:56PM +0530, S, Deepak wrote:
On 1/27/2014 10:22 PM, Daniel Vetter wrote:
On Mon, Jan 27, 2014 at 09:35:06PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
When we enter RC6 and GFX Clocks are off, the voltage remains higher
than Vmin. When we try to set the freq to RPn, it might fail since the
Gfx clocks are down. So to fix this in Gfx idle, Bring the GFX clock up
and set the freq to RPn then move GFx down.
v2: remove vlv_update_rps_cur_delay function. Update commit message (Daniel)
v3: Fix the timeout during wait for gfx clock (Jesse)
v4: addressed comments on set freq and punit wait (Ville)
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 4
drivers/gpu/drm/i915/intel_pm.c | 53 -
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 242f540..feaa83b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4944,6 +4944,10 @@
GEN6_PM_RP_DOWN_THRESHOLD | \
GEN6_PM_RP_DOWN_TIMEOUT)
+#define VLV_GTLC_SURVIVABILITY_REG 0x130098
+#define VLV_GFX_CLK_STATUS_BIT (13)
+#define VLV_GFX_CLK_FORCE_ON_BIT (12)
+
#define GEN6_GT_GFX_RC6_LOCKED0x138104
#define VLV_COUNTER_CONTROL 0x138104
#define VLV_COUNT_RANGE_HIGH(115)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c6a07c9..84e20d0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3035,6 +3035,56 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
trace_intel_gpu_freq_change(val * 50);
}
+/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+ *
+ * * If Gfx is Idle, then
+ * 1. Mask Turbo interrupts
+ * 2. Bring up Gfx clock
+ * 3. Change the freq to Rpn and wait till P-Unit updates freq
+ * 4. Clear the Force GFX CLK ON bit so that Gfx can down
+ * 5. Unmask Turbo interrupts
+*/
+static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
+{
+ /*
+* When we are idle. Drop to min voltage state.
+*/
+
+ if (dev_priv-rps.cur_delay = dev_priv-rps.min_delay)
+ return;
+
+ /* Mask turbo interrupt so that they will not come in between */
+ I915_WRITE(GEN6_PMINTRMSK, 0x);
+
+ /* Bring up the Gfx clock */
+ I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG) |
+ VLV_GFX_CLK_FORCE_ON_BIT);
+
+ if (wait_for_atomic(((VLV_GFX_CLK_STATUS_BIT
Do we really need an atomic register busy loop here? Afaics this is only
called from process context, so the normal wait_for macro should be good
enough ...
-Daniel
I agree, the reason why i did the _atomic as we observed delay in
scheduling the wait_for and we ended up spending lot of time here.
I will wait for other comments, If i dont get any, I will address
this comment and submit the patch.
Now that we're using Chris' idle-clamping rps logic this would be run from
a work queue. So I hope that the potential scheduling delays here won't
affect things badly. If it does you can stick with the _atomic, but then
it needs a comment.
But I really hope that we only call this from worker threads, so shouldn't
matter too badly if there's a bit a scheduler delay.
-Daniel
I verified and we don't have a scheduling delays, I will change this to
_atomic to wait_for
-Deepak
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 5)) {
+ DRM_ERROR(GFX_CLK_ON request timed out\n);
+ return;
+ }
+
+ vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
dev_priv-rps.min_delay);
+
+ if (wait_for_atomic(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
+GENFREQSTATUS) == 0, 5))
+ DRM_DEBUG_DRIVER(timed out waiting for Punit\n);
+
+ /* Release the Gfx clock */
+ I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG)
+ ~VLV_GFX_CLK_FORCE_ON_BIT);
+
+ /* Unmask Turbo interrupts */
+ I915_WRITE(GEN6_PMINTRMSK, ~(GEN6_PM_RPS_EVENTS |
+ GEN6_PM_RP_UP_EI_EXPIRED));
+}
+
+
+
void gen6_rps_idle(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = dev_priv-dev;
@@ -3042,7 +3092,7 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
mutex_lock(dev_priv-rps.hw_lock);
if (dev_priv-rps.enabled) {
if (IS_VALLEYVIEW(dev))
- valleyview_set_rps(dev_priv-dev,
dev_priv-rps.min_delay
Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/vlv: WA for Turbo and RC6 to work together.
On 1/21/2014 8:48 PM, Ville Syrjälä wrote:
On Mon, Jan 20, 2014 at 06:40:26PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
With RC6 enabled, BYT has an HW issue in determining the right
Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide
on increasing/decreasing the freq. This logic will monitor C0
counters of render/media power-wells over EI period and takes
necessary action based on these values
Do we have any idea what kind of performance impact this should
have?
We don't have any performance impact with this WA. But without this WA,
since we don't get the down threshold interrupts and we end up running
at higher freq which will impact the power for Workload like media
playback.
v2: resolved conflict in i915_reg.h
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 13
drivers/gpu/drm/i915/i915_irq.c | 151 ++--
drivers/gpu/drm/i915/i915_reg.h | 19 +
drivers/gpu/drm/i915/intel_pm.c | 54 ++
4 files changed, 220 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e89b9f4..1d76461 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -942,10 +942,23 @@ struct intel_gen6_power_mgmt {
u8 rp1_delay;
u8 rp0_delay;
u8 hw_max;
+ u8 hw_min;
bool rp_up_masked;
bool rp_down_masked;
+ u32 cz_freq;
+ u32 ei_interrupt_count;
+
+ u32 cz_ts_up_ei;
+ u32 render_up_EI_C0;
+ u32 media_up_EI_C0;
+ u32 cz_ts_down_ei;
+ u32 render_down_EI_C0;
+ u32 media_down_EI_C0;
+
+ bool use_RC0_residency_for_turbo;
+
int last_adj;
enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d0d87ed..f4a3660 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -965,6 +965,123 @@ static void notify_ring(struct drm_device *dev,
i915_queue_hangcheck(dev);
}
+/**
+ * vlv_calc_delay_from_C0_counters - Increase/Decrease freq based on GPU
+ * busy-ness calculated from C0 counters of render media power wells
+ * @dev_priv: DRM device private
+ *
+ */
+static u32 vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv)
+{
+ u32 cz_ts = 0;
+ u32 render_count = 0, media_count = 0;
+ u32 elapsed_render = 0, elapsed_media = 0;
+ u32 elapsed_time = 0;
+ u32 residency_C0_up = 0, residency_C0_down = 0;
+ u8 new_delay;
+
+ dev_priv-rps.ei_interrupt_count++;
+
+ WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock));
+
+ cz_ts = vlv_punit_read(dev_priv, PUNIT_REG_CZ_TIMESTAMP);
+
+ render_count = I915_READ(VLV_RENDER_C0_COUNT_REG);
+ media_count = I915_READ(VLV_MEDIA_C0_COUNT_REG);
+
+ if (0 == dev_priv-rps.cz_ts_up_ei) {
People don't generally like this style.
+
+ dev_priv-rps.cz_ts_up_ei = dev_priv-rps.cz_ts_down_ei = cz_ts;
+ dev_priv-rps.render_up_EI_C0 = dev_priv-rps.render_down_EI_C0
+ = render_count;
+ dev_priv-rps.media_up_EI_C0 = dev_priv-rps.media_down_EI_C0
+ = media_count;
+
+ return dev_priv-rps.cur_delay;
+ }
+
+ elapsed_time = cz_ts - dev_priv-rps.cz_ts_up_ei;
+ dev_priv-rps.cz_ts_up_ei = cz_ts;
+
+ elapsed_render = render_count - dev_priv-rps.render_up_EI_C0;
+ dev_priv-rps.render_up_EI_C0 = render_count;
+
+ elapsed_media = media_count - dev_priv-rps.media_up_EI_C0;
+ dev_priv-rps.media_up_EI_C0 = media_count;
+
+ /* Convert all the counters into common unit of milli sec */
+ elapsed_time /= VLV_CZ_CLOCK_TO_MILLI_SEC;
+ elapsed_render /= (dev_priv-rps.cz_freq / 1000);
+ elapsed_media /= (dev_priv-rps.cz_freq / 1000);
We already have mem_freq, so cz_freq is duplicated information. Also
you're storing cz_freq in Hz and always throw away the last three
digits. I'd say either just use something like
DIV_ROUND_CLOSEST(mem_freq*1000, 4) or if the extra precision is useful,
change it so that mem_freq is in kHz from the start.
+
+ /* Calculate overall C0 residency percentage only
+ * if elapsed time is non zero
+ */
Weird formatting. Happens more later.
+ if (elapsed_time) {
+ residency_C0_up = ((max(elapsed_render, elapsed_media)
+ * 100) / elapsed_time);
+ }
+
+ /* To down throttle, C0 residency should be less than down threshold
+ * for continous EI intervals. So calculate down EI counters
+ * once in VLV_INT_COUNT_FOR_DOWN_EI
+ */
+ if (VLV_INT_COUNT_FOR_DOWN_EI == dev_priv-rps.ei_interrupt_count) {
Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/vlv: WA for Turbo and RC6 to work together.
On 1/22/2014 10:04 PM, Jesse Barnes wrote: On Tue, 21 Jan 2014 17:18:59 +0200 Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Mon, Jan 20, 2014 at 06:40:26PM +0530, deepa...@intel.com wrote: From: Deepak S deepa...@intel.com With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness. WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these values Do we have any idea what kind of performance impact this should have? So aside from the code review comments, it sounds like there are two high level issues: 1) keeping existing boost code from Chris (as mentioned by Ville) 2) power measurements vs current upstream Given that upstream is a bit different than when this code was forked off, it could be that we don't need this. Can you sanity check things by getting some power measurements with and without this patch? It looks like 1/3 and 2/3 will be required in any case though. Assuming 3/3 does show a benefit, the other question is whether keeping the current turbo boost code makes sense, so you'd have to port those changes from the gen6_pm_rps_work() from Chris and measure again... Thanks, Sure Jesse, I will do power measurement and get back to you on the results. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.
We're not actually waiting for Punit here. Should we?
Ideally yes, we need to wait for the Punit to grant the freq. Based on your
suggestion on vlv_update_rps_cur_delay that the punit will recheck the
situation periodically, and it will try to use PUNIT_REG_GPU_FREQ_REQ. I
removed the wait form this patch.
I do believe we need to wait here. To make sure the requested freq is set
before bring down the Gfx clk.
Also valleyview_set_rps() won't actually do anything if cur_delay ==
rpe_delay. Are we required to actually poke the Punit here, or is it enough
that we had already previously requested =rpe_delay? In that case I think it
might make sense to skip this if cur_delay = rpe_delay.
But if we really need to poke Punit to make sure it changes the
frequency/voltage, then I guess we should force the issue by eg. setting
cur_delay=0 just before calling valleyview_set_rps().
I agree, I think instead of valleyview_set_rps we can use vlv_punit_write
and request the freq and wait for punit to grant the freq if required.
Btw, still using outlook, I will send the next replay from new email client :)
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Tuesday, January 21, 2014 8:13 PM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/vlv: WA to fix Voltage not
getting dropped to Vmin when Gfx is power gated.
On Mon, Jan 20, 2014 at 06:40:25PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
When we enter RC6 and GFX Clocks are off, the voltage remains higher
than Vmin. When we try to set the freq to RPe, it might fail since the
Gfx clocks are down. So to fix this in Gfx idle, Bring the GFX clock
up and set the freq to RPe then move GFx down.
v2: remove vlv_update_rps_cur_delay function. Update commit message
(Daniel)
v3: Fix the timeout during wait for gfx clock (Jesse)
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 4
drivers/gpu/drm/i915/intel_pm.c | 48
-
2 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h index cc2f3de..e1d5f31 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4944,6 +4944,10 @@
GEN6_PM_RP_DOWN_THRESHOLD | \
GEN6_PM_RP_DOWN_TIMEOUT)
+#define VLV_GTLC_SURVIVABILITY_REG 0x130098
+#define VLV_GFX_CLK_STATUS_BIT (13)
+#define VLV_GFX_CLK_FORCE_ON_BIT (12)
+
#define GEN6_GT_GFX_RC6_LOCKED 0x138104
#define VLV_COUNTER_CONTROL 0x138104
#define VLV_COUNT_RANGE_HIGH (115)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index d00a2cf..86d87e5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3035,6 +3035,51 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
trace_intel_gpu_freq_change(val * 50); }
+/* vlv_set_rps_idle: Set the frequency to Rpe if Gfx clocks are down
+ *
+ * * If Gfx is Idle, then
+ * 1. Mask Turbo interrupts
+ * 2. Bring up Gfx clock
+ * 3. Change the freq to Rpe and wait till P-Unit updates freq
+ * 4. Clear the Force GFX CLK ON bit so that Gfx can down
+ * 5. Unmask Turbo interrupts
+*/
+static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) {
+ /*
+ * When we are idle. Drop to min voltage state.
+ */
+
+ if (dev_priv-rps.cur_delay == dev_priv-rps.rpe_delay)
+ return;
+
+ /* Mask turbo interrupt so that they will not come in between */
+ I915_WRITE(GEN6_PMINTRMSK, 0x);
+
+ /* Bring up the Gfx clock */
+ I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG) |
+ VLV_GFX_CLK_FORCE_ON_BIT);
+
+ if (wait_for_atomic_us(((VLV_GFX_CLK_STATUS_BIT
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 500)) {
+ DRM_ERROR(GFX_CLK_ON request timed out\n);
+ return;
+ }
+
+ valleyview_set_rps(dev_priv-dev, dev_priv-rps.rpe_delay);
We're not actually waiting for Punit here. Should we?
Also valleyview_set_rps() won't actually do anything if cur_delay == rpe_delay.
Are we required to actually poke the Punit here, or is it enough that we had
already previously requested =rpe_delay? In that case I think it might make
sense to skip this if cur_delay = rpe_delay.
But if we really need to poke Punit to make sure it changes the
frequency/voltage, then I guess we should force the issue by eg. setting
cur_delay=0 just before calling valleyview_set_rps().
+
+ /* Release the Gfx clock */
+ I915_WRITE(VLV_GTLC_SURVIVABILITY_REG
Re: [Intel-gfx] [PATCH v3 1/3] drm/i915: Disable/Enable PM Intrrupts based on the current freq.
Please use the symbolic constants in i915_reg.h, we can reuse the ones below
GEN6_PMIER. Also Chris had some comment to move this code around, but I didn't
really understand what he wants. Chris, can you please clarify?
Hi Chris,
Can you please clarify on this?
Thanks
Deepak
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Tuesday, January 14, 2014 2:52 PM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org; Chris Wilson
Subject: Re: [Intel-gfx] [PATCH v3 1/3] drm/i915: Disable/Enable PM Intrrupts
based on the current freq.
On Thu, Jan 09, 2014 at 07:31:08PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
When current delay is already at max delay, Let's disable the PM UP
THRESHOLD INTRRUPTS, so that we will not get further interrupts until
current delay is less than max delay, Also request for the PM DOWN
THRESHOLD INTRRUPTS to indicate the decrease in clock freq. and
viceversa for PM DOWN THRESHOLD INTRRUPTS.
v2: Use bool variables (Daniel)
v3: Fix Interrupt masking bit (Deepak)
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/i915_irq.c | 31 +--
drivers/gpu/drm/i915/intel_pm.c | 3 +++
3 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h index cc8afff..d49e674 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -943,6 +943,9 @@ struct intel_gen6_power_mgmt {
u8 rp0_delay;
u8 hw_max;
+ bool rp_up_masked;
+ bool rp_down_masked;
+
int last_adj;
enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c index 1d44c79..e87d47a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -988,7 +988,20 @@ static void gen6_pm_rps_work(struct work_struct *work)
adj *= 2;
else
adj = 1;
- new_delay = dev_priv-rps.cur_delay + adj;
+
+ if (dev_priv-rps.cur_delay = dev_priv-rps.max_delay) {
+ I915_WRITE(GEN6_PMINTRMSK,
+ I915_READ(GEN6_PMINTRMSK) | 1 5);
Please use the symbolic constants in i915_reg.h, we can reuse the ones below
GEN6_PMIER. Also Chris had some comment to move this code around, but I didn't
really understand what he wants. Chris, can you please clarify?
Thanks, Daniel
+ dev_priv-rps.rp_up_masked = true;
+ new_delay = dev_priv-rps.cur_delay;
+ } else
+ new_delay = dev_priv-rps.cur_delay + adj;
+
+ if (dev_priv-rps.rp_down_masked) {
+ I915_WRITE(GEN6_PMINTRMSK,
+ I915_READ(GEN6_PMINTRMSK) ~(1 4));
+ dev_priv-rps.rp_down_masked = false;
+ }
/*
* For better performance, jump directly @@ -1007,7 +1020,21 @@
static void gen6_pm_rps_work(struct work_struct *work)
adj *= 2;
else
adj = -1;
- new_delay = dev_priv-rps.cur_delay + adj;
+
+ if (dev_priv-rps.cur_delay = dev_priv-rps.min_delay) {
+ I915_WRITE(GEN6_PMINTRMSK,
+ I915_READ(GEN6_PMINTRMSK) | 1 4);
+ dev_priv-rps.rp_down_masked = true;
+ new_delay = dev_priv-rps.cur_delay;
+ } else
+ new_delay = dev_priv-rps.cur_delay + adj;
+
+ if (dev_priv-rps.rp_up_masked) {
+ I915_WRITE(GEN6_PMINTRMSK,
+ I915_READ(GEN6_PMINTRMSK) ~(1 5));
+ dev_priv-rps.rp_up_masked = false;
+ }
+
} else { /* unknown event */
new_delay = dev_priv-rps.cur_delay;
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 469170c..9c950e4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3628,6 +3628,9 @@ static void valleyview_enable_rps(struct drm_device
*dev)
vlv_gpu_freq(dev_priv, dev_priv-rps.rpe_delay),
dev_priv-rps.rpe_delay);
+ dev_priv-rps.rp_up_masked = false;
+ dev_priv-rps.rp_down_masked = false;
+
valleyview_set_rps(dev_priv-dev, dev_priv-rps.rpe_delay);
gen6_enable_rps_interrupts(dev);
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http
Re: [Intel-gfx] [PATCH v2 2/3] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.
Yeah if we need to bring the gfx clocks up (which makes sense) then I guess we
need this. I'm not sure about the wait on the punit though; that could end up
penalizing us in bursty workloads, since the punit can take quite some time to
update the freq, but I don't actually see a wait here?
Ville suggested we don't need wait after requesting the freq. AFAIK the punit
will recheck the situation periodically, and it will try to use
PUNIT_REG_GPU_FREQ_REQ.
Also, is the 500ms timeout really required for the gfx clock? That's a long
time to potentially hold the mutex and delay any clock boosting or other
activity...
Yes agree, we don't need 500ms. I will change the delay to 5ms and submit a
new command.
Thanks
Deepak
-Original Message-
From: Jesse Barnes [mailto:jbar...@virtuousgeek.org]
Sent: Tuesday, January 14, 2014 4:23 AM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 2/3] drm/i915/vlv: WA to fix Voltage not
getting dropped to Vmin when Gfx is power gated.
On Thu, 9 Jan 2014 19:31:09 +0530
deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
When we enter RC6 and GFX Clocks are off, the voltage remains higher
than Vmin. When we try to set the freq to RPe, it might fail since the
Gfx clocks are down. So to fix this in Gfx idle, Bring the GFX clock
up and set the freq to RPe then move GFx down.
v2: remove vlv_update_rps_cur_delay function. Update commit message
(Daniel)
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 4
drivers/gpu/drm/i915/intel_pm.c | 48
-
2 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h index a699efd..e37831f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4940,6 +4940,10 @@
GEN6_PM_RP_DOWN_THRESHOLD | \
GEN6_PM_RP_DOWN_TIMEOUT)
+#define VLV_GTLC_SURVIVABILITY_REG 0x130098
+#define VLV_GFX_CLK_STATUS_BIT (13)
+#define VLV_GFX_CLK_FORCE_ON_BIT (12)
+
#define GEN6_GT_GFX_RC6_LOCKED 0x138104
#define VLV_COUNTER_CONTROL 0x138104
#define VLV_COUNT_RANGE_HIGH (115)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 9c950e4..a8e05fe 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3050,6 +3050,51 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
trace_intel_gpu_freq_change(val * 50); }
+/* vlv_set_rps_idle: Set the frequency to Rpe if Gfx clocks are down
+ *
+ * * If Gfx is Idle, then
+ * 1. Mask Turbo interrupts
+ * 2. Bring up Gfx clock
+ * 3. Change the freq to Rpe and wait till P-Unit updates freq
+ * 4. Clear the Force GFX CLK ON bit so that Gfx can down
+ * 5. Unmask Turbo interrupts
+*/
+static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) {
+ /*
+ * When we are idle. Drop to min voltage state.
+ */
+
+ if (dev_priv-rps.cur_delay == dev_priv-rps.rpe_delay)
+ return;
+
+ /* Mask turbo interrupt so that they will not come in between */
+ I915_WRITE(GEN6_PMINTRMSK, 0x);
+
+ /* Bring up the Gfx clock */
+ I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG) |
+ VLV_GFX_CLK_FORCE_ON_BIT);
+
+ if (wait_for_atomic(((VLV_GFX_CLK_STATUS_BIT
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 500)) {
+ DRM_ERROR(GFX_CLK_ON request timed out\n);
+ return;
+ }
+
+ valleyview_set_rps(dev_priv-dev, dev_priv-rps.rpe_delay);
+
+ /* Release the Gfx clock */
+ I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG)
+ ~VLV_GFX_CLK_FORCE_ON_BIT);
+
+ /* Unmask Turbo interrupts */
+ I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS); }
+
+
+
void gen6_rps_idle(struct drm_i915_private *dev_priv) {
struct drm_device *dev = dev_priv-dev; @@ -3057,7 +3102,7 @@ void
gen6_rps_idle(struct drm_i915_private *dev_priv)
mutex_lock(dev_priv-rps.hw_lock);
if (dev_priv-rps.enabled) {
if (IS_VALLEYVIEW(dev))
- valleyview_set_rps(dev_priv-dev,
dev_priv-rps.min_delay);
+ vlv_set_rps_idle(dev_priv);
else
gen6_set_rps(dev_priv-dev, dev_priv-rps.min_delay);
dev_priv-rps.last_adj = 0;
@@ -4288,6 +4333,7 @@ void intel_gpu_ips_teardown(void)
i915_mch_dev = NULL;
spin_unlock_irq(mchdev_lock);
}
+
static void intel_init_emon(struct drm_device *dev) {
struct
Re: [Intel-gfx] [PATCH v3 1/3] drm/i915: Disable/Enable PM Intrrupts based on the current freq.
Thanks Daniel. I will wait for Chris feedback and then address the comments.
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Tuesday, January 14, 2014 2:52 PM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org; Chris Wilson
Subject: Re: [Intel-gfx] [PATCH v3 1/3] drm/i915: Disable/Enable PM Intrrupts
based on the current freq.
On Thu, Jan 09, 2014 at 07:31:08PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
When current delay is already at max delay, Let's disable the PM UP
THRESHOLD INTRRUPTS, so that we will not get further interrupts until
current delay is less than max delay, Also request for the PM DOWN
THRESHOLD INTRRUPTS to indicate the decrease in clock freq. and
viceversa for PM DOWN THRESHOLD INTRRUPTS.
v2: Use bool variables (Daniel)
v3: Fix Interrupt masking bit (Deepak)
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/i915_irq.c | 31 +--
drivers/gpu/drm/i915/intel_pm.c | 3 +++
3 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h index cc8afff..d49e674 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -943,6 +943,9 @@ struct intel_gen6_power_mgmt {
u8 rp0_delay;
u8 hw_max;
+ bool rp_up_masked;
+ bool rp_down_masked;
+
int last_adj;
enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c index 1d44c79..e87d47a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -988,7 +988,20 @@ static void gen6_pm_rps_work(struct work_struct *work)
adj *= 2;
else
adj = 1;
- new_delay = dev_priv-rps.cur_delay + adj;
+
+ if (dev_priv-rps.cur_delay = dev_priv-rps.max_delay) {
+ I915_WRITE(GEN6_PMINTRMSK,
+ I915_READ(GEN6_PMINTRMSK) | 1 5);
Please use the symbolic constants in i915_reg.h, we can reuse the ones below
GEN6_PMIER. Also Chris had some comment to move this code around, but I didn't
really understand what he wants. Chris, can you please clarify?
Thanks, Daniel
+ dev_priv-rps.rp_up_masked = true;
+ new_delay = dev_priv-rps.cur_delay;
+ } else
+ new_delay = dev_priv-rps.cur_delay + adj;
+
+ if (dev_priv-rps.rp_down_masked) {
+ I915_WRITE(GEN6_PMINTRMSK,
+ I915_READ(GEN6_PMINTRMSK) ~(1 4));
+ dev_priv-rps.rp_down_masked = false;
+ }
/*
* For better performance, jump directly @@ -1007,7 +1020,21 @@
static void gen6_pm_rps_work(struct work_struct *work)
adj *= 2;
else
adj = -1;
- new_delay = dev_priv-rps.cur_delay + adj;
+
+ if (dev_priv-rps.cur_delay = dev_priv-rps.min_delay) {
+ I915_WRITE(GEN6_PMINTRMSK,
+ I915_READ(GEN6_PMINTRMSK) | 1 4);
+ dev_priv-rps.rp_down_masked = true;
+ new_delay = dev_priv-rps.cur_delay;
+ } else
+ new_delay = dev_priv-rps.cur_delay + adj;
+
+ if (dev_priv-rps.rp_up_masked) {
+ I915_WRITE(GEN6_PMINTRMSK,
+ I915_READ(GEN6_PMINTRMSK) ~(1 5));
+ dev_priv-rps.rp_up_masked = false;
+ }
+
} else { /* unknown event */
new_delay = dev_priv-rps.cur_delay;
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 469170c..9c950e4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3628,6 +3628,9 @@ static void valleyview_enable_rps(struct drm_device
*dev)
vlv_gpu_freq(dev_priv, dev_priv-rps.rpe_delay),
dev_priv-rps.rpe_delay);
+ dev_priv-rps.rp_up_masked = false;
+ dev_priv-rps.rp_down_masked = false;
+
valleyview_set_rps(dev_priv-dev, dev_priv-rps.rpe_delay);
gen6_enable_rps_interrupts(dev);
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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 v2] drm/i915: Bring UP Power Wells before disabling RC6.
Hi Chris, During the development of runtime power management we faced some random issue around RC6 during the suspend. After some analyzing, we found by following the flow of doing Forcewake Get before the RC6 Disable and forcewake Put after we enable the RC6 solved most of the issues. This patch adds the Forcewake get before disabling the RC6 My commit message was to generic I guess :) I will change the commit message. The flow is between the driver and punit. :) Thanks Deepak -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Thursday, January 9, 2014 7:32 PM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Bring UP Power Wells before disabling RC6. On Thu, Jan 09, 2014 at 07:28:27PM +0530, deepa...@intel.com wrote: From: Deepak S deepa...@intel.com We need do forcewake before Disabling RC6, This is what the BIOS expects while going into suspend. What suspend are we talking about? When does the BIOS run, how is that serialised with the get/put of forcewake? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Disable/Enable PM Intrrupts based on the current freq.
Hi Chris, If I understand correctly we are using GEN6_RP_INTERRUPT_LIMITS to Make sure we continue to get interrupts until we hit the minimum or maximum frequencies for gen6 right? Also, we do setup the gen6_set_rps_thresholds based on the power in gen6_set_rps right? Instead of adding it in set_rps_thresholds, would it be better to add the Disable/Enable PM Intrrupts based on the current freq in valleyview_set_rps? Let me know if my understanding is right. Thanks Deepak -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Tuesday, December 17, 2013 8:46 PM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Disable/Enable PM Intrrupts based on the current freq. On Tue, Dec 17, 2013 at 08:35:40PM +0530, deepa...@intel.com wrote: From: Deepak S deepa...@intel.com When current delay is already at max delay, Let's disable the PM UP THRESHOLD INTRRUPTS, so that we will not get further interrupts until current delay is less than max delay, Also request for the PM DOWN THRESHOLD INTRRUPTS to indicate the decrease in clock freq. and viceversa for PM DOWN THRESHOLD INTRRUPTS. v2: Use bool variables (Daniel) Signed-off-by: Deepak S deepa...@intel.com Nak. Why not do this with the rest of the threshold interrupt adjusting code? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Disable/Enable PM Intrrupts based on the current freq.
Hi Ville,
On VLV, we do get the interrupts when we should not. That is when we already
in max_delay we get the up threshold interrupts
Thanks
Deepak
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Monday, December 9, 2013 3:42 PM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 3/4] drm/i915: Disable/Enable PM Intrrupts
based on the current freq.
On Sun, Dec 08, 2013 at 02:16:45PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
When current delay is already at max delay, Let's disable the PM UP
THRESHOLD INTRRUPTS, so that we will not get further interrupts until
current delay is less than max delay, Also request for the PM DOWN
THRESHOLD INTRRUPTS to indicate the decrease in clock freq. and
viceversa for PM DOWN THRESHOLD INTRRUPTS.
Are we actually getting these interrupts when we shouldn't? On non-VLV I think
GEN6_RP_INTERRUPT_LIMITS should prevent it, but I don't really know about VLV.
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/i915_irq.c | 31 +--
drivers/gpu/drm/i915/intel_pm.c | 3 +++
3 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h index a62ac0c..d52a2db 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -915,6 +915,9 @@ struct intel_gen6_power_mgmt {
u8 rp0_delay;
u8 hw_max;
+ u8 rp_up_masked;
+ u8 rp_down_masked;
+
int last_adj;
enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c index 4bde03a..cd82fdd 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -996,7 +996,20 @@ static void gen6_pm_rps_work(struct work_struct *work)
adj *= 2;
else
adj = 1;
- new_delay = dev_priv-rps.cur_delay + adj;
+
+ if (dev_priv-rps.cur_delay = dev_priv-rps.max_delay) {
+ I915_WRITE(GEN6_PMINTRMSK,
+ I915_READ(GEN6_PMINTRMSK) | 1 5);
+ dev_priv-rps.rp_up_masked = 1;
+ new_delay = dev_priv-rps.cur_delay;
+ } else
+ new_delay = dev_priv-rps.cur_delay + adj;
+
+ if (dev_priv-rps.rp_down_masked) {
+ I915_WRITE(GEN6_PMINTRMSK,
+ I915_READ(GEN6_PMINTRMSK) | ~(1 4));
+ dev_priv-rps.rp_down_masked = 0;
+ }
/*
* For better performance, jump directly @@ -1015,7 +1028,21 @@
static void gen6_pm_rps_work(struct work_struct *work)
adj *= 2;
else
adj = -1;
- new_delay = dev_priv-rps.cur_delay + adj;
+
+ if (dev_priv-rps.cur_delay = dev_priv-rps.max_delay) {
+ I915_WRITE(GEN6_PMINTRMSK,
+ I915_READ(GEN6_PMINTRMSK) | 1 4);
+ dev_priv-rps.rp_down_masked = 1;
+ new_delay = dev_priv-rps.cur_delay;
+ } else
+ new_delay = dev_priv-rps.cur_delay + adj;
+
+ if (dev_priv-rps.rp_up_masked) {
+ I915_WRITE(GEN6_PMINTRMSK,
+ I915_READ(GEN6_PMINTRMSK) | ~(1 5));
+ dev_priv-rps.rp_up_masked = 0;
+ }
+
} else { /* unknown event */
new_delay = dev_priv-rps.cur_delay;
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 716ca24..6b80ec4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4186,6 +4186,9 @@ static void valleyview_enable_rps(struct drm_device
*dev)
vlv_gpu_freq(dev_priv, dev_priv-rps.rpe_delay),
dev_priv-rps.rpe_delay);
+ dev_priv-rps.rp_up_masked = 0;
+ dev_priv-rps.rp_down_masked = 0;
+
valleyview_set_rps(dev_priv-dev, dev_priv-rps.rpe_delay);
gen6_enable_rps_interrupts(dev);
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Bring UP Power Wells before disabling RC6.
We faced some issue for not following the sequence.
I will add proper commit message and send it for review.
-Deepak
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Monday, December 9, 2013 10:49 PM
To: S, Deepak
Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915: Bring UP Power Wells before
disabling RC6.
On Mon, Dec 09, 2013 at 02:14:03PM +, S, Deepak wrote:
What precisely does this fix? All our register access is already wrapped in
get/put calls, so I'm a bit confused ... Is this to work around hw issues,
or is this what the bios expects when going into suspend?
Yes Daniel, this was sequence recommended when going into suspend/Resume path.
So the BIOS falls over if we don't do this? In that case I think we need to
reword the code comment to say that we're doing this for the BIOS.
Otherwise someone will remove this again, since our own code surely doesn't
need it.
-Daniel
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of
Daniel Vetter
Sent: Monday, December 9, 2013 1:31 PM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915: Bring UP Power Wells before
disabling RC6.
On Sun, Dec 08, 2013 at 01:52:46PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
Instead of waiting for HW to bringup the wells, We force the wells
up before disabling RC6. This is to avoid any register access when
wells are down.
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 2e1340f..089712a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3661,6 +3661,12 @@ static void gen6_disable_rps(struct
drm_device
*dev) static void valleyview_disable_rps(struct drm_device *dev) {
struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long irqflags;
+
+ /* We need to bring up the wells before disabling the RC6 */
+ spin_lock_irqsave(dev_priv-uncore.lock, irqflags);
+ dev_priv-uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_ALL);
+ spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
What precisely does this fix? All our register access is already wrapped in
get/put calls, so I'm a bit confused ... Is this to work around hw issues, or
is this what the bios expects when going into suspend?
-Daniel
I915_WRITE(GEN6_RC_CONTROL, 0);
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
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 2/4] drm/i915: set min delay to rpe delay (Efficient frequency) for better performace.
Hi Chris, My understanding is that running at efficient freq (RPe) where RPe is greater than the min freq (RPn) (RPe RPn), at same Vmin voltage should give us better performance right?. Please correct me if my understand is not right Thanks Deepak -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Sunday, December 8, 2013 9:37 PM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 2/4] drm/i915: set min delay to rpe delay (Efficient frequency) for better performace. On Sun, Dec 08, 2013 at 02:16:44PM +0530, deepa...@intel.com wrote: From: Deepak S deepa...@intel.com We use RPe here since it should match the Vmin we were shooting for. That should give us better perf than if we used the min freq available. System thermal can take the system to lowest possible freq (RP1). We are making sure, we calmp the freq to min_delay (RPe). Your perf arguments are fallacious. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Bring UP Power Wells before disabling RC6.
What precisely does this fix? All our register access is already wrapped in
get/put calls, so I'm a bit confused ... Is this to work around hw issues, or
is this what the bios expects when going into suspend?
Yes Daniel, this was sequence recommended when going into suspend/Resume path.
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Monday, December 9, 2013 1:31 PM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915: Bring UP Power Wells before
disabling RC6.
On Sun, Dec 08, 2013 at 01:52:46PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
Instead of waiting for HW to bringup the wells, We force the wells up
before disabling RC6. This is to avoid any register access when wells
are down.
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index 2e1340f..089712a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3661,6 +3661,12 @@ static void gen6_disable_rps(struct drm_device
*dev) static void valleyview_disable_rps(struct drm_device *dev) {
struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long irqflags;
+
+ /* We need to bring up the wells before disabling the RC6 */
+ spin_lock_irqsave(dev_priv-uncore.lock, irqflags);
+ dev_priv-uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_ALL);
+ spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
What precisely does this fix? All our register access is already wrapped in
get/put calls, so I'm a bit confused ... Is this to work around hw issues, or
is this what the bios expects when going into suspend?
-Daniel
I915_WRITE(GEN6_RC_CONTROL, 0);
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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 1/3] drm/i915: Verify address field of PCBR register.
Have you actually seen cases where the BIOS would be buggy enough to lock the
power context addres at 0? If so, we should just scream and run away instead
of blindly trying to write a new address to PCBR and pretending that things
are fine after that.
We had faced some issue doing the initial version of BIOS. I just verified it
and it looks fine and we can ignore this patch.
I think the right fix would be to verify the pcbr lock bit and if it is set but
still pcbr base address is zero then we should disable the rc6.
Thanks
Deepak
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Monday, December 9, 2013 3:05 PM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915: Verify address field of PCBR
register.
On Sun, Dec 08, 2013 at 01:52:35PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
On VLV the PCBR register has other bits besides the pcbr address
field. Verify only address field setup by BIOS to make sure we don't
misinterpret the PCBR setup.
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index e6d98fe..2e1340f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4036,7 +4036,12 @@ static void valleyview_setup_pctx(struct drm_device
*dev)
int pctx_size = 24*1024;
pcbr = I915_READ(VLV_PCBR);
- if (pcbr) {
+
+ /* PCBR Format: Bits 31:12 - Base address of Process Context
It's called power context, not process context.
+ * Bits 11:1 - Reserved
These are all 0
+ * Bit 0 - PCBR Lock
And if this is 1, then we can't change the address anyway, so I don't think
this patch makes much sense.
Have you actually seen cases where the BIOS would be buggy enough to lock the
power context addres at 0? If so, we should just scream and run away instead of
blindly trying to write a new address to PCBR and pretending that things are
fine after that.
+ * Check only address field if already setup by BIOS */
+ if (pcbr 12) {
/* BIOS set it up already, grab the pre-alloc'd space */
int pcbr_offset;
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915: update current freq properly before requesting new freq.
I am trying to get more details. I will update the thread once I have some
clarification. Thanks for reviewing.
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Monday, December 9, 2013 3:34 PM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/4] drm/i915: update current freq properly
before requesting new freq.
On Sun, Dec 08, 2013 at 02:16:43PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
on VLV, P-Unit doesn't garauntee that last requested freq by driver is
actually the current running frequency. We need to make sure we update
the cur freq. before requesitng new freq.
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 8
drivers/gpu/drm/i915/intel_pm.c | 31 +++
3 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h index 780f815..a62ac0c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2416,6 +2416,7 @@ extern bool ironlake_set_drps(struct drm_device
*dev, u8 val); extern void intel_init_pch_refclk(struct drm_device
*dev); extern void gen6_set_rps(struct drm_device *dev, u8 val);
extern void valleyview_set_rps(struct drm_device *dev, u8 val);
+extern bool vlv_update_rps_cur_delay(struct drm_i915_private
+*dev_priv);
extern int valleyview_rps_max_freq(struct drm_i915_private
*dev_priv); extern int valleyview_rps_min_freq(struct
drm_i915_private *dev_priv); extern void intel_detect_pch(struct
drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c index 2715600..4bde03a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -982,6 +982,14 @@ static void gen6_pm_rps_work(struct work_struct
*work)
mutex_lock(dev_priv-rps.hw_lock);
+ /* Make sure we have current freq updated properly. Doing this
+ * here becuase, on VLV, P-Unit doesnt garauntee that last requested
+ * freq by driver is actually the current running frequency
+ */
+
+ if (IS_VALLEYVIEW(dev_priv-dev))
+ vlv_update_rps_cur_delay(dev_priv);
+
adj = dev_priv-rps.last_adj;
if (pm_iir GEN6_PM_RP_UP_THRESHOLD) {
if (adj 0)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index e6d98fe..7f6c747 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3607,6 +3607,35 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv)
mutex_unlock(dev_priv-rps.hw_lock);
}
+/*
+ * Wait until the previous freq change has completed,
+ * or the timeout elapsed, and then update our notion
+ * of the current GPU frequency.
+ */
+bool vlv_update_rps_cur_delay(struct drm_i915_private *dev_priv) {
+ u32 pval;
+
+ WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock));
+
+ if (wait_for(((pval = vlv_punit_read(dev_priv,
+ PUNIT_REG_GPU_FREQ_STS))
+ GENFREQSTATUS) == 0, 10))
+ DRM_DEBUG_DRIVER(timed out waiting for Punit\n);
+
+ pval = 8;
+
+ if (pval != dev_priv-rps.cur_delay)
+ DRM_DEBUG_DRIVER(Punit overrode GPU freq: %d MHz (%u)
requested, but got %d Mhz (%u)\n,
+ vlv_gpu_freq(dev_priv, dev_priv-rps.cur_delay),
+ dev_priv-rps.cur_delay,
+ vlv_gpu_freq(dev_priv, pval), pval);
+
+ dev_priv-rps.cur_delay = pval;
+ return true;
+}
I just killed this guys a while ago. If you think we need to resurrect it, you
should do it w/ git revert to make it clear where it came from.
But I'd want more justification than what you have provided. My understanding
is that PUNIT_REG_GPU_FREQ_STS alwasy reflects the current operating frequency
of the GPU, and that can be affected by thermal conditions (and media turbo,
which I'll ignore for simplicity) in addition to the frequency requested by the
driver. AFAIK the punit will recheck the situation periodically, and it will
try to use PUNIT_REG_GPU_FREQ_REQ. It will check the thermal conditions to
figure out if it needs to further limit the frequency. Once the thermal
conditions permit it, the frequency should return back to the last requested
turbo frequency, without the driver having to rewrite PUNIT_REG_GPU_FREQ_REQ.
If I'm right updating cur_delay based on PUNIT_REG_GPU_FREQ_STS is clearly the
wrong thing to do. So I think we need more details on what the punit does in
order to figure out what's the right thing to do here.
+
+
void valleyview_set_rps(struct drm_device *dev, u8 val) {
struct drm_i915_private *dev_priv = dev-dev_private; @@ -3615,6
+3644,8 @@ void valleyview_set_rps(struct drm_device
Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: Update Wait for FIFO and wait for 20 free entries. v2
Thanks Daniel. -Original Message- From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Wednesday, December 4, 2013 2:10 PM To: S, Deepak Cc: Chris Wilson; intel-gfx Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: Update Wait for FIFO and wait for 20 free entries. v2 [re-adding intel-gfx] On Tue, Dec 3, 2013 at 5:05 PM, S, Deepak deepa...@intel.com wrote: Hi Daniel/Chris, I spent some time in digging through the specs and also has chatted with couple of people. Below is my understanding of the FIFO. On SB, Out of 64 FIFO Entries, 20 Entries will be used by HW and remaining 44 will be used by the SW,. I think due to this reason, we have a threshold of 20 Entries. On VLV, HW and SW can access all 64 fifo entries, I don't think having a threshold of 20 Entries is mandatory on VLV. Also, since both SW and HW can access all 64 Entries. I think on VLV, we need to update the fifo_count before waiting for the FIFO. Please correct me if I am working. Looks sane. I've added this to the commit message and merged your patch. Thanks, Daniel Thanks Deepak -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Friday, November 29, 2013 7:33 PM To: S, Deepak Cc: Chris Wilson; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: Update Wait for FIFO and wait for 20 free entries. v2 On Fri, Nov 29, 2013 at 11:53:44AM +, S, Deepak wrote: Sure Chris, I will recheck the spec and change the commit accordingly. I guess the big question is why vlv is special. We've had these 20 fifo entries ever since gen6, so I'd also really like to know what suddenly changed. Even the 20 entries have just been copied from a spec with no explation. So if this is to allow hw writes to the gt from the display, then I guess we would need this change on all gen6+ platforms? Hence digging through specs or dragging a hw engineer into this discussion would be highly appreciated. Thanks, Daniel -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Friday, November 29, 2013 5:07 PM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: Update Wait for FIFO and wait for 20 free entries. v2 On Fri, Nov 29, 2013 at 11:22:32AM +, S, Deepak wrote: Hi Chris, In VLV, both hardware and software can use the write fifo in parallel, we are adding this change as a water mark to make sure we atleast have 20 free entries .This will help us to avoid software mmio write being dropped. Please think some more and describe the change exactly. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- 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 v2] drm/i915/vlv: Update Wait for FIFO and wait for 20 free entries. v2
Hi Daniel, Sure, I don't have any contact with the hw engineer yet, I will try to get the right contact asap. Thanks Deepak -Original Message- From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Wednesday, December 4, 2013 4:37 PM To: S, Deepak Cc: Chris Wilson; intel-gfx Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: Update Wait for FIFO and wait for 20 free entries. v2 [Dragging the discussion back to the public.] I agree that the 20 fifo limit is fairly arbitrary and it's still racy. Otoh that seems to be a bit that the hw engineers simply fumbled, so I guess we don't have much choice here. I've only merged the patch to dinq since there's no real report of this fixing anything and we have a bit of time to figure out something better. If there is such a thing even. Deepak, can you please poke hw engineers a bit how they've thought the driver should run this exactly? Thanks, Daniel On Wed, Dec 4, 2013 at 10:48 AM, S, Deepak deepa...@intel.com wrote: Agreed. But if we don't have a threshold, we might hit a high probability of dropping writes if HW or SW uses all 64 entries right?. Can we create a wq and check for the fifo entries at a periodic interval, this will reduce the fifo checking before every mmio write. Thank Deepak S -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Wednesday, December 4, 2013 3:04 PM To: S, Deepak Cc: Daniel Vetter Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: Update Wait for FIFO and wait for 20 free entries. v2 On Tue, Dec 03, 2013 at 04:05:07PM +, S, Deepak wrote: Hi Daniel/Chris, I spent some time in digging through the specs and also has chatted with couple of people. Below is my understanding of the FIFO. On SB, Out of 64 FIFO Entries, 20 Entries will be used by HW and remaining 44 will be used by the SW,. I think due to this reason, we have a threshold of 20 Entries. On VLV, HW and SW can access all 64 fifo entries, I don't think having a threshold of 20 Entries is mandatory on VLV. Also, since both SW and HW can access all 64 Entries. I think on VLV, we need to update the fifo_count before waiting for the FIFO. But there is also no point in waiting for at least 20 entries either. So the current code does not make sense for VLV in that light. Also note that the code is currently like it is as the mmio read before every write adds significant CPU overhead. There is also the problem that if the hw can consume the entire FIFO, it can also do so between us checking for a free entry and use performing the mmio. It seems to be broken by design... -Chris -- Chris Wilson, Intel Open Source Technology Centre -- 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 v2] drm/i915/vlv: Update Wait for FIFO and wait for 20 free entries. v2
Hi Chris, In VLV, both hardware and software can use the write fifo in parallel, we are adding this change as a water mark to make sure we atleast have 20 free entries .This will help us to avoid software mmio write being dropped. Thanks Deepak -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Friday, November 29, 2013 4:09 PM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: Update Wait for FIFO and wait for 20 free entries. v2 On Fri, Nov 29, 2013 at 03:44:31PM +0530, deepa...@intel.com wrote: From: Deepak S deepa...@intel.com On VLV, FIFO will be shared by both SW and HW. So, we read the free entries through register and update dev_priv variable and wait for only 20 entries to be free But the whole point of leaving 20 entries is for hardware has a portion of the fifo it can use for its own nefarious deeds. The hw has been emitting mmio through the write fifo since its inception on gen6, so what is so different for vlv? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: Update Wait for FIFO and wait for 20 free entries. v2
Sure Chris, I will recheck the spec and change the commit accordingly. -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Friday, November 29, 2013 5:07 PM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/vlv: Update Wait for FIFO and wait for 20 free entries. v2 On Fri, Nov 29, 2013 at 11:22:32AM +, S, Deepak wrote: Hi Chris, In VLV, both hardware and software can use the write fifo in parallel, we are adding this change as a water mark to make sure we atleast have 20 free entries .This will help us to avoid software mmio write being dropped. Please think some more and describe the change exactly. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/vlv: use parallel context restore when coming out of RC6
Patches looks fine. Reviewed-by: Deepak S deepa...@inel.commailto:jbar...@virtuousgeek.org From: Jesse Barnes jbar...@virtuousgeek.orgmailto:jbar...@virtuousgeek.org Date: Fri, Nov 15, 2013 at 11:02 PM Subject: [Intel-gfx] [PATCH 2/2] drm/i915/vlv: use parallel context restore when coming out of RC6 To: intel-gfx@lists.freedesktop.orgmailto:intel-gfx@lists.freedesktop.org Setting this bit restores all ring contexts in parallel rather than serially. Matches current BWG recommendations. Tested-by: Meng, Mengmeng mengmeng.m...@intel.commailto:mengmeng.m...@intel.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.orgmailto:jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 849e595..40b1136 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4891,6 +4891,7 @@ #define GEN6_RC_CTL_RC6_ENABLE (118) #define GEN6_RC_CTL_RC1e_ENABLE (120) #define GEN6_RC_CTL_RC7_ENABLE (122) +#define VLV_RC_CTL_CTX_RST_PARALLEL (124) #define GEN7_RC_CTL_TO_MODE (128) #define GEN6_RC_CTL_EI_MODE(x) ((x)27) #define GEN6_RC_CTL_HW_ENABLE(131) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5d3912a..6a21d11 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4110,7 +4110,7 @@ static void valleyview_enable_rps(struct drm_device *dev) VLV_MEDIA_RC6_COUNT_EN | VLV_RENDER_RC6_COUNT_EN)); if (intel_enable_rc6(dev) INTEL_RC6_ENABLE) - rc6_mode = GEN7_RC_CTL_TO_MODE; + rc6_mode = GEN7_RC_CTL_TO_MODE | VLV_RC_CTL_CTX_RST_PARALLEL; intel_print_rc6_info(dev, rc6_mode); -- 1.8.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.orgmailto:Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Deepak S ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/vlv: use a lower RC6 timeout on VLV
Patches looks fine. Reviewed-by: Deepak S deepa...@inel.commailto:jbar...@virtuousgeek.org From: Jesse Barnes jbar...@virtuousgeek.orgmailto:jbar...@virtuousgeek.org Date: Fri, Nov 15, 2013 at 11:02 PM Subject: [Intel-gfx] [PATCH 1/2] drm/i915/vlv: use a lower RC6 timeout on VLV To: intel-gfx@lists.freedesktop.orgmailto:intel-gfx@lists.freedesktop.org We use timeout mode, and we need to lower the timeout to get good RC6 residency when loads are running. This gets me from 0% residency during glxgears to 77%, which is a pretty good improvement. This value also matches the current BWG recommentations. Tested-by: Meng, Mengmeng mengmeng.m...@intel.commailto:mengmeng.m...@intel.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.orgmailto:jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 172efa0..5d3912a 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4102,7 +4102,7 @@ static void valleyview_enable_rps(struct drm_device *dev) for_each_ring(ring, dev_priv, i) I915_WRITE(RING_MAX_IDLE(ring-mmio_base), 10); - I915_WRITE(GEN6_RC6_THRESHOLD, 0xc350); + I915_WRITE(GEN6_RC6_THRESHOLD, 0x557); /* allows RC6 residency counter to work */ I915_WRITE(VLV_COUNTER_CONTROL, -- 1.8.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.orgmailto:Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Deepak S ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/vlv: correct promotion timer value for RC6 Timeout method.
Yes I have reviewed both the patches. Thanks Deepak -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Thursday, November 28, 2013 1:03 PM To: S, Deepak Cc: Jesse Barnes; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/vlv: correct promotion timer value for RC6 Timeout method. On Thu, Nov 28, 2013 at 03:35:45AM +, S, Deepak wrote: Hi Jesse, Your patches looks fine to me. I think lets go with your patches and we can abandon mine. Does that count as a full Reviewed-by? If so please reply to the patches with it. Thanks, Daniel Thanks Deepak -Original Message- From: Jesse Barnes [mailto:jbar...@virtuousgeek.org] Sent: Wednesday, November 27, 2013 10:50 PM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/vlv: correct promotion timer value for RC6 Timeout method. On Wed, 27 Nov 2013 21:14:03 +0530 deepa...@intel.com wrote: From: Deepak S deepa...@intel.com For RC6 Timeout method, we need to set promotion timer to 1750 us ( 1367 * 1.28 us) Signed-off-by: Deepak S deepa...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 04e9863..cf3d54d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4102,7 +4102,8 @@ static void valleyview_enable_rps(struct drm_device *dev) for_each_ring(ring, dev_priv, i) I915_WRITE(RING_MAX_IDLE(ring-mmio_base), 10); - I915_WRITE(GEN6_RC6_THRESHOLD, 0xc350); + /* Timer for RC6 Timeout Mode set to 1750 us ( 1367 * 1.28 us) */ + I915_WRITE(GEN6_RC6_THRESHOLD, 0x557); /* allows RC6 residency counter to work */ I915_WRITE(VLV_COUNTER_CONTROL, Yeah I just sent this one too. I'm fine with either one getting in. I also submitted one to do parallel restore of ring state, which we also want. Care to review my earlier ones? http://lists.freedesktop.org/archives/intel-gfx/2013-November/036112.h tml http://lists.freedesktop.org/archives/intel-gfx/2013-November/036111.h tml Thanks, -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 v2 0/3] drm/i915: Split VLV forcewake routines.
Yup, your right it should be version 3, my mistake I missed it while updating. Just verified the patches in nightly branch, All the changes are present. -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Thursday, November 28, 2013 1:06 PM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v2 0/3] drm/i915: Split VLV forcewake routines. On Thu, Nov 28, 2013 at 09:20:40AM +0530, deepa...@intel.com wrote: From: Deepak S deepa...@intel.com Valleyview has power wells MEDIA RENDER and by spliting vlv force wake routines and individually controling Media/Render well, We have seen power savings in the lower sub-1W range on different workloads, e.g. glbenchmark, media playback v2: Addressed review comments. I've alredy pulled it all in and fixed things up while applying ;-) And iirc this is v3 of this series if we count the not-split patch as v1. Please double-check that I didn't miss anything in latest -nightly. -Daniel Deepak S (3): drm/i915: Add power well arguments to force wake routines. drm/i915/vlv: Valleyview support for forcewake Individual power wells. v2 drm/i915: Enabling DebugFS for valleyview forcewake counts. v2 drivers/gpu/drm/i915/i915_debugfs.c | 22 ++-- drivers/gpu/drm/i915/i915_drv.h | 32 - drivers/gpu/drm/i915/i915_reg.h | 4 +- drivers/gpu/drm/i915/intel_display.c| 4 +- drivers/gpu/drm/i915/intel_pm.c | 22 ++-- drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ++- drivers/gpu/drm/i915/intel_uncore.c | 217 +--- 7 files changed, 246 insertions(+), 69 deletions(-) -- 1.8.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 3/3] drm/i915: Enabling DebugFS for valleyview forcewake counts
Thanks Daniel.
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Wednesday, November 27, 2013 9:30 PM
To: S, Deepak
Cc: Jesse Barnes; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: Enabling DebugFS for valleyview
forcewake counts
On Tue, Nov 26, 2013 at 10:12:48AM +, S, Deepak wrote:
Yes Jesse, this is a warning fix.
Yeah, those should be separate or at least mentioned in the commit message. But
I have this one already so it naturally dropped out. All three patches merged
to dinq, thanks.
-Daniel
-Original Message-
From: Jesse Barnes [mailto:jbar...@virtuousgeek.org]
Sent: Monday, November 25, 2013 10:09 PM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: Enabling DebugFS for
valleyview forcewake counts
On Sat, 23 Nov 2013 14:55:44 +0530
deepa...@intel.com wrote:
@@ -2349,7 +2357,7 @@ static int pipe_crc_set_source(struct
drm_device *dev, enum pipe pipe, {
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[pipe];
- u32 val;
+ u32 val = 0;
int ret;
if (pipe_crc-source == source)
Spurious warning fix? Otherwise looks fine.
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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 1/2] drm/i915/vlv: correct promotion timer value for RC6 Timeout method.
Hi Jesse, Your patches looks fine to me. I think lets go with your patches and we can abandon mine. Thanks Deepak -Original Message- From: Jesse Barnes [mailto:jbar...@virtuousgeek.org] Sent: Wednesday, November 27, 2013 10:50 PM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/vlv: correct promotion timer value for RC6 Timeout method. On Wed, 27 Nov 2013 21:14:03 +0530 deepa...@intel.com wrote: From: Deepak S deepa...@intel.com For RC6 Timeout method, we need to set promotion timer to 1750 us ( 1367 * 1.28 us) Signed-off-by: Deepak S deepa...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 04e9863..cf3d54d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4102,7 +4102,8 @@ static void valleyview_enable_rps(struct drm_device *dev) for_each_ring(ring, dev_priv, i) I915_WRITE(RING_MAX_IDLE(ring-mmio_base), 10); - I915_WRITE(GEN6_RC6_THRESHOLD, 0xc350); + /* Timer for RC6 Timeout Mode set to 1750 us ( 1367 * 1.28 us) */ + I915_WRITE(GEN6_RC6_THRESHOLD, 0x557); /* allows RC6 residency counter to work */ I915_WRITE(VLV_COUNTER_CONTROL, Yeah I just sent this one too. I'm fine with either one getting in. I also submitted one to do parallel restore of ring state, which we also want. Care to review my earlier ones? http://lists.freedesktop.org/archives/intel-gfx/2013-November/036112.html http://lists.freedesktop.org/archives/intel-gfx/2013-November/036111.html Thanks, -- Jesse Barnes, 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 2/3] drm/i915/vlv: Valleyview support for forcewake Individual power wells.
Hi Chris, We are using single write fifo for the multiple power wells. Since Valleyview as only supports only one write fifo. Thanks Deepak -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, November 25, 2013 8:18 PM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915/vlv: Valleyview support for forcewake Individual power wells. On Sat, Nov 23, 2013 at 02:55:43PM +0530, deepa...@intel.com wrote: From: Deepak S deepa...@intel.com Split vlv force wake routines to help individually control Media/Render well based on the register access. Do you mind clarifying if there if a single write fifo for the multiple power wells? Just something that worried me reading through the code. Otherwise, lgtm. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Enabling DebugFS for valleyview forcewake counts
Yes Jesse, this is a warning fix.
-Original Message-
From: Jesse Barnes [mailto:jbar...@virtuousgeek.org]
Sent: Monday, November 25, 2013 10:09 PM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: Enabling DebugFS for valleyview
forcewake counts
On Sat, 23 Nov 2013 14:55:44 +0530
deepa...@intel.com wrote:
@@ -2349,7 +2357,7 @@ static int pipe_crc_set_source(struct drm_device
*dev, enum pipe pipe, {
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[pipe];
- u32 val;
+ u32 val = 0;
int ret;
if (pipe_crc-source == source)
Spurious warning fix? Otherwise looks fine.
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
--
Jesse Barnes, 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] drm/i915/vlv: Add VLV specific force wake routines.
Thanks Jesse. We will re-run and compare the power numbers. -Original Message- From: Jesse Barnes [mailto:jbar...@virtuousgeek.org] Sent: Saturday, November 23, 2013 2:43 AM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: Add VLV specific force wake routines. Also, you might re-run your power numbers with Chris's patch to drop the force wake ref around the irq get/put. That's the only one we normally take at runtime, so getting rid of it should give you even greater power savings in conjunction with the RC6 timeout update patch I already pushed. Thanks, Jesse On Wed, 20 Nov 2013 16:33:22 + S, Deepak deepa...@intel.com wrote: Thanks Jesse, I wil split the patches and send it for review. Thanks Deepak -Original Message- From: Jesse Barnes [mailto:jbar...@virtuousgeek.org] Sent: Wednesday, November 20, 2013 9:35 PM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: Add VLV specific force wake routines. On Wed, 20 Nov 2013 06:00:24 + S, Deepak deepa...@intel.com wrote: Hi Jesse, Thanks for the review. Below is my response. - why not use the callback to __vlv_force_wake_* from gen6_gt_force_wake_*? i.e. why is VLV special here? [Deepak] Gen6 has a single power well whereas the VLV is has spate wells. This was the reason for the separate function - having a new gen7_media_force_wake function may be better than passing an engine around, and would touch fewer pieces of code [Deepak] Even Gen7 is also as single Power Well. Having common function between gen7 and vlv might be difficult to individually handle the wells. - have you done measurements on this? given how infrequently we ought to be waking the wells when they're idle, and how long we generally keep them awake, is this a real power win? [Deepak] By Individually controlling the wells we observed around 100mW - 200mW saving in different scenarios (GL Beanchmark Media playback). wow, that's a significant savings. Can you split the patch into one that adds the power well arg, and another that adds the VLV support for the split? That would make it easier to review. Thanks, -- Jesse Barnes, Intel Open Source Technology Center -- Jesse Barnes, 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] drm/i915/vlv: Add VLV specific force wake routines.
Thanks Jesse, I wil split the patches and send it for review. Thanks Deepak -Original Message- From: Jesse Barnes [mailto:jbar...@virtuousgeek.org] Sent: Wednesday, November 20, 2013 9:35 PM To: S, Deepak Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: Add VLV specific force wake routines. On Wed, 20 Nov 2013 06:00:24 + S, Deepak deepa...@intel.com wrote: Hi Jesse, Thanks for the review. Below is my response. - why not use the callback to __vlv_force_wake_* from gen6_gt_force_wake_*? i.e. why is VLV special here? [Deepak] Gen6 has a single power well whereas the VLV is has spate wells. This was the reason for the separate function - having a new gen7_media_force_wake function may be better than passing an engine around, and would touch fewer pieces of code [Deepak] Even Gen7 is also as single Power Well. Having common function between gen7 and vlv might be difficult to individually handle the wells. - have you done measurements on this? given how infrequently we ought to be waking the wells when they're idle, and how long we generally keep them awake, is this a real power win? [Deepak] By Individually controlling the wells we observed around 100mW - 200mW saving in different scenarios (GL Beanchmark Media playback). wow, that's a significant savings. Can you split the patch into one that adds the power well arg, and another that adds the VLV support for the split? That would make it easier to review. Thanks, -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
