Re: [Intel-gfx] [RFC PATCH v3 04/17] drm/i915: Implement bind and unbind of object

Niranjana Vishwanathapura
2022-09-01T20:06:26Z

On Thu, Sep 01, 2022 at 03:31:13PM +1000, Dave Airlie wrote:
>On Sun, 28 Aug 2022 at 05:45, Andi Shyti <andi.shyti@linux.intel.com> wrote:
>>
>> From: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>>
>> Implement the bind and unbind of an object at the specified GPU virtual
>> addresses.
>>
>> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>> Signed-off-by: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/Makefile | 1 +
>> drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h | 21 ++
>> .../drm/i915/gem/i915_gem_vm_bind_object.c | 322 ++++++++++++++++++
>> drivers/gpu/drm/i915/gt/intel_gtt.c | 10 +
>> drivers/gpu/drm/i915/gt/intel_gtt.h | 9 +
>> drivers/gpu/drm/i915/i915_driver.c | 1 +
>> drivers/gpu/drm/i915/i915_vma.c | 3 +-
>> drivers/gpu/drm/i915/i915_vma.h | 2 -
>> drivers/gpu/drm/i915/i915_vma_types.h | 14 +
>> include/uapi/drm/i915_drm.h | 163 +++++++++
>> 10 files changed, 543 insertions(+), 3 deletions(-)
>> create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
>> create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
>>

<snip>

>> diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h
>> index be6e028c3b57d..f746fecae85ed 100644
>> --- a/drivers/gpu/drm/i915/i915_vma_types.h
>> +++ b/drivers/gpu/drm/i915/i915_vma_types.h
>> @@ -289,6 +289,20 @@ struct i915_vma {
>> /** This object's place on the active/inactive lists */
>> struct list_head vm_link;
>>
>> + /** @vm_bind_link: node for the vm_bind related lists of vm */
>> + struct list_head vm_bind_link;
>> +
>> + /** Interval tree structures for persistent vma */
>> +
>> + /** @rb: node for the interval tree of vm for persistent vmas */
>> + struct rb_node rb;
>> + /** @start: start endpoint of the rb node */
>> + u64 start;
>> + /** @last: Last endpoint of the rb node */
>> + u64 last;
>> + /** @__subtree_last: last in subtree */
>> + u64 __subtree_last;
>
>Was a drm_mm node considered for this or was it overkill?
>

We already have a drm_mm node (i915_vma.node). But currently in i915
driver, VA managment and binding of vmas are tightly coupled.
Ideally we want to decouple it and then use the same drm_mm node for
persistent vma lookup as well, instead of this new interval tree.
But decouple it is not trivial I think it needs to be carefully
done in a separate patch series to not cause any regression.

The new interval/rb tree here is an optimization for fast lookup of
persistent vma (instead of list walk) whether it is bound or not.
Eventually though, with above cleanup we should be able to use the
i915_vma.node for vma lookup (even when it is not bound).

I was briefly discussed in earlier version of this series (though topic
was different there).
https://lists.freedesktop.org/archives/intel-gfx/2022-July/301159.html

Niranjana

>Dave.