Re: [PATCH 10/38] drm/i915/selftests: Allocate mock ring/timeline per context

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

On 18/01/2019 14:00, Chris Wilson wrote:
> To correctly simulate preemption between contexts, we need independent
> timelines along each context. Make it so.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/selftests/mock_engine.c | 90 ++++++++++----------
>   1 file changed, 47 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c
> index 9fe5b2c8f8d4..8b8d51af7d6a 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_engine.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c
> @@ -30,6 +30,36 @@ struct mock_ring {
>   	struct i915_timeline timeline;
>   };
>   
> +static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
> +{
> +	const unsigned long sz = PAGE_SIZE / 2;
> +	struct mock_ring *ring;
> +
> +	ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
> +	if (!ring)
> +		return NULL;
> +
> +	i915_timeline_init(engine->i915, &ring->timeline, engine->name);
> +
> +	ring->base.size = sz;
> +	ring->base.effective_size = sz;
> +	ring->base.vaddr = (void *)(ring + 1);
> +	ring->base.timeline = &ring->timeline;
> +
> +	INIT_LIST_HEAD(&ring->base.request_list);
> +	intel_ring_update_space(&ring->base);
> +
> +	return &ring->base;
> +}
> +
> +static void mock_ring_free(struct intel_ring *base)
> +{
> +	struct mock_ring *ring = container_of(base, typeof(*ring), base);
> +
> +	i915_timeline_fini(&ring->timeline);
> +	kfree(ring);
> +}
> +
>   static struct mock_request *first_request(struct mock_engine *engine)
>   {
>   	return list_first_entry_or_null(&engine->hw_queue,
> @@ -80,6 +110,9 @@ static void mock_context_unpin(struct intel_context *ce)
>   static void mock_context_destroy(struct intel_context *ce)
>   {
>   	GEM_BUG_ON(ce->pin_count);
> +
> +	if (ce->ring)
> +		mock_ring_free(ce->ring);
>   }
>   
>   static const struct intel_context_ops mock_context_ops = {
> @@ -93,13 +126,22 @@ mock_context_pin(struct intel_engine_cs *engine,
>   {
>   	struct intel_context *ce = to_intel_context(ctx, engine);
>   
> -	if (!ce->pin_count++) {
> -		i915_gem_context_get(ctx);
> -		ce->ring = engine->buffer;
> -		ce->ops = &mock_context_ops;
> +	if (ce->pin_count++)
> +		return ce;
> +
> +	if (!ce->ring) {
> +		ce->ring = mock_ring(engine);
> +		if (!ce->ring)
> +			goto err;
>   	}
>   
> +	ce->ops = &mock_context_ops;
> +	i915_gem_context_get(ctx);
>   	return ce;
> +
> +err:
> +	ce->pin_count = 0;
> +	return ERR_PTR(-ENOMEM);
>   }
>   
>   static int mock_request_alloc(struct i915_request *request)
> @@ -143,36 +185,6 @@ static void mock_submit_request(struct i915_request *request)
>   	spin_unlock_irq(&engine->hw_lock);
>   }
>   
> -static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
> -{
> -	const unsigned long sz = PAGE_SIZE / 2;
> -	struct mock_ring *ring;
> -
> -	ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
> -	if (!ring)
> -		return NULL;
> -
> -	i915_timeline_init(engine->i915, &ring->timeline, engine->name);
> -
> -	ring->base.size = sz;
> -	ring->base.effective_size = sz;
> -	ring->base.vaddr = (void *)(ring + 1);
> -	ring->base.timeline = &ring->timeline;
> -
> -	INIT_LIST_HEAD(&ring->base.request_list);
> -	intel_ring_update_space(&ring->base);
> -
> -	return &ring->base;
> -}
> -
> -static void mock_ring_free(struct intel_ring *base)
> -{
> -	struct mock_ring *ring = container_of(base, typeof(*ring), base);
> -
> -	i915_timeline_fini(&ring->timeline);
> -	kfree(ring);
> -}
> -
>   struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
>   				    const char *name,
>   				    int id)
> @@ -207,17 +219,11 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
>   	timer_setup(&engine->hw_delay, hw_delay_complete, 0);
>   	INIT_LIST_HEAD(&engine->hw_queue);
>   
> -	engine->base.buffer = mock_ring(&engine->base);
> -	if (!engine->base.buffer)
> -		goto err_breadcrumbs;
> -
>   	if (IS_ERR(intel_context_pin(i915->kernel_context, &engine->base)))
> -		goto err_ring;
> +		goto err_breadcrumbs;
>   
>   	return &engine->base;
>   
> -err_ring:
> -	mock_ring_free(engine->base.buffer);
>   err_breadcrumbs:
>   	intel_engine_fini_breadcrumbs(&engine->base);
>   	i915_timeline_fini(&engine->base.timeline);
> @@ -260,8 +266,6 @@ void mock_engine_free(struct intel_engine_cs *engine)
>   
>   	__intel_context_unpin(engine->i915->kernel_context, engine);
>   
> -	mock_ring_free(engine->buffer);
> -
>   	intel_engine_fini_breadcrumbs(engine);
>   	i915_timeline_fini(&engine->timeline);
>   
> 

I was sure I reviewed this yesterday or so, but even my sent mail folder 
disagrees.. strange.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx