Rebased series on drm-intel-next

Rebased series on drm-intel-next

[Chris Wilson]

My outstanding patchset rebased upon Eric's drm-intel-next.

The first 5 implement LRU eviction. Daniel may want to paint it a
different colour, but is happy enough to do so later. :)

(7) improves reliability of error-state, but I still need to work on
improving i965+ error capture and make it a slow task to appease Dave Airlie.
(9) Enables panel fitting for ILK+.
(14) Enables updating of watermarks on mode/dpms change for ILK. This
patches exposes another bug on my x201s (or rather makes it easier to hit
since we now attempt to enable SR after disabling a pipe).
(16) Checks that all waits on a register are bounded.

The rest are fairly minor changes. The spinlock removal has been dropped,
pending work on making sure hangcheck really does work across all
generations.

[PATCH 01/20] drm/i915: Append the object onto the inactive list on binding.

[Chris Wilson]

In order to properly track bound objects, they need to exist on one of
the inactive/active lists or be pinned. As this is a requirement, do the
work inside i915_gem_bind_to_gtt() rather than dotted around the
callsites.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b0fb394..75e7b89 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1137,7 +1137,6 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct drm_gem_object *obj = vma->vm_private_data;
 	struct drm_device *dev = obj->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj_priv = to_intel_bo(obj);
 	pgoff_t page_offset;
 	unsigned long pfn;
@@ -1155,8 +1154,6 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 		if (ret)
 			goto unlock;
 
-		list_add_tail(&obj_priv->list, &dev_priv->mm.inactive_list);
-
 		ret = i915_gem_object_set_to_gtt_domain(obj, write);
 		if (ret)
 			goto unlock;
@@ -1363,7 +1360,6 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file_priv)
 {
 	struct drm_i915_gem_mmap_gtt *args = data;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_gem_object *obj;
 	struct drm_i915_gem_object *obj_priv;
 	int ret;
@@ -1409,7 +1405,6 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
 			mutex_unlock(&dev->struct_mutex);
 			return ret;
 		}
-		list_add_tail(&obj_priv->list, &dev_priv->mm.inactive_list);
 	}
 
 	drm_gem_object_unreference(obj);
@@ -2723,6 +2718,9 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment)
 	atomic_inc(&dev->gtt_count);
 	atomic_add(obj->size, &dev->gtt_memory);
 
+	/* keep track of bounds object by adding it to the inactive list */
+	list_add_tail(&obj_priv->list, &dev_priv->mm.inactive_list);
+
 	/* Assert that the object is not currently in any GPU domain. As it
 	 * wasn't in the GTT, there shouldn't be any way it could have been in
 	 * a GPU cache
@@ -4223,8 +4221,7 @@ i915_gem_object_pin(struct drm_gem_object *obj, uint32_t alignment)
 		atomic_inc(&dev->pin_count);
 		atomic_add(obj->size, &dev->pin_memory);
 		if (!obj_priv->active &&
-		    (obj->write_domain & I915_GEM_GPU_DOMAINS) == 0 &&
-		    !list_empty(&obj_priv->list))
+		    (obj->write_domain & I915_GEM_GPU_DOMAINS) == 0)
 			list_del_init(&obj_priv->list);
 	}
 	i915_verify_inactive(dev, __FILE__, __LINE__);
-- 
1.7.1

[PATCH 02/20] drm/i915: prepare for fair lru eviction

[Chris Wilson]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

This does two little changes:

- Add an alignment parameter for evict_something. It's not really great to
  whack a carefully sized hole into the gtt with the wrong alignment.
  Especially since the fallback path is a full evict.

- With the inactive scan stuff we need to evict more that one object, so
  move the unbind call into the helper function that scans for the object
  to be evicted, too.  And adjust its name.

No functional changes in this patch, just preparation.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c |   67 ++++++++++++++++++++++++---------------
 1 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 75e7b89..f150bfd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -35,6 +35,7 @@
 #include <linux/swap.h>
 #include <linux/pci.h>
 
+static uint32_t i915_gem_get_gtt_alignment(struct drm_gem_object *obj);
 static int i915_gem_object_flush_gpu_write_domain(struct drm_gem_object *obj);
 static void i915_gem_object_flush_gtt_write_domain(struct drm_gem_object *obj);
 static void i915_gem_object_flush_cpu_write_domain(struct drm_gem_object *obj);
@@ -48,7 +49,8 @@ static int i915_gem_object_wait_rendering(struct drm_gem_object *obj);
 static int i915_gem_object_bind_to_gtt(struct drm_gem_object *obj,
 					   unsigned alignment);
 static void i915_gem_clear_fence_reg(struct drm_gem_object *obj);
-static int i915_gem_evict_something(struct drm_device *dev, int min_size);
+static int i915_gem_evict_something(struct drm_device *dev, int min_size,
+				    unsigned alignment);
 static int i915_gem_evict_from_inactive_list(struct drm_device *dev);
 static int i915_gem_phys_pwrite(struct drm_device *dev, struct drm_gem_object *obj,
 				struct drm_i915_gem_pwrite *args,
@@ -313,7 +315,8 @@ i915_gem_object_get_pages_or_evict(struct drm_gem_object *obj)
 	if (ret == -ENOMEM) {
 		struct drm_device *dev = obj->dev;
 
-		ret = i915_gem_evict_something(dev, obj->size);
+		ret = i915_gem_evict_something(dev, obj->size,
+					       i915_gem_get_gtt_alignment(obj));
 		if (ret)
 			return ret;
 
@@ -2005,10 +2008,12 @@ i915_gem_object_unbind(struct drm_gem_object *obj)
 	return ret;
 }
 
-static struct drm_gem_object *
-i915_gem_find_inactive_object(struct drm_device *dev, int min_size)
+static int
+i915_gem_scan_inactive_list_and_evict(struct drm_device *dev, int min_size,
+				      unsigned alignment, int *found)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_gem_object *obj;
 	struct drm_i915_gem_object *obj_priv;
 	struct drm_gem_object *best = NULL;
 	struct drm_gem_object *first = NULL;
@@ -2022,14 +2027,31 @@ i915_gem_find_inactive_object(struct drm_device *dev, int min_size)
 			    (!best || obj->size < best->size)) {
 				best = obj;
 				if (best->size == min_size)
-					return best;
+					break;
 			}
 			if (!first)
 			    first = obj;
 		}
 	}
 
-	return best ? best : first;
+	obj = best ? best : first;
+
+	if (!obj) {
+		*found = 0;
+		return 0;
+	}
+
+	*found = 1;
+
+#if WATCH_LRU
+	DRM_INFO("%s: evicting %p\n", __func__, obj);
+#endif
+	obj_priv = to_intel_bo(obj);
+	BUG_ON(obj_priv->pin_count != 0);
+	BUG_ON(obj_priv->active);
+
+	/* Wait on the rendering and unbind the buffer. */
+	return i915_gem_object_unbind(obj);
 }
 
 static int
@@ -2115,11 +2137,11 @@ i915_gem_evict_everything(struct drm_device *dev)
 }
 
 static int
-i915_gem_evict_something(struct drm_device *dev, int min_size)
+i915_gem_evict_something(struct drm_device *dev,
+			 int min_size, unsigned alignment)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	struct drm_gem_object *obj;
-	int ret;
+	int ret, found;
 
 	struct intel_ring_buffer *render_ring = &dev_priv->render_ring;
 	struct intel_ring_buffer *bsd_ring = &dev_priv->bsd_ring;
@@ -2129,20 +2151,11 @@ i915_gem_evict_something(struct drm_device *dev, int min_size)
 		/* If there's an inactive buffer available now, grab it
 		 * and be done.
 		 */
-		obj = i915_gem_find_inactive_object(dev, min_size);
-		if (obj) {
-			struct drm_i915_gem_object *obj_priv;
-
-#if WATCH_LRU
-			DRM_INFO("%s: evicting %p\n", __func__, obj);
-#endif
-			obj_priv = to_intel_bo(obj);
-			BUG_ON(obj_priv->pin_count != 0);
-			BUG_ON(obj_priv->active);
-
-			/* Wait on the rendering and unbind the buffer. */
-			return i915_gem_object_unbind(obj);
-		}
+		ret = i915_gem_scan_inactive_list_and_evict(dev, min_size,
+							    alignment,
+							    &found);
+		if (found)
+			return ret;
 
 		/* If we didn't get anything, but the ring is still processing
 		 * things, wait for the next to finish and hopefully leave us
@@ -2184,6 +2197,7 @@ i915_gem_evict_something(struct drm_device *dev, int min_size)
 		 * will get moved to inactive.
 		 */
 		if (!list_empty(&dev_priv->mm.flushing_list)) {
+			struct drm_gem_object *obj = NULL;
 			struct drm_i915_gem_object *obj_priv;
 
 			/* Find an object that we can immediately reuse */
@@ -2661,7 +2675,7 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment)
 #if WATCH_LRU
 		DRM_INFO("%s: GTT full, evicting something\n", __func__);
 #endif
-		ret = i915_gem_evict_something(dev, obj->size);
+		ret = i915_gem_evict_something(dev, obj->size, alignment);
 		if (ret)
 			return ret;
 
@@ -2679,7 +2693,8 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment)
 
 		if (ret == -ENOMEM) {
 			/* first try to clear up some space from the GTT */
-			ret = i915_gem_evict_something(dev, obj->size);
+			ret = i915_gem_evict_something(dev, obj->size,
+						       alignment);
 			if (ret) {
 				/* now try to shrink everyone else */
 				if (gfpmask) {
@@ -2709,7 +2724,7 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment)
 		drm_mm_put_block(obj_priv->gtt_space);
 		obj_priv->gtt_space = NULL;
 
-		ret = i915_gem_evict_something(dev, obj->size);
+		ret = i915_gem_evict_something(dev, obj->size, alignment);
 		if (ret)
 			return ret;
 
-- 
1.7.1

[PATCH 03/20] drm/i915: Use a common seqno for all rings.

[Chris Wilson]

This will be used by the eviction logic to maintain fairness between the
rings.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h         |    3 +-
 drivers/gpu/drm/i915/i915_gem.c         |    2 +
 drivers/gpu/drm/i915/intel_ringbuffer.c |   46 +++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_ringbuffer.h |    1 -
 4 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1510565..def6ee0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -244,6 +244,7 @@ typedef struct drm_i915_private {
 	struct pci_dev *bridge_dev;
 	struct intel_ring_buffer render_ring;
 	struct intel_ring_buffer bsd_ring;
+	uint32_t next_seqno;
 
 	drm_dma_handle_t *status_page_dmah;
 	void *seqno_page;
@@ -573,8 +574,6 @@ typedef struct drm_i915_private {
 		 */
 		struct delayed_work retire_work;
 
-		uint32_t next_gem_seqno;
-
 		/**
 		 * Waiting sequence number, if any
 		 */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f150bfd..45b9982 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4714,6 +4714,8 @@ i915_gem_init_ringbuffer(struct drm_device *dev)
 			goto cleanup_render_ring;
 	}
 
+	dev_priv->next_seqno = 1;
+
 	return 0;
 
 cleanup_render_ring:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a5d664e..3a02425 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -33,18 +33,35 @@
 #include "i915_drm.h"
 #include "i915_trace.h"
 
+static u32 i915_gem_get_seqno(struct drm_device *dev)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	u32 seqno;
+
+	seqno = dev_priv->next_seqno;
+
+	/* reserve 0 for non-seqno */
+	if (++dev_priv->next_seqno == 0)
+		dev_priv->next_seqno = 1;
+
+	return seqno;
+}
+
 static void
 render_ring_flush(struct drm_device *dev,
 		struct intel_ring_buffer *ring,
 		u32	invalidate_domains,
 		u32	flush_domains)
 {
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	u32 cmd;
+
 #if WATCH_EXEC
 	DRM_INFO("%s: invalidate %08x flush %08x\n", __func__,
 		  invalidate_domains, flush_domains);
 #endif
-	u32 cmd;
-	trace_i915_gem_request_flush(dev, ring->next_seqno,
+
+	trace_i915_gem_request_flush(dev, dev_priv->next_seqno,
 				     invalidate_domains, flush_domains);
 
 	if ((invalidate_domains | flush_domains) & I915_GEM_GPU_DOMAINS) {
@@ -233,9 +250,10 @@ render_ring_add_request(struct drm_device *dev,
 		struct drm_file *file_priv,
 		u32 flush_domains)
 {
-	u32 seqno;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	seqno = intel_ring_get_seqno(dev, ring);
+	u32 seqno;
+
+	seqno = i915_gem_get_seqno(dev);
 
 	if (IS_GEN6(dev)) {
 		BEGIN_LP_RING(6);
@@ -405,7 +423,9 @@ bsd_ring_add_request(struct drm_device *dev,
 		u32 flush_domains)
 {
 	u32 seqno;
-	seqno = intel_ring_get_seqno(dev, ring);
+
+	seqno = i915_gem_get_seqno(dev);
+
 	intel_ring_begin(dev, ring, 4);
 	intel_ring_emit(dev, ring, MI_STORE_DWORD_INDEX);
 	intel_ring_emit(dev, ring,
@@ -479,7 +499,7 @@ render_ring_dispatch_gem_execbuffer(struct drm_device *dev,
 	exec_start = (uint32_t) exec_offset + exec->batch_start_offset;
 	exec_len = (uint32_t) exec->batch_len;
 
-	trace_i915_gem_request_submit(dev, dev_priv->mm.next_gem_seqno + 1);
+	trace_i915_gem_request_submit(dev, dev_priv->next_seqno + 1);
 
 	count = nbox ? nbox : 1;
 
@@ -757,18 +777,6 @@ void intel_fill_struct(struct drm_device *dev,
 	intel_ring_advance(dev, ring);
 }
 
-u32 intel_ring_get_seqno(struct drm_device *dev,
-		struct intel_ring_buffer *ring)
-{
-	u32 seqno;
-	seqno = ring->next_seqno;
-
-	/* reserve 0 for non-seqno */
-	if (++ring->next_seqno == 0)
-		ring->next_seqno = 1;
-	return seqno;
-}
-
 struct intel_ring_buffer render_ring = {
 	.name			= "render ring",
 	.regs                   = {
@@ -786,7 +794,6 @@ struct intel_ring_buffer render_ring = {
 	.head			= 0,
 	.tail			= 0,
 	.space			= 0,
-	.next_seqno		= 1,
 	.user_irq_refcount	= 0,
 	.irq_gem_seqno		= 0,
 	.waiting_gem_seqno	= 0,
@@ -825,7 +832,6 @@ struct intel_ring_buffer bsd_ring = {
 	.head			= 0,
 	.tail			= 0,
 	.space			= 0,
-	.next_seqno		= 1,
 	.user_irq_refcount	= 0,
 	.irq_gem_seqno		= 0,
 	.waiting_gem_seqno	= 0,
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 9b67eea..525e7d3 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -26,7 +26,6 @@ struct  intel_ring_buffer {
 	unsigned int	head;
 	unsigned int	tail;
 	unsigned int	space;
-	u32		next_seqno;
 	struct intel_hw_status_page status_page;
 
 	u32		irq_gem_seqno;		/* last seq seem at irq time */
-- 
1.7.1

[PATCH 04/20] drm/i915: Move the eviction logic to its own file.

[Chris Wilson]

[-- Attachment #1: Type: text/plain, Size: 17317 bytes --]

The eviction code is the gnarly underbelly of memory management, and is
clearer if kept separated from the normal domain management in GEM.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/Makefile         |    1 +
 drivers/gpu/drm/i915/i915_drv.h       |    6 +
 drivers/gpu/drm/i915/i915_gem.c       |  231 +-----------------------------
 drivers/gpu/drm/i915/i915_gem_evict.c |  260 +++++++++++++++++++++++++++++++++
 4 files changed, 269 insertions(+), 229 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_gem_evict.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index da78f2c..384fd45 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -8,6 +8,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \
           i915_suspend.o \
 	  i915_gem.o \
 	  i915_gem_debug.o \
+	  i915_gem_evict.o \
 	  i915_gem_tiling.o \
 	  i915_trace_points.o \
 	  intel_display.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index def6ee0..12c8f47 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -982,6 +982,7 @@ int i915_gem_init_ringbuffer(struct drm_device *dev);
 void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
 int i915_gem_do_init(struct drm_device *dev, unsigned long start,
 		     unsigned long end);
+int i915_gpu_idle(struct drm_device *dev);
 int i915_gem_idle(struct drm_device *dev);
 uint32_t i915_add_request(struct drm_device *dev,
 		struct drm_file *file_priv,
@@ -1007,6 +1008,11 @@ int i915_gem_object_flush_write_domain(struct drm_gem_object *obj);
 void i915_gem_shrinker_init(void);
 void i915_gem_shrinker_exit(void);
 
+/* i915_gem_evict.c */
+int i915_gem_evict_something(struct drm_device *dev, int min_size, unsigned alignment);
+int i915_gem_evict_everything(struct drm_device *dev);
+int i915_gem_evict_inactive(struct drm_device *dev);
+
 /* i915_gem_tiling.c */
 void i915_gem_detect_bit_6_swizzle(struct drm_device *dev);
 void i915_gem_object_do_bit_17_swizzle(struct drm_gem_object *obj);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 45b9982..b5a7b00 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -49,9 +49,6 @@ static int i915_gem_object_wait_rendering(struct drm_gem_object *obj);
 static int i915_gem_object_bind_to_gtt(struct drm_gem_object *obj,
 					   unsigned alignment);
 static void i915_gem_clear_fence_reg(struct drm_gem_object *obj);
-static int i915_gem_evict_something(struct drm_device *dev, int min_size,
-				    unsigned alignment);
-static int i915_gem_evict_from_inactive_list(struct drm_device *dev);
 static int i915_gem_phys_pwrite(struct drm_device *dev, struct drm_gem_object *obj,
 				struct drm_i915_gem_pwrite *args,
 				struct drm_file *file_priv);
@@ -1885,19 +1882,6 @@ i915_gem_flush(struct drm_device *dev,
 				flush_domains);
 }
 
-static void
-i915_gem_flush_ring(struct drm_device *dev,
-	       uint32_t invalidate_domains,
-	       uint32_t flush_domains,
-	       struct intel_ring_buffer *ring)
-{
-	if (flush_domains & I915_GEM_DOMAIN_CPU)
-		drm_agp_chipset_flush(dev);
-	ring->flush(dev, ring,
-			invalidate_domains,
-			flush_domains);
-}
-
 /**
  * Ensures that all rendering to the object has completed and the object is
  * safe to unbind from the GTT or access from the CPU.
@@ -2008,53 +1992,7 @@ i915_gem_object_unbind(struct drm_gem_object *obj)
 	return ret;
 }
 
-static int
-i915_gem_scan_inactive_list_and_evict(struct drm_device *dev, int min_size,
-				      unsigned alignment, int *found)
-{
-	drm_i915_private_t *dev_priv = dev->dev_private;
-	struct drm_gem_object *obj;
-	struct drm_i915_gem_object *obj_priv;
-	struct drm_gem_object *best = NULL;
-	struct drm_gem_object *first = NULL;
-
-	/* Try to find the smallest clean object */
-	list_for_each_entry(obj_priv, &dev_priv->mm.inactive_list, list) {
-		struct drm_gem_object *obj = &obj_priv->base;
-		if (obj->size >= min_size) {
-			if ((!obj_priv->dirty ||
-			     i915_gem_object_is_purgeable(obj_priv)) &&
-			    (!best || obj->size < best->size)) {
-				best = obj;
-				if (best->size == min_size)
-					break;
-			}
-			if (!first)
-			    first = obj;
-		}
-	}
-
-	obj = best ? best : first;
-
-	if (!obj) {
-		*found = 0;
-		return 0;
-	}
-
-	*found = 1;
-
-#if WATCH_LRU
-	DRM_INFO("%s: evicting %p\n", __func__, obj);
-#endif
-	obj_priv = to_intel_bo(obj);
-	BUG_ON(obj_priv->pin_count != 0);
-	BUG_ON(obj_priv->active);
-
-	/* Wait on the rendering and unbind the buffer. */
-	return i915_gem_object_unbind(obj);
-}
-
-static int
+int
 i915_gpu_idle(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
@@ -2095,147 +2033,6 @@ i915_gpu_idle(struct drm_device *dev)
 	return ret;
 }
 
-static int
-i915_gem_evict_everything(struct drm_device *dev)
-{
-	drm_i915_private_t *dev_priv = dev->dev_private;
-	int ret;
-	bool lists_empty;
-
-	spin_lock(&dev_priv->mm.active_list_lock);
-	lists_empty = (list_empty(&dev_priv->mm.inactive_list) &&
-		       list_empty(&dev_priv->mm.flushing_list) &&
-		       list_empty(&dev_priv->render_ring.active_list) &&
-		       (!HAS_BSD(dev)
-			|| list_empty(&dev_priv->bsd_ring.active_list)));
-	spin_unlock(&dev_priv->mm.active_list_lock);
-
-	if (lists_empty)
-		return -ENOSPC;
-
-	/* Flush everything (on to the inactive lists) and evict */
-	ret = i915_gpu_idle(dev);
-	if (ret)
-		return ret;
-
-	BUG_ON(!list_empty(&dev_priv->mm.flushing_list));
-
-	ret = i915_gem_evict_from_inactive_list(dev);
-	if (ret)
-		return ret;
-
-	spin_lock(&dev_priv->mm.active_list_lock);
-	lists_empty = (list_empty(&dev_priv->mm.inactive_list) &&
-		       list_empty(&dev_priv->mm.flushing_list) &&
-		       list_empty(&dev_priv->render_ring.active_list) &&
-		       (!HAS_BSD(dev)
-			|| list_empty(&dev_priv->bsd_ring.active_list)));
-	spin_unlock(&dev_priv->mm.active_list_lock);
-	BUG_ON(!lists_empty);
-
-	return 0;
-}
-
-static int
-i915_gem_evict_something(struct drm_device *dev,
-			 int min_size, unsigned alignment)
-{
-	drm_i915_private_t *dev_priv = dev->dev_private;
-	int ret, found;
-
-	struct intel_ring_buffer *render_ring = &dev_priv->render_ring;
-	struct intel_ring_buffer *bsd_ring = &dev_priv->bsd_ring;
-	for (;;) {
-		i915_gem_retire_requests(dev);
-
-		/* If there's an inactive buffer available now, grab it
-		 * and be done.
-		 */
-		ret = i915_gem_scan_inactive_list_and_evict(dev, min_size,
-							    alignment,
-							    &found);
-		if (found)
-			return ret;
-
-		/* If we didn't get anything, but the ring is still processing
-		 * things, wait for the next to finish and hopefully leave us
-		 * a buffer to evict.
-		 */
-		if (!list_empty(&render_ring->request_list)) {
-			struct drm_i915_gem_request *request;
-
-			request = list_first_entry(&render_ring->request_list,
-						   struct drm_i915_gem_request,
-						   list);
-
-			ret = i915_wait_request(dev,
-					request->seqno, request->ring);
-			if (ret)
-				return ret;
-
-			continue;
-		}
-
-		if (HAS_BSD(dev) && !list_empty(&bsd_ring->request_list)) {
-			struct drm_i915_gem_request *request;
-
-			request = list_first_entry(&bsd_ring->request_list,
-						   struct drm_i915_gem_request,
-						   list);
-
-			ret = i915_wait_request(dev,
-					request->seqno, request->ring);
-			if (ret)
-				return ret;
-
-			continue;
-		}
-
-		/* If we didn't have anything on the request list but there
-		 * are buffers awaiting a flush, emit one and try again.
-		 * When we wait on it, those buffers waiting for that flush
-		 * will get moved to inactive.
-		 */
-		if (!list_empty(&dev_priv->mm.flushing_list)) {
-			struct drm_gem_object *obj = NULL;
-			struct drm_i915_gem_object *obj_priv;
-
-			/* Find an object that we can immediately reuse */
-			list_for_each_entry(obj_priv, &dev_priv->mm.flushing_list, list) {
-				obj = &obj_priv->base;
-				if (obj->size >= min_size)
-					break;
-
-				obj = NULL;
-			}
-
-			if (obj != NULL) {
-				uint32_t seqno;
-
-				i915_gem_flush_ring(dev,
-					       obj->write_domain,
-					       obj->write_domain,
-					       obj_priv->ring);
-				seqno = i915_add_request(dev, NULL,
-						obj->write_domain,
-						obj_priv->ring);
-				if (seqno == 0)
-					return -ENOMEM;
-				continue;
-			}
-		}
-
-		/* If we didn't do any of the above, there's no single buffer
-		 * large enough to swap out for the new one, so just evict
-		 * everything and start again. (This should be rare.)
-		 */
-		if (!list_empty (&dev_priv->mm.inactive_list))
-			return i915_gem_evict_from_inactive_list(dev);
-		else
-			return i915_gem_evict_everything(dev);
-	}
-}
-
 int
 i915_gem_object_get_pages(struct drm_gem_object *obj,
 			  gfp_t gfpmask)
@@ -4548,30 +4345,6 @@ void i915_gem_free_object(struct drm_gem_object *obj)
 	i915_gem_free_object_tail(obj);
 }
 
-/** Unbinds all inactive objects. */
-static int
-i915_gem_evict_from_inactive_list(struct drm_device *dev)
-{
-	drm_i915_private_t *dev_priv = dev->dev_private;
-
-	while (!list_empty(&dev_priv->mm.inactive_list)) {
-		struct drm_gem_object *obj;
-		int ret;
-
-		obj = &list_first_entry(&dev_priv->mm.inactive_list,
-					struct drm_i915_gem_object,
-					list)->base;
-
-		ret = i915_gem_object_unbind(obj);
-		if (ret != 0) {
-			DRM_ERROR("Error unbinding object: %d\n", ret);
-			return ret;
-		}
-	}
-
-	return 0;
-}
-
 int
 i915_gem_idle(struct drm_device *dev)
 {
@@ -4596,7 +4369,7 @@ i915_gem_idle(struct drm_device *dev)
 
 	/* Under UMS, be paranoid and evict. */
 	if (!drm_core_check_feature(dev, DRIVER_MODESET)) {
-		ret = i915_gem_evict_from_inactive_list(dev);
+		ret = i915_gem_evict_inactive(dev);
 		if (ret) {
 			mutex_unlock(&dev->struct_mutex);
 			return ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
new file mode 100644
index 0000000..479e450
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -0,0 +1,260 @@
+/*
+ * Copyright © 2008-2010 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:
+ *    Eric Anholt <eric@anholt.net>
+ *    Chris Wilson <chris@chris-wilson.co.uuk>
+ *
+ */
+
+#include "drmP.h"
+#include "drm.h"
+#include "i915_drv.h"
+#include "i915_drm.h"
+
+static inline int
+i915_gem_object_is_purgeable(struct drm_i915_gem_object *obj_priv)
+{
+	return obj_priv->madv == I915_MADV_DONTNEED;
+}
+
+static int
+i915_gem_scan_inactive_list_and_evict(struct drm_device *dev, int min_size,
+				      unsigned alignment, int *found)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_gem_object *obj;
+	struct drm_i915_gem_object *obj_priv;
+	struct drm_gem_object *best = NULL;
+	struct drm_gem_object *first = NULL;
+
+	/* Try to find the smallest clean object */
+	list_for_each_entry(obj_priv, &dev_priv->mm.inactive_list, list) {
+		struct drm_gem_object *obj = &obj_priv->base;
+		if (obj->size >= min_size) {
+			if ((!obj_priv->dirty ||
+			     i915_gem_object_is_purgeable(obj_priv)) &&
+			    (!best || obj->size < best->size)) {
+				best = obj;
+				if (best->size == min_size)
+					break;
+			}
+			if (!first)
+			    first = obj;
+		}
+	}
+
+	obj = best ? best : first;
+
+	if (!obj) {
+		*found = 0;
+		return 0;
+	}
+
+	*found = 1;
+
+#if WATCH_LRU
+	DRM_INFO("%s: evicting %p\n", __func__, obj);
+#endif
+	obj_priv = to_intel_bo(obj);
+	BUG_ON(obj_priv->pin_count != 0);
+	BUG_ON(obj_priv->active);
+
+	/* Wait on the rendering and unbind the buffer. */
+	return i915_gem_object_unbind(obj);
+}
+
+static void
+i915_gem_flush_ring(struct drm_device *dev,
+	       uint32_t invalidate_domains,
+	       uint32_t flush_domains,
+	       struct intel_ring_buffer *ring)
+{
+	if (flush_domains & I915_GEM_DOMAIN_CPU)
+		drm_agp_chipset_flush(dev);
+	ring->flush(dev, ring,
+			invalidate_domains,
+			flush_domains);
+}
+
+int
+i915_gem_evict_something(struct drm_device *dev,
+			 int min_size, unsigned alignment)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	int ret, found;
+
+	struct intel_ring_buffer *render_ring = &dev_priv->render_ring;
+	struct intel_ring_buffer *bsd_ring = &dev_priv->bsd_ring;
+	for (;;) {
+		i915_gem_retire_requests(dev);
+
+		/* If there's an inactive buffer available now, grab it
+		 * and be done.
+		 */
+		ret = i915_gem_scan_inactive_list_and_evict(dev, min_size,
+							    alignment,
+							    &found);
+		if (found)
+			return ret;
+
+		/* If we didn't get anything, but the ring is still processing
+		 * things, wait for the next to finish and hopefully leave us
+		 * a buffer to evict.
+		 */
+		if (!list_empty(&render_ring->request_list)) {
+			struct drm_i915_gem_request *request;
+
+			request = list_first_entry(&render_ring->request_list,
+						   struct drm_i915_gem_request,
+						   list);
+
+			ret = i915_do_wait_request(dev, request->seqno, true, request->ring);
+			if (ret)
+				return ret;
+
+			continue;
+		}
+
+		if (HAS_BSD(dev) && !list_empty(&bsd_ring->request_list)) {
+			struct drm_i915_gem_request *request;
+
+			request = list_first_entry(&bsd_ring->request_list,
+						   struct drm_i915_gem_request,
+						   list);
+
+			ret = i915_do_wait_request(dev, request->seqno, true, request->ring);
+			if (ret)
+				return ret;
+
+			continue;
+		}
+
+		/* If we didn't have anything on the request list but there
+		 * are buffers awaiting a flush, emit one and try again.
+		 * When we wait on it, those buffers waiting for that flush
+		 * will get moved to inactive.
+		 */
+		if (!list_empty(&dev_priv->mm.flushing_list)) {
+			struct drm_gem_object *obj = NULL;
+			struct drm_i915_gem_object *obj_priv;
+
+			/* Find an object that we can immediately reuse */
+			list_for_each_entry(obj_priv, &dev_priv->mm.flushing_list, list) {
+				obj = &obj_priv->base;
+				if (obj->size >= min_size)
+					break;
+
+				obj = NULL;
+			}
+
+			if (obj != NULL) {
+				uint32_t seqno;
+
+				i915_gem_flush_ring(dev,
+					       obj->write_domain,
+					       obj->write_domain,
+					       obj_priv->ring);
+				seqno = i915_add_request(dev, NULL,
+						obj->write_domain,
+						obj_priv->ring);
+				if (seqno == 0)
+					return -ENOMEM;
+				continue;
+			}
+		}
+
+		/* If we didn't do any of the above, there's no single buffer
+		 * large enough to swap out for the new one, so just evict
+		 * everything and start again. (This should be rare.)
+		 */
+		if (!list_empty(&dev_priv->mm.inactive_list))
+			return i915_gem_evict_inactive(dev);
+		else
+			return i915_gem_evict_everything(dev);
+	}
+}
+
+int
+i915_gem_evict_everything(struct drm_device *dev)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	int ret;
+	bool lists_empty;
+
+	spin_lock(&dev_priv->mm.active_list_lock);
+	lists_empty = (list_empty(&dev_priv->mm.inactive_list) &&
+		       list_empty(&dev_priv->mm.flushing_list) &&
+		       list_empty(&dev_priv->render_ring.active_list) &&
+		       (!HAS_BSD(dev)
+			|| list_empty(&dev_priv->bsd_ring.active_list)));
+	spin_unlock(&dev_priv->mm.active_list_lock);
+
+	if (lists_empty)
+		return -ENOSPC;
+
+	/* Flush everything (on to the inactive lists) and evict */
+	ret = i915_gpu_idle(dev);
+	if (ret)
+		return ret;
+
+	BUG_ON(!list_empty(&dev_priv->mm.flushing_list));
+
+	ret = i915_gem_evict_inactive(dev);
+	if (ret)
+		return ret;
+
+	spin_lock(&dev_priv->mm.active_list_lock);
+	lists_empty = (list_empty(&dev_priv->mm.inactive_list) &&
+		       list_empty(&dev_priv->mm.flushing_list) &&
+		       list_empty(&dev_priv->render_ring.active_list) &&
+		       (!HAS_BSD(dev)
+			|| list_empty(&dev_priv->bsd_ring.active_list)));
+	spin_unlock(&dev_priv->mm.active_list_lock);
+	BUG_ON(!lists_empty);
+
+	return 0;
+}
+
+/** Unbinds all inactive objects. */
+int
+i915_gem_evict_inactive(struct drm_device *dev)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+
+	while (!list_empty(&dev_priv->mm.inactive_list)) {
+		struct drm_gem_object *obj;
+		int ret;
+
+		obj = &list_first_entry(&dev_priv->mm.inactive_list,
+					struct drm_i915_gem_object,
+					list)->base;
+
+		ret = i915_gem_object_unbind(obj);
+		if (ret != 0) {
+			DRM_ERROR("Error unbinding object: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
-- 
1.7.1


[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 05/20] drm/i915: Implement fair lru eviction across both rings. (v2)

[Chris Wilson]

Based in a large part upon Daniel Vetter's implementation and adapted
for handling multiple rings in a single pass.

This should lead to better gtt usage and fixes the page-fault-of-doom
triggered. The fairness is provided by scanning through the GTT space
amalgamating space in rendering order. As soon as we have a contiguous
space in the GTT large enough for the new object (and its alignment),
evict any object which lies within that space. This should keep more
objects resident in the GTT.

Doing throughput testing on a PineView machine with cairo-perf-trace
indicates that there is very little difference with the new LRU scan,
perhaps a small improvement... Except oddly for the poppler trace.

Reference:

  Bug 15911 - Intermittent X crash (freeze)
  https://bugzilla.kernel.org/show_bug.cgi?id=15911

  Bug 20152 - cannot view JPG in firefox when running UXA
  https://bugs.freedesktop.org/show_bug.cgi?id=20152

  Bug 24369 - Hang when scrolling firefox page with window in front
  https://bugs.freedesktop.org/show_bug.cgi?id=24369

  Bug 28478 - Intermittent graphics lockups due to overflow/loop
  https://bugs.freedesktop.org/show_bug.cgi?id=28478

v2: Attempt to clarify the logic and order of eviction through the use
of comments and macros.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h       |    2 +
 drivers/gpu/drm/i915/i915_gem_evict.c |  279 +++++++++++++++++----------------
 2 files changed, 147 insertions(+), 134 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 12c8f47..6221f23 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -673,6 +673,8 @@ struct drm_i915_gem_object {
 	struct list_head list;
 	/** This object's place on GPU write list */
 	struct list_head gpu_write_list;
+	/** This object's place on eviction list */
+	struct list_head evict_list;
 
 	/**
 	 * This is set if the object is on the active or flushing lists
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 479e450..72cae3c 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -31,167 +31,178 @@
 #include "i915_drv.h"
 #include "i915_drm.h"
 
-static inline int
-i915_gem_object_is_purgeable(struct drm_i915_gem_object *obj_priv)
-{
-	return obj_priv->madv == I915_MADV_DONTNEED;
-}
-
-static int
-i915_gem_scan_inactive_list_and_evict(struct drm_device *dev, int min_size,
-				      unsigned alignment, int *found)
+static struct drm_i915_gem_object *
+i915_gem_next_active_object(struct drm_device *dev,
+			    struct list_head **render_iter,
+			    struct list_head **bsd_iter)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	struct drm_gem_object *obj;
-	struct drm_i915_gem_object *obj_priv;
-	struct drm_gem_object *best = NULL;
-	struct drm_gem_object *first = NULL;
-
-	/* Try to find the smallest clean object */
-	list_for_each_entry(obj_priv, &dev_priv->mm.inactive_list, list) {
-		struct drm_gem_object *obj = &obj_priv->base;
-		if (obj->size >= min_size) {
-			if ((!obj_priv->dirty ||
-			     i915_gem_object_is_purgeable(obj_priv)) &&
-			    (!best || obj->size < best->size)) {
-				best = obj;
-				if (best->size == min_size)
-					break;
-			}
-			if (!first)
-			    first = obj;
-		}
-	}
+	struct drm_i915_gem_object *render_obj = NULL, *bsd_obj = NULL;
 
-	obj = best ? best : first;
+	if (*render_iter != &dev_priv->render_ring.active_list)
+		render_obj = list_entry(*render_iter,
+					struct drm_i915_gem_object,
+					list);
 
-	if (!obj) {
-		*found = 0;
-		return 0;
-	}
+	if (HAS_BSD(dev)) {
+		if (*bsd_iter != &dev_priv->bsd_ring.active_list)
+			bsd_obj = list_entry(*bsd_iter,
+					     struct drm_i915_gem_object,
+					     list);
 
-	*found = 1;
+		if (render_obj == NULL) {
+			*bsd_iter = (*bsd_iter)->next;
+			return bsd_obj;
+		}
 
-#if WATCH_LRU
-	DRM_INFO("%s: evicting %p\n", __func__, obj);
-#endif
-	obj_priv = to_intel_bo(obj);
-	BUG_ON(obj_priv->pin_count != 0);
-	BUG_ON(obj_priv->active);
+		if (bsd_obj == NULL) {
+			*render_iter = (*render_iter)->next;
+			return render_obj;
+		}
 
-	/* Wait on the rendering and unbind the buffer. */
-	return i915_gem_object_unbind(obj);
+		/* XXX can we handle seqno wrapping? */
+		if (render_obj->last_rendering_seqno < bsd_obj->last_rendering_seqno) {
+			*render_iter = (*render_iter)->next;
+			return render_obj;
+		} else {
+			*bsd_iter = (*bsd_iter)->next;
+			return bsd_obj;
+		}
+	} else {
+		*render_iter = (*render_iter)->next;
+		return render_obj;
+	}
 }
 
-static void
-i915_gem_flush_ring(struct drm_device *dev,
-	       uint32_t invalidate_domains,
-	       uint32_t flush_domains,
-	       struct intel_ring_buffer *ring)
+static bool
+mark_free(struct drm_i915_gem_object *obj_priv,
+	   struct list_head *unwind)
 {
-	if (flush_domains & I915_GEM_DOMAIN_CPU)
-		drm_agp_chipset_flush(dev);
-	ring->flush(dev, ring,
-			invalidate_domains,
-			flush_domains);
+	list_add(&obj_priv->evict_list, unwind);
+	return drm_mm_scan_add_block(obj_priv->gtt_space);
 }
 
+#define i915_for_each_active_object(OBJ, R, B) \
+	*(R) = dev_priv->render_ring.active_list.next; \
+	*(B) = dev_priv->bsd_ring.active_list.next; \
+	while (((OBJ) = i915_gem_next_active_object(dev, (R), (B))) != NULL)
+
 int
-i915_gem_evict_something(struct drm_device *dev,
-			 int min_size, unsigned alignment)
+i915_gem_evict_something(struct drm_device *dev, int min_size, unsigned alignment)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	int ret, found;
-
-	struct intel_ring_buffer *render_ring = &dev_priv->render_ring;
-	struct intel_ring_buffer *bsd_ring = &dev_priv->bsd_ring;
-	for (;;) {
-		i915_gem_retire_requests(dev);
-
-		/* If there's an inactive buffer available now, grab it
-		 * and be done.
-		 */
-		ret = i915_gem_scan_inactive_list_and_evict(dev, min_size,
-							    alignment,
-							    &found);
-		if (found)
-			return ret;
+	struct list_head eviction_list, unwind_list;
+	struct drm_i915_gem_object *obj_priv, *tmp_obj_priv;
+	struct list_head *render_iter, *bsd_iter;
+	int ret = 0;
 
-		/* If we didn't get anything, but the ring is still processing
-		 * things, wait for the next to finish and hopefully leave us
-		 * a buffer to evict.
-		 */
-		if (!list_empty(&render_ring->request_list)) {
-			struct drm_i915_gem_request *request;
+	i915_gem_retire_requests(dev);
 
-			request = list_first_entry(&render_ring->request_list,
-						   struct drm_i915_gem_request,
-						   list);
+	/* Re-check for free space after retiring requests */
+	if (drm_mm_search_free(&dev_priv->mm.gtt_space,
+			       min_size, alignment, 0))
+		return 0;
 
-			ret = i915_do_wait_request(dev, request->seqno, true, request->ring);
-			if (ret)
-				return ret;
+	/*
+	 * The goal is to evict objects and amalgamate space in LRU order.
+	 * The oldest idle objects reside on the inactive list, which is in
+	 * retirement order. The next objects to retire are those on the (per
+	 * ring) active list that do not have an outstanding flush. Once the
+	 * hardware reports completion (the seqno is updated after the
+	 * batchbuffer has been finished) the clean buffer objects would
+	 * be retired to the inactive list. Any dirty objects would be added
+	 * to the tail of the flushing list. So after processing the clean
+	 * active objects we need to emit a MI_FLUSH to retire the flushing
+	 * list, hence the retirement order of the flushing list is in
+	 * advance of the dirty objects on the active lists.
+	 *
+	 * The retirement sequence is thus:
+	 *   1. Inactive objects (already retired)
+	 *   2. Clean active objects
+	 *   3. Flushing list
+	 *   4. Dirty active objects.
+	 *
+	 * On each list, the oldest objects lie at the HEAD with the freshest
+	 * object on the TAIL.
+	 */
+
+	INIT_LIST_HEAD(&unwind_list);
+	drm_mm_init_scan(&dev_priv->mm.gtt_space, min_size, alignment);
+
+	/* First see if there is a large enough contiguous idle region... */
+	list_for_each_entry(obj_priv, &dev_priv->mm.inactive_list, list) {
+		if (mark_free(obj_priv, &unwind_list))
+			goto found;
+	}
 
+	/* Now merge in the soon-to-be-expired objects... */
+	i915_for_each_active_object(obj_priv, &render_iter, &bsd_iter) {
+		/* Does the object require an outstanding flush? */
+		if (obj_priv->base.write_domain || obj_priv->pin_count)
 			continue;
-		}
 
-		if (HAS_BSD(dev) && !list_empty(&bsd_ring->request_list)) {
-			struct drm_i915_gem_request *request;
-
-			request = list_first_entry(&bsd_ring->request_list,
-						   struct drm_i915_gem_request,
-						   list);
+		if (mark_free(obj_priv, &unwind_list))
+			goto found;
+	}
 
-			ret = i915_do_wait_request(dev, request->seqno, true, request->ring);
-			if (ret)
-				return ret;
+	/* Finally add anything with a pending flush (in order of retirement) */
+	list_for_each_entry(obj_priv, &dev_priv->mm.flushing_list, list) {
+		if (obj_priv->pin_count)
+			continue;
 
+		if (mark_free(obj_priv, &unwind_list))
+			goto found;
+	}
+	i915_for_each_active_object(obj_priv, &render_iter, &bsd_iter) {
+		if (! obj_priv->base.write_domain || obj_priv->pin_count)
 			continue;
-		}
 
-		/* If we didn't have anything on the request list but there
-		 * are buffers awaiting a flush, emit one and try again.
-		 * When we wait on it, those buffers waiting for that flush
-		 * will get moved to inactive.
-		 */
-		if (!list_empty(&dev_priv->mm.flushing_list)) {
-			struct drm_gem_object *obj = NULL;
-			struct drm_i915_gem_object *obj_priv;
-
-			/* Find an object that we can immediately reuse */
-			list_for_each_entry(obj_priv, &dev_priv->mm.flushing_list, list) {
-				obj = &obj_priv->base;
-				if (obj->size >= min_size)
-					break;
-
-				obj = NULL;
-			}
-
-			if (obj != NULL) {
-				uint32_t seqno;
-
-				i915_gem_flush_ring(dev,
-					       obj->write_domain,
-					       obj->write_domain,
-					       obj_priv->ring);
-				seqno = i915_add_request(dev, NULL,
-						obj->write_domain,
-						obj_priv->ring);
-				if (seqno == 0)
-					return -ENOMEM;
-				continue;
-			}
+		if (mark_free(obj_priv, &unwind_list))
+			goto found;
+	}
+
+	/* Nothing found, clean up and bail out! */
+	list_for_each_entry(obj_priv, &unwind_list, evict_list) {
+		ret = drm_mm_scan_remove_block(obj_priv->gtt_space);
+		BUG_ON(ret);
+	}
+
+	/* We expect the caller to unpin, evict all and try again, or give up.
+	 * So calling i915_gem_evict_everything() is unnecessary.
+	 */
+	return -ENOSPC;
+
+found:
+	INIT_LIST_HEAD(&eviction_list);
+	list_for_each_entry_safe(obj_priv, tmp_obj_priv,
+				 &unwind_list, evict_list) {
+		if (drm_mm_scan_remove_block(obj_priv->gtt_space)) {
+			/* drm_mm doesn't allow any other other operations while
+			 * scanning, therefore store to be evicted objects on a
+			 * temporary list. */
+			list_move(&obj_priv->evict_list, &eviction_list);
 		}
+	}
 
-		/* If we didn't do any of the above, there's no single buffer
-		 * large enough to swap out for the new one, so just evict
-		 * everything and start again. (This should be rare.)
-		 */
-		if (!list_empty(&dev_priv->mm.inactive_list))
-			return i915_gem_evict_inactive(dev);
-		else
-			return i915_gem_evict_everything(dev);
+	/* Unbinding will emit any required flushes */
+	list_for_each_entry_safe(obj_priv, tmp_obj_priv,
+				 &eviction_list, evict_list) {
+#if WATCH_LRU
+		DRM_INFO("%s: evicting %p\n", __func__, obj);
+#endif
+		ret = i915_gem_object_unbind(&obj_priv->base);
+		if (ret)
+			return ret;
 	}
+
+	/* The just created free hole should be on the top of the free stack
+	 * maintained by drm_mm, so this BUG_ON actually executes in O(1).
+	 * Furthermore all accessed data has just recently been used, so it
+	 * should be really fast, too. */
+	BUG_ON(!drm_mm_search_free(&dev_priv->mm.gtt_space, min_size,
+				   alignment, 0));
+
+	return 0;
 }
 
 int
-- 
1.7.1

[PATCH 06/20] drm/i915: Maintain LRU order of inactive objects upon access by CPU

[Chris Wilson]

In order to reduce the penalty of fallbacks under memory pressure and to
avoid a potential immediate ping-pong of evicting a mmaped buffer, we
move the object to the tail of the inactive list when a page is freshly
faulted or the object is moved into the CPU domain.

We choose not to protect the CPU objects from casual eviction,
preferring to keep the GPU active for as long as possible.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b5a7b00..9211dda 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1036,6 +1036,10 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 		ret = i915_gem_object_set_to_cpu_domain(obj, write_domain != 0);
 	}
 
+	/* Maintain LRU order of "inactive" objects */
+	if (ret == 0 && obj_priv->gtt_space && !obj_priv->active)
+		list_move_tail(&obj_priv->list, &dev_priv->mm.inactive_list);
+
 	drm_gem_object_unreference(obj);
 	mutex_unlock(&dev->struct_mutex);
 	return ret;
@@ -1137,6 +1141,7 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct drm_gem_object *obj = vma->vm_private_data;
 	struct drm_device *dev = obj->dev;
+	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj_priv = to_intel_bo(obj);
 	pgoff_t page_offset;
 	unsigned long pfn;
@@ -1166,6 +1171,9 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 			goto unlock;
 	}
 
+	if (!obj_priv->active)
+		list_move_tail(&obj_priv->list, &dev_priv->mm.inactive_list);
+
 	pfn = ((dev->agp->base + obj_priv->gtt_offset) >> PAGE_SHIFT) +
 		page_offset;
 
-- 
1.7.1

Re: [PATCH 06/20] drm/i915: Maintain LRU order of inactive objects upon access by CPU

[Daniel Vetter]

On Sat, Aug 07, 2010 at 11:01:25AM +0100, Chris Wilson wrote:
> In order to reduce the penalty of fallbacks under memory pressure and to
> avoid a potential immediate ping-pong of evicting a mmaped buffer, we
> move the object to the tail of the inactive list when a page is freshly
> faulted or the object is moved into the CPU domain.
> 
> We choose not to protect the CPU objects from casual eviction,
> preferring to keep the GPU active for as long as possible.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

This patch is broken - you're missing an && !obj_priv->pin_count in both
if-clauses. Think frontbuffer rendering - that's at least how I've tracked
it down.

-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

[PATCH] drm/i915: Maintain LRU order of inactive objects upon access by CPU (v2)

[Chris Wilson]

In order to reduce the penalty of fallbacks under memory pressure and to
avoid a potential immediate ping-pong of evicting a mmaped buffer, we
move the object to the tail of the inactive list when a page is freshly
faulted or the object is moved into the CPU domain.

We choose not to protect the CPU objects from casual eviction,
preferring to keep the GPU active for as long as possible.

v2: Daniel Vetter found a bug where I forgot that pinned objects are
kept off the inactive list.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b5a7b00..8f3e0c1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -57,6 +57,14 @@ static void i915_gem_free_object_tail(struct drm_gem_object *obj);
 static LIST_HEAD(shrink_list);
 static DEFINE_SPINLOCK(shrink_list_lock);
 
+static inline bool
+i915_gem_object_is_inactive(struct drm_i915_gem_object *obj_priv)
+{
+	return obj_priv->gtt_space &&
+		!obj_priv->active &&
+		obj_priv->pin_count == 0;
+}
+
 int i915_gem_do_init(struct drm_device *dev, unsigned long start,
 		     unsigned long end)
 {
@@ -1036,6 +1044,11 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 		ret = i915_gem_object_set_to_cpu_domain(obj, write_domain != 0);
 	}
 
+	
+	/* Maintain LRU order of "inactive" objects */
+	if (ret == 0 && i915_gem_object_is_inactive(obj_priv))
+		list_move_tail(&obj_priv->list, &dev_priv->mm.inactive_list);
+
 	drm_gem_object_unreference(obj);
 	mutex_unlock(&dev->struct_mutex);
 	return ret;
@@ -1137,6 +1150,7 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct drm_gem_object *obj = vma->vm_private_data;
 	struct drm_device *dev = obj->dev;
+	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj_priv = to_intel_bo(obj);
 	pgoff_t page_offset;
 	unsigned long pfn;
@@ -1166,6 +1180,9 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 			goto unlock;
 	}
 
+	if (i915_gem_object_is_inactive(obj_priv))
+		list_move_tail(&obj_priv->list, &dev_priv->mm.inactive_list);
+
 	pfn = ((dev->agp->base + obj_priv->gtt_offset) >> PAGE_SHIFT) +
 		page_offset;
 
-- 
1.7.1

[PATCH 07/20] drm/i915: Record error batch buffers using iomem

[Chris Wilson]

Directly read the GTT mapping for the contents of the batch buffers
rather than relying on possibly stale CPU caches. Also for completeness
scan the flushing/inactive lists for the current buffers - we are
collecting error state after all.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_irq.c |   64 ++++++++++++++++++++++++++++++++++----
 1 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 854ab1e..5161cea 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -425,9 +425,11 @@ static struct drm_i915_error_object *
 i915_error_object_create(struct drm_device *dev,
 			 struct drm_gem_object *src)
 {
+	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct drm_i915_error_object *dst;
 	struct drm_i915_gem_object *src_priv;
 	int page, page_count;
+	u32 reloc_offset;
 
 	if (src == NULL)
 		return NULL;
@@ -442,18 +444,27 @@ i915_error_object_create(struct drm_device *dev,
 	if (dst == NULL)
 		return NULL;
 
+	reloc_offset = src_priv->gtt_offset;
 	for (page = 0; page < page_count; page++) {
-		void *s, *d = kmalloc(PAGE_SIZE, GFP_ATOMIC);
 		unsigned long flags;
+		void __iomem *s;
+		void *d;
 
+		d = kmalloc(PAGE_SIZE, GFP_ATOMIC);
 		if (d == NULL)
 			goto unwind;
+
 		local_irq_save(flags);
-		s = kmap_atomic(src_priv->pages[page], KM_IRQ0);
-		memcpy(d, s, PAGE_SIZE);
-		kunmap_atomic(s, KM_IRQ0);
+		s = io_mapping_map_atomic_wc(dev_priv->mm.gtt_mapping,
+					     reloc_offset,
+					     KM_IRQ0);
+		memcpy_fromio(d, s, PAGE_SIZE);
+		io_mapping_unmap_atomic(s, KM_IRQ0);
 		local_irq_restore(flags);
+
 		dst->pages[page] = d;
+
+		reloc_offset += PAGE_SIZE;
 	}
 	dst->page_count = page_count;
 	dst->gtt_offset = src_priv->gtt_offset;
@@ -613,18 +624,57 @@ static void i915_capture_error_state(struct drm_device *dev)
 
 		if (batchbuffer[1] == NULL &&
 		    error->acthd >= obj_priv->gtt_offset &&
-		    error->acthd < obj_priv->gtt_offset + obj->size &&
-		    batchbuffer[0] != obj)
+		    error->acthd < obj_priv->gtt_offset + obj->size)
 			batchbuffer[1] = obj;
 
 		count++;
 	}
+	/* Scan the other lists for completeness for those bizarre errors. */
+	if (batchbuffer[0] == NULL || batchbuffer[1] == NULL) {
+		list_for_each_entry(obj_priv, &dev_priv->mm.flushing_list, list) {
+			struct drm_gem_object *obj = &obj_priv->base;
+
+			if (batchbuffer[0] == NULL &&
+			    bbaddr >= obj_priv->gtt_offset &&
+			    bbaddr < obj_priv->gtt_offset + obj->size)
+				batchbuffer[0] = obj;
+
+			if (batchbuffer[1] == NULL &&
+			    error->acthd >= obj_priv->gtt_offset &&
+			    error->acthd < obj_priv->gtt_offset + obj->size)
+				batchbuffer[1] = obj;
+
+			if (batchbuffer[0] && batchbuffer[1])
+				break;
+		}
+	}
+	if (batchbuffer[0] == NULL || batchbuffer[1] == NULL) {
+		list_for_each_entry(obj_priv, &dev_priv->mm.inactive_list, list) {
+			struct drm_gem_object *obj = &obj_priv->base;
+
+			if (batchbuffer[0] == NULL &&
+			    bbaddr >= obj_priv->gtt_offset &&
+			    bbaddr < obj_priv->gtt_offset + obj->size)
+				batchbuffer[0] = obj;
+
+			if (batchbuffer[1] == NULL &&
+			    error->acthd >= obj_priv->gtt_offset &&
+			    error->acthd < obj_priv->gtt_offset + obj->size)
+				batchbuffer[1] = obj;
+
+			if (batchbuffer[0] && batchbuffer[1])
+				break;
+		}
+	}
 
 	/* We need to copy these to an anonymous buffer as the simplest
 	 * method to avoid being overwritten by userpace.
 	 */
 	error->batchbuffer[0] = i915_error_object_create(dev, batchbuffer[0]);
-	error->batchbuffer[1] = i915_error_object_create(dev, batchbuffer[1]);
+	if (batchbuffer[1] != batchbuffer[0])
+		error->batchbuffer[1] = i915_error_object_create(dev, batchbuffer[1]);
+	else
+		error->batchbuffer[1] = NULL;
 
 	/* Record the ringbuffer */
 	error->ringbuffer = i915_error_object_create(dev,
-- 
1.7.1

[PATCH 08/20] drm/i915/sdvo: Markup a few constant strings.

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_sdvo.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 1e31724..69cf724 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -50,7 +50,7 @@
 #define IS_TV_OR_LVDS(c) (c->output_flag & (SDVO_TV_MASK | SDVO_LVDS_MASK))
 
 
-static char *tv_format_names[] = {
+static const char *tv_format_names[] = {
 	"NTSC_M"   , "NTSC_J"  , "NTSC_443",
 	"PAL_B"    , "PAL_D"   , "PAL_G"   ,
 	"PAL_H"    , "PAL_I"   , "PAL_M"   ,
@@ -292,7 +292,7 @@ static bool intel_sdvo_write_byte(struct intel_sdvo *intel_sdvo, int addr, u8 ch
 /** Mapping of command numbers to names, for debug output */
 static const struct _sdvo_cmd_name {
 	u8 cmd;
-	char *name;
+	const char *name;
 } sdvo_cmd_names[] = {
     SDVO_CMD_NAME_ENTRY(SDVO_CMD_RESET),
     SDVO_CMD_NAME_ENTRY(SDVO_CMD_GET_DEVICE_CAPS),
-- 
1.7.1

[PATCH 09/20] drm/i915: Enable aspect/centering panel fitting for Ironlake.

[Chris Wilson]

[-- Attachment #1: Type: text/plain, Size: 11382 bytes --]

v2: Hook in DP paths to keep FULLSCREEN panel fitting on eDP.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/Makefile        |    1 +
 drivers/gpu/drm/i915/i915_drv.h      |    2 +
 drivers/gpu/drm/i915/intel_display.c |   16 ++---
 drivers/gpu/drm/i915/intel_dp.c      |   20 ++-----
 drivers/gpu/drm/i915/intel_drv.h     |    7 ++
 drivers/gpu/drm/i915/intel_lvds.c    |   32 +++-------
 drivers/gpu/drm/i915/intel_panel.c   |  111 ++++++++++++++++++++++++++++++++++
 7 files changed, 143 insertions(+), 46 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_panel.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 384fd45..5c8e534 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -19,6 +19,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \
 	  intel_hdmi.o \
 	  intel_sdvo.o \
 	  intel_modes.o \
+	  intel_panel.o \
 	  intel_i2c.o \
 	  intel_fb.o \
 	  intel_tv.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6221f23..4b0ffb6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -614,6 +614,8 @@ typedef struct drm_i915_private {
 	struct sdvo_device_mapping sdvo_mappings[2];
 	/* indicate whether the LVDS_BORDER should be enabled or not */
 	unsigned int lvds_border_bits;
+	/* Panel fitter placement and size for Ironlake+ */
+	u32 pch_pf_pos, pch_pf_size;
 
 	struct drm_crtc *plane_to_crtc_mapping[2];
 	struct drm_crtc *pipe_to_crtc_mapping[2];
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 24fbd0f..25e3866 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2005,15 +2005,13 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
 		/* Enable panel fitting for LVDS */
 		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)
 		    || HAS_eDP || intel_pch_has_edp(crtc)) {
-			temp = I915_READ(pf_ctl_reg);
-			I915_WRITE(pf_ctl_reg, temp | PF_ENABLE | PF_FILTER_MED_3x3);
-
-			/* currently full aspect */
-			I915_WRITE(pf_win_pos, 0);
-
-			I915_WRITE(pf_win_size,
-				   (dev_priv->panel_fixed_mode->hdisplay << 16) |
-				   (dev_priv->panel_fixed_mode->vdisplay));
+			if (dev_priv->pch_pf_size) {
+				temp = I915_READ(pf_ctl_reg);
+				I915_WRITE(pf_ctl_reg, temp | PF_ENABLE | PF_FILTER_MED_3x3);
+				I915_WRITE(pf_win_pos, dev_priv->pch_pf_pos);
+				I915_WRITE(pf_win_size, dev_priv->pch_pf_size);
+			} else
+				I915_WRITE(pf_ctl_reg, temp & ~PF_ENABLE);
 		}
 
 		/* Enable CPU pipe */
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c4c5868..cee5d9c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -523,21 +523,9 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct drm_display_mode *mode,
 
 	if ((IS_eDP(intel_dp) || IS_PCH_eDP(intel_dp)) &&
 	    dev_priv->panel_fixed_mode) {
-		struct drm_display_mode *fixed_mode = dev_priv->panel_fixed_mode;
-
-		adjusted_mode->hdisplay = fixed_mode->hdisplay;
-		adjusted_mode->hsync_start = fixed_mode->hsync_start;
-		adjusted_mode->hsync_end = fixed_mode->hsync_end;
-		adjusted_mode->htotal = fixed_mode->htotal;
-
-		adjusted_mode->vdisplay = fixed_mode->vdisplay;
-		adjusted_mode->vsync_start = fixed_mode->vsync_start;
-		adjusted_mode->vsync_end = fixed_mode->vsync_end;
-		adjusted_mode->vtotal = fixed_mode->vtotal;
-
-		adjusted_mode->clock = fixed_mode->clock;
-		drm_mode_set_crtcinfo(adjusted_mode, CRTC_INTERLACE_HALVE_V);
-
+		intel_fixed_panel_mode(dev_priv->panel_fixed_mode, adjusted_mode);
+		intel_pch_panel_fitting(dev, DRM_MODE_SCALE_FULLSCREEN,
+					mode, adjusted_mode);
 		/*
 		 * the mode->clock is used to calculate the Data&Link M/N
 		 * of the pipe. For the eDP the fixed clock should be used.
@@ -572,8 +560,10 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct drm_display_mode *mode,
 			      "count %d clock %d\n",
 			      intel_dp->link_bw, intel_dp->lane_count,
 			      adjusted_mode->clock);
+
 		return true;
 	}
+
 	return false;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a44b8cb..c552b06 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -186,6 +186,13 @@ extern bool intel_dpd_is_edp(struct drm_device *dev);
 extern void intel_edp_link_config (struct intel_encoder *, int *, int *);
 
 
+extern void intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
+				   struct drm_display_mode *adjusted_mode);
+extern void intel_pch_panel_fitting(struct drm_device *dev,
+				    int fitting_mode,
+				    struct drm_display_mode *mode,
+				    struct drm_display_mode *adjusted_mode);
+
 extern int intel_panel_fitter_pipe (struct drm_device *dev);
 extern void intel_crtc_load_lut(struct drm_crtc *crtc);
 extern void intel_encoder_prepare (struct drm_encoder *encoder);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 312ac30..cb5821e 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -246,26 +246,20 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 	/* If we don't have a panel mode, there is nothing we can do */
 	if (dev_priv->panel_fixed_mode == NULL)
 		return true;
+
 	/*
 	 * We have timings from the BIOS for the panel, put them in
 	 * to the adjusted mode.  The CRTC will be set up for this mode,
 	 * with the panel scaling set up to source from the H/VDisplay
 	 * of the original mode.
 	 */
-	adjusted_mode->hdisplay = dev_priv->panel_fixed_mode->hdisplay;
-	adjusted_mode->hsync_start =
-		dev_priv->panel_fixed_mode->hsync_start;
-	adjusted_mode->hsync_end =
-		dev_priv->panel_fixed_mode->hsync_end;
-	adjusted_mode->htotal = dev_priv->panel_fixed_mode->htotal;
-	adjusted_mode->vdisplay = dev_priv->panel_fixed_mode->vdisplay;
-	adjusted_mode->vsync_start =
-		dev_priv->panel_fixed_mode->vsync_start;
-	adjusted_mode->vsync_end =
-		dev_priv->panel_fixed_mode->vsync_end;
-	adjusted_mode->vtotal = dev_priv->panel_fixed_mode->vtotal;
-	adjusted_mode->clock = dev_priv->panel_fixed_mode->clock;
-	drm_mode_set_crtcinfo(adjusted_mode, CRTC_INTERLACE_HALVE_V);
+	intel_fixed_panel_mode(dev_priv->panel_fixed_mode, adjusted_mode);
+
+	if (HAS_PCH_SPLIT(dev)) {
+		intel_pch_panel_fitting(dev, intel_lvds->fitting_mode,
+					mode, adjusted_mode);
+		return true;
+	}
 
 	/* Make sure pre-965s set dither correctly */
 	if (!IS_I965G(dev)) {
@@ -278,10 +272,6 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 	    adjusted_mode->vdisplay == mode->vdisplay)
 		goto out;
 
-	/* full screen scale for now */
-	if (HAS_PCH_SPLIT(dev))
-		goto out;
-
 	/* 965+ wants fuzzy fitting */
 	if (IS_I965G(dev))
 		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) |
@@ -293,10 +283,8 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 	 * to register description and PRM.
 	 * Change the value here to see the borders for debugging
 	 */
-	if (!HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(BCLRPAT_A, 0);
-		I915_WRITE(BCLRPAT_B, 0);
-	}
+	I915_WRITE(BCLRPAT_A, 0);
+	I915_WRITE(BCLRPAT_B, 0);
 
 	switch (intel_lvds->fitting_mode) {
 	case DRM_MODE_SCALE_CENTER:
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
new file mode 100644
index 0000000..e7f5299
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -0,0 +1,111 @@
+/*
+ * Copyright © 2006-2010 Intel Corporation
+ * Copyright (c) 2006 Dave Airlie <airlied@linux.ie>
+ *
+ * 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:
+ *	Eric Anholt <eric@anholt.net>
+ *      Dave Airlie <airlied@linux.ie>
+ *      Jesse Barnes <jesse.barnes@intel.com>
+ *      Chris Wilson <chris@chris-wilson.co.uk>
+ */
+
+#include "intel_drv.h"
+
+void
+intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
+		       struct drm_display_mode *adjusted_mode)
+{
+	adjusted_mode->hdisplay = fixed_mode->hdisplay;
+	adjusted_mode->hsync_start = fixed_mode->hsync_start;
+	adjusted_mode->hsync_end = fixed_mode->hsync_end;
+	adjusted_mode->htotal = fixed_mode->htotal;
+
+	adjusted_mode->vdisplay = fixed_mode->vdisplay;
+	adjusted_mode->vsync_start = fixed_mode->vsync_start;
+	adjusted_mode->vsync_end = fixed_mode->vsync_end;
+	adjusted_mode->vtotal = fixed_mode->vtotal;
+
+	adjusted_mode->clock = fixed_mode->clock;
+
+	drm_mode_set_crtcinfo(adjusted_mode, CRTC_INTERLACE_HALVE_V);
+}
+
+/* adjusted_mode has been preset to be the panel's fixed mode */
+void
+intel_pch_panel_fitting(struct drm_device *dev,
+			int fitting_mode,
+			struct drm_display_mode *mode,
+			struct drm_display_mode *adjusted_mode)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int x, y, width, height;
+
+	x = y = width = height = 0;
+
+	/* Native modes don't need fitting */
+	if (adjusted_mode->hdisplay == mode->hdisplay &&
+	    adjusted_mode->vdisplay == mode->vdisplay)
+		goto done;
+
+	switch (fitting_mode) {
+	case DRM_MODE_SCALE_CENTER:
+		width = mode->hdisplay;
+		height = mode->vdisplay;
+		x = (adjusted_mode->hdisplay - width + 1)/2;
+		y = (adjusted_mode->vdisplay - height + 1)/2;
+		break;
+
+	case DRM_MODE_SCALE_ASPECT:
+		/* Scale but preserve the aspect ratio */
+		{
+			u32 scaled_width = adjusted_mode->hdisplay * mode->vdisplay;
+			u32 scaled_height = mode->hdisplay * adjusted_mode->vdisplay;
+			if (scaled_width > scaled_height) { /* pillar */
+				width = scaled_height / mode->vdisplay;
+				x = (adjusted_mode->hdisplay - width + 1) / 2;
+				y = 0;
+				height = adjusted_mode->vdisplay;
+			} else if (scaled_width < scaled_height) { /* letter */
+				height = scaled_width / mode->hdisplay;
+				y = (adjusted_mode->vdisplay - height + 1) / 2;
+				x = 0;
+				width = adjusted_mode->hdisplay;
+			} else {
+				x = y = 0;
+				width = adjusted_mode->hdisplay;
+				height = adjusted_mode->vdisplay;
+			}
+		}
+		break;
+
+	default:
+	case DRM_MODE_SCALE_FULLSCREEN:
+		x = y = 0;
+		width = adjusted_mode->hdisplay;
+		height = adjusted_mode->vdisplay;
+		break;
+	}
+
+done:
+	dev_priv->pch_pf_pos = (x << 16) | y;
+	dev_priv->pch_pf_size = (width << 16) | height;
+}
-- 
1.7.1


[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 10/20] drm/i915: Write to display base last.

[Chris Wilson]

Writing to the DSPBASE register triggers the double-buffered update to
all the control registers, so always write it last in the update
sequence.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 25e3866..874ae30 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1585,15 +1585,13 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		      Start, Offset, x, y, crtc->fb->pitch);
 	I915_WRITE(dspstride, crtc->fb->pitch);
 	if (IS_I965G(dev)) {
-		I915_WRITE(dspbase, Offset);
-		I915_READ(dspbase);
 		I915_WRITE(dspsurf, Start);
-		I915_READ(dspsurf);
 		I915_WRITE(dsptileoff, (y << 16) | x);
+		I915_WRITE(dspbase, Offset);
 	} else {
 		I915_WRITE(dspbase, Start + Offset);
-		I915_READ(dspbase);
 	}
+	POSTING_READ(dspbase);
 
 	if ((IS_I965G(dev) || plane == 0))
 		intel_update_fbc(crtc, &crtc->mode);
-- 
1.7.1

[PATCH 11/20] drm/i915: Truncate the shmem backing pages on purge

[Chris Wilson]

shmfs doesn't actually implement i_ops->truncate() so we were not
immedatiately releasing the backing purges when shrinking the gfx cache
under OOM. Instead use a combination of truncate_inode_pages() and
i_ops->truncate_range() as is used by shmem_delete_inode().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9211dda..6865202 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1496,9 +1496,16 @@ i915_gem_object_truncate(struct drm_gem_object *obj)
 	struct drm_i915_gem_object *obj_priv = to_intel_bo(obj);
 	struct inode *inode;
 
+	/* Our goal here is to return as much of the memory as
+	 * is possible back to the system as we are called from OOM.
+	 * To do this we must instruct the shmfs to drop all of its
+	 * backing pages, *now*. Here we mirror the actions taken
+	 * when by shmem_delete_inode() to release the backing store.
+	 */
 	inode = obj->filp->f_path.dentry->d_inode;
-	if (inode->i_op->truncate)
-		inode->i_op->truncate (inode);
+	truncate_inode_pages(inode->i_mapping, 0);
+	if (inode->i_op->truncate_range)
+		inode->i_op->truncate_range(inode, 0, (loff_t)-1);
 
 	obj_priv->madv = __I915_MADV_PURGED;
 }
-- 
1.7.1

[PATCH 12/20] drm/i915/display: Add pipe/plane information to dpms debugging

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 874ae30..07f1996 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1956,7 +1956,7 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
 	case DRM_MODE_DPMS_ON:
 	case DRM_MODE_DPMS_STANDBY:
 	case DRM_MODE_DPMS_SUSPEND:
-		DRM_DEBUG_KMS("crtc %d dpms on\n", pipe);
+		DRM_DEBUG_KMS("crtc %d/%d dpms on\n", pipe, plane);
 
 		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
 			temp = I915_READ(PCH_LVDS);
@@ -2142,10 +2142,10 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
 		intel_crtc_load_lut(crtc);
 
 		intel_update_fbc(crtc, &crtc->mode);
+		break;
 
-	break;
 	case DRM_MODE_DPMS_OFF:
-		DRM_DEBUG_KMS("crtc %d dpms off\n", pipe);
+		DRM_DEBUG_KMS("crtc %d/%d dpms off\n", pipe, plane);
 
 		drm_vblank_off(dev, pipe);
 		/* Disable display plane */
-- 
1.7.1

[PATCH 13/20] drm/i915/opregion: Use ASLE response codes defined in 0.1

[Chris Wilson]

Within i915_opregion.c there are two blocks of semantically identical
ASLE response codes defined. Only one of those matches the ACPI IGD
OpRegion Specification 0.1, use those.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Matthew Garrett <mjg59@srcf.ucam.org>
---
 drivers/gpu/drm/i915/i915_opregion.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_opregion.c b/drivers/gpu/drm/i915/i915_opregion.c
index 8fcc75c..ce402b2 100644
--- a/drivers/gpu/drm/i915/i915_opregion.c
+++ b/drivers/gpu/drm/i915/i915_opregion.c
@@ -114,10 +114,6 @@ struct opregion_asle {
 #define ASLE_REQ_MSK           0xf
 
 /* response bits of ASLE irq request */
-#define ASLE_ALS_ILLUM_FAIL    (2<<10)
-#define ASLE_BACKLIGHT_FAIL    (2<<12)
-#define ASLE_PFIT_FAIL         (2<<14)
-#define ASLE_PWM_FREQ_FAIL     (2<<16)
 #define ASLE_ALS_ILLUM_FAILED	(1<<10)
 #define ASLE_BACKLIGHT_FAILED	(1<<12)
 #define ASLE_PFIT_FAILED	(1<<14)
@@ -155,11 +151,11 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 	u32 max_backlight, level, shift;
 
 	if (!(bclp & ASLE_BCLP_VALID))
-		return ASLE_BACKLIGHT_FAIL;
+		return ASLE_BACKLIGHT_FAILED;
 
 	bclp &= ASLE_BCLP_MSK;
 	if (bclp < 0 || bclp > 255)
-		return ASLE_BACKLIGHT_FAIL;
+		return ASLE_BACKLIGHT_FAILED;
 
 	blc_pwm_ctl = I915_READ(BLC_PWM_CTL);
 	blc_pwm_ctl2 = I915_READ(BLC_PWM_CTL2);
@@ -211,7 +207,7 @@ static u32 asle_set_pfit(struct drm_device *dev, u32 pfit)
 	/* Panel fitting is currently controlled by the X code, so this is a
 	   noop until modesetting support works fully */
 	if (!(pfit & ASLE_PFIT_VALID))
-		return ASLE_PFIT_FAIL;
+		return ASLE_PFIT_FAILED;
 	return 0;
 }
 
-- 
1.7.1

[PATCH 14/20] drm/i915: Update watermarks for Ironlake after dpms changes

[Chris Wilson]

Previously, we only remembered to update the watermarks for i9xx, and
incorrectly assumed that the crtc->enabled flag was valid at that point
in the dpms cycle.

Note that on my x201s this makes a SR bug on pipe 1 much easier to hit.
(Since before this patch when disabling pipe 0, we either didn't update
the watermarks at all, or when we did we still thought we had two pipes
enabled and so disabled SR.)

References:

  Bug 28969 - [Arrandale] Screen flickers, suspect Self-Refresh
  https://bugs.freedesktop.org/show_bug.cgi?id=28969

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |   36 ++++++++++++++++++++-------------
 1 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 07f1996..1eae234 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2369,8 +2369,6 @@ static void i9xx_crtc_dpms(struct drm_crtc *crtc, int mode)
 	case DRM_MODE_DPMS_ON:
 	case DRM_MODE_DPMS_STANDBY:
 	case DRM_MODE_DPMS_SUSPEND:
-		intel_update_watermarks(dev);
-
 		/* Enable the DPLL */
 		temp = I915_READ(dpll_reg);
 		if ((temp & DPLL_VCO_ENABLE) == 0) {
@@ -2410,8 +2408,6 @@ static void i9xx_crtc_dpms(struct drm_crtc *crtc, int mode)
 		intel_crtc_dpms_overlay(intel_crtc, true);
 	break;
 	case DRM_MODE_DPMS_OFF:
-		intel_update_watermarks(dev);
-
 		/* Give the overlay scaler a chance to disable if it's on this pipe */
 		intel_crtc_dpms_overlay(intel_crtc, false);
 		drm_vblank_off(dev, pipe);
@@ -2476,12 +2472,26 @@ static void intel_crtc_dpms(struct drm_crtc *crtc, int mode)
 	int pipe = intel_crtc->pipe;
 	bool enabled;
 
-	dev_priv->display.dpms(crtc, mode);
-
 	intel_crtc->dpms_mode = mode;
-
 	intel_crtc->cursor_on = mode == DRM_MODE_DPMS_ON;
-	intel_crtc_update_cursor(crtc);
+
+	/* When switching on the display, ensure that SR is disabled
+	 * with multiple pipes prior to enabling to new pipe.
+	 *
+	 * When switching off the display, make sure the cursor is
+	 * properly hidden prior to disabling the pipe.
+	 */
+	if (mode == DRM_MODE_DPMS_ON)
+		intel_update_watermarks(dev);
+	else
+		intel_crtc_update_cursor(crtc);
+
+	dev_priv->display.dpms(crtc, mode);
+
+	if (mode == DRM_MODE_DPMS_ON)
+		intel_crtc_update_cursor(crtc);
+	else
+		intel_update_watermarks(dev);
 
 	if (!dev->primary->master)
 		return;
@@ -3362,12 +3372,11 @@ static void ironlake_update_wm(struct drm_device *dev,  int planea_clock,
 	int line_count;
 	int planea_htotal = 0, planeb_htotal = 0;
 	struct drm_crtc *crtc;
-	struct intel_crtc *intel_crtc;
 
 	/* Need htotal for all active display plane */
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-		intel_crtc = to_intel_crtc(crtc);
-		if (crtc->enabled) {
+		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+		if (intel_crtc->dpms_mode == DRM_MODE_DPMS_ON) {
 			if (intel_crtc->plane == 0)
 				planea_htotal = crtc->mode.htotal;
 			else
@@ -3527,7 +3536,6 @@ static void intel_update_watermarks(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
-	struct intel_crtc *intel_crtc;
 	int sr_hdisplay = 0;
 	unsigned long planea_clock = 0, planeb_clock = 0, sr_clock = 0;
 	int enabled = 0, pixel_size = 0;
@@ -3538,8 +3546,8 @@ static void intel_update_watermarks(struct drm_device *dev)
 
 	/* Get the clock config from both planes */
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-		intel_crtc = to_intel_crtc(crtc);
-		if (crtc->enabled) {
+		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+		if (intel_crtc->dpms_mode == DRM_MODE_DPMS_ON) {
 			enabled++;
 			if (intel_crtc->plane == 0) {
 				DRM_DEBUG_KMS("plane A (pipe %d) clock: %d\n",
-- 
1.7.1

[PATCH 15/20] drm/i915/ringbuffer: Set ring->gem_buffer = NULL on init unwind

[Chris Wilson]

The cleanup path for early abort failed to nullify the gem_buffer. The
likely consequence of this is zero, since a failure here should mean
aborting the module load.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3a02425..7823b96 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -608,9 +608,10 @@ err:
 int intel_init_ring_buffer(struct drm_device *dev,
 		struct intel_ring_buffer *ring)
 {
-	int ret;
 	struct drm_i915_gem_object *obj_priv;
 	struct drm_gem_object *obj;
+	int ret;
+
 	ring->dev = dev;
 
 	if (I915_NEED_GFX_HWS(dev)) {
@@ -623,16 +624,14 @@ int intel_init_ring_buffer(struct drm_device *dev,
 	if (obj == NULL) {
 		DRM_ERROR("Failed to allocate ringbuffer\n");
 		ret = -ENOMEM;
-		goto cleanup;
+		goto err_hws;
 	}
 
 	ring->gem_object = obj;
 
 	ret = i915_gem_object_pin(obj, ring->alignment);
-	if (ret != 0) {
-		drm_gem_object_unreference(obj);
-		goto cleanup;
-	}
+	if (ret)
+		goto err_unref;
 
 	obj_priv = to_intel_bo(obj);
 	ring->map.size = ring->size;
@@ -644,18 +643,14 @@ int intel_init_ring_buffer(struct drm_device *dev,
 	drm_core_ioremap_wc(&ring->map, dev);
 	if (ring->map.handle == NULL) {
 		DRM_ERROR("Failed to map ringbuffer.\n");
-		i915_gem_object_unpin(obj);
-		drm_gem_object_unreference(obj);
 		ret = -EINVAL;
-		goto cleanup;
+		goto err_unpin;
 	}
 
 	ring->virtual_start = ring->map.handle;
 	ret = ring->init(dev, ring);
-	if (ret != 0) {
-		intel_cleanup_ring_buffer(dev, ring);
-		return ret;
-	}
+	if (ret)
+		goto err_unmap;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		i915_kernel_lost_context(dev);
@@ -669,7 +664,15 @@ int intel_init_ring_buffer(struct drm_device *dev,
 	INIT_LIST_HEAD(&ring->active_list);
 	INIT_LIST_HEAD(&ring->request_list);
 	return ret;
-cleanup:
+
+err_unmap:
+	drm_core_ioremapfree(&ring->map, dev);
+err_unpin:
+	i915_gem_object_unpin(obj);
+err_unref:
+	drm_gem_object_unreference(obj);
+	ring->gem_object = NULL;
+err_hws:
 	cleanup_status_page(dev, ring);
 	return ret;
 }
-- 
1.7.1

[PATCH 16/20] drm/i915: Ensure that while(INREG()) are bounded (v2)

[Chris Wilson]

Add a new macro, wait_for, to simplify the act of waiting on a register
to change state. wait_for() takes three arguments, the condition to
inspect on every loop, the maximum amount of time to wait and whether to
yield the cpu for a length of time after each check.

v2: Upgrade failure messages to DRM_ERROR on the suggestion of
Eric Anholt. We do not expect to hit these conditions as they reflect
programming errors, so if we do we want to be notified.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_crt.c     |   17 ++++------
 drivers/gpu/drm/i915/intel_display.c |   58 +++++++--------------------------
 drivers/gpu/drm/i915/intel_dp.c      |   25 +++++---------
 drivers/gpu/drm/i915/intel_drv.h     |   14 ++++++++
 drivers/gpu/drm/i915/intel_lvds.c    |   12 +++----
 5 files changed, 48 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index cfcf854..c43176d 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -185,8 +185,9 @@ static bool intel_ironlake_crt_detect_hotplug(struct drm_connector *connector)
 	DRM_DEBUG_KMS("pch crt adpa 0x%x", adpa);
 	I915_WRITE(PCH_ADPA, adpa);
 
-	while ((I915_READ(PCH_ADPA) & ADPA_CRT_HOTPLUG_FORCE_TRIGGER) != 0)
-		;
+	if (wait_for((I915_READ(PCH_ADPA) & ADPA_CRT_HOTPLUG_FORCE_TRIGGER) == 0,
+		     1000, 1))
+		DRM_ERROR("timed out waiting for FORCE_TRIGGER");
 
 	if (HAS_PCH_CPT(dev)) {
 		I915_WRITE(PCH_ADPA, temp);
@@ -237,17 +238,13 @@ static bool intel_crt_detect_hotplug(struct drm_connector *connector)
 	hotplug_en |= CRT_HOTPLUG_FORCE_DETECT;
 
 	for (i = 0; i < tries ; i++) {
-		unsigned long timeout;
 		/* turn on the FORCE_DETECT */
 		I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
-		timeout = jiffies + msecs_to_jiffies(1000);
 		/* wait for FORCE_DETECT to go off */
-		do {
-			if (!(I915_READ(PORT_HOTPLUG_EN) &
-					CRT_HOTPLUG_FORCE_DETECT))
-				break;
-			msleep(1);
-		} while (time_after(timeout, jiffies));
+		if (wait_for((I915_READ(PORT_HOTPLUG_EN) &
+			      CRT_HOTPLUG_FORCE_DETECT) == 0,
+			     1000, 1))
+			DRM_ERROR("timed out waiting for FORCE_DETECT to go off");
 	}
 
 	stat = I915_READ(PORT_HOTPLUG_STAT);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1eae234..0bf683d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1037,7 +1037,6 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 void i8xx_disable_fbc(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	unsigned long timeout = jiffies + msecs_to_jiffies(1);
 	u32 fbc_ctl;
 
 	if (!I915_HAS_FBC(dev))
@@ -1052,12 +1051,9 @@ void i8xx_disable_fbc(struct drm_device *dev)
 	I915_WRITE(FBC_CONTROL, fbc_ctl);
 
 	/* Wait for compressing bit to clear */
-	while (I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) {
-		if (time_after(jiffies, timeout)) {
-			DRM_DEBUG_DRIVER("FBC idle timed out\n");
-			break;
-		}
-		; /* do nothing */
+	if (wait_for((I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) == 0, 10, 0)) {
+		DRM_DEBUG_KMS("FBC idle timed out\n");
+		return;
 	}
 
 	intel_wait_for_vblank(dev);
@@ -1943,7 +1939,6 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
 	int trans_vsync_reg = (pipe == 0) ? TRANS_VSYNC_A : TRANS_VSYNC_B;
 	int trans_dpll_sel = (pipe == 0) ? 0 : 1;
 	u32 temp;
-	int n;
 	u32 pipe_bpc;
 
 	temp = I915_READ(pipeconf_reg);
@@ -2134,9 +2129,8 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
 			I915_WRITE(transconf_reg, temp | TRANS_ENABLE);
 			I915_READ(transconf_reg);
 
-			while ((I915_READ(transconf_reg) & TRANS_STATE_ENABLE) == 0)
-				;
-
+			if (wait_for(I915_READ(transconf_reg) & TRANS_STATE_ENABLE, 10, 0))
+				DRM_ERROR("failed to enable transcoder\n");
 		}
 
 		intel_crtc_load_lut(crtc);
@@ -2167,20 +2161,10 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
 		temp = I915_READ(pipeconf_reg);
 		if ((temp & PIPEACONF_ENABLE) != 0) {
 			I915_WRITE(pipeconf_reg, temp & ~PIPEACONF_ENABLE);
-			I915_READ(pipeconf_reg);
-			n = 0;
+
 			/* wait for cpu pipe off, pipe state */
-			while ((I915_READ(pipeconf_reg) & I965_PIPECONF_ACTIVE) != 0) {
-				n++;
-				if (n < 60) {
-					udelay(500);
-					continue;
-				} else {
-					DRM_DEBUG_KMS("pipe %d off delay\n",
-								pipe);
-					break;
-				}
-			}
+			if (wait_for((I915_READ(pipeconf_reg) & I965_PIPECONF_ACTIVE) == 0, 50, 1))
+				DRM_ERROR("failed to turn off cpu pipe\n");
 		} else
 			DRM_DEBUG_KMS("crtc %d is disabled\n", pipe);
 
@@ -2241,20 +2225,10 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
 		temp = I915_READ(transconf_reg);
 		if ((temp & TRANS_ENABLE) != 0) {
 			I915_WRITE(transconf_reg, temp & ~TRANS_ENABLE);
-			I915_READ(transconf_reg);
-			n = 0;
+
 			/* wait for PCH transcoder off, transcoder state */
-			while ((I915_READ(transconf_reg) & TRANS_STATE_ENABLE) != 0) {
-				n++;
-				if (n < 60) {
-					udelay(500);
-					continue;
-				} else {
-					DRM_DEBUG_KMS("transcoder %d off "
-							"delay\n", pipe);
-					break;
-				}
-			}
+			if (wait_for((I915_READ(transconf_reg) & TRANS_STATE_ENABLE) == 0, 50, 1))
+				DRM_ERROR("failed to disable transcoder\n");
 		}
 
 		temp = I915_READ(transconf_reg);
@@ -5521,7 +5495,6 @@ void ironlake_enable_drps(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 rgvmodectl = I915_READ(MEMMODECTL);
 	u8 fmax, fmin, fstart, vstart;
-	int i = 0;
 
 	/* 100ms RC evaluation intervals */
 	I915_WRITE(RCUPEI, 100000);
@@ -5565,13 +5538,8 @@ void ironlake_enable_drps(struct drm_device *dev)
 	rgvmodectl |= MEMMODE_SWMODE_EN;
 	I915_WRITE(MEMMODECTL, rgvmodectl);
 
-	while (I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) {
-		if (i++ > 100) {
-			DRM_ERROR("stuck trying to change perf mode\n");
-			break;
-		}
-		msleep(1);
-	}
+	if (wait_for((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 1, 0))
+		DRM_ERROR("stuck trying to change perf mode\n");
 	msleep(1);
 
 	ironlake_set_drps(dev, fstart);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index cee5d9c..c6629bd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -759,22 +759,18 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 static void ironlake_edp_panel_on (struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	unsigned long timeout = jiffies + msecs_to_jiffies(5000);
-	u32 pp, pp_status;
+	u32 pp;
 
-	pp_status = I915_READ(PCH_PP_STATUS);
-	if (pp_status & PP_ON)
+	if (I915_READ(PCH_PP_STATUS) & PP_ON)
 		return;
 
 	pp = I915_READ(PCH_PP_CONTROL);
 	pp |= PANEL_UNLOCK_REGS | POWER_TARGET_ON;
 	I915_WRITE(PCH_PP_CONTROL, pp);
-	do {
-		pp_status = I915_READ(PCH_PP_STATUS);
-	} while (((pp_status & PP_ON) == 0) && !time_after(jiffies, timeout));
 
-	if (time_after(jiffies, timeout))
-		DRM_DEBUG_KMS("panel on wait timed out: 0x%08x\n", pp_status);
+	if (wait_for(I915_READ(PCH_PP_STATUS) & PP_ON, 5000, 10))
+		DRM_ERROR("panel on wait timed out: 0x%08x\n",
+			  I915_READ(PCH_PP_STATUS));
 
 	pp &= ~(PANEL_UNLOCK_REGS | EDP_FORCE_VDD);
 	I915_WRITE(PCH_PP_CONTROL, pp);
@@ -783,18 +779,15 @@ static void ironlake_edp_panel_on (struct drm_device *dev)
 static void ironlake_edp_panel_off (struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	unsigned long timeout = jiffies + msecs_to_jiffies(5000);
-	u32 pp, pp_status;
+	u32 pp;
 
 	pp = I915_READ(PCH_PP_CONTROL);
 	pp &= ~POWER_TARGET_ON;
 	I915_WRITE(PCH_PP_CONTROL, pp);
-	do {
-		pp_status = I915_READ(PCH_PP_STATUS);
-	} while ((pp_status & PP_ON) && !time_after(jiffies, timeout));
 
-	if (time_after(jiffies, timeout))
-		DRM_DEBUG_KMS("panel off wait timed out\n");
+	if (wait_for((I915_READ(PCH_PP_STATUS) & PP_ON) == 0, 5000, 10))
+		DRM_ERROR("panel off wait timed out: 0x%08x\n",
+			  I915_READ(PCH_PP_STATUS));
 
 	/* Make sure VDD is enabled so DP AUX will work */
 	pp |= EDP_FORCE_VDD;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c552b06..2a3eaaf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -32,6 +32,20 @@
 #include "drm_crtc.h"
 
 #include "drm_crtc_helper.h"
+
+#define wait_for(COND, MS, W) ({ \
+	unsigned long timeout__ = jiffies + msecs_to_jiffies(MS);	\
+	int ret__ = 0;							\
+	while (! (COND)) {						\
+		if (time_after(jiffies, timeout__)) {			\
+			ret__ = -ETIMEDOUT;				\
+			break;						\
+		}							\
+		if (W) msleep(W);					\
+	}								\
+	ret__;								\
+})
+
 /*
  * Display related stuff
  */
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index cb5821e..b819c10 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -96,7 +96,7 @@ static u32 intel_lvds_get_max_backlight(struct drm_device *dev)
 static void intel_lvds_set_power(struct drm_device *dev, bool on)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 pp_status, ctl_reg, status_reg, lvds_reg;
+	u32 ctl_reg, status_reg, lvds_reg;
 
 	if (HAS_PCH_SPLIT(dev)) {
 		ctl_reg = PCH_PP_CONTROL;
@@ -114,9 +114,8 @@ static void intel_lvds_set_power(struct drm_device *dev, bool on)
 
 		I915_WRITE(ctl_reg, I915_READ(ctl_reg) |
 			   POWER_TARGET_ON);
-		do {
-			pp_status = I915_READ(status_reg);
-		} while ((pp_status & PP_ON) == 0);
+		if (wait_for(I915_READ(status_reg) & PP_ON, 1000, 0))
+			DRM_ERROR("timed out waiting to enable LVDS pipe");
 
 		intel_lvds_set_backlight(dev, dev_priv->backlight_duty_cycle);
 	} else {
@@ -124,9 +123,8 @@ static void intel_lvds_set_power(struct drm_device *dev, bool on)
 
 		I915_WRITE(ctl_reg, I915_READ(ctl_reg) &
 			   ~POWER_TARGET_ON);
-		do {
-			pp_status = I915_READ(status_reg);
-		} while (pp_status & PP_ON);
+		if (wait_for((I915_READ(status_reg) & PP_ON) == 0, 1000, 0))
+			DRM_ERROR("timed out waiting for LVDS pipe to turn off");
 
 		I915_WRITE(lvds_reg, I915_READ(lvds_reg) & ~LVDS_PORT_EN);
 		POSTING_READ(lvds_reg);
-- 
1.7.1

Re: [PATCH 16/20] drm/i915: Ensure that while(INREG()) are bounded (v2)

[Carlos R. Mafra]

On Sat  7.Aug'10 at 11:01:35 +0100, Chris Wilson wrote:
>
> v2: Upgrade failure messages to DRM_ERROR on the suggestion of
> Eric Anholt. We do not expect to hit these conditions as they reflect
> programming errors, so if we do we want to be notified.

My dmesg from 2.6.36-rc5 is showing one of the messages introduced
in this patch. So I am notifying it :-)

> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -96,7 +96,7 @@ static u32 intel_lvds_get_max_backlight(struct drm_device *dev)
>  static void intel_lvds_set_power(struct drm_device *dev, bool on)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 pp_status, ctl_reg, status_reg, lvds_reg;
> +	u32 ctl_reg, status_reg, lvds_reg;
>  
>  	if (HAS_PCH_SPLIT(dev)) {
>  		ctl_reg = PCH_PP_CONTROL;
> @@ -114,9 +114,8 @@ static void intel_lvds_set_power(struct drm_device *dev, bool on)
>  
>  		I915_WRITE(ctl_reg, I915_READ(ctl_reg) |
>  			   POWER_TARGET_ON);
> -		do {
> -			pp_status = I915_READ(status_reg);
> -		} while ((pp_status & PP_ON) == 0);
> +		if (wait_for(I915_READ(status_reg) & PP_ON, 1000, 0))
> +			DRM_ERROR("timed out waiting to enable LVDS pipe");


[    0.954724] ACPI: Battery Slot [BAT0] (battery present)
[    2.266008] [drm:intel_lvds_set_power] *ERROR* timed out waiting to enable LVDS pipeConsole: switching to colour frame buffer device 160x50
[    2.274860] fb0: inteldrmfb frame buffer device

But I have this 1+ sec gap in there for as long as I remember.

Full dmesg is here:
http://www.aei.mpg.de/~crmafra/dmesg-2.6.36-rc5.txt

Is there anything else I can provide to help this?
I would love to get rid of the 1 sec delay there :-)

Re: [PATCH 16/20] drm/i915: Ensure that while(INREG()) are bounded (v2)

[Chris Wilson]

On Tue, 21 Sep 2010 16:22:00 +0200, "Carlos R. Mafra" <crmafra2@gmail.com> wrote:
> [    0.954724] ACPI: Battery Slot [BAT0] (battery present)
> [    2.266008] [drm:intel_lvds_set_power] *ERROR* timed out waiting to enable LVDS pipeConsole: switching to colour frame buffer device 160x50
> [    2.274860] fb0: inteldrmfb frame buffer device
> 
> But I have this 1+ sec gap in there for as long as I remember.
> 
> Full dmesg is here:
> http://www.aei.mpg.de/~crmafra/dmesg-2.6.36-rc5.txt
> 
> Is there anything else I can provide to help this?
> I would love to get rid of the 1 sec delay there :-)

Sure. Try

  git://git.kernel.org/pub/scm/linux/kernel/git/ickle/drm-intel.git
  drm-intel-next

which tries to avoid that busy spin.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 16/20] drm/i915: Ensure that while(INREG()) are bounded (v2)

[Jesse Barnes]

On Tue, 21 Sep 2010 15:35:27 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Tue, 21 Sep 2010 16:22:00 +0200, "Carlos R. Mafra" <crmafra2@gmail.com> wrote:
> > [    0.954724] ACPI: Battery Slot [BAT0] (battery present)
> > [    2.266008] [drm:intel_lvds_set_power] *ERROR* timed out waiting to enable LVDS pipeConsole: switching to colour frame buffer device 160x50
> > [    2.274860] fb0: inteldrmfb frame buffer device
> > 
> > But I have this 1+ sec gap in there for as long as I remember.
> > 
> > Full dmesg is here:
> > http://www.aei.mpg.de/~crmafra/dmesg-2.6.36-rc5.txt
> > 
> > Is there anything else I can provide to help this?
> > I would love to get rid of the 1 sec delay there :-)
> 
> Sure. Try
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/ickle/drm-intel.git
>   drm-intel-next

Panel power status affects whether other registers (timing, pipe
control, port) can be written, so not waiting for a power sequence to
complete can cause problems, so these patches make me a bit nervous.

I wonder why Carlos's power sequence is timing out after a second.
Maybe we're failing to program a required source bit before starting
his panel's power on sequence?  I ran into this when debugging some eDP
problems, it turned out we needed some unrelated clocks running just to
start the panel power sequence.  We can either program those clocks
around the panel power sequence, or possibly write the unlock value
before trying to turn on the panel.

Carlos, can you try adding

I915_WRITE(ctl_reg, I915_READ(ctl_reg) | PANEL_UNLOCK_REGS);
POSTING_READ(ctl_reg);

Above the actual write of the POWER_TARGET_ON value?  I wonder if it
will prevent the timeouts you've been seeing.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 16/20] drm/i915: Ensure that while(INREG()) are bounded (v2)

[Carlos R. Mafra]

On Di 21.Sep'10 at  8:55:44 -0700, Jesse Barnes wrote:
> On Tue, 21 Sep 2010 15:35:27 +0100
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > On Tue, 21 Sep 2010 16:22:00 +0200, "Carlos R. Mafra" <crmafra2@gmail.com> wrote:
> > > [    0.954724] ACPI: Battery Slot [BAT0] (battery present)
> > > [    2.266008] [drm:intel_lvds_set_power] *ERROR* timed out waiting to enable LVDS pipeConsole: switching to colour frame buffer device 160x50
> > > [    2.274860] fb0: inteldrmfb frame buffer device
> > > 
> > > But I have this 1+ sec gap in there for as long as I remember.
> > > 
> > > Full dmesg is here:
> > > http://www.aei.mpg.de/~crmafra/dmesg-2.6.36-rc5.txt
> > > 
> > > Is there anything else I can provide to help this?
> > > I would love to get rid of the 1 sec delay there :-)
> > 
> > Sure. Try
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/ickle/drm-intel.git
> >   drm-intel-next

I pulled that branch and tested it. The delay is gone but my external
monitor is not recognized.

[    0.951319] [drm] Initialized drm 1.1.0 20060810
[    0.951387] i915 0000:00:02.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
[    0.951439] i915 0000:00:02.0: setting latency timer to 64
[    0.959334] ACPI: Battery Slot [BAT0] (battery present)
[    1.006415] [drm] initialized overlay support
[    1.369450] Console: switching to colour frame buffer device 106x30
[    1.370675] fb0: inteldrmfb frame buffer device
[    1.370699] drm: registered panic notifier


> Panel power status affects whether other registers (timing, pipe
> control, port) can be written, so not waiting for a power sequence to
> complete can cause problems, so these patches make me a bit nervous.
> 
> I wonder why Carlos's power sequence is timing out after a second.
> Maybe we're failing to program a required source bit before starting
> his panel's power on sequence?  I ran into this when debugging some eDP
> problems, it turned out we needed some unrelated clocks running just to
> start the panel power sequence.  We can either program those clocks
> around the panel power sequence, or possibly write the unlock value
> before trying to turn on the panel.
> 
> Carlos, can you try adding
> 
> I915_WRITE(ctl_reg, I915_READ(ctl_reg) | PANEL_UNLOCK_REGS);
> POSTING_READ(ctl_reg);
> 
> Above the actual write of the POWER_TARGET_ON value?  I wonder if it
> will prevent the timeouts you've been seeing.

No, it didn't help. With the diff below on top of 2.6.36-rc5 I get this

[    0.951730] [drm] Initialized drm 1.1.0 20060810
[    0.951795] i915 0000:00:02.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
[    0.951847] i915 0000:00:02.0: setting latency timer to 64
[    0.955464] ACPI: Battery Slot [BAT0] (battery present)
[    1.004349] [drm] set up 7M of stolen space
[    1.009944] [drm] initialized overlay support
[    2.765007] [drm:intel_lvds_set_power] *ERROR* timed out waiting to enable LVDS pipeConsole: switching to colour frame buffer device 160x50
[    2.773828] fb0: inteldrmfb frame buffer device
[    2.773830] drm: registered panic notifier

Actual diff below, just to check if I got Jesse's suggestion right.

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 6ec39a8..56b4c0b 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -111,6 +111,8 @@ static void intel_lvds_set_power(struct drm_device *dev, bool on)
 	if (on) {
 		I915_WRITE(lvds_reg, I915_READ(lvds_reg) | LVDS_PORT_EN);
 		POSTING_READ(lvds_reg);
+		I915_WRITE(ctl_reg, I915_READ(ctl_reg) | PANEL_UNLOCK_REGS);
+		POSTING_READ(ctl_reg);
 
 		I915_WRITE(ctl_reg, I915_READ(ctl_reg) |
 			   POWER_TARGET_ON);

Re: [PATCH 16/20] drm/i915: Ensure that while(INREG()) are bounded (v2)

[Chris Wilson]

On Wed, 22 Sep 2010 10:32:55 +0200, "Carlos R. Mafra" <crmafra2@gmail.com> wrote:
> On Di 21.Sep'10 at  8:55:44 -0700, Jesse Barnes wrote:
> > On Tue, 21 Sep 2010 15:35:27 +0100
> > Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > 
> > > On Tue, 21 Sep 2010 16:22:00 +0200, "Carlos R. Mafra" <crmafra2@gmail.com> wrote:
> > > > [    0.954724] ACPI: Battery Slot [BAT0] (battery present)
> > > > [    2.266008] [drm:intel_lvds_set_power] *ERROR* timed out waiting to enable LVDS pipeConsole: switching to colour frame buffer device 160x50
> > > > [    2.274860] fb0: inteldrmfb frame buffer device
> > > > 
> > > > But I have this 1+ sec gap in there for as long as I remember.
> > > > 
> > > > Full dmesg is here:
> > > > http://www.aei.mpg.de/~crmafra/dmesg-2.6.36-rc5.txt
> > > > 
> > > > Is there anything else I can provide to help this?
> > > > I would love to get rid of the 1 sec delay there :-)
> > > 
> > > Sure. Try
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/ickle/drm-intel.git
> > >   drm-intel-next
> 
> I pulled that branch and tested it. The delay is gone but my external
> monitor is not recognized.

That'll be SDVO, I guess. On the one machine I have that actually uses
SDVO, it doesn't seem to like GMBUS. The joy of hardware.

If you can add drm.debug=0xe to your boot line, that would help with
identifying both issues.

Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 16/20] drm/i915: Ensure that while(INREG()) are bounded (v2)

[Carlos R. Mafra]

On Mi 22.Sep'10 at  9:43:47 +0100, Chris Wilson wrote:
> On Wed, 22 Sep 2010 10:32:55 +0200, "Carlos R. Mafra" <crmafra2@gmail.com> wrote:
> > On Di 21.Sep'10 at  8:55:44 -0700, Jesse Barnes wrote:
> > > On Tue, 21 Sep 2010 15:35:27 +0100
> > > Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > 
> > > > On Tue, 21 Sep 2010 16:22:00 +0200, "Carlos R. Mafra" <crmafra2@gmail.com> wrote:
> > > > > [    0.954724] ACPI: Battery Slot [BAT0] (battery present)
> > > > > [    2.266008] [drm:intel_lvds_set_power] *ERROR* timed out waiting to enable LVDS pipeConsole: switching to colour frame buffer device 160x50
> > > > > [    2.274860] fb0: inteldrmfb frame buffer device
> > > > > 
> > > > > But I have this 1+ sec gap in there for as long as I remember.
> > > > > 
> > > > > Full dmesg is here:
> > > > > http://www.aei.mpg.de/~crmafra/dmesg-2.6.36-rc5.txt
> > > > > 
> > > > > Is there anything else I can provide to help this?
> > > > > I would love to get rid of the 1 sec delay there :-)
> > > > 
> > > > Sure. Try
> > > > 
> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/ickle/drm-intel.git
> > > >   drm-intel-next
> > 
> > I pulled that branch and tested it. The delay is gone but my external
> > monitor is not recognized.
> 
> That'll be SDVO, I guess. On the one machine I have that actually uses
> SDVO, it doesn't seem to like GMBUS. The joy of hardware.

The external monitor is on VGA1 and it is capable of 1920x1080 resolution.

> If you can add drm.debug=0xe to your boot line, that would help with
> identifying both issues.

Ok, the dmesg with drm.debug=0xe is here:
http://www.aei.mpg.de/~crmafra/dmesg-drm.debug.txt

and the kernel I booted was 2.6.36-rc5 + the drm-intel-next branch.

If there is anything else I can do, let me know.

Re: [PATCH 16/20] drm/i915: Ensure that while(INREG()) are bounded (v2)

[Chris Wilson]

On Wed, 22 Sep 2010 12:48:23 +0200, "Carlos R. Mafra" <crmafra2@gmail.com> wrote:
> On Mi 22.Sep'10 at  9:43:47 +0100, Chris Wilson wrote:
> > That'll be SDVO, I guess. On the one machine I have that actually uses
> > SDVO, it doesn't seem to like GMBUS. The joy of hardware.
> 
> The external monitor is on VGA1 and it is capable of 1920x1080 resolution.

Ah, VGA. I'd just caught a regression with GMBUS and the handling of
crt_ddc_pin from VBIOS and pushed the fix to -next. That should restore
VGA. Still working on SDVO.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 16/20] drm/i915: Ensure that while(INREG()) are bounded (v2)

[Carlos R. Mafra]

On Mi 22.Sep'10 at 11:57:42 +0100, Chris Wilson wrote:
> On Wed, 22 Sep 2010 12:48:23 +0200, "Carlos R. Mafra" <crmafra2@gmail.com> wrote:
> > On Mi 22.Sep'10 at  9:43:47 +0100, Chris Wilson wrote:
> > > That'll be SDVO, I guess. On the one machine I have that actually uses
> > > SDVO, it doesn't seem to like GMBUS. The joy of hardware.
> > 
> > The external monitor is on VGA1 and it is capable of 1920x1080 resolution.
> 
> Ah, VGA. I'd just caught a regression with GMBUS and the handling of
> crt_ddc_pin from VBIOS and pushed the fix to -next. That should restore
> VGA. Still working on SDVO.

I pulled the branch again and the external monitor in the VGA port
still does not work (but no 1 sec delay).

Full dmesg here:
http://www.aei.mpg.de/~crmafra/dmesg-drm.debug2.txt

Re: [PATCH 16/20] drm/i915: Ensure that while(INREG()) are bounded (v2)

[Chris Wilson]

On Wed, 22 Sep 2010 15:42:55 +0200, "Carlos R. Mafra" <crmafra2@gmail.com> wrote:
> I pulled the branch again and the external monitor in the VGA port
> still does not work (but no 1 sec delay).

I overlooked this:

[    1.284053] [drm:drm_helper_probe_single_connector_modes],
[CONNECTOR:11:SVIDEO-1]
[    1.284057] [drm:drm_crtc_helper_set_mode], [CRTC:3]
[    1.284284] [drm:intel_crtc_mode_set], Mode for pipe A:
[    1.284287] [drm:drm_mode_debug_printmodeline], Modeline 0:"NTSC 480i"
0 107520 1280 1368 1496 1712 1024 1027 1034 1104 0x40 0x0
[    1.286026] [drm:drm_crtc_helper_set_mode], [ENCODER:12:TV-12] set
[MODE:0:NTSC 480i]
[    1.328023] [drm:intel_tv_detect_type], Detected S-Video TV connection
[    1.328043] [drm:i915_driver_irq_handler], 
[    1.328044] [drm:drm_helper_probe_single_connector_modes],
[CONNECTOR:11:SVIDEO-1] probed modes :
[    1.328047] [drm:drm_mode_debug_printmodeline], Modeline 35:"848x480"
30 14513 848 849 912 944 480 481 512 513 0x48 0x0
[    1.328051] [drm:drm_mode_debug_printmodeline], Modeline 32:"640x480"
30 11315 640 641 704 736 480 481 512 513 0x48 0x0
[    1.328056] [drm:drm_mode_debug_printmodeline], Modeline 34:"1024x768"
30 26886 1024 1025 1088 1120 768 769 800 801 0x40 0x0
[    1.328060] [drm:drm_mode_debug_printmodeline], Modeline 33:"800x600"
30 16998 800 801 864 896 600 601 632 633 0x40 0x0
...
[    1.366106] [drm:drm_mode_debug_printmodeline], Modeline 36:"1280x800"
60 68880 1280 1296 1344 1410 800 801 804 815 0x8 0xa
[    1.366110] [drm:drm_crtc_helper_set_config], encoder changed, full
mode switch
[    1.366112] [drm:drm_crtc_helper_set_config], crtc changed, full mode
switch
[    1.366115] [drm:drm_crtc_helper_set_config], [CONNECTOR:5:LVDS-1] to
[CRTC:4]
[    1.366117] [drm:drm_crtc_helper_set_config], [CONNECTOR:11:SVIDEO-1]
to [CRTC:3]
[    1.366120] [drm:drm_crtc_helper_set_config], attempting to set mode
from userspace

So we detect a rogue SVideo connection and do not have a free crtc for the
valid VGA output. The implication is that the TV detection is not working,
can you add a msleep(30) to intel_tv.c::intel_tv_detect_type() after the
POSTING_READ:

diff --git a/drivers/gpu/drm/i915/intel_tv.c
b/drivers/gpu/drm/i915/intel_tv.c
index 49ab11c..364a2f3 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1271,8 +1271,11 @@ intel_tv_detect_type (struct intel_tv *intel_tv)
        I915_WRITE(TV_DAC, tv_dac);
        POSTING_READ(TV_DAC);
 
+       msleep(30);
+

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 16/20] drm/i915: Ensure that while(INREG()) are bounded (v2)

[Carlos R. Mafra]

On Mi 22.Sep'10 at 15:29:56 +0100, Chris Wilson wrote:
> On Wed, 22 Sep 2010 15:42:55 +0200, "Carlos R. Mafra" <crmafra2@gmail.com> wrote:
> > I pulled the branch again and the external monitor in the VGA port
> > still does not work (but no 1 sec delay).
> 
> I overlooked this:
> 
> [    1.284053] [drm:drm_helper_probe_single_connector_modes],
> [CONNECTOR:11:SVIDEO-1]
> [    1.284057] [drm:drm_crtc_helper_set_mode], [CRTC:3]
> [    1.284284] [drm:intel_crtc_mode_set], Mode for pipe A:
> [    1.284287] [drm:drm_mode_debug_printmodeline], Modeline 0:"NTSC 480i"
> 0 107520 1280 1368 1496 1712 1024 1027 1034 1104 0x40 0x0
> [    1.286026] [drm:drm_crtc_helper_set_mode], [ENCODER:12:TV-12] set
> [MODE:0:NTSC 480i]
> [    1.328023] [drm:intel_tv_detect_type], Detected S-Video TV connection
> [    1.328043] [drm:i915_driver_irq_handler], 
> [    1.328044] [drm:drm_helper_probe_single_connector_modes],
> [CONNECTOR:11:SVIDEO-1] probed modes :
> [    1.328047] [drm:drm_mode_debug_printmodeline], Modeline 35:"848x480"
> 30 14513 848 849 912 944 480 481 512 513 0x48 0x0
> [    1.328051] [drm:drm_mode_debug_printmodeline], Modeline 32:"640x480"
> 30 11315 640 641 704 736 480 481 512 513 0x48 0x0
> [    1.328056] [drm:drm_mode_debug_printmodeline], Modeline 34:"1024x768"
> 30 26886 1024 1025 1088 1120 768 769 800 801 0x40 0x0
> [    1.328060] [drm:drm_mode_debug_printmodeline], Modeline 33:"800x600"
> 30 16998 800 801 864 896 600 601 632 633 0x40 0x0
> ...
> [    1.366106] [drm:drm_mode_debug_printmodeline], Modeline 36:"1280x800"
> 60 68880 1280 1296 1344 1410 800 801 804 815 0x8 0xa
> [    1.366110] [drm:drm_crtc_helper_set_config], encoder changed, full
> mode switch
> [    1.366112] [drm:drm_crtc_helper_set_config], crtc changed, full mode
> switch
> [    1.366115] [drm:drm_crtc_helper_set_config], [CONNECTOR:5:LVDS-1] to
> [CRTC:4]
> [    1.366117] [drm:drm_crtc_helper_set_config], [CONNECTOR:11:SVIDEO-1]
> to [CRTC:3]
> [    1.366120] [drm:drm_crtc_helper_set_config], attempting to set mode
> from userspace
> 
> So we detect a rogue SVideo connection and do not have a free crtc for the
> valid VGA output. The implication is that the TV detection is not working,
> can you add a msleep(30) to intel_tv.c::intel_tv_detect_type() after the
> POSTING_READ:

Oh, but the laptop has a SVideo connection. It's not connected to anything
though.

> diff --git a/drivers/gpu/drm/i915/intel_tv.c
> b/drivers/gpu/drm/i915/intel_tv.c
> index 49ab11c..364a2f3 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1271,8 +1271,11 @@ intel_tv_detect_type (struct intel_tv *intel_tv)
>         I915_WRITE(TV_DAC, tv_dac);
>         POSTING_READ(TV_DAC);
>  
> +       msleep(30);
> +

I did this.
Now there is an image in VGA output, but it detected the wrong resolutions:

[mafra@Pilar:~]$ xrandr
Screen 0: minimum 320 x 200, current 1280 x 800, maximum 8192 x 8192
LVDS1 connected 1280x800+0+0 (normal left inverted right x axis y axis) 0mm x 0mm
   1280x800       59.9*+
   1024x768       85.0     75.0     70.1     60.0
   832x624        74.6
   800x600        85.1     72.2     75.0     60.3     56.2
   640x480        85.0     72.8     75.0     59.9
   720x400        85.0
   640x400        85.1
   640x350        85.1
VGA1 connected 1024x768+0+0 (normal left inverted right x axis y axis) 0mm x 0mm
   1024x768       60.0*
   800x600        60.3     56.2
   848x480        60.0
   640x480        59.9     59.9
TV1 unknown connection (normal left inverted right x axis y axis)
   848x480        30.0 +
   640x480        30.0 +
   1024x768       30.0
   800x600        30.0
   
I also don't remember the TV1 from before (the other output
in the laptop is a S-Video).

The new dmesg is here:
http://www.aei.mpg.de/~crmafra/dmesg-drm.debug3.txt

Re: [PATCH 16/20] drm/i915: Ensure that while(INREG()) are bounded (v2)

[Chris Wilson]

On Wed, 22 Sep 2010 17:26:45 +0200, "Carlos R. Mafra" <crmafra2@gmail.com> wrote:
> On Mi 22.Sep'10 at 15:29:56 +0100, Chris Wilson wrote:
> > So we detect a rogue SVideo connection and do not have a free crtc for the
> > valid VGA output. The implication is that the TV detection is not working,
> > can you add a msleep(30) to intel_tv.c::intel_tv_detect_type() after the
> > POSTING_READ:
> 
> I did this.
> Now there is an image in VGA output, but it detected the wrong resolutions:

Ok, this confirms that the TV DAC state change detection needs a little
more refinement. I'd like to be able to do away with the sleep there, but
there really may be no other method to ensure the PLLs are stable before
enabling the detector. Alas.

A slight change in behaviour is that if we already have two outputs lit,
we cannot perform a load detection on the TV connector and so we report it
as unknown (if forced). In theory this should allow the user to reconfigure
the outputs in fewer steps (and certainly not prevent valid changes).

However, it appears VGA EDID probing is still broken. The crt_ddc_pin fix
worked-for-me, however my Crestline box is still not happy, so I am not
yet able to confirm whether or not its VGA is working.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 17/20] drm/i915/edp: Flush the write before waiting for PLLs

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0bf683d..2a32a7b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1665,6 +1665,7 @@ static void ironlake_enable_pll_edp (struct drm_crtc *crtc)
 	dpa_ctl = I915_READ(DP_A);
 	dpa_ctl |= DP_PLL_ENABLE;
 	I915_WRITE(DP_A, dpa_ctl);
+	POSTING_READ(DP_A);
 	udelay(200);
 }
 
-- 
1.7.1

[PATCH 18/20] drm/i915: FBC is updated within set_base() so remove second call in mode_set()

[Chris Wilson]

The FBC is dependent upon a few details of the framebuffer so it is
required to be updated within set_base(), so remove the redundant call
from mode_set().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2a32a7b..41b4caf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4171,9 +4171,6 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
 	/* Flush the plane changes */
 	ret = intel_pipe_set_base(crtc, x, y, old_fb);
 
-	if ((IS_I965G(dev) || plane == 0))
-		intel_update_fbc(crtc, &crtc->mode);
-
 	intel_update_watermarks(dev);
 
 	drm_vblank_post_modeset(dev, pipe);
-- 
1.7.1

[PATCH 19/20] drm/i915: Only update i845/i865 CURBASE when disabled (v2)

[Chris Wilson]

The i845 and i865 have a peculiarlity in that CURBASE is not the trigger
for the vsync update of the cursor registers but instead the
modification of that register is prohibited whilst the cursor is
enabled. Reorder the write sequence for CURPOS, CURCNTR and CURBASE on
i845 to i865 to match.

v2: Remove the checks for i845/i865 from within i9xx_cursor_update()

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |   91 +++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_drv.h     |    2 +-
 2 files changed, 63 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 41b4caf..6490d8b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4204,6 +4204,62 @@ void intel_crtc_load_lut(struct drm_crtc *crtc)
 	}
 }
 
+static void i845_update_cursor(struct drm_crtc *crtc, u32 base)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	bool visible = base != 0;
+	u32 cntl;
+
+	if (intel_crtc->cursor_visible == visible)
+		return;
+
+	cntl = I915_READ(CURACNTR);
+	if (visible) {
+		/* On these chipsets we can only modify the base whilst
+		 * the cursor is disabled.
+		 */
+		I915_WRITE(CURABASE, base);
+
+		cntl &= ~(CURSOR_FORMAT_MASK);
+		/* XXX width must be 64, stride 256 => 0x00 << 28 */
+		cntl |= CURSOR_ENABLE |
+			CURSOR_GAMMA_ENABLE |
+			CURSOR_FORMAT_ARGB;
+	} else
+		cntl &= ~(CURSOR_ENABLE | CURSOR_GAMMA_ENABLE);
+	I915_WRITE(CURACNTR, cntl);
+
+	intel_crtc->cursor_visible = visible;
+}
+
+static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int pipe = intel_crtc->pipe;
+	bool visible = base != 0;
+
+	if (intel_crtc->cursor_visible != visible) {
+		uint32_t cntl = I915_READ(pipe == 0 ? CURACNTR : CURBCNTR);
+		if (base) {
+			cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
+			cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+			cntl |= pipe << 28; /* Connect to correct pipe */
+		} else {
+			cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
+			cntl |= CURSOR_MODE_DISABLE;
+		}
+		I915_WRITE(pipe == 0 ? CURACNTR : CURBCNTR, cntl);
+
+		intel_crtc->cursor_visible = visible;
+	}
+	/* and commit changes on next vblank */
+	I915_WRITE(pipe == 0 ? CURABASE : CURBBASE, base);
+}
+
 /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
 static void intel_crtc_update_cursor(struct drm_crtc *crtc)
 {
@@ -4213,7 +4269,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc)
 	int pipe = intel_crtc->pipe;
 	int x = intel_crtc->cursor_x;
 	int y = intel_crtc->cursor_y;
-	uint32_t base, pos;
+	u32 base, pos;
 	bool visible;
 
 	pos = 0;
@@ -4247,37 +4303,14 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc)
 	pos |= y << CURSOR_Y_SHIFT;
 
 	visible = base != 0;
-	if (!visible && !intel_crtc->cursor_visble)
+	if (!visible && !intel_crtc->cursor_visible)
 		return;
 
 	I915_WRITE(pipe == 0 ? CURAPOS : CURBPOS, pos);
-	if (intel_crtc->cursor_visble != visible) {
-		uint32_t cntl = I915_READ(pipe == 0 ? CURACNTR : CURBCNTR);
-		if (base) {
-			/* Hooray for CUR*CNTR differences */
-			if (IS_MOBILE(dev) || IS_I9XX(dev)) {
-				cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
-				cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
-				cntl |= pipe << 28; /* Connect to correct pipe */
-			} else {
-				cntl &= ~(CURSOR_FORMAT_MASK);
-				cntl |= CURSOR_ENABLE;
-				cntl |= CURSOR_FORMAT_ARGB | CURSOR_GAMMA_ENABLE;
-			}
-		} else {
-			if (IS_MOBILE(dev) || IS_I9XX(dev)) {
-				cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
-				cntl |= CURSOR_MODE_DISABLE;
-			} else {
-				cntl &= ~(CURSOR_ENABLE | CURSOR_GAMMA_ENABLE);
-			}
-		}
-		I915_WRITE(pipe == 0 ? CURACNTR : CURBCNTR, cntl);
-
-		intel_crtc->cursor_visble = visible;
-	}
-	/* and commit changes on next vblank */
-	I915_WRITE(pipe == 0 ? CURABASE : CURBBASE, base);
+	if (IS_845G(dev) || IS_I865G(dev))
+		i845_update_cursor(crtc, base);
+	else
+		i9xx_update_cursor(crtc, base);
 
 	if (visible)
 		intel_mark_busy(dev, to_intel_framebuffer(crtc->fb)->obj);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2a3eaaf..6ba56e1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -168,7 +168,7 @@ struct intel_crtc {
 	uint32_t cursor_addr;
 	int16_t cursor_x, cursor_y;
 	int16_t cursor_width, cursor_height;
-	bool cursor_visble, cursor_on;
+	bool cursor_visible, cursor_on;
 };
 
 #define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
-- 
1.7.1

[PATCH 20/20] drm/i915: Apply i830 errata for cursor alignment

[Chris Wilson]

i830 requires 32bpp cursors to be aligned to 16KB, so we have to expose
the alignment parameter to i915_gem_attach_phys_object().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h      |    4 +++-
 drivers/gpu/drm/i915/i915_gem.c      |   11 ++++++-----
 drivers/gpu/drm/i915/intel_display.c |    4 +++-
 drivers/gpu/drm/i915/intel_overlay.c |    3 ++-
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4b0ffb6..8df6ac7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1000,7 +1000,9 @@ int i915_gem_object_set_to_gtt_domain(struct drm_gem_object *obj,
 				      int write);
 int i915_gem_object_set_to_display_plane(struct drm_gem_object *obj);
 int i915_gem_attach_phys_object(struct drm_device *dev,
-				struct drm_gem_object *obj, int id);
+				struct drm_gem_object *obj,
+				int id,
+				int align);
 void i915_gem_detach_phys_object(struct drm_device *dev,
 				 struct drm_gem_object *obj);
 void i915_gem_free_all_phys_object(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6865202..c8c6ecf 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4665,7 +4665,7 @@ i915_gem_load(struct drm_device *dev)
  * e.g. for cursor + overlay regs
  */
 int i915_gem_init_phys_object(struct drm_device *dev,
-			      int id, int size)
+			      int id, int size, int align)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct drm_i915_gem_phys_object *phys_obj;
@@ -4680,7 +4680,7 @@ int i915_gem_init_phys_object(struct drm_device *dev,
 
 	phys_obj->id = id;
 
-	phys_obj->handle = drm_pci_alloc(dev, size, 0);
+	phys_obj->handle = drm_pci_alloc(dev, size, align);
 	if (!phys_obj->handle) {
 		ret = -ENOMEM;
 		goto kfree_obj;
@@ -4762,7 +4762,9 @@ out:
 
 int
 i915_gem_attach_phys_object(struct drm_device *dev,
-			    struct drm_gem_object *obj, int id)
+			    struct drm_gem_object *obj,
+			    int id,
+			    int align)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj_priv;
@@ -4781,11 +4783,10 @@ i915_gem_attach_phys_object(struct drm_device *dev,
 		i915_gem_detach_phys_object(dev, obj);
 	}
 
-
 	/* create a new object */
 	if (!dev_priv->mm.phys_objs[id - 1]) {
 		ret = i915_gem_init_phys_object(dev, id,
-						obj->size);
+						obj->size, align);
 		if (ret) {
 			DRM_ERROR("failed to init phys object %d size: %zu\n", id, obj->size);
 			goto out;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6490d8b..53f3a98 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4375,8 +4375,10 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 
 		addr = obj_priv->gtt_offset;
 	} else {
+		int align = IS_I830(dev) ? 16 * 1024 : 256;
 		ret = i915_gem_attach_phys_object(dev, bo,
-						  (intel_crtc->pipe == 0) ? I915_GEM_PHYS_CURSOR_0 : I915_GEM_PHYS_CURSOR_1);
+						  (intel_crtc->pipe == 0) ? I915_GEM_PHYS_CURSOR_0 : I915_GEM_PHYS_CURSOR_1,
+						  align);
 		if (ret) {
 			DRM_ERROR("failed to attach phys object\n");
 			goto fail_locked;
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 9ae61aa..4f00390 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1367,7 +1367,8 @@ void intel_setup_overlay(struct drm_device *dev)
 		overlay->flip_addr = overlay->reg_bo->gtt_offset;
 	} else {
 		ret = i915_gem_attach_phys_object(dev, reg_bo,
-				I915_GEM_PHYS_OVERLAY_REGS);
+						  I915_GEM_PHYS_OVERLAY_REGS,
+						  0);
                 if (ret) {
                         DRM_ERROR("failed to attach phys overlay regs\n");
                         goto out_free_bo;
-- 
1.7.1

Re: [PATCH 20/20] drm/i915: Apply i830 errata for cursor alignment

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 253 bytes --]

On Sat,  7 Aug 2010 11:01:39 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> i830 requires 32bpp cursors to be aligned to 16KB, so we have to expose
> the alignment parameter to i915_gem_attach_phys_object().

Applied the whole series.  Thanks!

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx