diff mbox series

[08/28] drm/i915: Create a full object for mock_ring, v2.

Message ID 20211021103605.735002-8-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [01/28] drm/i915: Fix i915_request fence wait semantics | expand

Commit Message

Maarten Lankhorst Oct. 21, 2021, 10:35 a.m. UTC 
This allows us to finally get rid of all the assumptions that vma->obj is NULL.

Changes since v1:
- Ensure the mock_ring vma is pinned to prevent a fault.
- Pin it high to avoid failure in evict_for_vma selftest.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/mock_engine.c | 38 ++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 10 deletions(-)

Comments

Matthew Auld Oct. 21, 2021, 3:57 p.m. UTC | #1 
On Thu, 21 Oct 2021 at 11:36, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
>
> This allows us to finally get rid of all the assumptions that vma->obj is NULL.
>
> Changes since v1:
> - Ensure the mock_ring vma is pinned to prevent a fault.
> - Pin it high to avoid failure in evict_for_vma selftest.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gt/mock_engine.c | 38 ++++++++++++++++++++-------
>  1 file changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
> index 8b89215afe46..bb99fc03f503 100644
> --- a/drivers/gpu/drm/i915/gt/mock_engine.c
> +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
> @@ -35,9 +35,31 @@ static void mock_timeline_unpin(struct intel_timeline *tl)
>         atomic_dec(&tl->pin_count);
>  }
>
> +static struct i915_vma *create_ring_vma(struct i915_ggtt *ggtt, int size)
> +{
> +       struct i915_address_space *vm = &ggtt->vm;
> +       struct drm_i915_private *i915 = vm->i915;
> +       struct drm_i915_gem_object *obj;
> +       struct i915_vma *vma;
> +
> +       obj = i915_gem_object_create_internal(i915, size);
> +       if (IS_ERR(obj))
> +               return ERR_CAST(obj);

We didn't want to use the dummy object here also? I guess meh?

> +
> +       vma = i915_vma_instance(obj, vm, NULL);
> +       if (IS_ERR(vma))
> +               goto err;
> +
> +       return vma;
> +
> +err:
> +       i915_gem_object_put(obj);
> +       return vma;
> +}
> +
>  static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
>  {
> -       const unsigned long sz = PAGE_SIZE / 2;
> +       const unsigned long sz = PAGE_SIZE;

Is that significant?

Reviewed-by: Matthew Auld <matthew.auld@intel.com>


>         struct intel_ring *ring;
>
>         ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
> @@ -50,15 +72,11 @@ static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
>         ring->vaddr = (void *)(ring + 1);
>         atomic_set(&ring->pin_count, 1);
>
> -       ring->vma = i915_vma_alloc();
> -       if (!ring->vma) {
> +       ring->vma = create_ring_vma(engine->gt->ggtt, PAGE_SIZE);
> +       if (IS_ERR(ring->vma)) {
>                 kfree(ring);
>                 return NULL;
>         }
> -       i915_active_init(&ring->vma->active, NULL, NULL, 0);
> -       __set_bit(I915_VMA_GGTT_BIT, __i915_vma_flags(ring->vma));
> -       __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &ring->vma->node.flags);
> -       ring->vma->node.size = sz;
>
>         intel_ring_update_space(ring);
>
> @@ -67,8 +85,7 @@ static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
>
>  static void mock_ring_free(struct intel_ring *ring)
>  {
> -       i915_active_fini(&ring->vma->active);
> -       i915_vma_free(ring->vma);
> +       i915_vma_put(ring->vma);
>
>         kfree(ring);
>  }
> @@ -125,6 +142,7 @@ static void mock_context_unpin(struct intel_context *ce)
>
>  static void mock_context_post_unpin(struct intel_context *ce)
>  {
> +       i915_vma_unpin(ce->ring->vma);
>  }
>
>  static void mock_context_destroy(struct kref *ref)
> @@ -169,7 +187,7 @@ static int mock_context_alloc(struct intel_context *ce)
>  static int mock_context_pre_pin(struct intel_context *ce,
>                                 struct i915_gem_ww_ctx *ww, void **unused)
>  {
> -       return 0;
> +       return i915_vma_pin_ww(ce->ring->vma, ww, 0, 0, PIN_GLOBAL | PIN_HIGH);
>  }
>
>  static int mock_context_pin(struct intel_context *ce, void *unused)
> --
> 2.33.0
>
Maarten Lankhorst Oct. 22, 2021, 11:03 a.m. UTC | #2 
Op 21-10-2021 om 17:57 schreef Matthew Auld:
> On Thu, 21 Oct 2021 at 11:36, Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com> wrote:
>> This allows us to finally get rid of all the assumptions that vma->obj is NULL.
>>
>> Changes since v1:
>> - Ensure the mock_ring vma is pinned to prevent a fault.
>> - Pin it high to avoid failure in evict_for_vma selftest.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/mock_engine.c | 38 ++++++++++++++++++++-------
>>  1 file changed, 28 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
>> index 8b89215afe46..bb99fc03f503 100644
>> --- a/drivers/gpu/drm/i915/gt/mock_engine.c
>> +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
>> @@ -35,9 +35,31 @@ static void mock_timeline_unpin(struct intel_timeline *tl)
>>         atomic_dec(&tl->pin_count);
>>  }
>>
>> +static struct i915_vma *create_ring_vma(struct i915_ggtt *ggtt, int size)
>> +{
>> +       struct i915_address_space *vm = &ggtt->vm;
>> +       struct drm_i915_private *i915 = vm->i915;
>> +       struct drm_i915_gem_object *obj;
>> +       struct i915_vma *vma;
>> +
>> +       obj = i915_gem_object_create_internal(i915, size);
>> +       if (IS_ERR(obj))
>> +               return ERR_CAST(obj);
> We didn't want to use the dummy object here also? I guess meh?
>
>> +
>> +       vma = i915_vma_instance(obj, vm, NULL);
>> +       if (IS_ERR(vma))
>> +               goto err;
>> +
>> +       return vma;
>> +
>> +err:
>> +       i915_gem_object_put(obj);
>> +       return vma;
>> +}
>> +
>>  static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
>>  {
>> -       const unsigned long sz = PAGE_SIZE / 2;
>> +       const unsigned long sz = PAGE_SIZE;
> Is that significant?
>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>

vma->node.size has to be page aligned, since we create an actual vma, it was required to bump the size.

~Maarten

>>         struct intel_ring *ring;
>>
>>         ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
>> @@ -50,15 +72,11 @@ static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
>>         ring->vaddr = (void *)(ring + 1);
>>         atomic_set(&ring->pin_count, 1);
>>
>> -       ring->vma = i915_vma_alloc();
>> -       if (!ring->vma) {
>> +       ring->vma = create_ring_vma(engine->gt->ggtt, PAGE_SIZE);
>> +       if (IS_ERR(ring->vma)) {
>>                 kfree(ring);
>>                 return NULL;
>>         }
>> -       i915_active_init(&ring->vma->active, NULL, NULL, 0);
>> -       __set_bit(I915_VMA_GGTT_BIT, __i915_vma_flags(ring->vma));
>> -       __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &ring->vma->node.flags);
>> -       ring->vma->node.size = sz;
>>
>>         intel_ring_update_space(ring);
>>
>> @@ -67,8 +85,7 @@ static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
>>
>>  static void mock_ring_free(struct intel_ring *ring)
>>  {
>> -       i915_active_fini(&ring->vma->active);
>> -       i915_vma_free(ring->vma);
>> +       i915_vma_put(ring->vma);
>>
>>         kfree(ring);
>>  }
>> @@ -125,6 +142,7 @@ static void mock_context_unpin(struct intel_context *ce)
>>
>>  static void mock_context_post_unpin(struct intel_context *ce)
>>  {
>> +       i915_vma_unpin(ce->ring->vma);
>>  }
>>
>>  static void mock_context_destroy(struct kref *ref)
>> @@ -169,7 +187,7 @@ static int mock_context_alloc(struct intel_context *ce)
>>  static int mock_context_pre_pin(struct intel_context *ce,
>>                                 struct i915_gem_ww_ctx *ww, void **unused)
>>  {
>> -       return 0;
>> +       return i915_vma_pin_ww(ce->ring->vma, ww, 0, 0, PIN_GLOBAL | PIN_HIGH);
>>  }
>>
>>  static int mock_context_pin(struct intel_context *ce, void *unused)
>> --
>> 2.33.0
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index 8b89215afe46..bb99fc03f503 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -35,9 +35,31 @@  static void mock_timeline_unpin(struct intel_timeline *tl)
 	atomic_dec(&tl->pin_count);
 }
 
+static struct i915_vma *create_ring_vma(struct i915_ggtt *ggtt, int size)
+{
+	struct i915_address_space *vm = &ggtt->vm;
+	struct drm_i915_private *i915 = vm->i915;
+	struct drm_i915_gem_object *obj;
+	struct i915_vma *vma;
+
+	obj = i915_gem_object_create_internal(i915, size);
+	if (IS_ERR(obj))
+		return ERR_CAST(obj);
+
+	vma = i915_vma_instance(obj, vm, NULL);
+	if (IS_ERR(vma))
+		goto err;
+
+	return vma;
+
+err:
+	i915_gem_object_put(obj);
+	return vma;
+}
+
 static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
 {
-	const unsigned long sz = PAGE_SIZE / 2;
+	const unsigned long sz = PAGE_SIZE;
 	struct intel_ring *ring;
 
 	ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
@@ -50,15 +72,11 @@  static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
 	ring->vaddr = (void *)(ring + 1);
 	atomic_set(&ring->pin_count, 1);
 
-	ring->vma = i915_vma_alloc();
-	if (!ring->vma) {
+	ring->vma = create_ring_vma(engine->gt->ggtt, PAGE_SIZE);
+	if (IS_ERR(ring->vma)) {
 		kfree(ring);
 		return NULL;
 	}
-	i915_active_init(&ring->vma->active, NULL, NULL, 0);
-	__set_bit(I915_VMA_GGTT_BIT, __i915_vma_flags(ring->vma));
-	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &ring->vma->node.flags);
-	ring->vma->node.size = sz;
 
 	intel_ring_update_space(ring);
 
@@ -67,8 +85,7 @@  static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
 
 static void mock_ring_free(struct intel_ring *ring)
 {
-	i915_active_fini(&ring->vma->active);
-	i915_vma_free(ring->vma);
+	i915_vma_put(ring->vma);
 
 	kfree(ring);
 }
@@ -125,6 +142,7 @@  static void mock_context_unpin(struct intel_context *ce)
 
 static void mock_context_post_unpin(struct intel_context *ce)
 {
+	i915_vma_unpin(ce->ring->vma);
 }
 
 static void mock_context_destroy(struct kref *ref)
@@ -169,7 +187,7 @@  static int mock_context_alloc(struct intel_context *ce)
 static int mock_context_pre_pin(struct intel_context *ce,
 				struct i915_gem_ww_ctx *ww, void **unused)
 {
-	return 0;
+	return i915_vma_pin_ww(ce->ring->vma, ww, 0, 0, PIN_GLOBAL | PIN_HIGH);
 }
 
 static int mock_context_pin(struct intel_context *ce, void *unused)