Re: [Intel-gfx] [PATCH v8 06/19] drm/i915/gen8: Add PML4 structure

2015-08-03 Thread Michel Thierry

On 7/31/2015 6:35 PM, Goel, Akash wrote:

On 7/31/2015 5:42 PM, Michel Thierry wrote:

--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -689,9 +689,6 @@ gen8_ppgtt_insert_pte_entries(struct
i915_address_space *vm,
  pt_vaddr = NULL;

  for_each_sg_page(pages->sgl, &sg_iter, pages->nents, 0) {
-if (WARN_ON(pdpe >= GEN8_LEGACY_PDPES))
-break;
-

Apologize for this eleventh hour comment.
Would this change be better off in the later patch "Add 4 level support
in insert_entries and clear_range".


Makes sense.

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


Re: [Intel-gfx] [PATCH v8 06/19] drm/i915/gen8: Add PML4 structure

2015-07-31 Thread Goel, Akash



On 7/31/2015 5:42 PM, Michel Thierry wrote:

Introduces the Page Map Level 4 (PML4), ie. the new top level structure
of the page tables.

To facilitate testing, 48b mode will be available on Broadwell and
GEN9+, when i915.enable_ppgtt = 3.

v2: Remove unnecessary CONFIG_X86_64 checks, ppgtt code is already
32/64-bit safe (Chris).
v3: Add goto free_scratch in temp 48-bit mode init code (Akash).
v4: kfree the pdp until the 4lvl alloc/free patch (Akash).
v5: Postpone 48-bit code in sanitize_enable_ppgtt (Akash).

Cc: Akash Goel 
Signed-off-by: Michel Thierry 
---
  drivers/gpu/drm/i915/i915_drv.h |  3 ++-
  drivers/gpu/drm/i915/i915_gem_gtt.c | 30 +-
  drivers/gpu/drm/i915/i915_gem_gtt.h | 26 +-
  3 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 04aa34a..4729eaf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2498,7 +2498,8 @@ struct drm_i915_cmd_table {
  #define HAS_HW_CONTEXTS(dev)  (INTEL_INFO(dev)->gen >= 6)
  #define HAS_LOGICAL_RING_CONTEXTS(dev)(INTEL_INFO(dev)->gen >= 8)
  #define USES_PPGTT(dev)   (i915.enable_ppgtt)
-#define USES_FULL_PPGTT(dev)   (i915.enable_ppgtt == 2)
+#define USES_FULL_PPGTT(dev)   (i915.enable_ppgtt >= 2)
+#define USES_FULL_48BIT_PPGTT(dev) (i915.enable_ppgtt == 3)

  #define HAS_OVERLAY(dev)  (INTEL_INFO(dev)->has_overlay)
  #define OVERLAY_NEEDS_PHYSICAL(dev)   
(INTEL_INFO(dev)->overlay_needs_physical)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 7f71746..ba99b67 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -689,9 +689,6 @@ gen8_ppgtt_insert_pte_entries(struct i915_address_space *vm,
pt_vaddr = NULL;

for_each_sg_page(pages->sgl, &sg_iter, pages->nents, 0) {
-   if (WARN_ON(pdpe >= GEN8_LEGACY_PDPES))
-   break;
-

Apologize for this eleventh hour comment.
Would this change be better off in the later patch "Add 4 level support 
in insert_entries and clear_range".


Best regards
Akash


if (pt_vaddr == NULL) {
struct i915_page_directory *pd = 
pdp->page_directory[pdpe];
struct i915_page_table *pt = pd->page_table[pde];
@@ -1105,14 +1102,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
return ret;

ppgtt->base.start = 0;
-   ppgtt->base.total = 1ULL << 32;
-   if (IS_ENABLED(CONFIG_X86_32))
-   /* While we have a proliferation of size_t variables
-* we cannot represent the full ppgtt size on 32bit,
-* so limit it to the same size as the GGTT (currently
-* 2GiB).
-*/
-   ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
ppgtt->base.cleanup = gen8_ppgtt_cleanup;
ppgtt->base.allocate_va_range = gen8_alloc_va_range;
ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
@@ -1122,10 +,25 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)

ppgtt->switch_mm = gen8_mm_switch;

-   ret = __pdp_init(false, &ppgtt->pdp);
+   if (!USES_FULL_48BIT_PPGTT(ppgtt->base.dev)) {
+   ret = __pdp_init(false, &ppgtt->pdp);

-   if (ret)
+   if (ret)
+   goto free_scratch;
+
+   ppgtt->base.total = 1ULL << 32;
+   if (IS_ENABLED(CONFIG_X86_32))
+   /* While we have a proliferation of size_t variables
+* we cannot represent the full ppgtt size on 32bit,
+* so limit it to the same size as the GGTT (currently
+* 2GiB).
+*/
+   ppgtt->base.total = 
to_i915(ppgtt->base.dev)->gtt.base.total;
+   } else {
+   ppgtt->base.total = 1ULL << 48;
+   ret = -EPERM; /* Not yet implemented */
goto free_scratch;
+   }

return 0;

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 87e389c..04bc66f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -88,9 +88,17 @@ typedef uint64_t gen8_pde_t;
   * PDPE  |  PDE  |  PTE  | offset
   * The difference as compared to normal x86 3 level page table is the PDPEs 
are
   * programmed via register.
+ *
+ * GEN8 48b legacy style address is defined as a 4 level page table:
+ * 47:39 | 38:30 | 29:21 | 20:12 |  11:0
+ * PML4E | PDPE  |  PDE  |  PTE  | offset
   */
+#define GEN8_PML4ES_PER_PML4   512
+#define GEN8_PML4E_SHIFT   39
  #define GEN8_PDPE_SHIFT   30
-#define GEN8_PDPE_MASK 0x3
+/* NB: GEN8_PDPE_MASK is untrue for 32b platforms, but it has no impact on 32