Re: [Intel-xe] [RFC v2 5/6] drm/xe/xe_migrate.c: Use NULL 1G PTE mapped at 255GiB VA for ccs clear.

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Tue, 2023-11-21 at 15:39 +0530, Himal Prasad Ghimiray wrote:
> Get rid of the cleared bo, instead use null 1G PTE mapped at 255GiB
> offset, this can be used for both dgfx and igfx.
> 
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Signed-off-by: Himal Prasad Ghimiray
> <himal.prasad.ghimiray@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_migrate.c | 56 ++++++-------------------------
> --
>  1 file changed, 10 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_migrate.c
> b/drivers/gpu/drm/xe/xe_migrate.c
> index 06706fad67aa..bdcb20f23531 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
> @@ -54,8 +54,8 @@ struct xe_migrate {
>         u64 batch_base_ofs;
>         /** @usm_batch_base_ofs: VM offset of the usm batch buffer */
>         u64 usm_batch_base_ofs;
> -       /** @cleared_vram_ofs: VM offset of @cleared_bo. */
> -       u64 cleared_vram_ofs;
> +       /** @cleared_mem_ofs: VM offset of @cleared_bo. */
> +       u64 cleared_mem_ofs;

Can we also remove xe_migrate::cleared_bo?

>         /**
>          * @fence: dma-fence representing the last migration job
> batch.
>          * Protected by @job_mutex.
> @@ -125,41 +125,6 @@ static u64 xe_migrate_vram_ofs(struct xe_device
> *xe, u64 addr)
>         return addr + (256ULL << xe_pt_shift(2));
>  }
>  
> -/*
> - * For flat CCS clearing we need a cleared chunk of memory to copy
> from,
> - * since the CCS clearing mode of XY_FAST_COLOR_BLT appears to be
> buggy
> - * (it clears on only 14 bytes in each chunk of 16).
> - * If clearing the main surface one can use the part of the main
> surface
> - * already cleared, but for clearing as part of copying non-
> compressed
> - * data out of system memory, we don't readily have a cleared part
> of
> - * VRAM to copy from, so create one to use for that case.
> - */
> -static int xe_migrate_create_cleared_bo(struct xe_migrate *m, struct
> xe_vm *vm)
> -{
> -       struct xe_tile *tile = m->tile;
> -       struct xe_device *xe = vm->xe;
> -       size_t cleared_size;
> -       u64 vram_addr;
> -
> -       if (!xe_device_has_flat_ccs(xe))
> -               return 0;
> -
> -       cleared_size = xe_device_ccs_bytes(xe,
> MAX_PREEMPTDISABLE_TRANSFER);
> -       cleared_size = PAGE_ALIGN(cleared_size);
> -       m->cleared_bo = xe_bo_create_pin_map(xe, tile, vm,
> cleared_size,
> -                                            ttm_bo_type_kernel,
> -                                           
> XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> -                                           
> XE_BO_CREATE_PINNED_BIT);
> -       if (IS_ERR(m->cleared_bo))
> -               return PTR_ERR(m->cleared_bo);
> -
> -       xe_map_memset(xe, &m->cleared_bo->vmap, 0, 0x00,
> cleared_size);
> -       vram_addr = xe_bo_addr(m->cleared_bo, 0, XE_PAGE_SIZE);
> -       m->cleared_vram_ofs = xe_migrate_vram_ofs(xe, vram_addr);
> -
> -       return 0;
> -}
> -
>  static int xe_migrate_prepare_vm(struct xe_tile *tile, struct
> xe_migrate *m,
>                                  struct xe_vm *vm)
>  {
> @@ -170,7 +135,6 @@ static int xe_migrate_prepare_vm(struct xe_tile
> *tile, struct xe_migrate *m,
>         u32 map_ofs, level, i;
>         struct xe_bo *bo, *batch = tile->mem.kernel_bb_pool->bo;
>         u64 entry;
> -       int ret;
>  
>         /* Can't bump NUM_PT_SLOTS too high */
>         BUILD_BUG_ON(NUM_PT_SLOTS > SZ_2M/XE_PAGE_SIZE);
> @@ -190,12 +154,6 @@ static int xe_migrate_prepare_vm(struct xe_tile
> *tile, struct xe_migrate *m,
>         if (IS_ERR(bo))
>                 return PTR_ERR(bo);
>  
> -       ret = xe_migrate_create_cleared_bo(m, vm);
> -       if (ret) {
> -               xe_bo_put(bo);
> -               return ret;
> -       }
> -
>         entry = vm->pt_ops->pde_encode_bo(bo, bo->size -
> XE_PAGE_SIZE, pat_index);
>         xe_pt_write(xe, &vm->pt_root[id]->bo->vmap, 0, entry);
>  
> @@ -262,6 +220,12 @@ static int xe_migrate_prepare_vm(struct xe_tile
> *tile, struct xe_migrate *m,
>                           (i + 1) * 8, u64, entry);
>         }

A short comment that we set up a 1GiB NULL mapping at 255GiB offset.

>  
> +       level = 2;
> +       xe_map_wr(xe, &bo->vmap, map_ofs + XE_PAGE_SIZE * level + 255
> * 8, u64,
> +                 vm->pt_ops->pte_encode_addr(xe, 0, pat_index,
> level, IS_DGFX(xe), 0)
> +                 | XE_PTE_NULL);
> +       m->cleared_mem_ofs = (255ULL << xe_pt_shift(level));
> +



>         /* Identity map the entire vram at 256GiB offset */
>         if (IS_DGFX(xe)) {
>                 u64 pos, ofs, flags;
> @@ -616,7 +580,7 @@ static u32 xe_migrate_ccs_copy(struct xe_migrate
> *m,
>                  * Otherwise if the bo doesn't have any CCS metadata
> attached,
>                  * we still need to clear it for security reasons.
>                  */
> -               u64 ccs_src_ofs =  src_is_vram ? src_ofs : m-
> >cleared_vram_ofs;
> +               u64 ccs_src_ofs =  src_is_vram ? src_ofs : m-
> >cleared_mem_ofs;
>  
>                 emit_copy_ccs(gt, bb,
>                               dst_ofs, true,
> @@ -1005,7 +969,7 @@ struct dma_fence *xe_migrate_clear(struct
> xe_migrate *m,
>  
>                 if (xe_device_has_flat_ccs(xe) && clear_vram) {
>                         emit_copy_ccs(gt, bb, clear_L0_ofs, true,
> -                                     m->cleared_vram_ofs, false,
> clear_L0);
> +                                     m->cleared_mem_ofs, false,
> clear_L0);
>                         flush_flags = MI_FLUSH_DW_CCS;
>                 }
>