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

[PATCH 0/4] 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: Bring UP Power Wells before disabling RC6. - Reviewer:
Patch     drm/i915: Don't save/restore RS when not used - Reviewer:
Patch     drm/i915: honour forced connector modes - Reviewer:
Patch     drm/i915/bdw: Map unused PDPs to a scratch page - Reviewer:


Ben Widawsky (1):
  drm/i915: Don't save/restore RS when not used

Bob Beckett (1):
  drm/i915/bdw: Map unused PDPs to a scratch page

Chris Wilson (1):
  drm/i915: honour forced connector modes

Deepak S (1):
  drm/i915: Bring UP Power Wells before disabling RC6.

 drivers/gpu/drm/i915/i915_gem_context.c | 10 +++--
 drivers/gpu/drm/i915/i915_gem_gtt.c     | 79 ++++++++++++++++++++++++++-------
 drivers/gpu/drm/i915/i915_gem_gtt.h     |  2 +
 drivers/gpu/drm/i915/intel_fbdev.c      | 33 +++++---------
 drivers/gpu/drm/i915/intel_pm.c         |  6 +++
 5 files changed, 89 insertions(+), 41 deletions(-)

-- 
1.9.3

[PATCH 1/4] 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)

Reviewer: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Deepak S <deepak.s@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.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 41de760..9f96baa 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3527,8 +3527,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);
 }
 
-- 
1.9.3

[PATCH 2/4] drm/i915: Don't save/restore RS when not used

[Rodrigo Vivi]

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

v2: fix conflict on rebase.

Cc: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 3b99390..15ec7e4 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -563,6 +563,7 @@ mi_set_context(struct intel_engine_cs *ring,
 	       struct intel_context *new_context,
 	       u32 hw_flags)
 {
+	u32 flags = hw_flags | MI_MM_SPACE_GTT;
 	int ret;
 
 	/* w/a: If Flush TLB Invalidation Mode is enabled, driver must do a TLB
@@ -576,6 +577,10 @@ mi_set_context(struct intel_engine_cs *ring,
 			return ret;
 	}
 
+	/* These flags are for resource streamer on HSW+ */
+	if (!IS_HASWELL(ring->dev) && INTEL_INFO(ring->dev)->gen < 8)
+		flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
+
 	ret = intel_ring_begin(ring, 6);
 	if (ret)
 		return ret;
@@ -589,10 +594,7 @@ mi_set_context(struct intel_engine_cs *ring,
 	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_emit(ring, MI_SET_CONTEXT);
 	intel_ring_emit(ring, i915_gem_obj_ggtt_offset(new_context->legacy_hw_ctx.rcs_state) |
-			MI_MM_SPACE_GTT |
-			MI_SAVE_EXT_STATE_EN |
-			MI_RESTORE_EXT_STATE_EN |
-			hw_flags);
+			flags);
 	/*
 	 * w/a: MI_SET_CONTEXT must always be followed by MI_NOOP
 	 * WaMiSetContext_Hang:snb,ivb,vlv
-- 
1.9.3

[PATCH 3/4] drm/i915: honour forced connector modes

[Rodrigo Vivi]

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

In the move over to use BIOS connector configs, we lost the ability to
force a specific set of connectors on or off.  Try to remedy that by
dropping back to the old behavior if we detect a hard coded connector
config that tries to enable a connector (disabling is easy!).

Based on earlier patches by Jesse Barnes.

v2: Remove Jesse's patch

Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_fbdev.c | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index f475414..5d879d18 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -331,24 +331,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 	int num_connectors_enabled = 0;
 	int num_connectors_detected = 0;
 
-	/*
-	 * If the user specified any force options, just bail here
-	 * and use that config.
-	 */
-	for (i = 0; i < fb_helper->connector_count; i++) {
-		struct drm_fb_helper_connector *fb_conn;
-		struct drm_connector *connector;
-
-		fb_conn = fb_helper->connector_info[i];
-		connector = fb_conn->connector;
-
-		if (!enabled[i])
-			continue;
-
-		if (connector->force != DRM_FORCE_UNSPECIFIED)
-			return false;
-	}
-
 	save_enabled = kcalloc(dev->mode_config.num_connector, sizeof(bool),
 			       GFP_KERNEL);
 	if (!save_enabled)
@@ -374,8 +356,18 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 			continue;
 		}
 
+		if (connector->force == DRM_FORCE_OFF) {
+			DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n",
+				      connector->name);
+			enabled[i] = false;
+			continue;
+		}
+
 		encoder = connector->encoder;
 		if (!encoder || WARN_ON(!encoder->crtc)) {
+			if (connector->force > DRM_FORCE_OFF)
+				goto bail;
+
 			DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n",
 				      connector->name);
 			enabled[i] = false;
@@ -394,8 +386,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 		for (j = 0; j < fb_helper->connector_count; j++) {
 			if (crtcs[j] == new_crtc) {
 				DRM_DEBUG_KMS("fallback: cloned configuration\n");
-				fallback = true;
-				goto out;
+				goto bail;
 			}
 		}
 
@@ -466,8 +457,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 		fallback = true;
 	}
 
-out:
 	if (fallback) {
+bail:
 		DRM_DEBUG_KMS("Not using firmware configuration\n");
 		memcpy(enabled, save_enabled, dev->mode_config.num_connector);
 		kfree(save_enabled);
-- 
1.9.3

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

[PATCH 4/4] drm/i915/bdw: Map unused PDPs to a scratch page

[Rodrigo Vivi]

From: Bob Beckett <robert.beckett@intel.com>

Create a scratch page for the two unused PDPs and set all the PTEs
for them to point to it.

This patch addresses a page fault, and subsequent hang in pipe
control flush. In these cases, the Main Graphic Arbiter Error
register [0x40A0] showed a TLB Page Fault error, and a high memory
address (higher than the size of our PPGTT) was reported in the
Fault TLB RD Data0 register (0x4B10).

PDP2 & PDP3 were not set because, in theory, they aren't required
for our PPGTT size, but they should be mapped to a scratch page
anyway.

v2: Rebase on latest nightly.

Signed-off-by: Michel Thierry <michel.thierry@intel.com> (v1)
Signed-off-by: Dave Gordon <david.s.gordon@intel.com> (v2)
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 79 +++++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/i915_gem_gtt.h |  2 +
 2 files changed, 65 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b4b7cfd..7cb18e7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -361,6 +361,11 @@ static void gen8_ppgtt_free(const struct i915_hw_ppgtt *ppgtt)
 		kfree(ppgtt->gen8_pt_dma_addr[i]);
 	}
 
+	/* Unused PDPs are always assigned to scratch page */
+	for (i = ppgtt->num_pd_pages; i < GEN8_LEGACY_PDPS; i++)
+		kfree(ppgtt->gen8_pt_dma_addr[i]);
+	__free_page(ppgtt->scratch_page);
+
 	__free_pages(ppgtt->pd_pages, get_order(ppgtt->num_pd_pages << PAGE_SHIFT));
 }
 
@@ -385,6 +390,13 @@ static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt)
 					       PCI_DMA_BIDIRECTIONAL);
 		}
 	}
+
+	/* Unused PDPs are always assigned to scratch page */
+	for (i = ppgtt->num_pd_pages; i < GEN8_LEGACY_PDPS; i++) {
+		if (ppgtt->pd_dma_addr[i])
+			pci_unmap_page(hwdev, ppgtt->pd_dma_addr[i],
+				PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
+	}
 }
 
 static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
@@ -471,10 +483,21 @@ static int gen8_ppgtt_allocate_dma(struct i915_hw_ppgtt *ppgtt)
 static int gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt,
 						const int max_pdp)
 {
-	ppgtt->pd_pages = alloc_pages(GFP_KERNEL, get_order(max_pdp << PAGE_SHIFT));
-	if (!ppgtt->pd_pages)
+	/* Scratch page for unmapped PDP's */
+	ppgtt->scratch_page = alloc_page(GFP_KERNEL);
+	if (!ppgtt->scratch_page)
 		return -ENOMEM;
 
+	/* Must allocate space for all 4 PDPs. HW has implemented cache which
+	 * pre-fetches entries; that pre-fetch can attempt access for entries
+	 * even if no resources are located in that range.
+	 */
+	ppgtt->pd_pages = alloc_pages(GFP_KERNEL, GEN8_LEGACY_PDPS);
+	if (!ppgtt->pd_pages) {
+		__free_page(ppgtt->scratch_page);
+		return -ENOMEM;
+	}
+
 	ppgtt->num_pd_pages = 1 << get_order(max_pdp << PAGE_SHIFT);
 	BUG_ON(ppgtt->num_pd_pages > GEN8_LEGACY_PDPS);
 
@@ -492,6 +515,7 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt,
 
 	ret = gen8_ppgtt_allocate_page_tables(ppgtt, max_pdp);
 	if (ret) {
+		__free_page(ppgtt->scratch_page);
 		__free_pages(ppgtt->pd_pages, get_order(max_pdp << PAGE_SHIFT));
 		return ret;
 	}
@@ -526,18 +550,25 @@ static int gen8_ppgtt_setup_page_directories(struct i915_hw_ppgtt *ppgtt,
 
 static int gen8_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt,
 					const int pd,
-					const int pt)
+					const int pt,
+					const int max_pdp)
 {
 	dma_addr_t pt_addr;
 	struct page *p;
 	int ret;
 
-	p = ppgtt->gen8_pt_pages[pd][pt];
-	pt_addr = pci_map_page(ppgtt->base.dev->pdev,
-			       p, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
-	ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pt_addr);
-	if (ret)
-		return ret;
+	/* Unused PDPs need to have their ptes pointing to the
+	 * existing scratch page.
+	 */
+	if (pd < max_pdp) {
+		p = ppgtt->gen8_pt_pages[pd][pt];
+		pt_addr = pci_map_page(ppgtt->base.dev->pdev,
+					p, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
+		ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pt_addr);
+		if (ret)
+			return ret;
+	} else
+		pt_addr = ppgtt->scratch_dma_addr;
 
 	ppgtt->gen8_pt_dma_addr[pd][pt] = pt_addr;
 
@@ -559,6 +590,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
 	const int max_pdp = DIV_ROUND_UP(size, 1 << 30);
 	const int min_pt_pages = GEN8_PDES_PER_PAGE * max_pdp;
 	int i, j, ret;
+	gen8_gtt_pte_t *pt_vaddr, scratch_pte;
 
 	if (size % (1<<30))
 		DRM_INFO("Pages will be wasted unless GTT size (%llu) is divisible by 1GB\n", size);
@@ -568,30 +600,38 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
 	if (ret)
 		return ret;
 
-	/*
-	 * 2. Create DMA mappings for the page directories and page tables.
-	 */
-	for (i = 0; i < max_pdp; i++) {
+	/* 2. Map the scratch page */
+	ppgtt->scratch_dma_addr =
+		pci_map_page(ppgtt->base.dev->pdev,
+			     ppgtt->scratch_page, 0, PAGE_SIZE,
+			     PCI_DMA_BIDIRECTIONAL);
+
+	ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, ppgtt->scratch_dma_addr);
+	if (ret)
+		goto bail;
+
+	/* 3. Create DMA mappings for the page directories and page tables. */
+	for (i = 0; i < GEN8_LEGACY_PDPS; i++) {
 		ret = gen8_ppgtt_setup_page_directories(ppgtt, i);
 		if (ret)
 			goto bail;
 
 		for (j = 0; j < GEN8_PDES_PER_PAGE; j++) {
-			ret = gen8_ppgtt_setup_page_tables(ppgtt, i, j);
+			ret = gen8_ppgtt_setup_page_tables(ppgtt, i, j, max_pdp);
 			if (ret)
 				goto bail;
 		}
 	}
 
 	/*
-	 * 3. Map all the page directory entires to point to the page tables
+	 * 4. Map all the page directory entries to point to the page tables
 	 * we've allocated.
 	 *
 	 * For now, the PPGTT helper functions all require that the PDEs are
 	 * plugged in correctly. So we do that now/here. For aliasing PPGTT, we
 	 * will never need to touch the PDEs again.
 	 */
-	for (i = 0; i < max_pdp; i++) {
+	for (i = 0; i < GEN8_LEGACY_PDPS; i++) {
 		gen8_ppgtt_pde_t *pd_vaddr;
 		pd_vaddr = kmap_atomic(&ppgtt->pd_pages[i]);
 		for (j = 0; j < GEN8_PDES_PER_PAGE; j++) {
@@ -614,6 +654,13 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
 
 	ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.total, true);
 
+	scratch_pte = gen8_pte_encode(ppgtt->base.scratch.addr,
+					I915_CACHE_LLC, true);
+	pt_vaddr = kmap_atomic(ppgtt->scratch_page);
+	for (i = 0; i < GEN8_PTES_PER_PAGE; i++)
+		pt_vaddr[i] = scratch_pte;
+	kunmap_atomic(pt_vaddr);
+
 	DRM_DEBUG_DRIVER("Allocated %d pages for page directories (%d wasted)\n",
 			 ppgtt->num_pd_pages, ppgtt->num_pd_pages - max_pdp);
 	DRM_DEBUG_DRIVER("Allocated %d pages for page tables (%lld wasted)\n",
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 666c938..02032b3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -249,6 +249,7 @@ struct i915_hw_ppgtt {
 		struct page **gen8_pt_pages[GEN8_LEGACY_PDPS];
 	};
 	struct page *pd_pages;
+	struct page *scratch_page;
 	union {
 		uint32_t pd_offset;
 		dma_addr_t pd_dma_addr[GEN8_LEGACY_PDPS];
@@ -258,6 +259,7 @@ struct i915_hw_ppgtt {
 		dma_addr_t *gen8_pt_dma_addr[4];
 	};
 
+	dma_addr_t scratch_dma_addr;
 	struct intel_context *ctx;
 
 	int (*enable)(struct i915_hw_ppgtt *ppgtt);
-- 
1.9.3

Re: [PATCH 4/4] drm/i915/bdw: Map unused PDPs to a scratch page

[Daniel Vetter]

On Mon, Aug 18, 2014 at 10:35:30AM -0700, Rodrigo Vivi wrote:
> From: Bob Beckett <robert.beckett@intel.com>
> 
> Create a scratch page for the two unused PDPs and set all the PTEs
> for them to point to it.
> 
> This patch addresses a page fault, and subsequent hang in pipe
> control flush. In these cases, the Main Graphic Arbiter Error
> register [0x40A0] showed a TLB Page Fault error, and a high memory
> address (higher than the size of our PPGTT) was reported in the
> Fault TLB RD Data0 register (0x4B10).
> 
> PDP2 & PDP3 were not set because, in theory, they aren't required
> for our PPGTT size, but they should be mapped to a scratch page
> anyway.
> 
> v2: Rebase on latest nightly.
> 
> Signed-off-by: Michel Thierry <michel.thierry@intel.com> (v1)
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com> (v2)
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

No idea about this one, especially since there's tons of other bdw ppgtt
patches in-flight. I've merged all the others though.

Aside: We need to figure out how to make people review their -collector
assignments actually, it seems to totally not work :(
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch