[PATCH 0/9] drm-intel-collector - update

[PATCH 0/9] drm-intel-collector - update

[Rodrigo Vivi]

This is another drm-intel-collector updated notice:
http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector

Here goes the update list in order for better reviewers assignment:

Patch     drm/i915: Propagate errors on failed PPGTT - Reviewer:
Patch     drm/i915: wrap crtc enable/disable - Reviewer:
Patch     drm/i915: make crtc enable/disable asynchronous - Reviewer:
Patch     drm/i915: Propagate PCI read/write errors during vga_set_state() - Reviewer:
Patch     drm/i915: Short-circuit no-op vga_set_state() - Reviewer:
Patch     drm/i915: Verify address field of PCBR register. - Reviewer:
Patch     drm/i915: Bring UP Power Wells before disabling RC6. - Reviewer:
Patch     drm/i915: Flush GPU rendering with a lockless wait during a pagefault - Reviewer:
Patch     drm/i915: PF CRC may not work on HSW - Reviewer:

Overall Process:

drm-intel-collector - review request
 1. Daniel pushs drm-intel-testing (every 2 weeks)
 2. I rebase drm-intel-collector onto drm-intel-testing
 3. Add Reviewer: tag with voluntered reviewers. If you don't believe you should be assigned on a particular patch please don't get mad just tell you wont review or volunteer someone else.
 4. I resubmit remaining patches for review without picking any new (drm-intel-collector - review request)

drm-intel-collector - updated
 5. One week later I collect all simple (1-2) patches that wasn't yet reviewed and not queued by Daniel from one testing update until another.
 6. Request automated QA's PRTS automated i-g-t tests comparing drm-intel-testing x drm-intel-collector
 7. If tests are ok I send the update notification or the patches as a series to intel-gfx mailing list for better tracking and to be reviewed. (drm-intel-collector - updated)
 8. Let me know volunteers for review new patches and also let me know if I've picked any patch that I shouldn't.

There are some reasons that some patches can be left behind:
1. Your patch didn't applied cleanly and I couldn't easily solve the conflicts.
2. Kernel didn't compiled with your patch.
3. 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.


Ben Widawsky (1):
  drm/i915: Propagate errors on failed PPGTT

Chris Wilson (3):
  drm/i915: Propagate PCI read/write errors during vga_set_state()
  drm/i915: Short-circuit no-op vga_set_state()
  drm/i915: Flush GPU rendering with a lockless wait during a pagefault

Deepak S (2):
  drm/i915: Verify address field of PCBR register.
  drm/i915: Bring UP Power Wells before disabling RC6.

Jesse Barnes (2):
  drm/i915: wrap crtc enable/disable
  drm/i915: make crtc enable/disable asynchronous

Ville Syrjälä (1):
  drm/i915: PF CRC may not work on HSW

 drivers/gpu/drm/i915/i915_debugfs.c  |  29 ++++++++-
 drivers/gpu/drm/i915/i915_drv.c      |   2 +-
 drivers/gpu/drm/i915/i915_drv.h      |   4 +-
 drivers/gpu/drm/i915/i915_gem.c      |  17 ++++-
 drivers/gpu/drm/i915/i915_gem_gtt.c  |  20 ++++--
 drivers/gpu/drm/i915/i915_irq.c      |  10 ++-
 drivers/gpu/drm/i915/i915_reg.h      |   1 +
 drivers/gpu/drm/i915/intel_display.c | 116 +++++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h     |   5 ++
 drivers/gpu/drm/i915/intel_pm.c      |  15 ++++-
 10 files changed, 182 insertions(+), 37 deletions(-)

-- 
1.8.3.1

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

[PATCH 1/9] drm/i915: Propagate errors on failed PPGTT

[Rodrigo Vivi]

From: Ben Widawsky <benjamin.widawsky@intel.com>

Clean up the return values/error handling so it's obvious what is going
on. This was tripped over in the PPGTT branch where code was added to do
a ret = foo() near the top, and this ended up bypassing some error cases
later.

These errors shouldn't exist with today's code, but a future patch
will add a ret = ..., and this the value of ret needs to be set
explicitly.

v2: Missed an error in alloc_page for gen6

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6e858e1..2996c83 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -362,7 +362,7 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
 static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
 {
 	struct page *pt_pages;
-	int i, j, ret = -ENOMEM;
+	int i, j, ret;
 	const int max_pdp = DIV_ROUND_UP(size, 1 << 30);
 	const int num_pt_pages = GEN8_PDES_PER_PAGE * max_pdp;
 
@@ -408,14 +408,17 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
 		temp = pci_map_page(ppgtt->base.dev->pdev,
 				    &ppgtt->pd_pages[i], 0,
 				    PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
-		if (pci_dma_mapping_error(ppgtt->base.dev->pdev, temp))
+		ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, temp);
+		if (ret)
 			goto err_out;
 
 		ppgtt->pd_dma_addr[i] = temp;
 
 		ppgtt->gen8_pt_dma_addr[i] = kmalloc(sizeof(dma_addr_t) * GEN8_PDES_PER_PAGE, GFP_KERNEL);
-		if (!ppgtt->gen8_pt_dma_addr[i])
+		if (!ppgtt->gen8_pt_dma_addr[i]) {
+			ret = -ENOMEM;
 			goto err_out;
+		}
 
 		for (j = 0; j < GEN8_PDES_PER_PAGE; j++) {
 			struct page *p = &pt_pages[i * GEN8_PDES_PER_PAGE + j];
@@ -423,7 +426,8 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
 					    p, 0, PAGE_SIZE,
 					    PCI_DMA_BIDIRECTIONAL);
 
-			if (pci_dma_mapping_error(ppgtt->base.dev->pdev, temp))
+			ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, temp);
+			if (ret)
 				goto err_out;
 
 			ppgtt->gen8_pt_dma_addr[i][j] = temp;
@@ -907,14 +911,18 @@ alloc:
 
 	for (i = 0; i < ppgtt->num_pd_entries; i++) {
 		ppgtt->pt_pages[i] = alloc_page(GFP_KERNEL);
-		if (!ppgtt->pt_pages[i])
+		if (!ppgtt->pt_pages[i]) {
+			ret = -ENOMEM;
 			goto err_pt_alloc;
+		}
 	}
 
 	ppgtt->pt_dma_addr = kcalloc(ppgtt->num_pd_entries, sizeof(dma_addr_t),
 				     GFP_KERNEL);
-	if (!ppgtt->pt_dma_addr)
+	if (!ppgtt->pt_dma_addr) {
+		ret = -ENOMEM;
 		goto err_pt_alloc;
+	}
 
 	for (i = 0; i < ppgtt->num_pd_entries; i++) {
 		dma_addr_t pt_addr;
-- 
1.8.3.1

[PATCH 2/9] drm/i915: wrap crtc enable/disable

[Rodrigo Vivi]

From: Jesse Barnes <jbarnes@virtuousgeek.org>

This allows us to hide queuing of enable/disable later.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |  2 +-
 drivers/gpu/drm/i915/i915_drv.h      |  4 +-
 drivers/gpu/drm/i915/intel_display.c | 83 +++++++++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 4 files changed, 71 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2d05d7c..2213791 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -461,7 +461,7 @@ static int i915_drm_freeze(struct drm_device *dev)
 		 */
 		mutex_lock(&dev->mode_config.mutex);
 		list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
-			dev_priv->display.crtc_disable(crtc);
+			intel_queue_crtc_disable(crtc);
 		mutex_unlock(&dev->mode_config.mutex);
 
 		intel_modeset_suspend_hw(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9ba8eab..5968859 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -434,8 +434,8 @@ struct drm_i915_display_funcs {
 	int (*crtc_mode_set)(struct drm_crtc *crtc,
 			     int x, int y,
 			     struct drm_framebuffer *old_fb);
-	void (*crtc_enable)(struct drm_crtc *crtc);
-	void (*crtc_disable)(struct drm_crtc *crtc);
+	void (*_crtc_enable)(struct drm_crtc *crtc);
+	void (*_crtc_disable)(struct drm_crtc *crtc);
 	void (*off)(struct drm_crtc *crtc);
 	void (*write_eld)(struct drm_connector *connector,
 			  struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4d4a0d9..21a950d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1742,6 +1742,46 @@ static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
 	I915_WRITE(_TRANSA_CHICKEN2, val);
 }
 
+static void intel_crtc_disable_work(struct work_struct *work)
+{
+	struct intel_crtc *intel_crtc = container_of(work, struct intel_crtc,
+						     disable_work);
+}
+
+void intel_queue_crtc_disable(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	queue_work(dev_priv->wq, &intel_crtc->disable_work);
+}
+
+static void intel_crtc_enable_work(struct work_struct *work)
+{
+	struct intel_crtc *intel_crtc = container_of(work, struct intel_crtc,
+						     enable_work);
+}
+
+static void intel_queue_crtc_enable(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	queue_work(dev_priv->wq, &intel_crtc->enable_work);
+}
+
+static void intel_sync_crtc(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	flush_work(&intel_crtc->disable_work);
+	flush_work(&intel_crtc->enable_work);
+}
+
 /**
  * intel_enable_pipe - enable a pipe, asserting requirements
  * @dev_priv: i915 private structure
@@ -4328,7 +4368,6 @@ static void intel_crtc_update_sarea(struct drm_crtc *crtc,
 void intel_crtc_update_dpms(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_encoder *intel_encoder;
 	bool enable = false;
 
@@ -4336,9 +4375,9 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
 		enable |= intel_encoder->connectors_active;
 
 	if (enable)
-		dev_priv->display.crtc_enable(crtc);
+		intel_queue_crtc_enable(crtc);
 	else
-		dev_priv->display.crtc_disable(crtc);
+		intel_queue_crtc_disable(crtc);
 
 	intel_crtc_update_sarea(crtc, enable);
 }
@@ -4353,7 +4392,7 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
 	/* crtc should still be enabled when we disable it. */
 	WARN_ON(!crtc->enabled);
 
-	dev_priv->display.crtc_disable(crtc);
+	dev_priv->display._crtc_disable(crtc);
 	intel_crtc->eld_vld = false;
 	intel_crtc_update_sarea(crtc, false);
 	dev_priv->display.off(crtc);
@@ -7575,6 +7614,8 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 		goto fail;
 	}
 
+	intel_sync_crtc(crtc);
+
 	/* we only need to pin inside GTT if cursor is non-phy */
 	mutex_lock(&dev->struct_mutex);
 	if (!dev_priv->info->cursor_needs_physical) {
@@ -7660,6 +7701,8 @@ static int intel_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
 	intel_crtc->cursor_x = clamp_t(int, x, SHRT_MIN, SHRT_MAX);
 	intel_crtc->cursor_y = clamp_t(int, y, SHRT_MIN, SHRT_MAX);
 
+	intel_sync_crtc(crtc);
+
 	if (intel_crtc->active)
 		intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
 
@@ -8675,6 +8718,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	if (work == NULL)
 		return -ENOMEM;
 
+	/* Complete any pending CRTC enable/disable */
+	intel_sync_crtc(crtc);
+
 	work->event = event;
 	work->crtc = crtc;
 	work->old_fb_obj = to_intel_framebuffer(old_fb)->obj;
@@ -9670,8 +9716,10 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 		intel_crtc_disable(&intel_crtc->base);
 
 	for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) {
-		if (intel_crtc->base.enabled)
-			dev_priv->display.crtc_disable(&intel_crtc->base);
+		if (intel_crtc->base.enabled) {
+			intel_queue_crtc_disable(&intel_crtc->base);
+			intel_sync_crtc(&intel_crtc->base);
+		}
 	}
 
 	/* crtc->mode is already used by the ->mode_set callbacks, hence we need
@@ -9712,7 +9760,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
 	for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc)
-		dev_priv->display.crtc_enable(&intel_crtc->base);
+		intel_queue_crtc_enable(&intel_crtc->base);
 
 	/* FIXME: add subpixel order */
 done:
@@ -10299,6 +10347,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base;
 
 	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
+
+	INIT_WORK(intel_crtc->enable_work, intel_crtc_enable_work);
+	INIT_WORK(intel_crtc->disable_work, intel_crtc_disable_work);
 }
 
 enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
@@ -10716,29 +10767,29 @@ static void intel_init_display(struct drm_device *dev)
 	if (HAS_DDI(dev)) {
 		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
 		dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
-		dev_priv->display.crtc_enable = haswell_crtc_enable;
-		dev_priv->display.crtc_disable = haswell_crtc_disable;
+		dev_priv->display._crtc_enable = haswell_crtc_enable;
+		dev_priv->display._crtc_disable = haswell_crtc_disable;
 		dev_priv->display.off = haswell_crtc_off;
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
 		dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
-		dev_priv->display.crtc_enable = ironlake_crtc_enable;
-		dev_priv->display.crtc_disable = ironlake_crtc_disable;
+		dev_priv->display._crtc_enable = ironlake_crtc_enable;
+		dev_priv->display._crtc_disable = ironlake_crtc_disable;
 		dev_priv->display.off = ironlake_crtc_off;
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
-		dev_priv->display.crtc_enable = valleyview_crtc_enable;
-		dev_priv->display.crtc_disable = i9xx_crtc_disable;
+		dev_priv->display._crtc_enable = valleyview_crtc_enable;
+		dev_priv->display._crtc_disable = i9xx_crtc_disable;
 		dev_priv->display.off = i9xx_crtc_off;
 		dev_priv->display.update_plane = i9xx_update_plane;
 	} else {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
 		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
-		dev_priv->display.crtc_enable = i9xx_crtc_enable;
-		dev_priv->display.crtc_disable = i9xx_crtc_disable;
+		dev_priv->display._crtc_enable = i9xx_crtc_enable;
+		dev_priv->display._crtc_disable = i9xx_crtc_disable;
 		dev_priv->display.off = i9xx_crtc_off;
 		dev_priv->display.update_plane = i9xx_update_plane;
 	}
@@ -11138,7 +11189,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 		 * ...  */
 		plane = crtc->plane;
 		crtc->plane = !plane;
-		dev_priv->display.crtc_disable(&crtc->base);
+		intel_queue_crtc_disable(&crtc->base);
 		crtc->plane = plane;
 
 		/* ... and break all links. */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 44067bc..5b5b51e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -727,6 +727,7 @@ void hsw_enable_ips(struct intel_crtc *crtc);
 void hsw_disable_ips(struct intel_crtc *crtc);
 void intel_display_set_init_power(struct drm_device *dev, bool enable);
 int valleyview_get_vco(struct drm_i915_private *dev_priv);
+void intel_queue_crtc_disable(struct drm_crtc *crtc);
 
 /* intel_dp.c */
 void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
-- 
1.8.3.1

[PATCH 3/9] drm/i915: make crtc enable/disable asynchronous

[Rodrigo Vivi]

From: Jesse Barnes <jbarnes@virtuousgeek.org>

The intent is to get back to userspace as quickly as possible so it can
start doing drawing or whatever.  It should also allow our
suspend/resume/init time to improve a lot.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_irq.c      | 10 +++++++++-
 drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |  4 ++++
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index e9c94c9..749c20f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -917,8 +917,16 @@ static void i915_hotplug_work_func(struct work_struct *work)
 		intel_connector = to_intel_connector(connector);
 		intel_encoder = intel_connector->encoder;
 		if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
-			if (intel_encoder->hot_plug)
+			if (intel_encoder->hot_plug) {
+				struct drm_crtc *crtc =
+					intel_encoder->base.crtc;
+				if (crtc) {
+					mutex_lock(&crtc->mutex);
+					intel_sync_crtc(intel_encoder->base.crtc);
+					mutex_unlock(&crtc->mutex);
+				}
 				intel_encoder->hot_plug(intel_encoder);
+			}
 			if (intel_hpd_irq_event(dev, connector))
 				changed = true;
 		}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 21a950d..6ecd9da 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1746,6 +1746,11 @@ static void intel_crtc_disable_work(struct work_struct *work)
 {
 	struct intel_crtc *intel_crtc = container_of(work, struct intel_crtc,
 						     disable_work);
+	struct drm_i915_private *dev_priv = intel_crtc->base.dev->dev_private;
+
+	mutex_lock(&intel_crtc->base.mutex);
+	dev_priv->display._crtc_disable(&intel_crtc->base);
+	mutex_unlock(&intel_crtc->base.mutex);
 }
 
 void intel_queue_crtc_disable(struct drm_crtc *crtc)
@@ -1761,6 +1766,11 @@ static void intel_crtc_enable_work(struct work_struct *work)
 {
 	struct intel_crtc *intel_crtc = container_of(work, struct intel_crtc,
 						     enable_work);
+	struct drm_i915_private *dev_priv = intel_crtc->base.dev->dev_private;
+
+	mutex_lock(&intel_crtc->base.mutex);
+	dev_priv->display._crtc_enable(&intel_crtc->base);
+	mutex_unlock(&intel_crtc->base.mutex);
 }
 
 static void intel_queue_crtc_enable(struct drm_crtc *crtc)
@@ -1772,14 +1782,16 @@ static void intel_queue_crtc_enable(struct drm_crtc *crtc)
 	queue_work(dev_priv->wq, &intel_crtc->enable_work);
 }
 
-static void intel_sync_crtc(struct drm_crtc *crtc)
+void intel_sync_crtc(struct drm_crtc *crtc)
 {
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
+	WARN(!mutex_is_locked(&intel_crtc->base.mutex), "need crtc mutex\n");
+
+	mutex_unlock(&intel_crtc->base.mutex);
 	flush_work(&intel_crtc->disable_work);
 	flush_work(&intel_crtc->enable_work);
+	mutex_lock(&intel_crtc->base.mutex);
 }
 
 /**
@@ -9781,8 +9793,9 @@ static int intel_set_mode(struct drm_crtc *crtc,
 
 	ret = __intel_set_mode(crtc, mode, x, y, fb);
 
-	if (ret == 0)
-		intel_modeset_check_state(crtc->dev);
+	/* FIXME: need to check after the CRTC changes have been applied */
+//	if (ret == 0)
+//		intel_modeset_check_state(crtc->dev);
 
 	return ret;
 }
@@ -10348,8 +10361,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 
 	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
 
-	INIT_WORK(intel_crtc->enable_work, intel_crtc_enable_work);
-	INIT_WORK(intel_crtc->disable_work, intel_crtc_disable_work);
+	INIT_WORK(&intel_crtc->enable_work, intel_crtc_enable_work);
+	INIT_WORK(&intel_crtc->disable_work, intel_crtc_disable_work);
 }
 
 enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5b5b51e..f104fe1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -376,6 +376,9 @@ struct intel_crtc {
 		/* watermarks currently being used  */
 		struct intel_pipe_wm active;
 	} wm;
+
+	struct work_struct enable_work;
+	struct work_struct disable_work;
 };
 
 struct intel_plane_wm_parameters {
@@ -728,6 +731,7 @@ void hsw_disable_ips(struct intel_crtc *crtc);
 void intel_display_set_init_power(struct drm_device *dev, bool enable);
 int valleyview_get_vco(struct drm_i915_private *dev_priv);
 void intel_queue_crtc_disable(struct drm_crtc *crtc);
+void intel_sync_crtc(struct drm_crtc *crtc);
 
 /* intel_dp.c */
 void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
-- 
1.8.3.1

[PATCH 4/9] drm/i915: Propagate PCI read/write errors during vga_set_state()

[Rodrigo Vivi]

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

This has very little effect other than log the errors in case of failure,
and we then hope for the best.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6ecd9da..1e9dd84 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11570,12 +11570,21 @@ int intel_modeset_vga_set_state(struct drm_device *dev, bool state)
 	unsigned reg = INTEL_INFO(dev)->gen >= 6 ? SNB_GMCH_CTRL : INTEL_GMCH_CTRL;
 	u16 gmch_ctrl;
 
-	pci_read_config_word(dev_priv->bridge_dev, reg, &gmch_ctrl);
+	if (pci_read_config_word(dev_priv->bridge_dev, reg, &gmch_ctrl)) {
+		DRM_ERROR("failed to read control word\n");
+		return -EIO;
+	}
+
 	if (state)
 		gmch_ctrl &= ~INTEL_GMCH_VGA_DISABLE;
 	else
 		gmch_ctrl |= INTEL_GMCH_VGA_DISABLE;
-	pci_write_config_word(dev_priv->bridge_dev, reg, gmch_ctrl);
+
+	if (pci_write_config_word(dev_priv->bridge_dev, reg, gmch_ctrl)) {
+		DRM_ERROR("failed to write control word\n");
+		return -EIO;
+	}
+
 	return 0;
 }
 
-- 
1.8.3.1

[PATCH 5/9] drm/i915: Short-circuit no-op vga_set_state()

[Rodrigo Vivi]

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

Touching the VGA registers risks a hard machine hang, at least on this
ivb machine after removing a conflicting efifb. This is more than likely
related to the discovery that VGA IO decode on the more recent PCH
platforms is terminally broken.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1e9dd84..c411bf8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11575,6 +11575,9 @@ int intel_modeset_vga_set_state(struct drm_device *dev, bool state)
 		return -EIO;
 	}
 
+	if (!!(gmch_ctrl & INTEL_GMCH_VGA_DISABLE) == !state)
+		return 0;
+
 	if (state)
 		gmch_ctrl &= ~INTEL_GMCH_VGA_DISABLE;
 	else
-- 
1.8.3.1

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

[PATCH 6/9] drm/i915: Verify address field of PCBR register.

[Rodrigo Vivi]

From: Deepak S <deepak.s@intel.com>

RC6 should be enabled only if the PCBR register is programmed properly
either BIOS or Gfx. This patches address the case where PCBR
allocation fails due buggy BIOS or due to stolen memory allocation
failed.

v2: Add #define for magic numbers (Daniel)

v3: Use VLV_PCBR_ADDR_SHIFT instead of MASK (Jani)

Signed-off-by: Deepak S <deepak.s@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 1 +
 drivers/gpu/drm/i915/intel_pm.c | 9 +++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cc3ea04..41905ab 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4987,6 +4987,7 @@
 #define GEN6_GT_GFX_RC6				0x138108
 #define GEN6_GT_GFX_RC6p			0x13810C
 #define GEN6_GT_GFX_RC6pp			0x138110
+#define VLV_PCBR_ADDR_SHIFT			12
 
 #define GEN6_PCODE_MAILBOX			0x138124
 #define   GEN6_PCODE_READY			(1<<31)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f74d7f5..8caffb3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3602,7 +3602,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_ring_buffer *ring;
-	u32 gtfifodbg, val, hw_max, hw_min, rc6_mode = 0;
+	u32 gtfifodbg, val, hw_max, hw_min, rc6_mode = 0, pcbr;
 	int i;
 
 	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
@@ -3647,7 +3647,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
 		   _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH |
 				      VLV_MEDIA_RC6_COUNT_EN |
 				      VLV_RENDER_RC6_COUNT_EN));
-	if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE)
+
+	/* Enable RC6 Only if the PCBR address is configured either by
+	 * BIOS or Gfx Driver */
+	pcbr = I915_READ(VLV_PCBR);
+	if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE &&
+						(pcbr >> VLV_PCBR_ADDR_SHIFT))
 		rc6_mode = GEN7_RC_CTL_TO_MODE | VLV_RC_CTL_CTX_RST_PARALLEL;
 
 	intel_print_rc6_info(dev, rc6_mode);
-- 
1.8.3.1

[PATCH 7/9] drm/i915: Bring UP Power Wells before disabling RC6.

[Rodrigo Vivi]

From: Deepak S <deepak.s@intel.com>

We need do forcewake before Disabling RC6, This is what the BIOS
expects while going into suspend.

v2: updated commit message. (Daniel)

Signed-off-by: Deepak S <deepak.s@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8caffb3..2d74bf9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3184,8 +3184,14 @@ static void valleyview_disable_rps(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	/* we're doing forcewake before Disabling RC6,
+	 * This what the BIOS expects when going into suspend */
+	gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
+
 	I915_WRITE(GEN6_RC_CONTROL, 0);
 
+	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
+
 	gen6_disable_rps_interrupts(dev);
 
 	if (dev_priv->vlv_pctx) {
-- 
1.8.3.1

[PATCH 8/9] drm/i915: Flush GPU rendering with a lockless wait during a pagefault

[Rodrigo Vivi]

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

Arjan van de Ven reported that on his test machine that he was seeing
stalls of greater than 1 frame greatly impacting the user experience. He
tracked this down to being the locked flush during a pagefault as being
the culprit hogging the struct_mutex and so blocking any other user from
proceeding. Stalling on a pagefault is bad behaviour on userspace's
part, for one it means that they are ignoring the coherency rules on
pointer access through the GTT, but fortunately we can apply the same
trick as the set-to-domain ioctl to do a lightweight, nonblocking flush
of outstanding rendering first.

"Prior to the patch it looks like this
(this one testrun does not show the 20ms+ I've seen occasionally)

  4.99 ms     2.36 ms    31360  __wait_seqno i915_wait_seqno i915_gem_object_wait_rendering i915_gem_object_set_to_gtt_domain i915_gem_fault __do_fault handle_
+pte_fault handle_mm_fault __do_page_fault do_page_fault page_fault
   4.99 ms     2.75 ms   107751  __wait_seqno i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   4.99 ms     1.63 ms     1666  i915_mutex_lock_interruptible i915_gem_fault __do_fault handle_pte_fault handle_mm_fault __do_page_fault do_page_fault page_fa
+ult
   4.93 ms     2.45 ms      980  i915_mutex_lock_interruptible intel_crtc_page_flip drm_mode_page_flip_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_
+sysret
   4.89 ms     2.20 ms     3283  i915_mutex_lock_interruptible i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   4.34 ms     1.66 ms     1715  i915_mutex_lock_interruptible i915_gem_pwrite_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   3.73 ms     3.73 ms       49  i915_mutex_lock_interruptible i915_gem_set_domain_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   3.17 ms     0.33 ms      931  i915_mutex_lock_interruptible i915_gem_madvise_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   2.97 ms     0.43 ms     1029  i915_mutex_lock_interruptible i915_gem_busy_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   2.55 ms     0.51 ms      735  i915_gem_get_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret

After the patch it looks like this:

   4.99 ms     2.14 ms    22212  __wait_seqno i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   4.86 ms     0.99 ms    14170  __wait_seqno i915_gem_object_wait_rendering__nonblocking i915_gem_fault __do_fault handle_pte_fault handle_mm_fault __do_page_
+fault do_page_fault page_fault
   3.59 ms     1.31 ms      325  i915_gem_get_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   3.37 ms     3.37 ms       65  i915_mutex_lock_interruptible i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   2.58 ms     2.58 ms       65  i915_mutex_lock_interruptible i915_gem_do_execbuffer.isra.23 i915_gem_execbuffer2 drm_ioctl i915_compat_ioctl compat_sys_ioctl
+ia32_sysret
   2.19 ms     2.19 ms       65  i915_mutex_lock_interruptible intel_crtc_page_flip drm_mode_page_flip_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_
+sysret
   2.18 ms     2.18 ms       65  i915_mutex_lock_interruptible i915_gem_busy_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   1.66 ms     1.66 ms       65  i915_gem_set_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret

It may not look like it, but this is quite a large difference, and I've
been unable to reproduce > 5 msec delays at all, while before they do
happen (just not in the trace above)."

gem_gtt_hog on an old Pineview (GMA3150),
before: 4969.119ms
after:  4122.749ms

Reported-by: Arjan van de Ven <arjan.van.de.ven@intel.com>
Testcase: igt/gem_gtt_hog
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a8a069f..6008d88 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1184,7 +1184,7 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
  */
 static __must_check int
 i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
-					    struct drm_file *file,
+					    struct drm_i915_file_private *file_priv,
 					    bool readonly)
 {
 	struct drm_device *dev = obj->base.dev;
@@ -1211,7 +1211,7 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
 
 	reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
 	mutex_unlock(&dev->struct_mutex);
-	ret = __wait_seqno(ring, seqno, reset_counter, true, NULL, file->driver_priv);
+	ret = __wait_seqno(ring, seqno, reset_counter, true, NULL, file_priv);
 	mutex_lock(&dev->struct_mutex);
 	if (ret)
 		return ret;
@@ -1260,7 +1260,9 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	 * We will repeat the flush holding the lock in the normal manner
 	 * to catch cases where we are gazumped.
 	 */
-	ret = i915_gem_object_wait_rendering__nonblocking(obj, file, !write_domain);
+	ret = i915_gem_object_wait_rendering__nonblocking(obj,
+							  file->driver_priv,
+							  !write_domain);
 	if (ret)
 		goto unref;
 
@@ -1392,6 +1394,15 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 
 	trace_i915_gem_object_fault(obj, page_offset, true, write);
 
+	/* Try to flush the object off the GPU first without holding the lock.
+	 * Upon reacquiring the lock, we will perform our sanity checks and then
+	 * repeat the flush holding the lock in the normal manner to catch cases
+	 * where we are gazumped.
+	 */
+	ret = i915_gem_object_wait_rendering__nonblocking(obj, NULL, !write);
+	if (ret)
+		goto unlock;
+
 	/* Access to snoopable pages through the GTT is incoherent. */
 	if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev)) {
 		ret = -EINVAL;
-- 
1.8.3.1

[PATCH 9/9] drm/i915: PF CRC may not work on HSW

[Rodrigo Vivi]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When using pipe A and transcoder EDP w/o panel fitter on
HSW, the PF CRC isn't available as the panel fitter is entirely
bypassed. Check for this and refuse to give out CRCs.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2dc05c3..de020c0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2548,7 +2548,30 @@ static int ilk_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
 	return 0;
 }
 
-static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
+static bool hsw_crc_source_pf_ok(struct drm_device *dev, enum pipe pipe)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+	bool ok;
+
+	if (!IS_HASWELL(dev) || pipe != PIPE_A)
+		return true;
+
+	mutex_lock(&crtc->base.mutex);
+
+	/* pipe A -> no panel fitter -> transcoder EDP == no PF CRC */
+	ok = !crtc->active ||
+		crtc->config.cpu_transcoder != TRANSCODER_EDP ||
+		crtc->config.pch_pfit.enabled;
+
+	mutex_unlock(&crtc->base.mutex);
+
+	return ok;
+}
+
+static int ivb_pipe_crc_ctl_reg(struct drm_device *dev,
+				enum pipe pipe,
+				enum intel_pipe_crc_source *source,
 				uint32_t *val)
 {
 	if (*source == INTEL_PIPE_CRC_SOURCE_AUTO)
@@ -2562,6 +2585,8 @@ static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
 		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_IVB;
 		break;
 	case INTEL_PIPE_CRC_SOURCE_PF:
+		if (!hsw_crc_source_pf_ok(dev, pipe))
+			return -EINVAL;
 		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PF_IVB;
 		break;
 	case INTEL_PIPE_CRC_SOURCE_NONE:
@@ -2598,7 +2623,7 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
 	else if (IS_GEN5(dev) || IS_GEN6(dev))
 		ret = ilk_pipe_crc_ctl_reg(&source, &val);
 	else
-		ret = ivb_pipe_crc_ctl_reg(&source, &val);
+		ret = ivb_pipe_crc_ctl_reg(dev, pipe, &source, &val);
 
 	if (ret != 0)
 		return ret;
-- 
1.8.3.1

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

Re: [PATCH 4/9] drm/i915: Propagate PCI read/write errors during vga_set_state()

[Daniel Vetter]

On Fri, Feb 07, 2014 at 06:37:02PM -0200, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> This has very little effect other than log the errors in case of failure,
> and we then hope for the best.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-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

Re: [PATCH 5/9] drm/i915: Short-circuit no-op vga_set_state()

[Daniel Vetter]

On Fri, Feb 07, 2014 at 06:37:03PM -0200, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Touching the VGA registers risks a hard machine hang, at least on this
> ivb machine after removing a conflicting efifb. This is more than likely
> related to the discovery that VGA IO decode on the more recent PCH
> platforms is terminally broken.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-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

Re: [PATCH 9/9] drm/i915: PF CRC may not work on HSW

[Daniel Vetter]

On Fri, Feb 07, 2014 at 06:37:07PM -0200, Rodrigo Vivi wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> When using pipe A and transcoder EDP w/o panel fitter on
> HSW, the PF CRC isn't available as the panel fitter is entirely
> bypassed. Check for this and refuse to give out CRCs.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

I've discussed this a bit with Ville and I think the right fix would be to
use the DDI A CRC in this case, at least for the auto source, similarly to
who we do magic stuff for DP on g4x/vlv. I don't have a suitable hsw
machine, only one with desktop edp on DDI E around. So any volunteers?

Cheers, Daniel
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2dc05c3..de020c0 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2548,7 +2548,30 @@ static int ilk_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
>  	return 0;
>  }
>  
> -static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
> +static bool hsw_crc_source_pf_ok(struct drm_device *dev, enum pipe pipe)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> +	bool ok;
> +
> +	if (!IS_HASWELL(dev) || pipe != PIPE_A)
> +		return true;
> +
> +	mutex_lock(&crtc->base.mutex);
> +
> +	/* pipe A -> no panel fitter -> transcoder EDP == no PF CRC */
> +	ok = !crtc->active ||
> +		crtc->config.cpu_transcoder != TRANSCODER_EDP ||
> +		crtc->config.pch_pfit.enabled;
> +
> +	mutex_unlock(&crtc->base.mutex);
> +
> +	return ok;
> +}
> +
> +static int ivb_pipe_crc_ctl_reg(struct drm_device *dev,
> +				enum pipe pipe,
> +				enum intel_pipe_crc_source *source,
>  				uint32_t *val)
>  {
>  	if (*source == INTEL_PIPE_CRC_SOURCE_AUTO)
> @@ -2562,6 +2585,8 @@ static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
>  		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_IVB;
>  		break;
>  	case INTEL_PIPE_CRC_SOURCE_PF:
> +		if (!hsw_crc_source_pf_ok(dev, pipe))
> +			return -EINVAL;
>  		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PF_IVB;
>  		break;
>  	case INTEL_PIPE_CRC_SOURCE_NONE:
> @@ -2598,7 +2623,7 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
>  	else if (IS_GEN5(dev) || IS_GEN6(dev))
>  		ret = ilk_pipe_crc_ctl_reg(&source, &val);
>  	else
> -		ret = ivb_pipe_crc_ctl_reg(&source, &val);
> +		ret = ivb_pipe_crc_ctl_reg(dev, pipe, &source, &val);
>  
>  	if (ret != 0)
>  		return ret;
> -- 
> 1.8.3.1
> 
> _______________________________________________
> 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

Re: [PATCH 8/9] drm/i915: Flush GPU rendering with a lockless wait during a pagefault

[Damien Lespiau]

On Fri, Feb 07, 2014 at 06:37:06PM -0200, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Arjan van de Ven reported that on his test machine that he was seeing
> stalls of greater than 1 frame greatly impacting the user experience. He
> tracked this down to being the locked flush during a pagefault as being
> the culprit hogging the struct_mutex and so blocking any other user from
> proceeding. Stalling on a pagefault is bad behaviour on userspace's
> part, for one it means that they are ignoring the coherency rules on
> pointer access through the GTT, but fortunately we can apply the same
> trick as the set-to-domain ioctl to do a lightweight, nonblocking flush
> of outstanding rendering first.
> 
> "Prior to the patch it looks like this
> (this one testrun does not show the 20ms+ I've seen occasionally)
> 
>   4.99 ms     2.36 ms    31360  __wait_seqno i915_wait_seqno i915_gem_object_wait_rendering i915_gem_object_set_to_gtt_domain i915_gem_fault __do_fault handle_
> +pte_fault handle_mm_fault __do_page_fault do_page_fault page_fault
>    4.99 ms     2.75 ms   107751  __wait_seqno i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
>    4.99 ms     1.63 ms     1666  i915_mutex_lock_interruptible i915_gem_fault __do_fault handle_pte_fault handle_mm_fault __do_page_fault do_page_fault page_fa
> +ult
>    4.93 ms     2.45 ms      980  i915_mutex_lock_interruptible intel_crtc_page_flip drm_mode_page_flip_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_
> +sysret
>    4.89 ms     2.20 ms     3283  i915_mutex_lock_interruptible i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
>    4.34 ms     1.66 ms     1715  i915_mutex_lock_interruptible i915_gem_pwrite_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
>    3.73 ms     3.73 ms       49  i915_mutex_lock_interruptible i915_gem_set_domain_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
>    3.17 ms     0.33 ms      931  i915_mutex_lock_interruptible i915_gem_madvise_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
>    2.97 ms     0.43 ms     1029  i915_mutex_lock_interruptible i915_gem_busy_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
>    2.55 ms     0.51 ms      735  i915_gem_get_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> 
> After the patch it looks like this:
> 
>    4.99 ms     2.14 ms    22212  __wait_seqno i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
>    4.86 ms     0.99 ms    14170  __wait_seqno i915_gem_object_wait_rendering__nonblocking i915_gem_fault __do_fault handle_pte_fault handle_mm_fault __do_page_
> +fault do_page_fault page_fault
>    3.59 ms     1.31 ms      325  i915_gem_get_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
>    3.37 ms     3.37 ms       65  i915_mutex_lock_interruptible i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
>    2.58 ms     2.58 ms       65  i915_mutex_lock_interruptible i915_gem_do_execbuffer.isra.23 i915_gem_execbuffer2 drm_ioctl i915_compat_ioctl compat_sys_ioctl
> +ia32_sysret
>    2.19 ms     2.19 ms       65  i915_mutex_lock_interruptible intel_crtc_page_flip drm_mode_page_flip_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_
> +sysret
>    2.18 ms     2.18 ms       65  i915_mutex_lock_interruptible i915_gem_busy_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
>    1.66 ms     1.66 ms       65  i915_gem_set_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> 
> It may not look like it, but this is quite a large difference, and I've
> been unable to reproduce > 5 msec delays at all, while before they do
> happen (just not in the trace above)."
> 
> gem_gtt_hog on an old Pineview (GMA3150),
> before: 4969.119ms
> after:  4122.749ms
> 
> Reported-by: Arjan van de Ven <arjan.van.de.ven@intel.com>
> Testcase: igt/gem_gtt_hog
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

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

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index a8a069f..6008d88 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1184,7 +1184,7 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
>   */
>  static __must_check int
>  i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
> -					    struct drm_file *file,
> +					    struct drm_i915_file_private *file_priv,
>  					    bool readonly)
>  {
>  	struct drm_device *dev = obj->base.dev;
> @@ -1211,7 +1211,7 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
>  
>  	reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
>  	mutex_unlock(&dev->struct_mutex);
> -	ret = __wait_seqno(ring, seqno, reset_counter, true, NULL, file->driver_priv);
> +	ret = __wait_seqno(ring, seqno, reset_counter, true, NULL, file_priv);
>  	mutex_lock(&dev->struct_mutex);
>  	if (ret)
>  		return ret;
> @@ -1260,7 +1260,9 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>  	 * We will repeat the flush holding the lock in the normal manner
>  	 * to catch cases where we are gazumped.
>  	 */
> -	ret = i915_gem_object_wait_rendering__nonblocking(obj, file, !write_domain);
> +	ret = i915_gem_object_wait_rendering__nonblocking(obj,
> +							  file->driver_priv,
> +							  !write_domain);
>  	if (ret)
>  		goto unref;
>  
> @@ -1392,6 +1394,15 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  
>  	trace_i915_gem_object_fault(obj, page_offset, true, write);
>  
> +	/* Try to flush the object off the GPU first without holding the lock.
> +	 * Upon reacquiring the lock, we will perform our sanity checks and then
> +	 * repeat the flush holding the lock in the normal manner to catch cases
> +	 * where we are gazumped.
> +	 */
> +	ret = i915_gem_object_wait_rendering__nonblocking(obj, NULL, !write);
> +	if (ret)
> +		goto unlock;
> +
>  	/* Access to snoopable pages through the GTT is incoherent. */
>  	if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev)) {
>  		ret = -EINVAL;
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/9] drm/i915: Flush GPU rendering with a lockless wait during a pagefault

[Daniel Vetter]

On Mon, Feb 10, 2014 at 03:52:00PM +0000, Damien Lespiau wrote:
> On Fri, Feb 07, 2014 at 06:37:06PM -0200, Rodrigo Vivi wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > Arjan van de Ven reported that on his test machine that he was seeing
> > stalls of greater than 1 frame greatly impacting the user experience. He
> > tracked this down to being the locked flush during a pagefault as being
> > the culprit hogging the struct_mutex and so blocking any other user from
> > proceeding. Stalling on a pagefault is bad behaviour on userspace's
> > part, for one it means that they are ignoring the coherency rules on
> > pointer access through the GTT, but fortunately we can apply the same
> > trick as the set-to-domain ioctl to do a lightweight, nonblocking flush
> > of outstanding rendering first.
> > 
> > "Prior to the patch it looks like this
> > (this one testrun does not show the 20ms+ I've seen occasionally)
> > 
> >   4.99 ms     2.36 ms    31360  __wait_seqno i915_wait_seqno i915_gem_object_wait_rendering i915_gem_object_set_to_gtt_domain i915_gem_fault __do_fault handle_
> > +pte_fault handle_mm_fault __do_page_fault do_page_fault page_fault
> >    4.99 ms     2.75 ms   107751  __wait_seqno i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> >    4.99 ms     1.63 ms     1666  i915_mutex_lock_interruptible i915_gem_fault __do_fault handle_pte_fault handle_mm_fault __do_page_fault do_page_fault page_fa
> > +ult
> >    4.93 ms     2.45 ms      980  i915_mutex_lock_interruptible intel_crtc_page_flip drm_mode_page_flip_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_
> > +sysret
> >    4.89 ms     2.20 ms     3283  i915_mutex_lock_interruptible i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> >    4.34 ms     1.66 ms     1715  i915_mutex_lock_interruptible i915_gem_pwrite_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> >    3.73 ms     3.73 ms       49  i915_mutex_lock_interruptible i915_gem_set_domain_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> >    3.17 ms     0.33 ms      931  i915_mutex_lock_interruptible i915_gem_madvise_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> >    2.97 ms     0.43 ms     1029  i915_mutex_lock_interruptible i915_gem_busy_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> >    2.55 ms     0.51 ms      735  i915_gem_get_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> > 
> > After the patch it looks like this:
> > 
> >    4.99 ms     2.14 ms    22212  __wait_seqno i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> >    4.86 ms     0.99 ms    14170  __wait_seqno i915_gem_object_wait_rendering__nonblocking i915_gem_fault __do_fault handle_pte_fault handle_mm_fault __do_page_
> > +fault do_page_fault page_fault
> >    3.59 ms     1.31 ms      325  i915_gem_get_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> >    3.37 ms     3.37 ms       65  i915_mutex_lock_interruptible i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> >    2.58 ms     2.58 ms       65  i915_mutex_lock_interruptible i915_gem_do_execbuffer.isra.23 i915_gem_execbuffer2 drm_ioctl i915_compat_ioctl compat_sys_ioctl
> > +ia32_sysret
> >    2.19 ms     2.19 ms       65  i915_mutex_lock_interruptible intel_crtc_page_flip drm_mode_page_flip_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_
> > +sysret
> >    2.18 ms     2.18 ms       65  i915_mutex_lock_interruptible i915_gem_busy_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> >    1.66 ms     1.66 ms       65  i915_gem_set_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
> > 
> > It may not look like it, but this is quite a large difference, and I've
> > been unable to reproduce > 5 msec delays at all, while before they do
> > happen (just not in the trace above)."
> > 
> > gem_gtt_hog on an old Pineview (GMA3150),
> > before: 4969.119ms
> > after:  4122.749ms
> > 
> > Reported-by: Arjan van de Ven <arjan.van.de.ven@intel.com>
> > Testcase: igt/gem_gtt_hog
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@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

Re: [PATCH 3/9] drm/i915: make crtc enable/disable asynchronous

[Jesse Barnes]

On Fri,  7 Feb 2014 18:37:01 -0200
Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:

> From: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> The intent is to get back to userspace as quickly as possible so it can
> start doing drawing or whatever.  It should also allow our
> suspend/resume/init time to improve a lot.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c      | 10 +++++++++-
>  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_drv.h     |  4 ++++
>  3 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index e9c94c9..749c20f 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -917,8 +917,16 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  		intel_connector = to_intel_connector(connector);
>  		intel_encoder = intel_connector->encoder;
>  		if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
> -			if (intel_encoder->hot_plug)
> +			if (intel_encoder->hot_plug) {
> +				struct drm_crtc *crtc =
> +					intel_encoder->base.crtc;
> +				if (crtc) {
> +					mutex_lock(&crtc->mutex);
> +					intel_sync_crtc(intel_encoder->base.crtc);
> +					mutex_unlock(&crtc->mutex);
> +				}
>  				intel_encoder->hot_plug(intel_encoder);
> +			}
>  			if (intel_hpd_irq_event(dev, connector))
>  				changed = true;
>  		}
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 21a950d..6ecd9da 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1746,6 +1746,11 @@ static void intel_crtc_disable_work(struct work_struct *work)
>  {
>  	struct intel_crtc *intel_crtc = container_of(work, struct intel_crtc,
>  						     disable_work);
> +	struct drm_i915_private *dev_priv = intel_crtc->base.dev->dev_private;
> +
> +	mutex_lock(&intel_crtc->base.mutex);
> +	dev_priv->display._crtc_disable(&intel_crtc->base);
> +	mutex_unlock(&intel_crtc->base.mutex);
>  }
>  
>  void intel_queue_crtc_disable(struct drm_crtc *crtc)
> @@ -1761,6 +1766,11 @@ static void intel_crtc_enable_work(struct work_struct *work)
>  {
>  	struct intel_crtc *intel_crtc = container_of(work, struct intel_crtc,
>  						     enable_work);
> +	struct drm_i915_private *dev_priv = intel_crtc->base.dev->dev_private;
> +
> +	mutex_lock(&intel_crtc->base.mutex);
> +	dev_priv->display._crtc_enable(&intel_crtc->base);
> +	mutex_unlock(&intel_crtc->base.mutex);
>  }
>  
>  static void intel_queue_crtc_enable(struct drm_crtc *crtc)
> @@ -1772,14 +1782,16 @@ static void intel_queue_crtc_enable(struct drm_crtc *crtc)
>  	queue_work(dev_priv->wq, &intel_crtc->enable_work);
>  }
>  
> -static void intel_sync_crtc(struct drm_crtc *crtc)
> +void intel_sync_crtc(struct drm_crtc *crtc)
>  {
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
> +	WARN(!mutex_is_locked(&intel_crtc->base.mutex), "need crtc mutex\n");
> +
> +	mutex_unlock(&intel_crtc->base.mutex);
>  	flush_work(&intel_crtc->disable_work);
>  	flush_work(&intel_crtc->enable_work);
> +	mutex_lock(&intel_crtc->base.mutex);
>  }
>  
>  /**
> @@ -9781,8 +9793,9 @@ static int intel_set_mode(struct drm_crtc *crtc,
>  
>  	ret = __intel_set_mode(crtc, mode, x, y, fb);
>  
> -	if (ret == 0)
> -		intel_modeset_check_state(crtc->dev);
> +	/* FIXME: need to check after the CRTC changes have been applied */
> +//	if (ret == 0)
> +//		intel_modeset_check_state(crtc->dev);
>  
>  	return ret;
>  }
> @@ -10348,8 +10361,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  
>  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
>  
> -	INIT_WORK(intel_crtc->enable_work, intel_crtc_enable_work);
> -	INIT_WORK(intel_crtc->disable_work, intel_crtc_disable_work);
> +	INIT_WORK(&intel_crtc->enable_work, intel_crtc_enable_work);
> +	INIT_WORK(&intel_crtc->disable_work, intel_crtc_disable_work);
>  }
>  
>  enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5b5b51e..f104fe1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -376,6 +376,9 @@ struct intel_crtc {
>  		/* watermarks currently being used  */
>  		struct intel_pipe_wm active;
>  	} wm;
> +
> +	struct work_struct enable_work;
> +	struct work_struct disable_work;
>  };
>  
>  struct intel_plane_wm_parameters {
> @@ -728,6 +731,7 @@ void hsw_disable_ips(struct intel_crtc *crtc);
>  void intel_display_set_init_power(struct drm_device *dev, bool enable);
>  int valleyview_get_vco(struct drm_i915_private *dev_priv);
>  void intel_queue_crtc_disable(struct drm_crtc *crtc);
> +void intel_sync_crtc(struct drm_crtc *crtc);
>  
>  /* intel_dp.c */
>  void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);

What ever happened with this?  Ville and Daniel, can you take a look?
This has the potential to let other stuff run in the 200-2000ms it
takes us to do a mode set...

Thanks,
Jesse

-- 
Jesse Barnes, Intel Open Source Technology Center