[Intel-gfx] [PATCH v4 2/4] drm/i915: Use the vma resource as argument for gtt binding / unbinding

[Zeng, Oak]

Regards,
Oak

> -----Original Message-----
> From: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> Sent: January 5, 2022 8:44 AM
> To: Zeng, Oak <oak.zeng at intel.com>; intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org; Bloomfield, Jon
> <jon.bloomfield at intel.com>; Vetter, Daniel <daniel.vetter at intel.com>; Wilson, Chris P <chris.p.wilson at intel.com>
> Cc: Auld, Matthew <matthew.auld at intel.com>
> Subject: Re: [Intel-gfx] [PATCH v4 2/4] drm/i915: Use the vma resource as argument for gtt binding / unbinding
> 
> 
> On 1/4/22 17:07, Thomas Hellström wrote:
> > Hi, Oak,
> >
> > On 1/4/22 16:35, Zeng, Oak wrote:
> >>
> >> Regards,
> >> Oak
> >>
> >>> -----Original Message-----
> >>> From: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> >>> Sent: January 4, 2022 3:29 AM
> >>> To: Zeng, Oak <oak.zeng at intel.com>; intel-gfx at lists.freedesktop.org;
> >>> dri-devel at lists.freedesktop.org
> >>> Cc: Auld, Matthew <matthew.auld at intel.com>
> >>> Subject: Re: [Intel-gfx] [PATCH v4 2/4] drm/i915: Use the vma
> >>> resource as argument for gtt binding / unbinding
> >>>
> >>> Hi, Oak.
> >>>
> >>> On 1/4/22 00:08, Zeng, Oak wrote:
> >>>> Regards,
> >>>> Oak
> >>> Looks like your emails always start with "Regards, Oak". a
> >>> misconfiguration?
> >> My mail client (outlook) is set to automatically add a regards, when
> >> I compose new mail or reply email. Not a big problem 😊
> >>
> >>>
> >>>>> -----Original Message-----
> >>>>> From: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> >>>>> Sent: January 3, 2022 1:58 PM
> >>>>> To: Zeng, Oak <oak.zeng at intel.com>;
> >>>>> intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org
> >>>>> Cc: Auld, Matthew <matthew.auld at intel.com>
> >>>>> Subject: Re: [Intel-gfx] [PATCH v4 2/4] drm/i915: Use the vma
> >>>>> resource as argument for gtt binding / unbinding
> >>>>>
> >>>>> Hi, Oak.
> >>>>>
> >>>>> On 1/3/22 19:17, Zeng, Oak wrote:
> >>>>>> Regards,
> >>>>>> Oak
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On
> >>>>>>> Behalf Of Thomas Hellström
> >>>>>>> Sent: January 3, 2022 7:00 AM
> >>>>>>> To: intel-gfx at lists.freedesktop.org;
> >>>>>>> dri-devel at lists.freedesktop.org
> >>>>>>> Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>; Auld,
> >>>>>>> Matthew <matthew.auld at intel.com>
> >>>>>>> Subject: [Intel-gfx] [PATCH v4 2/4] drm/i915: Use the vma
> >>>>>>> resource as argument for gtt binding / unbinding
> >>>>>>>
> >>>>>>> When introducing asynchronous unbinding, the vma itself may no
> >>>>>>> longer
> >>>>>>> be alive when the actual binding or unbinding takes place.
> >>>>>> Can we take an extra reference counter of the vma to keep the vma
> >>>>>> alive, until the actual binding/unbinding takes place?
> >>>>> The point here is that that's not needed, and should be avoided.
> >>>> Can you explain more why "keeping vma alive until unbinding takes
> >>>> place" should be avoided?
> >>>>
> >>>> As I understand it, your series introduce asynchronized unbinding.
> >>>> But since vma might be no longer alive at the time of unbinding.
> >>> To overcome this difficulty, you introduce a vma resource structure
> >>> and you guarantee vma resource is alive at bind/unbind time. So
> >>> you can use vma resource for the bind/unbind operation. My question
> >>> is, can we achieve the asynchronized unbinding still using vma
> >>> structure by keeping vma structure alive ( by ref count it). This
> >>> way the change should be much smaller (compared to this series). Why
> >>> it is harmful to keep the vma alive? Maybe you have other reasons to
> >>> introduce vma resource that I don't see.
> >>>
> >>> When we allow asynchronous unbinding, it's allowed to immediately
> >>> rebind
> >>> the vma, possibly into the same gpu virtual address, but with different
> >>> pages. And when doing that we don't want to block waiting for the
> >>> unbind
> >>> to execute.
> >> Imagine this sequence:
> >>
> >> 1. Virtual address a1 is bound to physical page p1
> >> 2. Unbind a1 from p1, asynchronous so actual unbind not happen yet
> >> 3. bind a1 to physical page p2, page table is changed, now a1
> >> pointing to p2 in page table.
> >> 4. #2 now take place now - since in page table, a1 points to p2 now,
> >> does a1 point to scratch page after #4?
> >
> > Here, 3) will also become async, awaiting any pending unbinds in the
> > address range provided to 3), in particular, the bind in 3) will await
> > 4). See i915_vma_resource_bind_dep_await(), and the discussion on
> > whether or not to use an interval tree for this at the start of
> > i915_vma_resource.c
> >
> >> In fact, we could allow a large number of outstanding binds
> >>> and unbinds for a vma, which makes the vma structure unsuitable to
> >>> track
> >>> this, since there will no longer be a single mapping between a set of
> >>> active pages and a vma, or a virtual gpu range and a vma.
> >> I agree that if pages - vma - virtual gpu range is not 1:1:1 mapping,
> >> we need introduce a finer-grained vma resource to for the non-1:1
> >> mapping. I also understand the asynchronous unbinding utilize the
> >> virtual address space more effectively. But my feeling is that this
> >> non-1:1 mapping makes our program hard to understand and maintain.
> >> Since this non- 1:1 mapping is introduced by asynchronous
> >> binding/unbinding, maybe the real question here is, is it really
> >> benefit to introduce asynchronous unbinding?
> >
> > That's a relevant question, which I've asked myself a couple of times.
> > Async unbinding has complicated things like error capture and indeed
> > complicates the understanding of the binding process as well.
> >
> > The main gain is that we avoid a sync point at LMEM eviction, enabling
> > us to pipeline eviction, moving forward it may also find use in the
> > shrinker and for user-space prematurely wanting to re-use softpin
> > addresses.
> >
> > /Thomas
> >
> >>
> >> I am still not familiar enough to the codes. I suggest other experts
> >> to take a look also. @Bloomfield, Jon @Vetter, Daniel @Wilson, Chris P.
> 
> It might make sense here to point out as well that the direction from
> the arch team is towards moving towards gpu-writes of page-table entries
> for binding and unbinding, also keeping small PCI bars in mind, which
> will more or less force us to allow async unbinding for maintained
> performance.

I agree we need to support asynchronous bind/unbind. My main argue
Here was, should we introduce a vma resource structure to enable async bind/unbind, versus,
Change the life time of vma structure to achieve async bind/unbind - ie, still introduce a interval
tree for vma (not vma resource in your patch) that is to be unbind but still not unbound. When I
looked more into your patches, I realized if we reuse vma structure, we will need to add a lot of 
members to the vma structure. maybe your method of introducing vma resource is cleaner.

Regards,
Oak 


> 
> /Thomas
> 
> 
> >>
> >> Regards,
> >> Oak
> >>> Thanks,
> >>>
> >>> /Thomas
> >>>
> >>>> Regards,
> >>>> Oak
> >>>>
> >>>>    If the
> >>>>> vma is no longer alive, that means nobody uses it anymore, but the
> >>>>> GPU
> >>>>> may still have work in the pipe that references the GPU virtual
> >>>>> address.
> >>>>>
> >>>>> /Thomas.
> >>>>>