From 535881a8c50b79085327e7dbe26a4c55f3e1591b Mon Sep 17 00:00:00 2001 From: Rodrigo Vivi Date: Fri, 15 Dec 2023 15:45:48 +0000 Subject: [PATCH] drm/xe/uapi: Document the memory_region bitmask MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The uAPI should stay generic in regarding to the bitmask. It is the userspace responsibility to check for the type/class of the memory, without any assumption. Also add comments inside the code to explain how it is actually constructed so we don't accidentally change the assignment of the instance and the masks. No functional change in this patch. It only explains and document the memory_region masks. A further follow-up work with the organization of all memory regions around struct xe_mem_regions is desired, but not part of this patch. Signed-off-by: Rodrigo Vivi Reviewed-by: Lucas De Marchi Acked-by: José Roberto de Souza Acked-by: Mateusz Naklicki Signed-off-by: Francois Dugast --- drivers/gpu/drm/xe/xe_query.c | 19 +++++++++++++++++++ include/uapi/drm/xe_drm.h | 23 ++++++++++++++++++----- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c index 56d61bf596b2b..9b35673b286c8 100644 --- a/drivers/gpu/drm/xe/xe_query.c +++ b/drivers/gpu/drm/xe/xe_query.c @@ -266,6 +266,11 @@ static int query_mem_regions(struct xe_device *xe, man = ttm_manager_type(&xe->ttm, XE_PL_TT); mem_regions->mem_regions[0].mem_class = DRM_XE_MEM_REGION_CLASS_SYSMEM; + /* + * The instance needs to be a unique number that represents the index + * in the placement mask used at xe_gem_create_ioctl() for the + * xe_bo_create() placement. + */ mem_regions->mem_regions[0].instance = 0; mem_regions->mem_regions[0].min_page_size = PAGE_SIZE; mem_regions->mem_regions[0].total_size = man->size << PAGE_SHIFT; @@ -381,6 +386,20 @@ static int query_gt_list(struct xe_device *xe, struct drm_xe_device_query *query gt_list->gt_list[id].tile_id = gt_to_tile(gt)->id; gt_list->gt_list[id].gt_id = gt->info.id; gt_list->gt_list[id].reference_clock = gt->info.reference_clock; + /* + * The mem_regions indexes in the mask below need to + * directly identify the struct + * drm_xe_query_mem_regions' instance constructed at + * query_mem_regions() + * + * For our current platforms: + * Bit 0 -> System Memory + * Bit 1 -> VRAM0 on Tile0 + * Bit 2 -> VRAM1 on Tile1 + * However the uAPI is generic and it's userspace's + * responsibility to check the mem_class, without any + * assumption. + */ if (!IS_DGFX(xe)) gt_list->gt_list[id].near_mem_regions = 0x1; else diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h index 5a01d033b7803..6c719ba8fc8e1 100644 --- a/include/uapi/drm/xe_drm.h +++ b/include/uapi/drm/xe_drm.h @@ -256,10 +256,9 @@ struct drm_xe_mem_region { */ __u16 mem_class; /** - * @instance: The instance for this region. - * - * The @mem_class and @instance taken together will always give - * a unique pair. + * @instance: The unique ID for this region, which serves as the + * index in the placement bitmask used as argument for + * &DRM_IOCTL_XE_GEM_CREATE */ __u16 instance; /** @@ -404,6 +403,10 @@ struct drm_xe_gt { * @near_mem_regions: Bit mask of instances from * drm_xe_query_mem_regions that are nearest to the current engines * of this GT. + * Each index in this mask refers directly to the struct + * drm_xe_query_mem_regions' instance, no assumptions should + * be made about order. The type of each region is described + * by struct drm_xe_query_mem_regions' mem_class. */ __u64 near_mem_regions; /** @@ -412,6 +415,10 @@ struct drm_xe_gt { * In general, they have extra indirections when compared to the * @near_mem_regions. For a discrete device this could mean system * memory and memory living in a different tile. + * Each index in this mask refers directly to the struct + * drm_xe_query_mem_regions' instance, no assumptions should + * be made about order. The type of each region is described + * by struct drm_xe_query_mem_regions' mem_class. */ __u64 far_mem_regions; /** @reserved: Reserved */ @@ -652,7 +659,13 @@ struct drm_xe_gem_create { */ __u64 size; - /** @placement: A mask of memory instances of where BO can be placed. */ + /** + * @placement: A mask of memory instances of where BO can be placed. + * Each index in this mask refers directly to the struct + * drm_xe_query_mem_regions' instance, no assumptions should + * be made about order. The type of each region is described + * by struct drm_xe_query_mem_regions' mem_class. + */ __u32 placement; #define DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING (1 << 0) -- 2.30.2