[Intel-gfx] [PATCH 01/12] drm/i915/bdw: Make pdp allocation more dynamic
Michel Thierry
michel.thierry at intel.com
Wed Mar 18 03:15:51 PDT 2015
On 3/3/2015 11:48 AM, akash goel wrote:
> On Fri, Feb 20, 2015 at 11:15 PM, Michel Thierry
> <michel.thierry at intel.com> wrote:
>> From: Ben Widawsky<benjamin.widawsky at intel.com>
>>
>> This transitional patch doesn't do much for the existing code. However,
>> it should make upcoming patches to use the full 48b address space a bit
>> easier to swallow. The patch also introduces the PML4, ie. the new top
>> level structure of the page tables.
>>
>> v2: Renamed pdp_free to be similar to pd/pt (unmap_and_free_pdp),
>> To facilitate testing, 48b mode will be available on Broadwell, when
>> i915.enable_ppgtt = 3.
>>
>> Signed-off-by: Ben Widawsky<ben at bwidawsk.net>
>> Signed-off-by: Michel Thierry<michel.thierry at intel.com> (v2)
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 7 ++-
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 108 +++++++++++++++++++++++++++++-------
>> drivers/gpu/drm/i915/i915_gem_gtt.h | 41 +++++++++++---
>> 3 files changed, 126 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 2dedd43..af0d149 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2432,7 +2432,12 @@ 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)
>> +#ifdef CONFIG_64BIT
>> +# define USES_FULL_48BIT_PPGTT(dev) (i915.enable_ppgtt == 3)
>> +#else
>> +# define USES_FULL_48BIT_PPGTT(dev) false
>> +#endif
>>
>> #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 ff86501..489f8db 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -100,10 +100,17 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
>> {
>> bool has_aliasing_ppgtt;
>> bool has_full_ppgtt;
>> + bool has_full_64bit_ppgtt;
>>
>> has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6;
>> has_full_ppgtt = INTEL_INFO(dev)->gen >= 7;
>>
>> +#ifdef CONFIG_64BIT
>> + has_full_64bit_ppgtt = IS_BROADWELL(dev) && false; /* FIXME: 64b */
>> +#else
>> + has_full_64bit_ppgtt = false;
>> +#endif
>> +
>> if (intel_vgpu_active(dev))
>> has_full_ppgtt = false; /* emulation is too hard */
>>
>> @@ -121,6 +128,9 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
>> if (enable_ppgtt == 2 && has_full_ppgtt)
>> return 2;
>>
>> + if (enable_ppgtt == 3 && has_full_64bit_ppgtt)
>> + return 3;
>> +
>> #ifdef CONFIG_INTEL_IOMMU
>> /* Disable ppgtt on SNB if VT-d is on. */
>> if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped) {
>> @@ -462,6 +472,45 @@ free_pd:
>> return ERR_PTR(ret);
>> }
>>
>> +static void __pdp_fini(struct i915_page_directory_pointer_entry *pdp)
>> +{
>> + kfree(pdp->used_pdpes);
>> + kfree(pdp->page_directory);
>> + /* HACK */
>> + pdp->page_directory = NULL;
>> +}
>> +
>> +static void unmap_and_free_pdp(struct i915_page_directory_pointer_entry *pdp,
>> + struct drm_device *dev)
>> +{
>> + __pdp_fini(pdp);
>> + if (USES_FULL_48BIT_PPGTT(dev))
>> + kfree(pdp);
>> +}
>> +
>> +static int __pdp_init(struct i915_page_directory_pointer_entry *pdp,
>> + struct drm_device *dev)
>> +{
>> + size_t pdpes = I915_PDPES_PER_PDP(dev);
>> +
>> + pdp->used_pdpes = kcalloc(BITS_TO_LONGS(pdpes),
>> + sizeof(unsigned long),
>> + GFP_KERNEL);
>> + if (!pdp->used_pdpes)
>> + return -ENOMEM;
>> +
>> + pdp->page_directory = kcalloc(pdpes, sizeof(*pdp->page_directory), GFP_KERNEL);
>> + if (!pdp->page_directory) {
>> + kfree(pdp->used_pdpes);
>> + /* the PDP might be the statically allocated top level. Keep it
>> + * as clean as possible */
>> + pdp->used_pdpes = NULL;
>> + return -ENOMEM;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> /* Broadwell Page Directory Pointer Descriptors */
>> static int gen8_write_pdp(struct intel_engine_cs *ring,
>> unsigned entry,
>> @@ -491,7 +540,7 @@ static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt,
>> {
>> int i, ret;
>>
>> - for (i = GEN8_LEGACY_PDPES - 1; i >= 0; i--) {
>> + for (i = 3; i >= 0; i--) {
>> struct i915_page_directory_entry *pd = ppgtt->pdp.page_directory[i];
>> dma_addr_t pd_daddr = pd ? pd->daddr : ppgtt->scratch_pd->daddr;
>> /* The page directory might be NULL, but we need to clear out
>> @@ -580,9 +629,6 @@ static void gen8_ppgtt_insert_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;
>> -
>> if (pt_vaddr == NULL) {
>> struct i915_page_directory_entry *pd = ppgtt->pdp.page_directory[pdpe];
>> struct i915_page_table_entry *pt = pd->page_tables[pde];
>> @@ -664,7 +710,8 @@ static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt)
>> struct pci_dev *hwdev = ppgtt->base.dev->pdev;
>> int i, j;
>>
>> - for_each_set_bit(i, ppgtt->pdp.used_pdpes, GEN8_LEGACY_PDPES) {
>> + for_each_set_bit(i, ppgtt->pdp.used_pdpes,
>> + I915_PDPES_PER_PDP(ppgtt->base.dev)) {
>> struct i915_page_directory_entry *pd;
>>
>> if (WARN_ON(!ppgtt->pdp.page_directory[i]))
>> @@ -696,13 +743,15 @@ static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt)
>> {
>> int i;
>>
>> - for_each_set_bit(i, ppgtt->pdp.used_pdpes, GEN8_LEGACY_PDPES) {
>> + for_each_set_bit(i, ppgtt->pdp.used_pdpes,
>> + I915_PDPES_PER_PDP(ppgtt->base.dev)) {
>> if (WARN_ON(!ppgtt->pdp.page_directory[i]))
>> continue;
>>
>> gen8_free_page_tables(ppgtt->pdp.page_directory[i], ppgtt->base.dev);
>> unmap_and_free_pd(ppgtt->pdp.page_directory[i], ppgtt->base.dev);
>> }
>> + unmap_and_free_pdp(&ppgtt->pdp, ppgtt->base.dev);
> 'ppgtt->scratch_pd' is not being de-allocated.
>
> Probably it can be de-allocated explicitly from here, after the call
> to unmap_and_free_pdp or
> from the gen8_ppgtt_cleanup function .
Right, unmap_and_free_pt(ppgtt->scratch_pd, ppgtt->base.dev) was
missing. It was added in "drm/i915/bdw: Update pdp switch and point
unused PDPs to scratch page" v3. It'll still be there in the rebased
version of this patch.
>> }
>>
>> static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
>> @@ -799,8 +848,9 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
>> struct i915_page_directory_entry *pd;
>> uint64_t temp;
>> uint32_t pdpe;
>> + size_t pdpes = I915_PDPES_PER_PDP(ppgtt->base.dev);
>>
>> - BUG_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
>> + BUG_ON(!bitmap_empty(new_pds, pdpes));
>>
>> /* FIXME: PPGTT container_of won't work for 64b */
>> BUG_ON((start + length) > 0x800000000ULL);
>> @@ -820,18 +870,19 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
>> return 0;
>>
>> unwind_out:
>> - for_each_set_bit(pdpe, new_pds, GEN8_LEGACY_PDPES)
>> + for_each_set_bit(pdpe, new_pds, pdpes)
>> unmap_and_free_pd(pdp->page_directory[pdpe], ppgtt->base.dev);
>>
>> return -ENOMEM;
>> }
>>
>> static inline void
>> -free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned long **new_pts)
>> +free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned long **new_pts,
>> + size_t pdpes)
>> {
>> int i;
>>
>> - for (i = 0; i < GEN8_LEGACY_PDPES; i++)
>> + for (i = 0; i < pdpes; i++)
>> kfree(new_pts[i]);
>> kfree(new_pts);
>> kfree(new_pds);
>> @@ -841,13 +892,14 @@ free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned long **new_pts)
>> * of these are based on the number of PDPEs in the system.
>> */
>> int __must_check alloc_gen8_temp_bitmaps(unsigned long **new_pds,
>> - unsigned long ***new_pts)
>> + unsigned long ***new_pts,
>> + size_t pdpes)
>> {
>> int i;
>> unsigned long *pds;
>> unsigned long **pts;
>>
>> - pds = kcalloc(BITS_TO_LONGS(GEN8_LEGACY_PDPES), sizeof(unsigned long), GFP_KERNEL);
>> + pds = kcalloc(BITS_TO_LONGS(pdpes), sizeof(unsigned long), GFP_KERNEL);
>> if (!pds)
>> return -ENOMEM;
>>
>> @@ -857,7 +909,7 @@ int __must_check alloc_gen8_temp_bitmaps(unsigned long **new_pds,
>> return -ENOMEM;
>> }
>>
>> - for (i = 0; i < GEN8_LEGACY_PDPES; i++) {
>> + for (i = 0; i < pdpes; i++) {
>> pts[i] = kcalloc(BITS_TO_LONGS(GEN8_PDES_PER_PAGE),
>> sizeof(unsigned long), GFP_KERNEL);
>> if (!pts[i])
>> @@ -870,7 +922,7 @@ int __must_check alloc_gen8_temp_bitmaps(unsigned long **new_pds,
>> return 0;
>>
>> err_out:
>> - free_gen8_temp_bitmaps(pds, pts);
>> + free_gen8_temp_bitmaps(pds, pts, pdpes);
>> return -ENOMEM;
>> }
>>
>> @@ -886,6 +938,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
>> const uint64_t orig_length = length;
>> uint64_t temp;
>> uint32_t pdpe;
>> + size_t pdpes = I915_PDPES_PER_PDP(dev);
>> int ret;
>>
>> #ifndef CONFIG_64BIT
>> @@ -903,7 +956,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
>> if (WARN_ON(start + length < start))
>> return -ERANGE;
>>
>> - ret = alloc_gen8_temp_bitmaps(&new_page_dirs, &new_page_tables);
>> + ret = alloc_gen8_temp_bitmaps(&new_page_dirs, &new_page_tables, pdpes);
>> if (ret)
>> return ret;
>>
>> @@ -911,7 +964,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
>> ret = gen8_ppgtt_alloc_page_directories(ppgtt, &ppgtt->pdp, start, length,
>> new_page_dirs);
>> if (ret) {
>> - free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
>> + free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
>> return ret;
>> }
>>
>> @@ -968,7 +1021,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
>> set_bit(pdpe, ppgtt->pdp.used_pdpes);
>> }
>>
>> - free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
>> + free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
>> return 0;
>>
>> err_out:
>> @@ -977,13 +1030,19 @@ err_out:
>> unmap_and_free_pt(pd->page_tables[temp], vm->dev);
>> }
>>
>> - for_each_set_bit(pdpe, new_page_dirs, GEN8_LEGACY_PDPES)
>> + for_each_set_bit(pdpe, new_page_dirs, pdpes)
>> unmap_and_free_pd(ppgtt->pdp.page_directory[pdpe], vm->dev);
>>
>> - free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
>> + free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
>> return ret;
>> }
>>
>> +static void gen8_ppgtt_fini_common(struct i915_hw_ppgtt *ppgtt)
>> +{
>> + unmap_and_free_pt(ppgtt->scratch_pd, ppgtt->base.dev);
>> + unmap_and_free_pdp(&ppgtt->pdp, ppgtt->base.dev);
>> +}
>> +
>> /**
>> * GEN8 legacy ppgtt programming is accomplished through a max 4 PDP registers
>> * with a net effect resembling a 2-level page table in normal x86 terms. Each
>> @@ -1004,6 +1063,15 @@ static int gen8_ppgtt_init_common(struct i915_hw_ppgtt *ppgtt, uint64_t size)
>>
>> ppgtt->switch_mm = gen8_mm_switch;
>>
>> + if (!USES_FULL_48BIT_PPGTT(ppgtt->base.dev)) {
>> + int ret = __pdp_init(&ppgtt->pdp, false);
>> + if (ret) {
>> + unmap_and_free_pt(ppgtt->scratch_pd, ppgtt->base.dev);
>> + return ret;
>> + }
>> + } else
>> + return -EPERM; /* Not yet implemented */
>> +
>> return 0;
>> }
>>
>> @@ -1025,7 +1093,7 @@ static int gen8_aliasing_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>> * eventually. */
>> ret = gen8_alloc_va_range(&ppgtt->base, start, size);
>> if (ret) {
>> - unmap_and_free_pt(ppgtt->scratch_pd, ppgtt->base.dev);
>> + gen8_ppgtt_fini_common(ppgtt);
>> return ret;
>> }
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
>> index c68ec3a..a33c6e9 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
>> @@ -85,8 +85,12 @@ typedef gen8_gtt_pte_t gen8_ppgtt_pde_t;
>> * The difference as compared to normal x86 3 level page table is the PDPEs are
>> * programmed via register.
>> */
>> +#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 32b page
>> + * tables */
>> +#define GEN8_PDPE_MASK 0x1ff
>> #define GEN8_PDE_SHIFT 21
>> #define GEN8_PDE_MASK 0x1ff
>> #define GEN8_PTE_SHIFT 12
>> @@ -95,6 +99,13 @@ typedef gen8_gtt_pte_t gen8_ppgtt_pde_t;
>> #define GEN8_PTES_PER_PAGE (PAGE_SIZE / sizeof(gen8_gtt_pte_t))
>> #define GEN8_PDES_PER_PAGE (PAGE_SIZE / sizeof(gen8_ppgtt_pde_t))
>>
>> +#ifdef CONFIG_64BIT
>> +# define I915_PDPES_PER_PDP(dev) (USES_FULL_48BIT_PPGTT(dev) ?\
>> + GEN8_PML4ES_PER_PML4 : GEN8_LEGACY_PDPES)
>> +#else
>> +# define I915_PDPES_PER_PDP GEN8_LEGACY_PDPES
>> +#endif
>> +
>> #define PPAT_UNCACHED_INDEX (_PAGE_PWT | _PAGE_PCD)
>> #define PPAT_CACHED_PDE_INDEX 0 /* WB LLC */
>> #define PPAT_CACHED_INDEX _PAGE_PAT /* WB LLCeLLC */
>> @@ -210,9 +221,17 @@ struct i915_page_directory_entry {
>> };
>>
>> struct i915_page_directory_pointer_entry {
>> - /* struct page *page; */
>> - DECLARE_BITMAP(used_pdpes, GEN8_LEGACY_PDPES);
>> - struct i915_page_directory_entry *page_directory[GEN8_LEGACY_PDPES];
>> + struct page *page;
>> + dma_addr_t daddr;
>> + unsigned long *used_pdpes;
>> + struct i915_page_directory_entry **page_directory;
>> +};
>> +
>> +struct i915_pml4 {
>> + struct page *page;
>> + dma_addr_t daddr;
>> + DECLARE_BITMAP(used_pml4es, GEN8_PML4ES_PER_PML4);
>> + struct i915_page_directory_pointer_entry *pdps[GEN8_PML4ES_PER_PML4];
>> };
>>
>> struct i915_address_space {
>> @@ -302,8 +321,9 @@ struct i915_hw_ppgtt {
>> struct drm_mm_node node;
>> unsigned long pd_dirty_rings;
>> union {
>> - struct i915_page_directory_pointer_entry pdp;
>> - struct i915_page_directory_entry pd;
>> + struct i915_pml4 pml4; /* GEN8+ & 64b PPGTT */
>> + struct i915_page_directory_pointer_entry pdp; /* GEN8+ */
>> + struct i915_page_directory_entry pd; /* GEN6-7 */
>> };
>>
>> union {
>> @@ -399,14 +419,17 @@ static inline uint32_t gen6_pde_index(uint32_t addr)
>> temp = min(temp, length), \
>> start += temp, length -= temp)
>>
>> -#define gen8_for_each_pdpe(pd, pdp, start, length, temp, iter) \
>> - for (iter = gen8_pdpe_index(start), pd = (pdp)->page_directory[iter]; \
>> - length > 0 && iter < GEN8_LEGACY_PDPES; \
>> +#define gen8_for_each_pdpe_e(pd, pdp, start, length, temp, iter, b) \
>> + for (iter = gen8_pdpe_index(start), pd = (pdp)->page_directory[iter]; \
>> + length > 0 && (iter < b); \
>> pd = (pdp)->page_directory[++iter], \
>> temp = ALIGN(start+1, 1 << GEN8_PDPE_SHIFT) - start, \
>> temp = min(temp, length), \
>> start += temp, length -= temp)
>>
>> +#define gen8_for_each_pdpe(pd, pdp, start, length, temp, iter) \
>> + gen8_for_each_pdpe_e(pd, pdp, start, length, temp, iter, I915_PDPES_PER_PDP(dev))
>> +
>> /* Clamp length to the next page_directory boundary */
>> static inline uint64_t gen8_clamp_pd(uint64_t start, uint64_t length)
>> {
>> --
>> 2.1.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list