[Intel-gfx] [PATCH 2/2] drm/i915/gt: Re-do the intel-gtt split
Lucas De Marchi
lucas.demarchi at intel.com
Fri Jun 17 21:38:58 UTC 2022
On Fri, Jun 17, 2022 at 12:15:51PM -0700, Matt Roper wrote:
>On Thu, Jun 16, 2022 at 03:49:43PM -0700, Lucas De Marchi wrote:
>> Re-do what was attempted in commit 7a5c922377b4 ("drm/i915/gt: Split
>> intel-gtt functions by arch"). The goal of that commit was to split the
>> handlers for older hardware that depend on intel-gtt.ko so i915 can
>> be built for non-x86 archs, after some more patches. Other archs do not
>> need intel-gtt.ko.
>>
>> Main issue with the previous approach: it moved all the hooks, including
>> the gen8, which is used by all platforms gen8 and newer. Re-do the
>> split moving only the handlers for gen < 6, which are the only ones
>> calling out to the separate module.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>> drivers/gpu/drm/i915/Makefile | 2 +-
>> drivers/gpu/drm/i915/gt/intel_ggtt.c | 559 +++++++++++++++++-
>> drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c | 132 +++++
>> drivers/gpu/drm/i915/gt/intel_ggtt_gmch.h | 27 +
>> drivers/gpu/drm/i915/gt/intel_gt.c | 5 +-
>> drivers/gpu/drm/i915/gt/intel_gt.h | 9 -
>> drivers/gpu/drm/i915/gt/intel_gt_gmch.c | 654 ----------------------
>> drivers/gpu/drm/i915/gt/intel_gt_gmch.h | 46 --
>> drivers/gpu/drm/i915/gt/intel_gtt.h | 12 +-
>> 9 files changed, 713 insertions(+), 733 deletions(-)
>> create mode 100644 drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c
>> create mode 100644 drivers/gpu/drm/i915/gt/intel_ggtt_gmch.h
>> delete mode 100644 drivers/gpu/drm/i915/gt/intel_gt_gmch.c
>> delete mode 100644 drivers/gpu/drm/i915/gt/intel_gt_gmch.h
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index d2b18f03a33c..4166cd76997e 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -129,7 +129,7 @@ gt-y += \
>> gt/shmem_utils.o \
>> gt/sysfs_engines.o
>> # x86 intel-gtt module support
>> -gt-$(CONFIG_X86) += gt/intel_gt_gmch.o
>> +gt-$(CONFIG_X86) += gt/intel_ggtt_gmch.o
>> # autogenerated null render state
>> gt-y += \
>> gt/gen6_renderstate.o \
>> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
>> index e6b2eb122ad7..a83d6858b766 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
>> @@ -3,16 +3,18 @@
>> * Copyright © 2020 Intel Corporation
>> */
>>
>> -#include <linux/types.h>
>> #include <asm/set_memory.h>
>> #include <asm/smp.h>
>> +#include <linux/types.h>
>> +#include <linux/stop_machine.h>
>>
>> #include <drm/i915_drm.h>
>> +#include <drm/intel-gtt.h>
>>
>> #include "gem/i915_gem_lmem.h"
>>
>> +#include "intel_ggtt_gmch.h"
>> #include "intel_gt.h"
>> -#include "intel_gt_gmch.h"
>> #include "intel_gt_regs.h"
>> #include "i915_drv.h"
>> #include "i915_scatterlist.h"
>> @@ -181,7 +183,7 @@ void gen6_ggtt_invalidate(struct i915_ggtt *ggtt)
>> spin_unlock_irq(&uncore->lock);
>> }
>>
>> -void gen8_ggtt_invalidate(struct i915_ggtt *ggtt)
>> +static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt)
>> {
>> struct intel_uncore *uncore = ggtt->vm.gt->uncore;
>>
>> @@ -218,11 +220,232 @@ u64 gen8_ggtt_pte_encode(dma_addr_t addr,
>> return pte;
>> }
>>
>> +static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte)
>> +{
>> + writeq(pte, addr);
>> +}
>> +
>> +static void gen8_ggtt_insert_page(struct i915_address_space *vm,
>> + dma_addr_t addr,
>> + u64 offset,
>> + enum i915_cache_level level,
>> + u32 flags)
>> +{
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> + gen8_pte_t __iomem *pte =
>> + (gen8_pte_t __iomem *)ggtt->gsm + offset / I915_GTT_PAGE_SIZE;
>> +
>> + gen8_set_pte(pte, gen8_ggtt_pte_encode(addr, level, flags));
>> +
>> + ggtt->invalidate(ggtt);
>> +}
>> +
>> +static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
>> + struct i915_vma_resource *vma_res,
>> + enum i915_cache_level level,
>> + u32 flags)
>> +{
>> + const gen8_pte_t pte_encode = gen8_ggtt_pte_encode(0, level, flags);
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> + gen8_pte_t __iomem *gte;
>> + gen8_pte_t __iomem *end;
>> + struct sgt_iter iter;
>> + dma_addr_t addr;
>> +
>> + /*
>> + * Note that we ignore PTE_READ_ONLY here. The caller must be careful
>> + * not to allow the user to override access to a read only page.
>> + */
>> +
>> + gte = (gen8_pte_t __iomem *)ggtt->gsm;
>> + gte += vma_res->start / I915_GTT_PAGE_SIZE;
>> + end = gte + vma_res->node_size / I915_GTT_PAGE_SIZE;
>> +
>> + for_each_sgt_daddr(addr, iter, vma_res->bi.pages)
>> + gen8_set_pte(gte++, pte_encode | addr);
>> + GEM_BUG_ON(gte > end);
>> +
>> + /* Fill the allocated but "unused" space beyond the end of the buffer */
>> + while (gte < end)
>> + gen8_set_pte(gte++, vm->scratch[0]->encode);
>> +
>> + /*
>> + * We want to flush the TLBs only after we're certain all the PTE
>> + * updates have finished.
>> + */
>> + ggtt->invalidate(ggtt);
>> +}
>> +
>> +static void gen6_ggtt_insert_page(struct i915_address_space *vm,
>> + dma_addr_t addr,
>> + u64 offset,
>> + enum i915_cache_level level,
>> + u32 flags)
>> +{
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> + gen6_pte_t __iomem *pte =
>> + (gen6_pte_t __iomem *)ggtt->gsm + offset / I915_GTT_PAGE_SIZE;
>> +
>> + iowrite32(vm->pte_encode(addr, level, flags), pte);
>> +
>> + ggtt->invalidate(ggtt);
>> +}
>> +
>> +/*
>> + * Binds an object into the global gtt with the specified cache level.
>> + * The object will be accessible to the GPU via commands whose operands
>> + * reference offsets within the global GTT as well as accessible by the GPU
>> + * through the GMADR mapped BAR (i915->mm.gtt->gtt).
>> + */
>> +static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
>> + struct i915_vma_resource *vma_res,
>> + enum i915_cache_level level,
>> + u32 flags)
>> +{
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> + gen6_pte_t __iomem *gte;
>> + gen6_pte_t __iomem *end;
>> + struct sgt_iter iter;
>> + dma_addr_t addr;
>> +
>> + gte = (gen6_pte_t __iomem *)ggtt->gsm;
>> + gte += vma_res->start / I915_GTT_PAGE_SIZE;
>> + end = gte + vma_res->node_size / I915_GTT_PAGE_SIZE;
>> +
>> + for_each_sgt_daddr(addr, iter, vma_res->bi.pages)
>> + iowrite32(vm->pte_encode(addr, level, flags), gte++);
>> + GEM_BUG_ON(gte > end);
>> +
>> + /* Fill the allocated but "unused" space beyond the end of the buffer */
>> + while (gte < end)
>> + iowrite32(vm->scratch[0]->encode, gte++);
>> +
>> + /*
>> + * We want to flush the TLBs only after we're certain all the PTE
>> + * updates have finished.
>> + */
>> + ggtt->invalidate(ggtt);
>> +}
>> +
>> +static void nop_clear_range(struct i915_address_space *vm,
>> + u64 start, u64 length)
>> +{
>> +}
>> +
>> +static void gen8_ggtt_clear_range(struct i915_address_space *vm,
>> + u64 start, u64 length)
>> +{
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> + unsigned int first_entry = start / I915_GTT_PAGE_SIZE;
>> + unsigned int num_entries = length / I915_GTT_PAGE_SIZE;
>> + const gen8_pte_t scratch_pte = vm->scratch[0]->encode;
>> + gen8_pte_t __iomem *gtt_base =
>> + (gen8_pte_t __iomem *)ggtt->gsm + first_entry;
>> + const int max_entries = ggtt_total_entries(ggtt) - first_entry;
>> + int i;
>> +
>> + if (WARN(num_entries > max_entries,
>> + "First entry = %d; Num entries = %d (max=%d)\n",
>> + first_entry, num_entries, max_entries))
>> + num_entries = max_entries;
>> +
>> + for (i = 0; i < num_entries; i++)
>> + gen8_set_pte(>t_base[i], scratch_pte);
>> +}
>> +
>> +static void bxt_vtd_ggtt_wa(struct i915_address_space *vm)
>> +{
>> + /*
>> + * Make sure the internal GAM fifo has been cleared of all GTT
>> + * writes before exiting stop_machine(). This guarantees that
>> + * any aperture accesses waiting to start in another process
>> + * cannot back up behind the GTT writes causing a hang.
>> + * The register can be any arbitrary GAM register.
>> + */
>> + intel_uncore_posting_read_fw(vm->gt->uncore, GFX_FLSH_CNTL_GEN6);
>> +}
>> +
>> +struct insert_page {
>> + struct i915_address_space *vm;
>> + dma_addr_t addr;
>> + u64 offset;
>> + enum i915_cache_level level;
>> +};
>> +
>> +static int bxt_vtd_ggtt_insert_page__cb(void *_arg)
>> +{
>> + struct insert_page *arg = _arg;
>> +
>> + gen8_ggtt_insert_page(arg->vm, arg->addr, arg->offset, arg->level, 0);
>> + bxt_vtd_ggtt_wa(arg->vm);
>> +
>> + return 0;
>> +}
>> +
>> +static void bxt_vtd_ggtt_insert_page__BKL(struct i915_address_space *vm,
>> + dma_addr_t addr,
>> + u64 offset,
>> + enum i915_cache_level level,
>> + u32 unused)
>> +{
>> + struct insert_page arg = { vm, addr, offset, level };
>> +
>> + stop_machine(bxt_vtd_ggtt_insert_page__cb, &arg, NULL);
>> +}
>> +
>> +struct insert_entries {
>> + struct i915_address_space *vm;
>> + struct i915_vma_resource *vma_res;
>> + enum i915_cache_level level;
>> + u32 flags;
>> +};
>> +
>> +static int bxt_vtd_ggtt_insert_entries__cb(void *_arg)
>> +{
>> + struct insert_entries *arg = _arg;
>> +
>> + gen8_ggtt_insert_entries(arg->vm, arg->vma_res, arg->level, arg->flags);
>> + bxt_vtd_ggtt_wa(arg->vm);
>> +
>> + return 0;
>> +}
>> +
>> +static void bxt_vtd_ggtt_insert_entries__BKL(struct i915_address_space *vm,
>> + struct i915_vma_resource *vma_res,
>> + enum i915_cache_level level,
>> + u32 flags)
>> +{
>> + struct insert_entries arg = { vm, vma_res, level, flags };
>> +
>> + stop_machine(bxt_vtd_ggtt_insert_entries__cb, &arg, NULL);
>> +}
>> +
>> +static void gen6_ggtt_clear_range(struct i915_address_space *vm,
>> + u64 start, u64 length)
>> +{
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> + unsigned int first_entry = start / I915_GTT_PAGE_SIZE;
>> + unsigned int num_entries = length / I915_GTT_PAGE_SIZE;
>> + gen6_pte_t scratch_pte, __iomem *gtt_base =
>> + (gen6_pte_t __iomem *)ggtt->gsm + first_entry;
>> + const int max_entries = ggtt_total_entries(ggtt) - first_entry;
>> + int i;
>> +
>> + if (WARN(num_entries > max_entries,
>> + "First entry = %d; Num entries = %d (max=%d)\n",
>> + first_entry, num_entries, max_entries))
>> + num_entries = max_entries;
>> +
>> + scratch_pte = vm->scratch[0]->encode;
>> + for (i = 0; i < num_entries; i++)
>> + iowrite32(scratch_pte, >t_base[i]);
>> +}
>> +
>> void intel_ggtt_bind_vma(struct i915_address_space *vm,
>> - struct i915_vm_pt_stash *stash,
>> - struct i915_vma_resource *vma_res,
>> - enum i915_cache_level cache_level,
>> - u32 flags)
>> + struct i915_vm_pt_stash *stash,
>> + struct i915_vma_resource *vma_res,
>> + enum i915_cache_level cache_level,
>> + u32 flags)
>> {
>> u32 pte_flags;
>>
>> @@ -243,7 +466,7 @@ void intel_ggtt_bind_vma(struct i915_address_space *vm,
>> }
>>
>> void intel_ggtt_unbind_vma(struct i915_address_space *vm,
>> - struct i915_vma_resource *vma_res)
>> + struct i915_vma_resource *vma_res)
>> {
>> vm->clear_range(vm, vma_res->start, vma_res->vma_size);
>> }
>> @@ -560,12 +783,317 @@ void i915_ggtt_driver_late_release(struct drm_i915_private *i915)
>> dma_resv_fini(&ggtt->vm._resv);
>> }
>>
>> -struct resource intel_pci_resource(struct pci_dev *pdev, int bar)
>> +static unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl)
>> +{
>> + snb_gmch_ctl >>= SNB_GMCH_GGMS_SHIFT;
>> + snb_gmch_ctl &= SNB_GMCH_GGMS_MASK;
>> + return snb_gmch_ctl << 20;
>> +}
>> +
>> +static unsigned int gen8_get_total_gtt_size(u16 bdw_gmch_ctl)
>> +{
>> + bdw_gmch_ctl >>= BDW_GMCH_GGMS_SHIFT;
>> + bdw_gmch_ctl &= BDW_GMCH_GGMS_MASK;
>> + if (bdw_gmch_ctl)
>> + bdw_gmch_ctl = 1 << bdw_gmch_ctl;
>> +
>> +#ifdef CONFIG_X86_32
>> + /* Limit 32b platforms to a 2GB GGTT: 4 << 20 / pte size * I915_GTT_PAGE_SIZE */
>> + if (bdw_gmch_ctl > 4)
>> + bdw_gmch_ctl = 4;
>> +#endif
>> +
>> + return bdw_gmch_ctl << 20;
>> +}
>> +
>> +static unsigned int chv_get_total_gtt_size(u16 gmch_ctrl)
>> +{
>> + gmch_ctrl >>= SNB_GMCH_GGMS_SHIFT;
>> + gmch_ctrl &= SNB_GMCH_GGMS_MASK;
>> +
>> + if (gmch_ctrl)
>> + return 1 << (20 + gmch_ctrl);
>> +
>> + return 0;
>> +}
>> +
>> +static unsigned int gen6_gttmmadr_size(struct drm_i915_private *i915)
>> +{
>> + /*
>> + * GEN6: GTTMMADR size is 4MB and GTTADR starts at 2MB offset
>> + * GEN8: GTTMMADR size is 16MB and GTTADR starts at 8MB offset
>> + */
>> + GEM_BUG_ON(GRAPHICS_VER(i915) < 6);
>> + return (GRAPHICS_VER(i915) < 8) ? SZ_4M : SZ_16M;
>> +}
>> +
>> +static unsigned int gen6_gttadr_offset(struct drm_i915_private *i915)
>> +{
>> + return gen6_gttmmadr_size(i915) / 2;
>> +}
>> +
>> +static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
>> +{
>> + struct drm_i915_private *i915 = ggtt->vm.i915;
>> + struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
>> + phys_addr_t phys_addr;
>> + u32 pte_flags;
>> + int ret;
>> +
>> + GEM_WARN_ON(pci_resource_len(pdev, 0) != gen6_gttmmadr_size(i915));
>> + phys_addr = pci_resource_start(pdev, 0) + gen6_gttadr_offset(i915);
>> +
>> + /*
>> + * On BXT+/ICL+ writes larger than 64 bit to the GTT pagetable range
>> + * will be dropped. For WC mappings in general we have 64 byte burst
>> + * writes when the WC buffer is flushed, so we can't use it, but have to
>> + * resort to an uncached mapping. The WC issue is easily caught by the
>> + * readback check when writing GTT PTE entries.
>> + */
>> + if (IS_GEN9_LP(i915) || GRAPHICS_VER(i915) >= 11)
>> + ggtt->gsm = ioremap(phys_addr, size);
>> + else
>> + ggtt->gsm = ioremap_wc(phys_addr, size);
>> + if (!ggtt->gsm) {
>> + drm_err(&i915->drm, "Failed to map the ggtt page table\n");
>> + return -ENOMEM;
>> + }
>> +
>> + kref_init(&ggtt->vm.resv_ref);
>> + ret = setup_scratch_page(&ggtt->vm);
>> + if (ret) {
>> + drm_err(&i915->drm, "Scratch setup failed\n");
>> + /* iounmap will also get called at remove, but meh */
>> + iounmap(ggtt->gsm);
>> + return ret;
>> + }
>> +
>> + pte_flags = 0;
>> + if (i915_gem_object_is_lmem(ggtt->vm.scratch[0]))
>> + pte_flags |= PTE_LM;
>> +
>> + ggtt->vm.scratch[0]->encode =
>> + ggtt->vm.pte_encode(px_dma(ggtt->vm.scratch[0]),
>> + I915_CACHE_NONE, pte_flags);
>> +
>> + return 0;
>> +}
>> +
>> +static void gen6_gmch_remove(struct i915_address_space *vm)
>> +{
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> +
>> + iounmap(ggtt->gsm);
>> + free_scratch(vm);
>> +}
>> +
>> +static struct resource pci_resource(struct pci_dev *pdev, int bar)
>> {
>> return (struct resource)DEFINE_RES_MEM(pci_resource_start(pdev, bar),
>> pci_resource_len(pdev, bar));
>> }
>>
>> +static int gen8_gmch_probe(struct i915_ggtt *ggtt)
>> +{
>> + struct drm_i915_private *i915 = ggtt->vm.i915;
>> + struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
>> + unsigned int size;
>> + u16 snb_gmch_ctl;
>> +
>> + /* TODO: We're not aware of mappable constraints on gen8 yet */
>
>We could probably take the opportunity to drop this TODO while moving
>the function. Given how old gen8 is now I think it's safe to say that
>there aren't any extra constraints we haven't found out about yet. :-)
>
>> + if (!HAS_LMEM(i915)) {
>> + ggtt->gmadr = pci_resource(pdev, 2);
>> + ggtt->mappable_end = resource_size(&ggtt->gmadr);
>> + }
>> +
>> + pci_read_config_word(pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
>> + if (IS_CHERRYVIEW(i915))
>> + size = chv_get_total_gtt_size(snb_gmch_ctl);
>> + else
>> + size = gen8_get_total_gtt_size(snb_gmch_ctl);
>> +
>> + ggtt->vm.alloc_pt_dma = alloc_pt_dma;
>> + ggtt->vm.alloc_scratch_dma = alloc_pt_dma;
>> + ggtt->vm.lmem_pt_obj_flags = I915_BO_ALLOC_PM_EARLY;
>> +
>> + ggtt->vm.total = (size / sizeof(gen8_pte_t)) * I915_GTT_PAGE_SIZE;
>> + ggtt->vm.cleanup = gen6_gmch_remove;
>> + ggtt->vm.insert_page = gen8_ggtt_insert_page;
>> + ggtt->vm.clear_range = nop_clear_range;
>> + if (intel_scanout_needs_vtd_wa(i915))
>> + ggtt->vm.clear_range = gen8_ggtt_clear_range;
>> +
>> + ggtt->vm.insert_entries = gen8_ggtt_insert_entries;
>> +
>> + /*
>> + * Serialize GTT updates with aperture access on BXT if VT-d is on,
>> + * and always on CHV.
>> + */
>> + if (intel_vm_no_concurrent_access_wa(i915)) {
>> + ggtt->vm.insert_entries = bxt_vtd_ggtt_insert_entries__BKL;
>> + ggtt->vm.insert_page = bxt_vtd_ggtt_insert_page__BKL;
>> + ggtt->vm.bind_async_flags =
>> + I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND;
>> + }
>> +
>> + ggtt->invalidate = gen8_ggtt_invalidate;
>> +
>> + ggtt->vm.vma_ops.bind_vma = intel_ggtt_bind_vma;
>> + ggtt->vm.vma_ops.unbind_vma = intel_ggtt_unbind_vma;
>> +
>> + ggtt->vm.pte_encode = gen8_ggtt_pte_encode;
>> +
>> + setup_private_pat(ggtt->vm.gt->uncore);
>> +
>> + return ggtt_probe_common(ggtt, size);
>> +}
>> +
>> +static u64 snb_pte_encode(dma_addr_t addr,
>> + enum i915_cache_level level,
>> + u32 flags)
>> +{
>> + gen6_pte_t pte = GEN6_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID;
>> +
>> + switch (level) {
>> + case I915_CACHE_L3_LLC:
>> + case I915_CACHE_LLC:
>> + pte |= GEN6_PTE_CACHE_LLC;
>> + break;
>> + case I915_CACHE_NONE:
>> + pte |= GEN6_PTE_UNCACHED;
>> + break;
>> + default:
>> + MISSING_CASE(level);
>> + }
>> +
>> + return pte;
>> +}
>> +
>> +static u64 ivb_pte_encode(dma_addr_t addr,
>> + enum i915_cache_level level,
>> + u32 flags)
>> +{
>> + gen6_pte_t pte = GEN6_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID;
>> +
>> + switch (level) {
>> + case I915_CACHE_L3_LLC:
>> + pte |= GEN7_PTE_CACHE_L3_LLC;
>> + break;
>> + case I915_CACHE_LLC:
>> + pte |= GEN6_PTE_CACHE_LLC;
>> + break;
>> + case I915_CACHE_NONE:
>> + pte |= GEN6_PTE_UNCACHED;
>> + break;
>> + default:
>> + MISSING_CASE(level);
>> + }
>> +
>> + return pte;
>> +}
>> +
>> +static u64 byt_pte_encode(dma_addr_t addr,
>> + enum i915_cache_level level,
>> + u32 flags)
>> +{
>> + gen6_pte_t pte = GEN6_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID;
>> +
>> + if (!(flags & PTE_READ_ONLY))
>> + pte |= BYT_PTE_WRITEABLE;
>> +
>> + if (level != I915_CACHE_NONE)
>> + pte |= BYT_PTE_SNOOPED_BY_CPU_CACHES;
>> +
>> + return pte;
>> +}
>> +
>> +static u64 hsw_pte_encode(dma_addr_t addr,
>> + enum i915_cache_level level,
>> + u32 flags)
>> +{
>> + gen6_pte_t pte = HSW_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID;
>> +
>> + if (level != I915_CACHE_NONE)
>> + pte |= HSW_WB_LLC_AGE3;
>> +
>> + return pte;
>> +}
>> +
>> +static u64 iris_pte_encode(dma_addr_t addr,
>> + enum i915_cache_level level,
>> + u32 flags)
>> +{
>> + gen6_pte_t pte = HSW_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID;
>> +
>> + switch (level) {
>> + case I915_CACHE_NONE:
>> + break;
>> + case I915_CACHE_WT:
>> + pte |= HSW_WT_ELLC_LLC_AGE3;
>> + break;
>> + default:
>> + pte |= HSW_WB_ELLC_LLC_AGE3;
>> + break;
>> + }
>> +
>> + return pte;
>> +}
>> +
>> +static int gen6_gmch_probe(struct i915_ggtt *ggtt)
>> +{
>> + struct drm_i915_private *i915 = ggtt->vm.i915;
>> + struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
>> + unsigned int size;
>> + u16 snb_gmch_ctl;
>> +
>> + ggtt->gmadr = pci_resource(pdev, 2);
>> + ggtt->mappable_end = resource_size(&ggtt->gmadr);
>> +
>> + /*
>> + * 64/512MB is the current min/max we actually know of, but this is
>> + * just a coarse sanity check.
>> + */
>> + if (ggtt->mappable_end < (64 << 20) ||
>> + ggtt->mappable_end > (512 << 20)) {
>> + drm_err(&i915->drm, "Unknown GMADR size (%pa)\n",
>> + &ggtt->mappable_end);
>> + return -ENXIO;
>> + }
>> +
>> + pci_read_config_word(pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
>> +
>> + size = gen6_get_total_gtt_size(snb_gmch_ctl);
>> + ggtt->vm.total = (size / sizeof(gen6_pte_t)) * I915_GTT_PAGE_SIZE;
>> +
>> + ggtt->vm.alloc_pt_dma = alloc_pt_dma;
>> + ggtt->vm.alloc_scratch_dma = alloc_pt_dma;
>> +
>> + ggtt->vm.clear_range = nop_clear_range;
>> + if (!HAS_FULL_PPGTT(i915) || intel_scanout_needs_vtd_wa(i915))
>> + ggtt->vm.clear_range = gen6_ggtt_clear_range;
>> + ggtt->vm.insert_page = gen6_ggtt_insert_page;
>> + ggtt->vm.insert_entries = gen6_ggtt_insert_entries;
>> + ggtt->vm.cleanup = gen6_gmch_remove;
>> +
>> + ggtt->invalidate = gen6_ggtt_invalidate;
>> +
>> + if (HAS_EDRAM(i915))
>> + ggtt->vm.pte_encode = iris_pte_encode;
>> + else if (IS_HASWELL(i915))
>> + ggtt->vm.pte_encode = hsw_pte_encode;
>> + else if (IS_VALLEYVIEW(i915))
>> + ggtt->vm.pte_encode = byt_pte_encode;
>> + else if (GRAPHICS_VER(i915) >= 7)
>> + ggtt->vm.pte_encode = ivb_pte_encode;
>> + else
>> + ggtt->vm.pte_encode = snb_pte_encode;
>> +
>> + ggtt->vm.vma_ops.bind_vma = intel_ggtt_bind_vma;
>> + ggtt->vm.vma_ops.unbind_vma = intel_ggtt_unbind_vma;
>> +
>> + return ggtt_probe_common(ggtt, size);
>> +}
>> +
>> static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt)
>> {
>> struct drm_i915_private *i915 = gt->i915;
>> @@ -576,12 +1104,12 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt)
>> ggtt->vm.dma = i915->drm.dev;
>> dma_resv_init(&ggtt->vm._resv);
>>
>> - if (GRAPHICS_VER(i915) <= 5)
>> - ret = intel_gt_gmch_gen5_probe(ggtt);
>> + if (GRAPHICS_VER(i915) < 6)
>> + ret = intel_ggtt_gmch_probe(ggtt);
>> else if (GRAPHICS_VER(i915) < 8)
>> - ret = intel_gt_gmch_gen6_probe(ggtt);
>> + ret = gen6_gmch_probe(ggtt);
>> else
>> - ret = intel_gt_gmch_gen8_probe(ggtt);
>> + ret = gen8_gmch_probe(ggtt);
>
>We could also take the opportunity to reverse the if/else ladder here
>and put it in the i915-preferred "newest at top" order.
>
>> if (ret) {
>> dma_resv_fini(&ggtt->vm._resv);
>> return ret;
>> @@ -635,7 +1163,10 @@ int i915_ggtt_probe_hw(struct drm_i915_private *i915)
>>
>> int i915_ggtt_enable_hw(struct drm_i915_private *i915)
>> {
>> - return intel_gt_gmch_gen5_enable_hw(i915);
>> + if (GRAPHICS_VER(i915) < 6)
>> + return intel_ggtt_gmch_enable_hw(i915);
>> +
>> + return 0;
>> }
>>
>> void i915_ggtt_enable_guc(struct i915_ggtt *ggtt)
>> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c b/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c
>> new file mode 100644
>> index 000000000000..1c15825d4bd3
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c
>> @@ -0,0 +1,132 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#include "intel_ggtt_gmch.h"
>> +
>> +#include <drm/intel-gtt.h>
>> +#include <drm/i915_drm.h>
>> +
>> +#include <linux/agp_backend.h>
>> +
>> +#include "i915_drv.h"
>> +#include "i915_utils.h"
>> +#include "intel_gtt.h"
>> +#include "intel_gt_regs.h"
>> +#include "intel_gt.h"
>> +
>> +static void gen5_ggtt_insert_page(struct i915_address_space *vm,
>
>All the "gen5_" prefixes in this file seem a bit misleading if they wind
>up getting used on earlier platforms too; most of the driver uses the
>prefix to indicate the first platform/architecture that the function was
yeah... I think at the time we didn't want to say "gen1" (or whatever is
the oldest one we support), so we just left with gen5.
>used on. Maybe we should also rename these with a "gmch_" prefix as
>well to indicate that they're intended for the platforms where the GMCH
>was an independent chip and not integrated into the CPU?
Yeah, now that that intel-gtt symbols are renamed, we can use gmch_*
prefix that calls out to intel_gmch_* symbols from that module.
>
>
>Anyway, all of my review comments are just bikeshedding so feel free to
but I like the color of your shed :)
I will squash these changes here and make a note in the
commit message. And also mention something I forgot to say... that I
removed some dead code for gen12 from needs_idle_maps() since that
function is not called for anything above gen5.
>use or ignore them as you see fit. Either way, this series should fix
>the issue and restore GGTT handling for non-x86.
>
>Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
thanks. I will keep the r-b in the next version.
Lucas De Marchi
More information about the Intel-gfx
mailing list