| Message ID | 20220928061918.6340-5-niranjana.vishwanathapura@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/vm_bind: Add VM_BIND functionality | expand |
On 28/09/2022 07:19, Niranjana Vishwanathapura wrote: > Add i915_vma_instance_persistent() to create persistent vmas. > Persistent vmas will use i915_gtt_view to support partial binding. > > vma_lookup is tied to segment of the object instead of section > of VA space. Hence, it do not support aliasing. ie., multiple > mappings (at different VA) point to the same gtt_view of object. > Skip vma_lookup for persistent vmas to support aliasing. > > Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_vma.c | 39 ++++++++++++++++++++++++--- > drivers/gpu/drm/i915/i915_vma.h | 16 +++++++++-- > drivers/gpu/drm/i915/i915_vma_types.h | 7 +++++ > 3 files changed, 57 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c > index f17c09ead7d7..5839e1f55f00 100644 > --- a/drivers/gpu/drm/i915/i915_vma.c > +++ b/drivers/gpu/drm/i915/i915_vma.c > @@ -109,7 +109,8 @@ static void __i915_vma_retire(struct i915_active *ref) > static struct i915_vma * > vma_create(struct drm_i915_gem_object *obj, > struct i915_address_space *vm, > - const struct i915_gtt_view *view) > + const struct i915_gtt_view *view, > + bool skip_lookup_cache) > { > struct i915_vma *pos = ERR_PTR(-E2BIG); > struct i915_vma *vma; > @@ -196,6 +197,9 @@ vma_create(struct drm_i915_gem_object *obj, > __set_bit(I915_VMA_GGTT_BIT, __i915_vma_flags(vma)); > } > > + if (skip_lookup_cache) > + goto skip_rb_insert; > + > rb = NULL; > p = &obj->vma.tree.rb_node; > while (*p) { > @@ -220,6 +224,7 @@ vma_create(struct drm_i915_gem_object *obj, > rb_link_node(&vma->obj_node, rb, p); > rb_insert_color(&vma->obj_node, &obj->vma.tree); > > +skip_rb_insert: > if (i915_vma_is_ggtt(vma)) > /* > * We put the GGTT vma at the start of the vma-list, followed > @@ -299,7 +304,34 @@ i915_vma_instance(struct drm_i915_gem_object *obj, > > /* vma_create() will resolve the race if another creates the vma */ > if (unlikely(!vma)) > - vma = vma_create(obj, vm, view); > + vma = vma_create(obj, vm, view, false); > + > + GEM_BUG_ON(!IS_ERR(vma) && i915_vma_compare(vma, vm, view)); > + return vma; > +} > + > +/** > + * i915_vma_create_persistent - create a persistent VMA > + * @obj: parent &struct drm_i915_gem_object to be mapped > + * @vm: address space in which the mapping is located > + * @view: additional mapping requirements > + * > + * Creates a persistent vma. > + * > + * Returns the vma, or an error pointer. > + */ > +struct i915_vma * > +i915_vma_create_persistent(struct drm_i915_gem_object *obj, > + struct i915_address_space *vm, > + const struct i915_gtt_view *view) > +{ > + struct i915_vma *vma; > + > + GEM_BUG_ON(!kref_read(&vm->ref)); > + > + vma = vma_create(obj, vm, view, true); > + if (!IS_ERR(vma)) > + i915_vma_set_persistent(vma); > > GEM_BUG_ON(!IS_ERR(vma) && i915_vma_compare(vma, vm, view)); > return vma; > @@ -1666,7 +1698,8 @@ static void release_references(struct i915_vma *vma, struct intel_gt *gt, > > spin_lock(&obj->vma.lock); > list_del(&vma->obj_link); > - if (!RB_EMPTY_NODE(&vma->obj_node)) > + if (!i915_vma_is_persistent(vma) && Thinking out loud - maybe you don't need the extra condition? But it is good for self-documenting purposes in any case. > + !RB_EMPTY_NODE(&vma->obj_node)) > rb_erase(&vma->obj_node, &obj->vma.tree); > > spin_unlock(&obj->vma.lock); > diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h > index aecd9c64486b..51e712de380a 100644 > --- a/drivers/gpu/drm/i915/i915_vma.h > +++ b/drivers/gpu/drm/i915/i915_vma.h > @@ -44,6 +44,10 @@ struct i915_vma * > i915_vma_instance(struct drm_i915_gem_object *obj, > struct i915_address_space *vm, > const struct i915_gtt_view *view); > +struct i915_vma * > +i915_vma_create_persistent(struct drm_i915_gem_object *obj, > + struct i915_address_space *vm, > + const struct i915_gtt_view *view); > > void i915_vma_unpin_and_release(struct i915_vma **p_vma, unsigned int flags); > #define I915_VMA_RELEASE_MAP BIT(0) > @@ -138,6 +142,16 @@ static inline u32 i915_ggtt_pin_bias(struct i915_vma *vma) > return i915_vm_to_ggtt(vma->vm)->pin_bias; > } > > +static inline bool i915_vma_is_persistent(const struct i915_vma *vma) > +{ > + return test_bit(I915_VMA_PERSISTENT_BIT, __i915_vma_flags(vma)); > +} > + > +static inline void i915_vma_set_persistent(struct i915_vma *vma) > +{ > + set_bit(I915_VMA_PERSISTENT_BIT, __i915_vma_flags(vma)); > +} > + > static inline struct i915_vma *i915_vma_get(struct i915_vma *vma) > { > i915_gem_object_get(vma->obj); > @@ -164,8 +178,6 @@ i915_vma_compare(struct i915_vma *vma, > { > ptrdiff_t cmp; > > - GEM_BUG_ON(view && !i915_is_ggtt_or_dpt(vm)); Or explicitly add persistent? Regards, Tvrtko > - > cmp = ptrdiff(vma->vm, vm); > if (cmp) > return cmp; > diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h > index ec0f6c9f57d0..2200f1f103ba 100644 > --- a/drivers/gpu/drm/i915/i915_vma_types.h > +++ b/drivers/gpu/drm/i915/i915_vma_types.h > @@ -264,6 +264,13 @@ struct i915_vma { > #define I915_VMA_SCANOUT_BIT 17 > #define I915_VMA_SCANOUT ((int)BIT(I915_VMA_SCANOUT_BIT)) > > +/** > + * I915_VMA_PERSISTENT_BIT: > + * The vma is persistent (created with VM_BIND call). > + */ > +#define I915_VMA_PERSISTENT_BIT 19 > +#define I915_VMA_PERSISTENT ((int)BIT(I915_VMA_PERSISTENT_BIT)) > + > struct i915_active active; > > #define I915_VMA_PAGES_BIAS 24
Hi Niranjana, On Tue, Sep 27, 2022 at 11:19:06PM -0700, Niranjana Vishwanathapura wrote: > Add i915_vma_instance_persistent() to create persistent vmas. > Persistent vmas will use i915_gtt_view to support partial binding. > > vma_lookup is tied to segment of the object instead of section > of VA space. Hence, it do not support aliasing. ie., multiple > mappings (at different VA) point to the same gtt_view of object. > Skip vma_lookup for persistent vmas to support aliasing. > > Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_vma.c | 39 ++++++++++++++++++++++++--- > drivers/gpu/drm/i915/i915_vma.h | 16 +++++++++-- > drivers/gpu/drm/i915/i915_vma_types.h | 7 +++++ > 3 files changed, 57 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c > index f17c09ead7d7..5839e1f55f00 100644 > --- a/drivers/gpu/drm/i915/i915_vma.c > +++ b/drivers/gpu/drm/i915/i915_vma.c > @@ -109,7 +109,8 @@ static void __i915_vma_retire(struct i915_active *ref) > static struct i915_vma * > vma_create(struct drm_i915_gem_object *obj, > struct i915_address_space *vm, > - const struct i915_gtt_view *view) > + const struct i915_gtt_view *view, > + bool skip_lookup_cache) > { > struct i915_vma *pos = ERR_PTR(-E2BIG); > struct i915_vma *vma; > @@ -196,6 +197,9 @@ vma_create(struct drm_i915_gem_object *obj, > __set_bit(I915_VMA_GGTT_BIT, __i915_vma_flags(vma)); > } > > + if (skip_lookup_cache) > + goto skip_rb_insert; > + > rb = NULL; > p = &obj->vma.tree.rb_node; > while (*p) { > @@ -220,6 +224,7 @@ vma_create(struct drm_i915_gem_object *obj, > rb_link_node(&vma->obj_node, rb, p); > rb_insert_color(&vma->obj_node, &obj->vma.tree); > > +skip_rb_insert: > if (i915_vma_is_ggtt(vma)) > /* > * We put the GGTT vma at the start of the vma-list, followed > @@ -299,7 +304,34 @@ i915_vma_instance(struct drm_i915_gem_object *obj, > > /* vma_create() will resolve the race if another creates the vma */ > if (unlikely(!vma)) > - vma = vma_create(obj, vm, view); > + vma = vma_create(obj, vm, view, false); > + > + GEM_BUG_ON(!IS_ERR(vma) && i915_vma_compare(vma, vm, view)); > + return vma; > +} > + > +/** > + * i915_vma_create_persistent - create a persistent VMA > + * @obj: parent &struct drm_i915_gem_object to be mapped > + * @vm: address space in which the mapping is located > + * @view: additional mapping requirements > + * > + * Creates a persistent vma. > + * > + * Returns the vma, or an error pointer. > + */ > +struct i915_vma * > +i915_vma_create_persistent(struct drm_i915_gem_object *obj, > + struct i915_address_space *vm, > + const struct i915_gtt_view *view) > +{ > + struct i915_vma *vma; > + > + GEM_BUG_ON(!kref_read(&vm->ref)); > + > + vma = vma_create(obj, vm, view, true); > + if (!IS_ERR(vma)) > + i915_vma_set_persistent(vma); > > GEM_BUG_ON(!IS_ERR(vma) && i915_vma_compare(vma, vm, view)); > return vma; > @@ -1666,7 +1698,8 @@ static void release_references(struct i915_vma *vma, struct intel_gt *gt, > > spin_lock(&obj->vma.lock); > list_del(&vma->obj_link); > - if (!RB_EMPTY_NODE(&vma->obj_node)) > + if (!i915_vma_is_persistent(vma) && > + !RB_EMPTY_NODE(&vma->obj_node)) > rb_erase(&vma->obj_node, &obj->vma.tree); > > spin_unlock(&obj->vma.lock); > diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h > index aecd9c64486b..51e712de380a 100644 > --- a/drivers/gpu/drm/i915/i915_vma.h > +++ b/drivers/gpu/drm/i915/i915_vma.h > @@ -44,6 +44,10 @@ struct i915_vma * > i915_vma_instance(struct drm_i915_gem_object *obj, > struct i915_address_space *vm, > const struct i915_gtt_view *view); > +struct i915_vma * > +i915_vma_create_persistent(struct drm_i915_gem_object *obj, > + struct i915_address_space *vm, > + const struct i915_gtt_view *view); > > void i915_vma_unpin_and_release(struct i915_vma **p_vma, unsigned int flags); > #define I915_VMA_RELEASE_MAP BIT(0) > @@ -138,6 +142,16 @@ static inline u32 i915_ggtt_pin_bias(struct i915_vma *vma) > return i915_vm_to_ggtt(vma->vm)->pin_bias; > } > > +static inline bool i915_vma_is_persistent(const struct i915_vma *vma) > +{ > + return test_bit(I915_VMA_PERSISTENT_BIT, __i915_vma_flags(vma)); > +} > + > +static inline void i915_vma_set_persistent(struct i915_vma *vma) > +{ > + set_bit(I915_VMA_PERSISTENT_BIT, __i915_vma_flags(vma)); > +} > + > static inline struct i915_vma *i915_vma_get(struct i915_vma *vma) > { > i915_gem_object_get(vma->obj); > @@ -164,8 +178,6 @@ i915_vma_compare(struct i915_vma *vma, > { > ptrdiff_t cmp; > > - GEM_BUG_ON(view && !i915_is_ggtt_or_dpt(vm)); > - > cmp = ptrdiff(vma->vm, vm); > if (cmp) > return cmp; > diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h > index ec0f6c9f57d0..2200f1f103ba 100644 > --- a/drivers/gpu/drm/i915/i915_vma_types.h > +++ b/drivers/gpu/drm/i915/i915_vma_types.h > @@ -264,6 +264,13 @@ struct i915_vma { > #define I915_VMA_SCANOUT_BIT 17 > #define I915_VMA_SCANOUT ((int)BIT(I915_VMA_SCANOUT_BIT)) > > +/** > + * I915_VMA_PERSISTENT_BIT: > + * The vma is persistent (created with VM_BIND call). > + */ > +#define I915_VMA_PERSISTENT_BIT 19 > +#define I915_VMA_PERSISTENT ((int)BIT(I915_VMA_PERSISTENT_BIT)) > + are we using I915_VMA_PERSISTENT anywhere? Andi > struct i915_active active; > > #define I915_VMA_PAGES_BIAS 24 > -- > 2.21.0.rc0.32.g243a4c7e27
On Wed, Sep 28, 2022 at 08:38:39AM +0100, Tvrtko Ursulin wrote: > >On 28/09/2022 07:19, Niranjana Vishwanathapura wrote: >>Add i915_vma_instance_persistent() to create persistent vmas. >>Persistent vmas will use i915_gtt_view to support partial binding. >> >>vma_lookup is tied to segment of the object instead of section >>of VA space. Hence, it do not support aliasing. ie., multiple >>mappings (at different VA) point to the same gtt_view of object. >>Skip vma_lookup for persistent vmas to support aliasing. >> >>Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> >>Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> >>--- >> drivers/gpu/drm/i915/i915_vma.c | 39 ++++++++++++++++++++++++--- >> drivers/gpu/drm/i915/i915_vma.h | 16 +++++++++-- >> drivers/gpu/drm/i915/i915_vma_types.h | 7 +++++ >> 3 files changed, 57 insertions(+), 5 deletions(-) >> >>diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c >>index f17c09ead7d7..5839e1f55f00 100644 >>--- a/drivers/gpu/drm/i915/i915_vma.c >>+++ b/drivers/gpu/drm/i915/i915_vma.c >>@@ -109,7 +109,8 @@ static void __i915_vma_retire(struct i915_active *ref) >> static struct i915_vma * >> vma_create(struct drm_i915_gem_object *obj, >> struct i915_address_space *vm, >>- const struct i915_gtt_view *view) >>+ const struct i915_gtt_view *view, >>+ bool skip_lookup_cache) >> { >> struct i915_vma *pos = ERR_PTR(-E2BIG); >> struct i915_vma *vma; >>@@ -196,6 +197,9 @@ vma_create(struct drm_i915_gem_object *obj, >> __set_bit(I915_VMA_GGTT_BIT, __i915_vma_flags(vma)); >> } >>+ if (skip_lookup_cache) >>+ goto skip_rb_insert; >>+ >> rb = NULL; >> p = &obj->vma.tree.rb_node; >> while (*p) { >>@@ -220,6 +224,7 @@ vma_create(struct drm_i915_gem_object *obj, >> rb_link_node(&vma->obj_node, rb, p); >> rb_insert_color(&vma->obj_node, &obj->vma.tree); >>+skip_rb_insert: >> if (i915_vma_is_ggtt(vma)) >> /* >> * We put the GGTT vma at the start of the vma-list, followed >>@@ -299,7 +304,34 @@ i915_vma_instance(struct drm_i915_gem_object *obj, >> /* vma_create() will resolve the race if another creates the vma */ >> if (unlikely(!vma)) >>- vma = vma_create(obj, vm, view); >>+ vma = vma_create(obj, vm, view, false); >>+ >>+ GEM_BUG_ON(!IS_ERR(vma) && i915_vma_compare(vma, vm, view)); >>+ return vma; >>+} >>+ >>+/** >>+ * i915_vma_create_persistent - create a persistent VMA >>+ * @obj: parent &struct drm_i915_gem_object to be mapped >>+ * @vm: address space in which the mapping is located >>+ * @view: additional mapping requirements >>+ * >>+ * Creates a persistent vma. >>+ * >>+ * Returns the vma, or an error pointer. >>+ */ >>+struct i915_vma * >>+i915_vma_create_persistent(struct drm_i915_gem_object *obj, >>+ struct i915_address_space *vm, >>+ const struct i915_gtt_view *view) >>+{ >>+ struct i915_vma *vma; >>+ >>+ GEM_BUG_ON(!kref_read(&vm->ref)); >>+ >>+ vma = vma_create(obj, vm, view, true); >>+ if (!IS_ERR(vma)) >>+ i915_vma_set_persistent(vma); >> GEM_BUG_ON(!IS_ERR(vma) && i915_vma_compare(vma, vm, view)); >> return vma; >>@@ -1666,7 +1698,8 @@ static void release_references(struct i915_vma *vma, struct intel_gt *gt, >> spin_lock(&obj->vma.lock); >> list_del(&vma->obj_link); >>- if (!RB_EMPTY_NODE(&vma->obj_node)) >>+ if (!i915_vma_is_persistent(vma) && > >Thinking out loud - maybe you don't need the extra condition? But it >is good for self-documenting purposes in any case. Thanks, yah, it is not needed, will remove this update. > >>+ !RB_EMPTY_NODE(&vma->obj_node)) >> rb_erase(&vma->obj_node, &obj->vma.tree); >> spin_unlock(&obj->vma.lock); >>diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h >>index aecd9c64486b..51e712de380a 100644 >>--- a/drivers/gpu/drm/i915/i915_vma.h >>+++ b/drivers/gpu/drm/i915/i915_vma.h >>@@ -44,6 +44,10 @@ struct i915_vma * >> i915_vma_instance(struct drm_i915_gem_object *obj, >> struct i915_address_space *vm, >> const struct i915_gtt_view *view); >>+struct i915_vma * >>+i915_vma_create_persistent(struct drm_i915_gem_object *obj, >>+ struct i915_address_space *vm, >>+ const struct i915_gtt_view *view); >> void i915_vma_unpin_and_release(struct i915_vma **p_vma, unsigned int flags); >> #define I915_VMA_RELEASE_MAP BIT(0) >>@@ -138,6 +142,16 @@ static inline u32 i915_ggtt_pin_bias(struct i915_vma *vma) >> return i915_vm_to_ggtt(vma->vm)->pin_bias; >> } >>+static inline bool i915_vma_is_persistent(const struct i915_vma *vma) >>+{ >>+ return test_bit(I915_VMA_PERSISTENT_BIT, __i915_vma_flags(vma)); >>+} >>+ >>+static inline void i915_vma_set_persistent(struct i915_vma *vma) >>+{ >>+ set_bit(I915_VMA_PERSISTENT_BIT, __i915_vma_flags(vma)); >>+} >>+ >> static inline struct i915_vma *i915_vma_get(struct i915_vma *vma) >> { >> i915_gem_object_get(vma->obj); >>@@ -164,8 +178,6 @@ i915_vma_compare(struct i915_vma *vma, >> { >> ptrdiff_t cmp; >>- GEM_BUG_ON(view && !i915_is_ggtt_or_dpt(vm)); >Or explicitly add persistent? Ok, will update. Regards, Niranjana > >Regards, > >Tvrtko > >>- >> cmp = ptrdiff(vma->vm, vm); >> if (cmp) >> return cmp; >>diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h >>index ec0f6c9f57d0..2200f1f103ba 100644 >>--- a/drivers/gpu/drm/i915/i915_vma_types.h >>+++ b/drivers/gpu/drm/i915/i915_vma_types.h >>@@ -264,6 +264,13 @@ struct i915_vma { >> #define I915_VMA_SCANOUT_BIT 17 >> #define I915_VMA_SCANOUT ((int)BIT(I915_VMA_SCANOUT_BIT)) >>+/** >>+ * I915_VMA_PERSISTENT_BIT: >>+ * The vma is persistent (created with VM_BIND call). >>+ */ >>+#define I915_VMA_PERSISTENT_BIT 19 >>+#define I915_VMA_PERSISTENT ((int)BIT(I915_VMA_PERSISTENT_BIT)) >>+ >> struct i915_active active; >> #define I915_VMA_PAGES_BIAS 24
On Wed, Sep 28, 2022 at 04:44:08PM +0200, Andi Shyti wrote: >Hi Niranjana, > >On Tue, Sep 27, 2022 at 11:19:06PM -0700, Niranjana Vishwanathapura wrote: >> Add i915_vma_instance_persistent() to create persistent vmas. >> Persistent vmas will use i915_gtt_view to support partial binding. >> >> vma_lookup is tied to segment of the object instead of section >> of VA space. Hence, it do not support aliasing. ie., multiple >> mappings (at different VA) point to the same gtt_view of object. >> Skip vma_lookup for persistent vmas to support aliasing. >> >> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> >> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> >> --- >> drivers/gpu/drm/i915/i915_vma.c | 39 ++++++++++++++++++++++++--- >> drivers/gpu/drm/i915/i915_vma.h | 16 +++++++++-- >> drivers/gpu/drm/i915/i915_vma_types.h | 7 +++++ >> 3 files changed, 57 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c >> index f17c09ead7d7..5839e1f55f00 100644 >> --- a/drivers/gpu/drm/i915/i915_vma.c >> +++ b/drivers/gpu/drm/i915/i915_vma.c >> @@ -109,7 +109,8 @@ static void __i915_vma_retire(struct i915_active *ref) >> static struct i915_vma * >> vma_create(struct drm_i915_gem_object *obj, >> struct i915_address_space *vm, >> - const struct i915_gtt_view *view) >> + const struct i915_gtt_view *view, >> + bool skip_lookup_cache) >> { >> struct i915_vma *pos = ERR_PTR(-E2BIG); >> struct i915_vma *vma; >> @@ -196,6 +197,9 @@ vma_create(struct drm_i915_gem_object *obj, >> __set_bit(I915_VMA_GGTT_BIT, __i915_vma_flags(vma)); >> } >> >> + if (skip_lookup_cache) >> + goto skip_rb_insert; >> + >> rb = NULL; >> p = &obj->vma.tree.rb_node; >> while (*p) { >> @@ -220,6 +224,7 @@ vma_create(struct drm_i915_gem_object *obj, >> rb_link_node(&vma->obj_node, rb, p); >> rb_insert_color(&vma->obj_node, &obj->vma.tree); >> >> +skip_rb_insert: >> if (i915_vma_is_ggtt(vma)) >> /* >> * We put the GGTT vma at the start of the vma-list, followed >> @@ -299,7 +304,34 @@ i915_vma_instance(struct drm_i915_gem_object *obj, >> >> /* vma_create() will resolve the race if another creates the vma */ >> if (unlikely(!vma)) >> - vma = vma_create(obj, vm, view); >> + vma = vma_create(obj, vm, view, false); >> + >> + GEM_BUG_ON(!IS_ERR(vma) && i915_vma_compare(vma, vm, view)); >> + return vma; >> +} >> + >> +/** >> + * i915_vma_create_persistent - create a persistent VMA >> + * @obj: parent &struct drm_i915_gem_object to be mapped >> + * @vm: address space in which the mapping is located >> + * @view: additional mapping requirements >> + * >> + * Creates a persistent vma. >> + * >> + * Returns the vma, or an error pointer. >> + */ >> +struct i915_vma * >> +i915_vma_create_persistent(struct drm_i915_gem_object *obj, >> + struct i915_address_space *vm, >> + const struct i915_gtt_view *view) >> +{ >> + struct i915_vma *vma; >> + >> + GEM_BUG_ON(!kref_read(&vm->ref)); >> + >> + vma = vma_create(obj, vm, view, true); >> + if (!IS_ERR(vma)) >> + i915_vma_set_persistent(vma); >> >> GEM_BUG_ON(!IS_ERR(vma) && i915_vma_compare(vma, vm, view)); >> return vma; >> @@ -1666,7 +1698,8 @@ static void release_references(struct i915_vma *vma, struct intel_gt *gt, >> >> spin_lock(&obj->vma.lock); >> list_del(&vma->obj_link); >> - if (!RB_EMPTY_NODE(&vma->obj_node)) >> + if (!i915_vma_is_persistent(vma) && >> + !RB_EMPTY_NODE(&vma->obj_node)) >> rb_erase(&vma->obj_node, &obj->vma.tree); >> >> spin_unlock(&obj->vma.lock); >> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h >> index aecd9c64486b..51e712de380a 100644 >> --- a/drivers/gpu/drm/i915/i915_vma.h >> +++ b/drivers/gpu/drm/i915/i915_vma.h >> @@ -44,6 +44,10 @@ struct i915_vma * >> i915_vma_instance(struct drm_i915_gem_object *obj, >> struct i915_address_space *vm, >> const struct i915_gtt_view *view); >> +struct i915_vma * >> +i915_vma_create_persistent(struct drm_i915_gem_object *obj, >> + struct i915_address_space *vm, >> + const struct i915_gtt_view *view); >> >> void i915_vma_unpin_and_release(struct i915_vma **p_vma, unsigned int flags); >> #define I915_VMA_RELEASE_MAP BIT(0) >> @@ -138,6 +142,16 @@ static inline u32 i915_ggtt_pin_bias(struct i915_vma *vma) >> return i915_vm_to_ggtt(vma->vm)->pin_bias; >> } >> >> +static inline bool i915_vma_is_persistent(const struct i915_vma *vma) >> +{ >> + return test_bit(I915_VMA_PERSISTENT_BIT, __i915_vma_flags(vma)); >> +} >> + >> +static inline void i915_vma_set_persistent(struct i915_vma *vma) >> +{ >> + set_bit(I915_VMA_PERSISTENT_BIT, __i915_vma_flags(vma)); >> +} >> + >> static inline struct i915_vma *i915_vma_get(struct i915_vma *vma) >> { >> i915_gem_object_get(vma->obj); >> @@ -164,8 +178,6 @@ i915_vma_compare(struct i915_vma *vma, >> { >> ptrdiff_t cmp; >> >> - GEM_BUG_ON(view && !i915_is_ggtt_or_dpt(vm)); >> - >> cmp = ptrdiff(vma->vm, vm); >> if (cmp) >> return cmp; >> diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h >> index ec0f6c9f57d0..2200f1f103ba 100644 >> --- a/drivers/gpu/drm/i915/i915_vma_types.h >> +++ b/drivers/gpu/drm/i915/i915_vma_types.h >> @@ -264,6 +264,13 @@ struct i915_vma { >> #define I915_VMA_SCANOUT_BIT 17 >> #define I915_VMA_SCANOUT ((int)BIT(I915_VMA_SCANOUT_BIT)) >> >> +/** >> + * I915_VMA_PERSISTENT_BIT: >> + * The vma is persistent (created with VM_BIND call). >> + */ >> +#define I915_VMA_PERSISTENT_BIT 19 >> +#define I915_VMA_PERSISTENT ((int)BIT(I915_VMA_PERSISTENT_BIT)) >> + > >are we using I915_VMA_PERSISTENT anywhere? Thanks, we are not. I just followed the convention here. Looks like many definitions are not used here. Will remove. Regards, Niranjana > >Andi > >> struct i915_active active; >> >> #define I915_VMA_PAGES_BIAS 24 >> -- >> 2.21.0.rc0.32.g243a4c7e27
On 28/09/2022 07:19, Niranjana Vishwanathapura wrote: > Add i915_vma_instance_persistent() to create persistent vmas. > Persistent vmas will use i915_gtt_view to support partial binding. > > vma_lookup is tied to segment of the object instead of section > of VA space. Hence, it do not support aliasing. ie., multiple > mappings (at different VA) point to the same gtt_view of object. > Skip vma_lookup for persistent vmas to support aliasing. > > Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> Acked-by: Matthew Auld <matthew.auld@intel.com>
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index f17c09ead7d7..5839e1f55f00 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -109,7 +109,8 @@ static void __i915_vma_retire(struct i915_active *ref) static struct i915_vma * vma_create(struct drm_i915_gem_object *obj, struct i915_address_space *vm, - const struct i915_gtt_view *view) + const struct i915_gtt_view *view, + bool skip_lookup_cache) { struct i915_vma *pos = ERR_PTR(-E2BIG); struct i915_vma *vma; @@ -196,6 +197,9 @@ vma_create(struct drm_i915_gem_object *obj, __set_bit(I915_VMA_GGTT_BIT, __i915_vma_flags(vma)); } + if (skip_lookup_cache) + goto skip_rb_insert; + rb = NULL; p = &obj->vma.tree.rb_node; while (*p) { @@ -220,6 +224,7 @@ vma_create(struct drm_i915_gem_object *obj, rb_link_node(&vma->obj_node, rb, p); rb_insert_color(&vma->obj_node, &obj->vma.tree); +skip_rb_insert: if (i915_vma_is_ggtt(vma)) /* * We put the GGTT vma at the start of the vma-list, followed @@ -299,7 +304,34 @@ i915_vma_instance(struct drm_i915_gem_object *obj, /* vma_create() will resolve the race if another creates the vma */ if (unlikely(!vma)) - vma = vma_create(obj, vm, view); + vma = vma_create(obj, vm, view, false); + + GEM_BUG_ON(!IS_ERR(vma) && i915_vma_compare(vma, vm, view)); + return vma; +} + +/** + * i915_vma_create_persistent - create a persistent VMA + * @obj: parent &struct drm_i915_gem_object to be mapped + * @vm: address space in which the mapping is located + * @view: additional mapping requirements + * + * Creates a persistent vma. + * + * Returns the vma, or an error pointer. + */ +struct i915_vma * +i915_vma_create_persistent(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_gtt_view *view) +{ + struct i915_vma *vma; + + GEM_BUG_ON(!kref_read(&vm->ref)); + + vma = vma_create(obj, vm, view, true); + if (!IS_ERR(vma)) + i915_vma_set_persistent(vma); GEM_BUG_ON(!IS_ERR(vma) && i915_vma_compare(vma, vm, view)); return vma; @@ -1666,7 +1698,8 @@ static void release_references(struct i915_vma *vma, struct intel_gt *gt, spin_lock(&obj->vma.lock); list_del(&vma->obj_link); - if (!RB_EMPTY_NODE(&vma->obj_node)) + if (!i915_vma_is_persistent(vma) && + !RB_EMPTY_NODE(&vma->obj_node)) rb_erase(&vma->obj_node, &obj->vma.tree); spin_unlock(&obj->vma.lock); diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h index aecd9c64486b..51e712de380a 100644 --- a/drivers/gpu/drm/i915/i915_vma.h +++ b/drivers/gpu/drm/i915/i915_vma.h @@ -44,6 +44,10 @@ struct i915_vma * i915_vma_instance(struct drm_i915_gem_object *obj, struct i915_address_space *vm, const struct i915_gtt_view *view); +struct i915_vma * +i915_vma_create_persistent(struct drm_i915_gem_object *obj, + struct i915_address_space *vm, + const struct i915_gtt_view *view); void i915_vma_unpin_and_release(struct i915_vma **p_vma, unsigned int flags); #define I915_VMA_RELEASE_MAP BIT(0) @@ -138,6 +142,16 @@ static inline u32 i915_ggtt_pin_bias(struct i915_vma *vma) return i915_vm_to_ggtt(vma->vm)->pin_bias; } +static inline bool i915_vma_is_persistent(const struct i915_vma *vma) +{ + return test_bit(I915_VMA_PERSISTENT_BIT, __i915_vma_flags(vma)); +} + +static inline void i915_vma_set_persistent(struct i915_vma *vma) +{ + set_bit(I915_VMA_PERSISTENT_BIT, __i915_vma_flags(vma)); +} + static inline struct i915_vma *i915_vma_get(struct i915_vma *vma) { i915_gem_object_get(vma->obj); @@ -164,8 +178,6 @@ i915_vma_compare(struct i915_vma *vma, { ptrdiff_t cmp; - GEM_BUG_ON(view && !i915_is_ggtt_or_dpt(vm)); - cmp = ptrdiff(vma->vm, vm); if (cmp) return cmp; diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h index ec0f6c9f57d0..2200f1f103ba 100644 --- a/drivers/gpu/drm/i915/i915_vma_types.h +++ b/drivers/gpu/drm/i915/i915_vma_types.h @@ -264,6 +264,13 @@ struct i915_vma { #define I915_VMA_SCANOUT_BIT 17 #define I915_VMA_SCANOUT ((int)BIT(I915_VMA_SCANOUT_BIT)) +/** + * I915_VMA_PERSISTENT_BIT: + * The vma is persistent (created with VM_BIND call). + */ +#define I915_VMA_PERSISTENT_BIT 19 +#define I915_VMA_PERSISTENT ((int)BIT(I915_VMA_PERSISTENT_BIT)) + struct i915_active active; #define I915_VMA_PAGES_BIAS 24