| Message ID | 1424454366-19006-7-git-send-email-michel.thierry@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 20, 2015 at 11:16 PM, Michel Thierry <michel.thierry@intel.com> wrote: > From: Ben Widawsky <benjamin.widawsky@intel.com> > > Map is easy, it's the same register as the PDP descriptor 0, but it only > has one entry. > > v2: PML4 update in legacy context switch is left for historic reasons, > the preferred mode of operation is with lrc context based submission. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 56 +++++++++++++++++++++++++++++++++---- > drivers/gpu/drm/i915/i915_gem_gtt.h | 4 ++- > drivers/gpu/drm/i915/i915_reg.h | 1 + > 3 files changed, 55 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index edada33..fb06f67 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -192,6 +192,9 @@ static inline gen8_ppgtt_pde_t gen8_pde_encode(struct drm_device *dev, > return pde; > } > > +#define gen8_pdpe_encode gen8_pde_encode > +#define gen8_pml4e_encode gen8_pde_encode > + > static gen6_gtt_pte_t snb_pte_encode(dma_addr_t addr, > enum i915_cache_level level, > bool valid, u32 unused) > @@ -592,8 +595,8 @@ static int gen8_write_pdp(struct intel_engine_cs *ring, > return 0; > } > > -static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt, > - struct intel_engine_cs *ring) > +static int gen8_legacy_mm_switch(struct i915_hw_ppgtt *ppgtt, > + struct intel_engine_cs *ring) > { > int i, ret; > > @@ -610,6 +613,12 @@ static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt, > return 0; > } > > +static int gen8_48b_mm_switch(struct i915_hw_ppgtt *ppgtt, > + struct intel_engine_cs *ring) > +{ > + return gen8_write_pdp(ring, 0, ppgtt->pml4.daddr); > +} > + > static void gen8_ppgtt_clear_range(struct i915_address_space *vm, > uint64_t start, > uint64_t length, > @@ -753,6 +762,37 @@ static void gen8_map_pagetable_range(struct i915_address_space *vm, > kunmap_atomic(page_directory); > } > > +static void gen8_map_page_directory(struct i915_page_directory_pointer_entry *pdp, > + struct i915_page_directory_entry *pd, > + int index, > + struct drm_device *dev) > +{ > + gen8_ppgtt_pdpe_t *page_directorypo; > + gen8_ppgtt_pdpe_t pdpe; > + > + /* We do not need to clflush because no platform requiring flush > + * supports 64b pagetables. */ Would be more appropriate to place this comment, either after the ‘if’ condition or at the end of the function (where clflush would have been placed, had LLC not been there for platforms supporting 64 bit). And same comment can be probably added, at the end of gen8_map_page_directory_pointer function also. > + if (!USES_FULL_48BIT_PPGTT(dev)) > + return; > + > + page_directorypo = kmap_atomic(pdp->page); > + pdpe = gen8_pdpe_encode(dev, pd->daddr, I915_CACHE_LLC); > + page_directorypo[index] = pdpe; > + kunmap_atomic(page_directorypo); > +} > + > +static void gen8_map_page_directory_pointer(struct i915_pml4 *pml4, > + struct i915_page_directory_pointer_entry *pdp, > + int index, > + struct drm_device *dev) > +{ > + gen8_ppgtt_pml4e_t *pagemap = kmap_atomic(pml4->page); > + gen8_ppgtt_pml4e_t pml4e = gen8_pml4e_encode(dev, pdp->daddr, I915_CACHE_LLC); > + BUG_ON(!USES_FULL_48BIT_PPGTT(dev)); > + pagemap[index] = pml4e; > + kunmap_atomic(pagemap); > +} > + > static void gen8_free_page_tables(struct i915_page_directory_entry *pd, struct drm_device *dev) > { > int i; > @@ -1124,6 +1164,7 @@ static int gen8_alloc_va_range_3lvl(struct i915_address_space *vm, > > set_bit(pdpe, pdp->used_pdpes); > gen8_map_pagetable_range(vm, pd, start, length); > + gen8_map_page_directory(pdp, pd, pdpe, dev); > } > > free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes); > @@ -1192,6 +1233,8 @@ static int gen8_alloc_va_range_4lvl(struct i915_address_space *vm, > ret = gen8_alloc_va_range_3lvl(vm, pdp, start, length); > if (ret) > goto err_out; > + > + gen8_map_page_directory_pointer(pml4, pdp, pml4e, vm->dev); > } > > bitmap_or(pml4->used_pml4es, new_pdps, pml4->used_pml4es, > @@ -1251,14 +1294,14 @@ static int gen8_ppgtt_init_common(struct i915_hw_ppgtt *ppgtt, uint64_t size) > ppgtt->base.cleanup = gen8_ppgtt_cleanup; > ppgtt->base.insert_entries = gen8_ppgtt_insert_entries; > > - ppgtt->switch_mm = gen8_mm_switch; > - > if (USES_FULL_48BIT_PPGTT(ppgtt->base.dev)) { > int ret = pml4_init(ppgtt); > if (ret) { > unmap_and_free_pt(ppgtt->scratch_pd, ppgtt->base.dev); > return ret; > } > + > + ppgtt->switch_mm = gen8_48b_mm_switch; > } else { > int ret = __pdp_init(&ppgtt->pdp, false); > if (ret) { > @@ -1266,6 +1309,7 @@ static int gen8_ppgtt_init_common(struct i915_hw_ppgtt *ppgtt, uint64_t size) > return ret; > } > > + ppgtt->switch_mm = gen8_legacy_mm_switch; > trace_i915_page_directory_pointer_entry_alloc(&ppgtt->base, 0, 0, GEN8_PML4E_SHIFT); > } > > @@ -1295,6 +1339,7 @@ static int gen8_aliasing_ppgtt_init(struct i915_hw_ppgtt *ppgtt) > return ret; > } > > + /* FIXME: PML4 */ > gen8_for_each_pdpe(pd, pdp, start, size, temp, pdpe) > gen8_map_pagetable_range(&ppgtt->base, pd,start, size); > > @@ -1500,8 +1545,9 @@ static void gen8_ppgtt_enable(struct drm_device *dev) > int j; > > for_each_ring(ring, dev_priv, j) { > + u32 four_level = USES_FULL_48BIT_PPGTT(dev) ? GEN8_GFX_PPGTT_64B : 0; > I915_WRITE(RING_MODE_GEN7(ring), > - _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE)); > + _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE | four_level)); > } > } > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h > index 1477f54..1f4cdb1 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -38,7 +38,9 @@ struct drm_i915_file_private; > > typedef uint32_t gen6_gtt_pte_t; > typedef uint64_t gen8_gtt_pte_t; > -typedef gen8_gtt_pte_t gen8_ppgtt_pde_t; > +typedef gen8_gtt_pte_t gen8_ppgtt_pde_t; > +typedef gen8_ppgtt_pde_t gen8_ppgtt_pdpe_t; > +typedef gen8_ppgtt_pdpe_t gen8_ppgtt_pml4e_t; > > #define gtt_total_entries(gtt) ((gtt).base.total >> PAGE_SHIFT) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 1dc91de..305e5b7 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1338,6 +1338,7 @@ enum skl_disp_power_wells { > #define GFX_REPLAY_MODE (1<<11) > #define GFX_PSMI_GRANULARITY (1<<10) > #define GFX_PPGTT_ENABLE (1<<9) > +#define GEN8_GFX_PPGTT_64B (1<<7) > > #define VLV_DISPLAY_BASE 0x180000 > #define VLV_MIPI_BASE VLV_DISPLAY_BASE > -- > 2.1.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Mar 03, 2015 at 06:31:03PM +0530, akash goel wrote: > On Fri, Feb 20, 2015 at 11:16 PM, Michel Thierry > <michel.thierry@intel.com> wrote: > > +static void gen8_map_page_directory(struct i915_page_directory_pointer_entry *pdp, > > + struct i915_page_directory_entry *pd, > > + int index, > > + struct drm_device *dev) > > +{ > > + gen8_ppgtt_pdpe_t *page_directorypo; > > + gen8_ppgtt_pdpe_t pdpe; > > + > > + /* We do not need to clflush because no platform requiring flush > > + * supports 64b pagetables. */ > > Would be more appropriate to place this comment, either after the ‘if’ > condition or > at the end of the function (where clflush would have been placed, had > LLC not been there for platforms supporting 64 bit). > And same comment can be probably added, at the end of > gen8_map_page_directory_pointer function also. > > > + if (!USES_FULL_48BIT_PPGTT(dev)) > > + return; Ok this on a lot of levels: - a function calle map_something, which doesn't actually return a map. Must be renamed asap to something that makes sense, in the kernel everything call map_foo actually maps foo somewhere and returns where. - The comment is fairly useless since it doesn't mention which platforms flushing is required on. Either we need to split functions up more into 4G and 48bit variants if this difference is due to the pagetable layout. Or we need to replace it with appropriate platform checks. Or maybe this if is just dead code and should be remove entirely? -Daniel
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index edada33..fb06f67 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -192,6 +192,9 @@ static inline gen8_ppgtt_pde_t gen8_pde_encode(struct drm_device *dev, return pde; } +#define gen8_pdpe_encode gen8_pde_encode +#define gen8_pml4e_encode gen8_pde_encode + static gen6_gtt_pte_t snb_pte_encode(dma_addr_t addr, enum i915_cache_level level, bool valid, u32 unused) @@ -592,8 +595,8 @@ static int gen8_write_pdp(struct intel_engine_cs *ring, return 0; } -static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt, - struct intel_engine_cs *ring) +static int gen8_legacy_mm_switch(struct i915_hw_ppgtt *ppgtt, + struct intel_engine_cs *ring) { int i, ret; @@ -610,6 +613,12 @@ static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt, return 0; } +static int gen8_48b_mm_switch(struct i915_hw_ppgtt *ppgtt, + struct intel_engine_cs *ring) +{ + return gen8_write_pdp(ring, 0, ppgtt->pml4.daddr); +} + static void gen8_ppgtt_clear_range(struct i915_address_space *vm, uint64_t start, uint64_t length, @@ -753,6 +762,37 @@ static void gen8_map_pagetable_range(struct i915_address_space *vm, kunmap_atomic(page_directory); } +static void gen8_map_page_directory(struct i915_page_directory_pointer_entry *pdp, + struct i915_page_directory_entry *pd, + int index, + struct drm_device *dev) +{ + gen8_ppgtt_pdpe_t *page_directorypo; + gen8_ppgtt_pdpe_t pdpe; + + /* We do not need to clflush because no platform requiring flush + * supports 64b pagetables. */ + if (!USES_FULL_48BIT_PPGTT(dev)) + return; + + page_directorypo = kmap_atomic(pdp->page); + pdpe = gen8_pdpe_encode(dev, pd->daddr, I915_CACHE_LLC); + page_directorypo[index] = pdpe; + kunmap_atomic(page_directorypo); +} + +static void gen8_map_page_directory_pointer(struct i915_pml4 *pml4, + struct i915_page_directory_pointer_entry *pdp, + int index, + struct drm_device *dev) +{ + gen8_ppgtt_pml4e_t *pagemap = kmap_atomic(pml4->page); + gen8_ppgtt_pml4e_t pml4e = gen8_pml4e_encode(dev, pdp->daddr, I915_CACHE_LLC); + BUG_ON(!USES_FULL_48BIT_PPGTT(dev)); + pagemap[index] = pml4e; + kunmap_atomic(pagemap); +} + static void gen8_free_page_tables(struct i915_page_directory_entry *pd, struct drm_device *dev) { int i; @@ -1124,6 +1164,7 @@ static int gen8_alloc_va_range_3lvl(struct i915_address_space *vm, set_bit(pdpe, pdp->used_pdpes); gen8_map_pagetable_range(vm, pd, start, length); + gen8_map_page_directory(pdp, pd, pdpe, dev); } free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes); @@ -1192,6 +1233,8 @@ static int gen8_alloc_va_range_4lvl(struct i915_address_space *vm, ret = gen8_alloc_va_range_3lvl(vm, pdp, start, length); if (ret) goto err_out; + + gen8_map_page_directory_pointer(pml4, pdp, pml4e, vm->dev); } bitmap_or(pml4->used_pml4es, new_pdps, pml4->used_pml4es, @@ -1251,14 +1294,14 @@ static int gen8_ppgtt_init_common(struct i915_hw_ppgtt *ppgtt, uint64_t size) ppgtt->base.cleanup = gen8_ppgtt_cleanup; ppgtt->base.insert_entries = gen8_ppgtt_insert_entries; - ppgtt->switch_mm = gen8_mm_switch; - if (USES_FULL_48BIT_PPGTT(ppgtt->base.dev)) { int ret = pml4_init(ppgtt); if (ret) { unmap_and_free_pt(ppgtt->scratch_pd, ppgtt->base.dev); return ret; } + + ppgtt->switch_mm = gen8_48b_mm_switch; } else { int ret = __pdp_init(&ppgtt->pdp, false); if (ret) { @@ -1266,6 +1309,7 @@ static int gen8_ppgtt_init_common(struct i915_hw_ppgtt *ppgtt, uint64_t size) return ret; } + ppgtt->switch_mm = gen8_legacy_mm_switch; trace_i915_page_directory_pointer_entry_alloc(&ppgtt->base, 0, 0, GEN8_PML4E_SHIFT); } @@ -1295,6 +1339,7 @@ static int gen8_aliasing_ppgtt_init(struct i915_hw_ppgtt *ppgtt) return ret; } + /* FIXME: PML4 */ gen8_for_each_pdpe(pd, pdp, start, size, temp, pdpe) gen8_map_pagetable_range(&ppgtt->base, pd,start, size); @@ -1500,8 +1545,9 @@ static void gen8_ppgtt_enable(struct drm_device *dev) int j; for_each_ring(ring, dev_priv, j) { + u32 four_level = USES_FULL_48BIT_PPGTT(dev) ? GEN8_GFX_PPGTT_64B : 0; I915_WRITE(RING_MODE_GEN7(ring), - _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE)); + _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE | four_level)); } } diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 1477f54..1f4cdb1 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -38,7 +38,9 @@ struct drm_i915_file_private; typedef uint32_t gen6_gtt_pte_t; typedef uint64_t gen8_gtt_pte_t; -typedef gen8_gtt_pte_t gen8_ppgtt_pde_t; +typedef gen8_gtt_pte_t gen8_ppgtt_pde_t; +typedef gen8_ppgtt_pde_t gen8_ppgtt_pdpe_t; +typedef gen8_ppgtt_pdpe_t gen8_ppgtt_pml4e_t; #define gtt_total_entries(gtt) ((gtt).base.total >> PAGE_SHIFT) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 1dc91de..305e5b7 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1338,6 +1338,7 @@ enum skl_disp_power_wells { #define GFX_REPLAY_MODE (1<<11) #define GFX_PSMI_GRANULARITY (1<<10) #define GFX_PPGTT_ENABLE (1<<9) +#define GEN8_GFX_PPGTT_64B (1<<7) #define VLV_DISPLAY_BASE 0x180000 #define VLV_MIPI_BASE VLV_DISPLAY_BASE