* [PATCH 1/4] drm/i915: Set AGPBUSY# bit in init_clock_gating
2014-02-25 13:13 [PATCH 0/4] drm/i915: Gen2/3 C3 wakeup stuff ville.syrjala
@ 2014-02-25 13:13 ` ville.syrjala
2014-02-25 13:13 ` [PATCH v2 2/4] drm/i915: Flip the sense of AGPBUSY_DIS bit ville.syrjala
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: ville.syrjala @ 2014-02-25 13:13 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
I don't see why we wouldn't want interrupts to wake up the CPU from C3
always, so just set the AGPBUSY# bit in gen3_init_clock_gating().
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 7 -------
drivers/gpu/drm/i915/intel_pm.c | 3 +++
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f68aee3..910eb63 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2377,10 +2377,6 @@ static int i915_enable_vblank(struct drm_device *dev, int pipe)
else
i915_enable_pipestat(dev_priv, pipe,
PIPE_VBLANK_INTERRUPT_STATUS);
-
- /* maintain vblank delivery even in deep C-states */
- if (INTEL_INFO(dev)->gen == 3)
- I915_WRITE(INSTPM, _MASKED_BIT_DISABLE(INSTPM_AGPBUSY_DIS));
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
return 0;
@@ -2444,9 +2440,6 @@ static void i915_disable_vblank(struct drm_device *dev, int pipe)
unsigned long irqflags;
spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
- if (INTEL_INFO(dev)->gen == 3)
- I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_AGPBUSY_DIS));
-
i915_disable_pipestat(dev_priv, pipe,
PIPE_VBLANK_INTERRUPT_STATUS |
PIPE_START_VBLANK_INTERRUPT_STATUS);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a6b877a..25cd9dc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5120,6 +5120,9 @@ static void gen3_init_clock_gating(struct drm_device *dev)
/* IIR "flip pending" means done if this bit is set */
I915_WRITE(ECOSKPD, _MASKED_BIT_DISABLE(ECO_FLIP_DONE));
+
+ /* interrupts should cause a wake up from C3 */
+ I915_WRITE(INSTPM, _MASKED_BIT_DISABLE(INSTPM_AGPBUSY_DIS));
}
static void i85x_init_clock_gating(struct drm_device *dev)
--
1.8.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 2/4] drm/i915: Flip the sense of AGPBUSY_DIS bit
2014-02-25 13:13 [PATCH 0/4] drm/i915: Gen2/3 C3 wakeup stuff ville.syrjala
2014-02-25 13:13 ` [PATCH 1/4] drm/i915: Set AGPBUSY# bit in init_clock_gating ville.syrjala
@ 2014-02-25 13:13 ` ville.syrjala
2014-02-25 13:13 ` [PATCH 3/4] drm/i915: Enable interrupt-based AGPBUSY# enable on 85x ville.syrjala
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: ville.syrjala @ 2014-02-25 13:13 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
My Gen3 Bspec lists the AGPBUSY# bit in INSTPM as an enable bit rather
than a disable bit. Our code has the opposite idea. Make the code match
the spec.
Might fix some gen3 C3 related interrupt delivery problems. Untested
due to lack of hardware.
v2: call it AGPBUSY_INT_EN to make it clearer it has to do with interrupts
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 2 +-
drivers/gpu/drm/i915/intel_pm.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2f564ce..e6bff6c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -838,7 +838,7 @@
#define I915_ERROR_INSTRUCTION (1<<0)
#define INSTPM 0x020c0
#define INSTPM_SELF_EN (1<<12) /* 915GM only */
-#define INSTPM_AGPBUSY_DIS (1<<11) /* gen3: when disabled, pending interrupts
+#define INSTPM_AGPBUSY_INT_EN (1<<11) /* gen3: when disabled, pending interrupts
will not assert AGPBUSY# and will only
be delivered when out of C3. */
#define INSTPM_FORCE_ORDERING (1<<7) /* GEN6+ */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 25cd9dc..e8b748d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5122,7 +5122,7 @@ static void gen3_init_clock_gating(struct drm_device *dev)
I915_WRITE(ECOSKPD, _MASKED_BIT_DISABLE(ECO_FLIP_DONE));
/* interrupts should cause a wake up from C3 */
- I915_WRITE(INSTPM, _MASKED_BIT_DISABLE(INSTPM_AGPBUSY_DIS));
+ I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_AGPBUSY_INT_EN));
}
static void i85x_init_clock_gating(struct drm_device *dev)
--
1.8.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/4] drm/i915: Enable interrupt-based AGPBUSY# enable on 85x
2014-02-25 13:13 [PATCH 0/4] drm/i915: Gen2/3 C3 wakeup stuff ville.syrjala
2014-02-25 13:13 ` [PATCH 1/4] drm/i915: Set AGPBUSY# bit in init_clock_gating ville.syrjala
2014-02-25 13:13 ` [PATCH v2 2/4] drm/i915: Flip the sense of AGPBUSY_DIS bit ville.syrjala
@ 2014-02-25 13:13 ` ville.syrjala
2014-02-25 13:13 ` [PATCH 4/4] drm/i915: Move the C3 LP write bit setup to gen3_init_clock_gating() for KMS ville.syrjala
2014-05-28 8:19 ` [PATCH 0/4] drm/i915: Gen2/3 C3 wakeup stuff Chris Wilson
4 siblings, 0 replies; 9+ messages in thread
From: ville.syrjala @ 2014-02-25 13:13 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
85x also has a similar AGPBUSY# bit as gen3. Enable it to make
sure vblank interrupts don't get dealyed during C3 state.
There's also another bit which controls whether AGPBUSY# is asserted
based on pending cacheable cycles and interrupts, or just based on
pending commands in the ring and interrupts. Select the cacheable
cycles mode since that seems to be the new way of doing things in
85x, and it does give slightly better C3 residency numbers with
glxgears running.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 4 ++++
drivers/gpu/drm/i915/intel_pm.c | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e6bff6c..cd8e2d5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -919,6 +919,10 @@
#define MI_ARB_DISPLAY_PRIORITY_A_B (0 << 0) /* display A > display B */
#define MI_ARB_DISPLAY_PRIORITY_B_A (1 << 0) /* display B > display A */
+#define MI_STATE 0x020e4 /* gen2 only */
+#define MI_AGPBUSY_INT_EN (1 << 1) /* 85x only */
+#define MI_AGPBUSY_830_MODE (1 << 0) /* 85x only */
+
#define CACHE_MODE_0 0x02120 /* 915+ only */
#define CM0_PIPELINED_RENDER_FLUSH_DISABLE (1<<8)
#define CM0_IZ_OPT_DISABLE (1<<6)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e8b748d..b6ab781 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5130,6 +5130,10 @@ static void i85x_init_clock_gating(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
I915_WRITE(RENCLK_GATE_D1, SV_CLOCK_GATE_DISABLE);
+
+ /* interrupts should cause a wake up from C3 */
+ I915_WRITE(MI_STATE, _MASKED_BIT_ENABLE(MI_AGPBUSY_INT_EN) |
+ _MASKED_BIT_DISABLE(MI_AGPBUSY_830_MODE));
}
static void i830_init_clock_gating(struct drm_device *dev)
--
1.8.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/4] drm/i915: Move the C3 LP write bit setup to gen3_init_clock_gating() for KMS
2014-02-25 13:13 [PATCH 0/4] drm/i915: Gen2/3 C3 wakeup stuff ville.syrjala
` (2 preceding siblings ...)
2014-02-25 13:13 ` [PATCH 3/4] drm/i915: Enable interrupt-based AGPBUSY# enable on 85x ville.syrjala
@ 2014-02-25 13:13 ` ville.syrjala
2014-05-28 8:19 ` [PATCH 0/4] drm/i915: Gen2/3 C3 wakeup stuff Chris Wilson
4 siblings, 0 replies; 9+ messages in thread
From: ville.syrjala @ 2014-02-25 13:13 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Move the MI_ARB_STATE MI_ARB_C3_LP_WRITE_ENABLE setup to
gen3_init_clock_gating() from i915_gem_load() when KMS is enabled. Leave
it in i915_gem_load() for the UMS case, but add an explcit check, just
to make it easier to spot it when we eventually rip out UMS support.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/intel_pm.c | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3618bb0..0b1a360 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4598,7 +4598,7 @@ i915_gem_load(struct drm_device *dev)
init_waitqueue_head(&dev_priv->gpu_error.reset_queue);
/* On GEN3 we really need to make sure the ARB C3 LP bit is set */
- if (IS_GEN3(dev)) {
+ if (!drm_core_check_feature(dev, DRIVER_MODESET) && IS_GEN3(dev)) {
I915_WRITE(MI_ARB_STATE,
_MASKED_BIT_ENABLE(MI_ARB_C3_LP_WRITE_ENABLE));
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b6ab781..a01dfff 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5123,6 +5123,9 @@ static void gen3_init_clock_gating(struct drm_device *dev)
/* interrupts should cause a wake up from C3 */
I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_AGPBUSY_INT_EN));
+
+ /* On GEN3 we really need to make sure the ARB C3 LP bit is set */
+ I915_WRITE(MI_ARB_STATE, _MASKED_BIT_ENABLE(MI_ARB_C3_LP_WRITE_ENABLE));
}
static void i85x_init_clock_gating(struct drm_device *dev)
--
1.8.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 0/4] drm/i915: Gen2/3 C3 wakeup stuff
2014-02-25 13:13 [PATCH 0/4] drm/i915: Gen2/3 C3 wakeup stuff ville.syrjala
` (3 preceding siblings ...)
2014-02-25 13:13 ` [PATCH 4/4] drm/i915: Move the C3 LP write bit setup to gen3_init_clock_gating() for KMS ville.syrjala
@ 2014-05-28 8:19 ` Chris Wilson
2014-05-28 9:46 ` Ville Syrjälä
2014-05-28 12:51 ` Daniel Vetter
4 siblings, 2 replies; 9+ messages in thread
From: Chris Wilson @ 2014-05-28 8:19 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Tue, Feb 25, 2014 at 03:13:37PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> I tried to fix the C3 vs. vblank interrupt issues reportd in [1], but
> it looks like the AGPBUSY# stuff doesn't help there for some reason. I
> guess either the board isn't wired correctly, or we're missing
> something else. I doubt the BM wakup mechanism itself would be
> broken since then I would expect the machine to lock up when someone
> does DMA while in C3. IIRC I actually had that kind of an issue on
> some old VIA chipset long ago.
>
> Anyways, my 855gm actually supports C3, and on that machine the MI_STATE
> AGPBUSY# stuff is effective. So I'm going to assume that gen3 behaviour
> should match, and so I'm just sticking it all into .init_clock_gating()
> for both gen2 and gen3.
>
> I also found another gen3 C3 bit in i915_gem_load(). I think it would
> be better to collect that into .init_clock_gating() as well. But I left
> it also in i915_gem_load() for UMS.
>
> [1] https://bugs.freedesktop.org/show_bug.cgi?id=30364
>
> Ville Syrjälä (4):
> drm/i915: Set AGPBUSY# bit in init_clock_gating
> drm/i915: Flip the sense of AGPBUSY_DIS bit
> drm/i915: Enable interrupt-based AGPBUSY# enable on 85x
> drm/i915: Move the C3 LP write bit setup to gen3_init_clock_gating()
> for KMS
They all look sensible, seem to better match the docs than the
existing code and make the code easier to read (apart from the
UMS frobbing!), so:
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
The slow vblank delivery issue remains iirc though.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 0/4] drm/i915: Gen2/3 C3 wakeup stuff
2014-05-28 8:19 ` [PATCH 0/4] drm/i915: Gen2/3 C3 wakeup stuff Chris Wilson
@ 2014-05-28 9:46 ` Ville Syrjälä
2014-05-28 9:59 ` Chris Wilson
2014-05-28 12:51 ` Daniel Vetter
1 sibling, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2014-05-28 9:46 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Wed, May 28, 2014 at 09:19:05AM +0100, Chris Wilson wrote:
> On Tue, Feb 25, 2014 at 03:13:37PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > I tried to fix the C3 vs. vblank interrupt issues reportd in [1], but
> > it looks like the AGPBUSY# stuff doesn't help there for some reason. I
> > guess either the board isn't wired correctly, or we're missing
> > something else. I doubt the BM wakup mechanism itself would be
> > broken since then I would expect the machine to lock up when someone
> > does DMA while in C3. IIRC I actually had that kind of an issue on
> > some old VIA chipset long ago.
> >
> > Anyways, my 855gm actually supports C3, and on that machine the MI_STATE
> > AGPBUSY# stuff is effective. So I'm going to assume that gen3 behaviour
> > should match, and so I'm just sticking it all into .init_clock_gating()
> > for both gen2 and gen3.
> >
> > I also found another gen3 C3 bit in i915_gem_load(). I think it would
> > be better to collect that into .init_clock_gating() as well. But I left
> > it also in i915_gem_load() for UMS.
> >
> > [1] https://bugs.freedesktop.org/show_bug.cgi?id=30364
> >
> > Ville Syrjälä (4):
> > drm/i915: Set AGPBUSY# bit in init_clock_gating
> > drm/i915: Flip the sense of AGPBUSY_DIS bit
> > drm/i915: Enable interrupt-based AGPBUSY# enable on 85x
> > drm/i915: Move the C3 LP write bit setup to gen3_init_clock_gating()
> > for KMS
>
> They all look sensible, seem to better match the docs than the
> existing code and make the code easier to read (apart from the
> UMS frobbing!), so:
The UMS frobbing is optional. I just figured that when we eventually
rip out UMS it would be easier to spot that register write since
gem_load() is a rather weird place for such things.
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> The slow vblank delivery issue remains iirc though.
Yeah supposedly that's the case on gen3. This series does fix my 855
though. Unfortunately I've not come across any gen3 machines that
suffe from this. I tried it on Mika's gen3 which IIRC did support C3,
but vblank interrupts got delivered in a timely fashion regardless of
the AGPBUSY bit.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/4] drm/i915: Gen2/3 C3 wakeup stuff
2014-05-28 9:46 ` Ville Syrjälä
@ 2014-05-28 9:59 ` Chris Wilson
0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2014-05-28 9:59 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Wed, May 28, 2014 at 12:46:12PM +0300, Ville Syrjälä wrote:
> On Wed, May 28, 2014 at 09:19:05AM +0100, Chris Wilson wrote:
> > The slow vblank delivery issue remains iirc though.
>
> Yeah supposedly that's the case on gen3. This series does fix my 855
> though. Unfortunately I've not come across any gen3 machines that
> suffe from this. I tried it on Mika's gen3 which IIRC did support C3,
> but vblank interrupts got delivered in a timely fashion regardless of
> the AGPBUSY bit.
I do have an affected 915gm + pentium-IIIm (iirc), I think I tested the
patches at the time (to no avail), and will certainly do so once these
are upstream. Just takes a while as the CPU isn't even the bottleneck on
that machine!
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/4] drm/i915: Gen2/3 C3 wakeup stuff
2014-05-28 8:19 ` [PATCH 0/4] drm/i915: Gen2/3 C3 wakeup stuff Chris Wilson
2014-05-28 9:46 ` Ville Syrjälä
@ 2014-05-28 12:51 ` Daniel Vetter
1 sibling, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2014-05-28 12:51 UTC (permalink / raw)
To: Chris Wilson, ville.syrjala, intel-gfx
On Wed, May 28, 2014 at 09:19:05AM +0100, Chris Wilson wrote:
> On Tue, Feb 25, 2014 at 03:13:37PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > I tried to fix the C3 vs. vblank interrupt issues reportd in [1], but
> > it looks like the AGPBUSY# stuff doesn't help there for some reason. I
> > guess either the board isn't wired correctly, or we're missing
> > something else. I doubt the BM wakup mechanism itself would be
> > broken since then I would expect the machine to lock up when someone
> > does DMA while in C3. IIRC I actually had that kind of an issue on
> > some old VIA chipset long ago.
> >
> > Anyways, my 855gm actually supports C3, and on that machine the MI_STATE
> > AGPBUSY# stuff is effective. So I'm going to assume that gen3 behaviour
> > should match, and so I'm just sticking it all into .init_clock_gating()
> > for both gen2 and gen3.
> >
> > I also found another gen3 C3 bit in i915_gem_load(). I think it would
> > be better to collect that into .init_clock_gating() as well. But I left
> > it also in i915_gem_load() for UMS.
> >
> > [1] https://bugs.freedesktop.org/show_bug.cgi?id=30364
> >
> > Ville Syrjälä (4):
> > drm/i915: Set AGPBUSY# bit in init_clock_gating
> > drm/i915: Flip the sense of AGPBUSY_DIS bit
> > drm/i915: Enable interrupt-based AGPBUSY# enable on 85x
> > drm/i915: Move the C3 LP write bit setup to gen3_init_clock_gating()
> > for KMS
>
> They all look sensible, seem to better match the docs than the
> existing code and make the code easier to read (apart from the
> UMS frobbing!), so:
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Thanks, all merged to dinq.
> The slow vblank delivery issue remains iirc though.
I also wonder whether this could explain the pipe crc issues QA is seeing
on pnv. I'll ping the bug.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 9+ messages in thread