[i-g-t,2/4] i915/gem_ctx_engine: Exercise for_each_context_engine() with custom engine[]
diff mbox series

Message ID 20200214194016.4054376-2-chris@chris-wilson.co.uk
State New
Headers show
Series
  • [i-g-t,1/4] i915/gem_ctx_engines: Exercise 0 engines[]
Related show

Commit Message

Chris Wilson Feb. 14, 2020, 7:40 p.m. UTC
Set up a custom engine map with no engines, and check that the
for_each_context_engine() correctly iterates over nothing.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_ctx_engines.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Antonio Argenziano Feb. 14, 2020, 9:49 p.m. UTC | #1
On 14/02/20 11:40, Chris Wilson wrote:
> Set up a custom engine map with no engines, and check that the
> for_each_context_engine() correctly iterates over nothing.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   tests/i915/gem_ctx_engines.c | 28 ++++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
> 
> diff --git a/tests/i915/gem_ctx_engines.c b/tests/i915/gem_ctx_engines.c
> index 063140e0f..6a2edd1e0 100644
> --- a/tests/i915/gem_ctx_engines.c
> +++ b/tests/i915/gem_ctx_engines.c
> @@ -549,6 +549,31 @@ static void independent(int i915)
>   	gem_context_destroy(i915, param.ctx_id);
>   }
>   
> +static void libapi(int i915)
> +{
> +	struct i915_context_param_engines engines = {};

Is there a case for invalid engines as well?

Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>

> +	struct drm_i915_gem_context_param p = {
> +		.ctx_id = gem_context_create(i915),
> +		.param = I915_CONTEXT_PARAM_ENGINES,
> +		.value = to_user_pointer(&engines),
> +		.size = sizeof(engines),
> +	};
> +	const struct intel_execution_engine2 *e;
> +	unsigned int count = 0;
> +
> +	gem_context_set_param(i915, &p);
> +
> +	for_each_context_engine(i915, p.ctx_id, e)
> +		count++;
> +	igt_assert_eq(count, 0);
> +
> +	____for_each_physical_engine(i915, p.ctx_id, e)
> +		count++;
> +	igt_assert_eq(count, 0);
> +
> +	gem_context_destroy(i915, p.ctx_id);
> +}
> +
>   igt_main
>   {
>   	int i915 = -1;
> @@ -584,6 +609,9 @@ igt_main
>   	igt_subtest("independent")
>   		independent(i915);
>   
> +	igt_subtest("libapi")
> +		libapi(i915);
> +
>   	igt_fixture
>   		igt_stop_hang_detector();
>   }
>
Chris Wilson Feb. 14, 2020, 9:57 p.m. UTC | #2
Quoting Antonio Argenziano (2020-02-14 21:49:16)
> 
> 
> On 14/02/20 11:40, Chris Wilson wrote:
> > Set up a custom engine map with no engines, and check that the
> > for_each_context_engine() correctly iterates over nothing.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   tests/i915/gem_ctx_engines.c | 28 ++++++++++++++++++++++++++++
> >   1 file changed, 28 insertions(+)
> > 
> > diff --git a/tests/i915/gem_ctx_engines.c b/tests/i915/gem_ctx_engines.c
> > index 063140e0f..6a2edd1e0 100644
> > --- a/tests/i915/gem_ctx_engines.c
> > +++ b/tests/i915/gem_ctx_engines.c
> > @@ -549,6 +549,31 @@ static void independent(int i915)
> >       gem_context_destroy(i915, param.ctx_id);
> >   }
> >   
> > +static void libapi(int i915)
> > +{
> > +     struct i915_context_param_engines engines = {};
> 
> Is there a case for invalid engines as well?

One would have to think what the behaviour should be :)

for_each_context_engine() should iterate over every engine defined,
providing you with (e->class, e->instance, e->pretty_name).

Invalid will still have an entry, maybe with "unknown".

for_each_physical_engine would skip invalid entries that are rejected by
the kernel. It's really just
	for_each_context_engine()
		for_each_if(gem_has_ring())

and I think I should drop the second loop here and focus on testing that
for_each_context_engine() simply reports back the class:inst we put
into the context.
-Chris

Patch
diff mbox series

diff --git a/tests/i915/gem_ctx_engines.c b/tests/i915/gem_ctx_engines.c
index 063140e0f..6a2edd1e0 100644
--- a/tests/i915/gem_ctx_engines.c
+++ b/tests/i915/gem_ctx_engines.c
@@ -549,6 +549,31 @@  static void independent(int i915)
 	gem_context_destroy(i915, param.ctx_id);
 }
 
+static void libapi(int i915)
+{
+	struct i915_context_param_engines engines = {};
+	struct drm_i915_gem_context_param p = {
+		.ctx_id = gem_context_create(i915),
+		.param = I915_CONTEXT_PARAM_ENGINES,
+		.value = to_user_pointer(&engines),
+		.size = sizeof(engines),
+	};
+	const struct intel_execution_engine2 *e;
+	unsigned int count = 0;
+
+	gem_context_set_param(i915, &p);
+
+	for_each_context_engine(i915, p.ctx_id, e)
+		count++;
+	igt_assert_eq(count, 0);
+
+	____for_each_physical_engine(i915, p.ctx_id, e)
+		count++;
+	igt_assert_eq(count, 0);
+
+	gem_context_destroy(i915, p.ctx_id);
+}
+
 igt_main
 {
 	int i915 = -1;
@@ -584,6 +609,9 @@  igt_main
 	igt_subtest("independent")
 		independent(i915);
 
+	igt_subtest("libapi")
+		libapi(i915);
+
 	igt_fixture
 		igt_stop_hang_detector();
 }