Underneath it's the same stuff, so things like the PTE_LM bits for the GTT should just keep working as-is.

Signed-off-by: Matthew Auld matthew.auld@intel.com Reviewed-by: Tvrtko Ursulin tvrtko.ursulin@intel.com --- drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c index ce1c83c13d05..017db8f71130 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c @@ -19,7 +19,10 @@ const struct drm_i915_gem_object_ops i915_gem_lmem_obj_ops = {

bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj) { - return obj->ops == &i915_gem_lmem_obj_ops; + struct intel_memory_region *mr = obj->mm.region; + + return mr && (mr->type == INTEL_MEMORY_LOCAL || + mr->type == INTEL_MEMORY_STOLEN_LOCAL); }

struct drm_i915_gem_object *

Stolen memory is always allocated as physically contiguous pages, so mark the object flags as such. It looks like the flags were previously just ignored so this had no effect. In the future we might to add the proper plumbing for passing the flags all over the way down from the caller, but for now we don't have a use for that.

Signed-off-by: Matthew Auld matthew.auld@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com --- drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c index b43929da8de8..c5b64b2400e8 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c @@ -637,10 +637,17 @@ static int __i915_gem_object_create_stolen(struct intel_memory_region *mem, { static struct lock_class_key lock_class; unsigned int cache_level; + unsigned int flags; int err;

+ /* + * Stolen objects are always physically contiguous since we just + * allocate one big block underneath using the drm_mm range allocator. + */ + flags = I915_BO_ALLOC_CONTIGUOUS; + drm_gem_private_object_init(&mem->i915->drm, &obj->base, stolen->size); - i915_gem_object_init(obj, &i915_gem_object_stolen_ops, &lock_class, 0); + i915_gem_object_init(obj, &i915_gem_object_stolen_ops, &lock_class, flags);

obj->stolen = stolen; obj->read_domains = I915_GEM_DOMAIN_CPU | I915_GEM_DOMAIN_GTT; @@ -699,8 +706,7 @@ struct drm_i915_gem_object * i915_gem_object_create_stolen(struct drm_i915_private *i915, resource_size_t size) { - return i915_gem_object_create_region(i915->mm.stolen_region, - size, I915_BO_ALLOC_CONTIGUOUS); + return i915_gem_object_create_region(i915->mm.stolen_region, size, 0); }

static int init_stolen_smem(struct intel_memory_region *mem)

On 21/04/2021 11:46, Matthew Auld wrote:

From: CQ Tang cq.tang@intel.com

Add "REGION_STOLEN" device info to dg1, create stolen memory region from upper portion of local device memory, starting from DSMBASE.

v2: - s/drm_info/drm_dbg; userspace likely doesn't care about stolen. - mem->type is only setup after the region probe, so setting the name as stolen-local or stolen-system based on this value won't work. Split system vs local stolen setup to fix this. - kill all the region->devmem/is_devmem stuff. We already differentiate the different types of stolen so such things shouldn't be needed anymore. v3: - split stolen lmem vs smem ops(Tvrtko) - add shortcut for stolen region in i915(Tvrtko) - sanity check dsm base vs bar size(Xinyun) v4(Tvrtko): - more cleanup - add some TODOs

Signed-off-by: CQ Tang cq.tang@intel.com Signed-off-by: Matthew Auld matthew.auld@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Xinyun Liu xinyun.liu@intel.com

---

drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 132 ++++++++++++++++++--- drivers/gpu/drm/i915/gem/i915_gem_stolen.h | 3 +- drivers/gpu/drm/i915/i915_drv.h | 7 ++ drivers/gpu/drm/i915/i915_pci.c | 2 +- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_memory_region.c | 13 +- drivers/gpu/drm/i915/intel_memory_region.h | 5 +- 7 files changed, 140 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c index b0597de206de..13a7932cfe1a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c @@ -10,6 +10,7 @@ #include <drm/drm_mm.h> #include <drm/i915_drm.h>

+#include "gem/i915_gem_lmem.h" #include "gem/i915_gem_region.h" #include "i915_drv.h" #include "i915_gem_stolen.h" @@ -121,6 +122,14 @@ static int i915_adjust_stolen(struct drm_i915_private *i915, } }

• /*

• * With stolen lmem, we don't need to check if the address range
  

• * overlaps with the non-stolen system memory range, since lmem is local
  

• * to the gpu.
  

• */
  

• if (HAS_LMEM(i915))

• return 0;
  

• /*
  • Verify that nothing else uses this physical address. Stolen
  • memory should be reserved by the BIOS and hidden from the

@@ -374,8 +383,9 @@ static void icl_get_stolen_reserved(struct drm_i915_private *i915, } }

-static int i915_gem_init_stolen(struct drm_i915_private *i915) +static int i915_gem_init_stolen(struct intel_memory_region *mem) {

• struct drm_i915_private *i915 = mem->i915; struct intel_uncore *uncore = &i915->uncore; resource_size_t reserved_base, stolen_top; resource_size_t reserved_total, reserved_size;

@@ -396,10 +406,10 @@ static int i915_gem_init_stolen(struct drm_i915_private *i915) return 0; }

• if (resource_size(&intel_graphics_stolen_res) == 0)

• if (resource_size(&mem->region) == 0) return 0;

• i915->dsm = intel_graphics_stolen_res;

• i915->dsm = mem->region;

  if (i915_adjust_stolen(i915, &i915->dsm)) return 0;

@@ -688,39 +698,123 @@ struct drm_i915_gem_object * i915_gem_object_create_stolen(struct drm_i915_private *i915, resource_size_t size) {

• return i915_gem_object_create_region(i915->mm.regions[INTEL_REGION_STOLEN_SMEM],

• return i915_gem_object_create_region(i915->mm.stolen_region, size, I915_BO_ALLOC_CONTIGUOUS); }

-static int init_stolen(struct intel_memory_region *mem) +static int init_stolen_smem(struct intel_memory_region *mem) {

• intel_memory_region_set_name(mem, "stolen");
• /*
  • Initialise stolen early so that we may reserve preallocated
  • objects for the BIOS to KMS transition.
  */
• return i915_gem_init_stolen(mem->i915);

• return i915_gem_init_stolen(mem); }

-static void release_stolen(struct intel_memory_region *mem) +static void release_stolen_smem(struct intel_memory_region *mem) { i915_gem_cleanup_stolen(mem->i915); }

-static const struct intel_memory_region_ops i915_region_stolen_ops = {

• .init = init_stolen,
• .release = release_stolen,

+static const struct intel_memory_region_ops i915_region_stolen_smem_ops = {

• .init = init_stolen_smem,
• .release = release_stolen_smem, .init_object = _i915_gem_object_stolen_init, };

-struct intel_memory_region *i915_gem_stolen_setup(struct drm_i915_private *i915) +static int init_stolen_lmem(struct intel_memory_region *mem) +{

• int err;
• if (GEM_WARN_ON(resource_size(&mem->region) == 0))

• return -ENODEV;
  

• if (!io_mapping_init_wc(&mem->iomap,

• 		mem->io_start,
  

• 		resource_size(&mem->region)))
  

• return -EIO;
  

• /*

• * TODO: For stolen lmem we mostly just care about populating the dsm
  

• * related bits and setting up the drm_mm allocator for the range.
  

• * Perhaps split up i915_gem_init_stolen() for this.
  

• */
  

• err = i915_gem_init_stolen(mem);
• if (err)

• goto err_fini;
  

• return 0;

+err_fini:

• io_mapping_fini(&mem->iomap);
• return err;

+}

+static void release_stolen_lmem(struct intel_memory_region *mem) {

• return intel_memory_region_create(i915,

• 			  intel_graphics_stolen_res.start,
  

• 			  resource_size(&intel_graphics_stolen_res),
  

• 			  PAGE_SIZE, 0,
  

• 			  &i915_region_stolen_ops);
  

• io_mapping_fini(&mem->iomap);
• i915_gem_cleanup_stolen(mem->i915);

+}

+static const struct intel_memory_region_ops i915_region_stolen_lmem_ops = {

• .init = init_stolen_lmem,
• .release = release_stolen_lmem,
• .init_object = _i915_gem_object_stolen_init,

+};

+struct intel_memory_region * +i915_gem_stolen_lmem_setup(struct drm_i915_private *i915) +{

• struct intel_uncore *uncore = &i915->uncore;
• struct pci_dev *pdev = i915->drm.pdev;
• struct intel_memory_region *mem;
• resource_size_t io_start;
• resource_size_t lmem_size;
• u64 lmem_base;
• lmem_base = intel_uncore_read64(uncore, GEN12_DSMBASE);
• if (GEM_WARN_ON(lmem_base >= pci_resource_len(pdev, 2)))

• return ERR_PTR(-ENODEV);
  

• lmem_size = pci_resource_len(pdev, 2) - lmem_base;
• io_start = pci_resource_start(pdev, 2) + lmem_base;
• mem = intel_memory_region_create(i915, lmem_base, lmem_size,

• 			 I915_GTT_PAGE_SIZE_4K, io_start,
  

• 			 &i915_region_stolen_lmem_ops);
  

• if (IS_ERR(mem))

• return mem;
  

• /*

• * TODO: consider creating common helper to just print all the
  

• * interesting stuff from intel_memory_region, which we can use for all
  

• * our probed regions.
  

• */
  

• drm_dbg(&i915->drm, "Stolen Local memory IO start: %pa\n",

• &mem->io_start);
  

• intel_memory_region_set_name(mem, "stolen-local");
• return mem;

+}

+struct intel_memory_region* +i915_gem_stolen_smem_setup(struct drm_i915_private *i915) +{

• struct intel_memory_region *mem;

• mem = intel_memory_region_create(i915,

• 			 intel_graphics_stolen_res.start,
  

• 			 resource_size(&intel_graphics_stolen_res),
  

• 			 PAGE_SIZE, 0,
  

• 			 &i915_region_stolen_smem_ops);
  

• if (IS_ERR(mem))

• return mem;
  

• intel_memory_region_set_name(mem, "stolen-system");

• return mem; }

  struct drm_i915_gem_object *

@@ -728,7 +822,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private *i915, resource_size_t stolen_offset, resource_size_t size) {

• struct intel_memory_region *mem = i915->mm.regions[INTEL_REGION_STOLEN_SMEM];

• struct intel_memory_region *mem = i915->mm.stolen_region; struct drm_i915_gem_object *obj; struct drm_mm_node *stolen; int ret;

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h index b03489706796..2bec6c367b9c 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h @@ -21,7 +21,8 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv, u64 end); void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv, struct drm_mm_node *node); -struct intel_memory_region *i915_gem_stolen_setup(struct drm_i915_private *i915); +struct intel_memory_region *i915_gem_stolen_smem_setup(struct drm_i915_private *i915); +struct intel_memory_region *i915_gem_stolen_lmem_setup(struct drm_i915_private *i915); struct drm_i915_gem_object * i915_gem_object_create_stolen(struct drm_i915_private *dev_priv, resource_size_t size); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e20294e9227a..0b44333eb703 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -514,6 +514,13 @@ struct intel_l3_parity { };

struct i915_gem_mm {

• /*

• * Shortcut for the stolen region. This points to either
  

• * INTEL_REGION_STOLEN_SMEM for integrated platforms, or
  

• * INTEL_REGION_STOLEN_LMEM for discrete, or NULL if the device doesn't
  

• * support stolen.
  

• */
  

• struct intel_memory_region *stolen_region; /** Memory allocator for GTT stolen memory */ struct drm_mm stolen; /** Protects the usage of the GTT stolen memory allocator. This is

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 7786217638ed..c678e0663d80 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -908,7 +908,7 @@ static const struct intel_device_info rkl_info = { };

#define DGFX_FEATURES \

• .memory_regions = REGION_SMEM | REGION_LMEM, \

• .memory_regions = REGION_SMEM | REGION_LMEM | REGION_STOLEN_LMEM, \ .has_master_unit_irq = 1, \ .has_llc = 0, \ .has_snoop = 1, \

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index f80d656331f4..ea20058bc13f 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -12191,6 +12191,7 @@ enum skl_power_gate { #define GEN12_GLOBAL_MOCS(i) _MMIO(0x4000 + (i) * 4) /* Global MOCS regs */

#define GEN12_GSMBASE _MMIO(0x108100) +#define GEN12_DSMBASE _MMIO(0x1080C0)

/* gamt regs */ #define GEN8_L3_LRA_1_GPGPU _MMIO(0x4dd4) diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c index bf837b6bb185..481a487faca6 100644 --- a/drivers/gpu/drm/i915/intel_memory_region.c +++ b/drivers/gpu/drm/i915/intel_memory_region.c @@ -22,6 +22,10 @@ static const struct { .class = INTEL_MEMORY_STOLEN_SYSTEM, .instance = 0, },

• [INTEL_REGION_STOLEN_LMEM] = {

• .class = INTEL_MEMORY_STOLEN_LOCAL,
  

• .instance = 0,
  

• }, };

  struct intel_memory_region *

@@ -278,8 +282,15 @@ int intel_memory_regions_hw_probe(struct drm_i915_private *i915) case INTEL_MEMORY_SYSTEM: mem = i915_gem_shmem_setup(i915); break;

• case INTEL_MEMORY_STOLEN_LOCAL:
  

• 	mem = i915_gem_stolen_lmem_setup(i915);
  

• 	if (!IS_ERR(mem))
  

• 		i915->mm.stolen_region = mem;
  

• 	break;
  

  case INTEL_MEMORY_STOLEN_SYSTEM:

• 	mem = i915_gem_stolen_setup(i915);
  

• 	mem = i915_gem_stolen_smem_setup(i915);
  

• 	if (!IS_ERR(mem))
  

• 		i915->mm.stolen_region = mem;
  break;
  

  default: continue;

diff --git a/drivers/gpu/drm/i915/intel_memory_region.h b/drivers/gpu/drm/i915/intel_memory_region.h index edd49067c8ca..4c8ec15af55f 100644 --- a/drivers/gpu/drm/i915/intel_memory_region.h +++ b/drivers/gpu/drm/i915/intel_memory_region.h @@ -26,18 +26,21 @@ enum intel_memory_type { INTEL_MEMORY_SYSTEM = 0, INTEL_MEMORY_LOCAL, INTEL_MEMORY_STOLEN_SYSTEM,

• INTEL_MEMORY_STOLEN_LOCAL, };

  enum intel_region_id { INTEL_REGION_SMEM = 0, INTEL_REGION_LMEM, INTEL_REGION_STOLEN_SMEM,

• INTEL_REGION_STOLEN_LMEM, INTEL_REGION_UNKNOWN, /* Should be last */ };

  #define REGION_SMEM BIT(INTEL_REGION_SMEM) #define REGION_LMEM BIT(INTEL_REGION_LMEM) #define REGION_STOLEN_SMEM BIT(INTEL_REGION_STOLEN_SMEM)

+#define REGION_STOLEN_LMEM BIT(INTEL_REGION_STOLEN_LMEM)

#define I915_ALLOC_MIN_PAGE_SIZE BIT(0) #define I915_ALLOC_CONTIGUOUS BIT(1) @@ -82,7 +85,7 @@ struct intel_memory_region { u16 type; u16 instance; enum intel_region_id id;

• char name[8];

• char name[16];

  struct list_head reserved;

With the TODOs in place, I think I can finally see the road to clean stolen setup. Thanks for refining it at this stage.

Reviewed-by: Tvrtko Ursulin tvrtko.ursulin@intel.com

Regards,

Tvrtko