Rev 1.67 of intagp (agp_i810.c) was added to deal with the problem

where stuff was bound for a software fallback while we were vt switched,
so we couldn't assume that the aperture was clear.

Turns out that the 855 at least does not like this (the !stolen bits of
the bar at least are write-only), and this caused a gpu lockup on any
suspend that used that activate handler.

So this diff fixes it in a more complete way:

Firstly revert 1.67 of agp_i810.c, secondly add a DVACT_QUIESCE handler
to inteldrm, and in it we do three things:

1) set a flag that causes any ioctl or pagefault to sleep until the flag
is cleared so that we don't end up with something doing the following
(for example:)
- bind object
- do another blocking operation in the same ioctl/fault
< suspend happens here, followed by resume>
- wake up assuming that the object is still bound and continue along our
merry way

2) Wait for all current entrypoints to finish (so that 1) is actually
effective we need to first quiesce our callers before we continue).

3) unbind everything that is in the gtt

On resume we clear the quiet flag, and everything can continue as
expected.

Behaves as expected for me on: 855 (x40 in acpi mode), gm965 (x61s) and
arrandale (x201) over multiple suspend cycles. Also tested by several on
tech.

3 files changed, 123 insertions, 44 deletions

diff --git a/sys/dev/pci/agp_i810.c b/sys/dev/pci/agp_i810.c
index 8a58a8c8af4..75071f6a0a3 100644
--- a/sys/dev/pci/agp_i810.c
+++ b/sys/dev/pci/agp_i810.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: agp_i810.c,v 1.68 2010/08/31 19:20:55 oga Exp $	*/
+/*	$OpenBSD: agp_i810.c,v 1.69 2010/09/06 15:00:50 oga Exp $	*/
 
 /*-
  * Copyright (c) 2000 Doug Rabson
@@ -80,7 +80,6 @@ struct agp_i810_softc {
 	struct agp_gatt		*gatt;
 	struct vga_pci_bar	*map;
 	struct vga_pci_bar	*gtt_map;
-	u_int32_t		*gtt_backup;	/* saved gtt for suspend */
 	bus_dmamap_t		 scrib_dmamap;
 	bus_addr_t		 isc_apaddr;
 	bus_size_t		 isc_apsize;	/* current aperture size */
@@ -299,14 +298,6 @@ agp_i810_attach(struct device *parent, struct device *self, void *aux)
 		goto out;
 	}
 
-	/*
-	 * Backup array to save gtt contents on suspend since we may lose
-	 * BAR contents. Most agp drivers do not have this problem since the
-	 * GTT ptes are in dma memory.
-	 */
-	isc->gtt_backup = malloc(sizeof(*isc->gtt_backup) *
-	    (isc->isc_apsize / 4096), M_AGP, M_NOWAIT | M_ZERO); 
-
 	switch (isc->chiptype) {
 	case CHIP_I810:
 		/* Some i810s have on-chip memory called dcache */
@@ -548,26 +539,13 @@ agp_i810_activate(struct device *arg, int act)
 		break;
 	}
 
+	/*
+	 * Anything kept in agp over a suspend/resume cycle (and thus by X
+	 * over a vt switch cycle) is undefined upon resume.
+	 */
 	switch (act) {
-	case DVACT_SUSPEND:
-		/*
-		 * most agp-like drivers have the GTT ptes in dma memory, so
-		 * just need the setup to be repeated on resume.
-		 * in this case the gtt is held in a BAR, and thus we should
-		 * restore the data on resume to make sure that we
-		 * don't lose any state that we are depending on.
-		 */
-		if (isc->gtt_backup != NULL) {
-			bus_space_read_region_4(bst, bsh, offset,
-			    isc->gtt_backup, isc->isc_apsize / 4096);
-		}
-		break;
 	case DVACT_RESUME:
 		agp_i810_configure(isc);
-		if (isc->gtt_backup != NULL) {
-			bus_space_write_region_4(bst, bsh, offset,
-			    isc->gtt_backup, isc->isc_apsize / 4096);
-		}
 		break;
 	}
 
diff --git a/sys/dev/pci/drm/i915_drv.c b/sys/dev/pci/drm/i915_drv.c
index 0ca51731a48..4cd513714b2 100644
--- a/sys/dev/pci/drm/i915_drv.c
+++ b/sys/dev/pci/drm/i915_drv.c
@@ -65,6 +65,7 @@ void	inteldrm_attach(struct device *, struct device *, void *);
 int	inteldrm_detach(struct device *, int);
 int	inteldrm_activate(struct device *, int);
 int	inteldrm_ioctl(struct drm_device *, u_long, caddr_t, struct drm_file *);
+int	inteldrm_doioctl(struct drm_device *, u_long, caddr_t, struct drm_file *);
 int	inteldrm_intr(void *);
 int	inteldrm_ironlake_intr(void *);
 void	inteldrm_lastclose(struct drm_device *);
@@ -81,6 +82,7 @@ int	inteldrm_fault(struct drm_obj *, struct uvm_faultinfo *, off_t,
 void	inteldrm_wipe_mappings(struct drm_obj *);
 void	inteldrm_purge_obj(struct drm_obj *);
 void	inteldrm_set_max_obj_size(struct inteldrm_softc *);
+void	inteldrm_quiesce(struct inteldrm_softc *);
 
 /* For reset and suspend */
 int	inteldrm_save_state(struct inteldrm_softc *);
@@ -159,7 +161,7 @@ int	i915_gem_init_ringbuffer(struct inteldrm_softc *);
 int	inteldrm_start_ring(struct inteldrm_softc *);
 void	i915_gem_cleanup_ringbuffer(struct inteldrm_softc *);
 int	i915_gem_ring_throttle(struct drm_device *, struct drm_file *);
-int	i915_gem_evict_inactive(struct inteldrm_softc *);
+int	i915_gem_evict_inactive(struct inteldrm_softc *, int);
 int	i915_gem_get_relocs_from_user(struct drm_i915_gem_exec_object2 *,
 	    u_int32_t, struct drm_i915_gem_relocation_entry **);
 int	i915_gem_put_relocs_to_user(struct drm_i915_gem_exec_object2 *,
@@ -535,11 +537,17 @@ inteldrm_activate(struct device *arg, int act)
 	struct inteldrm_softc	*dev_priv = (struct inteldrm_softc *)arg;
 
 	switch (act) {
+	case DVACT_QUIESCE:
+		inteldrm_quiesce(dev_priv);
+		break;
 	case DVACT_SUSPEND:
 		inteldrm_save_state(dev_priv);
 		break;
 	case DVACT_RESUME:
 		inteldrm_restore_state(dev_priv);
+		/* entrypoints can stop sleeping now */
+		atomic_clearbits_int(&dev_priv->sc_flags, INTELDRM_QUIET);
+		wakeup(&dev_priv->flags);
 		break;
 	}
 
@@ -560,6 +568,27 @@ inteldrm_ioctl(struct drm_device *dev, u_long cmd, caddr_t data,
     struct drm_file *file_priv)
 {
 	struct inteldrm_softc	*dev_priv = dev->dev_private;
+	int			 error = 0;
+
+	while ((dev_priv->sc_flags & INTELDRM_QUIET) && error == 0)
+		error = tsleep(&dev_priv->flags, PCATCH, "intelioc", 0);
+	if (error)
+		return (error);
+	dev_priv->entries++;
+
+	error = inteldrm_doioctl(dev, cmd, data, file_priv);
+
+	dev_priv->entries--;
+	if (dev_priv->sc_flags & INTELDRM_QUIET)
+		wakeup(&dev_priv->entries);
+	return (error);
+}
+
+int
+inteldrm_doioctl(struct drm_device *dev, u_long cmd, caddr_t data,
+    struct drm_file *file_priv)
+{
+	struct inteldrm_softc	*dev_priv = dev->dev_private;
 
 	if (file_priv->authenticated == 1) {
 		switch (cmd) {
@@ -1956,7 +1985,8 @@ i915_gem_evict_something(struct inteldrm_softc *dev_priv, size_t min_size,
 		 * everything and start again. (This should be rare.)
 		 */
 		if (!TAILQ_EMPTY(&dev_priv->mm.inactive_list))
-			return (i915_gem_evict_inactive(dev_priv));
+			return (i915_gem_evict_inactive(dev_priv,
+			    interruptible));
 		else
 			return (i915_gem_evict_everything(dev_priv,
 			    interruptible));
@@ -2024,7 +2054,7 @@ i915_gem_evict_everything(struct inteldrm_softc *dev_priv, int interruptible)
 		return (ENOMEM);
 
 	if ((ret = i915_wait_request(dev_priv, seqno, interruptible)) != 0 ||
-	    (ret = i915_gem_evict_inactive(dev_priv)) != 0)
+	    (ret = i915_gem_evict_inactive(dev_priv, interruptible)) != 0)
 		return (ret);
 
 	/*
@@ -2379,6 +2409,7 @@ inteldrm_fault(struct drm_obj *obj, struct uvm_faultinfo *ufi, off_t offset,
     vm_prot_t access_type, int flags)
 {
 	struct drm_device	*dev = obj->dev;
+	struct inteldrm_softc	*dev_priv = dev->dev_private;
 	struct inteldrm_obj	*obj_priv = (struct inteldrm_obj *)obj;
 	paddr_t			 paddr;
 	int			 lcv, ret;
@@ -2386,6 +2417,30 @@ inteldrm_fault(struct drm_obj *obj, struct uvm_faultinfo *ufi, off_t offset,
 	vm_prot_t		 mapprot;
 	boolean_t		 locked = TRUE;
 
+	/* Are we about to suspend?, if so wait until we're done */
+	if (dev_priv->sc_flags & INTELDRM_QUIET) {
+		/* we're about to sleep, unlock the map etc */
+		uvmfault_unlockall(ufi, NULL, &obj->uobj, NULL);
+		while (dev_priv->sc_flags & INTELDRM_QUIET)
+			tsleep(&dev_priv->flags, 0, "intelflt", 0);
+		dev_priv->entries++;
+		/*
+		 * relock so we're in the same state we would be in if we
+		 * were not quiesced before
+		 */
+		locked = uvmfault_relock(ufi);
+		if (locked) {
+			drm_lock_obj(obj);
+		} else {
+			dev_priv->entries--;
+			if (dev_priv->sc_flags & INTELDRM_QUIET)
+				wakeup(&dev_priv->entries);
+			return (VM_PAGER_REFAULT);
+		}
+	} else {
+		dev_priv->entries++;
+	}
+
 	if (rw_enter(&dev->dev_lock, RW_NOSLEEP | RW_READ) != 0) {
 		uvmfault_unlockall(ufi, NULL, &obj->uobj, NULL);
 		DRM_READLOCK();
@@ -2396,6 +2451,9 @@ inteldrm_fault(struct drm_obj *obj, struct uvm_faultinfo *ufi, off_t offset,
 	if (locked)
 		drm_hold_object_locked(obj);
 	else { /* obj already unlocked */
+		dev_priv->entries--;
+		if (dev_priv->sc_flags & INTELDRM_QUIET)
+			wakeup(&dev_priv->entries);
 		return (VM_PAGER_REFAULT);
 	}
 
@@ -2467,26 +2525,33 @@ inteldrm_fault(struct drm_obj *obj, struct uvm_faultinfo *ufi, off_t offset,
 			uvmfault_unlockall(ufi, ufi->entry->aref.ar_amap,
 			    NULL, NULL);
 			DRM_READUNLOCK();
+			dev_priv->entries--;
+			if (dev_priv->sc_flags & INTELDRM_QUIET)
+				wakeup(&dev_priv->entries);
 			uvm_wait("intelflt");
 			return (VM_PAGER_REFAULT);
 		}
 	}
-	drm_unhold_object(obj);
-	uvmfault_unlockall(ufi, ufi->entry->aref.ar_amap, NULL, NULL);
-	DRM_READUNLOCK();
-	pmap_update(ufi->orig_map->pmap);
-	return (VM_PAGER_OK);
-
 error:
-	/*
-	 * EIO means we're wedged so when we reset the gpu this will
-	 * work, so don't segfault. XXX only on resettable chips
-	 */
 	drm_unhold_object(obj);
 	uvmfault_unlockall(ufi, ufi->entry->aref.ar_amap, NULL, NULL);
 	DRM_READUNLOCK();
+	dev_priv->entries--;
+	if (dev_priv->sc_flags & INTELDRM_QUIET)
+		wakeup(&dev_priv->entries);
 	pmap_update(ufi->orig_map->pmap);
-	return ((ret == EIO) ?  VM_PAGER_REFAULT : VM_PAGER_ERROR);
+	if (ret == EIO) {
+		/*
+		 * EIO means we're wedged, so upon resetting the gpu we'll
+		 * be alright and can refault. XXX only on resettable chips.
+		 */
+		ret = VM_PAGER_REFAULT;
+	} else if (ret) {
+		ret = VM_PAGER_ERROR;
+	} else {
+		ret = VM_PAGER_OK;
+	}
+	return (ret);
 }
 
 void
@@ -3783,7 +3848,7 @@ i915_gem_free_object(struct drm_obj *obj)
 
 /* Clear out the inactive list and unbind everything in it. */
 int
-i915_gem_evict_inactive(struct inteldrm_softc *dev_priv)
+i915_gem_evict_inactive(struct inteldrm_softc *dev_priv, int interruptible)
 {
 	struct inteldrm_obj	*obj_priv;
 	int			 ret = 0;
@@ -3800,7 +3865,7 @@ i915_gem_evict_inactive(struct inteldrm_softc *dev_priv)
 		mtx_leave(&dev_priv->list_lock);
 
 		drm_hold_object(&obj_priv->obj);
-		ret = i915_gem_object_unbind(&obj_priv->obj, 1);
+		ret = i915_gem_object_unbind(&obj_priv->obj, interruptible);
 		drm_unhold_and_unref(&obj_priv->obj);
 
 		mtx_enter(&dev_priv->list_lock);
@@ -3812,6 +3877,35 @@ i915_gem_evict_inactive(struct inteldrm_softc *dev_priv)
 	return (ret);
 }
 
+void
+inteldrm_quiesce(struct inteldrm_softc *dev_priv)
+{
+	struct drm_device	*dev = (struct drm_device *)dev_priv->drmdev;
+	/*
+	 * Right now we depend on X vt switching, so we should be
+	 * already suspended, but fallbacks may fault, etc.
+	 * Since we can't readback the gtt to reset what we have, make
+	 * sure that everything is unbound.
+	 */
+	KASSERT(dev_priv->mm.suspended);
+	KASSERT(dev_priv->ring.ring_obj == NULL);
+	atomic_setbits_int(&dev_priv->sc_flags, INTELDRM_QUIET);
+	while (dev_priv->entries)
+		tsleep(&dev_priv->entries, 0, "intelquiet", 0);
+	/*
+	 * nothing should be dirty WRT the chip, only stuff that's bound
+	 * for gtt mapping. Nothing should be pinned over vt switch, if it
+	 * is then rendering corruption will occur due to api misuse, shame.
+	 */
+	KASSERT(TAILQ_EMPTY(&dev_priv->mm.flushing_list));
+	KASSERT(TAILQ_EMPTY(&dev_priv->mm.active_list));
+	/* Disabled because root could panic the kernel if this was enabled */
+	/* KASSERT(dev->pin_count == 0); */
+
+	/* can't fail since uninterruptible */
+	(void)i915_gem_evict_inactive(dev_priv, 0);
+}
+
 int
 i915_gem_idle(struct inteldrm_softc *dev_priv)
 {
@@ -4285,7 +4379,7 @@ inteldrm_hung(void *arg, void *reset_type)
 	mtx_leave(&dev_priv->list_lock);
 
 	/* unbind everything */
-	(void)i915_gem_evict_inactive(dev_priv);
+	(void)i915_gem_evict_inactive(dev_priv, 0);
 
 	if (HAS_RESET(dev_priv))
 		dev_priv->mm.wedged = 0;
diff --git a/sys/dev/pci/drm/i915_drv.h b/sys/dev/pci/drm/i915_drv.h
index 2df3cd5c2a2..ab5d501b41c 100644
--- a/sys/dev/pci/drm/i915_drv.h
+++ b/sys/dev/pci/drm/i915_drv.h
@@ -140,6 +140,13 @@ struct inteldrm_softc {
 	int			 fence_reg_start; /* 4 by default */
 	int			 num_fence_regs; /* 8 pre-965, 16 post */
 
+#define	INTELDRM_QUIET		0x01 /* suspend close, get off the hardware */
+#define	INTELDRM_WEDGED		0x02 /* chipset hung pending reset */
+#define	INTELDRM_SUSPENDED	0x04 /* in vt switch, no commands */
+	int			 sc_flags; /* quiet, suspended, hung */
+	/* number of ioctls + faults in flight */
+	int			 entries;
+
 	/* protects inactive, flushing, active and exec locks */
 	struct mutex		 list_lock;