On 16/09/2021 14:41, Maarten Lankhorst wrote:
Op 03-09-2021 om 12:48 schreef Tvrtko Ursulin:
On 03/09/2021 10:31, Maarten Lankhorst wrote:
Op 31-08-2021 om 12:29 schreef Tvrtko Ursulin:
On 31/08/2021 10:34, Maarten Lankhorst wrote:
Op 31-08-2021 om 11:18 schreef Tvrtko Ursulin:
On 30/08/2021 13:09, Maarten Lankhorst wrote:
vma->obj and vma->resv are now never NULL, and some checks can be removed.
Is the direction here compatible with SVM / VM_BIND?
Yeah, it should be. The changes here make the obj->resv->lock the main lock, so it should at least simplify locking for VM_BIND.
Hm but what will vma->obj point to in case of SVM, when there is no GEM BO?
Probably to one of the bo's in i915_vm, or a dummy bo that least shares the vm_resv object, similar to the aliasing gtt handling.
As a long term or short term solution? Worried that would waste a lot of SLAB space just for convenience (whole struct drm_i915_gem_object just to store a single pointer to a dma_resv object, if I got that right), while it should be possible to come up with a cleaner design.
Even for the upcoming page granularity work we will need multiple VMAs point to single GEM bo in ppgtt and that, when SVM is considered, could for instance mean that VMAs should instead be backed by some new backing store objects, which in turn may (or may not) point to GEM BOs.
Regards,
Tvrtko
We could revisit this if it's required for SVM, since we can always optimize that code later, I don't think it's a problem to remove it for now, especially since it's a lot easier if VMA becomes a pointer to a memory slab for an object only, without its own lifetime rules. :)
Not sure what you meant with "pointer to memory slab for an object"?
But on the high level, what worries me is whether the direction is not
wrong. Sure you can change it all again later, but if we are moving
towards the world where VMAs are fundamentally and predominantly *not*
backed by GEM buffer objects, then I have to ask the question whether
this plan of rewriting everything twice is the most efficient one.
Maybe its not that scary, not sure, but again, all I see is a large-ish
series which implements some very important functionality, and _seems_
to rely on what I think is a design direction incompatible with where I
thought we needed to go.
I suppose all I can do is ask you to verify this direction with Daniel.
Maybe you already have but I did not see it in public at least. So
adding him to CC.
Regards,
Tvrtko
