Re: [Intel-gfx] [PATCH 01/10] drm/i915: add interface to simulate gpu hangs
On Fri, 27 Apr 2012 15:17:38 +0200
Daniel Vetter wrote:
> gpu reset is a very important piece of our infrastructure.
> Unfortunately we only really it test by actually hanging the gpu,
> which often has bad side-effects for the entire system. And the gpu
> hang handling code is one of the rather complicated pieces of code we
> have, consisting of
> - hang detection
> - error capture
> - actual gpu reset
> - reset of all the gem bookkeeping
> - reinitialition of the entire gpu
>
> This patch adds a debugfs to selectively stopping rings by ceasing to
> update the hw tail pointer, which will result in the gpu no longer
> updating it's head pointer and eventually to the hangcheck firing.
> This way we can exercise the gpu hang code under controlled conditions
> without a dying gpu taking down the entire systems.
You mean, tail pointer?
>
> Patch motivated by me forgetting to properly reinitialize ppgtt after
> a gpu reset.
>
> Usage:
>
> echo $((1 << $ringnum)) > i915_ring_stop # stops one ring
>
> echo 0x > i915_ring_stop # stops all, future-proof version
>
> then run whatever testload is desired. i915_ring_stop automatically
> resets after a gpu hang is detected to avoid hanging the gpu to fast
> and declaring it wedged.
>
> v2: Incorporate feedback from Chris Wilson.
>
> v3: Add the missing cleanup.
>
> Signed-Off-by: Daniel Vetter
> Reviewed-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 65
> +++
> drivers/gpu/drm/i915/i915_drv.h |2 +
> drivers/gpu/drm/i915/intel_ringbuffer.c |4 ++ 3 files changed, 71
> insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c index 120db46..3f6e28e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1585,6 +1585,64 @@ static const struct file_operations
> i915_wedged_fops = { };
>
> static ssize_t
> +i915_ring_stop_read(struct file *filp,
> + char __user *ubuf,
> + size_t max,
> + loff_t *ppos)
> +{
> + struct drm_device *dev = filp->private_data;
> + drm_i915_private_t *dev_priv = dev->dev_private;
> + char buf[80];
> + int len;
> +
> + len = snprintf(buf, sizeof(buf),
> +"0x%08x\n", dev_priv->stop_rings);
> +
> + if (len > sizeof(buf))
> + len = sizeof(buf);
> +
> + return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> +}
> +
> +static ssize_t
> +i915_ring_stop_write(struct file *filp,
> + const char __user *ubuf,
> + size_t cnt,
> + loff_t *ppos)
> +{
> + struct drm_device *dev = filp->private_data;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + char buf[20];
> + int val = 0;
> +
> + if (cnt > 0) {
> + if (cnt > sizeof(buf) - 1)
> + return -EINVAL;
> +
> + if (copy_from_user(buf, ubuf, cnt))
> + return -EFAULT;
> + buf[cnt] = 0;
> +
> + val = simple_strtoul(buf, NULL, 0);
> + }
> +
> + DRM_DEBUG_DRIVER("Stopping rings 0x%08x\n", val);
> +
> + mutex_lock(&dev->struct_mutex);
> + dev_priv->stop_rings = val;
> + mutex_unlock(&dev->struct_mutex);
> +
> + return cnt;
> +}
Not a huge deal, but I'd go with an interruptible mutex lock there.
Also, I think it would be cool if you did a tail update when going from
!0->0 stop_rings.
> +
> +static const struct file_operations i915_ring_stop_fops = {
> + .owner = THIS_MODULE,
> + .open = simple_open,
> + .read = i915_ring_stop_read,
> + .write = i915_ring_stop_write,
> + .llseek = default_llseek,
> +};
> +static ssize_t
> i915_max_freq_read(struct file *filp,
> char __user *ubuf,
> size_t max,
> @@ -1884,6 +1942,11 @@ int i915_debugfs_init(struct drm_minor *minor)
> &i915_cache_sharing_fops);
> if (ret)
> return ret;
> + ret = i915_debugfs_create(minor->debugfs_root, minor,
> + "i915_ring_stop",
> + &i915_ring_stop_fops);
> + if (ret)
> + return ret;
>
> return drm_debugfs_create_files(i915_debugfs_list,
> I915_DEBUGFS_ENTRIES,
> @@ -1902,6 +1965,8 @@ void i915_debugfs_cleanup(struct drm_minor
> *minor) 1, minor);
> drm_debugfs_remove_files((struct drm_info_list *)
> &i915_cache_sharing_fops, 1, minor);
> + drm_debugfs_remove_files((struct drm_info_list *)
> &i915_ring_stop_fops,
> + 1, minor);
> }
>
> #endif /* CONFIG_DEBUG_FS */
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index 0095c8d..5e62b35 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -413,6 +413,8 @@ typed
Re: [Intel-gfx] [PATCH 09/12] drm/i915: timeout parameter for seqno wait
On Fri, 27 Apr 2012 17:00:20 +0200 Daniel Vetter wrote: > On Thu, Apr 26, 2012 at 04:03:06PM -0700, Ben Widawsky wrote: > > Insert a wait parameter in the code so we can possibly timeout on a > > seqno wait if need be. The code should be functionally the same as > > before because all the callers will continue to retry if an > > arbitrary timeout elapses. > > > > We'd like to have nanosecond granularity, but the only way to do > > this is with hrtimer, and that doesn't fit well with the needs of > > this code. > > > > Signed-off-by: Ben Widawsky > > I have to admit, I'm a bit unhappy with this swiss-army-tool > wait_seqno and what it looks like. What about copy&pasting > __wait_seqno_timeout, which is always interruptible? > -Daniel I'm going to put the onus on you for this bikeshed to test it, and see how you like it. I have tried have the other function, and I felt this one looked better. Though I assume I have to fix the rebase error that Chris pointed out anyway. So I'll do a v3 with the separate function *after* you confirm you really want it. Otherwise, you'll just get the rebase fail fix. Ben ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] l3 parity tool
This tool is used to program, and read the l3 parity information.
Signed-off-by: Ben Widawsky
---
lib/intel_chipset.h |2 +
tools/Makefile.am |3 +-
tools/intel_l3_parity.c | 150 +++
3 files changed, 154 insertions(+), 1 deletion(-)
create mode 100644 tools/intel_l3_parity.c
diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h
index 668bf59..7bc5e17 100755
--- a/lib/intel_chipset.h
+++ b/lib/intel_chipset.h
@@ -82,6 +82,7 @@
#define PCI_CHIP_IVYBRIDGE_GT2 0x0162
#define PCI_CHIP_IVYBRIDGE_M_GT1 0x0156 /* mobile */
#define PCI_CHIP_IVYBRIDGE_M_GT2 0x0166
+#define PCI_CHIP_IVYBRIDGE_S_GT2 0x016a
#define PCI_CHIP_IVYBRIDGE_S 0x015a /* server */
#define IS_MOBILE(devid) (devid == PCI_CHIP_I855_GM || \
@@ -149,6 +150,7 @@
devid == PCI_CHIP_IVYBRIDGE_GT2 || \
devid == PCI_CHIP_IVYBRIDGE_M_GT1 || \
devid == PCI_CHIP_IVYBRIDGE_M_GT2 || \
+devid == PCI_CHIP_IVYBRIDGE_S_GT2 || \
devid == PCI_CHIP_IVYBRIDGE_S)
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 058835c..d352316 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -14,7 +14,8 @@ bin_PROGRAMS =\
intel_reg_snapshot \
intel_reg_write \
intel_reg_read \
- intel_forcewaked
+ intel_forcewaked\
+ intel_l3_parity
noinst_PROGRAMS = \
intel_dump_decode \
diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
new file mode 100644
index 000..1c16903
--- /dev/null
+++ b/tools/intel_l3_parity.c
@@ -0,0 +1,150 @@
+/*
+ * Copyright © 2012 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Ben Widawsky
+ *
+ */
+
+#define _GNU_SOURCE
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "drmtest.h"
+
+#define NUM_BANKS 4
+#define NUM_SUBBANKS 8
+#define NUM_REGS (NUM_BANKS * NUM_SUBBANKS)
+
+struct __attribute__ ((__packed__)) l3_log_register {
+ uint32_t row0_enable: 1;
+ uint32_t rsvd2 : 4;
+ uint32_t row0 : 11;
+ uint32_t row1_enable: 1;
+ uint32_t rsvd1 : 4;
+ uint32_t row1 : 11;
+} l3log[NUM_BANKS][NUM_SUBBANKS];
+
+static void dumpit(void)
+{
+ int i, j;
+
+ for (i = 0; i < NUM_BANKS; i++) {
+ for (j = 0; j < NUM_SUBBANKS; j++) {
+ struct l3_log_register *reg = &l3log[i][j];
+
+ if (reg->row0_enable)
+ printf("Row %d, Bank %d, Subbank %d is disable\n",
+ reg->row0, i, j);
+ if (reg->row1_enable)
+ printf("Row %d, Bank %d, Subbank %d is disable\n",
+ reg->row1, i, j);
+ }
+ }
+}
+
+static int disable_rbs(int row, int bank, int sbank)
+{
+ struct l3_log_register *reg = &l3log[bank][sbank];
+
+ // can't map more than 2 rows
+ if (reg->row0_enable && reg->row1_enable)
+ return -1;
+
+ // can't remap the same row twice
+ if ((reg->row0_enable && reg->row0 == row) ||
+ (reg->row1_enable && reg->row1 == row)) {
+ return -1;
+ }
+
+ if (reg->row0_enable) {
+ reg->row1 = row;
+ reg->row1_enable = 1;
+ } else {
+ reg->row0 = row;
+ reg->row0_enable = 1;
+ }
+
+ return 0;
+}
+
+static int do_parse(int argc, char *argv[])
+{
+ int row, bank, sbank, i, ret;
+
+ for (i = 1; i < argc; i++) {
+ ret = sscanf(argv[i], "%d,
[Intel-gfx] [PATCH 5/5] drm/i915: l3 parity sysfs interface
Dumb binary interfaces which allow root-only updates of the cache
remapping registers. As mentioned in a previous patch, software using
this interface needs to know about HW limits, and other programming
considerations as the kernel interface does no checking for these things
on the root-only interface.
v1: Drop extra posting reads (Chris)
Return negative values in the sysfs interfaces on errors (Chris)
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_sysfs.c | 128 -
1 file changed, 126 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
b/drivers/gpu/drm/i915/i915_sysfs.c
index 79f8344..ed77cbf 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include "intel_drv.h"
#include "i915_drv.h"
static u32 calc_residency(struct drm_device *dev, const u32 reg)
@@ -92,20 +93,143 @@ static struct attribute_group rc6_attr_group = {
.attrs = rc6_attrs
};
+static int l3_access_valid(struct drm_device *dev, loff_t offset)
+{
+ if (!IS_IVYBRIDGE(dev))
+ return -EPERM;
+
+ if (offset % 4 != 0)
+ return -EPERM;
+
+ if (offset >= GEN7_L3LOG_SIZE)
+ return -ENXIO;
+
+ return 0;
+}
+
+static ssize_t
+i915_l3_read(struct file *filp, struct kobject *kobj,
+struct bin_attribute *attr, char *buf,
+loff_t offset, size_t count)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
+ struct drm_device *drm_dev = dminor->dev;
+ struct drm_i915_private *dev_priv = drm_dev->dev_private;
+ uint32_t misccpctl;
+ int i, ret;
+
+ ret = l3_access_valid(drm_dev, offset);
+ if (ret)
+ return ret;
+
+ ret = i915_mutex_lock_interruptible(drm_dev);
+ if (ret)
+ return ret;
+
+ misccpctl = I915_READ(GEN7_MISCCPCTL);
+ I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE);
+
+ for (i = offset; count >= 4 && i < GEN7_L3LOG_SIZE; i += 4, count -= 4)
+ *((uint32_t *)(&buf[i])) = I915_READ(GEN7_L3LOG_BASE + i);
+
+ I915_WRITE(GEN7_MISCCPCTL, misccpctl);
+
+ mutex_unlock(&drm_dev->struct_mutex);
+
+ return i - offset;
+}
+
+static ssize_t
+i915_l3_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf,
+ loff_t offset, size_t count)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
+ struct drm_device *drm_dev = dminor->dev;
+ struct drm_i915_private *dev_priv = drm_dev->dev_private;
+ u32 *temp = NULL;
+ int i, ret;
+
+ ret = l3_access_valid(drm_dev, offset);
+ if (ret)
+ return ret;
+
+ ret = i915_mutex_lock_interruptible(drm_dev);
+ if (ret)
+ return ret;
+
+ if (!dev_priv->mm.l3_remap_info) {
+ temp = kzalloc(GEN7_L3LOG_SIZE, GFP_KERNEL);
+ if (!temp) {
+ mutex_unlock(&drm_dev->struct_mutex);
+ return -ENOMEM;
+ }
+ }
+
+ ret = i915_gpu_idle(drm_dev, true);
+ if (ret) {
+ kfree(temp);
+ mutex_unlock(&drm_dev->struct_mutex);
+ return ret;
+ }
+
+ /* TODO: Ideally we really want a GPU reset here to make sure errors
+* aren't propagated Since I cannot find a stable way to reset the GPU
+* at this point it is left as a TODO.
+ */
+
+ if (dev_priv->mm.l3_remap_info)
+ temp = dev_priv->mm.l3_remap_info;
+
+ dev_priv->mm.l3_remap_info = temp;
+
+ for (i = offset; count >= 4 && i < GEN7_L3LOG_SIZE; i += 4, count -= 4)
{
+ u32 remap = I915_READ(GEN7_L3LOG_BASE + i);
+ if (remap && remap != *temp)
+ DRM_ERROR("0x%x was already programmed to %x\n",
+ GEN7_L3LOG_BASE + i, remap);
+ *temp++ = *(uint32_t *)(&buf[i]);
+ }
+
+ i915_gem_l3_remap(drm_dev);
+
+ mutex_unlock(&drm_dev->struct_mutex);
+
+ return offset - i;
+}
+
+static struct bin_attribute dpf_attrs = {
+ .attr = {.name = "l3_parity", .mode = (S_IRUSR | S_IWUSR)},
+ .size = GEN7_L3LOG_SIZE,
+ .read = i915_l3_read,
+ .write = i915_l3_write,
+ .mmap = NULL
+};
+
void i915_setup_sysfs(struct drm_device *dev)
{
int ret;
- /* ILK doesn't have any residency information */
+ /* ILK and below don't yet have relevant sysfs files */
if (INTEL_INFO(dev)->gen < 6)
return;
ret = sysfs_merge_group(&dev->primary->kdev.kobj, &rc6_attr_group);
if (ret)
-
[Intel-gfx] [PATCH 4/5] drm/i915: remap l3 on hw init
If any l3 rows have been previously remapped, we must remap them after
GPU reset/resume too.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_drv.h |3 +++
drivers/gpu/drm/i915/i915_gem.c | 26 ++
drivers/gpu/drm/i915/i915_reg.h |3 +++
3 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9505fc0..e9efe17 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -641,6 +641,8 @@ typedef struct drm_i915_private {
/** PPGTT used for aliasing the PPGTT with the GTT */
struct i915_hw_ppgtt *aliasing_ppgtt;
+ u32 *l3_remap_info;
+
struct shrinker inactive_shrinker;
/**
@@ -1290,6 +1292,7 @@ int __must_check i915_gem_object_set_domain(struct
drm_i915_gem_object *obj,
uint32_t write_domain);
int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj);
int __must_check i915_gem_init_hw(struct drm_device *dev);
+void i915_gem_l3_remap(struct drm_device *dev);
void i915_gem_init_swizzling(struct drm_device *dev);
void i915_gem_init_ppgtt(struct drm_device *dev);
void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7bc4a40..bb3ef9f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3475,6 +3475,30 @@ i915_gem_idle(struct drm_device *dev)
return 0;
}
+void i915_gem_l3_remap(struct drm_device *dev)
+{
+ drm_i915_private_t *dev_priv = dev->dev_private;
+ u32 misccpctl;
+ int i;
+
+ if (!dev_priv->mm.l3_remap_info)
+ return;
+
+ WARN_ON(!IS_IVYBRIDGE(dev));
+
+ misccpctl = I915_READ(GEN7_MISCCPCTL);
+ I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE);
+ POSTING_READ(GEN7_MISCCPCTL);
+
+ for (i = 0; i < GEN7_L3LOG_SIZE; i += 4)
+ I915_WRITE(GEN7_L3LOG_BASE + i,
dev_priv->mm.l3_remap_info[i/4]);
+
+ /* Make sure all the writes land before disabling dop clock gating */
+ POSTING_READ(GEN7_L3LOG_BASE);
+
+ I915_WRITE(GEN7_MISCCPCTL, misccpctl);
+}
+
void i915_gem_init_swizzling(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = dev->dev_private;
@@ -3566,6 +3590,8 @@ i915_gem_init_hw(struct drm_device *dev)
drm_i915_private_t *dev_priv = dev->dev_private;
int ret;
+ i915_gem_l3_remap(dev);
+
i915_gem_init_swizzling(dev);
ret = intel_init_render_ring_buffer(dev);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 72db6a9..8c546fd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4047,6 +4047,9 @@
((reg & GEN7_L3CDERRST1_SUBBANK_MASK) >> 8)
#define GEN7_L3CDERRST1_ENABLE (1<<7)
+#define GEN7_L3LOG_BASE0xB070
+#define GEN7_L3LOG_SIZE0x80
+
#define G4X_AUD_VID_DID0x62020
#define INTEL_AUDIO_DEVCL 0x808629FB
#define INTEL_AUDIO_DEVBLC 0x80862801
--
1.7.10
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] drm/i915: enable parity error interrupts
The previous patch put all the code, and handlers in place. It should
now be safe to enable the parity error interrupt. The parity error must
be unmasked in both the GTIMR, and the CS IMR. Unfortunately, the docs
aren't clear about this; nevertheless it's the truth.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_irq.c |4 ++--
drivers/gpu/drm/i915/intel_ringbuffer.c | 14 --
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 81e5a7d..334e1b3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2235,13 +2235,13 @@ static int ivybridge_irq_postinstall(struct drm_device
*dev)
DE_PIPEB_VBLANK_IVB);
POSTING_READ(DEIER);
- dev_priv->gt_irq_mask = ~0;
+ dev_priv->gt_irq_mask = ~GT_GEN7_L3_PARITY_ERROR_INTERRUPT;
I915_WRITE(GTIIR, I915_READ(GTIIR));
I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
render_irqs = GT_USER_INTERRUPT | GEN6_BSD_USER_INTERRUPT |
- GEN6_BLITTER_USER_INTERRUPT;
+ GEN6_BLITTER_USER_INTERRUPT | GT_GEN7_L3_PARITY_ERROR_INTERRUPT;
I915_WRITE(GTIER, render_irqs);
POSTING_READ(GTIER);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3b38157..02762b1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -420,6 +420,9 @@ static int init_render_ring(struct intel_ring_buffer *ring)
INSTPM_FORCE_ORDERING << 16 | INSTPM_FORCE_ORDERING);
}
+ if (IS_IVYBRIDGE(dev))
+ I915_WRITE_IMR(ring, ~GEN6_RENDER_L3_PARITY_ERROR);
+
return ret;
}
@@ -770,7 +773,11 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring)
spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (ring->irq_refcount++ == 0) {
- I915_WRITE_IMR(ring, ~ring->irq_enable_mask);
+ if (IS_IVYBRIDGE(dev) && ring->id == RCS)
+ I915_WRITE_IMR(ring, ~(ring->irq_enable_mask |
+ GEN6_RENDER_L3_PARITY_ERROR));
+ else
+ I915_WRITE_IMR(ring, ~ring->irq_enable_mask);
dev_priv->gt_irq_mask &= ~ring->irq_enable_mask;
I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
POSTING_READ(GTIMR);
@@ -789,7 +796,10 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring)
spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (--ring->irq_refcount == 0) {
- I915_WRITE_IMR(ring, ~0);
+ if (IS_IVYBRIDGE(dev) && ring->id == RCS)
+ I915_WRITE_IMR(ring, ~GEN6_RENDER_L3_PARITY_ERROR);
+ else
+ I915_WRITE_IMR(ring, ~0);
dev_priv->gt_irq_mask |= ring->irq_enable_mask;
I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
POSTING_READ(GTIMR);
--
1.7.10
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] drm/i915: Dynamic Parity Detection handling
On IVB hardware we are given an interrupt whenever a L3 parity error
occurs in the L3 cache. The L3 cache is used by internal GPU clients
only. This is a very rare occurrence (in fact to test this I need to
use specially instrumented silicon).
When a row in the L3 cache detects a parity error the HW generates an
interrupt. The interrupt is masked in GTIMR until we get a chance to
read some registers and alert userspace via a uevent. With this
information userspace can use a sysfs interface (follow-up patch) to
remap those rows.
Way above my level of understanding, but if a given row fails, it is
statistically more likely to fail again than a row which has not failed.
Therefore it is desirable for an operating system to maintain a lifelong
list of failing rows and always remap any bad rows on driver load.
Hardware limits the number of rows that are remappable per bank/subbank,
and should more than that many rows detect parity errors, software
should maintain a list of the most frequent errors, and remap those
rows.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_drv.h |2 +
drivers/gpu/drm/i915/i915_irq.c | 83 +++
drivers/gpu/drm/i915/i915_reg.h | 17
3 files changed, 102 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 69e1539..9505fc0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -804,6 +804,8 @@ typedef struct drm_i915_private {
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
+
+ struct work_struct parity_error_work;
} drm_i915_private_t;
enum hdmi_force_audio {
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ab023ca..81e5a7d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -430,6 +430,83 @@ static void gen6_pm_rps_work(struct work_struct *work)
mutex_unlock(&dev_priv->dev->struct_mutex);
}
+
+/**
+ * ivybridge_parity_work - Workqueue called when a parity error interrupt
+ * occurred.
+ *
+ * Doesn't actually do anything except notify userspace so that userspace may
+ * disable things later on.
+ */
+static void ivybridge_parity_work(struct work_struct *work)
+{
+ drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,
+ parity_error_work);
+
+ u32 error_status, row, bank, subbank;
+ char *parity_event[5];
+ uint32_t misccpctl;
+ unsigned long flags;
+
+ /* We must turn off DOP level clock gating to access the L3 registers.
+* In order to prevent a get/put style interface, acquire struct mutex
+* any time we access those registers.
+*/
+ mutex_lock(&dev_priv->dev->struct_mutex);
+
+ misccpctl = I915_READ(GEN7_MISCCPCTL);
+ I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE);
+ POSTING_READ(GEN7_MISCCPCTL);
+
+ error_status = I915_READ(GEN7_L3CDERRST1);
+ row = GEN7_PARITY_ERROR_ROW(error_status);
+ bank = GEN7_PARITY_ERROR_BANK(error_status);
+ subbank = GEN7_PARITY_ERROR_SUBBANK(error_status);
+
+ I915_WRITE(GEN7_L3CDERRST1, GEN7_PARITY_ERROR_VALID |
+ GEN7_L3CDERRST1_ENABLE);
+ POSTING_READ(GEN7_L3CDERRST1);
+
+ I915_WRITE(GEN7_MISCCPCTL, misccpctl);
+
+ spin_lock_irqsave(&dev_priv->irq_lock, flags);
+ dev_priv->gt_irq_mask &= ~GT_GEN7_L3_PARITY_ERROR_INTERRUPT;
+ I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
+ spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+
+ mutex_unlock(&dev_priv->dev->struct_mutex);
+
+ parity_event[0] = "L3_PARITY_ERROR=1";
+ parity_event[1] = kasprintf(GFP_KERNEL, "ROW=%d", row);
+ parity_event[2] = kasprintf(GFP_KERNEL, "BANK=%d", bank);
+ parity_event[3] = kasprintf(GFP_KERNEL, "SUBBANK=%d", subbank);
+ parity_event[4] = NULL;
+
+ kobject_uevent_env(&dev_priv->dev->primary->kdev.kobj,
+ KOBJ_CHANGE, parity_event);
+
+ kfree(parity_event[3]);
+ kfree(parity_event[2]);
+ kfree(parity_event[1]);
+}
+
+void ivybridge_handle_parity_error(struct drm_device *dev)
+{
+ drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+ unsigned long flags;
+
+ if (WARN_ON(IS_GEN6(dev)))
+ return;
+
+ spin_lock_irqsave(&dev_priv->irq_lock, flags);
+ dev_priv->gt_irq_mask |= GT_GEN7_L3_PARITY_ERROR_INTERRUPT;
+ I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
+ spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+
+ queue_work(dev_priv->wq, &dev_priv->parity_error_work);
+ DRM_INFO("Parity error interrupt. Scheduling work\n");
+}
+
static void snb_gt_irq_handler(struct drm_device *dev,
struct drm_i915_private *dev_priv,
[Intel-gfx] [PATCH 1/5] drm/i915: Use a global lock for modifying global irq flags
From: Chris Wilson
We were attempting to use a per-ring spinlock whilst modifying global
IRQ flags. A recipe for rare missed interrupts.
Signed-off-by: Chris Wilson
Acked-by: Ben Widawsky
Signed-off-by: Daniel Vetter
Conflicts:
drivers/gpu/drm/i915/intel_ringbuffer.c
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 31 ++-
drivers/gpu/drm/i915/intel_ringbuffer.h |3 +--
2 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 12d9bc7..3b38157 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -613,17 +613,18 @@ gen5_ring_get_irq(struct intel_ring_buffer *ring)
{
struct drm_device *dev = ring->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
+ unsigned long flags;
if (!dev->irq_enabled)
return false;
- spin_lock(&ring->irq_lock);
+ spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (ring->irq_refcount++ == 0) {
dev_priv->gt_irq_mask &= ~ring->irq_enable_mask;
I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
POSTING_READ(GTIMR);
}
- spin_unlock(&ring->irq_lock);
+ spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
return true;
}
@@ -633,14 +634,15 @@ gen5_ring_put_irq(struct intel_ring_buffer *ring)
{
struct drm_device *dev = ring->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
+ unsigned long flags;
- spin_lock(&ring->irq_lock);
+ spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (--ring->irq_refcount == 0) {
dev_priv->gt_irq_mask |= ring->irq_enable_mask;
I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
POSTING_READ(GTIMR);
}
- spin_unlock(&ring->irq_lock);
+ spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
}
static bool
@@ -648,17 +650,18 @@ i9xx_ring_get_irq(struct intel_ring_buffer *ring)
{
struct drm_device *dev = ring->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
+ unsigned long flags;
if (!dev->irq_enabled)
return false;
- spin_lock(&ring->irq_lock);
+ spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (ring->irq_refcount++ == 0) {
dev_priv->irq_mask &= ~ring->irq_enable_mask;
I915_WRITE(IMR, dev_priv->irq_mask);
POSTING_READ(IMR);
}
- spin_unlock(&ring->irq_lock);
+ spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
return true;
}
@@ -668,14 +671,15 @@ i9xx_ring_put_irq(struct intel_ring_buffer *ring)
{
struct drm_device *dev = ring->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
+ unsigned long flags;
- spin_lock(&ring->irq_lock);
+ spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (--ring->irq_refcount == 0) {
dev_priv->irq_mask |= ring->irq_enable_mask;
I915_WRITE(IMR, dev_priv->irq_mask);
POSTING_READ(IMR);
}
- spin_unlock(&ring->irq_lock);
+ spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
}
void intel_ring_setup_status_page(struct intel_ring_buffer *ring)
@@ -754,6 +758,7 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring)
{
struct drm_device *dev = ring->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
+ unsigned long flags;
if (!dev->irq_enabled)
return false;
@@ -763,14 +768,14 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring)
* blt/bsd rings on ivb. */
gen6_gt_force_wake_get(dev_priv);
- spin_lock(&ring->irq_lock);
+ spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (ring->irq_refcount++ == 0) {
I915_WRITE_IMR(ring, ~ring->irq_enable_mask);
dev_priv->gt_irq_mask &= ~ring->irq_enable_mask;
I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
POSTING_READ(GTIMR);
}
- spin_unlock(&ring->irq_lock);
+ spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
return true;
}
@@ -780,15 +785,16 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring)
{
struct drm_device *dev = ring->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
+ unsigned long flags;
- spin_lock(&ring->irq_lock);
+ spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (--ring->irq_refcount == 0) {
I915_WRITE_IMR(ring, ~0);
dev_priv->gt_irq_mask |= ring->irq_enable_mask;
I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
POSTING_READ(GTIMR);
}
- spin_unlock(&ring->irq_lock);
+ spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
gen6_gt_force_wake_put(dev_priv);
}
@@ -922,7 +928,6 @@ static int int
[Intel-gfx] [PATCH 0/5] Dynamic Parity Detection/Correction
Unfortunately dinq is not working on my IVB at this moment, so I was forced to base these patches on din ie. that's why I've added Chris' patch to the series manually. Regarding whether or not to actually upstream these patches, I think it would be awesome if distros could let us know how interested they are in incorporating this. It is of particular use for any applications using the GPU for compute. Even if distros don't want it, have the uevent/interrupt is nice to incorporate, but I would think twice about the sysfs interface. Now for the explanation (you may want to get a coffee first): Internal to the GPU is a cache referred to in docs as L3. The smallest unit of the cache which is addressable is called a row. There are x rows in each subbank, and y subbanks in each of the z banks. HW provides two extra rows per subbank, and a software mechanism to remap these rows. The addressing after remapping is handled transparently to software. There is also an interrupt generated by the render CS to tell us when a parity error occurs in one of the rows. There is one portion currently unimplemented in the series; we are required to issue a GPU reset before we remap a row. The documents I have do not make it clear *exactly* why the gpu reset must occur, but I believe, similar to Linux, it is the windows mechanism for basically telling GPU clients that whatever work they've submitted needs to be resubmitted. There are various clients which use the L3, however none of these should be utilized during simple modeset/fbcon. Therefore, I believe the following algorithm is guaranteed to work: 1. On boot check some non-volatile storage for bad r/b/s 2. load i915 3. disable bad rbs ASAP 4. Wait forever for uevent of bad r/b/s 5. store r/b/s in some non-volatile storage 6. reboot; goto 1 If we had the reset working, we could avoid the reboot, and instead do: 1. On boot check some non-volatile storage for bad r/b/s 2. load i915 3. disable bad rbs ASAP 4. Wait forever for uevent of bad r/b/s 5. store r/b/s in some non-volatile storage 6. gpu reset; goto 3 The reset is essentially used to "automatically" make all GPU clients aware that they may need to resubmit their data. The problem with algorithm #2 without the reset is that there is no way (afaict) to map the RBS to a BO, and so we have no way to even figure out if the bad data was propagated to the BO. So an alternative to reset is if system software detects the uevent, it can send a signal to all known (or computation based) GPU clients. See the intel-gpu-tools app as a reference for how to use the sysfs interface. Ben Widawsky (4): drm/i915: Dynamic Parity Detection handling drm/i915: enable parity error interrupts drm/i915: remap l3 on hw init drm/i915: l3 parity sysfs interface Chris Wilson (1): drm/i915: Use a global lock for modifying global irq flags drivers/gpu/drm/i915/i915_drv.h |5 ++ drivers/gpu/drm/i915/i915_gem.c | 26 +++ drivers/gpu/drm/i915/i915_irq.c | 87 - drivers/gpu/drm/i915/i915_reg.h | 20 + drivers/gpu/drm/i915/i915_sysfs.c | 128 ++- drivers/gpu/drm/i915/intel_ringbuffer.c | 45 +++ drivers/gpu/drm/i915/intel_ringbuffer.h |3 +- 7 files changed, 293 insertions(+), 21 deletions(-) -- 1.7.10 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/10] drm/i915: kill gen4 gpu reset code
On Fri, 27 Apr 2012 21:17:02 +0200, Daniel Vetter wrote: > On Fri, Apr 27, 2012 at 11:49:16AM -0700, Eric Anholt wrote: > > On Fri, 27 Apr 2012 15:17:47 +0200, Daniel Vetter > > wrote: > > > It's busted. Compared to ilk, snb and ivb where I've run the i-g-t > > > hangman test in a loop for a few hours, my gm45 never managed to reset > > > the gpu. And Chris Wilson confirmed on irc that he's never seen a case > > > where the gen4 gpu reset code actually helped. > > > > I remember originally when the hangcheck stuff landed, on a 965gm > > running compiz, dragging a window with a bad shader around and seeing a > > stutter as it reset the hardware successfully before crashing soon after > > (then turning off the hanging plugin and continuing to use the system). > > I'm confused - so hangcheck once indeed managed to reset a i965gm gpu or > never? As far as I can remember, at one point it was reliably resetting. pgpNnpKVYTQh1.pgp Description: PGP signature ___ 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: quirk disable i915 backlight on Dell XPS 13z
On Fri, 2012-04-27 at 23:25 +0200, Daniel Vetter wrote:
> Hm. Can you please install intel-gpu-tools and attach the output of
> intel_reg_dumper? I have an idea ...
Here it is with intel_backlight/brightness set to 1 ("strobe light"
behavior occurring):
PGETBL_CTL: 0x
GEN6_INSTDONE_1: 0xfffb
GEN6_INSTDONE_2: 0x03305e3f
CPU_VGACNTRL: 0x8000 (disabled)
DIGITAL_PORT_HOTPLUG_CNTRL: 0x
RR_HW_CTL: 0x (low 0, high 0)
FDI_PLL_BIOS_0: 0x
FDI_PLL_BIOS_1: 0x
FDI_PLL_BIOS_2: 0x
DISPLAY_PORT_PLL_BIOS_0: 0x
DISPLAY_PORT_PLL_BIOS_1: 0x
DISPLAY_PORT_PLL_BIOS_2: 0x
FDI_PLL_FREQ_CTL: 0x
PIPEACONF: 0xc050 (enabled, active, 6bpc)
HTOTAL_A: 0x05d90555 (1366 active, 1498 total)
HBLANK_A: 0x05d90555 (1366 start, 1498 end)
HSYNC_A: 0x05a50585 (1414 start, 1446 end)
VTOTAL_A: 0x031502ff (768 active, 790 total)
VBLANK_A: 0x031502ff (768 start, 790 end)
VSYNC_A: 0x03040300 (769 start, 773 end)
VSYNCSHIFT_A: 0x
PIPEASRC: 0x055502ff (1366, 768)
PIPEA_DATA_M1: 0x7e1380e4 (TU 64, val 0x1380e4 1278180)
PIPEA_DATA_N1: 0x0020f580 (val 0x20f580 216)
PIPEA_DATA_M2: 0x (TU 1, val 0x0 0)
PIPEA_DATA_N2: 0x (val 0x0 0)
PIPEA_LINK_M1: 0x00011562 (val 0x11562 71010)
PIPEA_LINK_N1: 0x00041eb0 (val 0x41eb0 27)
PIPEA_LINK_M2: 0x (val 0x0 0)
PIPEA_LINK_N2: 0x (val 0x0 0)
DSPACNTR: 0xd8004400 (enabled)
DSPABASE: 0x
DSPASTRIDE: 0x1600 (88)
DSPASURF: 0x009bf000
DSPATILEOFF: 0x (0, 0)
PIPEBCONF: 0x (disabled, inactive, 8bpc)
HTOTAL_B: 0x (1 active, 1 total)
HBLANK_B: 0x (1 start, 1 end)
HSYNC_B: 0x (1 start, 1 end)
VTOTAL_B: 0x (1 active, 1 total)
VBLANK_B: 0x (1 start, 1 end)
VSYNC_B: 0x (1 start, 1 end)
VSYNCSHIFT_B: 0x
DSPBCNTR: 0x4000 (disabled)
DSPBBASE: 0x
DSPBSTRIDE: 0x (0)
DSPBSURF: 0x
DSPBTILEOFF: 0x (0, 0)
PIPEBSRC: 0x (1, 1)
PIPEB_DATA_M1: 0x (TU 1, val 0x0 0)
PIPEB_DATA_N1: 0x (val 0x0 0)
PIPEB_DATA_M2: 0x (TU 1, val 0x0 0)
PIPEB_DATA_N2: 0x (val 0x0 0)
PIPEB_LINK_M1: 0x (val 0x0 0)
PIPEB_LINK_N1: 0x (val 0x0 0)
PIPEB_LINK_M2: 0x (val 0x0 0)
PIPEB_LINK_N2: 0x (val 0x0 0)
PFA_CTL_1: 0x (disable, auto_scale yes,
auto_scale_cal no, v_filter enable, vadapt disable, mode least, filter_sel
programmed,chroma pre-filter disable, vert3tap auto, v_inter_invert field 1)
PFA_CTL_2: 0x7e80 (vscale 0.988281)
PFA_CTL_3: 0x3f40 (vscale initial phase 0.494141)
PFA_CTL_4: 0x7d54 (hscale 0.979126)
PFA_WIN_POS: 0x (0, 0)
PFA_WIN_SIZE: 0x (0, 0)
PFB_CTL_1: 0x (disable, auto_scale yes,
auto_scale_cal no, v_filter enable, vadapt disable, mode least, filter_sel
programmed,chroma pre-filter disable, vert3tap auto, v_inter_invert field 1)
PFB_CTL_2: 0x (vscale 0.00)
PFB_CTL_3: 0x (vscale initial phase 0.00)
PFB_CTL_4: 0x (hscale 0.00)
PFB_WIN_POS: 0x (0, 0)
PFB_WIN_SIZE: 0x (0, 0)
PCH_DREF_CONTROL: 0x1402 (cpu source disable, ssc_source
enable, nonspread_source enable, superspread_source disable, ssc4_mode
downspread, ssc1 enable, ssc4 disable)
PCH_RAWCLK_FREQ: 0x007d (FDL_TP1 timer 0.5us, FDL_TP2 timer
1.5us, freq 125)
PCH_DPLL_TMR_CFG: 0x0271186a
PCH_SSC4_PARMS: 0x01204860
PCH_SSC4_AUX_PARMS: 0x29c5
PCH_DPLL_SEL: 0x0008 (TransA DPLL enable (DPLL A), TransB
DPLL disable (DPLL (null)))
PCH_DPLL_ANALOG_CTL: 0x8000
PCH_DPLL_A: 0x88046004 (
Re: [Intel-gfx] [PATCH 2/2] drm/i915: quirk disable i915 backlight on Dell XPS 13z
On Fri, Apr 27, 2012 at 01:56:09PM -0700, Kamal Mostafa wrote: > On Thu, 2012-04-26 at 22:07 +0200, Daniel Vetter wrote: > > ... > > To cut things short: This time around I want more justification for the > > quirk than just "this makes this one machine work somehow". > > > A bit more detail... > > On this Dell XPS 13z "Ultrabook" (sandybridge_m 0x0116) when > intel_backlight/brightness gets stuffed with any value except 0 or > max_brightness, the backlight cycles between flashing (like a strobe > light!) and then pulsating (bright to dim to bright). That > flashing/pulsating cycle repeats continuously, about every 2 seconds. > The behavior is affected by the value stuffed into brightness more or > less along the lines of: > >0: very dim, totally stable >1: flashes like crazy for about 1.5 sec, then pulsates for 0.5 sec > 1000: flashes for about 0.5 sec, then pulsates for about 1.5 sec > 2000: flashes very briefly, then pulsates for about 2 sec > 3000: flickers, then pulsates for about 2 sec > 4000 pulsates continuously, every 2 sec > 4882: (max_brightness) full brightness, stable > > This behavior manifests both in X and in a text VT, and occurs with or > without the presence of other backlight interfaces besides > intel_backlight. It does not appear to me to be a userspace problem. > > The additional wrinkle is that this machine presents an acpi_video0 > backlight interface as well, and it even works properly -- but *only* > after you specifically stuff 0 into intel_backlight/brightness (or if > intel_backlight is disabled by the proposed quirk). > > Any non-zero intel_backlight/brightness value prevents acpi_video0 from > working. When intel_backlight is set to max_brightness (like at boot), > acpi_video0/brightness seems to have no effect at all; when > intel_backlight is set to other non-zero values, the flashing/pulsating > behavior occurs. > > I'd be (quite) happy to test a proper intel_backlight fix, but in the > meantime disabling it by quirk seems appropriate for this machine, since > that allows the acpi_video0 interface to work out of the box. > > Thanks for considering it, Hm. Can you please install intel-gpu-tools and attach the output of intel_reg_dumper? I have an idea ... Thanks, Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ 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: quirk disable i915 backlight on Dell XPS 13z
On Thu, 2012-04-26 at 22:07 +0200, Daniel Vetter wrote:
> ...
> To cut things short: This time around I want more justification for the
> quirk than just "this makes this one machine work somehow".
A bit more detail...
On this Dell XPS 13z "Ultrabook" (sandybridge_m 0x0116) when
intel_backlight/brightness gets stuffed with any value except 0 or
max_brightness, the backlight cycles between flashing (like a strobe
light!) and then pulsating (bright to dim to bright). That
flashing/pulsating cycle repeats continuously, about every 2 seconds.
The behavior is affected by the value stuffed into brightness more or
less along the lines of:
0: very dim, totally stable
1: flashes like crazy for about 1.5 sec, then pulsates for 0.5 sec
1000: flashes for about 0.5 sec, then pulsates for about 1.5 sec
2000: flashes very briefly, then pulsates for about 2 sec
3000: flickers, then pulsates for about 2 sec
4000 pulsates continuously, every 2 sec
4882: (max_brightness) full brightness, stable
This behavior manifests both in X and in a text VT, and occurs with or
without the presence of other backlight interfaces besides
intel_backlight. It does not appear to me to be a userspace problem.
The additional wrinkle is that this machine presents an acpi_video0
backlight interface as well, and it even works properly -- but *only*
after you specifically stuff 0 into intel_backlight/brightness (or if
intel_backlight is disabled by the proposed quirk).
Any non-zero intel_backlight/brightness value prevents acpi_video0 from
working. When intel_backlight is set to max_brightness (like at boot),
acpi_video0/brightness seems to have no effect at all; when
intel_backlight is set to other non-zero values, the flashing/pulsating
behavior occurs.
I'd be (quite) happy to test a proper intel_backlight fix, but in the
meantime disabling it by quirk seems appropriate for this machine, since
that allows the acpi_video0 interface to work out of the box.
Thanks for considering it,
-Kamal
On Wed, 2012-04-25 at 10:28 -0700, Kamal Mostafa wrote:
> From: Robert Hooker
>
> Dell XPS 13z exhibits problems (backlight flashing/pulsating) when
> intel_backlight is enabled at all, so disable it.
>
> BugLink: https://launchpad.net/bugs/954661
> Signed-off-by: Robert Hooker
> Signed-off-by: Kamal Mostafa
> ---
> drivers/gpu/drm/i915/intel_display.c | 17 +
> 1 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 5908cd5..0c4cac4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9100,6 +9100,19 @@ static void quirk_ssc_force_disable(struct drm_device
> *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> dev_priv->quirks |= QUIRK_LVDS_SSC_DISABLE;
> }
> +
> +/*
> + * Some machines (Dell XPS 13z) exhibit problems with i915 control of the
> + * backlight registers; Others may need the intel_backlight interface
> + * disabled for some other reason.
> + */
> +static void quirk_backlight_disable(struct drm_device *dev)
> +{
> + if (i915_enable_backlight == -1) {
> + i915_enable_backlight = 0;
> + DRM_DEBUG_DRIVER("disabling intel_backlight interface via quirk\n");
> + }
> +}
>
> struct intel_quirk {
> int device;
> @@ -9133,6 +9146,10 @@ struct intel_quirk intel_quirks[] = {
>
> /* Sony Vaio Y cannot use SSC on LVDS */
> { 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable },
> +
> + /* Dell XPS 13z needs to disable the intel_backlight interface
> +(LP: #954661) */
> + { 0x0116, 0x1028, 0x052e, quirk_backlight_disable },
> };
>
> static void intel_init_quirks(struct drm_device *dev)
___
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: i915.enable_backlight=0 disables intel_backlight
On Thu, 2012-04-26 at 22:07 +0200, Daniel Vetter wrote:
> On Wed, Apr 25, 2012 at 10:28:41AM -0700, Kamal Mostafa wrote:
> > i915.enable_backlight=0 can be used to disable i915 backlight control
> > and the /sys/class/backlight/intel_backlight interface -- useful for
> > systems where intel_backight conflicts with BIOS backlight control.
> >
> > BugLink: https://launchpad.net/bugs/954661
> > Signed-off-by: Kamal Mostafa
>
> Ok, I've just gone through the fun of merging a set of backlight quirks a
> few weeks back. Then noticed that an awful lot of machines seem to be
> affected and later on read about a few interesting bits in the
> documentation. Turns out the hw is all good, it's just the driver totally
> mishandling the backlight.
>
> To cut things short: This time around I want more justification for the
> quirk than just "this makes this one machine work somehow".
The enable_backlight modparam (this PATCH 1/2) would be generally useful
to have, whether or not we quirk-disable it for that particular machine.
Consider accepting this first patch 1/2 on its own?
-Kamal
> > ---
> > drivers/gpu/drm/i915/i915_drv.c|6 ++
> > drivers/gpu/drm/i915/i915_drv.h|1 +
> > drivers/gpu/drm/i915/intel_panel.c | 12
> > 3 files changed, 19 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index ae8a64f..ddb947b 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -112,6 +112,12 @@ module_param_named(i915_enable_ppgtt,
> > i915_enable_ppgtt, int, 0600);
> > MODULE_PARM_DESC(i915_enable_ppgtt,
> > "Enable PPGTT (default: true)");
> >
> > +int i915_enable_backlight __read_mostly = -1;
> > +module_param_named(enable_backlight, i915_enable_backlight, int, 0644);
> > +MODULE_PARM_DESC(enable_backlight,
> > + "Enable backlight control and the intel_backlight interface. "
> > + "(default: -1 (auto))");
> > +
> > static struct drm_driver driver;
> > extern int intel_agp_enabled;
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 5fabc6c..6e52a42 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1087,6 +1087,7 @@ extern int i915_enable_rc6 __read_mostly;
> > extern int i915_enable_fbc __read_mostly;
> > extern bool i915_enable_hangcheck __read_mostly;
> > extern int i915_enable_ppgtt __read_mostly;
> > +extern int i915_enable_backlight __read_mostly;
> >
> > extern int i915_suspend(struct drm_device *dev, pm_message_t state);
> > extern int i915_resume(struct drm_device *dev);
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c
> > b/drivers/gpu/drm/i915/intel_panel.c
> > index 48177ec..fcecbd2 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -259,6 +259,9 @@ void intel_panel_disable_backlight(struct drm_device
> > *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > + if (!i915_enable_backlight)
> > + return;
> > +
> > dev_priv->backlight_enabled = false;
> > intel_panel_actually_set_backlight(dev, 0);
> > }
> > @@ -267,6 +270,9 @@ void intel_panel_enable_backlight(struct drm_device
> > *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > + if (!i915_enable_backlight)
> > + return;
> > +
> > if (dev_priv->backlight_level == 0)
> > dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
> >
> > @@ -333,6 +339,9 @@ int intel_panel_setup_backlight(struct drm_device *dev)
> > struct backlight_properties props;
> > struct drm_connector *connector;
> >
> > + if (!i915_enable_backlight)
> > + return 0;
> > +
> > intel_panel_init_backlight(dev);
> >
> > if (dev_priv->int_lvds_connector)
> > @@ -368,6 +377,9 @@ void intel_panel_destroy_backlight(struct drm_device
> > *dev)
> > #else
> > int intel_panel_setup_backlight(struct drm_device *dev)
> > {
> > + if (!i915_enable_backlight)
> > + return;
> > +
> > intel_panel_init_backlight(dev);
> > return 0;
> > }
> > --
> > 1.7.5.4
> >
> > ___
> > 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] properly enable the blc controller on the right pipe
Daniel, Depending upon how things are set up, this might help. Yeah, thanks a lot, works great now! I removed patch 3/4 and 4/4 and pushed this one; so currently applied are: [PATCH 1/4] drm/i915: clear up backlight inversion confusion on gen4 [PATCH 2/4] drm/i915: completely revert the invert brightness quirk [PATCH] properly enable the blc controller on the right pipe Hey, that was just a quick hack to check things ;-) Sorry, too many things in parallel. Spent some time reading the docs you mentioned - will continue to work on the backlight brightness problem and prepare another patch. I just need some more time. Thanks, -Carsten. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Set the Stencil Cache eviction policy to non-LRA mode.
On Fri, Apr 27, 2012 at 12:55:12PM -0700, Kenneth Graunke wrote:
> Clearing bit 5 of CACHE_MODE_0 is necessary to prevent GPU hangs in
> OpenGL programs such as Google MapsGL, Google Earth, and gzdoom when
> using separate stencil buffers. Without it, the GPU tries to use the
> LRA eviction policy, which isn't supported. This was supposed to be off
> by default, but seems to be on for many machines.
>
> This cannot be done in gen6_init_clock_gating with most of the other
> workaround bits; the render ring needs to exist. Otherwise, the
> register write gets dropped on the floor (one printk will show it
> changed, but a second printk immediately following shows the value
> reverts to the old one).
>
> v2: Don't write the Gen6 registers on Gen7+.
>
> Cc: sta...@kernel.org
> Cc: Rob Castle
> Cc: Eric Appleman
> Cc: aaron...@gmx.net
> Cc: Keith Packard
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47535
> Signed-off-by: Kenneth Graunke
Reviewed-by: Daniel Vetter
Dave, would be great if you can include this patch into drm-fixes right
away. It fixes gpu hangs on various apps when using HiZ on reported by a
lot of ppl. And it would help if this lands earlier in sorting out some
conflicts with drm-intel-next.
Thanks, Daniel
> ---
> drivers/gpu/drm/i915/i915_reg.h |1 +
> drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++
> 2 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b4bb1ef..9d24d65 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -568,6 +568,7 @@
> #define CM0_MASK_SHIFT 16
> #define CM0_IZ_OPT_DISABLE (1<<6)
> #define CM0_ZR_OPT_DISABLE (1<<5)
> +#defineCM0_STC_EVICT_DISABLE_LRA_SNB (1<<5)
> #define CM0_DEPTH_EVICT_DISABLE (1<<4)
> #define CM0_COLOR_EVICT_DISABLE (1<<3)
> #define CM0_DEPTH_WRITE_DISABLE (1<<1)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index f75806e..af76723 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -398,6 +398,16 @@ static int init_render_ring(struct intel_ring_buffer
> *ring)
> return ret;
> }
>
> + if (IS_GEN6(dev)) {
> + /* From the Sandybridge PRM, volume 1 part 3, page 24:
> + * "If this bit is set, STCunit will have LRA as replacement
> + * policy. [...] This bit must be reset. LRA replacement
> + * policy is not supported."
> + */
> + I915_WRITE(CACHE_MODE_0,
> +CM0_STC_EVICT_DISABLE_LRA_SNB << CM0_MASK_SHIFT);
> + }
> +
> if (INTEL_INFO(dev)->gen >= 6) {
> I915_WRITE(INSTPM,
> INSTPM_FORCE_ORDERING << 16 | INSTPM_FORCE_ORDERING);
> --
> 1.7.10
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Set the Stencil Cache eviction policy to non-LRA mode.
Clearing bit 5 of CACHE_MODE_0 is necessary to prevent GPU hangs in
OpenGL programs such as Google MapsGL, Google Earth, and gzdoom when
using separate stencil buffers. Without it, the GPU tries to use the
LRA eviction policy, which isn't supported. This was supposed to be off
by default, but seems to be on for many machines.
This cannot be done in gen6_init_clock_gating with most of the other
workaround bits; the render ring needs to exist. Otherwise, the
register write gets dropped on the floor (one printk will show it
changed, but a second printk immediately following shows the value
reverts to the old one).
v2: Don't write the Gen6 registers on Gen7+.
Cc: sta...@kernel.org
Cc: Rob Castle
Cc: Eric Appleman
Cc: aaron...@gmx.net
Cc: Keith Packard
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47535
Signed-off-by: Kenneth Graunke
---
drivers/gpu/drm/i915/i915_reg.h |1 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++
2 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b4bb1ef..9d24d65 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -568,6 +568,7 @@
#define CM0_MASK_SHIFT 16
#define CM0_IZ_OPT_DISABLE (1<<6)
#define CM0_ZR_OPT_DISABLE (1<<5)
+#define CM0_STC_EVICT_DISABLE_LRA_SNB (1<<5)
#define CM0_DEPTH_EVICT_DISABLE (1<<4)
#define CM0_COLOR_EVICT_DISABLE (1<<3)
#define CM0_DEPTH_WRITE_DISABLE (1<<1)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index f75806e..af76723 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -398,6 +398,16 @@ static int init_render_ring(struct intel_ring_buffer *ring)
return ret;
}
+ if (IS_GEN6(dev)) {
+ /* From the Sandybridge PRM, volume 1 part 3, page 24:
+* "If this bit is set, STCunit will have LRA as replacement
+* policy. [...] This bit must be reset. LRA replacement
+* policy is not supported."
+*/
+ I915_WRITE(CACHE_MODE_0,
+ CM0_STC_EVICT_DISABLE_LRA_SNB << CM0_MASK_SHIFT);
+ }
+
if (INTEL_INFO(dev)->gen >= 6) {
I915_WRITE(INSTPM,
INSTPM_FORCE_ORDERING << 16 | INSTPM_FORCE_ORDERING);
--
1.7.10
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Set the Stencil Cache eviction policy to non-LRA mode.
Clearing bit 5 of CACHE_MODE_0 is necessary to prevent GPU hangs in
OpenGL programs such as Google MapsGL, Google Earth, and gzdoom when
using separate stencil buffers. Without it, the GPU tries to use the
LRA eviction policy, which isn't supported. This was supposed to be off
by default, but seems to be on for many machines.
This cannot be done in gen6_init_clock_gating with most of the other
workaround bits; the render ring needs to exist. Otherwise, the
register write gets dropped on the floor (one printk will show it
changed, but a second printk immediately following shows the value
reverts to the old one).
Cc: sta...@kernel.org
Cc: Rob Castle
Cc: Eric Appleman
Cc: aaron...@gmx.net
Cc: Keith Packard
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47535
Signed-off-by: Kenneth Graunke
---
drivers/gpu/drm/i915/i915_reg.h |1 +
drivers/gpu/drm/i915/intel_ringbuffer.c |8
2 files changed, 9 insertions(+)
Note that this patch applies against Daniel's drm-intel-fixes tree.
In drm-intel-next-queued, it may conflict with commit
009be664ecc77d58d3c27fb22347b969745a329a.
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b4bb1ef..9d24d65 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -568,6 +568,7 @@
#define CM0_MASK_SHIFT 16
#define CM0_IZ_OPT_DISABLE (1<<6)
#define CM0_ZR_OPT_DISABLE (1<<5)
+#define CM0_STC_EVICT_DISABLE_LRA_SNB (1<<5)
#define CM0_DEPTH_EVICT_DISABLE (1<<4)
#define CM0_COLOR_EVICT_DISABLE (1<<3)
#define CM0_DEPTH_WRITE_DISABLE (1<<1)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index f75806e..80fce51 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -401,6 +401,14 @@ static int init_render_ring(struct intel_ring_buffer *ring)
if (INTEL_INFO(dev)->gen >= 6) {
I915_WRITE(INSTPM,
INSTPM_FORCE_ORDERING << 16 | INSTPM_FORCE_ORDERING);
+
+ /* From the Sandybridge PRM, volume 1 part 3, page 24:
+* "If this bit is set, STCunit will have LRA as replacement
+* policy. [...] This bit must be reset. LRA replacement
+* policy is not supported."
+*/
+ I915_WRITE(CACHE_MODE_0,
+ CM0_STC_EVICT_DISABLE_LRA_SNB << CM0_MASK_SHIFT);
}
return ret;
--
1.7.10
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/10] drm/i915: kill gen4 gpu reset code
On Fri, Apr 27, 2012 at 11:49:16AM -0700, Eric Anholt wrote: > On Fri, 27 Apr 2012 15:17:47 +0200, Daniel Vetter > wrote: > > It's busted. Compared to ilk, snb and ivb where I've run the i-g-t > > hangman test in a loop for a few hours, my gm45 never managed to reset > > the gpu. And Chris Wilson confirmed on irc that he's never seen a case > > where the gen4 gpu reset code actually helped. > > I remember originally when the hangcheck stuff landed, on a 965gm > running compiz, dragging a window with a bad shader around and seeing a > stutter as it reset the hardware successfully before crashing soon after > (then turning off the hanging plugin and continuing to use the system). I'm confused - so hangcheck once indeed managed to reset a i965gm gpu or never? -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/10] drm/i915: kill gen4 gpu reset code
On Fri, 27 Apr 2012 15:17:47 +0200, Daniel Vetter wrote: > It's busted. Compared to ilk, snb and ivb where I've run the i-g-t > hangman test in a loop for a few hours, my gm45 never managed to reset > the gpu. And Chris Wilson confirmed on irc that he's never seen a case > where the gen4 gpu reset code actually helped. I remember originally when the hangcheck stuff landed, on a 965gm running compiz, dragging a window with a bad shader around and seeing a stutter as it reset the hardware successfully before crashing soon after (then turning off the hanging plugin and continuing to use the system). pgpoC7GqRiJ5w.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: enable semaphores on gen6 if dmar is not active
On Tue, Apr 17, 2012 at 11:31:00AM +0200, Daniel Vetter wrote: > On Tue, Apr 03, 2012 at 12:32:05AM +0200, Daniel Vetter wrote: > > On Mon, Apr 02, 2012 at 02:44:14PM -0700, Andrew Lutomirski wrote: > > > On Mon, Apr 2, 2012 at 1:52 PM, Daniel Vetter wrote: > > > > On Mon, Apr 02, 2012 at 01:45:39PM -0700, Andrew Lutomirski wrote: > > > I've had a hard time reproducing the problems lately. The > > > unconditional instant hard hang went away a few months ago, I think. > > > I'll give it another shot when I get home to that machine. > > > > Well, I've managed to kill my machine shortly after login with semaphores, > > rc6 and dmar enabled. It hasn't died in the same configuration after a few > > hours of light usage (in my case that seems to be the best killer) with > > dmar disabled for just the gpu. > > > > So if you can give your machine some serious beating with the different > > options, that's really great. > > Do you already have some preliminary results or has your machine not yet > died with this patch applied? Ping? -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: gen6_enable_rps() wants to be called after ring initialisation
Currently we call gen6_enable_rps() (which writes into the per-ring
register mmio space) from intel_modeset_init_hw() which is called before
we initialise the rings. If we defer intel_modeset_init_hw() until
afterwards (in the intel_modeset_gem_init() phase) all is well.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_display.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index be59e8f..27eda9a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6792,6 +6792,7 @@ void intel_modeset_init_hw(struct drm_device *dev)
if (IS_IRONLAKE_M(dev)) {
ironlake_enable_drps(dev);
+ ironlake_enable_rc6(dev);
intel_init_emon(dev);
}
@@ -6849,8 +6850,6 @@ void intel_modeset_init(struct drm_device *dev)
i915_disable_vga(dev);
intel_setup_outputs(dev);
- intel_modeset_init_hw(dev);
-
INIT_WORK(&dev_priv->idle_work, intel_idle_update);
setup_timer(&dev_priv->idle_timer, intel_gpu_idle_timer,
(unsigned long)dev);
@@ -6861,8 +6860,7 @@ void intel_modeset_init(struct drm_device *dev)
void intel_modeset_gem_init(struct drm_device *dev)
{
- if (IS_IRONLAKE_M(dev))
- ironlake_enable_rc6(dev);
+ intel_modeset_init_hw(dev);
intel_setup_overlay(dev);
}
--
1.7.10
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/4] drm/i915: Re: clear up backlight inversion confusion on gen4
Hello All, I have given one hardware that is D2plug of marvell .it's running completely .but now i have to write Test application in C launguge. (1) UART test in C (2) NANDflash test in C (3) microSD test in C (4) SATA test in C (5) Ethernet test in C so plz anybody can help me ...' ' Regards Monark ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/12 v2] drm/i915: wait render timeout ioctl
On Thu, 26 Apr 2012 16:03:08 -0700, Ben Widawsky wrote:
> This helps implement glClientWaitSync.
>
> Finally we can use the new timed seqno waiting function to allow
> userspace to wait on a request with a timeout. This implements that
> interface.
>
> The new ioctl is very straight forward, there is a flags field which I
> envision may be useful for various flush permutations of the command.
What are the semantics of the ioctl? A simple use case would help
specify the interface here.
In particular, I can't tell whether the return value (timeout_ns) is
meant to be the time elapsed or the time remaining. What value is
returned in the timeout if we are interrupted before the wait completes?
Would
end = gettimeofday() + timeout;
do {
ret = i915_gem_wait(handle, 0, &timeout);
} while (ret == -1 && errno == EINTR);
assert(gettimeofday() <= end);
wait forever, or until the original timeout expires?
-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 09/12] drm/i915: timeout parameter for seqno wait
On Thu, Apr 26, 2012 at 04:03:06PM -0700, Ben Widawsky wrote:
> Insert a wait parameter in the code so we can possibly timeout on a
> seqno wait if need be. The code should be functionally the same as
> before because all the callers will continue to retry if an arbitrary
> timeout elapses.
>
> We'd like to have nanosecond granularity, but the only way to do this is
> with hrtimer, and that doesn't fit well with the needs of this code.
>
> Signed-off-by: Ben Widawsky
I have to admit, I'm a bit unhappy with this swiss-army-tool wait_seqno
and what it looks like. What about copy&pasting __wait_seqno_timeout,
which is always interruptible?
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 58
> +++
> 1 file changed, 46 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 0ae1a73..f054439 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1820,10 +1820,23 @@ i915_gem_retire_work_handler(struct work_struct *work)
> }
>
> static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
> - bool interruptible)
> + bool interruptible, long *usecs)
> {
> drm_i915_private_t *dev_priv = ring->dev->dev_private;
> - int ret = 0;
> + bool wait_forever = false;
> + long timeout, end;
> +
> + if (usecs == NULL || ((*usecs) < 0)) {
> + wait_forever = true;
> + timeout = msecs_to_jiffies(100);
> + } else
> + timeout = usecs_to_jiffies(*usecs);
> +
> + if (i915_seqno_passed(ring->get_seqno(ring), seqno))
> + return 0;
> +
> + if (WARN_ON(!ring->irq_get(ring)))
> + return -ENODEV;
>
> if (i915_seqno_passed(ring->get_seqno(ring), seqno))
> return 0;
> @@ -1836,17 +1849,40 @@ static int __wait_seqno(struct intel_ring_buffer
> *ring, u32 seqno,
> (i915_seqno_passed(ring->get_seqno(ring), seqno) || \
> atomic_read(&dev_priv->mm.wedged))
>
> + trace_i915_gem_request_wait_begin(ring, seqno);
> +again:
> if (interruptible)
> - ret = wait_event_interruptible(ring->irq_queue,
> -EXIT_COND);
> + end = wait_event_interruptible_timeout(ring->irq_queue,
> +EXIT_COND,
> +timeout);
> else
> - wait_event(ring->irq_queue, EXIT_COND);
> + end = wait_event_timeout(ring->irq_queue, EXIT_COND, timeout);
> +#undef EXIT_COND
> +
> + if (atomic_read(&dev_priv->mm.wedged))
> + end = -EAGAIN;
> +
> + if (end == 0 && wait_forever)
> + goto again;
>
> - ring->irq_put(ring);
> trace_i915_gem_request_wait_end(ring, seqno);
> -#undef EXIT_COND
> + ring->irq_put(ring);
>
> - return ret;
> + if (!wait_forever) {
> + BUG_ON(end == 0);
> + *usecs = jiffies_to_usecs(timeout - end);
> + }
> +
> + switch (end) {
> + case -EAGAIN: /* Wedged */
> + case -ERESTARTSYS: /* Signal */
> + return (int)end;
> + case 0: /* Tiemout */
> + return -ETIME;
> + default: /* Completed */
> + WARN_ON(end < 0); /* We're not aware of other errors */
> + return 0;
> + }
> }
>
> /**
> @@ -1891,9 +1927,7 @@ i915_wait_request(struct intel_ring_buffer *ring,
> seqno = request->seqno;
> }
>
> - ret = __wait_seqno(ring, seqno, dev_priv->mm.interruptible);
> - if (atomic_read(&dev_priv->mm.wedged))
> - ret = -EAGAIN;
> + ret = __wait_seqno(ring, seqno, dev_priv->mm.interruptible, NULL);
>
> return ret;
> }
> @@ -2981,7 +3015,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct
> drm_file *file)
> if (seqno == 0)
> return 0;
>
> - ret = __wait_seqno(ring, seqno, true);
> + ret = __wait_seqno(ring, seqno, true, NULL);
> if (ret == 0)
> queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, 0);
>
> --
> 1.7.10
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/12 v2] drm/i915: use __wait_seqno for ring throttle
On Thu, Apr 26, 2012 at 04:03:05PM -0700, Ben Widawsky wrote: > It turns out throttle had an almost identical bit of code to do the > wait. Now we can call the new helper directly. This is just a bonus, > and not needed for the overall series. > > v2: remove irq_get/put which is now in __wait_seqno (Ben) > > Signed-off-by: Ben Widawsky Queued for -next up to this patch, thanks for the patches. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/12 v2] drm/i915: drop polled waits from i915_wait_request
On Thu, Apr 26, 2012 at 04:03:02PM -0700, Ben Widawsky wrote: > The only time irq_get should fail is during unload or suspend. Both of > these points should try to quiesce the GPU before disabling interrupts > and so the atomic polling should never occur. > > This was recommended by Chris Wilson as a way of reducing added > complexity to the polled wait which I introduced in an RFC patch. > > 09:57 < ickle_> it's only there as a fudge for waiting after irqs > after uninstalled during s&r, we aren't actually meant to hit it > 09:57 < ickle_> so maybe we should just kill the code there and fix the > breakage > > v2: return -ENODEV instead of -EBUSY when irq_get fails > > Signed-off-by: Ben Widawsky Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/12 v2] drm/i915: kill waiting_seqno
On Thu, Apr 26, 2012 at 04:03:01PM -0700, Ben Widawsky wrote: > The waiting_seqno is not terribly useful, and as such we can remove it > so that we'll be able to extract lockless code. > > v2: Keep the information for error_state (Chris) > Check if ring is initialized in hangcheck (Chris) > Capture the waiting ring (Chris) > > Signed-off-by: Ben Widawsky Merged with the comment for the ring->obj == NULL check in hangcheck_idle clarified, thanks for the patch. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/12 v2] drm/i915: move vbetool invoked ier stuff
On Thu, Apr 26, 2012 at 04:03:00PM -0700, Ben Widawsky wrote: > This extra bit of interrupt enabling code doesn't belong in the wait > seqno function. If anything we should pull it out to a helper so the > throttle code can also use it. The history is a bit vague, but I am > going to attempt to just dump it, unless someone can argue otherwise. > > Removing this allows for a shared lock free wait seqno function. To keep > tabs on this issue though, the IER value is stored on error capture > (recommended by Chris Wilson) > > v2: fixed typo EIR->IER (Ben) > Fix some white space (Ben) > Move IER capture to globally instead of per ring (Ben) > > Signed-off-by: Ben Widawsky Queued for -next with the small addition that ier is a 16bit reg on gen2, thanks for the patch. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/12 v2] drm/i915: remove do_retire from i915_wait_request
On Thu, Apr 26, 2012 at 04:02:58PM -0700, Ben Widawsky wrote: > This originates from a hack by me to quickly fix a bug in an earlier > patch where we needed control over whether or not waiting on a seqno > actually did any retire list processing. Since the two operations aren't > clearly related, we should pull the parameter out of the wait function, > and make the caller responsible for retiring if the action is desired. > > The only function call site which did not get an explicit retire_request call > (on purpose) is i915_gem_inactive_shrink(). That code was already calling > retire_request a second time. > > v2: don't modify any behavior excepit i915_gem_inactive_shrink(Daniel) > > Signed-off-by: Ben Widawsky Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/12] drm/i915: remove some extra retiring
On Thu, Apr 26, 2012 at 04:02:59PM -0700, Ben Widawsky wrote:
> Not every caller of gpu_idle needs to retire requests. Try to pick only
> the callers that need it. This was originally combined with the previous
> patch in the first series on the mailing list.
>
> Signed-off-by: Ben Widawsky
> ---
> drivers/gpu/drm/i915/i915_dma.c|1 -
> drivers/gpu/drm/i915/i915_gem.c|1 -
> drivers/gpu/drm/i915/i915_gem_execbuffer.c |1 -
> drivers/gpu/drm/i915/intel_overlay.c |2 --
> 4 files changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 36c8d5f..e73389d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -2016,7 +2016,6 @@ int i915_driver_unload(struct drm_device *dev)
> ret = i915_gpu_idle(dev);
> if (ret)
> DRM_ERROR("failed to idle hardware: %d\n", ret);
> - i915_gem_retire_requests(dev);
> mutex_unlock(&dev->struct_mutex);
>
> /* Cancel the retire work handler, which should be idle now. */
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 25be0e0..3b731ef 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3387,7 +3387,6 @@ i915_gem_idle(struct drm_device *dev)
> mutex_unlock(&dev->struct_mutex);
> return ret;
> }
> - i915_gem_retire_requests(dev);
Imo this will still blow up resume on gen2/3 ... we may not leak any
request over a s/r cycle in general, because on resume we restart with the
seqno counting at 1. So when you then want to wait for a no longer active,
but not yet retired request after resume, that will take a while.
I also miss a bit of justification for the first hunk, but module unload
calling gpu_idle instead of gem_idle is a bit fishy in and off itself.
I think the parts below are ok, but I'm hunting an overlay regression in
dinq atm, so I'll hold of merging this.
-Daniel
> /* Under UMS, be paranoid and evict. */
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index cbba0aa..582f6c4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1223,7 +1223,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
> *data,
> ret = i915_gpu_idle(dev);
> if (ret)
> goto err;
> - i915_gem_retire_requests(dev);
>
> BUG_ON(ring->sync_seqno[i]);
> }
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c
> b/drivers/gpu/drm/i915/intel_overlay.c
> index e06e46a..07a5cad 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -228,7 +228,6 @@ static int intel_overlay_do_wait_request(struct
> intel_overlay *overlay,
> ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
> if (ret)
> return ret;
> - i915_gem_retire_requests(dev);
>
> overlay->last_flip_req = 0;
> return 0;
> @@ -450,7 +449,6 @@ static int intel_overlay_recover_from_interrupt(struct
> intel_overlay *overlay)
> ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
> if (ret)
> return ret;
> - i915_gem_retire_requests(dev);
>
> if (overlay->flip_tail)
> overlay->flip_tail(overlay);
> --
> 1.7.10
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/12] drm/i915: timeout parameter for seqno wait
On Thu, 26 Apr 2012 16:03:06 -0700, Ben Widawsky wrote:
> static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
> - bool interruptible)
> + bool interruptible, long *usecs)
> {
> drm_i915_private_t *dev_priv = ring->dev->dev_private;
> - int ret = 0;
> + bool wait_forever = false;
> + long timeout, end;
> +
> + if (usecs == NULL || ((*usecs) < 0)) {
> + wait_forever = true;
> + timeout = msecs_to_jiffies(100);
> + } else
> + timeout = usecs_to_jiffies(*usecs);
> +
> + if (i915_seqno_passed(ring->get_seqno(ring), seqno))
> + return 0;
> +
> + if (WARN_ON(!ring->irq_get(ring)))
> + return -ENODEV;
Rebase error, duplicated ring->irq_get/ring->get_seqno.
-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] Updated -next
On Fri, Apr 27, 2012 at 01:45:42PM +0200, Daniel Vetter wrote: > Hi Dave, > > A new drm-intel-next pull. Highlights: > - More gmbus patches from Daniel Kurtz, I think gmbus is now ready, all > known issues fixed. > - Fencing cleanup and pipelined fencing removal from Chris. > - rc6 residency interface from Ben, useful for powertop. > - Cleanups and code reorg around the ringbuffer code (Ben&me). > - Use hw semaphores in the pageflip code from Ben. > - More vlv stuff from Jesse, unfortunately his vlv cpu is doa, so less > merged than I've hoped for - we still have the unused function warning :( > - More hsw patches from Eugeni, again, not yet enabled fully. > - intel_pm.c refactoring from Eugeni. > - Ironlake sprite support from Chris. > - And various smaller improvements/fixes all over the place. > > Note that this pull request also contains a backmerge of -rc3 to sort out > a few things in -next. I've also had to frob the shortlog a bit to exclude > anything that -rc3 brings in with this pull. > > Regression wise we have a few strange bugs going on, but for all of them > closer inspection revealed that they've been pre-existing, just now > slightly more likely to be hit. And for most of them we have a patch > already. Otherwise QA has not reported any regressions, and I'm also not > aware of anything bad happening in 3.4. > > For 3.4 Ken discovered that one of the snb workarounds in -next is > required to fix hangs in google maps and tons of other apps, so expect > another -fixes pull. > > Cheers, Daniel > > > The following changes since commit effbc4fd8e37e41d6f2bb6bcc611c14b4fbdcf9b: > > Merge branch 'drm-intel-next' of > git://people.freedesktop.org/~danvet/drm-intel into drm-core-next (2012-04-12 > 10:27:01 +0100) > > are available in the git repository at: > > > git://people.freedesktop.org/~danvet/drm-intel > tags/drm-intel-next-2012-04-23 I've forgot to mention that this branch has a very funky conflict with Linus' tree. Essentially it's just a few patches touching the same file (not even the same functions), but the git diff algo gets completely confused and thinks that the drm-intel-next branch completely rewrote large parts of intel_display.c. In truth we've just moved a few functions out of intel_display.c into intel_pm.c. But because the part in intel_display.c where git is all confused about is also changed in the 3.4 tree, and we have a merge conflict where none should be. drm-intel-testing has the merge resolution, in case anyone needs it. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/10] drm/i915: kill gen4 gpu reset code
It's busted. Compared to ilk, snb and ivb where I've run the i-g-t
hangman test in a loop for a few hours, my gm45 never managed to reset
the gpu. And Chris Wilson confirmed on irc that he's never seen a case
where the gen4 gpu reset code actually helped.
Given that it's not unlikely that this is causing harm, kill it.
Signed-Off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_drv.c | 37 -
1 files changed, 0 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7a09c28..0578c79 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -703,40 +703,6 @@ static int i8xx_do_reset(struct drm_device *dev)
return 0;
}
-static int i965_reset_complete(struct drm_device *dev)
-{
- u8 gdrst;
- pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst);
- return gdrst & 0x1;
-}
-
-static int i965_do_reset(struct drm_device *dev)
-{
- int ret;
- u8 gdrst;
-
- /*
-* Set the domains we want to reset (GRDOM/bits 2 and 3) as
-* well as the reset bit (GR/bit 0). Setting the GR bit
-* triggers the reset; when done, the hardware will clear it.
-*/
- pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst);
- pci_write_config_byte(dev->pdev, I965_GDRST,
- gdrst | GRDOM_RENDER |
- GRDOM_RESET_ENABLE);
- ret = wait_for(i965_reset_complete(dev), 500);
- if (ret)
- return ret;
-
- /* We can't reset render&media without also resetting display ... */
- pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst);
- pci_write_config_byte(dev->pdev, I965_GDRST,
- gdrst | GRDOM_MEDIA |
- GRDOM_RESET_ENABLE);
-
- return wait_for(i965_reset_complete(dev), 500);
-}
-
static int ironlake_do_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -805,9 +771,6 @@ static int intel_gpu_reset(struct drm_device *dev)
case 5:
ret = ironlake_do_reset(dev);
break;
- case 4:
- ret = i965_do_reset(dev);
- break;
case 2:
ret = i8xx_do_reset(dev);
break;
--
1.7.9
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/10] drm/i915: remove modeset reset from i915_reset
On gen4+ we don't reset the display unit, so resetting the complete
modeset state should not be necessary.
We can't do reset on gen3 anyway, which leaves us with gen2 reset:
According to Chris Wilson, that doesn't work so great, so he suggested
we just ignore that. If the need ever arrises, we can re-add it later
on.
Signed-Off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_drv.c | 10 --
1 files changed, 0 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0c97b9b..7a09c28 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -902,21 +902,11 @@ int i915_reset(struct drm_device *dev)
intel_modeset_init_hw(dev);
drm_irq_uninstall(dev);
- drm_mode_config_reset(dev);
drm_irq_install(dev);
} else {
mutex_unlock(&dev->struct_mutex);
}
- /*
-* Perform a full modeset as on later generations, e.g. Ironlake, we may
-* need to retrain the display link and cannot just restore the register
-* values.
-*/
- mutex_lock(&dev->mode_config.mutex);
- drm_helper_resume_force_mode(dev);
- mutex_unlock(&dev->mode_config.mutex);
-
return 0;
}
--
1.7.9
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/10] drm/i915: also reset the media engine on gen4/5
... we actually use it.
Unfortunately we can't reset both at the same time without also
resetting the display unit, so do render and media separately.
Also replace magic constants with proper #defines.
Signed-Off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_drv.c | 28 +---
drivers/gpu/drm/i915/i915_reg.h |1 +
2 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2ce4c0e..0c97b9b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -712,6 +712,7 @@ static int i965_reset_complete(struct drm_device *dev)
static int i965_do_reset(struct drm_device *dev)
{
+ int ret;
u8 gdrst;
/*
@@ -721,7 +722,17 @@ static int i965_do_reset(struct drm_device *dev)
*/
pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst);
pci_write_config_byte(dev->pdev, I965_GDRST,
- gdrst | GRDOM_RENDER | 0x1);
+ gdrst | GRDOM_RENDER |
+ GRDOM_RESET_ENABLE);
+ ret = wait_for(i965_reset_complete(dev), 500);
+ if (ret)
+ return ret;
+
+ /* We can't reset render&media without also resetting display ... */
+ pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst);
+ pci_write_config_byte(dev->pdev, I965_GDRST,
+ gdrst | GRDOM_MEDIA |
+ GRDOM_RESET_ENABLE);
return wait_for(i965_reset_complete(dev), 500);
}
@@ -729,9 +740,20 @@ static int i965_do_reset(struct drm_device *dev)
static int ironlake_do_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- u32 gdrst = I915_READ(MCHBAR_MIRROR_BASE + ILK_GDSR);
+ u32 gdrst;
+ int ret;
+
+ gdrst = I915_READ(MCHBAR_MIRROR_BASE + ILK_GDSR);
+ I915_WRITE(MCHBAR_MIRROR_BASE + ILK_GDSR,
+ gdrst | GRDOM_RENDER | GRDOM_RESET_ENABLE);
+ ret = wait_for(I915_READ(MCHBAR_MIRROR_BASE + ILK_GDSR) & 0x1, 500);
+ if (ret)
+ return ret;
+
+ /* We can't reset render&media without also resetting display ... */
+ gdrst = I915_READ(MCHBAR_MIRROR_BASE + ILK_GDSR);
I915_WRITE(MCHBAR_MIRROR_BASE + ILK_GDSR,
- gdrst | GRDOM_RENDER | 0x1);
+ gdrst | GRDOM_MEDIA | GRDOM_RESET_ENABLE);
return wait_for(I915_READ(MCHBAR_MIRROR_BASE + ILK_GDSR) & 0x1, 500);
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f1f4d8f..28aca90 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -82,6 +82,7 @@
#define GRDOM_FULL(0<<2)
#define GRDOM_RENDER (1<<2)
#define GRDOM_MEDIA (3<<2)
+#define GRDOM_RESET_ENABLE (1<<0)
#define GEN6_MBCUNIT_SNPCR 0x900c /* for LLC config */
#define GEN6_MBC_SNPCR_SHIFT 21
--
1.7.9
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/10] drm/i915: kill flags parameter for reset functions
Only half of them even cared, and it's always the same one.
Signed-Off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_drv.c | 29 +++--
drivers/gpu/drm/i915/i915_drv.h |2 +-
drivers/gpu/drm/i915/i915_irq.c |2 +-
3 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ab70a1a..2ce4c0e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -673,7 +673,7 @@ int i915_resume(struct drm_device *dev)
return 0;
}
-static int i8xx_do_reset(struct drm_device *dev, u8 flags)
+static int i8xx_do_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -710,7 +710,7 @@ static int i965_reset_complete(struct drm_device *dev)
return gdrst & 0x1;
}
-static int i965_do_reset(struct drm_device *dev, u8 flags)
+static int i965_do_reset(struct drm_device *dev)
{
u8 gdrst;
@@ -720,20 +720,22 @@ static int i965_do_reset(struct drm_device *dev, u8 flags)
* triggers the reset; when done, the hardware will clear it.
*/
pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst);
- pci_write_config_byte(dev->pdev, I965_GDRST, gdrst | flags | 0x1);
+ pci_write_config_byte(dev->pdev, I965_GDRST,
+ gdrst | GRDOM_RENDER | 0x1);
return wait_for(i965_reset_complete(dev), 500);
}
-static int ironlake_do_reset(struct drm_device *dev, u8 flags)
+static int ironlake_do_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
u32 gdrst = I915_READ(MCHBAR_MIRROR_BASE + ILK_GDSR);
- I915_WRITE(MCHBAR_MIRROR_BASE + ILK_GDSR, gdrst | flags | 0x1);
+ I915_WRITE(MCHBAR_MIRROR_BASE + ILK_GDSR,
+ gdrst | GRDOM_RENDER | 0x1);
return wait_for(I915_READ(MCHBAR_MIRROR_BASE + ILK_GDSR) & 0x1, 500);
}
-static int gen6_do_reset(struct drm_device *dev, u8 flags)
+static int gen6_do_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
@@ -768,7 +770,7 @@ static int gen6_do_reset(struct drm_device *dev, u8 flags)
return ret;
}
-static int intel_gpu_reset(struct drm_device *dev, u8 flags)
+static int intel_gpu_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
int ret = -ENODEV;
@@ -776,16 +778,16 @@ static int intel_gpu_reset(struct drm_device *dev, u8
flags)
switch (INTEL_INFO(dev)->gen) {
case 7:
case 6:
- ret = gen6_do_reset(dev, flags);
+ ret = gen6_do_reset(dev);
break;
case 5:
- ret = ironlake_do_reset(dev, flags);
+ ret = ironlake_do_reset(dev);
break;
case 4:
- ret = i965_do_reset(dev, flags);
+ ret = i965_do_reset(dev);
break;
case 2:
- ret = i8xx_do_reset(dev, flags);
+ ret = i8xx_do_reset(dev);
break;
}
@@ -806,7 +808,6 @@ static int intel_gpu_reset(struct drm_device *dev, u8 flags)
/**
* i915_reset - reset chip after a hang
* @dev: drm device to reset
- * @flags: reset domains
*
* Reset the chip. Useful if a hang is detected. Returns zero on successful
* reset or otherwise an error code.
@@ -819,7 +820,7 @@ static int intel_gpu_reset(struct drm_device *dev, u8 flags)
* - re-init interrupt state
* - re-init display
*/
-int i915_reset(struct drm_device *dev, u8 flags)
+int i915_reset(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = dev->dev_private;
int ret;
@@ -836,7 +837,7 @@ int i915_reset(struct drm_device *dev, u8 flags)
if (get_seconds() - dev_priv->last_gpu_reset < 5)
DRM_ERROR("GPU hanging too fast, declaring wedged!\n");
else
- ret = intel_gpu_reset(dev, flags);
+ ret = intel_gpu_reset(dev);
dev_priv->last_gpu_reset = get_seconds();
if (ret) {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2aaa594..fd61500 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1143,7 +1143,7 @@ extern long i915_compat_ioctl(struct file *filp, unsigned
int cmd,
extern int i915_emit_box(struct drm_device *dev,
struct drm_clip_rect *box,
int DR1, int DR4);
-extern int i915_reset(struct drm_device *dev, u8 flags);
+extern int i915_reset(struct drm_device *dev);
extern unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
extern unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b6c02eb..d7e68da 100644
--- a/dr
[Intel-gfx] [PATCH 06/10] drm/i915: make gpu hangman more resilient
- reset the stop_rings infrastructure while resetting the hw to
avoid angering the hangcheck right away (and potentially declaring
the gpu permanently wedged).
- ignore reset failures when hanging due to the hangman - we don't
have reset code for all generations.
v2: Ensure that we only ignore reset failures when the hw reset is not
implemented and not when it failed.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_drv.c | 12
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c4251a1..ab70a1a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -770,6 +770,7 @@ static int gen6_do_reset(struct drm_device *dev, u8 flags)
static int intel_gpu_reset(struct drm_device *dev, u8 flags)
{
+ struct drm_i915_private *dev_priv = dev->dev_private;
int ret = -ENODEV;
switch (INTEL_INFO(dev)->gen) {
@@ -788,6 +789,17 @@ static int intel_gpu_reset(struct drm_device *dev, u8
flags)
break;
}
+ /* Also reset the gpu hangman. */
+ if (dev_priv->stop_rings) {
+ DRM_DEBUG("Simulated gpu hang, resetting stop_rings\n");
+ dev_priv->stop_rings = 0;
+ if (ret == -ENODEV) {
+ DRM_ERROR("Reset not implemented, but ignoring "
+ "error for simulated gpu hangs\n");
+ ret = 0;
+ }
+ }
+
return ret;
}
--
1.7.9
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/10] drm/i915: extract intel_gpu_reset
Slightly cleans up the code and could be useful for e.g. Ben
Widawsky's hw context patches.
v2: New colours!
Cc: Ben Widawsky
Signed-Off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_drv.c | 43 --
1 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f5450bb..c4251a1 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -768,6 +768,29 @@ static int gen6_do_reset(struct drm_device *dev, u8 flags)
return ret;
}
+static int intel_gpu_reset(struct drm_device *dev, u8 flags)
+{
+ int ret = -ENODEV;
+
+ switch (INTEL_INFO(dev)->gen) {
+ case 7:
+ case 6:
+ ret = gen6_do_reset(dev, flags);
+ break;
+ case 5:
+ ret = ironlake_do_reset(dev, flags);
+ break;
+ case 4:
+ ret = i965_do_reset(dev, flags);
+ break;
+ case 2:
+ ret = i8xx_do_reset(dev, flags);
+ break;
+ }
+
+ return ret;
+}
+
/**
* i915_reset - reset chip after a hang
* @dev: drm device to reset
@@ -798,23 +821,11 @@ int i915_reset(struct drm_device *dev, u8 flags)
i915_gem_reset(dev);
ret = -ENODEV;
- if (get_seconds() - dev_priv->last_gpu_reset < 5) {
+ if (get_seconds() - dev_priv->last_gpu_reset < 5)
DRM_ERROR("GPU hanging too fast, declaring wedged!\n");
- } else switch (INTEL_INFO(dev)->gen) {
- case 7:
- case 6:
- ret = gen6_do_reset(dev, flags);
- break;
- case 5:
- ret = ironlake_do_reset(dev, flags);
- break;
- case 4:
- ret = i965_do_reset(dev, flags);
- break;
- case 2:
- ret = i8xx_do_reset(dev, flags);
- break;
- }
+ else
+ ret = intel_gpu_reset(dev, flags);
+
dev_priv->last_gpu_reset = get_seconds();
if (ret) {
DRM_ERROR("Failed to reset chip.\n");
--
1.7.9
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/10] drm/i915: simplify i915_reset a bit
- need_display is always true, scrap it.
- don't reacquire the mutex to do nothing after having restored the
gem state.
Signed-Off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_drv.c | 19 +--
1 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8a98f9a..f5450bb 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -787,11 +787,6 @@ static int gen6_do_reset(struct drm_device *dev, u8 flags)
int i915_reset(struct drm_device *dev, u8 flags)
{
drm_i915_private_t *dev_priv = dev->dev_private;
- /*
-* We really should only reset the display subsystem if we actually
-* need to
-*/
- bool need_display = true;
int ret;
if (!i915_try_reset)
@@ -863,22 +858,18 @@ int i915_reset(struct drm_device *dev, u8 flags)
drm_irq_uninstall(dev);
drm_mode_config_reset(dev);
drm_irq_install(dev);
-
- mutex_lock(&dev->struct_mutex);
+ } else {
+ mutex_unlock(&dev->struct_mutex);
}
- mutex_unlock(&dev->struct_mutex);
-
/*
* Perform a full modeset as on later generations, e.g. Ironlake, we may
* need to retrain the display link and cannot just restore the register
* values.
*/
- if (need_display) {
- mutex_lock(&dev->mode_config.mutex);
- drm_helper_resume_force_mode(dev);
- mutex_unlock(&dev->mode_config.mutex);
- }
+ mutex_lock(&dev->mode_config.mutex);
+ drm_helper_resume_force_mode(dev);
+ mutex_unlock(&dev->mode_config.mutex);
return 0;
}
--
1.7.9
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/10] drm/i915: allow the existing error_state to be destroyed
... by writing (anything) to i915_error_state.
This way we can simulate a bunch of gpu hangs and run the error_state
capture code every time (without the need to reload the module).
To make that happen we need to abandon the simple seq_file wrappers
provided by the drm core. While at it, but the new error_state
refcounting to some good use and associated. This should help greatly
when we finally get around to split up the giant single seq_file block
that the error_state file currently is into smaller parts.
v2: Actually squash all the fixes into the patch ...
Signed-Off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_debugfs.c | 90 ++-
1 files changed, 77 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index b46c198..4805df0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -691,21 +691,19 @@ static void i915_ring_error_state(struct seq_file *m,
seq_printf(m, " ring->tail: 0x%08x\n", error->cpu_ring_tail[ring]);
}
+struct i915_error_state_file_priv {
+ struct drm_device *dev;
+ struct drm_i915_error_state *error;
+};
+
static int i915_error_state(struct seq_file *m, void *unused)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
- struct drm_device *dev = node->minor->dev;
+ struct i915_error_state_file_priv *error_priv = m->private;
+ struct drm_device *dev = error_priv->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
- struct drm_i915_error_state *error;
- unsigned long flags;
+ struct drm_i915_error_state *error = error_priv->error;
int i, j, page, offset, elt;
- spin_lock_irqsave(&dev_priv->error_lock, flags);
- error = dev_priv->first_error;
- if (error)
- kref_get(&error->ref);
- spin_unlock_irqrestore(&dev_priv->error_lock, flags);
-
if (!error) {
seq_printf(m, "no error state collected\n");
return 0;
@@ -792,11 +790,71 @@ static int i915_error_state(struct seq_file *m, void
*unused)
if (error->display)
intel_display_print_error_state(m, dev, error->display);
- kref_put(&error->ref, i915_error_state_free);
-
return 0;
}
+static ssize_t
+i915_error_state_write(struct file *filp,
+ const char __user *ubuf,
+ size_t cnt,
+ loff_t *ppos)
+{
+ struct seq_file *m = filp->private_data;
+ struct i915_error_state_file_priv *error_priv = m->private;
+ struct drm_device *dev = error_priv->dev;
+
+ DRM_DEBUG_DRIVER("Resetting error state\n");
+
+ mutex_lock(&dev->struct_mutex);
+ i915_destroy_error_state(dev);
+ mutex_unlock(&dev->struct_mutex);
+
+ return cnt;
+}
+
+static int i915_error_state_open(struct inode *inode, struct file *file)
+{
+ struct drm_device *dev = inode->i_private;
+ drm_i915_private_t *dev_priv = dev->dev_private;
+ struct i915_error_state_file_priv *error_priv;
+ unsigned long flags;
+
+ error_priv = kzalloc(sizeof(*error_priv), GFP_KERNEL);
+ if (!error_priv)
+ return -ENOMEM;
+
+ error_priv->dev = dev;
+
+ spin_lock_irqsave(&dev_priv->error_lock, flags);
+ error_priv->error = dev_priv->first_error;
+ if (error_priv->error)
+ kref_get(&error_priv->error->ref);
+ spin_unlock_irqrestore(&dev_priv->error_lock, flags);
+
+ return single_open(file, i915_error_state, error_priv);
+}
+
+static int i915_error_state_release(struct inode *inode, struct file *file)
+{
+ struct seq_file *m = file->private_data;
+ struct i915_error_state_file_priv *error_priv = m->private;
+
+ if (error_priv->error)
+ kref_put(&error_priv->error->ref, i915_error_state_free);
+ kfree(error_priv);
+
+ return single_release(inode, file);
+}
+
+static const struct file_operations i915_error_state_fops = {
+ .owner = THIS_MODULE,
+ .open = i915_error_state_open,
+ .read = seq_read,
+ .write = i915_error_state_write,
+ .llseek = default_llseek,
+ .release = i915_error_state_release,
+};
+
static int i915_rstdby_delays(struct seq_file *m, void *unused)
{
struct drm_info_node *node = (struct drm_info_node *) m->private;
@@ -1645,6 +1703,7 @@ static const struct file_operations i915_ring_stop_fops =
{
.write = i915_ring_stop_write,
.llseek = default_llseek,
};
+
static ssize_t
i915_max_freq_read(struct file *filp,
char __user *ubuf,
@@ -1899,7 +1958,6 @@ static struct drm_info_list i915_debugfs_list[] = {
{"i915_gem_hws", i915_hws_info, 0, (void *)RCS},
{"i915_gem_hws_blt", i915_hws_info, 0, (void *)BCS},
{"i915_gem_hws_bsd", i915_hws_info, 0, (void *)VCS},
- {"i915_error_state",
[Intel-gfx] [PATCH 02/10] drm/i915: rework dev->first_error locking
- reduce the irq disabled section, even for a debugfs file this was
way too long.
- always disable irqs when taking the lock.
v2: Thou shalt not mistake locking for reference counting, so:
- reference count the error_state to protect from concurent freeeing.
This will be only really used in the next patch.
Signed-Off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_debugfs.c | 13 -
drivers/gpu/drm/i915/i915_drv.h |4
drivers/gpu/drm/i915/i915_irq.c | 12 +++-
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3f6e28e..b46c198 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -701,12 +701,16 @@ static int i915_error_state(struct seq_file *m, void
*unused)
int i, j, page, offset, elt;
spin_lock_irqsave(&dev_priv->error_lock, flags);
- if (!dev_priv->first_error) {
+ error = dev_priv->first_error;
+ if (error)
+ kref_get(&error->ref);
+ spin_unlock_irqrestore(&dev_priv->error_lock, flags);
+
+ if (!error) {
seq_printf(m, "no error state collected\n");
- goto out;
+ return 0;
}
- error = dev_priv->first_error;
seq_printf(m, "Time: %ld s %ld us\n", error->time.tv_sec,
error->time.tv_usec);
@@ -788,8 +792,7 @@ static int i915_error_state(struct seq_file *m, void
*unused)
if (error->display)
intel_display_print_error_state(m, dev, error->display);
-out:
- spin_unlock_irqrestore(&dev_priv->error_lock, flags);
+ kref_put(&error->ref, i915_error_state_free);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5e62b35..2aaa594 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -39,6 +39,7 @@
#include
#include
#include
+#include
/* General customization:
*/
@@ -171,6 +172,7 @@ struct sdvo_device_mapping {
struct intel_display_error_state;
struct drm_i915_error_state {
+ struct kref ref;
u32 eir;
u32 pgtbl_er;
u32 pipestat[I915_MAX_PIPES];
@@ -466,6 +468,7 @@ typedef struct drm_i915_private {
unsigned int fsb_freq, mem_freq, is_ddr3;
spinlock_t error_lock;
+ /* Protected by dev->error_lock. */
struct drm_i915_error_state *first_error;
struct work_struct error_work;
struct completion error_completion;
@@ -1163,6 +1166,7 @@ extern int i915_vblank_pipe_get(struct drm_device *dev,
void *data,
struct drm_file *file_priv);
extern int i915_vblank_swap(struct drm_device *dev, void *data,
struct drm_file *file_priv);
+void i915_error_state_free(struct kref *error_ref);
void
i915_enable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5c360ee..b6c02eb 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -869,10 +869,11 @@ i915_error_object_free(struct drm_i915_error_object *obj)
kfree(obj);
}
-static void
-i915_error_state_free(struct drm_device *dev,
- struct drm_i915_error_state *error)
+void
+i915_error_state_free(struct kref *error_ref)
{
+ struct drm_i915_error_state *error = container_of(error_ref,
+ typeof(*error), ref);
int i;
for (i = 0; i < ARRAY_SIZE(error->ring); i++) {
@@ -1120,6 +1121,7 @@ static void i915_capture_error_state(struct drm_device
*dev)
DRM_INFO("capturing error event; look for more information in
/debug/dri/%d/i915_error_state\n",
dev->primary->index);
+ kref_init(&error->ref);
error->eir = I915_READ(EIR);
error->pgtbl_er = I915_READ(PGTBL_ER);
for_each_pipe(pipe)
@@ -1181,7 +1183,7 @@ static void i915_capture_error_state(struct drm_device
*dev)
spin_unlock_irqrestore(&dev_priv->error_lock, flags);
if (error)
- i915_error_state_free(dev, error);
+ i915_error_state_free(&error->ref);
}
void i915_destroy_error_state(struct drm_device *dev)
@@ -1196,7 +1198,7 @@ void i915_destroy_error_state(struct drm_device *dev)
spin_unlock_irqrestore(&dev_priv->error_lock, flags);
if (error)
- i915_error_state_free(dev, error);
+ kref_put(&error->ref, i915_error_state_free);
}
#else
#define i915_capture_error_state(x)
--
1.7.9
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/10] drm/i915: add interface to simulate gpu hangs
gpu reset is a very important piece of our infrastructure.
Unfortunately we only really it test by actually hanging the gpu,
which often has bad side-effects for the entire system. And the gpu
hang handling code is one of the rather complicated pieces of code we
have, consisting of
- hang detection
- error capture
- actual gpu reset
- reset of all the gem bookkeeping
- reinitialition of the entire gpu
This patch adds a debugfs to selectively stopping rings by ceasing to
update the hw tail pointer, which will result in the gpu no longer
updating it's head pointer and eventually to the hangcheck firing.
This way we can exercise the gpu hang code under controlled conditions
without a dying gpu taking down the entire systems.
Patch motivated by me forgetting to properly reinitialize ppgtt after
a gpu reset.
Usage:
echo $((1 << $ringnum)) > i915_ring_stop # stops one ring
echo 0x > i915_ring_stop # stops all, future-proof version
then run whatever testload is desired. i915_ring_stop automatically
resets after a gpu hang is detected to avoid hanging the gpu to fast
and declaring it wedged.
v2: Incorporate feedback from Chris Wilson.
v3: Add the missing cleanup.
Signed-Off-by: Daniel Vetter
Reviewed-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_debugfs.c | 65 +++
drivers/gpu/drm/i915/i915_drv.h |2 +
drivers/gpu/drm/i915/intel_ringbuffer.c |4 ++
3 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 120db46..3f6e28e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1585,6 +1585,64 @@ static const struct file_operations i915_wedged_fops = {
};
static ssize_t
+i915_ring_stop_read(struct file *filp,
+ char __user *ubuf,
+ size_t max,
+ loff_t *ppos)
+{
+ struct drm_device *dev = filp->private_data;
+ drm_i915_private_t *dev_priv = dev->dev_private;
+ char buf[80];
+ int len;
+
+ len = snprintf(buf, sizeof(buf),
+ "0x%08x\n", dev_priv->stop_rings);
+
+ if (len > sizeof(buf))
+ len = sizeof(buf);
+
+ return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+}
+
+static ssize_t
+i915_ring_stop_write(struct file *filp,
+const char __user *ubuf,
+size_t cnt,
+loff_t *ppos)
+{
+ struct drm_device *dev = filp->private_data;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ char buf[20];
+ int val = 0;
+
+ if (cnt > 0) {
+ if (cnt > sizeof(buf) - 1)
+ return -EINVAL;
+
+ if (copy_from_user(buf, ubuf, cnt))
+ return -EFAULT;
+ buf[cnt] = 0;
+
+ val = simple_strtoul(buf, NULL, 0);
+ }
+
+ DRM_DEBUG_DRIVER("Stopping rings 0x%08x\n", val);
+
+ mutex_lock(&dev->struct_mutex);
+ dev_priv->stop_rings = val;
+ mutex_unlock(&dev->struct_mutex);
+
+ return cnt;
+}
+
+static const struct file_operations i915_ring_stop_fops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .read = i915_ring_stop_read,
+ .write = i915_ring_stop_write,
+ .llseek = default_llseek,
+};
+static ssize_t
i915_max_freq_read(struct file *filp,
char __user *ubuf,
size_t max,
@@ -1884,6 +1942,11 @@ int i915_debugfs_init(struct drm_minor *minor)
&i915_cache_sharing_fops);
if (ret)
return ret;
+ ret = i915_debugfs_create(minor->debugfs_root, minor,
+ "i915_ring_stop",
+ &i915_ring_stop_fops);
+ if (ret)
+ return ret;
return drm_debugfs_create_files(i915_debugfs_list,
I915_DEBUGFS_ENTRIES,
@@ -1902,6 +1965,8 @@ void i915_debugfs_cleanup(struct drm_minor *minor)
1, minor);
drm_debugfs_remove_files((struct drm_info_list *)
&i915_cache_sharing_fops,
1, minor);
+ drm_debugfs_remove_files((struct drm_info_list *) &i915_ring_stop_fops,
+1, minor);
}
#endif /* CONFIG_DEBUG_FS */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0095c8d..5e62b35 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -413,6 +413,8 @@ typedef struct drm_i915_private {
uint32_t last_instdone;
uint32_t last_instdone1;
+ unsigned int stop_rings;
+
unsigned long cfb_size;
unsigned int cfb_fb;
enum plane cfb_plane;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 427b7c5..d2de16d 100644
Re: [Intel-gfx] Updated -next
Hi Dave, A new drm-intel-next pull. Highlights: - More gmbus patches from Daniel Kurtz, I think gmbus is now ready, all known issues fixed. - Fencing cleanup and pipelined fencing removal from Chris. - rc6 residency interface from Ben, useful for powertop. - Cleanups and code reorg around the ringbuffer code (Ben&me). - Use hw semaphores in the pageflip code from Ben. - More vlv stuff from Jesse, unfortunately his vlv cpu is doa, so less merged than I've hoped for - we still have the unused function warning :( - More hsw patches from Eugeni, again, not yet enabled fully. - intel_pm.c refactoring from Eugeni. - Ironlake sprite support from Chris. - And various smaller improvements/fixes all over the place. Note that this pull request also contains a backmerge of -rc3 to sort out a few things in -next. I've also had to frob the shortlog a bit to exclude anything that -rc3 brings in with this pull. Regression wise we have a few strange bugs going on, but for all of them closer inspection revealed that they've been pre-existing, just now slightly more likely to be hit. And for most of them we have a patch already. Otherwise QA has not reported any regressions, and I'm also not aware of anything bad happening in 3.4. For 3.4 Ken discovered that one of the snb workarounds in -next is required to fix hangs in google maps and tons of other apps, so expect another -fixes pull. Cheers, Daniel The following changes since commit effbc4fd8e37e41d6f2bb6bcc611c14b4fbdcf9b: Merge branch 'drm-intel-next' of git://people.freedesktop.org/~danvet/drm-intel into drm-core-next (2012-04-12 10:27:01 +0100) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel tags/drm-intel-next-2012-04-23 for you to fetch changes up to a85d4bcb8a0cd5b3c754f98ff91ef2b9b3a73bc5: drm/i915: rc6 residency (fix the fix) (2012-04-23 09:30:14 +0200) Armin Reese (1): drm/i915: Mask reserved bits in display/sprite address registers Ben Widawsky (10): drm/i915: add rc6 residency times to debugfs drm/i915: use semaphores for the display plane drm/i915: rc6 in sysfs drm/i915: i915_gem_object_sync must handle NULL drm/i915: fix for when semaphore updates fail drm/i915: hide (seqno-1) in ringbuffer code drm/i915: [sparse] trivial sparse fixes drm/i915: [sparse] don't use variable size arrays drm/i915: [GEN7] Use HW scheduler for fixed function shaders drm/i915: rc6 residency (fix the fix) Chris Wilson (25): drm/i915: Reorganise rules for get_fence/put_fence drm/i915: Ironlake shares the same video sprite controls as Sandybridge drm/i915: Allow concurrent read access between CPU and GPU domain drm/i915: Trigger hangcheck if we detect more a repeating missed IRQ drm/i915: Refactor the deferred PM_IIR handling into a single function drm/i915: Export the generic, not arch specific, intel_update_watermarks() drm/i915/sprite: Always enable the scaler on IronLake drm/i915/dp: Flush any outstanding work to turn the VDD off drm/i915: Always flush tiling changes before accessing through the GTT drm/i915: Replace open coded MI_BATCH_GTT drm/i915: Unpin the flip target if we fail to queue the flip drm/i915: intel_update_fbc() requires struct_mutex, so no longer atomic drm/i915: Don't set a MBZ bit in gen2/3 MI_FLUSH drm/i915: Wait for all pending operations to the fb before disabling the pipe drm/i915: Remove the pipelined parameter from get_fence() drm/i915: Remove fence pipelining drm/i915: Remove unused ring->setup_seqno drm/i915: Discard the unused obj->last_fenced_ring drm/i915: Simplify fence finding drm/i915: Remove the unsightly "optimisation" from flush_fence() drm/i915: Prepare to consolidate fence writing drm/i915: Refactor put_fence() to use the common fence writing routine drm/i915: Refactor fence clearing to use the common fence writing routine drm/i915: Refactor get_fence() to use the common fence writing routine drm/i915: Silence the change of LVDS sync polarity Daniel Kurtz (9): drm/i915/intel_i2c: handle zero-length writes drm/i915/intel_i2c: use double-buffered writes drm/i915/intel_i2c: always wait for IDLE before clearing NAK drm/i915/intel_i2c: use WAIT cycle, not STOP drm/i915/intel_i2c: use INDEX cycles for i2c read transactions drm/i915/intel_i2c: reuse GMBUS2 value read in polling loop drm/i915/intel_i2c: remove POSTING_READ() from gmbus transfers drm/i915/intel_i2c: handle zero-length reads drm/i915/intel_i2c: reduce verbosity of some messages Daniel Vetter (24): drm/i915: simplify ppgtt setup drm/i915: re-init modeset hw state after gpu reset drm/i915: rip out ring->irq_mask drm/i915: set ring->size in common ring setup code
Re: [Intel-gfx] [PATCH] drm/i915: use the new masked bit macro some more
On Thu, Apr 26, 2012 at 10:02:54PM +0200, Daniel Vetter wrote: > I've missed this one. > > v2: Chris Wilson noticed another register. > v3: Color choice improvements. > > Reviewed-by: Chris Wilson > Signed-Off-by: Daniel Vetter Queued for -next, thanks for the review. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/12] drm/i915: extract some common olr+wedge code
On Thu, 26 Apr 2012 16:03:07 -0700, Ben Widawsky wrote:
> +/*
> + * Compare seqno against outstanding lazy request. Emit a request if they are
> + * equal. Seqno is updated with the new value if a request was emitted.
> + */
> +static int
> +i915_gem_check_olr(struct intel_ring_buffer *ring, u32 *seqno)
> +{
> + int ret = 0;
> +
> + BUG_ON(!mutex_is_locked(&ring->dev->struct_mutex));
> +
> + if (*seqno == ring->outstanding_lazy_request) {
> + struct drm_i915_gem_request *request;
> +
> + request = kzalloc(sizeof(*request), GFP_KERNEL);
> + if (request == NULL)
> + return -ENOMEM;
> +
> + ret = i915_add_request(ring, NULL, request);
> + if (ret) {
> + kfree(request);
> + return ret;
> + }
> +
> + *seqno = request->seqno;
I'd love for this to be BUG_ON(seqno != request->seqno) and so drop the
out parameter, as it would tidy all the callers up.
> + }
> +
> + return ret;
-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 11/12 v2] drm/i915: wait render timeout ioctl
On Thu, 26 Apr 2012 16:03:08 -0700, Ben Widawsky wrote: > + mutex_unlock(&dev->struct_mutex); > + ret = __wait_seqno(ring, seqno, true, &timeout); > + drm_gem_object_unreference_unlocked(&obj->base); Once we have the seqno to wait on, we can drop the reference to the object. The reference to the ring will be persist whilst the device is open. Just saves doing an unsightly unlock/lock dance in the unref. -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] [RFC] drm/i915: Move the STC LRA eviction policy workaround after ring init.
On Thu, Apr 26, 2012 at 11:45:50PM -0700, Kenneth Graunke wrote: > Clearing bit 5 of CACHE_MODE_0 is necessary to prevent GPU hangs in > OpenGL programs such as Google MapsGL, Google Earth, and gzdoom. > > While commit 09be664ecc77d58 introduced the workaround, the register > write didn't actually stick: a printk and I915_READ immediately after > the write would return the new value, but a second one would show that > it had reverted to the original value...even with no intervening code. > > The hardware documentation mentions that the ring must be idle before > writing CACHE_MODE_0. This provided a clue that it might be necessary > to initialize the rings before attempting to program the register. Sure > enough, moving the write after ring initialization makes it stick. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47535 > Signed-off-by: Kenneth Graunke > --- > drivers/gpu/drm/i915/i915_dma.c |7 +++ > drivers/gpu/drm/i915/intel_pm.c |4 > 2 files changed, 7 insertions(+), 4 deletions(-) > > Here's a horrible patch---putting that one workaround bit directly inside > i915_load_modeset_init is clearly a terrible idea. There will obviously > be many more, and per-generation. I suspect that many more of the > workaround bits we're setting in init_clock_gating may need to be moved > later in the init process too...but I haven't checked to see which ones > are failing to stick. > > So I guess there's a couple questions: > > * Does it make sense to move ALL the init_clock_gating stuff to this > point in time? Or are there some registers that need to be done early, > where they are today? (If we can move them all, we could just move the > call to init_clock_gating and be done with it...) > > Apparently CACHE_MODE_0 is a context-specific register, while some others > are not. I like having all the workaround bits in one place, but that > may or may not be feasible... > > * Do we want to make an intel_apply_workarounds() function or such? > Perhaps use a function pointer that gets filled in on a per-chipset basis? > > * Is this the best time to set it? Later? Elsewhere? > > * What should the code look like long-term, and what would be easiest for > backporting to stable kernels? I'm working on a real fix for -next that cleans up our gem init handling and workaround setting, but for -fixes I guess we just need to add yet another cludge to init_render_ring ... That way it should get called at all the right places (driver load, resume and gpu reset). -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
