Re: [PATCH v4 07/24] drm/i915: Create page table allocators

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/20/2015 4:50 PM, Mika Kuoppala wrote:
Michel Thierry <michel.thierry@xxxxxxxxx> writes:

From: Ben Widawsky <benjamin.widawsky@xxxxxxxxx>

As we move toward dynamic page table allocation, it becomes much easier
to manage our data structures if break do things less coarsely by
breaking up all of our actions into individual tasks.  This makes the
code easier to write, read, and verify.

Aside from the dissection of the allocation functions, the patch
statically allocates the page table structures without a page directory.
This remains the same for all platforms,

The patch itself should not have much functional difference. The primary
noticeable difference is the fact that page tables are no longer
allocated, but rather statically declared as part of the page directory.
This has non-zero overhead, but things gain non-trivial complexity as a
result.

This patch exists for a few reasons:
1. Splitting out the functions allows easily combining GEN6 and GEN8
code. Page tables have no difference based on GEN8. As we'll see in a
future patch when we add the DMA mappings to the allocations, it
requires only one small change to make work, and error handling should
just fall into place.

2. Unless we always want to allocate all page tables under a given PDE,
we'll have to eventually break this up into an array of pointers (or
pointer to pointer).

3. Having the discrete functions is easier to review, and understand.
All allocations and frees now take place in just a couple of locations.
Reviewing, and catching leaks should be easy.

4. Less important: the GFP flags are confined to one location, which
makes playing around with such things trivial.

v2: Updated commit message to explain why this patch exists

v3: For lrc, s/pdp.page_directory[i].daddr/pdp.page_directory[i]->daddr/

v4: Renamed free_pt/pd_single functions to unmap_and_free_pt/pd (Daniel)

v5: Added additional safety checks in gen8 clear/free/unmap.

Signed-off-by: Ben Widawsky <ben@xxxxxxxxxxxx>
Signed-off-by: Michel Thierry <michel.thierry@xxxxxxxxx> (v3, v4, v5)
---
  drivers/gpu/drm/i915/i915_gem_gtt.c | 251 ++++++++++++++++++++++++------------
  drivers/gpu/drm/i915/i915_gem_gtt.h |   4 +-
  drivers/gpu/drm/i915/intel_lrc.c    |  16 +--
  3 files changed, 179 insertions(+), 92 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0fe5c1e..85ea535 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -275,6 +275,99 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr,
  	return pte;
  }
+static void unmap_and_free_pt(struct i915_page_table_entry *pt)
+{
+	if (WARN_ON(!pt->page))
+		return;
+	__free_page(pt->page);
+	kfree(pt);
+}
+
+static struct i915_page_table_entry *alloc_pt_single(void)
+{
+	struct i915_page_table_entry *pt;
+
+	pt = kzalloc(sizeof(*pt), GFP_KERNEL);
+	if (!pt)
+		return ERR_PTR(-ENOMEM);
+
+	pt->page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+	if (!pt->page) {
+		kfree(pt);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	return pt;
+}
+
+/**
+ * alloc_pt_range() - Allocate a multiple page tables
+ * @pd:		The page directory which will have at least @count entries
+ *		available to point to the allocated page tables.
+ * @pde:	First page directory entry for which we are allocating.
+ * @count:	Number of pages to allocate.
+ *
+ * Allocates multiple page table pages and sets the appropriate entries in the
+ * page table structure within the page directory. Function cleans up after
+ * itself on any failures.
+ *
+ * Return: 0 if allocation succeeded.
+ */
+static int alloc_pt_range(struct i915_page_directory_entry *pd, uint16_t pde, size_t count)
+{
+	int i, ret;
+
+	/* 512 is the max page tables per page_directory on any platform.
+	 * TODO: make WARN after patch series is done
+	 */
+	BUG_ON(pde + count > GEN6_PPGTT_PD_ENTRIES);
+
WARN_ON in here and return -EINVAL.

-Mika

I applied the changes in v6.

Thanks for the review.

-Michel

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

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

[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux