Re: [Intel-gfx] [PATCH 3/8] drm/i915/mtl: Add PTE encode function
> On Sun, Apr 23, 2023 at 12:37:27AM -0700, Yang, Fei wrote: >>> On Fri, Apr 21, 2023 at 10:27:22AM -0700, Yang, Fei wrote: > On Wed, Apr 19, 2023 at 04:00:53PM -0700, fei.y...@intel.com wrote: >> From: Fei Yang >> >> PTE encode functions are platform dependent. This patch implements >> PTE functions for MTL, and ensures the correct PTE encode function >> is used by calling pte_encode function pointer instead of the >> hardcoded gen8 version of PTE encode. >> >> Signed-off-by: Fei Yang >> Reviewed-by: Andrzej Hajda >> Reviewed-by: Andi Shyti >> Acked-by: Nirmoy Das > > Bspec: 45015, 45040 > >> --- >> drivers/gpu/drm/i915/display/intel_dpt.c | 2 +- >> drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 45 >> drivers/gpu/drm/i915/gt/intel_ggtt.c | 36 +-- >> 3 files changed, 72 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c >> index b8027392144d..c5eacfdba1a5 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dpt.c >> +++ b/drivers/gpu/drm/i915/display/intel_dpt.c >> @@ -300,7 +300,7 @@ intel_dpt_create(struct intel_framebuffer *fb) >>vm->vma_ops.bind_vma= dpt_bind_vma; >>vm->vma_ops.unbind_vma = dpt_unbind_vma; >> >> - vm->pte_encode = gen8_ggtt_pte_encode; >> + vm->pte_encode = vm->gt->ggtt->vm.pte_encode; >> >>dpt->obj = dpt_obj; >>dpt->obj->is_dpt = true; >> diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >> b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >> index 4daaa6f55668..11b91e0453c8 100644 >> --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >> +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >> @@ -55,6 +55,34 @@ static u64 gen8_pte_encode(dma_addr_t addr, >>return pte; >> } >> >> +static u64 mtl_pte_encode(dma_addr_t addr, >> + enum i915_cache_level level, >> + u32 flags) >> +{ >> + gen8_pte_t pte = addr | GEN8_PAGE_PRESENT | GEN8_PAGE_RW; >> + >> + if (unlikely(flags & PTE_READ_ONLY)) >> + pte &= ~GEN8_PAGE_RW; >> + >> + if (flags & PTE_LM) >> + pte |= GEN12_PPGTT_PTE_LM | GEN12_PPGTT_PTE_NC; > > GEN12_PPGTT_PTE_NC got defined in the previous patch as BIT(5). But > according to bspec 45040, bit 5 is ignored in the PTE encoding. What is > this trying to do? This takes effect only for PTE_LM, doesn't affect MTL. PTE_NC is needed for PVC (use of access counter). I believe this function was writen based on the one for PVC. And this function did get extended to cover all gen12 in a later patch. >>> >>> Even though MTL doesn't have local memory, PTE_LM is supposed to be >>> used on MTL for access to BAR2 stolen memory. >> >> You were right, but I still think this code is fine because this bit is >> ignored for MTL anyway and it is needed for other platforms with LMEM. >> Otherwise this code would have some sort of platform checking which is >> hard to do because we don't have platform info here. >> Or we would have to define another PTE encode function for platforms >> needing PTE_NC just for this one difference, then manage the function >> pointer correctly. > > MTL is the only platform that uses this function right now: > > + if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70)) > + ppgtt->vm.pte_encode = mtl_pte_encode; > + else > + ppgtt->vm.pte_encode = gen8_pte_encode; > > If this is intended for PVC, then you have it in the wrong function to > begin with (and it also shouldn't be in a patch labelled "mtl"). If > you're trying to future-proof for some post-MTL discrete platform, then > such code should be saved until we enable that platform so that it can > be properly reviewed. dropped GEN12_PPGTT_PTE_NC bit in v2 of https://patchwork.freedesktop.org/series/116900/ > Matt > >> >> -Fei >> >>> Matt >>> -Fei > Matt > >> + >> + switch (level) { >> + case I915_CACHE_NONE: >> + pte |= GEN12_PPGTT_PTE_PAT1; >> + break;
Re: [Intel-gfx] [PATCH 3/8] drm/i915/mtl: Add PTE encode function
On Sun, Apr 23, 2023 at 12:37:27AM -0700, Yang, Fei wrote: > > On Fri, Apr 21, 2023 at 10:27:22AM -0700, Yang, Fei wrote: > >>> On Wed, Apr 19, 2023 at 04:00:53PM -0700, fei.y...@intel.com wrote: > From: Fei Yang > > PTE encode functions are platform dependent. This patch implements > PTE functions for MTL, and ensures the correct PTE encode function > is used by calling pte_encode function pointer instead of the > hardcoded gen8 version of PTE encode. > > Signed-off-by: Fei Yang > Reviewed-by: Andrzej Hajda > Reviewed-by: Andi Shyti > Acked-by: Nirmoy Das > >>> > >>> Bspec: 45015, 45040 > >>> > --- > drivers/gpu/drm/i915/display/intel_dpt.c | 2 +- > drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 45 > drivers/gpu/drm/i915/gt/intel_ggtt.c | 36 +-- > 3 files changed, 72 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c > >>b/drivers/gpu/drm/i915/display/intel_dpt.c > index b8027392144d..c5eacfdba1a5 100644 > --- a/drivers/gpu/drm/i915/display/intel_dpt.c > +++ b/drivers/gpu/drm/i915/display/intel_dpt.c > @@ -300,7 +300,7 @@ intel_dpt_create(struct intel_framebuffer *fb) > vm->vma_ops.bind_vma= dpt_bind_vma; > vm->vma_ops.unbind_vma = dpt_unbind_vma; > > - vm->pte_encode = gen8_ggtt_pte_encode; > + vm->pte_encode = vm->gt->ggtt->vm.pte_encode; > > dpt->obj = dpt_obj; > dpt->obj->is_dpt = true; > diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > index 4daaa6f55668..11b91e0453c8 100644 > --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > @@ -55,6 +55,34 @@ static u64 gen8_pte_encode(dma_addr_t addr, > return pte; > } > > +static u64 mtl_pte_encode(dma_addr_t addr, > + enum i915_cache_level level, > + u32 flags) > +{ > + gen8_pte_t pte = addr | GEN8_PAGE_PRESENT | GEN8_PAGE_RW; > + > + if (unlikely(flags & PTE_READ_ONLY)) > + pte &= ~GEN8_PAGE_RW; > + > + if (flags & PTE_LM) > + pte |= GEN12_PPGTT_PTE_LM | GEN12_PPGTT_PTE_NC; > >>> > >>> GEN12_PPGTT_PTE_NC got defined in the previous patch as BIT(5). But > >>> according to bspec 45040, bit 5 is ignored in the PTE encoding. What is > >>> this trying to do? > >> > >> This takes effect only for PTE_LM, doesn't affect MTL. > >> PTE_NC is needed for PVC (use of access counter). > >> I believe this function was writen based on the one for PVC. And this > >> function did get extended to cover all gen12 in a later patch. > > > > Even though MTL doesn't have local memory, PTE_LM is supposed to be > > used on MTL for access to BAR2 stolen memory. > > You were right, but I still think this code is fine because this bit is > ignored for MTL anyway and it is needed for other platforms with LMEM. > Otherwise this code would have some sort of platform checking which is > hard to do because we don't have platform info here. > Or we would have to define another PTE encode function for platforms > needing PTE_NC just for this one difference, then manage the function > pointer correctly. MTL is the only platform that uses this function right now: + if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70)) + ppgtt->vm.pte_encode = mtl_pte_encode; + else + ppgtt->vm.pte_encode = gen8_pte_encode; If this is intended for PVC, then you have it in the wrong function to begin with (and it also shouldn't be in a patch labelled "mtl"). If you're trying to future-proof for some post-MTL discrete platform, then such code should be saved until we enable that platform so that it can be properly reviewed. Matt > > -Fei > > > Matt > > > >> -Fei > >>> Matt > >>> > + > + switch (level) { > + case I915_CACHE_NONE: > + pte |= GEN12_PPGTT_PTE_PAT1; > + break; -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation
RE: [Intel-gfx] [PATCH 3/8] drm/i915/mtl: Add PTE encode function
> On Fri, Apr 21, 2023 at 10:27:22AM -0700, Yang, Fei wrote: >>> On Wed, Apr 19, 2023 at 04:00:53PM -0700, fei.y...@intel.com wrote: From: Fei Yang PTE encode functions are platform dependent. This patch implements PTE functions for MTL, and ensures the correct PTE encode function is used by calling pte_encode function pointer instead of the hardcoded gen8 version of PTE encode. Signed-off-by: Fei Yang Reviewed-by: Andrzej Hajda Reviewed-by: Andi Shyti Acked-by: Nirmoy Das >>> >>> Bspec: 45015, 45040 >>> --- drivers/gpu/drm/i915/display/intel_dpt.c | 2 +- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 45 drivers/gpu/drm/i915/gt/intel_ggtt.c | 36 +-- 3 files changed, 72 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c >>b/drivers/gpu/drm/i915/display/intel_dpt.c index b8027392144d..c5eacfdba1a5 100644 --- a/drivers/gpu/drm/i915/display/intel_dpt.c +++ b/drivers/gpu/drm/i915/display/intel_dpt.c @@ -300,7 +300,7 @@ intel_dpt_create(struct intel_framebuffer *fb) vm->vma_ops.bind_vma= dpt_bind_vma; vm->vma_ops.unbind_vma = dpt_unbind_vma; - vm->pte_encode = gen8_ggtt_pte_encode; + vm->pte_encode = vm->gt->ggtt->vm.pte_encode; dpt->obj = dpt_obj; dpt->obj->is_dpt = true; diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index 4daaa6f55668..11b91e0453c8 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -55,6 +55,34 @@ static u64 gen8_pte_encode(dma_addr_t addr, return pte; } +static u64 mtl_pte_encode(dma_addr_t addr, + enum i915_cache_level level, + u32 flags) +{ + gen8_pte_t pte = addr | GEN8_PAGE_PRESENT | GEN8_PAGE_RW; + + if (unlikely(flags & PTE_READ_ONLY)) + pte &= ~GEN8_PAGE_RW; + + if (flags & PTE_LM) + pte |= GEN12_PPGTT_PTE_LM | GEN12_PPGTT_PTE_NC; >>> >>> GEN12_PPGTT_PTE_NC got defined in the previous patch as BIT(5). But >>> according to bspec 45040, bit 5 is ignored in the PTE encoding. What is >>> this trying to do? >> >> This takes effect only for PTE_LM, doesn't affect MTL. >> PTE_NC is needed for PVC (use of access counter). >> I believe this function was writen based on the one for PVC. And this >> function did get extended to cover all gen12 in a later patch. > > Even though MTL doesn't have local memory, PTE_LM is supposed to be > used on MTL for access to BAR2 stolen memory. You were right, but I still think this code is fine because this bit is ignored for MTL anyway and it is needed for other platforms with LMEM. Otherwise this code would have some sort of platform checking which is hard to do because we don't have platform info here. Or we would have to define another PTE encode function for platforms needing PTE_NC just for this one difference, then manage the function pointer correctly. -Fei > Matt > >> -Fei >>> Matt >>> + + switch (level) { + case I915_CACHE_NONE: + pte |= GEN12_PPGTT_PTE_PAT1; + break;
Re: [Intel-gfx] [PATCH 3/8] drm/i915/mtl: Add PTE encode function
On Fri, Apr 21, 2023 at 10:27:22AM -0700, Yang, Fei wrote: >> On Wed, Apr 19, 2023 at 04:00:53PM -0700, fei.y...@intel.com wrote: >>> From: Fei Yang >>> >>> PTE encode functions are platform dependent. This patch implements >>> PTE functions for MTL, and ensures the correct PTE encode function >>> is used by calling pte_encode function pointer instead of the >>> hardcoded gen8 version of PTE encode. >>> >>> Signed-off-by: Fei Yang >>> Reviewed-by: Andrzej Hajda >>> Reviewed-by: Andi Shyti >>> Acked-by: Nirmoy Das >> >> Bspec: 45015, 45040 >> >>> --- >>> drivers/gpu/drm/i915/display/intel_dpt.c | 2 +- >>> drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 45 >>> drivers/gpu/drm/i915/gt/intel_ggtt.c | 36 +-- >>> 3 files changed, 72 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c >b/drivers/gpu/drm/i915/display/intel_dpt.c >>> index b8027392144d..c5eacfdba1a5 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_dpt.c >>> +++ b/drivers/gpu/drm/i915/display/intel_dpt.c >>> @@ -300,7 +300,7 @@ intel_dpt_create(struct intel_framebuffer *fb) >>> vm->vma_ops.bind_vma = dpt_bind_vma; >>> vm->vma_ops.unbind_vma = dpt_unbind_vma; >>> >>> - vm->pte_encode = gen8_ggtt_pte_encode; >>> + vm->pte_encode = vm->gt->ggtt->vm.pte_encode; >>> >>> dpt->obj = dpt_obj; >>> dpt->obj->is_dpt = true; >>> diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >>> index 4daaa6f55668..11b91e0453c8 100644 >>> --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >>> +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >>> @@ -55,6 +55,34 @@ static u64 gen8_pte_encode(dma_addr_t addr, >>> return pte; >>> } >>> >>> +static u64 mtl_pte_encode(dma_addr_t addr, >>> + enum i915_cache_level level, >>> + u32 flags) >>> +{ >>> + gen8_pte_t pte = addr | GEN8_PAGE_PRESENT | GEN8_PAGE_RW; >>> + >>> + if (unlikely(flags & PTE_READ_ONLY)) >>> + pte &= ~GEN8_PAGE_RW; >>> + >>> + if (flags & PTE_LM) >>> + pte |= GEN12_PPGTT_PTE_LM | GEN12_PPGTT_PTE_NC; >> >> GEN12_PPGTT_PTE_NC got defined in the previous patch as BIT(5). But >> according to bspec 45040, bit 5 is ignored in the PTE encoding. What is >> this trying to do? >This takes effect only for PTE_LM, doesn't affect MTL. >PTE_NC is needed for PVC (use of access counter). >I believe this function was writen based on the one for PVC. And this >function >did get extended to cover all gen12 in a later patch. Even though MTL doesn't have local memory, PTE_LM is supposed to be used on MTL for access to BAR2 stolen memory. Matt >-Fei >> Matt >> >>> + >>> + switch (level) { >>> + case I915_CACHE_NONE: >>> + pte |= GEN12_PPGTT_PTE_PAT1; >>> + break; -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation
Re: [Intel-gfx] [PATCH 3/8] drm/i915/mtl: Add PTE encode function
> On Wed, Apr 19, 2023 at 04:00:53PM -0700, fei.y...@intel.com wrote: >> From: Fei Yang >> >> PTE encode functions are platform dependent. This patch implements >> PTE functions for MTL, and ensures the correct PTE encode function >> is used by calling pte_encode function pointer instead of the >> hardcoded gen8 version of PTE encode. >> >> Signed-off-by: Fei Yang >> Reviewed-by: Andrzej Hajda >> Reviewed-by: Andi Shyti >> Acked-by: Nirmoy Das > > Bspec: 45015, 45040 > >> --- >> drivers/gpu/drm/i915/display/intel_dpt.c | 2 +- >> drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 45 >> drivers/gpu/drm/i915/gt/intel_ggtt.c | 36 +-- >> 3 files changed, 72 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c >> b/drivers/gpu/drm/i915/display/intel_dpt.c >> index b8027392144d..c5eacfdba1a5 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dpt.c >> +++ b/drivers/gpu/drm/i915/display/intel_dpt.c >> @@ -300,7 +300,7 @@ intel_dpt_create(struct intel_framebuffer *fb) >>vm->vma_ops.bind_vma= dpt_bind_vma; >>vm->vma_ops.unbind_vma = dpt_unbind_vma; >> >> - vm->pte_encode = gen8_ggtt_pte_encode; >> + vm->pte_encode = vm->gt->ggtt->vm.pte_encode; >> >>dpt->obj = dpt_obj; >>dpt->obj->is_dpt = true; >> diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >> b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >> index 4daaa6f55668..11b91e0453c8 100644 >> --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >> +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c >> @@ -55,6 +55,34 @@ static u64 gen8_pte_encode(dma_addr_t addr, >>return pte; >> } >> >> +static u64 mtl_pte_encode(dma_addr_t addr, >> + enum i915_cache_level level, >> + u32 flags) >> +{ >> + gen8_pte_t pte = addr | GEN8_PAGE_PRESENT | GEN8_PAGE_RW; >> + >> + if (unlikely(flags & PTE_READ_ONLY)) >> + pte &= ~GEN8_PAGE_RW; >> + >> + if (flags & PTE_LM) >> + pte |= GEN12_PPGTT_PTE_LM | GEN12_PPGTT_PTE_NC; > > GEN12_PPGTT_PTE_NC got defined in the previous patch as BIT(5). But > according to bspec 45040, bit 5 is ignored in the PTE encoding. What is > this trying to do? This takes effect only for PTE_LM, doesn't affect MTL. PTE_NC is needed for PVC (use of access counter). I believe this function was writen based on the one for PVC. And this function did get extended to cover all gen12 in a later patch. -Fei > Matt > >> + >> + switch (level) { >> + case I915_CACHE_NONE: >> + pte |= GEN12_PPGTT_PTE_PAT1; >> + break;
Re: [Intel-gfx] [PATCH 3/8] drm/i915/mtl: Add PTE encode function
On Wed, Apr 19, 2023 at 04:00:53PM -0700, fei.y...@intel.com wrote: > From: Fei Yang > > PTE encode functions are platform dependent. This patch implements > PTE functions for MTL, and ensures the correct PTE encode function > is used by calling pte_encode function pointer instead of the > hardcoded gen8 version of PTE encode. > > Signed-off-by: Fei Yang > Reviewed-by: Andrzej Hajda > Reviewed-by: Andi Shyti > Acked-by: Nirmoy Das Bspec: 45015, 45040 > --- > drivers/gpu/drm/i915/display/intel_dpt.c | 2 +- > drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 45 > drivers/gpu/drm/i915/gt/intel_ggtt.c | 36 +-- > 3 files changed, 72 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c > b/drivers/gpu/drm/i915/display/intel_dpt.c > index b8027392144d..c5eacfdba1a5 100644 > --- a/drivers/gpu/drm/i915/display/intel_dpt.c > +++ b/drivers/gpu/drm/i915/display/intel_dpt.c > @@ -300,7 +300,7 @@ intel_dpt_create(struct intel_framebuffer *fb) > vm->vma_ops.bind_vma= dpt_bind_vma; > vm->vma_ops.unbind_vma = dpt_unbind_vma; > > - vm->pte_encode = gen8_ggtt_pte_encode; > + vm->pte_encode = vm->gt->ggtt->vm.pte_encode; > > dpt->obj = dpt_obj; > dpt->obj->is_dpt = true; > diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > index 4daaa6f55668..11b91e0453c8 100644 > --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > @@ -55,6 +55,34 @@ static u64 gen8_pte_encode(dma_addr_t addr, > return pte; > } > > +static u64 mtl_pte_encode(dma_addr_t addr, > + enum i915_cache_level level, > + u32 flags) > +{ > + gen8_pte_t pte = addr | GEN8_PAGE_PRESENT | GEN8_PAGE_RW; > + > + if (unlikely(flags & PTE_READ_ONLY)) > + pte &= ~GEN8_PAGE_RW; > + > + if (flags & PTE_LM) > + pte |= GEN12_PPGTT_PTE_LM | GEN12_PPGTT_PTE_NC; GEN12_PPGTT_PTE_NC got defined in the previous patch as BIT(5). But according to bspec 45040, bit 5 is ignored in the PTE encoding. What is this trying to do? Matt > + > + switch (level) { > + case I915_CACHE_NONE: > + pte |= GEN12_PPGTT_PTE_PAT1; > + break; > + case I915_CACHE_LLC: > + case I915_CACHE_L3_LLC: > + pte |= GEN12_PPGTT_PTE_PAT0 | GEN12_PPGTT_PTE_PAT1; > + break; > + case I915_CACHE_WT: > + pte |= GEN12_PPGTT_PTE_PAT0; > + break; > + } > + > + return pte; > +} > + > static void gen8_ppgtt_notify_vgt(struct i915_ppgtt *ppgtt, bool create) > { > struct drm_i915_private *i915 = ppgtt->vm.i915; > @@ -427,7 +455,7 @@ gen8_ppgtt_insert_pte(struct i915_ppgtt *ppgtt, > u32 flags) > { > struct i915_page_directory *pd; > - const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags); > + const gen8_pte_t pte_encode = ppgtt->vm.pte_encode(0, cache_level, > flags); > gen8_pte_t *vaddr; > > pd = i915_pd_entry(pdp, gen8_pd_index(idx, 2)); > @@ -580,7 +608,7 @@ static void gen8_ppgtt_insert_huge(struct > i915_address_space *vm, > enum i915_cache_level cache_level, > u32 flags) > { > - const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags); > + const gen8_pte_t pte_encode = vm->pte_encode(0, cache_level, flags); > unsigned int rem = sg_dma_len(iter->sg); > u64 start = vma_res->start; > > @@ -743,7 +771,7 @@ static void gen8_ppgtt_insert_entry(struct > i915_address_space *vm, > GEM_BUG_ON(pt->is_compact); > > vaddr = px_vaddr(pt); > - vaddr[gen8_pd_index(idx, 0)] = gen8_pte_encode(addr, level, flags); > + vaddr[gen8_pd_index(idx, 0)] = vm->pte_encode(addr, level, flags); > drm_clflush_virt_range(&vaddr[gen8_pd_index(idx, 0)], sizeof(*vaddr)); > } > > @@ -773,7 +801,7 @@ static void __xehpsdv_ppgtt_insert_entry_lm(struct > i915_address_space *vm, > } > > vaddr = px_vaddr(pt); > - vaddr[gen8_pd_index(idx, 0) / 16] = gen8_pte_encode(addr, level, flags); > + vaddr[gen8_pd_index(idx, 0) / 16] = vm->pte_encode(addr, level, flags); > } > > static void xehpsdv_ppgtt_insert_entry(struct i915_address_space *vm, > @@ -820,8 +848,8 @@ static int gen8_init_scratch(struct i915_address_space > *vm) > pte_flags |= PTE_LM; > > vm->scratch[0]->encode = > - gen8_pte_encode(px_dma(vm->scratch[0]), > - I915_CACHE_NONE, pte_flags); > + vm->pte_encode(px_dma(vm->scratch[0]), > +I915_CACHE_NONE, pte_flags); > > for (i = 1; i <= vm->top; i++) { > struct drm_i915_gem_object *obj; > @@ -963,7 +991,10 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt, >*/ > ppgtt->vm.alloc_scratch_dma = al
Re: [Intel-gfx] [PATCH 3/8] drm/i915/mtl: Add PTE encode function
Hi Fei, > PTE encode functions are platform dependent. This patch implements > PTE functions for MTL, and ensures the correct PTE encode function > is used by calling pte_encode function pointer instead of the > hardcoded gen8 version of PTE encode. > > Signed-off-by: Fei Yang I think nothing opened here... one comment from Nirmoy I see that has been addressed. Reviewed-by: Andi Shyti Reviewed-by: Andrzej Hajda Acked-by: Nirmoy Das Andi
Re: [Intel-gfx] [PATCH 3/8] drm/i915/mtl: Add PTE encode function
On 17.04.2023 08:24, fei.y...@intel.com wrote: From: Fei Yang PTE encode functions are platform dependent. This patch implements PTE functions for MTL, and ensures the correct PTE encode function is used by calling pte_encode function pointer instead of the hardcoded gen8 version of PTE encode. Signed-off-by: Fei Yang Reviewed-by: Andrzej Hajda Regards Andrzej --- drivers/gpu/drm/i915/display/intel_dpt.c | 2 +- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 45 drivers/gpu/drm/i915/gt/gen8_ppgtt.h | 3 ++ drivers/gpu/drm/i915/gt/intel_ggtt.c | 36 +-- 4 files changed, 75 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c index b8027392144d..c5eacfdba1a5 100644 --- a/drivers/gpu/drm/i915/display/intel_dpt.c +++ b/drivers/gpu/drm/i915/display/intel_dpt.c @@ -300,7 +300,7 @@ intel_dpt_create(struct intel_framebuffer *fb) vm->vma_ops.bind_vma= dpt_bind_vma; vm->vma_ops.unbind_vma = dpt_unbind_vma; - vm->pte_encode = gen8_ggtt_pte_encode; + vm->pte_encode = vm->gt->ggtt->vm.pte_encode; dpt->obj = dpt_obj; dpt->obj->is_dpt = true; diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index 4daaa6f55668..11b91e0453c8 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -55,6 +55,34 @@ static u64 gen8_pte_encode(dma_addr_t addr, return pte; } +static u64 mtl_pte_encode(dma_addr_t addr, + enum i915_cache_level level, + u32 flags) +{ + gen8_pte_t pte = addr | GEN8_PAGE_PRESENT | GEN8_PAGE_RW; + + if (unlikely(flags & PTE_READ_ONLY)) + pte &= ~GEN8_PAGE_RW; + + if (flags & PTE_LM) + pte |= GEN12_PPGTT_PTE_LM | GEN12_PPGTT_PTE_NC; + + switch (level) { + case I915_CACHE_NONE: + pte |= GEN12_PPGTT_PTE_PAT1; + break; + case I915_CACHE_LLC: + case I915_CACHE_L3_LLC: + pte |= GEN12_PPGTT_PTE_PAT0 | GEN12_PPGTT_PTE_PAT1; + break; + case I915_CACHE_WT: + pte |= GEN12_PPGTT_PTE_PAT0; + break; + } + + return pte; +} + static void gen8_ppgtt_notify_vgt(struct i915_ppgtt *ppgtt, bool create) { struct drm_i915_private *i915 = ppgtt->vm.i915; @@ -427,7 +455,7 @@ gen8_ppgtt_insert_pte(struct i915_ppgtt *ppgtt, u32 flags) { struct i915_page_directory *pd; - const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags); + const gen8_pte_t pte_encode = ppgtt->vm.pte_encode(0, cache_level, flags); gen8_pte_t *vaddr; pd = i915_pd_entry(pdp, gen8_pd_index(idx, 2)); @@ -580,7 +608,7 @@ static void gen8_ppgtt_insert_huge(struct i915_address_space *vm, enum i915_cache_level cache_level, u32 flags) { - const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags); + const gen8_pte_t pte_encode = vm->pte_encode(0, cache_level, flags); unsigned int rem = sg_dma_len(iter->sg); u64 start = vma_res->start; @@ -743,7 +771,7 @@ static void gen8_ppgtt_insert_entry(struct i915_address_space *vm, GEM_BUG_ON(pt->is_compact); vaddr = px_vaddr(pt); - vaddr[gen8_pd_index(idx, 0)] = gen8_pte_encode(addr, level, flags); + vaddr[gen8_pd_index(idx, 0)] = vm->pte_encode(addr, level, flags); drm_clflush_virt_range(&vaddr[gen8_pd_index(idx, 0)], sizeof(*vaddr)); } @@ -773,7 +801,7 @@ static void __xehpsdv_ppgtt_insert_entry_lm(struct i915_address_space *vm, } vaddr = px_vaddr(pt); - vaddr[gen8_pd_index(idx, 0) / 16] = gen8_pte_encode(addr, level, flags); + vaddr[gen8_pd_index(idx, 0) / 16] = vm->pte_encode(addr, level, flags); } static void xehpsdv_ppgtt_insert_entry(struct i915_address_space *vm, @@ -820,8 +848,8 @@ static int gen8_init_scratch(struct i915_address_space *vm) pte_flags |= PTE_LM; vm->scratch[0]->encode = - gen8_pte_encode(px_dma(vm->scratch[0]), - I915_CACHE_NONE, pte_flags); + vm->pte_encode(px_dma(vm->scratch[0]), + I915_CACHE_NONE, pte_flags); for (i = 1; i <= vm->top; i++) { struct drm_i915_gem_object *obj; @@ -963,7 +991,10 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt, */ ppgtt->vm.alloc_scratch_dma = alloc_pt_dma; - ppgtt->vm.pte_encode = gen8_pte_encode; + if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70)) + ppgtt->vm.pte_encode = mtl_pte_encode; + else + ppgtt->vm.pte_encode = gen8_pte_encode; ppgtt->vm.bind_async_flags = I915_VMA_LOCAL_BIND; ppgtt->vm.insert_ent
Re: [Intel-gfx] [PATCH 3/8] drm/i915/mtl: Add PTE encode function
Hi Fei, On Sun, Apr 16, 2023 at 11:24:58PM -0700, fei.y...@intel.com wrote: > From: Fei Yang > > PTE encode functions are platform dependent. This patch implements > PTE functions for MTL, and ensures the correct PTE encode function > is used by calling pte_encode function pointer instead of the > hardcoded gen8 version of PTE encode. > > Signed-off-by: Fei Yang Reviewed-by: Andi Shyti Andi