* [Intel-gfx] [PATCH v3 1/3] drm/i915: Distinction of memory regions
@ 2021-02-03 15:23 Matthew Auld
2021-02-03 15:23 ` [Intel-gfx] [PATCH v3 2/3] drm/i915/gtt/dg1: add PTE_LM plumbing for ppGTT Matthew Auld
2021-02-03 15:23 ` [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM plumbing for GGTT Matthew Auld
0 siblings, 2 replies; 8+ messages in thread
From: Matthew Auld @ 2021-02-03 15:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Chris Wilson
From: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
In preparation for Xe HP multi-tile architecture with multiple memory
regions, we need to be able differentiate multiple instances of device
local-memory.
Note that the region name is just to give it a human friendly
identifier, instead of using class/instance which also uniquely
identifies the region. So far the region name is only for our own
internal debugging in the kernel(like in the selftests), or debugfs
which prints the list of regions, including the regions name.
v2: add commentary for our current region name use
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_gt.c | 2 ++
drivers/gpu/drm/i915/gt/intel_region_lmem.c | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 35ff68ada4f1..ca76f93bc03d 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -68,6 +68,8 @@ int intel_gt_probe_lmem(struct intel_gt *gt)
mem->type = INTEL_MEMORY_LOCAL;
mem->instance = 0;
+ intel_memory_region_set_name(mem, "local%u", mem->instance);
+
GEM_BUG_ON(!HAS_REGION(i915, id));
GEM_BUG_ON(i915->mm.regions[id]);
i915->mm.regions[id] = mem;
diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
index 8c498e96b01d..be6f2c8f5184 100644
--- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
@@ -90,8 +90,6 @@ region_lmem_init(struct intel_memory_region *mem)
if (ret)
io_mapping_fini(&mem->iomap);
- intel_memory_region_set_name(mem, "local");
-
return ret;
}
--
2.26.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Intel-gfx] [PATCH v3 2/3] drm/i915/gtt/dg1: add PTE_LM plumbing for ppGTT
2021-02-03 15:23 [Intel-gfx] [PATCH v3 1/3] drm/i915: Distinction of memory regions Matthew Auld
@ 2021-02-03 15:23 ` Matthew Auld
2021-02-03 15:23 ` [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM plumbing for GGTT Matthew Auld
1 sibling, 0 replies; 8+ messages in thread
From: Matthew Auld @ 2021-02-03 15:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Chris Wilson
For the PTEs we get an LM bit, to signal whether the page resides in
SMEM or LMEM.
v2: just use gen8_pte_encode for dg1
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Signed-off-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 12 +++++++++++-
drivers/gpu/drm/i915/gt/intel_gtt.h | 3 +++
drivers/gpu/drm/i915/gt/intel_ppgtt.c | 4 ++++
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
index 03a9d4396373..176c19633412 100644
--- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
@@ -5,6 +5,8 @@
#include <linux/log2.h>
+#include "gem/i915_gem_lmem.h"
+
#include "gen8_ppgtt.h"
#include "i915_scatterlist.h"
#include "i915_trace.h"
@@ -35,6 +37,9 @@ static u64 gen8_pte_encode(dma_addr_t addr,
if (unlikely(flags & PTE_READ_ONLY))
pte &= ~_PAGE_RW;
+ if (flags & PTE_LM)
+ pte |= GEN12_PPGTT_PTE_LM;
+
switch (level) {
case I915_CACHE_NONE:
pte |= PPAT_UNCACHED;
@@ -558,6 +563,7 @@ static void gen8_ppgtt_insert(struct i915_address_space *vm,
static int gen8_init_scratch(struct i915_address_space *vm)
{
+ u32 pte_flags;
int ret;
int i;
@@ -581,9 +587,13 @@ static int gen8_init_scratch(struct i915_address_space *vm)
if (ret)
return ret;
+ pte_flags = vm->has_read_only;
+ if (i915_gem_object_is_lmem(vm->scratch[0]))
+ pte_flags |= PTE_LM;
+
vm->scratch[0]->encode =
gen8_pte_encode(px_dma(vm->scratch[0]),
- I915_CACHE_LLC, vm->has_read_only);
+ I915_CACHE_LLC, pte_flags);
for (i = 1; i <= vm->top; i++) {
struct drm_i915_gem_object *obj;
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
index 29c10fde8ce3..0eef625dd787 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -85,6 +85,8 @@ typedef u64 gen8_pte_t;
#define BYT_PTE_SNOOPED_BY_CPU_CACHES REG_BIT(2)
#define BYT_PTE_WRITEABLE REG_BIT(1)
+#define GEN12_PPGTT_PTE_LM BIT_ULL(11)
+
/*
* Cacheability Control is a 4-bit value. The low three bits are stored in bits
* 3:1 of the PTE, while the fourth bit is stored in bit 11 of the PTE.
@@ -264,6 +266,7 @@ struct i915_address_space {
enum i915_cache_level level,
u32 flags); /* Create a valid PTE */
#define PTE_READ_ONLY BIT(0)
+#define PTE_LM BIT(1)
void (*allocate_va_range)(struct i915_address_space *vm,
struct i915_vm_pt_stash *stash,
diff --git a/drivers/gpu/drm/i915/gt/intel_ppgtt.c b/drivers/gpu/drm/i915/gt/intel_ppgtt.c
index 3f940ae27028..a91955af50a6 100644
--- a/drivers/gpu/drm/i915/gt/intel_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ppgtt.c
@@ -5,6 +5,8 @@
#include <linux/slab.h>
+#include "gem/i915_gem_lmem.h"
+
#include "i915_trace.h"
#include "intel_gtt.h"
#include "gen6_ppgtt.h"
@@ -192,6 +194,8 @@ void ppgtt_bind_vma(struct i915_address_space *vm,
pte_flags = 0;
if (i915_gem_object_is_readonly(vma->obj))
pte_flags |= PTE_READ_ONLY;
+ if (i915_gem_object_is_lmem(vma->obj))
+ pte_flags |= PTE_LM;
vm->insert_entries(vm, vma, cache_level, pte_flags);
wmb();
--
2.26.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM plumbing for GGTT
2021-02-03 15:23 [Intel-gfx] [PATCH v3 1/3] drm/i915: Distinction of memory regions Matthew Auld
2021-02-03 15:23 ` [Intel-gfx] [PATCH v3 2/3] drm/i915/gtt/dg1: add PTE_LM plumbing for ppGTT Matthew Auld
@ 2021-02-03 15:23 ` Matthew Auld
2021-02-03 16:51 ` Tang, CQ
1 sibling, 1 reply; 8+ messages in thread
From: Matthew Auld @ 2021-02-03 15:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Chris Wilson
For the PTEs we get an LM bit, to signal whether the page resides in
SMEM or LMEM.
Based on a patch from Michel Thierry.
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_ggtt.c | 24 +++++++++++++++++++-----
drivers/gpu/drm/i915/gt/intel_gtt.h | 4 +++-
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index fc399ac16eda..b0b8ded834f0 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -10,6 +10,8 @@
#include <drm/i915_drm.h>
+#include "gem/i915_gem_lmem.h"
+
#include "intel_gt.h"
#include "i915_drv.h"
#include "i915_scatterlist.h"
@@ -189,7 +191,12 @@ static u64 gen8_ggtt_pte_encode(dma_addr_t addr,
enum i915_cache_level level,
u32 flags)
{
- return addr | _PAGE_PRESENT;
+ gen8_pte_t pte = addr | _PAGE_PRESENT;
+
+ if (flags & PTE_LM)
+ pte |= GEN12_GGTT_PTE_LM;
+
+ return pte;
}
static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte)
@@ -201,13 +208,13 @@ static void gen8_ggtt_insert_page(struct i915_address_space *vm,
dma_addr_t addr,
u64 offset,
enum i915_cache_level level,
- u32 unused)
+ 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, 0));
+ gen8_set_pte(pte, gen8_ggtt_pte_encode(addr, level, flags));
ggtt->invalidate(ggtt);
}
@@ -217,7 +224,7 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
enum i915_cache_level level,
u32 flags)
{
- const gen8_pte_t pte_encode = gen8_ggtt_pte_encode(0, level, 0);
+ 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;
@@ -459,6 +466,8 @@ static void ggtt_bind_vma(struct i915_address_space *vm,
pte_flags = 0;
if (i915_gem_object_is_readonly(obj))
pte_flags |= PTE_READ_ONLY;
+ if (i915_gem_object_is_lmem(obj))
+ pte_flags |= PTE_LM;
vm->insert_entries(vm, vma, cache_level, pte_flags);
vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
@@ -794,6 +803,7 @@ 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;
/* For Modern GENs the PTEs and register space are split in the BAR */
@@ -823,9 +833,13 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
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, 0);
+ I915_CACHE_NONE, pte_flags);
return 0;
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
index 0eef625dd787..24b5808df16d 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -85,7 +85,9 @@ typedef u64 gen8_pte_t;
#define BYT_PTE_SNOOPED_BY_CPU_CACHES REG_BIT(2)
#define BYT_PTE_WRITEABLE REG_BIT(1)
-#define GEN12_PPGTT_PTE_LM BIT_ULL(11)
+#define GEN12_PPGTT_PTE_LM BIT_ULL(11)
+
+#define GEN12_GGTT_PTE_LM BIT_ULL(1)
/*
* Cacheability Control is a 4-bit value. The low three bits are stored in bits
--
2.26.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM plumbing for GGTT
2021-02-03 15:23 ` [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM plumbing for GGTT Matthew Auld
@ 2021-02-03 16:51 ` Tang, CQ
2021-02-03 17:02 ` Matthew Auld
0 siblings, 1 reply; 8+ messages in thread
From: Tang, CQ @ 2021-02-03 16:51 UTC (permalink / raw)
To: Auld, Matthew, intel-gfx; +Cc: Chris Wilson
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Matthew Auld
> Sent: Wednesday, February 3, 2021 7:24 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Subject: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM plumbing
> for GGTT
>
> For the PTEs we get an LM bit, to signal whether the page resides in SMEM or
> LMEM.
>
> Based on a patch from Michel Thierry.
>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/gt/intel_ggtt.c | 24 +++++++++++++++++++-----
> drivers/gpu/drm/i915/gt/intel_gtt.h | 4 +++-
> 2 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> index fc399ac16eda..b0b8ded834f0 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> @@ -10,6 +10,8 @@
>
> #include <drm/i915_drm.h>
>
> +#include "gem/i915_gem_lmem.h"
> +
> #include "intel_gt.h"
> #include "i915_drv.h"
> #include "i915_scatterlist.h"
> @@ -189,7 +191,12 @@ static u64 gen8_ggtt_pte_encode(dma_addr_t addr,
> enum i915_cache_level level,
> u32 flags)
> {
> - return addr | _PAGE_PRESENT;
> + gen8_pte_t pte = addr | _PAGE_PRESENT;
> +
> + if (flags & PTE_LM)
> + pte |= GEN12_GGTT_PTE_LM;
> +
> + return pte;
> }
>
> static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte) @@ -201,13
> +208,13 @@ static void gen8_ggtt_insert_page(struct i915_address_space
> *vm,
> dma_addr_t addr,
> u64 offset,
> enum i915_cache_level level,
> - u32 unused)
> + 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, 0));
> + gen8_set_pte(pte, gen8_ggtt_pte_encode(addr, level, flags));
>
> ggtt->invalidate(ggtt);
> }
> @@ -217,7 +224,7 @@ static void gen8_ggtt_insert_entries(struct
> i915_address_space *vm,
> enum i915_cache_level level,
> u32 flags)
> {
> - const gen8_pte_t pte_encode = gen8_ggtt_pte_encode(0, level, 0);
> + 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;
> @@ -459,6 +466,8 @@ static void ggtt_bind_vma(struct i915_address_space
> *vm,
> pte_flags = 0;
> if (i915_gem_object_is_readonly(obj))
> pte_flags |= PTE_READ_ONLY;
> + if (i915_gem_object_is_lmem(obj))
> + pte_flags |= PTE_LM;
>
> vm->insert_entries(vm, vma, cache_level, pte_flags);
> vma->page_sizes.gtt = I915_GTT_PAGE_SIZE; @@ -794,6 +803,7 @@
> 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;
>
> /* For Modern GENs the PTEs and register space are split in the BAR
> */ @@ -823,9 +833,13 @@ static int ggtt_probe_common(struct i915_ggtt
> *ggtt, u64 size)
> 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, 0);
> + I915_CACHE_NONE, pte_flags);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h
> b/drivers/gpu/drm/i915/gt/intel_gtt.h
> index 0eef625dd787..24b5808df16d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> @@ -85,7 +85,9 @@ typedef u64 gen8_pte_t;
> #define BYT_PTE_SNOOPED_BY_CPU_CACHES REG_BIT(2)
> #define BYT_PTE_WRITEABLE REG_BIT(1)
>
> -#define GEN12_PPGTT_PTE_LM BIT_ULL(11)
> +#define GEN12_PPGTT_PTE_LM BIT_ULL(11)
> +
> +#define GEN12_GGTT_PTE_LM BIT_ULL(1)
Where does the Bspec say bit-1 is for LMEM?
--CQ
>
> /*
> * Cacheability Control is a 4-bit value. The low three bits are stored in bits
> --
> 2.26.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM plumbing for GGTT
2021-02-03 16:51 ` Tang, CQ
@ 2021-02-03 17:02 ` Matthew Auld
2021-02-03 18:01 ` Tang, CQ
0 siblings, 1 reply; 8+ messages in thread
From: Matthew Auld @ 2021-02-03 17:02 UTC (permalink / raw)
To: Tang, CQ; +Cc: intel-gfx, Auld, Matthew, Chris Wilson
On Wed, 3 Feb 2021 at 16:51, Tang, CQ <cq.tang@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
> > Matthew Auld
> > Sent: Wednesday, February 3, 2021 7:24 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Subject: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM plumbing
> > for GGTT
> >
> > For the PTEs we get an LM bit, to signal whether the page resides in SMEM or
> > LMEM.
> >
> > Based on a patch from Michel Thierry.
> >
> > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/gt/intel_ggtt.c | 24 +++++++++++++++++++-----
> > drivers/gpu/drm/i915/gt/intel_gtt.h | 4 +++-
> > 2 files changed, 22 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > index fc399ac16eda..b0b8ded834f0 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > @@ -10,6 +10,8 @@
> >
> > #include <drm/i915_drm.h>
> >
> > +#include "gem/i915_gem_lmem.h"
> > +
> > #include "intel_gt.h"
> > #include "i915_drv.h"
> > #include "i915_scatterlist.h"
> > @@ -189,7 +191,12 @@ static u64 gen8_ggtt_pte_encode(dma_addr_t addr,
> > enum i915_cache_level level,
> > u32 flags)
> > {
> > - return addr | _PAGE_PRESENT;
> > + gen8_pte_t pte = addr | _PAGE_PRESENT;
> > +
> > + if (flags & PTE_LM)
> > + pte |= GEN12_GGTT_PTE_LM;
> > +
> > + return pte;
> > }
> >
> > static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte) @@ -201,13
> > +208,13 @@ static void gen8_ggtt_insert_page(struct i915_address_space
> > *vm,
> > dma_addr_t addr,
> > u64 offset,
> > enum i915_cache_level level,
> > - u32 unused)
> > + 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, 0));
> > + gen8_set_pte(pte, gen8_ggtt_pte_encode(addr, level, flags));
> >
> > ggtt->invalidate(ggtt);
> > }
> > @@ -217,7 +224,7 @@ static void gen8_ggtt_insert_entries(struct
> > i915_address_space *vm,
> > enum i915_cache_level level,
> > u32 flags)
> > {
> > - const gen8_pte_t pte_encode = gen8_ggtt_pte_encode(0, level, 0);
> > + 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;
> > @@ -459,6 +466,8 @@ static void ggtt_bind_vma(struct i915_address_space
> > *vm,
> > pte_flags = 0;
> > if (i915_gem_object_is_readonly(obj))
> > pte_flags |= PTE_READ_ONLY;
> > + if (i915_gem_object_is_lmem(obj))
> > + pte_flags |= PTE_LM;
> >
> > vm->insert_entries(vm, vma, cache_level, pte_flags);
> > vma->page_sizes.gtt = I915_GTT_PAGE_SIZE; @@ -794,6 +803,7 @@
> > 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;
> >
> > /* For Modern GENs the PTEs and register space are split in the BAR
> > */ @@ -823,9 +833,13 @@ static int ggtt_probe_common(struct i915_ggtt
> > *ggtt, u64 size)
> > 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, 0);
> > + I915_CACHE_NONE, pte_flags);
> >
> > return 0;
> > }
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h
> > b/drivers/gpu/drm/i915/gt/intel_gtt.h
> > index 0eef625dd787..24b5808df16d 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> > @@ -85,7 +85,9 @@ typedef u64 gen8_pte_t;
> > #define BYT_PTE_SNOOPED_BY_CPU_CACHES REG_BIT(2)
> > #define BYT_PTE_WRITEABLE REG_BIT(1)
> >
> > -#define GEN12_PPGTT_PTE_LM BIT_ULL(11)
> > +#define GEN12_PPGTT_PTE_LM BIT_ULL(11)
> > +
> > +#define GEN12_GGTT_PTE_LM BIT_ULL(1)
>
> Where does the Bspec say bit-1 is for LMEM?
Bspec: 45015 <- GGTT
Bspec: 45040 <- ppGTT
I'll update the commit messages.
>
> --CQ
>
> >
> > /*
> > * Cacheability Control is a 4-bit value. The low three bits are stored in bits
> > --
> > 2.26.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM plumbing for GGTT
2021-02-03 17:02 ` Matthew Auld
@ 2021-02-03 18:01 ` Tang, CQ
2021-02-03 18:32 ` Matthew Auld
0 siblings, 1 reply; 8+ messages in thread
From: Tang, CQ @ 2021-02-03 18:01 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx, Auld, Matthew, Chris Wilson
> -----Original Message-----
> From: Matthew Auld <matthew.william.auld@gmail.com>
> Sent: Wednesday, February 3, 2021 9:03 AM
> To: Tang, CQ <cq.tang@intel.com>
> Cc: Auld, Matthew <matthew.auld@intel.com>; intel-
> gfx@lists.freedesktop.org; Chris Wilson <chris@chris-wilson.co.uk>
> Subject: Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM
> plumbing for GGTT
>
> On Wed, 3 Feb 2021 at 16:51, Tang, CQ <cq.tang@intel.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
> > > Of Matthew Auld
> > > Sent: Wednesday, February 3, 2021 7:24 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Subject: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM
> > > plumbing for GGTT
> > >
> > > For the PTEs we get an LM bit, to signal whether the page resides in
> > > SMEM or LMEM.
> > >
> > > Based on a patch from Michel Thierry.
> > >
> > > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Signed-off-by: Daniele Ceraolo Spurio
> > > <daniele.ceraolospurio@intel.com>
> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > > drivers/gpu/drm/i915/gt/intel_ggtt.c | 24 +++++++++++++++++++-----
> > > drivers/gpu/drm/i915/gt/intel_gtt.h | 4 +++-
> > > 2 files changed, 22 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > index fc399ac16eda..b0b8ded834f0 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > @@ -10,6 +10,8 @@
> > >
> > > #include <drm/i915_drm.h>
> > >
> > > +#include "gem/i915_gem_lmem.h"
> > > +
> > > #include "intel_gt.h"
> > > #include "i915_drv.h"
> > > #include "i915_scatterlist.h"
> > > @@ -189,7 +191,12 @@ static u64 gen8_ggtt_pte_encode(dma_addr_t
> addr,
> > > enum i915_cache_level level,
> > > u32 flags) {
> > > - return addr | _PAGE_PRESENT;
> > > + gen8_pte_t pte = addr | _PAGE_PRESENT;
> > > +
> > > + if (flags & PTE_LM)
> > > + pte |= GEN12_GGTT_PTE_LM;
> > > +
> > > + return pte;
> > > }
> > >
> > > static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte) @@
> > > -201,13
> > > +208,13 @@ static void gen8_ggtt_insert_page(struct
> > > +i915_address_space
> > > *vm,
> > > dma_addr_t addr,
> > > u64 offset,
> > > enum i915_cache_level level,
> > > - u32 unused)
> > > + 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, 0));
> > > + gen8_set_pte(pte, gen8_ggtt_pte_encode(addr, level, flags));
> > >
> > > ggtt->invalidate(ggtt);
> > > }
> > > @@ -217,7 +224,7 @@ static void gen8_ggtt_insert_entries(struct
> > > i915_address_space *vm,
> > > enum i915_cache_level level,
> > > u32 flags) {
> > > - const gen8_pte_t pte_encode = gen8_ggtt_pte_encode(0, level, 0);
> > > + 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;
> > > @@ -459,6 +466,8 @@ static void ggtt_bind_vma(struct
> > > i915_address_space *vm,
> > > pte_flags = 0;
> > > if (i915_gem_object_is_readonly(obj))
> > > pte_flags |= PTE_READ_ONLY;
> > > + if (i915_gem_object_is_lmem(obj))
> > > + pte_flags |= PTE_LM;
> > >
> > > vm->insert_entries(vm, vma, cache_level, pte_flags);
> > > vma->page_sizes.gtt = I915_GTT_PAGE_SIZE; @@ -794,6 +803,7 @@
> > > 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;
> > >
> > > /* For Modern GENs the PTEs and register space are split in
> > > the BAR */ @@ -823,9 +833,13 @@ static int ggtt_probe_common(struct
> > > i915_ggtt *ggtt, u64 size)
> > > 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, 0);
> > > + I915_CACHE_NONE, pte_flags);
> > >
> > > return 0;
> > > }
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h
> > > b/drivers/gpu/drm/i915/gt/intel_gtt.h
> > > index 0eef625dd787..24b5808df16d 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> > > +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> > > @@ -85,7 +85,9 @@ typedef u64 gen8_pte_t;
> > > #define BYT_PTE_SNOOPED_BY_CPU_CACHES REG_BIT(2)
> > > #define BYT_PTE_WRITEABLE REG_BIT(1)
> > >
> > > -#define GEN12_PPGTT_PTE_LM BIT_ULL(11)
> > > +#define GEN12_PPGTT_PTE_LM BIT_ULL(11)
> > > +
> > > +#define GEN12_GGTT_PTE_LM BIT_ULL(1)
> >
> > Where does the Bspec say bit-1 is for LMEM?
>
> Bspec: 45015 <- GGTT
> Bspec: 45040 <- ppGTT
I looked both document, I don't see bit-1 is used as DM indicator, it is R/W bit. I also see bit-11 is ignored.
The only place I see bit-11 as DM is Bspec 53521
--CQ
>
> I'll update the commit messages.
>
> >
> > --CQ
> >
> > >
> > > /*
> > > * Cacheability Control is a 4-bit value. The low three bits are
> > > stored in bits
> > > --
> > > 2.26.2
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM plumbing for GGTT
2021-02-03 18:01 ` Tang, CQ
@ 2021-02-03 18:32 ` Matthew Auld
2021-02-04 17:43 ` Piotr Piórkowski
0 siblings, 1 reply; 8+ messages in thread
From: Matthew Auld @ 2021-02-03 18:32 UTC (permalink / raw)
To: Tang, CQ; +Cc: intel-gfx, Auld, Matthew, Chris Wilson
On Wed, 3 Feb 2021 at 18:01, Tang, CQ <cq.tang@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Matthew Auld <matthew.william.auld@gmail.com>
> > Sent: Wednesday, February 3, 2021 9:03 AM
> > To: Tang, CQ <cq.tang@intel.com>
> > Cc: Auld, Matthew <matthew.auld@intel.com>; intel-
> > gfx@lists.freedesktop.org; Chris Wilson <chris@chris-wilson.co.uk>
> > Subject: Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM
> > plumbing for GGTT
> >
> > On Wed, 3 Feb 2021 at 16:51, Tang, CQ <cq.tang@intel.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
> > > > Of Matthew Auld
> > > > Sent: Wednesday, February 3, 2021 7:24 AM
> > > > To: intel-gfx@lists.freedesktop.org
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Subject: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM
> > > > plumbing for GGTT
> > > >
> > > > For the PTEs we get an LM bit, to signal whether the page resides in
> > > > SMEM or LMEM.
> > > >
> > > > Based on a patch from Michel Thierry.
> > > >
> > > > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > > Signed-off-by: Daniele Ceraolo Spurio
> > > > <daniele.ceraolospurio@intel.com>
> > > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > ---
> > > > drivers/gpu/drm/i915/gt/intel_ggtt.c | 24 +++++++++++++++++++-----
> > > > drivers/gpu/drm/i915/gt/intel_gtt.h | 4 +++-
> > > > 2 files changed, 22 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > > b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > > index fc399ac16eda..b0b8ded834f0 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > > @@ -10,6 +10,8 @@
> > > >
> > > > #include <drm/i915_drm.h>
> > > >
> > > > +#include "gem/i915_gem_lmem.h"
> > > > +
> > > > #include "intel_gt.h"
> > > > #include "i915_drv.h"
> > > > #include "i915_scatterlist.h"
> > > > @@ -189,7 +191,12 @@ static u64 gen8_ggtt_pte_encode(dma_addr_t
> > addr,
> > > > enum i915_cache_level level,
> > > > u32 flags) {
> > > > - return addr | _PAGE_PRESENT;
> > > > + gen8_pte_t pte = addr | _PAGE_PRESENT;
> > > > +
> > > > + if (flags & PTE_LM)
> > > > + pte |= GEN12_GGTT_PTE_LM;
> > > > +
> > > > + return pte;
> > > > }
> > > >
> > > > static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte) @@
> > > > -201,13
> > > > +208,13 @@ static void gen8_ggtt_insert_page(struct
> > > > +i915_address_space
> > > > *vm,
> > > > dma_addr_t addr,
> > > > u64 offset,
> > > > enum i915_cache_level level,
> > > > - u32 unused)
> > > > + 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, 0));
> > > > + gen8_set_pte(pte, gen8_ggtt_pte_encode(addr, level, flags));
> > > >
> > > > ggtt->invalidate(ggtt);
> > > > }
> > > > @@ -217,7 +224,7 @@ static void gen8_ggtt_insert_entries(struct
> > > > i915_address_space *vm,
> > > > enum i915_cache_level level,
> > > > u32 flags) {
> > > > - const gen8_pte_t pte_encode = gen8_ggtt_pte_encode(0, level, 0);
> > > > + 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;
> > > > @@ -459,6 +466,8 @@ static void ggtt_bind_vma(struct
> > > > i915_address_space *vm,
> > > > pte_flags = 0;
> > > > if (i915_gem_object_is_readonly(obj))
> > > > pte_flags |= PTE_READ_ONLY;
> > > > + if (i915_gem_object_is_lmem(obj))
> > > > + pte_flags |= PTE_LM;
> > > >
> > > > vm->insert_entries(vm, vma, cache_level, pte_flags);
> > > > vma->page_sizes.gtt = I915_GTT_PAGE_SIZE; @@ -794,6 +803,7 @@
> > > > 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;
> > > >
> > > > /* For Modern GENs the PTEs and register space are split in
> > > > the BAR */ @@ -823,9 +833,13 @@ static int ggtt_probe_common(struct
> > > > i915_ggtt *ggtt, u64 size)
> > > > 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, 0);
> > > > + I915_CACHE_NONE, pte_flags);
> > > >
> > > > return 0;
> > > > }
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h
> > > > b/drivers/gpu/drm/i915/gt/intel_gtt.h
> > > > index 0eef625dd787..24b5808df16d 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> > > > @@ -85,7 +85,9 @@ typedef u64 gen8_pte_t;
> > > > #define BYT_PTE_SNOOPED_BY_CPU_CACHES REG_BIT(2)
> > > > #define BYT_PTE_WRITEABLE REG_BIT(1)
> > > >
> > > > -#define GEN12_PPGTT_PTE_LM BIT_ULL(11)
> > > > +#define GEN12_PPGTT_PTE_LM BIT_ULL(11)
> > > > +
> > > > +#define GEN12_GGTT_PTE_LM BIT_ULL(1)
> > >
> > > Where does the Bspec say bit-1 is for LMEM?
> >
> > Bspec: 45015 <- GGTT
> > Bspec: 45040 <- ppGTT
>
> I looked both document, I don't see bit-1 is used as DM indicator, it is R/W bit. I also see bit-11 is ignored.
> The only place I see bit-11 as DM is Bspec 53521
I see bit-1 as "Local Memory" in 45015, and bit-11 as "Local Memory"
in 45040. Also there is no R/W bit for the GGTT on DG1, so not sure
what version of the spec you are looking at. Just to be clear there
are two different bits here, one for the ppGTT and one for the GGTT,
they both have different layouts for their respective PTEs.
Bspec: 53521 is not applicable to DG1.
>
> --CQ
>
>
> >
> > I'll update the commit messages.
> >
> > >
> > > --CQ
> > >
> > > >
> > > > /*
> > > > * Cacheability Control is a 4-bit value. The low three bits are
> > > > stored in bits
> > > > --
> > > > 2.26.2
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM plumbing for GGTT
2021-02-03 18:32 ` Matthew Auld
@ 2021-02-04 17:43 ` Piotr Piórkowski
0 siblings, 0 replies; 8+ messages in thread
From: Piotr Piórkowski @ 2021-02-04 17:43 UTC (permalink / raw)
To: Matthew Auld, Tang, CQ; +Cc: Chris Wilson, intel-gfx, Auld, Matthew
Matthew Auld <matthew.william.auld@gmail.com> wrote on śro [2021-lut-03 18:32:34 +0000]:
> On Wed, 3 Feb 2021 at 18:01, Tang, CQ <cq.tang@intel.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Matthew Auld <matthew.william.auld@gmail.com>
> > > Sent: Wednesday, February 3, 2021 9:03 AM
> > > To: Tang, CQ <cq.tang@intel.com>
> > > Cc: Auld, Matthew <matthew.auld@intel.com>; intel-
> > > gfx@lists.freedesktop.org; Chris Wilson <chris@chris-wilson.co.uk>
> > > Subject: Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM
> > > plumbing for GGTT
> > >
> > > On Wed, 3 Feb 2021 at 16:51, Tang, CQ <cq.tang@intel.com> wrote:
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
> > > > > Of Matthew Auld
> > > > > Sent: Wednesday, February 3, 2021 7:24 AM
> > > > > To: intel-gfx@lists.freedesktop.org
> > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Subject: [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM
> > > > > plumbing for GGTT
> > > > >
> > > > > For the PTEs we get an LM bit, to signal whether the page resides in
> > > > > SMEM or LMEM.
> > > > >
> > > > > Based on a patch from Michel Thierry.
> > > > >
> > > > > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > > > Signed-off-by: Daniele Ceraolo Spurio
> > > > > <daniele.ceraolospurio@intel.com>
> > > > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > ---
> > > > > drivers/gpu/drm/i915/gt/intel_ggtt.c | 24 +++++++++++++++++++-----
> > > > > drivers/gpu/drm/i915/gt/intel_gtt.h | 4 +++-
> > > > > 2 files changed, 22 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > > > b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > > > index fc399ac16eda..b0b8ded834f0 100644
> > > > > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > > > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > > > > @@ -10,6 +10,8 @@
> > > > >
> > > > > #include <drm/i915_drm.h>
> > > > >
> > > > > +#include "gem/i915_gem_lmem.h"
> > > > > +
> > > > > #include "intel_gt.h"
> > > > > #include "i915_drv.h"
> > > > > #include "i915_scatterlist.h"
> > > > > @@ -189,7 +191,12 @@ static u64 gen8_ggtt_pte_encode(dma_addr_t
> > > addr,
> > > > > enum i915_cache_level level,
> > > > > u32 flags) {
> > > > > - return addr | _PAGE_PRESENT;
> > > > > + gen8_pte_t pte = addr | _PAGE_PRESENT;
> > > > > +
> > > > > + if (flags & PTE_LM)
> > > > > + pte |= GEN12_GGTT_PTE_LM;
> > > > > +
> > > > > + return pte;
> > > > > }
> > > > >
> > > > > static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte) @@
> > > > > -201,13
> > > > > +208,13 @@ static void gen8_ggtt_insert_page(struct
> > > > > +i915_address_space
> > > > > *vm,
> > > > > dma_addr_t addr,
> > > > > u64 offset,
> > > > > enum i915_cache_level level,
> > > > > - u32 unused)
> > > > > + 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, 0));
> > > > > + gen8_set_pte(pte, gen8_ggtt_pte_encode(addr, level, flags));
> > > > >
> > > > > ggtt->invalidate(ggtt);
> > > > > }
> > > > > @@ -217,7 +224,7 @@ static void gen8_ggtt_insert_entries(struct
> > > > > i915_address_space *vm,
> > > > > enum i915_cache_level level,
> > > > > u32 flags) {
> > > > > - const gen8_pte_t pte_encode = gen8_ggtt_pte_encode(0, level, 0);
> > > > > + 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;
> > > > > @@ -459,6 +466,8 @@ static void ggtt_bind_vma(struct
> > > > > i915_address_space *vm,
> > > > > pte_flags = 0;
> > > > > if (i915_gem_object_is_readonly(obj))
> > > > > pte_flags |= PTE_READ_ONLY;
> > > > > + if (i915_gem_object_is_lmem(obj))
> > > > > + pte_flags |= PTE_LM;
> > > > >
> > > > > vm->insert_entries(vm, vma, cache_level, pte_flags);
> > > > > vma->page_sizes.gtt = I915_GTT_PAGE_SIZE; @@ -794,6 +803,7 @@
> > > > > 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;
> > > > >
> > > > > /* For Modern GENs the PTEs and register space are split in
> > > > > the BAR */ @@ -823,9 +833,13 @@ static int ggtt_probe_common(struct
> > > > > i915_ggtt *ggtt, u64 size)
> > > > > 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, 0);
> > > > > + I915_CACHE_NONE, pte_flags);
> > > > >
> > > > > return 0;
> > > > > }
> > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h
> > > > > b/drivers/gpu/drm/i915/gt/intel_gtt.h
> > > > > index 0eef625dd787..24b5808df16d 100644
> > > > > --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> > > > > +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> > > > > @@ -85,7 +85,9 @@ typedef u64 gen8_pte_t;
> > > > > #define BYT_PTE_SNOOPED_BY_CPU_CACHES REG_BIT(2)
> > > > > #define BYT_PTE_WRITEABLE REG_BIT(1)
> > > > >
> > > > > -#define GEN12_PPGTT_PTE_LM BIT_ULL(11)
> > > > > +#define GEN12_PPGTT_PTE_LM BIT_ULL(11)
> > > > > +
> > > > > +#define GEN12_GGTT_PTE_LM BIT_ULL(1)
> > > >
> > > > Where does the Bspec say bit-1 is for LMEM?
> > >
> > > Bspec: 45015 <- GGTT
> > > Bspec: 45040 <- ppGTT
> >
> > I looked both document, I don't see bit-1 is used as DM indicator, it is R/W bit. I also see bit-11 is ignored.
> > The only place I see bit-11 as DM is Bspec 53521
>
> I see bit-1 as "Local Memory" in 45015, and bit-11 as "Local Memory"
> in 45040. Also there is no R/W bit for the GGTT on DG1, so not sure
> what version of the spec you are looking at. Just to be clear there
> are two different bits here, one for the ppGTT and one for the GGTT,
> they both have different layouts for their respective PTEs.
>
> Bspec: 53521 is not applicable to DG1.
Matthew has right. Bspec 45015 is applicable here for GGTT.
I'm currently working on an internal series where I'm cleaning this area of code.
Based on 45015 I prepared this table:
/*
* GEN12 GGTT Table Entry format:
* 63:54 | 53:52 | 51:46 | 45:12 | 11:2 | 1 | 0
* ignored | PAT | ignored | address | ignored | LM | present
*/
Piotr
>
> >
> > --CQ
> >
> >
> > >
> > > I'll update the commit messages.
> > >
> > > >
> > > > --CQ
> > > >
> > > > >
> > > > > /*
> > > > > * Cacheability Control is a 4-bit value. The low three bits are
> > > > > stored in bits
> > > > > --
> > > > > 2.26.2
> > > > >
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-02-04 17:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-03 15:23 [Intel-gfx] [PATCH v3 1/3] drm/i915: Distinction of memory regions Matthew Auld
2021-02-03 15:23 ` [Intel-gfx] [PATCH v3 2/3] drm/i915/gtt/dg1: add PTE_LM plumbing for ppGTT Matthew Auld
2021-02-03 15:23 ` [Intel-gfx] [PATCH v3 3/3] drm/i915/gtt/dg1: add PTE_LM plumbing for GGTT Matthew Auld
2021-02-03 16:51 ` Tang, CQ
2021-02-03 17:02 ` Matthew Auld
2021-02-03 18:01 ` Tang, CQ
2021-02-03 18:32 ` Matthew Auld
2021-02-04 17:43 ` Piotr Piórkowski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.