From: Michal Wajdeczko <michal.wajdeczko@intel.com>
To: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>,
intel-xe@lists.freedesktop.org
Cc: "Michał Winiarski" <michal.winiarski@intel.com>,
"Tomasz Lis" <tomasz.lis@intel.com>,
"Matthew Brost" <matthew.brost@intel.com>,
"Matthew Auld" <matthew.auld@intel.com>
Subject: Re: [ONLY FOR INTERNAL REVIEW 1/3] drm/xe/vf: Create contexts for CCS read write
Date: Fri, 16 May 2025 18:45:13 +0200 [thread overview]
Message-ID: <91cbdebe-4ee8-4fdd-8291-a522c2fac927@intel.com> (raw)
In-Reply-To: <20250516114832.19046-2-satyanarayana.k.v.p@intel.com>
On 16.05.2025 13:48, Satyanarayana K V P wrote:
> Create two LRCs to handle CCS meta data read / write from CCS pool in the
> VM. Read context is used to hold GPU instructions to be executed at save
> time and write context is used to hold GPU instructions to be executed at
> the restore time.
>
> Allocate batch buffer pool using suballocator for both read and write
> contexts.
>
> Migration framework is reused to create LRCAs for read and write.
>
> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> ---
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Tomasz Lis <tomasz.lis@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> ---
> drivers/gpu/drm/xe/Makefile | 3 +-
> drivers/gpu/drm/xe/xe_device.c | 2 +
> drivers/gpu/drm/xe/xe_device_types.h | 15 +++
> drivers/gpu/drm/xe/xe_gt_debugfs.c | 17 ++-
> drivers/gpu/drm/xe/xe_sriov.c | 17 +++
> drivers/gpu/drm/xe/xe_sriov.h | 1 +
> drivers/gpu/drm/xe/xe_sriov_vf_ccs.c | 156 +++++++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_sriov_vf_ccs.h | 13 +++
> 8 files changed, 221 insertions(+), 3 deletions(-)
> create mode 100644 drivers/gpu/drm/xe/xe_sriov_vf_ccs.c
> create mode 100644 drivers/gpu/drm/xe/xe_sriov_vf_ccs.h
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index e4bf484d4121..ad2fb025463a 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -139,7 +139,8 @@ xe-y += \
> xe_guc_relay.o \
> xe_memirq.o \
> xe_sriov.o \
> - xe_sriov_vf.o
> + xe_sriov_vf.o \
> + xe_sriov_vf_ccs.o
>
> xe-$(CONFIG_PCI_IOV) += \
> xe_gt_sriov_pf.o \
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index d4b6e623aa48..c8cb25dbd0ac 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -929,6 +929,8 @@ int xe_device_probe(struct xe_device *xe)
>
> xe_vsec_init(xe);
>
> + err = xe_sriov_late_init(xe);
missing error handling
> +
> return devm_add_action_or_reset(xe->drm.dev, xe_device_sanitize, xe);
>
> err_unregister_display:
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 50b2bfa682ac..4f0b74bd0824 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -234,9 +234,24 @@ struct xe_tile {
> /** @sriov.pf.lmtt: Local Memory Translation Table. */
> struct xe_lmtt lmtt;
> } pf;
> +#define XE_CCS_READ_CTX 0
> +#define XE_CCS_WRITE_CTX 1
> +#define XE_CCS_RW_MAX_CTXS (XE_CCS_WRITE_CTX + 1)
> struct {
> /** @sriov.vf.ggtt_balloon: GGTT regions excluded from use. */
> struct xe_ggtt_node *ggtt_balloon[2];
> +
> + struct xe_ccs_rw_ctx {
> + /** @migrate: Migration helper for save/restore of CCS data */
> + struct xe_migrate *migrate;
> +
> + struct {
> + /** @ccs_rw_bb_pool: Pool from which batchbuffers
> + * are allocated.
> + */
> + struct xe_sa_manager *ccs_rw_bb_pool;
> + } mem;
> + } ccs_rw_ctx[XE_CCS_RW_MAX_CTXS];
maybe it will look better if we move most of this to the
xe_sriov_vf_ccs_types.h and leave just here just .ccs member:
struct xe_tile_vf_ccs ccs;
> } vf;
> } sriov;
>
> diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> index 119a55bb7580..1ff54bdcfc3d 100644
> --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> @@ -143,10 +143,23 @@ static int force_reset_sync(struct xe_gt *gt, struct drm_printer *p)
> static int sa_info(struct xe_gt *gt, struct drm_printer *p)
> {
> struct xe_tile *tile = gt_to_tile(gt);
> + struct xe_device *xe = gt_to_xe(gt);
> + struct xe_sa_manager *bb_pool;
>
> xe_pm_runtime_get(gt_to_xe(gt));
> - drm_suballoc_dump_debug_info(&tile->mem.kernel_bb_pool->base, p,
> - tile->mem.kernel_bb_pool->gpu_addr);
> +
> + drm_printf(p, "kernel_bb_pool info\n");
> + drm_printf(p, "-------------------------\n");
> + bb_pool = tile->mem.kernel_bb_pool;
> + drm_suballoc_dump_debug_info(&bb_pool->base, p, bb_pool->gpu_addr);
> +
> + if (IS_SRIOV_VF(xe)) {
maybe just add separate debugfs file instead of altering existing entry?
> + drm_printf(p, "\nccs_rw_bb_pool info\n");
please don't start log with \n
if separation line is required then use drm_puts(p, "\n");
> + drm_printf(p, "-------------------------\n");
> + bb_pool = tile->sriov.vf.ccs_rw_ctx[0].mem.ccs_rw_bb_pool;
> + drm_suballoc_dump_debug_info(&bb_pool->base, p, bb_pool->gpu_addr);
> + }
> +
> xe_pm_runtime_put(gt_to_xe(gt));
>
> return 0;
> diff --git a/drivers/gpu/drm/xe/xe_sriov.c b/drivers/gpu/drm/xe/xe_sriov.c
> index a0eab44c0e76..fa6286e123f0 100644
> --- a/drivers/gpu/drm/xe/xe_sriov.c
> +++ b/drivers/gpu/drm/xe/xe_sriov.c
> @@ -15,6 +15,7 @@
> #include "xe_sriov.h"
> #include "xe_sriov_pf.h"
> #include "xe_sriov_vf.h"
> +#include "xe_sriov_vf_ccs.h"
>
> /**
> * xe_sriov_mode_to_string - Convert enum value to string.
> @@ -157,3 +158,19 @@ const char *xe_sriov_function_name(unsigned int n, char *buf, size_t size)
> strscpy(buf, "PF", size);
> return buf;
> }
> +
> +/**
> + * xe_sriov_late_init() - SR-IOV late initialization functions.
> + * @xe: the &xe_device to initialize
> + *
* On VF this function will initialize code for CCS migration ...
or something similar
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int xe_sriov_late_init(struct xe_device *xe)
> +{
> + int err = 0;
> +
> + if (IS_SRIOV_VF(xe))
> + err = xe_sriov_vf_ccs_rw_init(xe);
> +
> + return err;
> +}
> diff --git a/drivers/gpu/drm/xe/xe_sriov.h b/drivers/gpu/drm/xe/xe_sriov.h
> index 688fbabf08f1..0e0c1abf2d14 100644
> --- a/drivers/gpu/drm/xe/xe_sriov.h
> +++ b/drivers/gpu/drm/xe/xe_sriov.h
> @@ -18,6 +18,7 @@ const char *xe_sriov_function_name(unsigned int n, char *buf, size_t len);
> void xe_sriov_probe_early(struct xe_device *xe);
> void xe_sriov_print_info(struct xe_device *xe, struct drm_printer *p);
> int xe_sriov_init(struct xe_device *xe);
> +int xe_sriov_late_init(struct xe_device *xe);
>
> static inline enum xe_sriov_mode xe_device_sriov_mode(const struct xe_device *xe)
> {
> diff --git a/drivers/gpu/drm/xe/xe_sriov_vf_ccs.c b/drivers/gpu/drm/xe/xe_sriov_vf_ccs.c
> new file mode 100644
> index 000000000000..a8a21336dc12
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_sriov_vf_ccs.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#include "instructions/xe_mi_commands.h"
> +#include "xe_bo.h"
> +#include "xe_device.h"
> +#include "xe_migrate.h"
> +#include "xe_sa.h"
> +#include "xe_sriov_printk.h"
> +#include "xe_sriov_vf_ccs.h"
> +
> +/**
> + * DOC: VF save/restore of compression Meta Data.
remove trailing dot
https://docs.kernel.org/doc-guide/kernel-doc.html#overview-documentation-comments
> + *
> + * VF KMD register two special context/LRCA.
> + *
> + * Save Context/LRCA: contain necessary cmds+page table to trigger Meta data /
> + * compression control surface (Aka CCS) save in regular System memory in VM.
> + *
> + * Restore Context/LRCA: contain necessary cmds+page table to trigger Meta data /
> + * compression control surface (Aka CCS) Restore from regular System memory in
> + * VM to corresponding CCS pool.
> + *
> + * Below diagram explain steps needed for VF save/Restore of compression Meta
> + * Data::
> + *
please double check the output, :: requires extra indent to follow
https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#literal-blocks
> + * CCS Save CCS Restore VF KMD Guc BCS
> + * LRCA LRCA
> + * | | | | |
> + * | | | | |
> + * | Create Save LRCA | | |
> + * [ ]<----------------------------- [ ] | |
> + * | | | | |
> + * | | | | |
> + * | | | Register LRCA with Guc | |
> + * | | [ ]--------------------------->[ ] |
> + * | | | | |
> + * | | | | |
> + * | | Create restore LRCA | | |
> + * | [ ]<------------------[ ] | |
> + * | | | | |
> + * | | | | |
> + * | | [ ]----------------------- | |
> + * | | [ ] Allocate main memory | | |
> + * | | [ ] Allocate CCS memory | | |
> + * | | [ ]<---------------------- | |
> + * | | | | |
> + * | | | | |
> + * | Update Main memory&CCS pages | | |
> + * | PPGTT + BB cmds to save | | |
> + * [ ]<------------------------------[ ] | |
> + * | | | | |
> + * | | | | |
> + * | | Update Main memory | | |
> + * | | & CCS pages PPGTT + | | |
> + * | | BB cms to restore | | |
> + * | [ ]<------------------[ ] | |
> + * | | | | |
> + * | | | | |
> + * | | VF Pause | |
> + * | | | |Schedule |
> + * | | | |CCS Save |
> + * | | | | LRCA |
> + * | | | [ ]------>[ ]
> + * | | | | |
> + * | | | | |
> + * | | VF Restore | |
> + * | | | | |
> + * | | | | |
> + * | | [ ]-------------- | |
> + * | | [ ] Fix up GGTT | | |
> + * | | [ ]<------------- | |
> + * | | | | |
> + * | | | | |
> + * | | | |Schedule |
> + * | | | |CCS |
> + * | | | |Restore |
> + * | | | |LRCA |
> + * | | | [ ]------>[ ]
> + * | | | | |
> + * | | | | |
> + *
> + */
> +
> +#define for_each_ccs_rw_ctx(id__) \
> + for ((id__) = 0; (id__) < XE_CCS_RW_MAX_CTXS; (id__)++)
> +
> +#define MAX_CCS_RW_BB_POOL_SIZE (SZ_4K * XE_PAGE_SIZE)
> +
> +static int alloc_bb_pool(struct xe_tile *tile, struct xe_ccs_rw_ctx *ctx)
> +{
> + struct xe_device *xe = tile_to_xe(tile);
> + struct xe_sa_manager *sa_manager;
> + int offset, err;
> +
> + sa_manager = xe_sa_bo_manager_init(tile, MAX_CCS_RW_BB_POOL_SIZE, SZ_16);
> +
> + if (IS_ERR(sa_manager)) {
> + xe_sriov_err(xe, "Suballocator init failed\n");
if xe_sa_bo_manager_init didn't already print an error, use %pe here
> + err = PTR_ERR(sa_manager);
> + return err;
> + }
> +
> + offset = 0;
> + xe_map_memset(xe, &sa_manager->bo->vmap, offset, MI_NOOP,
> + MAX_CCS_RW_BB_POOL_SIZE);
> +
> + offset = MAX_CCS_RW_BB_POOL_SIZE - sizeof(u32);
> + xe_map_wr(xe, &sa_manager->bo->vmap, offset, u32, MI_BATCH_BUFFER_END);
> +
> + ctx->mem.ccs_rw_bb_pool = sa_manager;
> +
> + return 0;
> +}
> +
> +/**
> + * xe_sriov_vf_ccs_save_restore_init - Setup LRCA for save & restore.
function name mismatch
> + * @xe: the &xe_device to start recovery on
> + *
> + * This function shall be called only by VF.
> + *
> + * Return: 0 on success. Negative error code on failure.
> + */
> +int xe_sriov_vf_ccs_rw_init(struct xe_device *xe)
> +{
> + struct xe_migrate *migrate;
> + struct xe_ccs_rw_ctx *ctx;
> + struct xe_tile *tile;
> + int tile_id, ctx_id;
> + int err = 0;
> +
> + if (!IS_SRIOV_VF(xe) || IS_DGFX(xe))
we shouldn't call this on VFs so assert shall be sufficient
and you check for IS_VF in xe_sriov_late_init()
> + return 0;
> +
> + for_each_tile(tile, xe, tile_id) {
> + for_each_ccs_rw_ctx(ctx_id) {
> + ctx = &tile->sriov.vf.ccs_rw_ctx[ctx_id];
> + migrate = xe_migrate_init(tile);
> + if (IS_ERR(migrate)) {
> + err = PTR_ERR(migrate);
> + goto err_ret;
> + }
> + ctx->migrate = migrate;
> +
> + err = alloc_bb_pool(tile, ctx);
> + if (err)
> + goto err_ret;
> + }
> + }
> + return 0;
> +
> +err_ret:
> + return err;
> +}
> diff --git a/drivers/gpu/drm/xe/xe_sriov_vf_ccs.h b/drivers/gpu/drm/xe/xe_sriov_vf_ccs.h
> new file mode 100644
> index 000000000000..c371aabb4d21
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_sriov_vf_ccs.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#ifndef _XE_SRIOV_VF_CCS_H_
> +#define _XE_SRIOV_VF_CCS_H_
> +
> +struct xe_device;
> +
> +int xe_sriov_vf_ccs_rw_init(struct xe_device *xe);
do you plan more init() functions?
can't it just be xe_sriov_vf_ccs_init() ?
> +
> +#endif
next prev parent reply other threads:[~2025-05-16 16:45 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-16 11:48 [ONLY FOR INTERNAL REVIEW 0/3] CCS save restore for IGPU Satyanarayana K V P
2025-05-16 11:38 ` ✓ CI.Patch_applied: success for " Patchwork
2025-05-16 11:38 ` ✗ CI.checkpatch: warning " Patchwork
2025-05-16 11:39 ` ✓ CI.KUnit: success " Patchwork
2025-05-16 11:48 ` [ONLY FOR INTERNAL REVIEW 1/3] drm/xe/vf: Create contexts for CCS read write Satyanarayana K V P
2025-05-16 16:45 ` Michal Wajdeczko [this message]
2025-05-16 11:48 ` [ONLY FOR INTERNAL REVIEW 2/3] drm/xe/vf: Attach and detach CCS copy commands with BO Satyanarayana K V P
2025-05-16 13:48 ` Matthew Auld
2025-05-16 11:48 ` [ONLY FOR INTERNAL REVIEW 3/3] drm/xe/vf: Register CCS read/write contexts with Guc Satyanarayana K V P
2025-05-16 11:50 ` ✓ CI.Build: success for CCS save restore for IGPU Patchwork
2025-05-16 11:52 ` ✗ CI.Hooks: failure " Patchwork
2025-05-16 11:54 ` ✓ CI.checksparse: success " Patchwork
2025-05-16 12:17 ` ✓ Xe.CI.BAT: " Patchwork
2025-05-17 4:31 ` ✗ Xe.CI.Full: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=91cbdebe-4ee8-4fdd-8291-a522c2fac927@intel.com \
--to=michal.wajdeczko@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=matthew.auld@intel.com \
--cc=matthew.brost@intel.com \
--cc=michal.winiarski@intel.com \
--cc=satyanarayana.k.v.p@intel.com \
--cc=tomasz.lis@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.