[Intel-xe] [PATCH] drm/xe: Let primary and media GT share a kernel_bb_pool
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Tue Apr 11 09:20:29 UTC 2023
Hey,
I guess youw ant this patch instead of the full_gt patch from my MTL series?
Makes sense.
On 2023-04-10 22:02, Matt Roper wrote:
> The media GT requires a valid gt->kernel_bb_pool during driver probe to
> allocate the WA and NOOP batchbuffers used to record default context
> images. Dynamically allocate the bb_pools so that the primary and media
> GT can use the same pool during driver init.
>
> The media GT still shouldn't be need the USM pool, so only hook up the
> kernel_bb_pool for now.
>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
> drivers/gpu/drm/xe/xe_bb.c | 2 +-
> drivers/gpu/drm/xe/xe_gt.c | 32 ++++++++++++++++++------------
> drivers/gpu/drm/xe/xe_gt_debugfs.c | 4 ++--
> drivers/gpu/drm/xe/xe_gt_types.h | 10 +++++++---
> drivers/gpu/drm/xe/xe_migrate.c | 4 ++--
> drivers/gpu/drm/xe/xe_sa.c | 23 ++++++++++++++-------
> drivers/gpu/drm/xe/xe_sa.h | 4 +---
> 7 files changed, 48 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_bb.c b/drivers/gpu/drm/xe/xe_bb.c
> index 7172801ee570..3deb2d55f421 100644
> --- a/drivers/gpu/drm/xe/xe_bb.c
> +++ b/drivers/gpu/drm/xe/xe_bb.c
> @@ -42,7 +42,7 @@ struct xe_bb *xe_bb_new(struct xe_gt *gt, u32 dwords, bool usm)
> * space to accomodate the platform-specific hardware prefetch
> * requirements.
> */
> - bb->bo = xe_sa_bo_new(!usm ? >->kernel_bb_pool : >->usm.bb_pool,
> + bb->bo = xe_sa_bo_new(!usm ? gt->kernel_bb_pool : gt->usm.bb_pool,
> 4 * (dwords + 1) + bb_prefetch(gt));
> if (IS_ERR(bb->bo)) {
> err = PTR_ERR(bb->bo);
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index daaf93e23bbf..4186f7f0d42f 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -137,7 +137,7 @@ static int emit_nop_job(struct xe_gt *gt, struct xe_engine *e)
> if (IS_ERR(bb))
> return PTR_ERR(bb);
>
> - batch_ofs = xe_bo_ggtt_addr(gt->kernel_bb_pool.bo);
> + batch_ofs = xe_bo_ggtt_addr(gt->kernel_bb_pool->bo);
> job = xe_bb_create_wa_job(e, bb, batch_ofs);
> if (IS_ERR(job)) {
> xe_bb_free(bb, NULL);
> @@ -186,7 +186,7 @@ static int emit_wa_job(struct xe_gt *gt, struct xe_engine *e)
> }
> }
>
> - batch_ofs = xe_bo_ggtt_addr(gt->kernel_bb_pool.bo);
> + batch_ofs = xe_bo_ggtt_addr(gt->kernel_bb_pool->bo);
> job = xe_bb_create_wa_job(e, bb, batch_ofs);
> if (IS_ERR(job)) {
> xe_bb_free(bb, NULL);
> @@ -439,26 +439,32 @@ static int all_fw_domain_init(struct xe_gt *gt)
> if (err)
> goto err_force_wake;
>
> - /*
> - * FIXME: This should be ok as SA should only be used by gt->migrate and
> - * vm->gt->migrate and both should be pointing to a non-media GT. But to
> - * realy safe, convert gt->kernel_bb_pool to a pointer and point a media
> - * GT to the kernel_bb_pool on a real tile.
> - */
> if (!xe_gt_is_media_type(gt)) {
> - err = xe_sa_bo_manager_init(gt, >->kernel_bb_pool, SZ_1M, 16);
> - if (err)
> + gt->kernel_bb_pool = xe_sa_bo_manager_init(gt, SZ_1M, 16);
Can you rename it to _create()?
Otherwise:
Reviewed-by: Maarten Lankhorst <maarten.lankhorst at intel.com>
> + if (IS_ERR(gt->kernel_bb_pool)) {
> + err = PTR_ERR(gt->kernel_bb_pool);
> goto err_force_wake;
> + }
>
> /*
> * USM has its only SA pool to non-block behind user operations
> */
> if (gt_to_xe(gt)->info.supports_usm) {
> - err = xe_sa_bo_manager_init(gt, >->usm.bb_pool,
> - SZ_1M, 16);
> - if (err)
> + gt->usm.bb_pool = xe_sa_bo_manager_init(gt, SZ_1M, 16);
> + if (IS_ERR(gt->usm.bb_pool)) {
> + err = PTR_ERR(gt->usm.bb_pool);
> goto err_force_wake;
> + }
> }
> + } else {
> + struct xe_gt *full_gt = xe_find_full_gt(gt);
> +
> + /*
> + * Media GT's kernel_bb_pool is only used while recording the
> + * default context during GT init. The USM pool should never
> + * be needed on the media GT.
> + */
> + gt->kernel_bb_pool = full_gt->kernel_bb_pool;
> }
>
> if (!xe_gt_is_media_type(gt)) {
> diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> index 9fab8017490f..c45486c2015a 100644
> --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> @@ -66,8 +66,8 @@ static int sa_info(struct seq_file *m, void *data)
> struct xe_gt *gt = node_to_gt(m->private);
> struct drm_printer p = drm_seq_file_printer(m);
>
> - drm_suballoc_dump_debug_info(>->kernel_bb_pool.base, &p,
> - gt->kernel_bb_pool.gpu_addr);
> + drm_suballoc_dump_debug_info(>->kernel_bb_pool->base, &p,
> + gt->kernel_bb_pool->gpu_addr);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
> index 9d3117fad2e4..7c47d67aa8be 100644
> --- a/drivers/gpu/drm/xe/xe_gt_types.h
> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> @@ -214,7 +214,7 @@ struct xe_gt {
> * behind any user operations which may have resulted in a
> * fault.
> */
> - struct xe_sa_manager bb_pool;
> + struct xe_sa_manager *bb_pool;
> /**
> * @reserved_bcs_instance: reserved BCS instance used for USM
> * operations (e.g. mmigrations, fixing page tables)
> @@ -304,8 +304,12 @@ struct xe_gt {
> /** @hw_engines: hardware engines on the GT */
> struct xe_hw_engine hw_engines[XE_NUM_HW_ENGINES];
>
> - /** @kernel_bb_pool: Pool from which batchbuffers are allocated */
> - struct xe_sa_manager kernel_bb_pool;
> + /**
> + * @kernel_bb_pool: Pool from which batchbuffers are allocated.
> + *
> + * Media GT shares a pool with its primary GT.
> + */
> + struct xe_sa_manager *kernel_bb_pool;
>
> /** @migrate: Migration helper for vram blits and clearing */
> struct xe_migrate *migrate;
> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index e8978440c725..8686a2a7b035 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
> @@ -161,7 +161,7 @@ static int xe_migrate_prepare_vm(struct xe_gt *gt, struct xe_migrate *m,
> u32 num_entries = NUM_PT_SLOTS, num_level = vm->pt_root[id]->level;
> u32 map_ofs, level, i;
> struct xe_device *xe = gt_to_xe(m->gt);
> - struct xe_bo *bo, *batch = gt->kernel_bb_pool.bo;
> + struct xe_bo *bo, *batch = gt->kernel_bb_pool->bo;
> u64 entry;
> int ret;
>
> @@ -229,7 +229,7 @@ static int xe_migrate_prepare_vm(struct xe_gt *gt, struct xe_migrate *m,
> m->batch_base_ofs = xe_migrate_vram_ofs(batch_addr);
>
> if (xe->info.supports_usm) {
> - batch = gt->usm.bb_pool.bo;
> + batch = gt->usm.bb_pool->bo;
> batch_addr = xe_bo_addr(batch, 0, GEN8_PAGE_SIZE,
> &is_vram);
> m->usm_batch_base_ofs = xe_migrate_vram_ofs(batch_addr);
> diff --git a/drivers/gpu/drm/xe/xe_sa.c b/drivers/gpu/drm/xe/xe_sa.c
> index 96c4b0ef24fe..c16f7c14ff52 100644
> --- a/drivers/gpu/drm/xe/xe_sa.c
> +++ b/drivers/gpu/drm/xe/xe_sa.c
> @@ -33,13 +33,18 @@ static void xe_sa_bo_manager_fini(struct drm_device *drm, void *arg)
> sa_manager->bo = NULL;
> }
>
> -int xe_sa_bo_manager_init(struct xe_gt *gt,
> - struct xe_sa_manager *sa_manager,
> - u32 size, u32 align)
> +struct xe_sa_manager *xe_sa_bo_manager_init(struct xe_gt *gt, u32 size, u32 align)
> {
> struct xe_device *xe = gt_to_xe(gt);
> u32 managed_size = size - SZ_4K;
> struct xe_bo *bo;
> + int ret;
> +
> + struct xe_sa_manager *sa_manager = drmm_kzalloc(>_to_xe(gt)->drm,
> + sizeof(*sa_manager),
> + GFP_KERNEL);
> + if (!sa_manager)
> + return ERR_PTR(-ENOMEM);
>
> sa_manager->bo = NULL;
>
> @@ -49,7 +54,7 @@ int xe_sa_bo_manager_init(struct xe_gt *gt,
> if (IS_ERR(bo)) {
> drm_err(&xe->drm, "failed to allocate bo for sa manager: %ld\n",
> PTR_ERR(bo));
> - return PTR_ERR(bo);
> + return (struct xe_sa_manager *)bo;
> }
> sa_manager->bo = bo;
>
> @@ -61,15 +66,19 @@ int xe_sa_bo_manager_init(struct xe_gt *gt,
> if (!sa_manager->cpu_ptr) {
> xe_bo_unpin_map_no_vm(sa_manager->bo);
> sa_manager->bo = NULL;
> - return -ENOMEM;
> + return ERR_PTR(-ENOMEM);
> }
> } else {
> sa_manager->cpu_ptr = bo->vmap.vaddr;
> memset(sa_manager->cpu_ptr, 0, bo->ttm.base.size);
> }
>
> - return drmm_add_action_or_reset(&xe->drm, xe_sa_bo_manager_fini,
> - sa_manager);
> + ret = drmm_add_action_or_reset(&xe->drm, xe_sa_bo_manager_fini,
> + sa_manager);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return sa_manager;
> }
>
> struct drm_suballoc *xe_sa_bo_new(struct xe_sa_manager *sa_manager,
> diff --git a/drivers/gpu/drm/xe/xe_sa.h b/drivers/gpu/drm/xe/xe_sa.h
> index 742282ef7179..3063fb34c720 100644
> --- a/drivers/gpu/drm/xe/xe_sa.h
> +++ b/drivers/gpu/drm/xe/xe_sa.h
> @@ -11,9 +11,7 @@ struct dma_fence;
> struct xe_bo;
> struct xe_gt;
>
> -int xe_sa_bo_manager_init(struct xe_gt *gt,
> - struct xe_sa_manager *sa_manager,
> - u32 size, u32 align);
> +struct xe_sa_manager *xe_sa_bo_manager_init(struct xe_gt *gt, u32 size, u32 align);
>
> struct drm_suballoc *xe_sa_bo_new(struct xe_sa_manager *sa_manager,
> u32 size);
More information about the Intel-xe
mailing list