From: Matthew Auld <matthew.auld@intel.com>
To: "K V P, Satyanarayana" <satyanarayana.k.v.p@intel.com>,
"intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
"Brost, Matthew" <matthew.brost@intel.com>
Subject: Re: [PATCH v3 5/8] drm/xe: add XE_BO_FLAG_PINNED_LATE_RESTORE
Date: Wed, 26 Mar 2025 09:03:47 +0000 [thread overview]
Message-ID: <8d12310e-1953-459e-a9d1-21db60630495@intel.com> (raw)
In-Reply-To: <LV3PR11MB86957A4ED4A4134F55A2B00CF9A62@LV3PR11MB8695.namprd11.prod.outlook.com>
On 26/03/2025 06:15, K V P, Satyanarayana wrote:
>> From: Auld, Matthew <matthew.auld@intel.com>
>> Sent: Friday, March 7, 2025 11:59 PM
>> To: intel-xe@lists.freedesktop.org
>> Cc: K V P, Satyanarayana <satyanarayana.k.v.p@intel.com>; Thomas Hellström
>> <thomas.hellstrom@linux.intel.com>; Brost, Matthew
>> <matthew.brost@intel.com>
>> Subject: [PATCH v3 5/8] drm/xe: add XE_BO_FLAG_PINNED_LATE_RESTORE
>>
>> With the idea of having more pinned objects using the blitter engine
>> where possible, during suspend/resume, mark the pinned objects which
>> can be done during the late phase once submission/migration has been
>> setup. Start out simple with lrc and page-tables from userspace.
>>
>> v2:
>> - s/early_restore/late_restore; early restore was way too bold with too
>> many places being impacted at once.
>>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_bo.c | 4 ++--
>> drivers/gpu/drm/xe/xe_bo.h | 9 +++++----
>> drivers/gpu/drm/xe/xe_bo_evict.c | 6 ++++--
>> drivers/gpu/drm/xe/xe_lrc.c | 10 +++++++---
>> drivers/gpu/drm/xe/xe_pt.c | 13 ++++++++-----
>> 5 files changed, 26 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
>> index f22b02700778..fe5a386d5dc8 100644
>> --- a/drivers/gpu/drm/xe/xe_bo.c
>> +++ b/drivers/gpu/drm/xe/xe_bo.c
>> @@ -1103,7 +1103,7 @@ int xe_bo_evict_pinned(struct xe_bo *bo)
>> goto out_unlock_bo;
>> }
>>
>> - if (xe_bo_is_user(bo)) {
>> + if (xe_bo_is_user(bo) || (bo->flags &
>> XE_BO_FLAG_PINNED_LATE_RESTORE)) {
>> struct xe_migrate *migrate;
>> struct dma_fence *fence;
>>
>> @@ -1198,7 +1198,7 @@ int xe_bo_restore_pinned(struct xe_bo *bo)
>> goto out_backup;
>> }
>>
>> - if (xe_bo_is_user(bo)) {
>> + if (xe_bo_is_user(bo) || (bo->flags &
>> XE_BO_FLAG_PINNED_LATE_RESTORE)) {
>> struct xe_migrate *migrate;
>> struct dma_fence *fence;
>>
>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
>> index 3937111c2adb..c1961eebd0bc 100644
>> --- a/drivers/gpu/drm/xe/xe_bo.h
>> +++ b/drivers/gpu/drm/xe/xe_bo.h
>> @@ -40,10 +40,11 @@
>> #define XE_BO_FLAG_NEEDS_2M BIT(16)
>> #define XE_BO_FLAG_GGTT_INVALIDATE BIT(17)
>> #define XE_BO_FLAG_PINNED_NORESTORE BIT(18)
>> -#define XE_BO_FLAG_GGTT0 BIT(19)
>> -#define XE_BO_FLAG_GGTT1 BIT(20)
>> -#define XE_BO_FLAG_GGTT2 BIT(21)
>> -#define XE_BO_FLAG_GGTT3 BIT(22)
>> +#define XE_BO_FLAG_PINNED_LATE_RESTORE BIT(19)
>> +#define XE_BO_FLAG_GGTT0 BIT(20)
>> +#define XE_BO_FLAG_GGTT1 BIT(21)
>> +#define XE_BO_FLAG_GGTT2 BIT(22)
>> +#define XE_BO_FLAG_GGTT3 BIT(23)
>> #define XE_BO_FLAG_GGTT_ALL (XE_BO_FLAG_GGTT0 | \
>> XE_BO_FLAG_GGTT1 | \
>> XE_BO_FLAG_GGTT2 | \
>
> nit: Can't we define XE_BO_FLAG_PINNED_LATE_RESTORE as BIT(23)? Do we see any issue with this?
> Other than this, LGTM.
I mostly just wanted it grouped with PINNED_NORESTORE since they are
related concepts.
> Reviewed-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
>
>> diff --git a/drivers/gpu/drm/xe/xe_bo_evict.c
>> b/drivers/gpu/drm/xe/xe_bo_evict.c
>> index 41a948de3583..0c2cfa534495 100644
>> --- a/drivers/gpu/drm/xe/xe_bo_evict.c
>> +++ b/drivers/gpu/drm/xe/xe_bo_evict.c
>> @@ -24,7 +24,8 @@ static int xe_evict_pinned(struct xe_device *xe, bool
>> memcpy_only)
>> if (!bo)
>> break;
>>
>> - if (!memcpy_only && !xe_bo_is_user(bo)) {
>> + if (!memcpy_only &&
>> + !(xe_bo_is_user(bo) || (bo->flags &
>> XE_BO_FLAG_PINNED_LATE_RESTORE))) {
>> list_move_tail(&bo->pinned_link, &skipped);
>> continue;
>> }
>> @@ -135,7 +136,8 @@ static int xe_restore_pinned(struct xe_device *xe,
>> bool memcpy_only)
>> if (!bo)
>> break;
>>
>> - if (memcpy_only && xe_bo_is_user(bo)) {
>> + if (memcpy_only &&
>> + (xe_bo_is_user(bo) || (bo->flags &
>> XE_BO_FLAG_PINNED_LATE_RESTORE))) {
>> list_move_tail(&bo->pinned_link, &skipped);
>> continue;
>> }
>> diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
>> index df3ceddede07..ffa07648690e 100644
>> --- a/drivers/gpu/drm/xe/xe_lrc.c
>> +++ b/drivers/gpu/drm/xe/xe_lrc.c
>> @@ -893,6 +893,7 @@ static int xe_lrc_init(struct xe_lrc *lrc, struct
>> xe_hw_engine *hwe,
>> void *init_data = NULL;
>> u32 arb_enable;
>> u32 lrc_size;
>> + u32 bo_flags;
>> int err;
>>
>> kref_init(&lrc->refcount);
>> @@ -901,15 +902,18 @@ static int xe_lrc_init(struct xe_lrc *lrc, struct
>> xe_hw_engine *hwe,
>> if (xe_gt_has_indirect_ring_state(gt))
>> lrc->flags |= XE_LRC_FLAG_INDIRECT_RING_STATE;
>>
>> + bo_flags = XE_BO_FLAG_VRAM_IF_DGFX(tile) | XE_BO_FLAG_GGTT |
>> + XE_BO_FLAG_GGTT_INVALIDATE;
>> + if (vm && vm->xef) /* userspace */
>> + bo_flags |= XE_BO_FLAG_PINNED_LATE_RESTORE;
>> +
>> /*
>> * FIXME: Perma-pinning LRC as we don't yet support moving GGTT
>> address
>> * via VM bind calls.
>> */
>> lrc->bo = xe_bo_create_pin_map(xe, tile, vm, lrc_size,
>> ttm_bo_type_kernel,
>> - XE_BO_FLAG_VRAM_IF_DGFX(tile) |
>> - XE_BO_FLAG_GGTT |
>> - XE_BO_FLAG_GGTT_INVALIDATE);
>> + bo_flags);
>> if (IS_ERR(lrc->bo))
>> return PTR_ERR(lrc->bo);
>>
>> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
>> index ffaf0d02dc7d..05209f0ffe5c 100644
>> --- a/drivers/gpu/drm/xe/xe_pt.c
>> +++ b/drivers/gpu/drm/xe/xe_pt.c
>> @@ -103,6 +103,7 @@ struct xe_pt *xe_pt_create(struct xe_vm *vm, struct
>> xe_tile *tile,
>> {
>> struct xe_pt *pt;
>> struct xe_bo *bo;
>> + u32 bo_flags;
>> int err;
>>
>> if (level) {
>> @@ -115,14 +116,16 @@ struct xe_pt *xe_pt_create(struct xe_vm *vm,
>> struct xe_tile *tile,
>> if (!pt)
>> return ERR_PTR(-ENOMEM);
>>
>> + bo_flags = XE_BO_FLAG_VRAM_IF_DGFX(tile) |
>> + XE_BO_FLAG_IGNORE_MIN_PAGE_SIZE |
>> XE_BO_FLAG_PINNED |
>> + XE_BO_FLAG_NO_RESV_EVICT | XE_BO_FLAG_PAGETABLE;
>> + if (vm->xef) /* userspace */
>> + bo_flags |= XE_BO_FLAG_PINNED_LATE_RESTORE;
>> +
>> pt->level = level;
>> bo = xe_bo_create_pin_map(vm->xe, tile, vm, SZ_4K,
>> ttm_bo_type_kernel,
>> - XE_BO_FLAG_VRAM_IF_DGFX(tile) |
>> - XE_BO_FLAG_IGNORE_MIN_PAGE_SIZE |
>> - XE_BO_FLAG_PINNED |
>> - XE_BO_FLAG_NO_RESV_EVICT |
>> - XE_BO_FLAG_PAGETABLE);
>> + bo_flags);
>> if (IS_ERR(bo)) {
>> err = PTR_ERR(bo);
>> goto err_kfree;
>> --
>> 2.48.1
>
next prev parent reply other threads:[~2025-03-26 9:03 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-07 18:28 [PATCH v3 0/8] Improve SRIOV VRAM provisioning Matthew Auld
2025-03-07 18:28 ` [PATCH v3 1/8] drm/xe: use backup object for pinned save/restore Matthew Auld
2025-03-07 18:28 ` [PATCH v3 2/8] drm/xe: split pinned save/restore into phases Matthew Auld
2025-03-07 18:29 ` [PATCH v3 3/8] drm/xe: Add XE_BO_FLAG_PINNED_NORESTORE Matthew Auld
2025-03-07 18:29 ` [PATCH v3 4/8] drm/xe/migrate: ignore CCS for kernel objects Matthew Auld
2025-03-26 6:09 ` K V P, Satyanarayana
2025-03-07 18:29 ` [PATCH v3 5/8] drm/xe: add XE_BO_FLAG_PINNED_LATE_RESTORE Matthew Auld
2025-03-26 6:15 ` K V P, Satyanarayana
2025-03-26 9:03 ` Matthew Auld [this message]
2025-03-07 18:29 ` [PATCH v3 6/8] drm/xe: unconditionally apply PINNED for pin_map() Matthew Auld
2025-03-07 18:29 ` [PATCH v3 7/8] drm/xe: allow non-contig VRAM kernel BO Matthew Auld
2025-03-07 18:29 ` [PATCH v3 8/8] drm/xe/sriov: support non-contig VRAM provisioning Matthew Auld
2025-03-07 20:51 ` ✓ CI.Patch_applied: success for Improve SRIOV VRAM provisioning (rev2) Patchwork
2025-03-07 20:52 ` ✓ CI.checkpatch: " Patchwork
2025-03-07 20:53 ` ✓ CI.KUnit: " Patchwork
2025-03-07 21:10 ` ✓ CI.Build: " Patchwork
2025-03-07 21:13 ` ✓ CI.Hooks: " Patchwork
2025-03-07 21:14 ` ✓ CI.checksparse: " Patchwork
2025-03-07 21:44 ` ✗ Xe.CI.BAT: failure " Patchwork
2025-03-09 1:59 ` ✗ Xe.CI.Full: " 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=8d12310e-1953-459e-a9d1-21db60630495@intel.com \
--to=matthew.auld@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=matthew.brost@intel.com \
--cc=satyanarayana.k.v.p@intel.com \
--cc=thomas.hellstrom@linux.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox