> On Fri, Apr 21, 2023 at 10:27:22AM -0700, Yang, Fei wrote:
>>> On Wed, Apr 19, 2023 at 04:00:53PM -0700, fei.yang@xxxxxxxxx wrote:
>>>> From: Fei Yang <fei.yang@xxxxxxxxx>
>>>>
>>>> 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 <fei.yang@xxxxxxxxx>
>>>> Reviewed-by: Andrzej Hajda <andrzej.hajda@xxxxxxxxx>
>>>> Reviewed-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
>>>> Acked-by: Nirmoy Das <nirmoy.das@xxxxxxxxx>
>>>
>>> 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;
