Re: [Intel-gfx] [PATCH 07/19] drm/i915: vma is always backed by an object.

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 16/09/2021 15:30, Tvrtko Ursulin wrote:
> 
> 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.

Okay I reminded myself of how the SVM is implemented and perhaps it is 
not a concern. It seems that it doesn't use the VMA for more than a 
temporary vehicle during PTE setup stage.

And for page granularity paths over legacy binding I think it should 
also be fine since, as you say, all VMAs can and should point to the 
same obj.

Regards,

Tvrtko