All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review
@ 2013-08-26 22:50 Rodrigo Vivi
  2013-08-26 22:50 ` [PATCH 01/17] drm/i915: Do not add an interrupt for a context switch Rodrigo Vivi
                   ` (17 more replies)
  0 siblings, 18 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:50 UTC (permalink / raw)
  To: intel-gfx

Hi all,

Let me introduce drm-intel-collector branch:
http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector

To describe drm-intel-collector I'll quote Daniel:
"The overall idea is to make sure that simple patches don't get lost.
Bigger patch series or feature work tends to not get lost, and really
trivial patches I tend to merge right away. But 1-2 patch stuff in
between is occasionally lost"

Process:

1. Daniel pushs drm-intel-testing
2. I rebase drm-intel-collector onto drm-intel-testing
3. I collect all simple (1-2) patches that wasn't yet reviewed and not queued by Daniel
4. Request automated QA's PRTS automated i-g-t tests comparing drm-intel-testing x drm-intel-collector
5. If tests are ok I send the patches as a series to intel-gfx mailing list for better tracking and to be reviewed.

There are some reasons that some patches can be left behind:
1. It was send so long time ago. I started with patches from Jul 26th.
2. Your patch didn't applied cleanly and I couldn't easily solve the conflicts.
3. Kernel didn't compiled with your patch.
4. I simply missed it. If you believe this is the case please warn me.

Please help me to get these patches reviewed and queued by Daniel.

Also, please let me know if you have further ideas how to improve this process.

Thanks in advance,
Rodrigo.

Chris Wilson (13):
  drm/i915: Do not add an interrupt for a context switch
  drm/i915: Rearrange the comments in i915_add_request()
  drm/i915: Pin pages whilst mapping the dma-buf
  drm/i915: Cancel outstanding modeset workers before suspend
  drm/i915: Always prefer CPU relocations with LLC
  drm/i915: Report requested frequency alongside current frequency in
    debugfs
  drm/i915: Move the conditional seqno query into the tracepoint
  drm/i915: Add some missing steps to i915_driver_load error path
  drm/i915: Asynchronously perform the set-base for a simple modeset
  drm/i915: Align tiled scanouts from stolen memory to 256k in the GTT
  drm/i915: Apply the force-detect VGA w/a to Valleyview
  drm/i915: Pair seqno completion tracepoint with its dispatch
  RFM drm/i915: Boost RPS frequency for CPU stalls

Daniel Vetter (1):
  drm/i915: check that the i965g/gm 4G limit is really obeyed

Jesse Barnes (2):
  drm/i915: split PCI IDs out into i915_drm.h v4
  x86: add early quirk for reserving Intel graphics stolen memory v5

Rodrigo Vivi (1):
  drm/i915: Enable Lower Slice on Haswell GT3.

 arch/x86/kernel/early-quirks.c             | 154 +++++++++++++++++++++
 drivers/gpu/drm/i915/i915_debugfs.c        |  11 +-
 drivers/gpu/drm/i915/i915_dma.c            |  19 ++-
 drivers/gpu/drm/i915/i915_drv.c            | 164 +++++-----------------
 drivers/gpu/drm/i915/i915_drv.h            |   3 +
 drivers/gpu/drm/i915/i915_gem.c            |  25 +++-
 drivers/gpu/drm/i915/i915_gem_context.c    |  12 +-
 drivers/gpu/drm/i915/i915_gem_dmabuf.c     |  41 +++---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   7 +-
 drivers/gpu/drm/i915/i915_irq.c            |   2 +-
 drivers/gpu/drm/i915/i915_reg.h            |  20 +--
 drivers/gpu/drm/i915/i915_trace.h          |  33 +++--
 drivers/gpu/drm/i915/intel_crt.c           |   2 +-
 drivers/gpu/drm/i915/intel_display.c       |  34 +++--
 include/drm/i915_drm.h                     |  34 +++++
 include/drm/i915_pciids.h                  | 211 +++++++++++++++++++++++++++++
 16 files changed, 566 insertions(+), 206 deletions(-)
 create mode 100644 include/drm/i915_pciids.h

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 01/17] drm/i915: Do not add an interrupt for a context switch
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
@ 2013-08-26 22:50 ` Rodrigo Vivi
  2013-08-30 14:17   ` Damien Lespiau
  2013-08-26 22:50 ` [PATCH 02/17] drm/i915: Rearrange the comments in i915_add_request() Rodrigo Vivi
                   ` (16 subsequent siblings)
  17 siblings, 1 reply; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:50 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Paulo Zanoni

From: Chris Wilson <chris@chris-wilson.co.uk>

We use the request to ensure we hold a reference to the context for the
duration that it remains in use by the ring. Each request only holds a
reference to the current context, hence we emit a request after
switching contexts with the final reference to the old context. However,
the extra interrupt caused by that request is not useful (no timing
critical function will wait for the context object), instead the overhead
of servicing the IRQ shows up in some (lightweight) benchmarks. In order
to keep the useful property of using the request to manage the context
lifetime, we want to add a dummy request that is associated with the
interrupt from the subsequent real request following the batch.

The extra interrupt was added as a side-effect of using
i915_add_request() in

commit 112522f6789581824903f6f72082b5b841a7f0f9
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu May 2 16:48:07 2013 +0300

    drm/i915: put context upon switching

v2: Daniel convinced me that the request here was solely for context
lifetime tracking and that we have the active ref to keep the object
alive whilst the MI_SET_CONTEXT. So the only concern then is which
context should get the blame for MI_SET_CONTEXT failing. The old scheme
added a request for the old context so that any hang upto and including
the switch away would mark the old context as guilty. Now any hang here
implicates the new context. However since we have already gone through a
complete flush with the last context in its last request, and all that
lies in no-man's-land is an invalidate flush and the MI_SET_CONTEXT, we
should be safe in not unduly placing blame on the new context.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         |  1 -
 drivers/gpu/drm/i915/i915_gem_context.c | 12 +-----------
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2d1cb10..56c9104 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2046,7 +2046,6 @@ int __i915_add_request(struct intel_ring_buffer *ring,
 	if (request == NULL)
 		return -ENOMEM;
 
-
 	/* Record the position of the start of the request so that
 	 * should we detect the updated seqno part-way through the
 	 * GPU processing the request, we never over-estimate the
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 403309c..b6da70b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -451,17 +451,7 @@ static int do_switch(struct i915_hw_context *to)
 		from->obj->dirty = 1;
 		BUG_ON(from->obj->ring != ring);
 
-		ret = i915_add_request(ring, NULL);
-		if (ret) {
-			/* Too late, we've already scheduled a context switch.
-			 * Try to undo the change so that the hw state is
-			 * consistent with out tracking. In case of emergency,
-			 * scream.
-			 */
-			WARN_ON(mi_set_context(ring, from, MI_RESTORE_INHIBIT));
-			return ret;
-		}
-
+		/* obj is kept alive until the next request by its active ref */
 		i915_gem_object_unpin(from->obj);
 		i915_gem_context_unreference(from);
 	}
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 02/17] drm/i915: Rearrange the comments in i915_add_request()
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
  2013-08-26 22:50 ` [PATCH 01/17] drm/i915: Do not add an interrupt for a context switch Rodrigo Vivi
@ 2013-08-26 22:50 ` Rodrigo Vivi
  2013-08-30 14:21   ` Damien Lespiau
  2013-08-26 22:50 ` [PATCH 03/17] drm/i915: Pin pages whilst mapping the dma-buf Rodrigo Vivi
                   ` (15 subsequent siblings)
  17 siblings, 1 reply; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:50 UTC (permalink / raw)
  To: intel-gfx

From: Chris Wilson <chris@chris-wilson.co.uk>

The comments were a little out-of-sequence with the code, forcing the
reader to jump around whilst reading. Whilst moving the comments around,
add one to explain the context reference.

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

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 56c9104..d5c8a02 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2063,8 +2063,6 @@ int __i915_add_request(struct intel_ring_buffer *ring,
 	request->ring = ring;
 	request->head = request_start;
 	request->tail = request_ring_position;
-	request->ctx = ring->last_context;
-	request->batch_obj = obj;
 
 	/* Whilst this request exists, batch_obj will be on the
 	 * active_list, and so will hold the active reference. Only when this
@@ -2072,7 +2070,12 @@ int __i915_add_request(struct intel_ring_buffer *ring,
 	 * inactive_list and lose its active reference. Hence we do not need
 	 * to explicitly hold another reference here.
 	 */
+	request->batch_obj = obj;
 
+	/* Hold a reference to the current context so that we can inspect
+	 * it later in case a hangcheck error event fires.
+	 */
+	request->ctx = ring->last_context;
 	if (request->ctx)
 		i915_gem_context_reference(request->ctx);
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 03/17] drm/i915: Pin pages whilst mapping the dma-buf
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
  2013-08-26 22:50 ` [PATCH 01/17] drm/i915: Do not add an interrupt for a context switch Rodrigo Vivi
  2013-08-26 22:50 ` [PATCH 02/17] drm/i915: Rearrange the comments in i915_add_request() Rodrigo Vivi
@ 2013-08-26 22:50 ` Rodrigo Vivi
  2013-08-30 14:27   ` Damien Lespiau
  2013-08-26 22:50 ` [PATCH 04/17] drm/i915: check that the i965g/gm 4G limit is really obeyed Rodrigo Vivi
                   ` (14 subsequent siblings)
  17 siblings, 1 reply; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:50 UTC (permalink / raw)
  To: intel-gfx

From: Chris Wilson <chris@chris-wilson.co.uk>

As we attempt to kmalloc after calling get_pages, there is a possibility
that the shrinker may reap the pages we just acquired. To prevent this
we need to increment the pages_pin_count early, so rearrange the code
and error paths to make it so.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_dmabuf.c | 41 ++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index e918b05..7d5752f 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -42,27 +42,24 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme
 
 	ret = i915_mutex_lock_interruptible(obj->base.dev);
 	if (ret)
-		return ERR_PTR(ret);
+		goto err;
 
 	ret = i915_gem_object_get_pages(obj);
-	if (ret) {
-		st = ERR_PTR(ret);
-		goto out;
-	}
+	if (ret)
+		goto err_unlock;
+
+	i915_gem_object_pin_pages(obj);
 
 	/* Copy sg so that we make an independent mapping */
 	st = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
 	if (st == NULL) {
-		st = ERR_PTR(-ENOMEM);
-		goto out;
+		ret = -ENOMEM;
+		goto err_unpin;
 	}
 
 	ret = sg_alloc_table(st, obj->pages->nents, GFP_KERNEL);
-	if (ret) {
-		kfree(st);
-		st = ERR_PTR(ret);
-		goto out;
-	}
+	if (ret)
+		goto err_free;
 
 	src = obj->pages->sgl;
 	dst = st->sgl;
@@ -73,17 +70,23 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme
 	}
 
 	if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir)) {
-		sg_free_table(st);
-		kfree(st);
-		st = ERR_PTR(-ENOMEM);
-		goto out;
+		ret =-ENOMEM;
+		goto err_free_sg;
 	}
 
-	i915_gem_object_pin_pages(obj);
-
-out:
 	mutex_unlock(&obj->base.dev->struct_mutex);
 	return st;
+
+err_free_sg:
+	sg_free_table(st);
+err_free:
+	kfree(st);
+err_unpin:
+	i915_gem_object_unpin_pages(obj);
+err_unlock:
+	mutex_unlock(&obj->base.dev->struct_mutex);
+err:
+	return ERR_PTR(ret);
 }
 
 static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 04/17] drm/i915: check that the i965g/gm 4G limit is really obeyed
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (2 preceding siblings ...)
  2013-08-26 22:50 ` [PATCH 03/17] drm/i915: Pin pages whilst mapping the dma-buf Rodrigo Vivi
@ 2013-08-26 22:50 ` Rodrigo Vivi
  2013-08-26 22:50 ` [PATCH 05/17] drm/i915: Cancel outstanding modeset workers before suspend Rodrigo Vivi
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:50 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

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

In truly crazy circumstances shmem might give us the wrong type of
page. So be a bit paranoid and double check this.

Cc: Rob Clark <robdclark@gmail.com>
References: http://lkml.org/lkml/2011/7/11/238
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d5c8a02..7c92923 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1829,6 +1829,9 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 			sg->length += PAGE_SIZE;
 		}
 		last_pfn = page_to_pfn(page);
+
+		/* Check that the i965g/gm workaround works. */
+		WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x00100000UL));
 	}
 #ifdef CONFIG_SWIOTLB
 	if (!swiotlb_nr_tbl())
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 05/17] drm/i915: Cancel outstanding modeset workers before suspend
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (3 preceding siblings ...)
  2013-08-26 22:50 ` [PATCH 04/17] drm/i915: check that the i965g/gm 4G limit is really obeyed Rodrigo Vivi
@ 2013-08-26 22:50 ` Rodrigo Vivi
  2013-08-26 22:50 ` [PATCH 06/17] drm/i915: split PCI IDs out into i915_drm.h v4 Rodrigo Vivi
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:50 UTC (permalink / raw)
  To: intel-gfx; +Cc: Liu, Chuansheng

From: Chris Wilson <chris@chris-wilson.co.uk>

Upon resume we will do a complete restoration of the mode and so reset
all tasks, but during suspend (and unload) we need to make sure that no
workers run concurrently with our suspend code. Or worse just after.

The issue was first raised whilst tackling a suspend issue with Takashi
Iwai (http://lists.freedesktop.org/archives/intel-gfx/2012-April/016738.html)
and then it was independently rediscovered by Chuanshen Lui.

v2: Rebase for the lost year.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Takashi Iwai <tiwai@suse.de>
Cc: "Liu, Chuansheng" <chuansheng.liu@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8ee15b4..7ac5ca5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2199,6 +2199,7 @@ extern void intel_modeset_init_hw(struct drm_device *dev);
 extern void intel_modeset_suspend_hw(struct drm_device *dev);
 extern void intel_modeset_init(struct drm_device *dev);
 extern void intel_modeset_gem_init(struct drm_device *dev);
+extern void intel_modeset_quiesce(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
 extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
 extern void intel_modeset_setup_hw_state(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bcb62fe..74dbc68 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10030,6 +10030,7 @@ void intel_modeset_init_hw(struct drm_device *dev)
 
 void intel_modeset_suspend_hw(struct drm_device *dev)
 {
+	intel_modeset_quiesce(dev);
 	intel_suspend_hw(dev);
 }
 
@@ -10474,9 +10475,19 @@ void intel_modeset_gem_init(struct drm_device *dev)
 	intel_modeset_setup_hw_state(dev, false);
 }
 
-void intel_modeset_cleanup(struct drm_device *dev)
+void intel_modeset_quiesce(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	cancel_work_sync(&dev_priv->hotplug_work);
+	cancel_work_sync(&dev_priv->rps.work);
+
+	/* catch all required for dev_priv->wq */
+	flush_scheduled_work();
+}
+
+void intel_modeset_cleanup(struct drm_device *dev)
+{
 	struct drm_crtc *crtc;
 
 	/*
@@ -10485,7 +10496,10 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	 * experience fancy races otherwise.
 	 */
 	drm_irq_uninstall(dev);
-	cancel_work_sync(&dev_priv->hotplug_work);
+
+	/* flush any delayed tasks or pending work */
+	intel_modeset_quiesce(dev);
+
 	/*
 	 * Due to the hpd irq storm handling the hotplug work can re-arm the
 	 * poll handlers. Hence disable polling after hpd handling is shut down.
@@ -10512,9 +10526,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
 
 	mutex_unlock(&dev->struct_mutex);
 
-	/* flush any delayed tasks or pending work */
-	flush_scheduled_work();
-
 	/* destroy backlight, if any, before the connectors */
 	intel_panel_destroy_backlight(dev);
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 06/17] drm/i915: split PCI IDs out into i915_drm.h v4
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (4 preceding siblings ...)
  2013-08-26 22:50 ` [PATCH 05/17] drm/i915: Cancel outstanding modeset workers before suspend Rodrigo Vivi
@ 2013-08-26 22:50 ` Rodrigo Vivi
  2013-08-29 17:27   ` Ben Widawsky
  2013-08-26 22:50 ` [PATCH 07/17] x86: add early quirk for reserving Intel graphics stolen memory v5 Rodrigo Vivi
                   ` (11 subsequent siblings)
  17 siblings, 1 reply; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:50 UTC (permalink / raw)
  To: intel-gfx

From: Jesse Barnes <jbarnes@virtuousgeek.org>

For use by userspace (at some point in the future) and other kernel code.

v2: move PCI IDs to uabi (Chris)
    move PCI IDs to drm/ (Dave)
v3: fixup Quanta detection - needs to come first (Daniel)
v4: fix up PCI match structure init for easier use by userspace (Chris)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c | 164 +++++++------------------------
 include/drm/i915_drm.h          |   2 +
 include/drm/i915_pciids.h       | 211 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 247 insertions(+), 130 deletions(-)
 create mode 100644 include/drm/i915_pciids.h

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 735dd56..3872f66 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -157,25 +157,6 @@ MODULE_PARM_DESC(prefault_disable,
 static struct drm_driver driver;
 extern int intel_agp_enabled;
 
-#define INTEL_VGA_DEVICE(id, info) {		\
-	.class = PCI_BASE_CLASS_DISPLAY << 16,	\
-	.class_mask = 0xff0000,			\
-	.vendor = 0x8086,			\
-	.device = id,				\
-	.subvendor = PCI_ANY_ID,		\
-	.subdevice = PCI_ANY_ID,		\
-	.driver_data = (unsigned long) info }
-
-#define INTEL_QUANTA_VGA_DEVICE(info) {		\
-	.class = PCI_BASE_CLASS_DISPLAY << 16,	\
-	.class_mask = 0xff0000,			\
-	.vendor = 0x8086,			\
-	.device = 0x16a,			\
-	.subvendor = 0x152d,			\
-	.subdevice = 0x8990,			\
-	.driver_data = (unsigned long) info }
-
-
 static const struct intel_device_info intel_i830_info = {
 	.gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2,
 	.has_overlay = 1, .overlay_needs_physical = 1,
@@ -350,118 +331,41 @@ static const struct intel_device_info intel_haswell_m_info = {
 	.has_vebox_ring = 1,
 };
 
+/*
+ * Make sure any device matches here are from most specific to most
+ * general.  For example, since the Quanta match is based on the subsystem
+ * and subvendor IDs, we need it to come before the more general IVB
+ * PCI ID matches, otherwise we'll use the wrong info struct above.
+ */
+#define INTEL_PCI_IDS \
+	INTEL_I830_IDS(&intel_i830_info),	\
+	INTEL_I845G_IDS(&intel_845g_info),	\
+	INTEL_I85X_IDS(&intel_i85x_info),	\
+	INTEL_I865G_IDS(&intel_i865g_info),	\
+	INTEL_I915G_IDS(&intel_i915g_info),	\
+	INTEL_I915GM_IDS(&intel_i915gm_info),	\
+	INTEL_I945G_IDS(&intel_i945g_info),	\
+	INTEL_I945GM_IDS(&intel_i945gm_info),	\
+	INTEL_I965G_IDS(&intel_i965g_info),	\
+	INTEL_G33_IDS(&intel_g33_info),		\
+	INTEL_I965GM_IDS(&intel_i965gm_info),	\
+	INTEL_GM45_IDS(&intel_gm45_info), 	\
+	INTEL_G45_IDS(&intel_g45_info), 	\
+	INTEL_PINEVIEW_IDS(&intel_pineview_info),	\
+	INTEL_IRONLAKE_D_IDS(&intel_ironlake_d_info),	\
+	INTEL_IRONLAKE_M_IDS(&intel_ironlake_m_info),	\
+	INTEL_SNB_D_IDS(&intel_sandybridge_d_info),	\
+	INTEL_SNB_M_IDS(&intel_sandybridge_m_info),	\
+	INTEL_IVB_Q_IDS(&intel_ivybridge_q_info), /* must be first IVB */ \
+	INTEL_IVB_M_IDS(&intel_ivybridge_m_info),	\
+	INTEL_IVB_D_IDS(&intel_ivybridge_d_info),	\
+	INTEL_HSW_D_IDS(&intel_haswell_d_info), \
+	INTEL_HSW_M_IDS(&intel_haswell_m_info), \
+	INTEL_VLV_M_IDS(&intel_valleyview_m_info),	\
+	INTEL_VLV_D_IDS(&intel_valleyview_d_info)
+
 static const struct pci_device_id pciidlist[] = {		/* aka */
-	INTEL_VGA_DEVICE(0x3577, &intel_i830_info),		/* I830_M */
-	INTEL_VGA_DEVICE(0x2562, &intel_845g_info),		/* 845_G */
-	INTEL_VGA_DEVICE(0x3582, &intel_i85x_info),		/* I855_GM */
-	INTEL_VGA_DEVICE(0x358e, &intel_i85x_info),
-	INTEL_VGA_DEVICE(0x2572, &intel_i865g_info),		/* I865_G */
-	INTEL_VGA_DEVICE(0x2582, &intel_i915g_info),		/* I915_G */
-	INTEL_VGA_DEVICE(0x258a, &intel_i915g_info),		/* E7221_G */
-	INTEL_VGA_DEVICE(0x2592, &intel_i915gm_info),		/* I915_GM */
-	INTEL_VGA_DEVICE(0x2772, &intel_i945g_info),		/* I945_G */
-	INTEL_VGA_DEVICE(0x27a2, &intel_i945gm_info),		/* I945_GM */
-	INTEL_VGA_DEVICE(0x27ae, &intel_i945gm_info),		/* I945_GME */
-	INTEL_VGA_DEVICE(0x2972, &intel_i965g_info),		/* I946_GZ */
-	INTEL_VGA_DEVICE(0x2982, &intel_i965g_info),		/* G35_G */
-	INTEL_VGA_DEVICE(0x2992, &intel_i965g_info),		/* I965_Q */
-	INTEL_VGA_DEVICE(0x29a2, &intel_i965g_info),		/* I965_G */
-	INTEL_VGA_DEVICE(0x29b2, &intel_g33_info),		/* Q35_G */
-	INTEL_VGA_DEVICE(0x29c2, &intel_g33_info),		/* G33_G */
-	INTEL_VGA_DEVICE(0x29d2, &intel_g33_info),		/* Q33_G */
-	INTEL_VGA_DEVICE(0x2a02, &intel_i965gm_info),		/* I965_GM */
-	INTEL_VGA_DEVICE(0x2a12, &intel_i965gm_info),		/* I965_GME */
-	INTEL_VGA_DEVICE(0x2a42, &intel_gm45_info),		/* GM45_G */
-	INTEL_VGA_DEVICE(0x2e02, &intel_g45_info),		/* IGD_E_G */
-	INTEL_VGA_DEVICE(0x2e12, &intel_g45_info),		/* Q45_G */
-	INTEL_VGA_DEVICE(0x2e22, &intel_g45_info),		/* G45_G */
-	INTEL_VGA_DEVICE(0x2e32, &intel_g45_info),		/* G41_G */
-	INTEL_VGA_DEVICE(0x2e42, &intel_g45_info),		/* B43_G */
-	INTEL_VGA_DEVICE(0x2e92, &intel_g45_info),		/* B43_G.1 */
-	INTEL_VGA_DEVICE(0xa001, &intel_pineview_info),
-	INTEL_VGA_DEVICE(0xa011, &intel_pineview_info),
-	INTEL_VGA_DEVICE(0x0042, &intel_ironlake_d_info),
-	INTEL_VGA_DEVICE(0x0046, &intel_ironlake_m_info),
-	INTEL_VGA_DEVICE(0x0102, &intel_sandybridge_d_info),
-	INTEL_VGA_DEVICE(0x0112, &intel_sandybridge_d_info),
-	INTEL_VGA_DEVICE(0x0122, &intel_sandybridge_d_info),
-	INTEL_VGA_DEVICE(0x0106, &intel_sandybridge_m_info),
-	INTEL_VGA_DEVICE(0x0116, &intel_sandybridge_m_info),
-	INTEL_VGA_DEVICE(0x0126, &intel_sandybridge_m_info),
-	INTEL_VGA_DEVICE(0x010A, &intel_sandybridge_d_info),
-	INTEL_VGA_DEVICE(0x0156, &intel_ivybridge_m_info), /* GT1 mobile */
-	INTEL_VGA_DEVICE(0x0166, &intel_ivybridge_m_info), /* GT2 mobile */
-	INTEL_VGA_DEVICE(0x0152, &intel_ivybridge_d_info), /* GT1 desktop */
-	INTEL_VGA_DEVICE(0x0162, &intel_ivybridge_d_info), /* GT2 desktop */
-	INTEL_VGA_DEVICE(0x015a, &intel_ivybridge_d_info), /* GT1 server */
-	INTEL_QUANTA_VGA_DEVICE(&intel_ivybridge_q_info), /* Quanta transcode */
-	INTEL_VGA_DEVICE(0x016a, &intel_ivybridge_d_info), /* GT2 server */
-	INTEL_VGA_DEVICE(0x0402, &intel_haswell_d_info), /* GT1 desktop */
-	INTEL_VGA_DEVICE(0x0412, &intel_haswell_d_info), /* GT2 desktop */
-	INTEL_VGA_DEVICE(0x0422, &intel_haswell_d_info), /* GT3 desktop */
-	INTEL_VGA_DEVICE(0x040a, &intel_haswell_d_info), /* GT1 server */
-	INTEL_VGA_DEVICE(0x041a, &intel_haswell_d_info), /* GT2 server */
-	INTEL_VGA_DEVICE(0x042a, &intel_haswell_d_info), /* GT3 server */
-	INTEL_VGA_DEVICE(0x0406, &intel_haswell_m_info), /* GT1 mobile */
-	INTEL_VGA_DEVICE(0x0416, &intel_haswell_m_info), /* GT2 mobile */
-	INTEL_VGA_DEVICE(0x0426, &intel_haswell_m_info), /* GT2 mobile */
-	INTEL_VGA_DEVICE(0x040B, &intel_haswell_d_info), /* GT1 reserved */
-	INTEL_VGA_DEVICE(0x041B, &intel_haswell_d_info), /* GT2 reserved */
-	INTEL_VGA_DEVICE(0x042B, &intel_haswell_d_info), /* GT3 reserved */
-	INTEL_VGA_DEVICE(0x040E, &intel_haswell_d_info), /* GT1 reserved */
-	INTEL_VGA_DEVICE(0x041E, &intel_haswell_d_info), /* GT2 reserved */
-	INTEL_VGA_DEVICE(0x042E, &intel_haswell_d_info), /* GT3 reserved */
-	INTEL_VGA_DEVICE(0x0C02, &intel_haswell_d_info), /* SDV GT1 desktop */
-	INTEL_VGA_DEVICE(0x0C12, &intel_haswell_d_info), /* SDV GT2 desktop */
-	INTEL_VGA_DEVICE(0x0C22, &intel_haswell_d_info), /* SDV GT3 desktop */
-	INTEL_VGA_DEVICE(0x0C0A, &intel_haswell_d_info), /* SDV GT1 server */
-	INTEL_VGA_DEVICE(0x0C1A, &intel_haswell_d_info), /* SDV GT2 server */
-	INTEL_VGA_DEVICE(0x0C2A, &intel_haswell_d_info), /* SDV GT3 server */
-	INTEL_VGA_DEVICE(0x0C06, &intel_haswell_m_info), /* SDV GT1 mobile */
-	INTEL_VGA_DEVICE(0x0C16, &intel_haswell_m_info), /* SDV GT2 mobile */
-	INTEL_VGA_DEVICE(0x0C26, &intel_haswell_m_info), /* SDV GT3 mobile */
-	INTEL_VGA_DEVICE(0x0C0B, &intel_haswell_d_info), /* SDV GT1 reserved */
-	INTEL_VGA_DEVICE(0x0C1B, &intel_haswell_d_info), /* SDV GT2 reserved */
-	INTEL_VGA_DEVICE(0x0C2B, &intel_haswell_d_info), /* SDV GT3 reserved */
-	INTEL_VGA_DEVICE(0x0C0E, &intel_haswell_d_info), /* SDV GT1 reserved */
-	INTEL_VGA_DEVICE(0x0C1E, &intel_haswell_d_info), /* SDV GT2 reserved */
-	INTEL_VGA_DEVICE(0x0C2E, &intel_haswell_d_info), /* SDV GT3 reserved */
-	INTEL_VGA_DEVICE(0x0A02, &intel_haswell_d_info), /* ULT GT1 desktop */
-	INTEL_VGA_DEVICE(0x0A12, &intel_haswell_d_info), /* ULT GT2 desktop */
-	INTEL_VGA_DEVICE(0x0A22, &intel_haswell_d_info), /* ULT GT3 desktop */
-	INTEL_VGA_DEVICE(0x0A0A, &intel_haswell_d_info), /* ULT GT1 server */
-	INTEL_VGA_DEVICE(0x0A1A, &intel_haswell_d_info), /* ULT GT2 server */
-	INTEL_VGA_DEVICE(0x0A2A, &intel_haswell_d_info), /* ULT GT3 server */
-	INTEL_VGA_DEVICE(0x0A06, &intel_haswell_m_info), /* ULT GT1 mobile */
-	INTEL_VGA_DEVICE(0x0A16, &intel_haswell_m_info), /* ULT GT2 mobile */
-	INTEL_VGA_DEVICE(0x0A26, &intel_haswell_m_info), /* ULT GT3 mobile */
-	INTEL_VGA_DEVICE(0x0A0B, &intel_haswell_d_info), /* ULT GT1 reserved */
-	INTEL_VGA_DEVICE(0x0A1B, &intel_haswell_d_info), /* ULT GT2 reserved */
-	INTEL_VGA_DEVICE(0x0A2B, &intel_haswell_d_info), /* ULT GT3 reserved */
-	INTEL_VGA_DEVICE(0x0A0E, &intel_haswell_m_info), /* ULT GT1 reserved */
-	INTEL_VGA_DEVICE(0x0A1E, &intel_haswell_m_info), /* ULT GT2 reserved */
-	INTEL_VGA_DEVICE(0x0A2E, &intel_haswell_m_info), /* ULT GT3 reserved */
-	INTEL_VGA_DEVICE(0x0D02, &intel_haswell_d_info), /* CRW GT1 desktop */
-	INTEL_VGA_DEVICE(0x0D12, &intel_haswell_d_info), /* CRW GT2 desktop */
-	INTEL_VGA_DEVICE(0x0D22, &intel_haswell_d_info), /* CRW GT3 desktop */
-	INTEL_VGA_DEVICE(0x0D0A, &intel_haswell_d_info), /* CRW GT1 server */
-	INTEL_VGA_DEVICE(0x0D1A, &intel_haswell_d_info), /* CRW GT2 server */
-	INTEL_VGA_DEVICE(0x0D2A, &intel_haswell_d_info), /* CRW GT3 server */
-	INTEL_VGA_DEVICE(0x0D06, &intel_haswell_m_info), /* CRW GT1 mobile */
-	INTEL_VGA_DEVICE(0x0D16, &intel_haswell_m_info), /* CRW GT2 mobile */
-	INTEL_VGA_DEVICE(0x0D26, &intel_haswell_m_info), /* CRW GT3 mobile */
-	INTEL_VGA_DEVICE(0x0D0B, &intel_haswell_d_info), /* CRW GT1 reserved */
-	INTEL_VGA_DEVICE(0x0D1B, &intel_haswell_d_info), /* CRW GT2 reserved */
-	INTEL_VGA_DEVICE(0x0D2B, &intel_haswell_d_info), /* CRW GT3 reserved */
-	INTEL_VGA_DEVICE(0x0D0E, &intel_haswell_d_info), /* CRW GT1 reserved */
-	INTEL_VGA_DEVICE(0x0D1E, &intel_haswell_d_info), /* CRW GT2 reserved */
-	INTEL_VGA_DEVICE(0x0D2E, &intel_haswell_d_info), /* CRW GT3 reserved */
-	INTEL_VGA_DEVICE(0x0f30, &intel_valleyview_m_info),
-	INTEL_VGA_DEVICE(0x0f31, &intel_valleyview_m_info),
-	INTEL_VGA_DEVICE(0x0f32, &intel_valleyview_m_info),
-	INTEL_VGA_DEVICE(0x0f33, &intel_valleyview_m_info),
-	INTEL_VGA_DEVICE(0x0157, &intel_valleyview_m_info),
-	INTEL_VGA_DEVICE(0x0155, &intel_valleyview_d_info),
+	INTEL_PCI_IDS,
 	{0, 0, 0}
 };
 
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 63d609d..7276a72 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -26,6 +26,7 @@
 #ifndef _I915_DRM_H_
 #define _I915_DRM_H_
 
+#include <drm/i915_pciids.h>
 #include <uapi/drm/i915_drm.h>
 
 /* For use by IPS driver */
@@ -34,4 +35,5 @@ extern bool i915_gpu_raise(void);
 extern bool i915_gpu_lower(void);
 extern bool i915_gpu_busy(void);
 extern bool i915_gpu_turbo_disable(void);
+
 #endif				/* _I915_DRM_H_ */
diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
new file mode 100644
index 0000000..8a10f5c
--- /dev/null
+++ b/include/drm/i915_pciids.h
@@ -0,0 +1,211 @@
+/*
+ * Copyright 2013 Intel Corporation
+ * All Rights Reserved.
+ *
+ * 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, sub license, 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.
+ */
+#ifndef _I915_PCIIDS_H
+#define _I915_PCIIDS_H
+
+/*
+ * A pci_device_id struct {
+ *	__u32 vendor, device;
+ *      __u32 subvendor, subdevice;
+ *	__u32 class, class_mask;
+ *	kernel_ulong_t driver_data;
+ * };
+ * Don't use C99 here because "class" is reserved and we want to
+ * give userspace flexibility.
+ */
+#define INTEL_VGA_DEVICE(id, info) {		\
+	0x8086,	id,				\
+	~0, ~0,					\
+	0x030000, 0xff0000,			\
+	(unsigned long) info }
+
+#define INTEL_QUANTA_VGA_DEVICE(info) {		\
+	0x8086,	0x16a,				\
+	0x152d,	0x8990,				\
+	0x030000, 0xff0000,			\
+	(unsigned long) info }
+
+#define INTEL_I830_IDS(info)				\
+	INTEL_VGA_DEVICE(0x3577, info)
+
+#define INTEL_I845G_IDS(info)				\
+	INTEL_VGA_DEVICE(0x2562, info)
+
+#define INTEL_I85X_IDS(info)				\
+	INTEL_VGA_DEVICE(0x3582, info), /* I855_GM */ \
+	INTEL_VGA_DEVICE(0x358e, info)
+
+#define INTEL_I865G_IDS(info)				\
+	INTEL_VGA_DEVICE(0x2572, info) /* I865_G */
+
+#define INTEL_I915G_IDS(info)				\
+	INTEL_VGA_DEVICE(0x2582, info), /* I915_G */ \
+	INTEL_VGA_DEVICE(0x258a, info)  /* E7221_G */
+
+#define INTEL_I915GM_IDS(info)				\
+	INTEL_VGA_DEVICE(0x2592, info) /* I915_GM */
+
+#define INTEL_I945G_IDS(info)				\
+	INTEL_VGA_DEVICE(0x2772, info) /* I945_G */
+
+#define INTEL_I945GM_IDS(info)				\
+	INTEL_VGA_DEVICE(0x27a2, info), /* I945_GM */ \
+	INTEL_VGA_DEVICE(0x27ae, info)  /* I945_GME */
+
+#define INTEL_I965G_IDS(info)				\
+	INTEL_VGA_DEVICE(0x2972, info), /* I946_GZ */	\
+	INTEL_VGA_DEVICE(0x2982, info),	/* G35_G */	\
+	INTEL_VGA_DEVICE(0x2992, info),	/* I965_Q */	\
+	INTEL_VGA_DEVICE(0x29a2, info)	/* I965_G */
+
+#define INTEL_G33_IDS(info)				\
+	INTEL_VGA_DEVICE(0x29b2, info), /* Q35_G */ \
+	INTEL_VGA_DEVICE(0x29c2, info),	/* G33_G */ \
+	INTEL_VGA_DEVICE(0x29d2, info)	/* Q33_G */
+
+#define INTEL_I965GM_IDS(info)				\
+	INTEL_VGA_DEVICE(0x2a02, info),	/* I965_GM */ \
+	INTEL_VGA_DEVICE(0x2a12, info)  /* I965_GME */
+
+#define INTEL_GM45_IDS(info)				\
+	INTEL_VGA_DEVICE(0x2a42, info) /* GM45_G */
+
+#define INTEL_G45_IDS(info)				\
+	INTEL_VGA_DEVICE(0x2e02, info), /* IGD_E_G */ \
+	INTEL_VGA_DEVICE(0x2e12, info), /* Q45_G */ \
+	INTEL_VGA_DEVICE(0x2e22, info), /* G45_G */ \
+	INTEL_VGA_DEVICE(0x2e32, info), /* G41_G */ \
+	INTEL_VGA_DEVICE(0x2e42, info), /* B43_G */ \
+	INTEL_VGA_DEVICE(0x2e92, info)	/* B43_G.1 */
+
+#define INTEL_PINEVIEW_IDS(info)			\
+	INTEL_VGA_DEVICE(0xa001, info),			\
+	INTEL_VGA_DEVICE(0xa011, info)
+
+#define INTEL_IRONLAKE_D_IDS(info) \
+	INTEL_VGA_DEVICE(0x0042, info)
+
+#define INTEL_IRONLAKE_M_IDS(info) \
+	INTEL_VGA_DEVICE(0x0046, info)
+
+#define INTEL_SNB_D_IDS(info) \
+	INTEL_VGA_DEVICE(0x0102, info), \
+	INTEL_VGA_DEVICE(0x0112, info), \
+	INTEL_VGA_DEVICE(0x0122, info), \
+	INTEL_VGA_DEVICE(0x010A, info)
+
+#define INTEL_SNB_M_IDS(info) \
+	INTEL_VGA_DEVICE(0x0106, info), \
+	INTEL_VGA_DEVICE(0x0116, info), \
+	INTEL_VGA_DEVICE(0x0126, info)
+
+#define INTEL_IVB_M_IDS(info) \
+	INTEL_VGA_DEVICE(0x0156, info), /* GT1 mobile */ \
+	INTEL_VGA_DEVICE(0x0166, info)  /* GT2 mobile */
+
+#define INTEL_IVB_D_IDS(info) \
+	INTEL_VGA_DEVICE(0x0152, info), /* GT1 desktop */ \
+	INTEL_VGA_DEVICE(0x0162, info), /* GT2 desktop */ \
+	INTEL_VGA_DEVICE(0x015a, info), /* GT1 server */ \
+	INTEL_VGA_DEVICE(0x016a, info)  /* GT2 server */
+
+#define INTEL_IVB_Q_IDS(info) \
+	INTEL_QUANTA_VGA_DEVICE(info) /* Quanta transcode */
+
+#define INTEL_HSW_D_IDS(info) \
+	INTEL_VGA_DEVICE(0x0402, info), /* GT1 desktop */ \
+	INTEL_VGA_DEVICE(0x0412, info), /* GT2 desktop */ \
+	INTEL_VGA_DEVICE(0x0422, info), /* GT3 desktop */ \
+	INTEL_VGA_DEVICE(0x040a, info), /* GT1 server */ \
+	INTEL_VGA_DEVICE(0x041a, info), /* GT2 server */ \
+	INTEL_VGA_DEVICE(0x042a, info), /* GT3 server */ \
+	INTEL_VGA_DEVICE(0x040B, info), /* GT1 reserved */ \
+	INTEL_VGA_DEVICE(0x041B, info), /* GT2 reserved */ \
+	INTEL_VGA_DEVICE(0x042B, info), /* GT3 reserved */ \
+	INTEL_VGA_DEVICE(0x040E, info), /* GT1 reserved */ \
+	INTEL_VGA_DEVICE(0x041E, info), /* GT2 reserved */ \
+	INTEL_VGA_DEVICE(0x042E, info), /* GT3 reserved */ \
+	INTEL_VGA_DEVICE(0x0C02, info), /* SDV GT1 desktop */ \
+	INTEL_VGA_DEVICE(0x0C12, info), /* SDV GT2 desktop */ \
+	INTEL_VGA_DEVICE(0x0C22, info), /* SDV GT3 desktop */ \
+	INTEL_VGA_DEVICE(0x0C0A, info), /* SDV GT1 server */ \
+	INTEL_VGA_DEVICE(0x0C1A, info), /* SDV GT2 server */ \
+	INTEL_VGA_DEVICE(0x0C2A, info), /* SDV GT3 server */ \
+	INTEL_VGA_DEVICE(0x0C0B, info), /* SDV GT1 reserved */ \
+	INTEL_VGA_DEVICE(0x0C1B, info), /* SDV GT2 reserved */ \
+	INTEL_VGA_DEVICE(0x0C2B, info), /* SDV GT3 reserved */ \
+	INTEL_VGA_DEVICE(0x0C0E, info), /* SDV GT1 reserved */ \
+	INTEL_VGA_DEVICE(0x0C1E, info), /* SDV GT2 reserved */ \
+	INTEL_VGA_DEVICE(0x0C2E, info), /* SDV GT3 reserved */ \
+	INTEL_VGA_DEVICE(0x0A02, info), /* ULT GT1 desktop */ \
+	INTEL_VGA_DEVICE(0x0A12, info), /* ULT GT2 desktop */ \
+	INTEL_VGA_DEVICE(0x0A22, info), /* ULT GT3 desktop */ \
+	INTEL_VGA_DEVICE(0x0A0A, info), /* ULT GT1 server */ \
+	INTEL_VGA_DEVICE(0x0A1A, info), /* ULT GT2 server */ \
+	INTEL_VGA_DEVICE(0x0A2A, info), /* ULT GT3 server */ \
+	INTEL_VGA_DEVICE(0x0A0B, info), /* ULT GT1 reserved */ \
+	INTEL_VGA_DEVICE(0x0A1B, info), /* ULT GT2 reserved */ \
+	INTEL_VGA_DEVICE(0x0A2B, info), /* ULT GT3 reserved */ \
+	INTEL_VGA_DEVICE(0x0D02, info), /* CRW GT1 desktop */ \
+	INTEL_VGA_DEVICE(0x0D12, info), /* CRW GT2 desktop */ \
+	INTEL_VGA_DEVICE(0x0D22, info), /* CRW GT3 desktop */ \
+	INTEL_VGA_DEVICE(0x0D0A, info), /* CRW GT1 server */ \
+	INTEL_VGA_DEVICE(0x0D1A, info), /* CRW GT2 server */ \
+	INTEL_VGA_DEVICE(0x0D2A, info), /* CRW GT3 server */ \
+	INTEL_VGA_DEVICE(0x0D0B, info), /* CRW GT1 reserved */ \
+	INTEL_VGA_DEVICE(0x0D1B, info), /* CRW GT2 reserved */ \
+	INTEL_VGA_DEVICE(0x0D2B, info), /* CRW GT3 reserved */ \
+	INTEL_VGA_DEVICE(0x0D0E, info), /* CRW GT1 reserved */ \
+	INTEL_VGA_DEVICE(0x0D1E, info), /* CRW GT2 reserved */ \
+	INTEL_VGA_DEVICE(0x0D2E, info)  /* CRW GT3 reserved */ \
+
+#define INTEL_HSW_M_IDS(info) \
+	INTEL_VGA_DEVICE(0x0406, info), /* GT1 mobile */ \
+	INTEL_VGA_DEVICE(0x0416, info), /* GT2 mobile */ \
+	INTEL_VGA_DEVICE(0x0426, info), /* GT2 mobile */ \
+	INTEL_VGA_DEVICE(0x0C06, info), /* SDV GT1 mobile */ \
+	INTEL_VGA_DEVICE(0x0C16, info), /* SDV GT2 mobile */ \
+	INTEL_VGA_DEVICE(0x0C26, info), /* SDV GT3 mobile */ \
+	INTEL_VGA_DEVICE(0x0A06, info), /* ULT GT1 mobile */ \
+	INTEL_VGA_DEVICE(0x0A16, info), /* ULT GT2 mobile */ \
+	INTEL_VGA_DEVICE(0x0A26, info), /* ULT GT3 mobile */ \
+	INTEL_VGA_DEVICE(0x0A0E, info), /* ULT GT1 reserved */ \
+	INTEL_VGA_DEVICE(0x0A1E, info), /* ULT GT2 reserved */ \
+	INTEL_VGA_DEVICE(0x0A2E, info), /* ULT GT3 reserved */ \
+	INTEL_VGA_DEVICE(0x0D06, info), /* CRW GT1 mobile */ \
+	INTEL_VGA_DEVICE(0x0D16, info), /* CRW GT2 mobile */ \
+	INTEL_VGA_DEVICE(0x0D26, info)  /* CRW GT3 mobile */
+
+#define INTEL_VLV_M_IDS(info) \
+	INTEL_VGA_DEVICE(0x0f30, info), \
+	INTEL_VGA_DEVICE(0x0f31, info), \
+	INTEL_VGA_DEVICE(0x0f32, info), \
+	INTEL_VGA_DEVICE(0x0f33, info), \
+	INTEL_VGA_DEVICE(0x0157, info)
+
+#define INTEL_VLV_D_IDS(info) \
+	INTEL_VGA_DEVICE(0x0155, info)
+
+#endif /* _I915_PCIIDS_H */
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 07/17] x86: add early quirk for reserving Intel graphics stolen memory v5
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (5 preceding siblings ...)
  2013-08-26 22:50 ` [PATCH 06/17] drm/i915: split PCI IDs out into i915_drm.h v4 Rodrigo Vivi
@ 2013-08-26 22:50 ` Rodrigo Vivi
  2013-08-26 22:51 ` [PATCH 08/17] drm/i915: Always prefer CPU relocations with LLC Rodrigo Vivi
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:50 UTC (permalink / raw)
  To: intel-gfx

From: Jesse Barnes <jbarnes@virtuousgeek.org>

Systems with Intel graphics controllers set aside memory exclusively for
gfx driver use.  This memory is not always marked in the E820 as
reserved or as RAM, and so is subject to overlap from E820 manipulation
later in the boot process.  On some systems, MMIO space is allocated on
top, despite the efforts of the "RAM buffer" approach, which simply
rounds memory boundaries up to 64M to try to catch space that may decode
as RAM and so is not suitable for MMIO.

v2: use read_pci_config for 32 bit reads instead of adding a new one
    (Chris)
    add gen6 stolen size function (Chris)
v3: use a function pointer (Chris)
    drop gen2 bits (Daniel)
v4: call e820_sanitize_map after adding the region
v5: fixup comments (Peter)
    simplify loop (Chris)

Acked-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 arch/x86/kernel/early-quirks.c  | 154 ++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h |  15 ----
 include/drm/i915_drm.h          |  32 +++++++++
 3 files changed, 186 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index 63bdb29..b3cd3eb 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -12,6 +12,7 @@
 #include <linux/pci.h>
 #include <linux/acpi.h>
 #include <linux/pci_ids.h>
+#include <drm/i915_drm.h>
 #include <asm/pci-direct.h>
 #include <asm/dma.h>
 #include <asm/io_apic.h>
@@ -216,6 +217,157 @@ static void __init intel_remapping_check(int num, int slot, int func)
 
 }
 
+/*
+ * Systems with Intel graphics controllers set aside memory exclusively
+ * for gfx driver use.  This memory is not marked in the E820 as reserved
+ * or as RAM, and so is subject to overlap from E820 manipulation later
+ * in the boot process.  On some systems, MMIO space is allocated on top,
+ * despite the efforts of the "RAM buffer" approach, which simply rounds
+ * memory boundaries up to 64M to try to catch space that may decode
+ * as RAM and so is not suitable for MMIO.
+ *
+ * And yes, so far on current devices the base addr is always under 4G.
+ */
+static u32 __init intel_stolen_base(int num, int slot, int func)
+{
+	u32 base;
+
+	/*
+	 * For the PCI IDs in this quirk, the stolen base is always
+	 * in 0x5c, aka the BDSM register (yes that's really what
+	 * it's called).
+	 */
+	base = read_pci_config(num, slot, func, 0x5c);
+	base &= ~((1<<20) - 1);
+
+	return base;
+}
+
+#define KB(x)	((x) * 1024)
+#define MB(x)	(KB (KB (x)))
+#define GB(x)	(MB (KB (x)))
+
+static size_t __init gen3_stolen_size(int num, int slot, int func)
+{
+	size_t stolen_size;
+	u16 gmch_ctrl;
+
+	gmch_ctrl = read_pci_config_16(0, 0, 0, I830_GMCH_CTRL);
+
+	switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
+	case I855_GMCH_GMS_STOLEN_1M:
+		stolen_size = MB(1);
+		break;
+	case I855_GMCH_GMS_STOLEN_4M:
+		stolen_size = MB(4);
+		break;
+	case I855_GMCH_GMS_STOLEN_8M:
+		stolen_size = MB(8);
+		break;
+	case I855_GMCH_GMS_STOLEN_16M:
+		stolen_size = MB(16);
+		break;
+	case I855_GMCH_GMS_STOLEN_32M:
+		stolen_size = MB(32);
+		break;
+	case I915_GMCH_GMS_STOLEN_48M:
+		stolen_size = MB(48);
+		break;
+	case I915_GMCH_GMS_STOLEN_64M:
+		stolen_size = MB(64);
+		break;
+	case G33_GMCH_GMS_STOLEN_128M:
+		stolen_size = MB(128);
+		break;
+	case G33_GMCH_GMS_STOLEN_256M:
+		stolen_size = MB(256);
+		break;
+	case INTEL_GMCH_GMS_STOLEN_96M:
+		stolen_size = MB(96);
+		break;
+	case INTEL_GMCH_GMS_STOLEN_160M:
+		stolen_size = MB(160);
+		break;
+	case INTEL_GMCH_GMS_STOLEN_224M:
+		stolen_size = MB(224);
+		break;
+	case INTEL_GMCH_GMS_STOLEN_352M:
+		stolen_size = MB(352);
+		break;
+	default:
+		stolen_size = 0;
+		break;
+	}
+
+	return stolen_size;
+}
+
+static size_t __init gen6_stolen_size(int num, int slot, int func)
+{
+	u16 gmch_ctrl;
+
+	gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
+	gmch_ctrl >>= SNB_GMCH_GMS_SHIFT;
+	gmch_ctrl &= SNB_GMCH_GMS_MASK;
+
+	return gmch_ctrl << 25; /* 32 MB units */
+}
+
+typedef size_t (*stolen_size_fn)(int num, int slot, int func);
+
+static struct pci_device_id intel_stolen_ids[] __initdata = {
+	INTEL_I915G_IDS(gen3_stolen_size),
+	INTEL_I915GM_IDS(gen3_stolen_size),
+	INTEL_I945G_IDS(gen3_stolen_size),
+	INTEL_I945GM_IDS(gen3_stolen_size),
+	INTEL_VLV_M_IDS(gen3_stolen_size),
+	INTEL_VLV_D_IDS(gen3_stolen_size),
+	INTEL_PINEVIEW_IDS(gen3_stolen_size),
+	INTEL_I965G_IDS(gen3_stolen_size),
+	INTEL_G33_IDS(gen3_stolen_size),
+	INTEL_I965GM_IDS(gen3_stolen_size),
+	INTEL_GM45_IDS(gen3_stolen_size),
+	INTEL_G45_IDS(gen3_stolen_size),
+	INTEL_IRONLAKE_D_IDS(gen3_stolen_size),
+	INTEL_IRONLAKE_M_IDS(gen3_stolen_size),
+	INTEL_SNB_D_IDS(gen6_stolen_size),
+	INTEL_SNB_M_IDS(gen6_stolen_size),
+	INTEL_IVB_M_IDS(gen6_stolen_size),
+	INTEL_IVB_D_IDS(gen6_stolen_size),
+	INTEL_HSW_D_IDS(gen6_stolen_size),
+	INTEL_HSW_M_IDS(gen6_stolen_size),
+};
+
+static void __init intel_graphics_stolen(int num, int slot, int func)
+{
+	size_t size;
+	int i;
+	u32 start;
+	u16 device, subvendor, subdevice;
+
+	device = read_pci_config_16(num, slot, func, PCI_DEVICE_ID);
+	subvendor = read_pci_config_16(num, slot, func,
+				       PCI_SUBSYSTEM_VENDOR_ID);
+	subdevice = read_pci_config_16(num, slot, func, PCI_SUBSYSTEM_ID);
+
+	for (i = 0; i < ARRAY_SIZE(intel_stolen_ids); i++) {
+		if (intel_stolen_ids[i].device == device) {
+			stolen_size_fn stolen_size =
+				(stolen_size_fn)intel_stolen_ids[i].driver_data;
+			size = stolen_size(num, slot, func);
+			start = intel_stolen_base(num, slot, func);
+			if (size && start) {
+				/* Mark this space as reserved */
+				e820_add_region(start, size, E820_RESERVED);
+				sanitize_e820_map(e820.map,
+						  ARRAY_SIZE(e820.map),
+						  &e820.nr_map);
+			}
+			return;
+		}
+	}
+}
+
 #define QFLAG_APPLY_ONCE 	0x1
 #define QFLAG_APPLIED		0x2
 #define QFLAG_DONE		(QFLAG_APPLY_ONCE|QFLAG_APPLIED)
@@ -251,6 +403,8 @@ static struct chipset early_qrk[] __initdata = {
 	  PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check },
 	{ PCI_VENDOR_ID_INTEL, 0x3406, PCI_CLASS_BRIDGE_HOST,
 	  PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check },
+	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, PCI_ANY_ID,
+	  QFLAG_APPLY_ONCE, intel_graphics_stolen },
 	{}
 };
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ee0660d..4c3cf22 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -33,21 +33,6 @@
 #define _MASKED_BIT_ENABLE(a) (((a) << 16) | (a))
 #define _MASKED_BIT_DISABLE(a) ((a) << 16)
 
-/*
- * The Bridge device's PCI config space has information about the
- * fb aperture size and the amount of pre-reserved memory.
- * This is all handled in the intel-gtt.ko module. i915.ko only
- * cares about the vga bit for the vga rbiter.
- */
-#define INTEL_GMCH_CTRL		0x52
-#define INTEL_GMCH_VGA_DISABLE  (1 << 1)
-#define SNB_GMCH_CTRL		0x50
-#define    SNB_GMCH_GGMS_SHIFT	8 /* GTT Graphics Memory Size */
-#define    SNB_GMCH_GGMS_MASK	0x3
-#define    SNB_GMCH_GMS_SHIFT   3 /* Graphics Mode Select */
-#define    SNB_GMCH_GMS_MASK    0x1f
-
-
 /* PCI config space */
 
 #define HPLLCC	0xc0 /* 855 only */
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 7276a72..3abfa6e 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -36,4 +36,36 @@ extern bool i915_gpu_lower(void);
 extern bool i915_gpu_busy(void);
 extern bool i915_gpu_turbo_disable(void);
 
+/*
+ * The Bridge device's PCI config space has information about the
+ * fb aperture size and the amount of pre-reserved memory.
+ * This is all handled in the intel-gtt.ko module. i915.ko only
+ * cares about the vga bit for the vga rbiter.
+ */
+#define INTEL_GMCH_CTRL		0x52
+#define INTEL_GMCH_VGA_DISABLE  (1 << 1)
+#define SNB_GMCH_CTRL		0x50
+#define    SNB_GMCH_GGMS_SHIFT	8 /* GTT Graphics Memory Size */
+#define    SNB_GMCH_GGMS_MASK	0x3
+#define    SNB_GMCH_GMS_SHIFT   3 /* Graphics Mode Select */
+#define    SNB_GMCH_GMS_MASK    0x1f
+
+#define I830_GMCH_CTRL			0x52
+
+#define I855_GMCH_GMS_MASK		0xF0
+#define I855_GMCH_GMS_STOLEN_0M		0x0
+#define I855_GMCH_GMS_STOLEN_1M		(0x1 << 4)
+#define I855_GMCH_GMS_STOLEN_4M		(0x2 << 4)
+#define I855_GMCH_GMS_STOLEN_8M		(0x3 << 4)
+#define I855_GMCH_GMS_STOLEN_16M	(0x4 << 4)
+#define I855_GMCH_GMS_STOLEN_32M	(0x5 << 4)
+#define I915_GMCH_GMS_STOLEN_48M	(0x6 << 4)
+#define I915_GMCH_GMS_STOLEN_64M	(0x7 << 4)
+#define G33_GMCH_GMS_STOLEN_128M	(0x8 << 4)
+#define G33_GMCH_GMS_STOLEN_256M	(0x9 << 4)
+#define INTEL_GMCH_GMS_STOLEN_96M	(0xa << 4)
+#define INTEL_GMCH_GMS_STOLEN_160M	(0xb << 4)
+#define INTEL_GMCH_GMS_STOLEN_224M	(0xc << 4)
+#define INTEL_GMCH_GMS_STOLEN_352M	(0xd << 4)
+
 #endif				/* _I915_DRM_H_ */
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 08/17] drm/i915: Always prefer CPU relocations with LLC
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (6 preceding siblings ...)
  2013-08-26 22:50 ` [PATCH 07/17] x86: add early quirk for reserving Intel graphics stolen memory v5 Rodrigo Vivi
@ 2013-08-26 22:51 ` Rodrigo Vivi
  2013-08-27 14:49   ` Ville Syrjälä
  2013-08-29 17:20   ` Ben Widawsky
  2013-08-26 22:51 ` [PATCH 09/17] drm/i915: Report requested frequency alongside current frequency in debugfs Rodrigo Vivi
                   ` (9 subsequent siblings)
  17 siblings, 2 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:51 UTC (permalink / raw)
  To: intel-gfx

From: Chris Wilson <chris@chris-wilson.co.uk>

A follow-on to the update of the LLC coherency logic is that we can rely
on the LLC being coherent with the CS for rewriting batchbuffers
irrespective of their cache domain. (This should have no effect
currently as all the batch buffers are expected to be I915_CACHE_LLC and
so using the cpu relocation path anyway.)

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

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 792c52a..3b64b9f 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -166,7 +166,8 @@ eb_destroy(struct eb_objects *eb)
 
 static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
 {
-	return (obj->base.write_domain == I915_GEM_DOMAIN_CPU ||
+	return (HAS_LLC(obj->base.dev) ||
+		obj->base.write_domain == I915_GEM_DOMAIN_CPU ||
 		!obj->map_and_fenceable ||
 		obj->cache_level != I915_CACHE_NONE);
 }
@@ -179,7 +180,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
 	char *vaddr;
 	int ret = -EINVAL;
 
-	ret = i915_gem_object_set_to_cpu_domain(obj, 1);
+	ret = i915_gem_object_set_to_cpu_domain(obj, true);
 	if (ret)
 		return ret;
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 09/17] drm/i915: Report requested frequency alongside current frequency in debugfs
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (7 preceding siblings ...)
  2013-08-26 22:51 ` [PATCH 08/17] drm/i915: Always prefer CPU relocations with LLC Rodrigo Vivi
@ 2013-08-26 22:51 ` Rodrigo Vivi
  2013-08-27 12:12   ` Rodrigo Vivi
  2013-08-26 22:51 ` [PATCH 10/17] drm/i915: Move the conditional seqno query into the tracepoint Rodrigo Vivi
                   ` (8 subsequent siblings)
  17 siblings, 1 reply; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:51 UTC (permalink / raw)
  To: intel-gfx

From: Chris Wilson <chris@chris-wilson.co.uk>

It can be useful to compare at times the current vs requested frequency
of the GPU, so provide the contents of RPNSWREQ alonside CAGF.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 55ab924..a6f4cb5 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -857,7 +857,7 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
 		u32 gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
 		u32 rp_state_limits = I915_READ(GEN6_RP_STATE_LIMITS);
 		u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
-		u32 rpstat, cagf;
+		u32 rpstat, cagf, reqf;
 		u32 rpupei, rpcurup, rpprevup;
 		u32 rpdownei, rpcurdown, rpprevdown;
 		int max_freq;
@@ -869,6 +869,14 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
 
 		gen6_gt_force_wake_get(dev_priv);
 
+		reqf = I915_READ(GEN6_RPNSWREQ);
+		reqf &= ~GEN6_TURBO_DISABLE;
+		if (IS_HASWELL(dev))
+			reqf >>= 24;
+		else
+			reqf >>= 25;
+		reqf *= GT_FREQUENCY_MULTIPLIER;
+
 		rpstat = I915_READ(GEN6_RPSTAT1);
 		rpupei = I915_READ(GEN6_RP_CUR_UP_EI);
 		rpcurup = I915_READ(GEN6_RP_CUR_UP);
@@ -893,6 +901,7 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
 			   gt_perf_status & 0xff);
 		seq_printf(m, "Render p-state limit: %d\n",
 			   rp_state_limits & 0xff);
+		seq_printf(m, "RPNSWREQ: %dMHz\n", reqf);
 		seq_printf(m, "CAGF: %dMHz\n", cagf);
 		seq_printf(m, "RP CUR UP EI: %dus\n", rpupei &
 			   GEN6_CURICONT_MASK);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 10/17] drm/i915: Move the conditional seqno query into the tracepoint
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (8 preceding siblings ...)
  2013-08-26 22:51 ` [PATCH 09/17] drm/i915: Report requested frequency alongside current frequency in debugfs Rodrigo Vivi
@ 2013-08-26 22:51 ` Rodrigo Vivi
  2013-08-26 22:51 ` [PATCH 11/17] drm/i915: Add some missing steps to i915_driver_load error path Rodrigo Vivi
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:51 UTC (permalink / raw)
  To: intel-gfx

From: Chris Wilson <chris@chris-wilson.co.uk>

We only wish to know the value of seqno when emitting the tracepoint, so
move the query from a parameter to the macro to inside the conditional
macro body so that the query is only evaluated when required.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a03b445..efac65b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -801,7 +801,7 @@ static void notify_ring(struct drm_device *dev,
 	if (ring->obj == NULL)
 		return;
 
-	trace_i915_gem_request_complete(ring, ring->get_seqno(ring, false));
+	trace_i915_gem_request_complete(ring);
 
 	wake_up_all(&ring->irq_queue);
 	i915_queue_hangcheck(dev);
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index e2c5ee6..a1797f6 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -304,9 +304,24 @@ DEFINE_EVENT(i915_gem_request, i915_gem_request_add,
 	    TP_ARGS(ring, seqno)
 );
 
-DEFINE_EVENT(i915_gem_request, i915_gem_request_complete,
-	    TP_PROTO(struct intel_ring_buffer *ring, u32 seqno),
-	    TP_ARGS(ring, seqno)
+TRACE_EVENT(i915_gem_request_complete,
+	    TP_PROTO(struct intel_ring_buffer *ring),
+	    TP_ARGS(ring),
+
+	    TP_STRUCT__entry(
+			     __field(u32, dev)
+			     __field(u32, ring)
+			     __field(u32, seqno)
+			     ),
+
+	    TP_fast_assign(
+			   __entry->dev = ring->dev->primary->index;
+			   __entry->ring = ring->id;
+			   __entry->seqno = ring->get_seqno(ring, false);
+			   ),
+
+	    TP_printk("dev=%u, ring=%u, seqno=%u",
+		      __entry->dev, __entry->ring, __entry->seqno)
 );
 
 DEFINE_EVENT(i915_gem_request, i915_gem_request_retire,
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 11/17] drm/i915: Add some missing steps to i915_driver_load error path
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (9 preceding siblings ...)
  2013-08-26 22:51 ` [PATCH 10/17] drm/i915: Move the conditional seqno query into the tracepoint Rodrigo Vivi
@ 2013-08-26 22:51 ` Rodrigo Vivi
  2013-08-26 22:51 ` [PATCH 12/17] drm/i915: Asynchronously perform the set-base for a simple modeset Rodrigo Vivi
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:51 UTC (permalink / raw)
  To: intel-gfx

From: Chris Wilson <chris@chris-wilson.co.uk>

We missed adding a few cleanup steps for recent additions.

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

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4c7669f..236e5e3 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1543,7 +1543,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	ret = i915_gem_gtt_init(dev);
 	if (ret)
-		goto put_bridge;
+		goto out_regs;
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		i915_kick_out_firmware_fb(dev_priv);
@@ -1572,7 +1572,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 				     aperture_size);
 	if (dev_priv->gtt.mappable == NULL) {
 		ret = -EIO;
-		goto out_rmmap;
+		goto out_gtt;
 	}
 
 	dev_priv->gtt.mtrr = arch_phys_wc_add(dev_priv->gtt.mappable_base,
@@ -1646,7 +1646,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 		ret = i915_load_modeset_init(dev);
 		if (ret < 0) {
 			DRM_ERROR("failed to init modeset\n");
-			goto out_gem_unload;
+			goto out_power_well;
 		}
 	} else {
 		/* Start out suspended in ums mode. */
@@ -1666,6 +1666,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	return 0;
 
+out_power_well:
+	if (HAS_POWER_WELL(dev))
+		i915_remove_power_well(dev);
+	drm_vblank_cleanup(dev);
 out_gem_unload:
 	if (dev_priv->mm.inactive_shrinker.shrink)
 		unregister_shrinker(&dev_priv->mm.inactive_shrinker);
@@ -1679,12 +1683,17 @@ out_gem_unload:
 out_mtrrfree:
 	arch_phys_wc_del(dev_priv->gtt.mtrr);
 	io_mapping_free(dev_priv->gtt.mappable);
+out_gtt:
+	list_del(&dev_priv->gtt.base.global_link);
+	drm_mm_takedown(&dev_priv->gtt.base.mm);
 	dev_priv->gtt.base.cleanup(&dev_priv->gtt.base);
-out_rmmap:
+out_regs:
 	pci_iounmap(dev->pdev, dev_priv->regs);
 put_bridge:
 	pci_dev_put(dev_priv->bridge_dev);
 free_priv:
+	if (dev_priv->slab)
+		kmem_cache_destroy(dev_priv->slab);
 	kfree(dev_priv);
 	return ret;
 }
@@ -1777,6 +1786,8 @@ int i915_driver_unload(struct drm_device *dev)
 	if (dev_priv->regs != NULL)
 		pci_iounmap(dev->pdev, dev_priv->regs);
 
+	drm_vblank_cleanup(dev);
+
 	intel_teardown_gmbus(dev);
 	intel_teardown_mchbar(dev);
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 12/17] drm/i915: Asynchronously perform the set-base for a simple modeset
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (10 preceding siblings ...)
  2013-08-26 22:51 ` [PATCH 11/17] drm/i915: Add some missing steps to i915_driver_load error path Rodrigo Vivi
@ 2013-08-26 22:51 ` Rodrigo Vivi
  2013-08-26 22:51 ` [PATCH 13/17] drm/i915: Align tiled scanouts from stolen memory to 256k in the GTT Rodrigo Vivi
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:51 UTC (permalink / raw)
  To: intel-gfx

From: Chris Wilson <chris@chris-wilson.co.uk>

A simple modeset, where we only wish to switch over to a new framebuffer
such as the transition from fbcon to X, takes around 30-60ms. This is
due to three factors:

1. We need to make sure the fb->obj is in the display domain, which
incurs a cache flush to ensure no dirt is left on the scanout.

2. We need to flush any pending rendering before performing the mmio
so that the frame is complete before it is shown.

3. We currently wait for the vblank after the mmio to be sure that the
old fb is no longer being shown before releasing it.

(1) can only be eliminated by userspace preparing the fb->obj in advance
to already be in the display domain. This can be done through use of the
create2 ioctl, or by reusing an existing fb->obj.

However, (2) and (3) are already solved by the existing page flip
mechanism, and it is surprisingly trivial to wire them up for use in the
set-base fast path. Though it can be argued that this represents a
subtle ABI break in that the set_config ioctl now returns before the old
framebuffer is unpinned. The danger is that userspace will start to
modify it before it is no longer being shown, however we should be able
to prevent that through proper domain tracking.

By combining all of the above, we can achieve an instaneous set_config:

[     6.601] (II) intel(0): switch to mode 2560x1440@60.0 on pipe 0 using DP2, position (0, 0), rotation normal
[     6.601] (II) intel(0): Setting screen physical size to 677 x 381

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 74dbc68..0da0ead1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9213,10 +9213,13 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 		ret = intel_set_mode(set->crtc, set->mode,
 				     set->x, set->y, set->fb);
 	} else if (config->fb_changed) {
-		intel_crtc_wait_for_pending_flips(set->crtc);
-
-		ret = intel_pipe_set_base(set->crtc,
-					  set->x, set->y, set->fb);
+		if (to_intel_framebuffer(set->fb)->obj->ring == NULL ||
+		    save_set.x != set->x || save_set.y != set->y ||
+		    intel_crtc_page_flip(set->crtc, set->fb, NULL)) {
+			intel_crtc_wait_for_pending_flips(set->crtc);
+			ret = intel_pipe_set_base(set->crtc,
+						  set->x, set->y, set->fb);
+		}
 	}
 
 	if (ret) {
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 13/17] drm/i915: Align tiled scanouts from stolen memory to 256k in the GTT
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (11 preceding siblings ...)
  2013-08-26 22:51 ` [PATCH 12/17] drm/i915: Asynchronously perform the set-base for a simple modeset Rodrigo Vivi
@ 2013-08-26 22:51 ` Rodrigo Vivi
  2013-08-26 22:51 ` [PATCH 14/17] drm/i915: Apply the force-detect VGA w/a to Valleyview Rodrigo Vivi
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:51 UTC (permalink / raw)
  To: intel-gfx

From: Chris Wilson <chris@chris-wilson.co.uk>

For unfathomable reasons this alignment appears to be required for tiled
scanouts being read from stolen memory. I can find no reference in the
w/a db to support this requirement, but the evidence of my own eyes says
this prevents many headaches.

Note that I have not tricked anything older than Sandybridge into using
stolen tiled scanouts, so the extra alignment may be required there as
well.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0da0ead1..cee9c0d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1837,6 +1837,8 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
 	case I915_TILING_X:
 		/* pin() will align the object as required by fence */
 		alignment = 0;
+		if (obj->stolen && INTEL_INFO(dev)->gen >= 6)
+			alignment = 256 * 1024;
 		break;
 	case I915_TILING_Y:
 		/* Despite that we check this in framebuffer_init userspace can
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 14/17] drm/i915: Apply the force-detect VGA w/a to Valleyview
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (12 preceding siblings ...)
  2013-08-26 22:51 ` [PATCH 13/17] drm/i915: Align tiled scanouts from stolen memory to 256k in the GTT Rodrigo Vivi
@ 2013-08-26 22:51 ` Rodrigo Vivi
  2013-08-27  9:27   ` Daniel Vetter
  2013-08-26 22:51 ` [PATCH 15/17] drm/i915: Pair seqno completion tracepoint with its dispatch Rodrigo Vivi
                   ` (3 subsequent siblings)
  17 siblings, 1 reply; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:51 UTC (permalink / raw)
  To: intel-gfx

From: Chris Wilson <chris@chris-wilson.co.uk>

It appears that Valleyview shares its VGA encoder with more recent
siblings and requires the same forced detection cycle after a hardware
reset before we can rely on hotplugging.

Reported-and-tested-by: kobeqin <kobe.qin@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67733
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_crt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index b5a3875..7475200 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -688,7 +688,7 @@ static void intel_crt_reset(struct drm_connector *connector)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crt *crt = intel_attached_crt(connector);
 
-	if (HAS_PCH_SPLIT(dev)) {
+	if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev)) {
 		u32 adpa;
 
 		adpa = I915_READ(crt->adpa_reg);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 15/17] drm/i915: Pair seqno completion tracepoint with its dispatch
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (13 preceding siblings ...)
  2013-08-26 22:51 ` [PATCH 14/17] drm/i915: Apply the force-detect VGA w/a to Valleyview Rodrigo Vivi
@ 2013-08-26 22:51 ` Rodrigo Vivi
  2013-08-26 22:51 ` [PATCH 16/17] RFM drm/i915: Boost RPS frequency for CPU stalls Rodrigo Vivi
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:51 UTC (permalink / raw)
  To: intel-gfx

From: Chris Wilson <chris@chris-wilson.co.uk>

In order to time how long a seqno is executed by a ring, we need to
measure both its insertion and its completion. (Using the completion of
the previous seqno as an estimate for when the GPU starts, if busy.) In
order to get an exact completion timestamp, we need irqs. This is
enabled by trace_i915_gem_ring_dispatch, so it makes more sens to pair
the completion event with that rather than the request.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  2 +-
 drivers/gpu/drm/i915/i915_irq.c            |  2 +-
 drivers/gpu/drm/i915/i915_trace.h          | 48 +++++++++++++++---------------
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3b64b9f..4f90cb3 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1127,7 +1127,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 			goto err;
 	}
 
-	trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), flags);
+	trace_i915_gem_ring_dispatch(ring, flags);
 
 	i915_gem_execbuffer_move_to_active(&eb->objects, vm, ring);
 	i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index efac65b..d61bd5b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -801,7 +801,7 @@ static void notify_ring(struct drm_device *dev,
 	if (ring->obj == NULL)
 		return;
 
-	trace_i915_gem_request_complete(ring);
+	trace_i915_gem_ring_complete(ring);
 
 	wake_up_all(&ring->irq_queue);
 	i915_queue_hangcheck(dev);
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index a1797f6..5c8e36a 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -234,8 +234,8 @@ TRACE_EVENT(i915_gem_evict_everything,
 );
 
 TRACE_EVENT(i915_gem_ring_dispatch,
-	    TP_PROTO(struct intel_ring_buffer *ring, u32 seqno, u32 flags),
-	    TP_ARGS(ring, seqno, flags),
+	    TP_PROTO(struct intel_ring_buffer *ring, u32 flags),
+	    TP_ARGS(ring, flags),
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
@@ -247,15 +247,35 @@ TRACE_EVENT(i915_gem_ring_dispatch,
 	    TP_fast_assign(
 			   __entry->dev = ring->dev->primary->index;
 			   __entry->ring = ring->id;
-			   __entry->seqno = seqno;
+			   __entry->seqno = intel_ring_get_seqno(ring),
 			   __entry->flags = flags;
-			   i915_trace_irq_get(ring, seqno);
+			   i915_trace_irq_get(ring, __entry->seqno);
 			   ),
 
 	    TP_printk("dev=%u, ring=%u, seqno=%u, flags=%x",
 		      __entry->dev, __entry->ring, __entry->seqno, __entry->flags)
 );
 
+TRACE_EVENT(i915_gem_ring_complete,
+	    TP_PROTO(struct intel_ring_buffer *ring),
+	    TP_ARGS(ring),
+
+	    TP_STRUCT__entry(
+			     __field(u32, dev)
+			     __field(u32, ring)
+			     __field(u32, seqno)
+			     ),
+
+	    TP_fast_assign(
+			   __entry->dev = ring->dev->primary->index;
+			   __entry->ring = ring->id;
+			   __entry->seqno = ring->get_seqno(ring, false);
+			   ),
+
+	    TP_printk("dev=%u, ring=%u, seqno=%u",
+		      __entry->dev, __entry->ring, __entry->seqno)
+);
+
 TRACE_EVENT(i915_gem_ring_flush,
 	    TP_PROTO(struct intel_ring_buffer *ring, u32 invalidate, u32 flush),
 	    TP_ARGS(ring, invalidate, flush),
@@ -304,26 +324,6 @@ DEFINE_EVENT(i915_gem_request, i915_gem_request_add,
 	    TP_ARGS(ring, seqno)
 );
 
-TRACE_EVENT(i915_gem_request_complete,
-	    TP_PROTO(struct intel_ring_buffer *ring),
-	    TP_ARGS(ring),
-
-	    TP_STRUCT__entry(
-			     __field(u32, dev)
-			     __field(u32, ring)
-			     __field(u32, seqno)
-			     ),
-
-	    TP_fast_assign(
-			   __entry->dev = ring->dev->primary->index;
-			   __entry->ring = ring->id;
-			   __entry->seqno = ring->get_seqno(ring, false);
-			   ),
-
-	    TP_printk("dev=%u, ring=%u, seqno=%u",
-		      __entry->dev, __entry->ring, __entry->seqno)
-);
-
 DEFINE_EVENT(i915_gem_request, i915_gem_request_retire,
 	    TP_PROTO(struct intel_ring_buffer *ring, u32 seqno),
 	    TP_ARGS(ring, seqno)
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 16/17] RFM drm/i915: Boost RPS frequency for CPU stalls
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (14 preceding siblings ...)
  2013-08-26 22:51 ` [PATCH 15/17] drm/i915: Pair seqno completion tracepoint with its dispatch Rodrigo Vivi
@ 2013-08-26 22:51 ` Rodrigo Vivi
  2013-08-26 22:56   ` Chris Wilson
  2013-08-26 22:51 ` [PATCH 17/17] drm/i915: Enable Lower Slice on Haswell GT3 Rodrigo Vivi
  2013-08-27  9:39 ` [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Daniel Vetter
  17 siblings, 1 reply; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:51 UTC (permalink / raw)
  To: intel-gfx; +Cc: Stéphane Marchesin, Zhuang, Lena

From: Chris Wilson <chris@chris-wilson.co.uk>

If we encounter a situation where the CPU blocks waiting for results
from the GPU, give the GPU a kick to boost its the frequency.

This should work to reduce user interface stalls and to quickly promote
mesa to high frequencies - but the cost is that our requested frequency
stalls high (as we do not idle for long enough before rc6 to start
reducing frequencies, nor are we aggressive at down clocking an
underused GPU). However, this should be mitigated by rc6 itself powering
off the GPU when idle, and that energy use is dependent upon the workload
of the GPU in addition to its frequency (e.g. the math or sampler
functions only consume power when used). Still, this is likely to
adversely affect light workloads.

Stéphane raised the concern that this will punish good applications and
reward bad applications - but due to the nature of how mesa performs its
client throttling, I believe all mesa applications will be roughly
equally affected.

RFM - request for measurement!

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: Stéphane Marchesin <stephane.marchesin@gmail.com>
Cc: "Meng, Mengmeng" <mengmeng.meng@intel.com>
Cc: "Zhuang, Lena" <lena.zhuang@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7c92923..50a70ee 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1016,6 +1016,15 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
 	if (WARN_ON(!ring->irq_get(ring)))
 		return -ENODEV;
 
+	if (dev_priv->info->gen >= 6) {
+		mutex_lock(&dev_priv->rps.hw_lock);
+		if (dev_priv->info->is_valleyview)
+			valleyview_set_rps(dev_priv->dev, dev_priv->rps.max_delay);
+		else
+			gen6_set_rps(dev_priv->dev, dev_priv->rps.max_delay);
+		mutex_unlock(&dev_priv->rps.hw_lock);
+	}
+
 	/* Record current time in case interrupted by signal, or wedged * */
 	getrawmonotonic(&before);
 
-- 
1.8.1.4

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

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 17/17] drm/i915: Enable Lower Slice on Haswell GT3.
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (15 preceding siblings ...)
  2013-08-26 22:51 ` [PATCH 16/17] RFM drm/i915: Boost RPS frequency for CPU stalls Rodrigo Vivi
@ 2013-08-26 22:51 ` Rodrigo Vivi
  2013-08-27 15:31   ` Rodrigo Vivi
  2013-08-27  9:39 ` [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Daniel Vetter
  17 siblings, 1 reply; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-26 22:51 UTC (permalink / raw)
  To: intel-gfx

Full HSW GT3 power can only be achieved with all Execution Units turned on.
This patch enables all EUs present on HSW GT3 by enabling lower slice.

Credits-by: Yejun Guo <yejun.guo@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 2 ++
 drivers/gpu/drm/i915/i915_gem.c | 5 +++++
 drivers/gpu/drm/i915/i915_reg.h | 5 +++++
 3 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7ac5ca5..e9b2f86 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1592,6 +1592,8 @@ struct drm_i915_file_private {
 				 ((dev)->pci_device & 0xFF00) == 0x0C00)
 #define IS_ULT(dev)		(IS_HASWELL(dev) && \
 				 ((dev)->pci_device & 0xFF00) == 0x0A00)
+#define IS_HSW_GT3(dev)		(IS_HASWELL(dev) && \
+				 ((dev)->pci_device & 0x00F0) == 0x0020)
 
 /*
  * The genX designation typically refers to the render engine, so render
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 50a70ee..1c72229 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4316,6 +4316,11 @@ i915_gem_init_hw(struct drm_device *dev)
 	if (dev_priv->ellc_size)
 		I915_WRITE(HSW_IDICR, I915_READ(HSW_IDICR) | IDIHASHMSK(0xf));
 
+	if (IS_HSW_GT3(dev))
+		I915_WRITE(MI_PREDICATE_RESULT_2, LOWER_SLICE_ENABLED);
+	else
+		I915_WRITE(MI_PREDICATE_RESULT_2, LOWER_SLICE_DISABLED);
+
 	if (HAS_PCH_NOP(dev)) {
 		u32 temp = I915_READ(GEN7_MSG_CTL);
 		temp &= ~(WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4c3cf22..e122b57 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -263,6 +263,11 @@
 #define  MI_SEMAPHORE_SYNC_VVE	    (1<<16) /* VECS wait for VCS  (VEVSYNC) */
 #define  MI_SEMAPHORE_SYNC_RVE	    (2<<16) /* VECS wait for RCS  (VERSYNC) */
 #define  MI_SEMAPHORE_SYNC_INVALID  (3<<16)
+
+#define MI_PREDICATE_RESULT_2	(0x2214)
+#define  LOWER_SLICE_ENABLED	(1<<0)
+#define  LOWER_SLICE_DISABLED	(0<<0)
+
 /*
  * 3D instructions used by the kernel
  */
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH 16/17] RFM drm/i915: Boost RPS frequency for CPU stalls
  2013-08-26 22:51 ` [PATCH 16/17] RFM drm/i915: Boost RPS frequency for CPU stalls Rodrigo Vivi
@ 2013-08-26 22:56   ` Chris Wilson
  0 siblings, 0 replies; 40+ messages in thread
From: Chris Wilson @ 2013-08-26 22:56 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Stéphane Marchesin, intel-gfx, Zhuang, Lena

On Mon, Aug 26, 2013 at 07:51:08PM -0300, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> If we encounter a situation where the CPU blocks waiting for results
> from the GPU, give the GPU a kick to boost its the frequency.
> 
> This should work to reduce user interface stalls and to quickly promote
> mesa to high frequencies - but the cost is that our requested frequency
> stalls high (as we do not idle for long enough before rc6 to start
> reducing frequencies, nor are we aggressive at down clocking an
> underused GPU). However, this should be mitigated by rc6 itself powering
> off the GPU when idle, and that energy use is dependent upon the workload
> of the GPU in addition to its frequency (e.g. the math or sampler
> functions only consume power when used). Still, this is likely to
> adversely affect light workloads.
> 
> Stéphane raised the concern that this will punish good applications and
> reward bad applications - but due to the nature of how mesa performs its
> client throttling, I believe all mesa applications will be roughly
> equally affected.
> 
> RFM - request for measurement!
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Stéphane Marchesin <stephane.marchesin@gmail.com>
> Cc: "Meng, Mengmeng" <mengmeng.meng@intel.com>
> Cc: "Zhuang, Lena" <lena.zhuang@intel.com>

This patch is superseded by later versions.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 14/17] drm/i915: Apply the force-detect VGA w/a to Valleyview
  2013-08-26 22:51 ` [PATCH 14/17] drm/i915: Apply the force-detect VGA w/a to Valleyview Rodrigo Vivi
@ 2013-08-27  9:27   ` Daniel Vetter
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Vetter @ 2013-08-27  9:27 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Mon, Aug 26, 2013 at 07:51:06PM -0300, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> It appears that Valleyview shares its VGA encoder with more recent
> siblings and requires the same forced detection cycle after a hardware
> reset before we can rely on hotplugging.
> 
> Reported-and-tested-by: kobeqin <kobe.qin@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67733
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Queued for -next with the bikeshed applied as discussed, thanks for the
patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review
  2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
                   ` (16 preceding siblings ...)
  2013-08-26 22:51 ` [PATCH 17/17] drm/i915: Enable Lower Slice on Haswell GT3 Rodrigo Vivi
@ 2013-08-27  9:39 ` Daniel Vetter
  2013-08-27 10:23   ` Chris Wilson
                     ` (2 more replies)
  17 siblings, 3 replies; 40+ messages in thread
From: Daniel Vetter @ 2013-08-27  9:39 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

Thanks for doing this Rodrigo. Review plan:

Patches 1-4: Damien
Patch 5: Some bikeshed pending from earlier review ...
Patch 6-7: Atm in limbo, I've thought they're merged into the x86 tree already.
Patch 8: Ville
Patch 9-11: Rodrigo
Patch 12: lalala ;-)
Patch 13: I guess this can wait until we allow userspace to allocate
from stolen. So in a way requirements for this are missing from the
series. Maybe just drop for now?
Patch 14: Merged
Patch 15-16: Part of Chris' rps tuning and instrumentation
improvements. We need to review the latest versions of these (I'd
volunteer Jesse for the rps tuning).
Patch 17: Should be tested by someone else with a gt3. Who has one?

Cheers, Daniel

On Tue, Aug 27, 2013 at 12:50 AM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> Hi all,
>
> Let me introduce drm-intel-collector branch:
> http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector
>
> To describe drm-intel-collector I'll quote Daniel:
> "The overall idea is to make sure that simple patches don't get lost.
> Bigger patch series or feature work tends to not get lost, and really
> trivial patches I tend to merge right away. But 1-2 patch stuff in
> between is occasionally lost"
>
> Process:
>
> 1. Daniel pushs drm-intel-testing
> 2. I rebase drm-intel-collector onto drm-intel-testing
> 3. I collect all simple (1-2) patches that wasn't yet reviewed and not queued by Daniel
> 4. Request automated QA's PRTS automated i-g-t tests comparing drm-intel-testing x drm-intel-collector
> 5. If tests are ok I send the patches as a series to intel-gfx mailing list for better tracking and to be reviewed.
>
> There are some reasons that some patches can be left behind:
> 1. It was send so long time ago. I started with patches from Jul 26th.
> 2. Your patch didn't applied cleanly and I couldn't easily solve the conflicts.
> 3. Kernel didn't compiled with your patch.
> 4. I simply missed it. If you believe this is the case please warn me.
>
> Please help me to get these patches reviewed and queued by Daniel.
>
> Also, please let me know if you have further ideas how to improve this process.
>
> Thanks in advance,
> Rodrigo.
>
> Chris Wilson (13):
>   drm/i915: Do not add an interrupt for a context switch
>   drm/i915: Rearrange the comments in i915_add_request()
>   drm/i915: Pin pages whilst mapping the dma-buf
>   drm/i915: Cancel outstanding modeset workers before suspend
>   drm/i915: Always prefer CPU relocations with LLC
>   drm/i915: Report requested frequency alongside current frequency in
>     debugfs
>   drm/i915: Move the conditional seqno query into the tracepoint
>   drm/i915: Add some missing steps to i915_driver_load error path
>   drm/i915: Asynchronously perform the set-base for a simple modeset
>   drm/i915: Align tiled scanouts from stolen memory to 256k in the GTT
>   drm/i915: Apply the force-detect VGA w/a to Valleyview
>   drm/i915: Pair seqno completion tracepoint with its dispatch
>   RFM drm/i915: Boost RPS frequency for CPU stalls
>
> Daniel Vetter (1):
>   drm/i915: check that the i965g/gm 4G limit is really obeyed
>
> Jesse Barnes (2):
>   drm/i915: split PCI IDs out into i915_drm.h v4
>   x86: add early quirk for reserving Intel graphics stolen memory v5
>
> Rodrigo Vivi (1):
>   drm/i915: Enable Lower Slice on Haswell GT3.
>
>  arch/x86/kernel/early-quirks.c             | 154 +++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_debugfs.c        |  11 +-
>  drivers/gpu/drm/i915/i915_dma.c            |  19 ++-
>  drivers/gpu/drm/i915/i915_drv.c            | 164 +++++-----------------
>  drivers/gpu/drm/i915/i915_drv.h            |   3 +
>  drivers/gpu/drm/i915/i915_gem.c            |  25 +++-
>  drivers/gpu/drm/i915/i915_gem_context.c    |  12 +-
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c     |  41 +++---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   7 +-
>  drivers/gpu/drm/i915/i915_irq.c            |   2 +-
>  drivers/gpu/drm/i915/i915_reg.h            |  20 +--
>  drivers/gpu/drm/i915/i915_trace.h          |  33 +++--
>  drivers/gpu/drm/i915/intel_crt.c           |   2 +-
>  drivers/gpu/drm/i915/intel_display.c       |  34 +++--
>  include/drm/i915_drm.h                     |  34 +++++
>  include/drm/i915_pciids.h                  | 211 +++++++++++++++++++++++++++++
>  16 files changed, 566 insertions(+), 206 deletions(-)
>  create mode 100644 include/drm/i915_pciids.h
>
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review
  2013-08-27  9:39 ` [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Daniel Vetter
@ 2013-08-27 10:23   ` Chris Wilson
  2013-08-27 12:48   ` Rodrigo Vivi
  2013-08-27 16:19   ` Chris Wilson
  2 siblings, 0 replies; 40+ messages in thread
From: Chris Wilson @ 2013-08-27 10:23 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Tue, Aug 27, 2013 at 11:39:52AM +0200, Daniel Vetter wrote:
> Thanks for doing this Rodrigo. Review plan:

Please when doing these summaries, include the patch titles.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 09/17] drm/i915: Report requested frequency alongside current frequency in debugfs
  2013-08-26 22:51 ` [PATCH 09/17] drm/i915: Report requested frequency alongside current frequency in debugfs Rodrigo Vivi
@ 2013-08-27 12:12   ` Rodrigo Vivi
  2013-08-27 12:36     ` Chris Wilson
  2013-08-28  8:15     ` Daniel Vetter
  0 siblings, 2 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-27 12:12 UTC (permalink / raw)
  To: intel-gfx

On Mon, Aug 26, 2013 at 7:51 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> It can be useful to compare at times the current vs requested frequency
> of the GPU, so provide the contents of RPNSWREQ alonside CAGF.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 55ab924..a6f4cb5 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -857,7 +857,7 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
>                 u32 gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
>                 u32 rp_state_limits = I915_READ(GEN6_RP_STATE_LIMITS);
>                 u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> -               u32 rpstat, cagf;
> +               u32 rpstat, cagf, reqf;
>                 u32 rpupei, rpcurup, rpprevup;
>                 u32 rpdownei, rpcurdown, rpprevdown;
>                 int max_freq;
> @@ -869,6 +869,14 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
>
>                 gen6_gt_force_wake_get(dev_priv);
>
> +               reqf = I915_READ(GEN6_RPNSWREQ);
> +               reqf &= ~GEN6_TURBO_DISABLE;
> +               if (IS_HASWELL(dev))
> +                       reqf >>= 24;
> +               else
> +                       reqf >>= 25;

I would prefer a define like HSW_REQF_SHIFT 24 close to HSW_FREQUENCY
to avoid people asking why magic 24/25 value.
I added the HSW_FREQUENCY there and still asked "why 24?"

But with or without this bikeshed:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

> +               reqf *= GT_FREQUENCY_MULTIPLIER;
> +
>                 rpstat = I915_READ(GEN6_RPSTAT1);
>                 rpupei = I915_READ(GEN6_RP_CUR_UP_EI);
>                 rpcurup = I915_READ(GEN6_RP_CUR_UP);
> @@ -893,6 +901,7 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
>                            gt_perf_status & 0xff);
>                 seq_printf(m, "Render p-state limit: %d\n",
>                            rp_state_limits & 0xff);
> +               seq_printf(m, "RPNSWREQ: %dMHz\n", reqf);
>                 seq_printf(m, "CAGF: %dMHz\n", cagf);
>                 seq_printf(m, "RP CUR UP EI: %dus\n", rpupei &
>                            GEN6_CURICONT_MASK);
> --
> 1.8.1.4
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 09/17] drm/i915: Report requested frequency alongside current frequency in debugfs
  2013-08-27 12:12   ` Rodrigo Vivi
@ 2013-08-27 12:36     ` Chris Wilson
  2013-08-28  8:15     ` Daniel Vetter
  1 sibling, 0 replies; 40+ messages in thread
From: Chris Wilson @ 2013-08-27 12:36 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Tue, Aug 27, 2013 at 09:12:40AM -0300, Rodrigo Vivi wrote:
> On Mon, Aug 26, 2013 at 7:51 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > It can be useful to compare at times the current vs requested frequency
> > of the GPU, so provide the contents of RPNSWREQ alonside CAGF.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 55ab924..a6f4cb5 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -857,7 +857,7 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
> >                 u32 gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
> >                 u32 rp_state_limits = I915_READ(GEN6_RP_STATE_LIMITS);
> >                 u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> > -               u32 rpstat, cagf;
> > +               u32 rpstat, cagf, reqf;
> >                 u32 rpupei, rpcurup, rpprevup;
> >                 u32 rpdownei, rpcurdown, rpprevdown;
> >                 int max_freq;
> > @@ -869,6 +869,14 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
> >
> >                 gen6_gt_force_wake_get(dev_priv);
> >
> > +               reqf = I915_READ(GEN6_RPNSWREQ);
> > +               reqf &= ~GEN6_TURBO_DISABLE;
> > +               if (IS_HASWELL(dev))
> > +                       reqf >>= 24;
> > +               else
> > +                       reqf >>= 25;
> 
> I would prefer a define like HSW_REQF_SHIFT 24 close to HSW_FREQUENCY
> to avoid people asking why magic 24/25 value.
> I added the HSW_FREQUENCY there and still asked "why 24?"

Heck, I wrote it straight out of the bspec, and was too lazy to chase
the meaning of CAGF_SHIFT simply because it is not obvious from reading
the code and require header chasing.

I'll let our hard working maintainer decide how he wants it if he
applies it. For longer term, I have this exported through perf anyway.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review
  2013-08-27  9:39 ` [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Daniel Vetter
  2013-08-27 10:23   ` Chris Wilson
@ 2013-08-27 12:48   ` Rodrigo Vivi
  2013-08-27 16:19   ` Chris Wilson
  2 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-27 12:48 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Tue, Aug 27, 2013 at 6:39 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> Thanks for doing this Rodrigo. Review plan:
>
> Patches 1-4: Damien
> Patch 5: Some bikeshed pending from earlier review ...
> Patch 6-7: Atm in limbo, I've thought they're merged into the x86 tree already.
> Patch 8: Ville
> Patch 9-11: Rodrigo
9 - done
10 - I think it can be left with 15-16.
11 - Sorry, but I couldn't review it. I think Ben is the better one to
review it.

> Patch 12: lalala ;-)
> Patch 13: I guess this can wait until we allow userspace to allocate
> from stolen. So in a way requirements for this are missing from the
> series. Maybe just drop for now?
> Patch 14: Merged
> Patch 15-16: Part of Chris' rps tuning and instrumentation
> improvements. We need to review the latest versions of these (I'd
> volunteer Jesse for the rps tuning).
> Patch 17: Should be tested by someone else with a gt3. Who has one?
>
> Cheers, Daniel
>
> On Tue, Aug 27, 2013 at 12:50 AM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
>> Hi all,
>>
>> Let me introduce drm-intel-collector branch:
>> http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector
>>
>> To describe drm-intel-collector I'll quote Daniel:
>> "The overall idea is to make sure that simple patches don't get lost.
>> Bigger patch series or feature work tends to not get lost, and really
>> trivial patches I tend to merge right away. But 1-2 patch stuff in
>> between is occasionally lost"
>>
>> Process:
>>
>> 1. Daniel pushs drm-intel-testing
>> 2. I rebase drm-intel-collector onto drm-intel-testing
>> 3. I collect all simple (1-2) patches that wasn't yet reviewed and not queued by Daniel
>> 4. Request automated QA's PRTS automated i-g-t tests comparing drm-intel-testing x drm-intel-collector
>> 5. If tests are ok I send the patches as a series to intel-gfx mailing list for better tracking and to be reviewed.
>>
>> There are some reasons that some patches can be left behind:
>> 1. It was send so long time ago. I started with patches from Jul 26th.
>> 2. Your patch didn't applied cleanly and I couldn't easily solve the conflicts.
>> 3. Kernel didn't compiled with your patch.
>> 4. I simply missed it. If you believe this is the case please warn me.
>>
>> Please help me to get these patches reviewed and queued by Daniel.
>>
>> Also, please let me know if you have further ideas how to improve this process.
>>
>> Thanks in advance,
>> Rodrigo.
>>
>> Chris Wilson (13):
>>   drm/i915: Do not add an interrupt for a context switch
>>   drm/i915: Rearrange the comments in i915_add_request()
>>   drm/i915: Pin pages whilst mapping the dma-buf
>>   drm/i915: Cancel outstanding modeset workers before suspend
>>   drm/i915: Always prefer CPU relocations with LLC
>>   drm/i915: Report requested frequency alongside current frequency in
>>     debugfs
>>   drm/i915: Move the conditional seqno query into the tracepoint
>>   drm/i915: Add some missing steps to i915_driver_load error path
>>   drm/i915: Asynchronously perform the set-base for a simple modeset
>>   drm/i915: Align tiled scanouts from stolen memory to 256k in the GTT
>>   drm/i915: Apply the force-detect VGA w/a to Valleyview
>>   drm/i915: Pair seqno completion tracepoint with its dispatch
>>   RFM drm/i915: Boost RPS frequency for CPU stalls
>>
>> Daniel Vetter (1):
>>   drm/i915: check that the i965g/gm 4G limit is really obeyed
>>
>> Jesse Barnes (2):
>>   drm/i915: split PCI IDs out into i915_drm.h v4
>>   x86: add early quirk for reserving Intel graphics stolen memory v5
>>
>> Rodrigo Vivi (1):
>>   drm/i915: Enable Lower Slice on Haswell GT3.
>>
>>  arch/x86/kernel/early-quirks.c             | 154 +++++++++++++++++++++
>>  drivers/gpu/drm/i915/i915_debugfs.c        |  11 +-
>>  drivers/gpu/drm/i915/i915_dma.c            |  19 ++-
>>  drivers/gpu/drm/i915/i915_drv.c            | 164 +++++-----------------
>>  drivers/gpu/drm/i915/i915_drv.h            |   3 +
>>  drivers/gpu/drm/i915/i915_gem.c            |  25 +++-
>>  drivers/gpu/drm/i915/i915_gem_context.c    |  12 +-
>>  drivers/gpu/drm/i915/i915_gem_dmabuf.c     |  41 +++---
>>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   7 +-
>>  drivers/gpu/drm/i915/i915_irq.c            |   2 +-
>>  drivers/gpu/drm/i915/i915_reg.h            |  20 +--
>>  drivers/gpu/drm/i915/i915_trace.h          |  33 +++--
>>  drivers/gpu/drm/i915/intel_crt.c           |   2 +-
>>  drivers/gpu/drm/i915/intel_display.c       |  34 +++--
>>  include/drm/i915_drm.h                     |  34 +++++
>>  include/drm/i915_pciids.h                  | 211 +++++++++++++++++++++++++++++
>>  16 files changed, 566 insertions(+), 206 deletions(-)
>>  create mode 100644 include/drm/i915_pciids.h
>>
>> --
>> 1.8.1.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 08/17] drm/i915: Always prefer CPU relocations with LLC
  2013-08-26 22:51 ` [PATCH 08/17] drm/i915: Always prefer CPU relocations with LLC Rodrigo Vivi
@ 2013-08-27 14:49   ` Ville Syrjälä
  2013-08-27 15:25     ` Daniel Vetter
  2013-08-29 17:20   ` Ben Widawsky
  1 sibling, 1 reply; 40+ messages in thread
From: Ville Syrjälä @ 2013-08-27 14:49 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Mon, Aug 26, 2013 at 07:51:00PM -0300, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> A follow-on to the update of the LLC coherency logic is that we can rely
> on the LLC being coherent with the CS for rewriting batchbuffers
> irrespective of their cache domain. (This should have no effect
> currently as all the batch buffers are expected to be I915_CACHE_LLC and
> so using the cpu relocation path anyway.)
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Makes sense.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 792c52a..3b64b9f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -166,7 +166,8 @@ eb_destroy(struct eb_objects *eb)
>  
>  static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
>  {
> -	return (obj->base.write_domain == I915_GEM_DOMAIN_CPU ||
> +	return (HAS_LLC(obj->base.dev) ||
> +		obj->base.write_domain == I915_GEM_DOMAIN_CPU ||
>  		!obj->map_and_fenceable ||
>  		obj->cache_level != I915_CACHE_NONE);
>  }
> @@ -179,7 +180,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
>  	char *vaddr;
>  	int ret = -EINVAL;
>  
> -	ret = i915_gem_object_set_to_cpu_domain(obj, 1);
> +	ret = i915_gem_object_set_to_cpu_domain(obj, true);
>  	if (ret)
>  		return ret;
>  
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 08/17] drm/i915: Always prefer CPU relocations with LLC
  2013-08-27 14:49   ` Ville Syrjälä
@ 2013-08-27 15:25     ` Daniel Vetter
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Vetter @ 2013-08-27 15:25 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Tue, Aug 27, 2013 at 05:49:24PM +0300, Ville Syrjälä wrote:
> On Mon, Aug 26, 2013 at 07:51:00PM -0300, Rodrigo Vivi wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > A follow-on to the update of the LLC coherency logic is that we can rely
> > on the LLC being coherent with the CS for rewriting batchbuffers
> > irrespective of their cache domain. (This should have no effect
> > currently as all the batch buffers are expected to be I915_CACHE_LLC and
> > so using the cpu relocation path anyway.)
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Makes sense.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 17/17] drm/i915: Enable Lower Slice on Haswell GT3.
  2013-08-26 22:51 ` [PATCH 17/17] drm/i915: Enable Lower Slice on Haswell GT3 Rodrigo Vivi
@ 2013-08-27 15:31   ` Rodrigo Vivi
  2013-08-27 17:05     ` Rodrigo Vivi
  0 siblings, 1 reply; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-27 15:31 UTC (permalink / raw)
  To: intel-gfx

Tested-by: Meng, Mengmeng <mengmeng.meng@intel.com>



On Mon, Aug 26, 2013 at 7:51 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> Full HSW GT3 power can only be achieved with all Execution Units turned on.
> This patch enables all EUs present on HSW GT3 by enabling lower slice.
>
> Credits-by: Yejun Guo <yejun.guo@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 2 ++
>  drivers/gpu/drm/i915/i915_gem.c | 5 +++++
>  drivers/gpu/drm/i915/i915_reg.h | 5 +++++
>  3 files changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7ac5ca5..e9b2f86 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1592,6 +1592,8 @@ struct drm_i915_file_private {
>                                  ((dev)->pci_device & 0xFF00) == 0x0C00)
>  #define IS_ULT(dev)            (IS_HASWELL(dev) && \
>                                  ((dev)->pci_device & 0xFF00) == 0x0A00)
> +#define IS_HSW_GT3(dev)                (IS_HASWELL(dev) && \
> +                                ((dev)->pci_device & 0x00F0) == 0x0020)
>
>  /*
>   * The genX designation typically refers to the render engine, so render
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 50a70ee..1c72229 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4316,6 +4316,11 @@ i915_gem_init_hw(struct drm_device *dev)
>         if (dev_priv->ellc_size)
>                 I915_WRITE(HSW_IDICR, I915_READ(HSW_IDICR) | IDIHASHMSK(0xf));
>
> +       if (IS_HSW_GT3(dev))
> +               I915_WRITE(MI_PREDICATE_RESULT_2, LOWER_SLICE_ENABLED);
> +       else
> +               I915_WRITE(MI_PREDICATE_RESULT_2, LOWER_SLICE_DISABLED);
> +
>         if (HAS_PCH_NOP(dev)) {
>                 u32 temp = I915_READ(GEN7_MSG_CTL);
>                 temp &= ~(WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4c3cf22..e122b57 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -263,6 +263,11 @@
>  #define  MI_SEMAPHORE_SYNC_VVE     (1<<16) /* VECS wait for VCS  (VEVSYNC) */
>  #define  MI_SEMAPHORE_SYNC_RVE     (2<<16) /* VECS wait for RCS  (VERSYNC) */
>  #define  MI_SEMAPHORE_SYNC_INVALID  (3<<16)
> +
> +#define MI_PREDICATE_RESULT_2  (0x2214)
> +#define  LOWER_SLICE_ENABLED   (1<<0)
> +#define  LOWER_SLICE_DISABLED  (0<<0)
> +
>  /*
>   * 3D instructions used by the kernel
>   */
> --
> 1.8.1.4
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review
  2013-08-27  9:39 ` [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Daniel Vetter
  2013-08-27 10:23   ` Chris Wilson
  2013-08-27 12:48   ` Rodrigo Vivi
@ 2013-08-27 16:19   ` Chris Wilson
  2013-08-27 17:04     ` Rodrigo Vivi
  2 siblings, 1 reply; 40+ messages in thread
From: Chris Wilson @ 2013-08-27 16:19 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Tue, Aug 27, 2013 at 11:39:52AM +0200, Daniel Vetter wrote:
> Patch 17: Should be tested by someone else with a gt3. Who has one?

More missing mails, I haven't got the patch to comment on, so bare with
me.

The title and changelog is misleading, this is not about enabling Lower
Slice at all.

Something like:

"drm/i915: Report enabled slices on Haswell GT3

Batchbuffers constructed by userspace can conditionalise their URB
allocations through the use of the MI_SET_PREDICATE command. This
command can read the MI_PREDICATE_RESULT_2 register to see how many
slices are enabled on GT3, and by virtue of the result, scale their
memory allocations to fit enabled memory.

Of course, this only works if the kernel sets the appropriate bit in the
register first."

This doesn't attempt to explain the complexity of why we have the same
informatiom in multiple registers and why the hw designers thought it
wise for sw to keep them all in sync...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review
  2013-08-27 16:19   ` Chris Wilson
@ 2013-08-27 17:04     ` Rodrigo Vivi
  0 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-27 17:04 UTC (permalink / raw)
  To: Chris Wilson, Daniel Vetter, Rodrigo Vivi, intel-gfx

On Tue, Aug 27, 2013 at 1:19 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Tue, Aug 27, 2013 at 11:39:52AM +0200, Daniel Vetter wrote:
>> Patch 17: Should be tested by someone else with a gt3. Who has one?
>
> More missing mails, I haven't got the patch to comment on, so bare with
> me.

No problem... it seems I'll have to resend it anyway ;)

>
> The title and changelog is misleading, this is not about enabling Lower
> Slice at all.

To be honest this is what I believed when I was playing with slices
shutdown few months ago...
But flutuation on performances tests might be confused some people (me
included).

>
> Something like:
>
> "drm/i915: Report enabled slices on Haswell GT3
>
> Batchbuffers constructed by userspace can conditionalise their URB
> allocations through the use of the MI_SET_PREDICATE command. This
> command can read the MI_PREDICATE_RESULT_2 register to see how many
> slices are enabled on GT3, and by virtue of the result, scale their
> memory allocations to fit enabled memory.

Agree... Will resend the patch modifying the comment...


> Of course, this only works if the kernel sets the appropriate bit in the
> register first."
>
> This doesn't attempt to explain the complexity of why we have the same
> informatiom in multiple registers and why the hw designers thought it
> wise for sw to keep them all in sync...

agree :/

> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 17/17] drm/i915: Enable Lower Slice on Haswell GT3.
  2013-08-27 15:31   ` Rodrigo Vivi
@ 2013-08-27 17:05     ` Rodrigo Vivi
  0 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Vivi @ 2013-08-27 17:05 UTC (permalink / raw)
  To: intel-gfx

Please don't merge this one... I'll resend it changing the commit
message and subject soon

On Tue, Aug 27, 2013 at 12:31 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> Tested-by: Meng, Mengmeng <mengmeng.meng@intel.com>
>
>
>
> On Mon, Aug 26, 2013 at 7:51 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
>> Full HSW GT3 power can only be achieved with all Execution Units turned on.
>> This patch enables all EUs present on HSW GT3 by enabling lower slice.
>>
>> Credits-by: Yejun Guo <yejun.guo@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h | 2 ++
>>  drivers/gpu/drm/i915/i915_gem.c | 5 +++++
>>  drivers/gpu/drm/i915/i915_reg.h | 5 +++++
>>  3 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 7ac5ca5..e9b2f86 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1592,6 +1592,8 @@ struct drm_i915_file_private {
>>                                  ((dev)->pci_device & 0xFF00) == 0x0C00)
>>  #define IS_ULT(dev)            (IS_HASWELL(dev) && \
>>                                  ((dev)->pci_device & 0xFF00) == 0x0A00)
>> +#define IS_HSW_GT3(dev)                (IS_HASWELL(dev) && \
>> +                                ((dev)->pci_device & 0x00F0) == 0x0020)
>>
>>  /*
>>   * The genX designation typically refers to the render engine, so render
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 50a70ee..1c72229 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -4316,6 +4316,11 @@ i915_gem_init_hw(struct drm_device *dev)
>>         if (dev_priv->ellc_size)
>>                 I915_WRITE(HSW_IDICR, I915_READ(HSW_IDICR) | IDIHASHMSK(0xf));
>>
>> +       if (IS_HSW_GT3(dev))
>> +               I915_WRITE(MI_PREDICATE_RESULT_2, LOWER_SLICE_ENABLED);
>> +       else
>> +               I915_WRITE(MI_PREDICATE_RESULT_2, LOWER_SLICE_DISABLED);
>> +
>>         if (HAS_PCH_NOP(dev)) {
>>                 u32 temp = I915_READ(GEN7_MSG_CTL);
>>                 temp &= ~(WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK);
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 4c3cf22..e122b57 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -263,6 +263,11 @@
>>  #define  MI_SEMAPHORE_SYNC_VVE     (1<<16) /* VECS wait for VCS  (VEVSYNC) */
>>  #define  MI_SEMAPHORE_SYNC_RVE     (2<<16) /* VECS wait for RCS  (VERSYNC) */
>>  #define  MI_SEMAPHORE_SYNC_INVALID  (3<<16)
>> +
>> +#define MI_PREDICATE_RESULT_2  (0x2214)
>> +#define  LOWER_SLICE_ENABLED   (1<<0)
>> +#define  LOWER_SLICE_DISABLED  (0<<0)
>> +
>>  /*
>>   * 3D instructions used by the kernel
>>   */
>> --
>> 1.8.1.4
>>
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 09/17] drm/i915: Report requested frequency alongside current frequency in debugfs
  2013-08-27 12:12   ` Rodrigo Vivi
  2013-08-27 12:36     ` Chris Wilson
@ 2013-08-28  8:15     ` Daniel Vetter
  1 sibling, 0 replies; 40+ messages in thread
From: Daniel Vetter @ 2013-08-28  8:15 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Tue, Aug 27, 2013 at 09:12:40AM -0300, Rodrigo Vivi wrote:
> On Mon, Aug 26, 2013 at 7:51 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > It can be useful to compare at times the current vs requested frequency
> > of the GPU, so provide the contents of RPNSWREQ alonside CAGF.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 55ab924..a6f4cb5 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -857,7 +857,7 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
> >                 u32 gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
> >                 u32 rp_state_limits = I915_READ(GEN6_RP_STATE_LIMITS);
> >                 u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> > -               u32 rpstat, cagf;
> > +               u32 rpstat, cagf, reqf;
> >                 u32 rpupei, rpcurup, rpprevup;
> >                 u32 rpdownei, rpcurdown, rpprevdown;
> >                 int max_freq;
> > @@ -869,6 +869,14 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
> >
> >                 gen6_gt_force_wake_get(dev_priv);
> >
> > +               reqf = I915_READ(GEN6_RPNSWREQ);
> > +               reqf &= ~GEN6_TURBO_DISABLE;
> > +               if (IS_HASWELL(dev))
> > +                       reqf >>= 24;
> > +               else
> > +                       reqf >>= 25;
> 
> I would prefer a define like HSW_REQF_SHIFT 24 close to HSW_FREQUENCY
> to avoid people asking why magic 24/25 value.
> I added the HSW_FREQUENCY there and still asked "why 24?"
> 
> But with or without this bikeshed:
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 08/17] drm/i915: Always prefer CPU relocations with LLC
  2013-08-26 22:51 ` [PATCH 08/17] drm/i915: Always prefer CPU relocations with LLC Rodrigo Vivi
  2013-08-27 14:49   ` Ville Syrjälä
@ 2013-08-29 17:20   ` Ben Widawsky
  2013-08-29 19:11     ` Daniel Vetter
  1 sibling, 1 reply; 40+ messages in thread
From: Ben Widawsky @ 2013-08-29 17:20 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Mon, Aug 26, 2013 at 07:51:00PM -0300, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> A follow-on to the update of the LLC coherency logic is that we can rely
> on the LLC being coherent with the CS for rewriting batchbuffers
> irrespective of their cache domain. (This should have no effect
> currently as all the batch buffers are expected to be I915_CACHE_LLC and
> so using the cpu relocation path anyway.)
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 792c52a..3b64b9f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -166,7 +166,8 @@ eb_destroy(struct eb_objects *eb)
>  
>  static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
>  {
> -	return (obj->base.write_domain == I915_GEM_DOMAIN_CPU ||
> +	return (HAS_LLC(obj->base.dev) ||
> +		obj->base.write_domain == I915_GEM_DOMAIN_CPU ||
>  		!obj->map_and_fenceable ||
>  		obj->cache_level != I915_CACHE_NONE);

Assuming the commit message is factually correct... the obj->cache_level
shouldn't factor into the equation at all.

>  }
> @@ -179,7 +180,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
>  	char *vaddr;
>  	int ret = -EINVAL;
>  
> -	ret = i915_gem_object_set_to_cpu_domain(obj, 1);
> +	ret = i915_gem_object_set_to_cpu_domain(obj, true);
>  	if (ret)
>  		return ret;
>  
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 06/17] drm/i915: split PCI IDs out into i915_drm.h v4
  2013-08-26 22:50 ` [PATCH 06/17] drm/i915: split PCI IDs out into i915_drm.h v4 Rodrigo Vivi
@ 2013-08-29 17:27   ` Ben Widawsky
  0 siblings, 0 replies; 40+ messages in thread
From: Ben Widawsky @ 2013-08-29 17:27 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Mon, Aug 26, 2013 at 07:50:58PM -0300, Rodrigo Vivi wrote:
> From: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> For use by userspace (at some point in the future) and other kernel code.
> 
> v2: move PCI IDs to uabi (Chris)
>     move PCI IDs to drm/ (Dave)
> v3: fixup Quanta detection - needs to come first (Daniel)
> v4: fix up PCI match structure init for easier use by userspace (Chris)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 164 +++++++------------------------
>  include/drm/i915_drm.h          |   2 +
>  include/drm/i915_pciids.h       | 211 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 247 insertions(+), 130 deletions(-)
>  create mode 100644 include/drm/i915_pciids.h
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 735dd56..3872f66 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -157,25 +157,6 @@ MODULE_PARM_DESC(prefault_disable,
>  static struct drm_driver driver;
>  extern int intel_agp_enabled;
>  
> -#define INTEL_VGA_DEVICE(id, info) {		\
> -	.class = PCI_BASE_CLASS_DISPLAY << 16,	\
> -	.class_mask = 0xff0000,			\
> -	.vendor = 0x8086,			\
> -	.device = id,				\
> -	.subvendor = PCI_ANY_ID,		\
> -	.subdevice = PCI_ANY_ID,		\
> -	.driver_data = (unsigned long) info }
> -
> -#define INTEL_QUANTA_VGA_DEVICE(info) {		\
> -	.class = PCI_BASE_CLASS_DISPLAY << 16,	\
> -	.class_mask = 0xff0000,			\
> -	.vendor = 0x8086,			\
> -	.device = 0x16a,			\
> -	.subvendor = 0x152d,			\
> -	.subdevice = 0x8990,			\
> -	.driver_data = (unsigned long) info }
> -
> -
>  static const struct intel_device_info intel_i830_info = {
>  	.gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2,
>  	.has_overlay = 1, .overlay_needs_physical = 1,
> @@ -350,118 +331,41 @@ static const struct intel_device_info intel_haswell_m_info = {
>  	.has_vebox_ring = 1,
>  };
>  
> +/*
> + * Make sure any device matches here are from most specific to most
> + * general.  For example, since the Quanta match is based on the subsystem
> + * and subvendor IDs, we need it to come before the more general IVB
> + * PCI ID matches, otherwise we'll use the wrong info struct above.
> + */
> +#define INTEL_PCI_IDS \
> +	INTEL_I830_IDS(&intel_i830_info),	\
> +	INTEL_I845G_IDS(&intel_845g_info),	\
> +	INTEL_I85X_IDS(&intel_i85x_info),	\
> +	INTEL_I865G_IDS(&intel_i865g_info),	\
> +	INTEL_I915G_IDS(&intel_i915g_info),	\
> +	INTEL_I915GM_IDS(&intel_i915gm_info),	\
> +	INTEL_I945G_IDS(&intel_i945g_info),	\
> +	INTEL_I945GM_IDS(&intel_i945gm_info),	\
> +	INTEL_I965G_IDS(&intel_i965g_info),	\
> +	INTEL_G33_IDS(&intel_g33_info),		\
> +	INTEL_I965GM_IDS(&intel_i965gm_info),	\
> +	INTEL_GM45_IDS(&intel_gm45_info), 	\
> +	INTEL_G45_IDS(&intel_g45_info), 	\
> +	INTEL_PINEVIEW_IDS(&intel_pineview_info),	\
> +	INTEL_IRONLAKE_D_IDS(&intel_ironlake_d_info),	\
> +	INTEL_IRONLAKE_M_IDS(&intel_ironlake_m_info),	\
> +	INTEL_SNB_D_IDS(&intel_sandybridge_d_info),	\
> +	INTEL_SNB_M_IDS(&intel_sandybridge_m_info),	\
> +	INTEL_IVB_Q_IDS(&intel_ivybridge_q_info), /* must be first IVB */ \
> +	INTEL_IVB_M_IDS(&intel_ivybridge_m_info),	\
> +	INTEL_IVB_D_IDS(&intel_ivybridge_d_info),	\
> +	INTEL_HSW_D_IDS(&intel_haswell_d_info), \
> +	INTEL_HSW_M_IDS(&intel_haswell_m_info), \
> +	INTEL_VLV_M_IDS(&intel_valleyview_m_info),	\
> +	INTEL_VLV_D_IDS(&intel_valleyview_d_info)
> +

I don't really see a benefit to making this list a separate define.
Seems just as well to use it directly in pciidlist[].

Acked-by: Ben Widawsky <ben@bwidawsk.net>

>  static const struct pci_device_id pciidlist[] = {		/* aka */
> -	INTEL_VGA_DEVICE(0x3577, &intel_i830_info),		/* I830_M */
> -	INTEL_VGA_DEVICE(0x2562, &intel_845g_info),		/* 845_G */
> -	INTEL_VGA_DEVICE(0x3582, &intel_i85x_info),		/* I855_GM */
> -	INTEL_VGA_DEVICE(0x358e, &intel_i85x_info),
> -	INTEL_VGA_DEVICE(0x2572, &intel_i865g_info),		/* I865_G */
> -	INTEL_VGA_DEVICE(0x2582, &intel_i915g_info),		/* I915_G */
> -	INTEL_VGA_DEVICE(0x258a, &intel_i915g_info),		/* E7221_G */
> -	INTEL_VGA_DEVICE(0x2592, &intel_i915gm_info),		/* I915_GM */
> -	INTEL_VGA_DEVICE(0x2772, &intel_i945g_info),		/* I945_G */
> -	INTEL_VGA_DEVICE(0x27a2, &intel_i945gm_info),		/* I945_GM */
> -	INTEL_VGA_DEVICE(0x27ae, &intel_i945gm_info),		/* I945_GME */
> -	INTEL_VGA_DEVICE(0x2972, &intel_i965g_info),		/* I946_GZ */
> -	INTEL_VGA_DEVICE(0x2982, &intel_i965g_info),		/* G35_G */
> -	INTEL_VGA_DEVICE(0x2992, &intel_i965g_info),		/* I965_Q */
> -	INTEL_VGA_DEVICE(0x29a2, &intel_i965g_info),		/* I965_G */
> -	INTEL_VGA_DEVICE(0x29b2, &intel_g33_info),		/* Q35_G */
> -	INTEL_VGA_DEVICE(0x29c2, &intel_g33_info),		/* G33_G */
> -	INTEL_VGA_DEVICE(0x29d2, &intel_g33_info),		/* Q33_G */
> -	INTEL_VGA_DEVICE(0x2a02, &intel_i965gm_info),		/* I965_GM */
> -	INTEL_VGA_DEVICE(0x2a12, &intel_i965gm_info),		/* I965_GME */
> -	INTEL_VGA_DEVICE(0x2a42, &intel_gm45_info),		/* GM45_G */
> -	INTEL_VGA_DEVICE(0x2e02, &intel_g45_info),		/* IGD_E_G */
> -	INTEL_VGA_DEVICE(0x2e12, &intel_g45_info),		/* Q45_G */
> -	INTEL_VGA_DEVICE(0x2e22, &intel_g45_info),		/* G45_G */
> -	INTEL_VGA_DEVICE(0x2e32, &intel_g45_info),		/* G41_G */
> -	INTEL_VGA_DEVICE(0x2e42, &intel_g45_info),		/* B43_G */
> -	INTEL_VGA_DEVICE(0x2e92, &intel_g45_info),		/* B43_G.1 */
> -	INTEL_VGA_DEVICE(0xa001, &intel_pineview_info),
> -	INTEL_VGA_DEVICE(0xa011, &intel_pineview_info),
> -	INTEL_VGA_DEVICE(0x0042, &intel_ironlake_d_info),
> -	INTEL_VGA_DEVICE(0x0046, &intel_ironlake_m_info),
> -	INTEL_VGA_DEVICE(0x0102, &intel_sandybridge_d_info),
> -	INTEL_VGA_DEVICE(0x0112, &intel_sandybridge_d_info),
> -	INTEL_VGA_DEVICE(0x0122, &intel_sandybridge_d_info),
> -	INTEL_VGA_DEVICE(0x0106, &intel_sandybridge_m_info),
> -	INTEL_VGA_DEVICE(0x0116, &intel_sandybridge_m_info),
> -	INTEL_VGA_DEVICE(0x0126, &intel_sandybridge_m_info),
> -	INTEL_VGA_DEVICE(0x010A, &intel_sandybridge_d_info),
> -	INTEL_VGA_DEVICE(0x0156, &intel_ivybridge_m_info), /* GT1 mobile */
> -	INTEL_VGA_DEVICE(0x0166, &intel_ivybridge_m_info), /* GT2 mobile */
> -	INTEL_VGA_DEVICE(0x0152, &intel_ivybridge_d_info), /* GT1 desktop */
> -	INTEL_VGA_DEVICE(0x0162, &intel_ivybridge_d_info), /* GT2 desktop */
> -	INTEL_VGA_DEVICE(0x015a, &intel_ivybridge_d_info), /* GT1 server */
> -	INTEL_QUANTA_VGA_DEVICE(&intel_ivybridge_q_info), /* Quanta transcode */
> -	INTEL_VGA_DEVICE(0x016a, &intel_ivybridge_d_info), /* GT2 server */
> -	INTEL_VGA_DEVICE(0x0402, &intel_haswell_d_info), /* GT1 desktop */
> -	INTEL_VGA_DEVICE(0x0412, &intel_haswell_d_info), /* GT2 desktop */
> -	INTEL_VGA_DEVICE(0x0422, &intel_haswell_d_info), /* GT3 desktop */
> -	INTEL_VGA_DEVICE(0x040a, &intel_haswell_d_info), /* GT1 server */
> -	INTEL_VGA_DEVICE(0x041a, &intel_haswell_d_info), /* GT2 server */
> -	INTEL_VGA_DEVICE(0x042a, &intel_haswell_d_info), /* GT3 server */
> -	INTEL_VGA_DEVICE(0x0406, &intel_haswell_m_info), /* GT1 mobile */
> -	INTEL_VGA_DEVICE(0x0416, &intel_haswell_m_info), /* GT2 mobile */
> -	INTEL_VGA_DEVICE(0x0426, &intel_haswell_m_info), /* GT2 mobile */
> -	INTEL_VGA_DEVICE(0x040B, &intel_haswell_d_info), /* GT1 reserved */
> -	INTEL_VGA_DEVICE(0x041B, &intel_haswell_d_info), /* GT2 reserved */
> -	INTEL_VGA_DEVICE(0x042B, &intel_haswell_d_info), /* GT3 reserved */
> -	INTEL_VGA_DEVICE(0x040E, &intel_haswell_d_info), /* GT1 reserved */
> -	INTEL_VGA_DEVICE(0x041E, &intel_haswell_d_info), /* GT2 reserved */
> -	INTEL_VGA_DEVICE(0x042E, &intel_haswell_d_info), /* GT3 reserved */
> -	INTEL_VGA_DEVICE(0x0C02, &intel_haswell_d_info), /* SDV GT1 desktop */
> -	INTEL_VGA_DEVICE(0x0C12, &intel_haswell_d_info), /* SDV GT2 desktop */
> -	INTEL_VGA_DEVICE(0x0C22, &intel_haswell_d_info), /* SDV GT3 desktop */
> -	INTEL_VGA_DEVICE(0x0C0A, &intel_haswell_d_info), /* SDV GT1 server */
> -	INTEL_VGA_DEVICE(0x0C1A, &intel_haswell_d_info), /* SDV GT2 server */
> -	INTEL_VGA_DEVICE(0x0C2A, &intel_haswell_d_info), /* SDV GT3 server */
> -	INTEL_VGA_DEVICE(0x0C06, &intel_haswell_m_info), /* SDV GT1 mobile */
> -	INTEL_VGA_DEVICE(0x0C16, &intel_haswell_m_info), /* SDV GT2 mobile */
> -	INTEL_VGA_DEVICE(0x0C26, &intel_haswell_m_info), /* SDV GT3 mobile */
> -	INTEL_VGA_DEVICE(0x0C0B, &intel_haswell_d_info), /* SDV GT1 reserved */
> -	INTEL_VGA_DEVICE(0x0C1B, &intel_haswell_d_info), /* SDV GT2 reserved */
> -	INTEL_VGA_DEVICE(0x0C2B, &intel_haswell_d_info), /* SDV GT3 reserved */
> -	INTEL_VGA_DEVICE(0x0C0E, &intel_haswell_d_info), /* SDV GT1 reserved */
> -	INTEL_VGA_DEVICE(0x0C1E, &intel_haswell_d_info), /* SDV GT2 reserved */
> -	INTEL_VGA_DEVICE(0x0C2E, &intel_haswell_d_info), /* SDV GT3 reserved */
> -	INTEL_VGA_DEVICE(0x0A02, &intel_haswell_d_info), /* ULT GT1 desktop */
> -	INTEL_VGA_DEVICE(0x0A12, &intel_haswell_d_info), /* ULT GT2 desktop */
> -	INTEL_VGA_DEVICE(0x0A22, &intel_haswell_d_info), /* ULT GT3 desktop */
> -	INTEL_VGA_DEVICE(0x0A0A, &intel_haswell_d_info), /* ULT GT1 server */
> -	INTEL_VGA_DEVICE(0x0A1A, &intel_haswell_d_info), /* ULT GT2 server */
> -	INTEL_VGA_DEVICE(0x0A2A, &intel_haswell_d_info), /* ULT GT3 server */
> -	INTEL_VGA_DEVICE(0x0A06, &intel_haswell_m_info), /* ULT GT1 mobile */
> -	INTEL_VGA_DEVICE(0x0A16, &intel_haswell_m_info), /* ULT GT2 mobile */
> -	INTEL_VGA_DEVICE(0x0A26, &intel_haswell_m_info), /* ULT GT3 mobile */
> -	INTEL_VGA_DEVICE(0x0A0B, &intel_haswell_d_info), /* ULT GT1 reserved */
> -	INTEL_VGA_DEVICE(0x0A1B, &intel_haswell_d_info), /* ULT GT2 reserved */
> -	INTEL_VGA_DEVICE(0x0A2B, &intel_haswell_d_info), /* ULT GT3 reserved */
> -	INTEL_VGA_DEVICE(0x0A0E, &intel_haswell_m_info), /* ULT GT1 reserved */
> -	INTEL_VGA_DEVICE(0x0A1E, &intel_haswell_m_info), /* ULT GT2 reserved */
> -	INTEL_VGA_DEVICE(0x0A2E, &intel_haswell_m_info), /* ULT GT3 reserved */
> -	INTEL_VGA_DEVICE(0x0D02, &intel_haswell_d_info), /* CRW GT1 desktop */
> -	INTEL_VGA_DEVICE(0x0D12, &intel_haswell_d_info), /* CRW GT2 desktop */
> -	INTEL_VGA_DEVICE(0x0D22, &intel_haswell_d_info), /* CRW GT3 desktop */
> -	INTEL_VGA_DEVICE(0x0D0A, &intel_haswell_d_info), /* CRW GT1 server */
> -	INTEL_VGA_DEVICE(0x0D1A, &intel_haswell_d_info), /* CRW GT2 server */
> -	INTEL_VGA_DEVICE(0x0D2A, &intel_haswell_d_info), /* CRW GT3 server */
> -	INTEL_VGA_DEVICE(0x0D06, &intel_haswell_m_info), /* CRW GT1 mobile */
> -	INTEL_VGA_DEVICE(0x0D16, &intel_haswell_m_info), /* CRW GT2 mobile */
> -	INTEL_VGA_DEVICE(0x0D26, &intel_haswell_m_info), /* CRW GT3 mobile */
> -	INTEL_VGA_DEVICE(0x0D0B, &intel_haswell_d_info), /* CRW GT1 reserved */
> -	INTEL_VGA_DEVICE(0x0D1B, &intel_haswell_d_info), /* CRW GT2 reserved */
> -	INTEL_VGA_DEVICE(0x0D2B, &intel_haswell_d_info), /* CRW GT3 reserved */
> -	INTEL_VGA_DEVICE(0x0D0E, &intel_haswell_d_info), /* CRW GT1 reserved */
> -	INTEL_VGA_DEVICE(0x0D1E, &intel_haswell_d_info), /* CRW GT2 reserved */
> -	INTEL_VGA_DEVICE(0x0D2E, &intel_haswell_d_info), /* CRW GT3 reserved */
> -	INTEL_VGA_DEVICE(0x0f30, &intel_valleyview_m_info),
> -	INTEL_VGA_DEVICE(0x0f31, &intel_valleyview_m_info),
> -	INTEL_VGA_DEVICE(0x0f32, &intel_valleyview_m_info),
> -	INTEL_VGA_DEVICE(0x0f33, &intel_valleyview_m_info),
> -	INTEL_VGA_DEVICE(0x0157, &intel_valleyview_m_info),
> -	INTEL_VGA_DEVICE(0x0155, &intel_valleyview_d_info),
> +	INTEL_PCI_IDS,
>  	{0, 0, 0}
>  };
>  
> diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> index 63d609d..7276a72 100644
> --- a/include/drm/i915_drm.h
> +++ b/include/drm/i915_drm.h
> @@ -26,6 +26,7 @@
>  #ifndef _I915_DRM_H_
>  #define _I915_DRM_H_
>  
> +#include <drm/i915_pciids.h>
>  #include <uapi/drm/i915_drm.h>
>  
>  /* For use by IPS driver */
> @@ -34,4 +35,5 @@ extern bool i915_gpu_raise(void);
>  extern bool i915_gpu_lower(void);
>  extern bool i915_gpu_busy(void);
>  extern bool i915_gpu_turbo_disable(void);
> +
>  #endif				/* _I915_DRM_H_ */
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> new file mode 100644
> index 0000000..8a10f5c
> --- /dev/null
> +++ b/include/drm/i915_pciids.h
> @@ -0,0 +1,211 @@
> +/*
> + * Copyright 2013 Intel Corporation
> + * All Rights Reserved.
> + *
> + * 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, sub license, 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.
> + */
> +#ifndef _I915_PCIIDS_H
> +#define _I915_PCIIDS_H
> +
> +/*
> + * A pci_device_id struct {
> + *	__u32 vendor, device;
> + *      __u32 subvendor, subdevice;
> + *	__u32 class, class_mask;
> + *	kernel_ulong_t driver_data;
> + * };
> + * Don't use C99 here because "class" is reserved and we want to
> + * give userspace flexibility.
> + */
> +#define INTEL_VGA_DEVICE(id, info) {		\
> +	0x8086,	id,				\
> +	~0, ~0,					\
> +	0x030000, 0xff0000,			\
> +	(unsigned long) info }
> +
> +#define INTEL_QUANTA_VGA_DEVICE(info) {		\
> +	0x8086,	0x16a,				\
> +	0x152d,	0x8990,				\
> +	0x030000, 0xff0000,			\
> +	(unsigned long) info }
> +
> +#define INTEL_I830_IDS(info)				\
> +	INTEL_VGA_DEVICE(0x3577, info)
> +
> +#define INTEL_I845G_IDS(info)				\
> +	INTEL_VGA_DEVICE(0x2562, info)
> +
> +#define INTEL_I85X_IDS(info)				\
> +	INTEL_VGA_DEVICE(0x3582, info), /* I855_GM */ \
> +	INTEL_VGA_DEVICE(0x358e, info)
> +
> +#define INTEL_I865G_IDS(info)				\
> +	INTEL_VGA_DEVICE(0x2572, info) /* I865_G */
> +
> +#define INTEL_I915G_IDS(info)				\
> +	INTEL_VGA_DEVICE(0x2582, info), /* I915_G */ \
> +	INTEL_VGA_DEVICE(0x258a, info)  /* E7221_G */
> +
> +#define INTEL_I915GM_IDS(info)				\
> +	INTEL_VGA_DEVICE(0x2592, info) /* I915_GM */
> +
> +#define INTEL_I945G_IDS(info)				\
> +	INTEL_VGA_DEVICE(0x2772, info) /* I945_G */
> +
> +#define INTEL_I945GM_IDS(info)				\
> +	INTEL_VGA_DEVICE(0x27a2, info), /* I945_GM */ \
> +	INTEL_VGA_DEVICE(0x27ae, info)  /* I945_GME */
> +
> +#define INTEL_I965G_IDS(info)				\
> +	INTEL_VGA_DEVICE(0x2972, info), /* I946_GZ */	\
> +	INTEL_VGA_DEVICE(0x2982, info),	/* G35_G */	\
> +	INTEL_VGA_DEVICE(0x2992, info),	/* I965_Q */	\
> +	INTEL_VGA_DEVICE(0x29a2, info)	/* I965_G */
> +
> +#define INTEL_G33_IDS(info)				\
> +	INTEL_VGA_DEVICE(0x29b2, info), /* Q35_G */ \
> +	INTEL_VGA_DEVICE(0x29c2, info),	/* G33_G */ \
> +	INTEL_VGA_DEVICE(0x29d2, info)	/* Q33_G */
> +
> +#define INTEL_I965GM_IDS(info)				\
> +	INTEL_VGA_DEVICE(0x2a02, info),	/* I965_GM */ \
> +	INTEL_VGA_DEVICE(0x2a12, info)  /* I965_GME */
> +
> +#define INTEL_GM45_IDS(info)				\
> +	INTEL_VGA_DEVICE(0x2a42, info) /* GM45_G */
> +
> +#define INTEL_G45_IDS(info)				\
> +	INTEL_VGA_DEVICE(0x2e02, info), /* IGD_E_G */ \
> +	INTEL_VGA_DEVICE(0x2e12, info), /* Q45_G */ \
> +	INTEL_VGA_DEVICE(0x2e22, info), /* G45_G */ \
> +	INTEL_VGA_DEVICE(0x2e32, info), /* G41_G */ \
> +	INTEL_VGA_DEVICE(0x2e42, info), /* B43_G */ \
> +	INTEL_VGA_DEVICE(0x2e92, info)	/* B43_G.1 */
> +
> +#define INTEL_PINEVIEW_IDS(info)			\
> +	INTEL_VGA_DEVICE(0xa001, info),			\
> +	INTEL_VGA_DEVICE(0xa011, info)
> +
> +#define INTEL_IRONLAKE_D_IDS(info) \
> +	INTEL_VGA_DEVICE(0x0042, info)
> +
> +#define INTEL_IRONLAKE_M_IDS(info) \
> +	INTEL_VGA_DEVICE(0x0046, info)
> +
> +#define INTEL_SNB_D_IDS(info) \
> +	INTEL_VGA_DEVICE(0x0102, info), \
> +	INTEL_VGA_DEVICE(0x0112, info), \
> +	INTEL_VGA_DEVICE(0x0122, info), \
> +	INTEL_VGA_DEVICE(0x010A, info)
> +
> +#define INTEL_SNB_M_IDS(info) \
> +	INTEL_VGA_DEVICE(0x0106, info), \
> +	INTEL_VGA_DEVICE(0x0116, info), \
> +	INTEL_VGA_DEVICE(0x0126, info)
> +
> +#define INTEL_IVB_M_IDS(info) \
> +	INTEL_VGA_DEVICE(0x0156, info), /* GT1 mobile */ \
> +	INTEL_VGA_DEVICE(0x0166, info)  /* GT2 mobile */
> +
> +#define INTEL_IVB_D_IDS(info) \
> +	INTEL_VGA_DEVICE(0x0152, info), /* GT1 desktop */ \
> +	INTEL_VGA_DEVICE(0x0162, info), /* GT2 desktop */ \
> +	INTEL_VGA_DEVICE(0x015a, info), /* GT1 server */ \
> +	INTEL_VGA_DEVICE(0x016a, info)  /* GT2 server */
> +
> +#define INTEL_IVB_Q_IDS(info) \
> +	INTEL_QUANTA_VGA_DEVICE(info) /* Quanta transcode */
> +
> +#define INTEL_HSW_D_IDS(info) \
> +	INTEL_VGA_DEVICE(0x0402, info), /* GT1 desktop */ \
> +	INTEL_VGA_DEVICE(0x0412, info), /* GT2 desktop */ \
> +	INTEL_VGA_DEVICE(0x0422, info), /* GT3 desktop */ \
> +	INTEL_VGA_DEVICE(0x040a, info), /* GT1 server */ \
> +	INTEL_VGA_DEVICE(0x041a, info), /* GT2 server */ \
> +	INTEL_VGA_DEVICE(0x042a, info), /* GT3 server */ \
> +	INTEL_VGA_DEVICE(0x040B, info), /* GT1 reserved */ \
> +	INTEL_VGA_DEVICE(0x041B, info), /* GT2 reserved */ \
> +	INTEL_VGA_DEVICE(0x042B, info), /* GT3 reserved */ \
> +	INTEL_VGA_DEVICE(0x040E, info), /* GT1 reserved */ \
> +	INTEL_VGA_DEVICE(0x041E, info), /* GT2 reserved */ \
> +	INTEL_VGA_DEVICE(0x042E, info), /* GT3 reserved */ \
> +	INTEL_VGA_DEVICE(0x0C02, info), /* SDV GT1 desktop */ \
> +	INTEL_VGA_DEVICE(0x0C12, info), /* SDV GT2 desktop */ \
> +	INTEL_VGA_DEVICE(0x0C22, info), /* SDV GT3 desktop */ \
> +	INTEL_VGA_DEVICE(0x0C0A, info), /* SDV GT1 server */ \
> +	INTEL_VGA_DEVICE(0x0C1A, info), /* SDV GT2 server */ \
> +	INTEL_VGA_DEVICE(0x0C2A, info), /* SDV GT3 server */ \
> +	INTEL_VGA_DEVICE(0x0C0B, info), /* SDV GT1 reserved */ \
> +	INTEL_VGA_DEVICE(0x0C1B, info), /* SDV GT2 reserved */ \
> +	INTEL_VGA_DEVICE(0x0C2B, info), /* SDV GT3 reserved */ \
> +	INTEL_VGA_DEVICE(0x0C0E, info), /* SDV GT1 reserved */ \
> +	INTEL_VGA_DEVICE(0x0C1E, info), /* SDV GT2 reserved */ \
> +	INTEL_VGA_DEVICE(0x0C2E, info), /* SDV GT3 reserved */ \
> +	INTEL_VGA_DEVICE(0x0A02, info), /* ULT GT1 desktop */ \
> +	INTEL_VGA_DEVICE(0x0A12, info), /* ULT GT2 desktop */ \
> +	INTEL_VGA_DEVICE(0x0A22, info), /* ULT GT3 desktop */ \
> +	INTEL_VGA_DEVICE(0x0A0A, info), /* ULT GT1 server */ \
> +	INTEL_VGA_DEVICE(0x0A1A, info), /* ULT GT2 server */ \
> +	INTEL_VGA_DEVICE(0x0A2A, info), /* ULT GT3 server */ \
> +	INTEL_VGA_DEVICE(0x0A0B, info), /* ULT GT1 reserved */ \
> +	INTEL_VGA_DEVICE(0x0A1B, info), /* ULT GT2 reserved */ \
> +	INTEL_VGA_DEVICE(0x0A2B, info), /* ULT GT3 reserved */ \
> +	INTEL_VGA_DEVICE(0x0D02, info), /* CRW GT1 desktop */ \
> +	INTEL_VGA_DEVICE(0x0D12, info), /* CRW GT2 desktop */ \
> +	INTEL_VGA_DEVICE(0x0D22, info), /* CRW GT3 desktop */ \
> +	INTEL_VGA_DEVICE(0x0D0A, info), /* CRW GT1 server */ \
> +	INTEL_VGA_DEVICE(0x0D1A, info), /* CRW GT2 server */ \
> +	INTEL_VGA_DEVICE(0x0D2A, info), /* CRW GT3 server */ \
> +	INTEL_VGA_DEVICE(0x0D0B, info), /* CRW GT1 reserved */ \
> +	INTEL_VGA_DEVICE(0x0D1B, info), /* CRW GT2 reserved */ \
> +	INTEL_VGA_DEVICE(0x0D2B, info), /* CRW GT3 reserved */ \
> +	INTEL_VGA_DEVICE(0x0D0E, info), /* CRW GT1 reserved */ \
> +	INTEL_VGA_DEVICE(0x0D1E, info), /* CRW GT2 reserved */ \
> +	INTEL_VGA_DEVICE(0x0D2E, info)  /* CRW GT3 reserved */ \
> +
> +#define INTEL_HSW_M_IDS(info) \
> +	INTEL_VGA_DEVICE(0x0406, info), /* GT1 mobile */ \
> +	INTEL_VGA_DEVICE(0x0416, info), /* GT2 mobile */ \
> +	INTEL_VGA_DEVICE(0x0426, info), /* GT2 mobile */ \
> +	INTEL_VGA_DEVICE(0x0C06, info), /* SDV GT1 mobile */ \
> +	INTEL_VGA_DEVICE(0x0C16, info), /* SDV GT2 mobile */ \
> +	INTEL_VGA_DEVICE(0x0C26, info), /* SDV GT3 mobile */ \
> +	INTEL_VGA_DEVICE(0x0A06, info), /* ULT GT1 mobile */ \
> +	INTEL_VGA_DEVICE(0x0A16, info), /* ULT GT2 mobile */ \
> +	INTEL_VGA_DEVICE(0x0A26, info), /* ULT GT3 mobile */ \
> +	INTEL_VGA_DEVICE(0x0A0E, info), /* ULT GT1 reserved */ \
> +	INTEL_VGA_DEVICE(0x0A1E, info), /* ULT GT2 reserved */ \
> +	INTEL_VGA_DEVICE(0x0A2E, info), /* ULT GT3 reserved */ \
> +	INTEL_VGA_DEVICE(0x0D06, info), /* CRW GT1 mobile */ \
> +	INTEL_VGA_DEVICE(0x0D16, info), /* CRW GT2 mobile */ \
> +	INTEL_VGA_DEVICE(0x0D26, info)  /* CRW GT3 mobile */
> +
> +#define INTEL_VLV_M_IDS(info) \
> +	INTEL_VGA_DEVICE(0x0f30, info), \
> +	INTEL_VGA_DEVICE(0x0f31, info), \
> +	INTEL_VGA_DEVICE(0x0f32, info), \
> +	INTEL_VGA_DEVICE(0x0f33, info), \
> +	INTEL_VGA_DEVICE(0x0157, info)
> +
> +#define INTEL_VLV_D_IDS(info) \
> +	INTEL_VGA_DEVICE(0x0155, info)
> +
> +#endif /* _I915_PCIIDS_H */
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 08/17] drm/i915: Always prefer CPU relocations with LLC
  2013-08-29 17:20   ` Ben Widawsky
@ 2013-08-29 19:11     ` Daniel Vetter
  2013-08-29 19:16       ` Ben Widawsky
  0 siblings, 1 reply; 40+ messages in thread
From: Daniel Vetter @ 2013-08-29 19:11 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: intel-gfx

On Thu, Aug 29, 2013 at 10:20:06AM -0700, Ben Widawsky wrote:
> On Mon, Aug 26, 2013 at 07:51:00PM -0300, Rodrigo Vivi wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > A follow-on to the update of the LLC coherency logic is that we can rely
> > on the LLC being coherent with the CS for rewriting batchbuffers
> > irrespective of their cache domain. (This should have no effect
> > currently as all the batch buffers are expected to be I915_CACHE_LLC and
> > so using the cpu relocation path anyway.)
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index 792c52a..3b64b9f 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -166,7 +166,8 @@ eb_destroy(struct eb_objects *eb)
> >  
> >  static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
> >  {
> > -	return (obj->base.write_domain == I915_GEM_DOMAIN_CPU ||
> > +	return (HAS_LLC(obj->base.dev) ||
> > +		obj->base.write_domain == I915_GEM_DOMAIN_CPU ||
> >  		!obj->map_and_fenceable ||
> >  		obj->cache_level != I915_CACHE_NONE);
> 
> Assuming the commit message is factually correct... the obj->cache_level
> shouldn't factor into the equation at all.

We stil need to take the cache level into account on non-llc machines ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 08/17] drm/i915: Always prefer CPU relocations with LLC
  2013-08-29 19:11     ` Daniel Vetter
@ 2013-08-29 19:16       ` Ben Widawsky
  0 siblings, 0 replies; 40+ messages in thread
From: Ben Widawsky @ 2013-08-29 19:16 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Thu, Aug 29, 2013 at 09:11:16PM +0200, Daniel Vetter wrote:
> On Thu, Aug 29, 2013 at 10:20:06AM -0700, Ben Widawsky wrote:
> > On Mon, Aug 26, 2013 at 07:51:00PM -0300, Rodrigo Vivi wrote:
> > > From: Chris Wilson <chris@chris-wilson.co.uk>
> > > 
> > > A follow-on to the update of the LLC coherency logic is that we can rely
> > > on the LLC being coherent with the CS for rewriting batchbuffers
> > > irrespective of their cache domain. (This should have no effect
> > > currently as all the batch buffers are expected to be I915_CACHE_LLC and
> > > so using the cpu relocation path anyway.)
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > index 792c52a..3b64b9f 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > @@ -166,7 +166,8 @@ eb_destroy(struct eb_objects *eb)
> > >  
> > >  static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
> > >  {
> > > -	return (obj->base.write_domain == I915_GEM_DOMAIN_CPU ||
> > > +	return (HAS_LLC(obj->base.dev) ||
> > > +		obj->base.write_domain == I915_GEM_DOMAIN_CPU ||
> > >  		!obj->map_and_fenceable ||
> > >  		obj->cache_level != I915_CACHE_NONE);
> > 
> > Assuming the commit message is factually correct... the obj->cache_level
> > shouldn't factor into the equation at all.
> 
> We stil need to take the cache level into account on non-llc machines ...
> -Daniel

I always forget I915_CACHE_LLC is overloaded to mean snoopable.

-- 
Ben Widawsky, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/17] drm/i915: Do not add an interrupt for a context switch
  2013-08-26 22:50 ` [PATCH 01/17] drm/i915: Do not add an interrupt for a context switch Rodrigo Vivi
@ 2013-08-30 14:17   ` Damien Lespiau
  0 siblings, 0 replies; 40+ messages in thread
From: Damien Lespiau @ 2013-08-30 14:17 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Ben Widawsky, Paulo Zanoni

On Mon, Aug 26, 2013 at 07:50:53PM -0300, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> We use the request to ensure we hold a reference to the context for the
> duration that it remains in use by the ring. Each request only holds a
> reference to the current context, hence we emit a request after
> switching contexts with the final reference to the old context. However,
> the extra interrupt caused by that request is not useful (no timing
> critical function will wait for the context object), instead the overhead
> of servicing the IRQ shows up in some (lightweight) benchmarks. In order
> to keep the useful property of using the request to manage the context
> lifetime, we want to add a dummy request that is associated with the
> interrupt from the subsequent real request following the batch.
> 
> The extra interrupt was added as a side-effect of using
> i915_add_request() in
> 
> commit 112522f6789581824903f6f72082b5b841a7f0f9
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Thu May 2 16:48:07 2013 +0300
> 
>     drm/i915: put context upon switching
> 
> v2: Daniel convinced me that the request here was solely for context
> lifetime tracking and that we have the active ref to keep the object
> alive whilst the MI_SET_CONTEXT. So the only concern then is which
> context should get the blame for MI_SET_CONTEXT failing. The old scheme
> added a request for the old context so that any hang upto and including
> the switch away would mark the old context as guilty. Now any hang here
> implicates the new context. However since we have already gone through a
> complete flush with the last context in its last request, and all that
> lies in no-man's-land is an invalidate flush and the MI_SET_CONTEXT, we
> should be safe in not unduly placing blame on the new context.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>

I'm somewhat new to the core GEM code, but I'm convinced by the commit
message and the patch does what it says. So:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_gem.c         |  1 -
>  drivers/gpu/drm/i915/i915_gem_context.c | 12 +-----------
>  2 files changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 2d1cb10..56c9104 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2046,7 +2046,6 @@ int __i915_add_request(struct intel_ring_buffer *ring,
>  	if (request == NULL)
>  		return -ENOMEM;
>  
> -
>  	/* Record the position of the start of the request so that
>  	 * should we detect the updated seqno part-way through the
>  	 * GPU processing the request, we never over-estimate the
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 403309c..b6da70b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -451,17 +451,7 @@ static int do_switch(struct i915_hw_context *to)
>  		from->obj->dirty = 1;
>  		BUG_ON(from->obj->ring != ring);
>  
> -		ret = i915_add_request(ring, NULL);
> -		if (ret) {
> -			/* Too late, we've already scheduled a context switch.
> -			 * Try to undo the change so that the hw state is
> -			 * consistent with out tracking. In case of emergency,
> -			 * scream.
> -			 */
> -			WARN_ON(mi_set_context(ring, from, MI_RESTORE_INHIBIT));
> -			return ret;
> -		}
> -
> +		/* obj is kept alive until the next request by its active ref */
>  		i915_gem_object_unpin(from->obj);
>  		i915_gem_context_unreference(from);
>  	}
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 02/17] drm/i915: Rearrange the comments in i915_add_request()
  2013-08-26 22:50 ` [PATCH 02/17] drm/i915: Rearrange the comments in i915_add_request() Rodrigo Vivi
@ 2013-08-30 14:21   ` Damien Lespiau
  0 siblings, 0 replies; 40+ messages in thread
From: Damien Lespiau @ 2013-08-30 14:21 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Mon, Aug 26, 2013 at 07:50:54PM -0300, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> The comments were a little out-of-sequence with the code, forcing the
> reader to jump around whilst reading. Whilst moving the comments around,
> add one to explain the context reference.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 56c9104..d5c8a02 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2063,8 +2063,6 @@ int __i915_add_request(struct intel_ring_buffer *ring,
>  	request->ring = ring;
>  	request->head = request_start;
>  	request->tail = request_ring_position;
> -	request->ctx = ring->last_context;
> -	request->batch_obj = obj;
>  
>  	/* Whilst this request exists, batch_obj will be on the
>  	 * active_list, and so will hold the active reference. Only when this
> @@ -2072,7 +2070,12 @@ int __i915_add_request(struct intel_ring_buffer *ring,
>  	 * inactive_list and lose its active reference. Hence we do not need
>  	 * to explicitly hold another reference here.
>  	 */
> +	request->batch_obj = obj;
>  
> +	/* Hold a reference to the current context so that we can inspect
> +	 * it later in case a hangcheck error event fires.
> +	 */
> +	request->ctx = ring->last_context;
>  	if (request->ctx)
>  		i915_gem_context_reference(request->ctx);
>  
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 03/17] drm/i915: Pin pages whilst mapping the dma-buf
  2013-08-26 22:50 ` [PATCH 03/17] drm/i915: Pin pages whilst mapping the dma-buf Rodrigo Vivi
@ 2013-08-30 14:27   ` Damien Lespiau
  2013-09-02  6:04     ` Daniel Vetter
  0 siblings, 1 reply; 40+ messages in thread
From: Damien Lespiau @ 2013-08-30 14:27 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Mon, Aug 26, 2013 at 07:50:55PM -0300, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> As we attempt to kmalloc after calling get_pages, there is a possibility
> that the shrinker may reap the pages we just acquired. To prevent this
> we need to increment the pages_pin_count early, so rearrange the code
> and error paths to make it so.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c | 41 ++++++++++++++++++----------------
>  1 file changed, 22 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> index e918b05..7d5752f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -42,27 +42,24 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme
>  
>  	ret = i915_mutex_lock_interruptible(obj->base.dev);
>  	if (ret)
> -		return ERR_PTR(ret);
> +		goto err;
>  
>  	ret = i915_gem_object_get_pages(obj);
> -	if (ret) {
> -		st = ERR_PTR(ret);
> -		goto out;
> -	}
> +	if (ret)
> +		goto err_unlock;
> +
> +	i915_gem_object_pin_pages(obj);
>  
>  	/* Copy sg so that we make an independent mapping */
>  	st = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
>  	if (st == NULL) {
> -		st = ERR_PTR(-ENOMEM);
> -		goto out;
> +		ret = -ENOMEM;
> +		goto err_unpin;
>  	}
>  
>  	ret = sg_alloc_table(st, obj->pages->nents, GFP_KERNEL);
> -	if (ret) {
> -		kfree(st);
> -		st = ERR_PTR(ret);
> -		goto out;
> -	}
> +	if (ret)
> +		goto err_free;
>  
>  	src = obj->pages->sgl;
>  	dst = st->sgl;
> @@ -73,17 +70,23 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme
>  	}
>  
>  	if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir)) {
> -		sg_free_table(st);
> -		kfree(st);
> -		st = ERR_PTR(-ENOMEM);
> -		goto out;
> +		ret =-ENOMEM;
> +		goto err_free_sg;
>  	}
>  
> -	i915_gem_object_pin_pages(obj);
> -
> -out:
>  	mutex_unlock(&obj->base.dev->struct_mutex);
>  	return st;
> +
> +err_free_sg:
> +	sg_free_table(st);
> +err_free:
> +	kfree(st);
> +err_unpin:
> +	i915_gem_object_unpin_pages(obj);
> +err_unlock:
> +	mutex_unlock(&obj->base.dev->struct_mutex);
> +err:
> +	return ERR_PTR(ret);
>  }
>  
>  static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 03/17] drm/i915: Pin pages whilst mapping the dma-buf
  2013-08-30 14:27   ` Damien Lespiau
@ 2013-09-02  6:04     ` Daniel Vetter
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Vetter @ 2013-09-02  6:04 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On Fri, Aug 30, 2013 at 03:27:47PM +0100, Damien Lespiau wrote:
> On Mon, Aug 26, 2013 at 07:50:55PM -0300, Rodrigo Vivi wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > As we attempt to kmalloc after calling get_pages, there is a possibility
> > that the shrinker may reap the pages we just acquired. To prevent this
> > we need to increment the pages_pin_count early, so rearrange the code
> > and error paths to make it so.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

I've merged the three reviewed patches to dinq, thanks.
-Daniel
> 
> -- 
> Damien
> 
> > ---
> >  drivers/gpu/drm/i915/i915_gem_dmabuf.c | 41 ++++++++++++++++++----------------
> >  1 file changed, 22 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> > index e918b05..7d5752f 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> > @@ -42,27 +42,24 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme
> >  
> >  	ret = i915_mutex_lock_interruptible(obj->base.dev);
> >  	if (ret)
> > -		return ERR_PTR(ret);
> > +		goto err;
> >  
> >  	ret = i915_gem_object_get_pages(obj);
> > -	if (ret) {
> > -		st = ERR_PTR(ret);
> > -		goto out;
> > -	}
> > +	if (ret)
> > +		goto err_unlock;
> > +
> > +	i915_gem_object_pin_pages(obj);
> >  
> >  	/* Copy sg so that we make an independent mapping */
> >  	st = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
> >  	if (st == NULL) {
> > -		st = ERR_PTR(-ENOMEM);
> > -		goto out;
> > +		ret = -ENOMEM;
> > +		goto err_unpin;
> >  	}
> >  
> >  	ret = sg_alloc_table(st, obj->pages->nents, GFP_KERNEL);
> > -	if (ret) {
> > -		kfree(st);
> > -		st = ERR_PTR(ret);
> > -		goto out;
> > -	}
> > +	if (ret)
> > +		goto err_free;
> >  
> >  	src = obj->pages->sgl;
> >  	dst = st->sgl;
> > @@ -73,17 +70,23 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme
> >  	}
> >  
> >  	if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir)) {
> > -		sg_free_table(st);
> > -		kfree(st);
> > -		st = ERR_PTR(-ENOMEM);
> > -		goto out;
> > +		ret =-ENOMEM;
> > +		goto err_free_sg;
> >  	}
> >  
> > -	i915_gem_object_pin_pages(obj);
> > -
> > -out:
> >  	mutex_unlock(&obj->base.dev->struct_mutex);
> >  	return st;
> > +
> > +err_free_sg:
> > +	sg_free_table(st);
> > +err_free:
> > +	kfree(st);
> > +err_unpin:
> > +	i915_gem_object_unpin_pages(obj);
> > +err_unlock:
> > +	mutex_unlock(&obj->base.dev->struct_mutex);
> > +err:
> > +	return ERR_PTR(ret);
> >  }
> >  
> >  static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
> > -- 
> > 1.8.1.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2013-09-02  6:03 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-26 22:50 [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Rodrigo Vivi
2013-08-26 22:50 ` [PATCH 01/17] drm/i915: Do not add an interrupt for a context switch Rodrigo Vivi
2013-08-30 14:17   ` Damien Lespiau
2013-08-26 22:50 ` [PATCH 02/17] drm/i915: Rearrange the comments in i915_add_request() Rodrigo Vivi
2013-08-30 14:21   ` Damien Lespiau
2013-08-26 22:50 ` [PATCH 03/17] drm/i915: Pin pages whilst mapping the dma-buf Rodrigo Vivi
2013-08-30 14:27   ` Damien Lespiau
2013-09-02  6:04     ` Daniel Vetter
2013-08-26 22:50 ` [PATCH 04/17] drm/i915: check that the i965g/gm 4G limit is really obeyed Rodrigo Vivi
2013-08-26 22:50 ` [PATCH 05/17] drm/i915: Cancel outstanding modeset workers before suspend Rodrigo Vivi
2013-08-26 22:50 ` [PATCH 06/17] drm/i915: split PCI IDs out into i915_drm.h v4 Rodrigo Vivi
2013-08-29 17:27   ` Ben Widawsky
2013-08-26 22:50 ` [PATCH 07/17] x86: add early quirk for reserving Intel graphics stolen memory v5 Rodrigo Vivi
2013-08-26 22:51 ` [PATCH 08/17] drm/i915: Always prefer CPU relocations with LLC Rodrigo Vivi
2013-08-27 14:49   ` Ville Syrjälä
2013-08-27 15:25     ` Daniel Vetter
2013-08-29 17:20   ` Ben Widawsky
2013-08-29 19:11     ` Daniel Vetter
2013-08-29 19:16       ` Ben Widawsky
2013-08-26 22:51 ` [PATCH 09/17] drm/i915: Report requested frequency alongside current frequency in debugfs Rodrigo Vivi
2013-08-27 12:12   ` Rodrigo Vivi
2013-08-27 12:36     ` Chris Wilson
2013-08-28  8:15     ` Daniel Vetter
2013-08-26 22:51 ` [PATCH 10/17] drm/i915: Move the conditional seqno query into the tracepoint Rodrigo Vivi
2013-08-26 22:51 ` [PATCH 11/17] drm/i915: Add some missing steps to i915_driver_load error path Rodrigo Vivi
2013-08-26 22:51 ` [PATCH 12/17] drm/i915: Asynchronously perform the set-base for a simple modeset Rodrigo Vivi
2013-08-26 22:51 ` [PATCH 13/17] drm/i915: Align tiled scanouts from stolen memory to 256k in the GTT Rodrigo Vivi
2013-08-26 22:51 ` [PATCH 14/17] drm/i915: Apply the force-detect VGA w/a to Valleyview Rodrigo Vivi
2013-08-27  9:27   ` Daniel Vetter
2013-08-26 22:51 ` [PATCH 15/17] drm/i915: Pair seqno completion tracepoint with its dispatch Rodrigo Vivi
2013-08-26 22:51 ` [PATCH 16/17] RFM drm/i915: Boost RPS frequency for CPU stalls Rodrigo Vivi
2013-08-26 22:56   ` Chris Wilson
2013-08-26 22:51 ` [PATCH 17/17] drm/i915: Enable Lower Slice on Haswell GT3 Rodrigo Vivi
2013-08-27 15:31   ` Rodrigo Vivi
2013-08-27 17:05     ` Rodrigo Vivi
2013-08-27  9:39 ` [PATCH 00/17] drm-intel-collector WW34 - Simple patches as series for review Daniel Vetter
2013-08-27 10:23   ` Chris Wilson
2013-08-27 12:48   ` Rodrigo Vivi
2013-08-27 16:19   ` Chris Wilson
2013-08-27 17:04     ` Rodrigo Vivi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.