From: Nirmoy Das <nirmoy.das@intel.com>
To: "Welty, Brian" <brian.welty@intel.com>, <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH v4 5/5] drm/xe: Refactor default device atomic settings
Date: Tue, 30 Apr 2024 18:23:58 +0200 [thread overview]
Message-ID: <d50f96bd-be0e-4a54-97e0-4d179c7a44b8@intel.com> (raw)
In-Reply-To: <41eb1fab-d9a5-45be-9c95-05a34fc7685e@intel.com>
Hi Brian,
On 4/29/2024 10:14 PM, Welty, Brian wrote:
>
>
> On 4/25/2024 3:23 PM, Nirmoy Das wrote:
>> The default behavior of device atomics depends on the
>> VM type and buffer allocation types. Device atomics are
>> expected to function with all types of allocations for
>> traditional applications/APIs. Additionally, in compute/SVM
>> API scenarios with fault mode or LR mode VMs, device atomics
>> must work with single-region allocations.
>
> I think additional patch may be needed..... for the Compute mode
> handling.
> I'm not sure correct thing will happened for 'shared'
> (multi-placement) allocations.
> HW will raise an atomic access page fault.
> Our page fault handler only checks this was atomic fault and proceeds to
> migrate the BO to VRAM. I think PTE_AE bit won't be set with your
> changes, so seems are in infinite loop of atomic access faults on same
> address.
>
> I think we need to fail the page fault in this case, so HW raise CAT
> error here? As atomic access not supposed to be allowed for above case
> until future uAPI is added?
>
> And need IGT test for this.
Thinking more into that, I just realized we need preferred location
madvise hint. Currently I am not aware of
a good way to change backend of SMEM+LMEM bo. We don't migrate the BO to
smem on CPU access, that would've helped too
but we need those to be implemented which is my next priority after
getting the defaults fixed with this series.
Regards,
Nirmoy
>
> -Brian
>
>
>> In all other cases
>> device atomics should be disabled by default.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_pt.c | 24 ++++++++++++++++++++----
>> drivers/gpu/drm/xe/xe_vm.c | 3 ++-
>> 2 files changed, 22 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
>> index 5b7930f46cf3..a8e9e8592c43 100644
>> --- a/drivers/gpu/drm/xe/xe_pt.c
>> +++ b/drivers/gpu/drm/xe/xe_pt.c
>> @@ -597,7 +597,6 @@ static int
>> xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
>> struct xe_vm_pgtable_update *entries, u32 *num_entries)
>> {
>> - struct xe_device *xe = tile_to_xe(tile);
>> struct xe_bo *bo = xe_vma_bo(vma);
>> bool is_devmem = !xe_vma_is_userptr(vma) && bo &&
>> (xe_bo_is_vram(bo) || xe_bo_is_stolen_devmem(bo));
>> @@ -619,9 +618,26 @@ xe_pt_stage_bind(struct xe_tile *tile, struct
>> xe_vma *vma,
>> struct xe_pt *pt = xe_vma_vm(vma)->pt_root[tile->id];
>> int ret;
>> - if ((vma->gpuva.flags & XE_VMA_ATOMIC_PTE_BIT) &&
>> - (is_devmem || !IS_DGFX(xe)))
>> - xe_walk.default_pte |= XE_USM_PPGTT_PTE_AE;
>> + /**
>> + * Default atomic expectations for different allocation
>> scenarios are as follows:
>> + *
>> + * 1. Traditional API: When the VM is not in fault mode or LR mode:
>> + * - Device atomics are expected to function with all
>> allocations.
>> + *
>> + * 2. Compute/SVM API: When the VM is either in fault mode or LR
>> mode:
>> + * - Device atomics are the default behavior when the bo is
>> placed in a single region.
>> + * - In all other cases device atomics will be disabled with
>> AE=0 until an application
>> + * request differently using a ioctl like madvise.
>> + */
>> + if (vma->gpuva.flags & XE_VMA_ATOMIC_PTE_BIT) {
>> + if (xe_vm_in_fault_mode(xe_vma_vm(vma)) ||
>> + xe_vm_in_lr_mode(xe_vma_vm(vma))) {
>> + if (bo && xe_bo_has_single_placement(bo))
>> + xe_walk.default_pte |= XE_USM_PPGTT_PTE_AE;
>> + } else {
>> + xe_walk.default_pte |= XE_USM_PPGTT_PTE_AE;
>> + }
>> + }
>> if (is_devmem) {
>> xe_walk.default_pte |= XE_PPGTT_PTE_DM;
>> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
>> index e41345c1627d..ac08b6fd537e 100644
>> --- a/drivers/gpu/drm/xe/xe_vm.c
>> +++ b/drivers/gpu/drm/xe/xe_vm.c
>> @@ -805,7 +805,8 @@ static struct xe_vma *xe_vma_create(struct xe_vm
>> *vm,
>> for_each_tile(tile, vm->xe, id)
>> vma->tile_mask |= 0x1 << id;
>> - if (GRAPHICS_VER(vm->xe) >= 20 || vm->xe->info.platform ==
>> XE_PVC)
>> + if (vm->xe->info.has_atomic_enable_pte_bit &&
>> + vm->xe->info.has_device_atomics_on_smem)
>> vma->gpuva.flags |= XE_VMA_ATOMIC_PTE_BIT;
>> vma->pat_index = pat_index;
next prev parent reply other threads:[~2024-04-30 16:24 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-25 22:23 [PATCH v4 0/5] Refactor default device atomic settings Nirmoy Das
2024-04-25 22:23 ` [PATCH v4 1/5] drm/xe: Introduce has_atomic_enable_pte_bit device info Nirmoy Das
2024-04-25 22:23 ` [PATCH v4 2/5] drm/xe: Move vm bind bo validation to a helper function Nirmoy Das
2024-04-25 22:23 ` [PATCH v4 3/5] drm/xe: Introduce has_device_atomics_on_smem device info Nirmoy Das
2024-04-29 20:19 ` Welty, Brian
2024-04-29 20:42 ` Nirmoy Das
2024-04-25 22:23 ` [PATCH v4 4/5] drm/xe: Add function to check if BO has single placement Nirmoy Das
2024-04-25 22:23 ` [PATCH v4 5/5] drm/xe: Refactor default device atomic settings Nirmoy Das
2024-04-29 20:14 ` Welty, Brian
2024-04-29 20:37 ` Nirmoy Das
2024-04-30 16:23 ` Nirmoy Das [this message]
2024-04-25 22:49 ` ✓ CI.Patch_applied: success for " Patchwork
2024-04-25 22:50 ` ✓ CI.checkpatch: " Patchwork
2024-04-25 22:51 ` ✓ CI.KUnit: " Patchwork
2024-04-25 23:04 ` ✓ CI.Build: " Patchwork
2024-04-25 23:06 ` ✗ CI.Hooks: failure " Patchwork
2024-04-25 23:07 ` ✗ CI.checksparse: warning " Patchwork
2024-04-25 23:30 ` ✗ CI.BAT: failure " Patchwork
2024-04-26 9:06 ` ✓ CI.Patch_applied: success for Refactor default device atomic settings (rev2) Patchwork
2024-04-26 9:06 ` ✓ CI.checkpatch: " Patchwork
2024-04-26 9:07 ` ✓ CI.KUnit: " Patchwork
2024-04-26 9:19 ` ✓ CI.Build: " Patchwork
2024-04-26 9:21 ` ✓ CI.Hooks: " Patchwork
2024-04-26 9:23 ` ✓ CI.checksparse: " Patchwork
2024-04-26 9:58 ` ✗ CI.BAT: 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=d50f96bd-be0e-4a54-97e0-4d179c7a44b8@intel.com \
--to=nirmoy.das@intel.com \
--cc=brian.welty@intel.com \
--cc=intel-xe@lists.freedesktop.org \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).