Re: [Intel-gfx] [PATCH 1/2] drm/i915: remove unexplained vblank wait in the DP off code
On Fri, Apr 11, 2014 at 02:25:41PM -0700, Jesse Barnes wrote: I don't think this is necessary; at least it doesn't appear to be on my BYT. Dropping it speeds up our shutdown code a little, in some cases resulting in faster init times. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org Also not required on my IVB. -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/bdw: cs-stall before state cache invld w/a
On Fri, Apr 11, 2014 at 06:46:08PM -0700, Ben Widawsky wrote:
We do this already for previous GENs. I guess we must do it for BDW too
according to DOCS.
Pipe_control with CS-stall bit set must be issued before a
pipe-control command that has the State Cache Invalidate bit set.
This does not solve the problem I have unfortunately.
I didn't check if this was in Ville's CHV series. If it was, I
apologize.
No, but it was posted by Ken some time ago already. I picked up Ken's
patches and was going to repost them as part of my next workarounds
series.
NOTE: I tried to use smaller lengths for the command, but nothing made
it happy except 6.
Cc: Kenneth Graunke kenn...@whitecape.org
Cc: Jordan Justen jljus...@gmail.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a9b04d1..092dea0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -266,17 +266,25 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring,
static int
gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
{
- int ret;
+ int ret, size = 4;
- ret = intel_ring_begin(ring, 4);
+ if (IS_BROADWELL(ring-dev))
+ size += 2;
+
+ ret = intel_ring_begin(ring, size);
if (ret)
return ret;
- intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4));
+ intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(size));
intel_ring_emit(ring, PIPE_CONTROL_CS_STALL |
PIPE_CONTROL_STALL_AT_SCOREBOARD);
intel_ring_emit(ring, 0);
intel_ring_emit(ring, 0);
+ if (IS_BROADWELL(ring-dev)) {
+ intel_ring_emit(ring, 0);
+ intel_ring_emit(ring, 0);
+ }
+
intel_ring_advance(ring);
return 0;
@@ -389,6 +397,11 @@ gen8_render_ring_flush(struct intel_ring_buffer *ring,
flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE;
flags |= PIPE_CONTROL_QW_WRITE;
flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
+
+ /* Workaround: we must issue a pipe_control with CS-stall bit
+ * set before a pipe_control command that has the state cache
+ * invalidate bit set. */
+ gen7_render_ring_cs_stall_wa(ring);
}
ret = intel_ring_begin(ring, 6);
--
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
[Intel-gfx] [PATCH] drm/i915: Always apply cursor width changes
It is possible for userspace to create a big object large enough for a
256x256, and then switch over to using it as a 64x64 cursor. This
requires the cursor update routines to check for a change in width on
every update, rather than just when the cursor is originally enabled.
This also fixes an issue with 845g/865g which cannot change the base
address of the cursor whilst it is active.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 106 +--
drivers/gpu/drm/i915/intel_drv.h | 3 +-
3 files changed, 53 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 4c5de60e5757..7799b1394741 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2450,7 +2450,7 @@ static int i915_display_info(struct seq_file *m, void
*unused)
active = cursor_position(dev, crtc-pipe, x, y);
seq_printf(m, \tcursor visible? %s, position (%d, %d),
addr 0x%08x, active? %s\n,
- yesno(crtc-cursor_visible),
+ yesno(crtc-cursor_base),
x, y, crtc-cursor_addr,
yesno(active));
}
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5e9cb9383f05..92d152694464 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7564,29 +7564,33 @@ static void i845_update_cursor(struct drm_crtc *crtc,
u32 base)
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- bool visible = base != 0;
- u32 cntl;
+ uint32_t cntl;
- if (intel_crtc-cursor_visible == visible)
- return;
-
- cntl = I915_READ(_CURACNTR);
- if (visible) {
+ if (base != intel_crtc-cursor_base) {
/* On these chipsets we can only modify the base whilst
* the cursor is disabled.
*/
+ if (intel_crtc-cursor_cntl) {
+ I915_WRITE(_CURACNTR, 0);
+ POSTING_READ(_CURACNTR);
+ intel_crtc-cursor_cntl = 0;
+ }
+
I915_WRITE(_CURABASE, base);
+ POSTING_READ(_CURABASE);
+ }
- cntl = ~(CURSOR_FORMAT_MASK);
- /* XXX width must be 64, stride 256 = 0x00 28 */
- cntl |= CURSOR_ENABLE |
+ /* XXX width must be 64, stride 256 = 0x00 28 */
+ cntl = 0;
+ if (base)
+ cntl = (CURSOR_ENABLE |
CURSOR_GAMMA_ENABLE |
- CURSOR_FORMAT_ARGB;
- } else
- cntl = ~(CURSOR_ENABLE | CURSOR_GAMMA_ENABLE);
- I915_WRITE(_CURACNTR, cntl);
-
- intel_crtc-cursor_visible = visible;
+ CURSOR_FORMAT_ARGB);
+ if (intel_crtc-cursor_cntl != cntl) {
+ I915_WRITE(_CURACNTR, cntl);
+ POSTING_READ(_CURACNTR);
+ intel_crtc-cursor_cntl = cntl;
+ }
}
static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
@@ -7595,16 +7599,12 @@ static void i9xx_update_cursor(struct drm_crtc *crtc,
u32 base)
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_crtc-pipe;
- bool visible = base != 0;
-
- if (intel_crtc-cursor_visible != visible) {
- int16_t width = intel_crtc-cursor_width;
- uint32_t cntl = I915_READ(CURCNTR(pipe));
- if (base) {
- cntl = ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
- cntl |= MCURSOR_GAMMA_ENABLE;
+ uint32_t cntl;
- switch (width) {
+ cntl = 0;
+ if (base) {
+ cntl = MCURSOR_GAMMA_ENABLE;
+ switch (intel_crtc-cursor_width) {
case 64:
cntl |= CURSOR_MODE_64_ARGB_AX;
break;
@@ -7617,18 +7617,16 @@ static void i9xx_update_cursor(struct drm_crtc *crtc,
u32 base)
default:
WARN_ON(1);
return;
- }
- cntl |= pipe 28; /* Connect to correct pipe */
- } else {
- cntl = ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
- cntl |= CURSOR_MODE_DISABLE;
}
+ cntl |= pipe 28; /* Connect to correct pipe */
+ }
+ if (intel_crtc-cursor_cntl != cntl) {
I915_WRITE(CURCNTR(pipe),
[Intel-gfx] [PATCH] drm/i915: Don't WARN about unexpected hpd interrupts on gmch platforms
The status bits are unconditionally set, the control bits only enable
the actual interrupt generation. Which means if we get some random
other interrupts we'll bogusly complain about them.
So restrict the WARN to platforms with a sane hotplug interrupt
handling scheme.
This WARN has been introduced in
commit b8f102e8bf71cacf33326360fdf9dcfd1a63925b
Author: Egbert Eich e...@suse.de
Date: Fri Jul 26 14:14:24 2013 +0200
drm/i915: Add messages useful for HPD storm detection debugging (v2)
Cc: Egbert Eich e...@suse.de
Cc: bitlord bitlord0...@gmail.com
Reported-by: bitlord bitlord0...@gmail.com
Cc: sta...@vger.kernel.org
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_irq.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7753249b3a95..f98ba4e6e70b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1362,10 +1362,20 @@ static inline void intel_hpd_irq_handler(struct
drm_device *dev,
spin_lock(dev_priv-irq_lock);
for (i = 1; i HPD_NUM_PINS; i++) {
- WARN_ONCE(hpd[i] hotplug_trigger
- dev_priv-hpd_stats[i].hpd_mark == HPD_DISABLED,
- Received HPD interrupt (0x%08x) on pin %d (0x%08x)
although disabled\n,
- hotplug_trigger, i, hpd[i]);
+ if (hpd[i] hotplug_trigger
+ dev_priv-hpd_stats[i].hpd_mark == HPD_DISABLED) {
+ /*
+* On GMCH platforms the interrupt mask bits only
+* prevent irq generation, not the setting of the
+* hotplug bits itself. So only WARN about unexpected
+* interrupts on saner platforms.
+*/
+ WARN_ONCE(INTEL_INFO(dev)-gen = 5
!IS_VALLEYVIEW(dev),
+ Received HPD interrupt (0x%08x) on pin %d
(0x%08x) although disabled\n,
+ hotplug_trigger, i, hpd[i]);
+
+ continue;
+ }
if (!(hpd[i] hotplug_trigger) ||
dev_priv-hpd_stats[i].hpd_mark != HPD_ENABLED)
--
1.8.5.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Always apply cursor width changes
On Sat, Apr 12, 2014 at 04:20:49PM +0100, Chris Wilson wrote:
It is possible for userspace to create a big object large enough for a
256x256, and then switch over to using it as a 64x64 cursor. This
requires the cursor update routines to check for a change in width on
every update, rather than just when the cursor is originally enabled.
This also fixes an issue with 845g/865g which cannot change the base
address of the cursor whilst it is active.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Hm, can we have a testcase for this please? I'm thinking of a black cursor
on black fb at full size, then using the same cursor at smaller size and
filling out the oversize with white. If the CRCs don't match we know that
we scan out a bit of white.
-Daniel
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 106
+--
drivers/gpu/drm/i915/intel_drv.h | 3 +-
3 files changed, 53 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 4c5de60e5757..7799b1394741 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2450,7 +2450,7 @@ static int i915_display_info(struct seq_file *m, void
*unused)
active = cursor_position(dev, crtc-pipe, x, y);
seq_printf(m, \tcursor visible? %s, position (%d, %d),
addr 0x%08x, active? %s\n,
-yesno(crtc-cursor_visible),
+yesno(crtc-cursor_base),
x, y, crtc-cursor_addr,
yesno(active));
}
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5e9cb9383f05..92d152694464 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7564,29 +7564,33 @@ static void i845_update_cursor(struct drm_crtc *crtc,
u32 base)
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- bool visible = base != 0;
- u32 cntl;
+ uint32_t cntl;
- if (intel_crtc-cursor_visible == visible)
- return;
-
- cntl = I915_READ(_CURACNTR);
- if (visible) {
+ if (base != intel_crtc-cursor_base) {
/* On these chipsets we can only modify the base whilst
* the cursor is disabled.
*/
+ if (intel_crtc-cursor_cntl) {
+ I915_WRITE(_CURACNTR, 0);
+ POSTING_READ(_CURACNTR);
+ intel_crtc-cursor_cntl = 0;
+ }
+
I915_WRITE(_CURABASE, base);
+ POSTING_READ(_CURABASE);
+ }
- cntl = ~(CURSOR_FORMAT_MASK);
- /* XXX width must be 64, stride 256 = 0x00 28 */
- cntl |= CURSOR_ENABLE |
+ /* XXX width must be 64, stride 256 = 0x00 28 */
+ cntl = 0;
+ if (base)
+ cntl = (CURSOR_ENABLE |
CURSOR_GAMMA_ENABLE |
- CURSOR_FORMAT_ARGB;
- } else
- cntl = ~(CURSOR_ENABLE | CURSOR_GAMMA_ENABLE);
- I915_WRITE(_CURACNTR, cntl);
-
- intel_crtc-cursor_visible = visible;
+ CURSOR_FORMAT_ARGB);
+ if (intel_crtc-cursor_cntl != cntl) {
+ I915_WRITE(_CURACNTR, cntl);
+ POSTING_READ(_CURACNTR);
+ intel_crtc-cursor_cntl = cntl;
+ }
}
static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
@@ -7595,16 +7599,12 @@ static void i9xx_update_cursor(struct drm_crtc *crtc,
u32 base)
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_crtc-pipe;
- bool visible = base != 0;
-
- if (intel_crtc-cursor_visible != visible) {
- int16_t width = intel_crtc-cursor_width;
- uint32_t cntl = I915_READ(CURCNTR(pipe));
- if (base) {
- cntl = ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
- cntl |= MCURSOR_GAMMA_ENABLE;
+ uint32_t cntl;
- switch (width) {
+ cntl = 0;
+ if (base) {
+ cntl = MCURSOR_GAMMA_ENABLE;
+ switch (intel_crtc-cursor_width) {
case 64:
cntl |= CURSOR_MODE_64_ARGB_AX;
break;
@@ -7617,18 +7617,16 @@ static void i9xx_update_cursor(struct drm_crtc *crtc,
u32 base)
default:
WARN_ON(1);
return;
- }
- cntl |= pipe 28; /* Connect to correct pipe */
- } else {
-
Re: [Intel-gfx] [PATCH] drm/i915: Don't WARN about unexpected hpd interrupts on gmch platforms
Daniel Vetter writes:
The status bits are unconditionally set, the control bits only enable
the actual interrupt generation. Which means if we get some random
other interrupts we'll bogusly complain about them.
So restrict the WARN to platforms with a sane hotplug interrupt
handling scheme.
This WARN has been introduced in
commit b8f102e8bf71cacf33326360fdf9dcfd1a63925b
Author: Egbert Eich e...@suse.de
Date: Fri Jul 26 14:14:24 2013 +0200
drm/i915: Add messages useful for HPD storm detection debugging (v2)
Cc: Egbert Eich e...@suse.de
Cc: bitlord bitlord0...@gmail.com
Reported-by: bitlord bitlord0...@gmail.com
Cc: sta...@vger.kernel.org
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_irq.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 7753249b3a95..f98ba4e6e70b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1362,10 +1362,20 @@ static inline void intel_hpd_irq_handler(struct
drm_device *dev,
spin_lock(dev_priv-irq_lock);
for (i = 1; i HPD_NUM_PINS; i++) {
-WARN_ONCE(hpd[i] hotplug_trigger
- dev_priv-hpd_stats[i].hpd_mark == HPD_DISABLED,
- Received HPD interrupt (0x%08x) on pin %d (0x%08x)
although disabled\n,
- hotplug_trigger, i, hpd[i]);
+if (hpd[i] hotplug_trigger
+dev_priv-hpd_stats[i].hpd_mark == HPD_DISABLED) {
+/*
+ * On GMCH platforms the interrupt mask bits only
+ * prevent irq generation, not the setting of the
+ * hotplug bits itself. So only WARN about unexpected
+ * interrupts on saner platforms.
+ */
+WARN_ONCE(INTEL_INFO(dev)-gen = 5
!IS_VALLEYVIEW(dev),
+ Received HPD interrupt (0x%08x) on pin %d
(0x%08x) although disabled\n,
+ hotplug_trigger, i, hpd[i]);
Personally I'd prefer the condition in the WARN..() macro to be the
unexpected condition you want to warn about. This makes it easier for
anybody not up to speed with the details of hotplug handling to understand
the code.
Of course the way you structure this avoids a lot of unnecessary tests.
But if this is a concern maybe the entire for loop should be restructured
with
if (!(hpd[i] hotplug_trigger))
continue;
right at the beginning.
+
+continue;
+}
if (!(hpd[i] hotplug_trigger) ||
dev_priv-hpd_stats[i].hpd_mark != HPD_ENABLED)
--
1.8.5.2
Cheers,
Egbert.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't WARN about unexpected hpd interrupts on gmch platforms
On Sat, Apr 12, 2014 at 8:12 PM, Egbert Eich e...@freedesktop.org wrote:
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 7753249b3a95..f98ba4e6e70b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1362,10 +1362,20 @@ static inline void intel_hpd_irq_handler(struct
drm_device *dev,
spin_lock(dev_priv-irq_lock);
for (i = 1; i HPD_NUM_PINS; i++) {
-WARN_ONCE(hpd[i] hotplug_trigger
- dev_priv-hpd_stats[i].hpd_mark == HPD_DISABLED,
- Received HPD interrupt (0x%08x) on pin %d (0x%08x)
although disabled\n,
- hotplug_trigger, i, hpd[i]);
+if (hpd[i] hotplug_trigger
+dev_priv-hpd_stats[i].hpd_mark == HPD_DISABLED) {
+/*
+ * On GMCH platforms the interrupt mask bits only
+ * prevent irq generation, not the setting of the
+ * hotplug bits itself. So only WARN about unexpected
+ * interrupts on saner platforms.
+ */
+WARN_ONCE(INTEL_INFO(dev)-gen = 5
!IS_VALLEYVIEW(dev),
+ Received HPD interrupt (0x%08x) on pin %d
(0x%08x) although disabled\n,
+ hotplug_trigger, i, hpd[i]);
Personally I'd prefer the condition in the WARN..() macro to be the
unexpected condition you want to warn about. This makes it easier for
anybody not up to speed with the details of hotplug handling to understand
the code.
Of course the way you structure this avoids a lot of unnecessary tests.
But if this is a concern maybe the entire for loop should be restructured
with
if (!(hpd[i] hotplug_trigger))
continue;
right at the beginning.
+
+continue;
+}
We want to skip the hpd handling in any case if the interrupt is
blocked, otherwise every time we have some unrelated interrupt on gmch
platforms while a hpd storm is ongoing we'll fire off the hotplug
work. Which we obviously don't want since that defeats the point of
the storm handling code.
But we only want to WARN on platforms where we can reliably stop the
status bits too, hence why I've nested the checks like this.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
