[Intel-xe] [PATCH 1/2] drm/xe: Move the memory region struct out xe_tile
Ruhl, Michael J
michael.j.ruhl at intel.com
Tue Aug 8 20:42:14 UTC 2023
>-----Original Message-----
>From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Oak
>Zeng
>Sent: Monday, August 7, 2023 10:44 PM
>To: intel-xe at lists.freedesktop.org
>Cc: --cc=michael.j.ruhl at intel.com
>Subject: [Intel-xe] [PATCH 1/2] drm/xe: Move the memory region struct out
>xe_tile
>
>Make a xe_mem_region structure which will be used in the
>coming patches. The new structure is used in both xe device
>level (xe->mem.vram) and xe_tile level (tile->vram).
>
>Make the definition of xe_mem_region.base to be the DPA of
>the vram and change codes according to this new definition.
>
>Also add a pagemap member to this structure. pagemap will be
>used to map GPU local memory to CPU's HPA space as ZONE_DEVICE.
>This is preparation work for introducing svm (shared virtual memory)
>
>Signed-off-by: Oak Zeng <oak.zeng at intel.com>
>---
> drivers/gpu/drm/xe/xe_bo.c | 2 +-
> drivers/gpu/drm/xe/xe_device_types.h | 96 ++++++++++++----------------
> drivers/gpu/drm/xe/xe_migrate.c | 2 +-
> drivers/gpu/drm/xe/xe_mmio.c | 7 +-
> 4 files changed, 48 insertions(+), 59 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
>index 284c86107a5f..a397a7224a99 100644
>--- a/drivers/gpu/drm/xe/xe_bo.c
>+++ b/drivers/gpu/drm/xe/xe_bo.c
>@@ -1457,7 +1457,7 @@ uint64_t vram_region_gpu_offset(struct
>ttm_resource *res)
> if (res->mem_type == XE_PL_STOLEN)
> return xe_ttm_stolen_gpu_offset(xe);
>
>- return xe->mem.vram.base + tile->mem.vram.base;
>+ return tile->mem.vram.base;
> }
>
> /**
>diff --git a/drivers/gpu/drm/xe/xe_device_types.h
>b/drivers/gpu/drm/xe/xe_device_types.h
>index f84ecb976f5d..a454a5098121 100644
>--- a/drivers/gpu/drm/xe/xe_device_types.h
>+++ b/drivers/gpu/drm/xe/xe_device_types.h
>@@ -57,6 +57,46 @@ struct xe_ggtt;
> const struct xe_tile * : (const struct xe_device *)((tile__)-
>>xe), \
> struct xe_tile * : (tile__)->xe)
>
>+/**
>+ * struct xe_mem_region - memory region structure
>+ * This is used to describe a memory region in xe
>+ * device, such as HBM memory or CXL extension memory.
>+ */
>+struct xe_mem_region {
>+ /** @io_start: IO start address of this VRAM instance */
>+ resource_size_t io_start;
>+ /**
>+ * @io_size: IO size of this VRAM instance
>+ *
>+ * This represents how much of this VRAM we can access
>+ * via the CPU through the VRAM BAR. This can be smaller
>+ * than @usable_size, in which case only part of VRAM is CPU
>+ * accessible (typically the first 256M). This
>+ * configuration is known as small-bar.
>+ */
>+ resource_size_t io_size;
>+ /** @base: This memory regions's DPA (device physical address) */
>+ resource_size_t base;
I think you can eliminate this value.... Like your comment (later indicates) this
value will be derived differently and keeping it here doesn't have any benefit.
>+ /**
>+ * @usable_size: usable size of VRAM
>+ *
>+ * Usable size of VRAM excluding reserved portions
>+ * (e.g stolen mem)
>+ */
>+ resource_size_t usable_size;
>+ /**
>+ * @actual_physical_size: Actual VRAM size
>+ *
>+ * Actual VRAM size including reserved portions
>+ * (e.g stolen mem)
>+ */
>+ resource_size_t actual_physical_size;
>+ /** @mapping: pointer to VRAM mappable space */
>+ void *__iomem mapping;
>+ /** @pagemap: Used to map device memory as ZONE_DEVICE */
>+ struct dev_pagemap pagemap;
>+};
>+
> /**
> * struct xe_tile - hardware tile structure
> *
>@@ -114,38 +154,7 @@ struct xe_tile {
> * Although VRAM is associated with a specific tile, it can
> * still be accessed by all tiles' GTs.
> */
>- struct {
>- /** @io_start: IO start address of this VRAM instance
>*/
>- resource_size_t io_start;
>- /**
>- * @io_size: IO size of this VRAM instance
>- *
>- * This represents how much of this VRAM we can
>access
>- * via the CPU through the VRAM BAR. This can be
>smaller
>- * than @size, in which case only part of VRAM is CPU
>- * accessible (typically the first 256M). This
>- * configuration is known as small-bar.
>- */
>- resource_size_t io_size;
>- /** @base: offset of VRAM starting base */
>- resource_size_t base;
>- /**
>- * @usable_size: usable size of VRAM
>- *
>- * Usable size of VRAM excluding reserved portions
>- * (e.g stolen mem)
>- */
>- resource_size_t usable_size;
>- /**
>- * @actual_physical_size: Actual VRAM size
>- *
>- * Actual VRAM size including reserved portions
>- * (e.g stolen mem)
>- */
>- resource_size_t actual_physical_size;
>- /** @mapping: pointer to VRAM mappable space */
>- void *__iomem mapping;
>- } vram;
>+ struct xe_mem_region vram;
>
> /** @vram_mgr: VRAM TTM manager */
> struct xe_ttm_vram_mgr *vram_mgr;
>@@ -264,28 +273,7 @@ struct xe_device {
> /** @mem: memory info for device */
> struct {
> /** @vram: VRAM info for device */
>- struct {
>- /** @io_start: IO start address of VRAM */
>- resource_size_t io_start;
>- /**
>- * @io_size: IO size of VRAM.
>- *
>- * This represents how much of VRAM the CPU can
>access
>- * via the VRAM BAR.
>- * On systems that do not support large BAR IO space,
>- * this can be smaller than the actual memory size, in
>- * which case only part of VRAM is CPU accessible
>- * (typically the first 256M). This configuration is
>- * known as small-bar.
>- */
>- resource_size_t io_size;
>- /** @size: Total size of VRAM */
>- resource_size_t size;
>- /** @base: Offset to apply for Device Physical
>Address control */
>- resource_size_t base;
>- /** @mapping: pointer to VRAM mappable space */
>- void *__iomem mapping;
>- } vram;
>+ struct xe_mem_region vram;
> /** @sys_mgr: system TTM manager */
> struct ttm_resource_manager sys_mgr;
> } mem;
>diff --git a/drivers/gpu/drm/xe/xe_migrate.c
>b/drivers/gpu/drm/xe/xe_migrate.c
>index 18c94022930f..956a96b38346 100644
>--- a/drivers/gpu/drm/xe/xe_migrate.c
>+++ b/drivers/gpu/drm/xe/xe_migrate.c
>@@ -264,7 +264,7 @@ static int xe_migrate_prepare_vm(struct xe_tile *tile,
>struct xe_migrate *m,
> * Use 1GB pages, it shouldn't matter the physical amount of
> * vram is less, when we don't access it.
> */
>- for (pos = 0; pos < xe->mem.vram.size; pos += SZ_1G, ofs +=
>8)
>+ for (pos = 0; pos < xe->mem.vram.actual_physical_size; pos
>+= SZ_1G, ofs += 8)
> xe_map_wr(xe, &bo->vmap, ofs, u64, pos | flags);
> }
>
>diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
>index aa9c573b1243..0d0966691eaa 100644
>--- a/drivers/gpu/drm/xe/xe_mmio.c
>+++ b/drivers/gpu/drm/xe/xe_mmio.c
>@@ -173,6 +173,7 @@ static int xe_determine_lmem_bar_size(struct
>xe_device *xe)
> if (!xe->mem.vram.io_size)
> return -EIO;
>
>+ /* XXX: Is this correct? Device's DPA should be decided by IAF */
Yeah, let's get rid of this... When the driver code that use DPA comes along, we
we can add it to the tile->mem.vram.base instead of using this value.
> xe->mem.vram.base = 0; /* DPA offset */
>
> /* set up a map to the total memory area. */
>@@ -281,7 +282,7 @@ int xe_mmio_probe_vram(struct xe_device *xe)
> return -ENODEV;
> }
>
>- tile->mem.vram.base = tile_offset;
>+ tile->mem.vram.base = xe->mem.vram.base + tile_offset;
So remove the vram.base from here.
When the DPA code comes into play, it will get added back with the appropriate value.
With those changes:
Reviewed-by: Michael J. Ruhl <michael.j.ruhl at intel.com>
M
> tile->mem.vram.usable_size = vram_size;
> tile->mem.vram.mapping = xe->mem.vram.mapping +
>tile_offset;
>
>@@ -304,10 +305,10 @@ int xe_mmio_probe_vram(struct xe_device *xe)
> io_size -= min_t(u64, tile_size, io_size);
> }
>
>- xe->mem.vram.size = total_size;
>+ xe->mem.vram.actual_physical_size = total_size;
>
> drm_info(&xe->drm, "Total VRAM: %pa, %pa\n", &xe-
>>mem.vram.io_start,
>- &xe->mem.vram.size);
>+ &xe->mem.vram.actual_physical_size);
> drm_info(&xe->drm, "Available VRAM: %pa, %pa\n", &xe-
>>mem.vram.io_start,
> &available_size);
>
>--
>2.26.3
More information about the Intel-xe
mailing list