* [PATCH] drm/i915: Fix up the inverse mapping for default ctx->engines[]
@ 2019-08-08 7:19 Chris Wilson
2019-08-08 7:21 ` Chris Wilson
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2019-08-08 7:19 UTC (permalink / raw)
To: intel-gfx
The order in which we store the engines inside default_engines() for the
legacy ctx->engines[] has to match the legacy I915_EXEC_RING selector
mapping in execbuf::user_map. If we present VCS2 as being the second
instance of the video engine, legacy userspace calls that I915_EXEC_BSD2
and so we need to insert it into the second video slot.
Fixes: 2edda80db3d0 ("drm/i915: Rename engines to match their user interface")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 ++-
drivers/gpu/drm/i915/gt/intel_engine_user.c | 49 +++++++++++++++++++++
drivers/gpu/drm/i915/gt/intel_gt_types.h | 1 +
3 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 1c5bc21a80ff..fae8ca72e240 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -279,6 +279,7 @@ static void free_engines_rcu(struct rcu_head *rcu)
static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
{
+ const struct intel_gt *gt = &ctx->i915->gt;
struct intel_engine_cs *engine;
struct i915_gem_engines *e;
enum intel_engine_id id;
@@ -288,7 +289,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
return ERR_PTR(-ENOMEM);
init_rcu_head(&e->rcu);
- for_each_engine(engine, ctx->i915, id) {
+ for_each_engine(engine, gt, id) {
struct intel_context *ce;
ce = intel_context_create(ctx, engine);
@@ -298,8 +299,8 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
}
e->engines[id] = ce;
+ e->num_engines = id + 1;
}
- e->num_engines = id;
return e;
}
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c
index c41ae05988a5..c72fcd177598 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -142,8 +142,54 @@ const char *intel_engine_class_repr(u8 class)
return uabi_names[class];
}
+struct legacy_ring {
+ struct intel_gt *gt;
+ u8 class;
+ u8 instance;
+};
+
+static int legacy_ring_id(const struct legacy_ring *ring)
+{
+ static const struct {
+ int base, max;
+ } map[] = {
+ [RENDER_CLASS] = { RCS0, 1 },
+ [COPY_ENGINE_CLASS] = { BCS0, 1 },
+ [VIDEO_DECODE_CLASS] = { VCS0, I915_MAX_VCS },
+ [VIDEO_ENHANCEMENT_CLASS] = { VECS0, I915_MAX_VECS },
+ };
+
+ if (GEM_DEBUG_WARN_ON(ring->class >= ARRAY_SIZE(map)))
+ return -1;
+
+ if (GEM_DEBUG_WARN_ON(ring->instance >= map[ring->class].max))
+ return -1;
+
+ return map[ring->class].base + ring->instance;
+}
+
+static void add_legacy_ring(struct legacy_ring *ring,
+ struct intel_engine_cs *engine)
+{
+ int id;
+
+ if (engine->gt != ring->gt || engine->class != ring->class) {
+ ring->gt = engine->gt;
+ ring->class = engine->class;
+ ring->instance = 0;
+ }
+
+ id = legacy_ring_id(ring);
+ if (unlikely(id == -1))
+ return;
+
+ ring->gt->engine[id] = engine;
+ ring->instance++;
+}
+
void intel_engines_driver_register(struct drm_i915_private *i915)
{
+ struct legacy_ring ring = {};
u8 uabi_instances[4] = {};
struct list_head *it, *next;
struct rb_node **p, *prev;
@@ -179,6 +225,9 @@ void intel_engines_driver_register(struct drm_i915_private *i915)
engine->uabi_class,
engine->uabi_instance) != engine);
+ /* Fix up the mapping to match default execbuf::user_map[] */
+ add_legacy_ring(&ring, engine);
+
prev = &engine->uabi_node;
p = &prev->rb_right;
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index 5fd11e361d03..789102f4f46b 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -78,6 +78,7 @@ struct intel_gt {
u32 pm_guc_events;
+ struct intel_engine_cs *engine[I915_NUM_ENGINES];
struct intel_engine_cs *engine_class[MAX_ENGINE_CLASS + 1]
[MAX_ENGINE_INSTANCE + 1];
};
--
2.23.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] drm/i915: Fix up the inverse mapping for default ctx->engines[]
2019-08-08 7:19 [PATCH] drm/i915: Fix up the inverse mapping for default ctx->engines[] Chris Wilson
@ 2019-08-08 7:21 ` Chris Wilson
2019-08-08 8:28 ` Mika Kuoppala
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2019-08-08 7:21 UTC (permalink / raw)
To: intel-gfx
The order in which we store the engines inside default_engines() for the
legacy ctx->engines[] has to match the legacy I915_EXEC_RING selector
mapping in execbuf::user_map. If we present VCS2 as being the second
instance of the video engine, legacy userspace calls that I915_EXEC_BSD2
and so we need to insert it into the second video slot.
Fixes: 2edda80db3d0 ("drm/i915: Rename engines to match their user interface")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
Contract the static mapping table.
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 ++-
drivers/gpu/drm/i915/gt/intel_engine_user.c | 49 +++++++++++++++++++++
drivers/gpu/drm/i915/gt/intel_gt_types.h | 1 +
3 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 1c5bc21a80ff..fae8ca72e240 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -279,6 +279,7 @@ static void free_engines_rcu(struct rcu_head *rcu)
static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
{
+ const struct intel_gt *gt = &ctx->i915->gt;
struct intel_engine_cs *engine;
struct i915_gem_engines *e;
enum intel_engine_id id;
@@ -288,7 +289,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
return ERR_PTR(-ENOMEM);
init_rcu_head(&e->rcu);
- for_each_engine(engine, ctx->i915, id) {
+ for_each_engine(engine, gt, id) {
struct intel_context *ce;
ce = intel_context_create(ctx, engine);
@@ -298,8 +299,8 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
}
e->engines[id] = ce;
+ e->num_engines = id + 1;
}
- e->num_engines = id;
return e;
}
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c
index c41ae05988a5..83dbc54c3d2a 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -142,8 +142,54 @@ const char *intel_engine_class_repr(u8 class)
return uabi_names[class];
}
+struct legacy_ring {
+ struct intel_gt *gt;
+ u8 class;
+ u8 instance;
+};
+
+static int legacy_ring_id(const struct legacy_ring *ring)
+{
+ static const struct {
+ u8 base, max;
+ } map[] = {
+ [RENDER_CLASS] = { RCS0, 1 },
+ [COPY_ENGINE_CLASS] = { BCS0, 1 },
+ [VIDEO_DECODE_CLASS] = { VCS0, I915_MAX_VCS },
+ [VIDEO_ENHANCEMENT_CLASS] = { VECS0, I915_MAX_VECS },
+ };
+
+ if (GEM_DEBUG_WARN_ON(ring->class >= ARRAY_SIZE(map)))
+ return -1;
+
+ if (GEM_DEBUG_WARN_ON(ring->instance >= map[ring->class].max))
+ return -1;
+
+ return map[ring->class].base + ring->instance;
+}
+
+static void add_legacy_ring(struct legacy_ring *ring,
+ struct intel_engine_cs *engine)
+{
+ int id;
+
+ if (engine->gt != ring->gt || engine->class != ring->class) {
+ ring->gt = engine->gt;
+ ring->class = engine->class;
+ ring->instance = 0;
+ }
+
+ id = legacy_ring_id(ring);
+ if (unlikely(id == -1))
+ return;
+
+ ring->gt->engine[id] = engine;
+ ring->instance++;
+}
+
void intel_engines_driver_register(struct drm_i915_private *i915)
{
+ struct legacy_ring ring = {};
u8 uabi_instances[4] = {};
struct list_head *it, *next;
struct rb_node **p, *prev;
@@ -179,6 +225,9 @@ void intel_engines_driver_register(struct drm_i915_private *i915)
engine->uabi_class,
engine->uabi_instance) != engine);
+ /* Fix up the mapping to match default execbuf::user_map[] */
+ add_legacy_ring(&ring, engine);
+
prev = &engine->uabi_node;
p = &prev->rb_right;
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index 5fd11e361d03..789102f4f46b 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -78,6 +78,7 @@ struct intel_gt {
u32 pm_guc_events;
+ struct intel_engine_cs *engine[I915_NUM_ENGINES];
struct intel_engine_cs *engine_class[MAX_ENGINE_CLASS + 1]
[MAX_ENGINE_INSTANCE + 1];
};
--
2.23.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Fix up the inverse mapping for default ctx->engines[]
2019-08-08 7:21 ` Chris Wilson
@ 2019-08-08 8:28 ` Mika Kuoppala
2019-08-08 8:41 ` Chris Wilson
0 siblings, 1 reply; 4+ messages in thread
From: Mika Kuoppala @ 2019-08-08 8:28 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> The order in which we store the engines inside default_engines() for the
> legacy ctx->engines[] has to match the legacy I915_EXEC_RING selector
> mapping in execbuf::user_map. If we present VCS2 as being the second
> instance of the video engine, legacy userspace calls that I915_EXEC_BSD2
> and so we need to insert it into the second video slot.
>
> Fixes: 2edda80db3d0 ("drm/i915: Rename engines to match their user interface")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> Contract the static mapping table.
> ---
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 ++-
> drivers/gpu/drm/i915/gt/intel_engine_user.c | 49 +++++++++++++++++++++
> drivers/gpu/drm/i915/gt/intel_gt_types.h | 1 +
> 3 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 1c5bc21a80ff..fae8ca72e240 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -279,6 +279,7 @@ static void free_engines_rcu(struct rcu_head *rcu)
>
> static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
> {
> + const struct intel_gt *gt = &ctx->i915->gt;
> struct intel_engine_cs *engine;
> struct i915_gem_engines *e;
> enum intel_engine_id id;
> @@ -288,7 +289,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
> return ERR_PTR(-ENOMEM);
>
> init_rcu_head(&e->rcu);
> - for_each_engine(engine, ctx->i915, id) {
> + for_each_engine(engine, gt, id) {
Don't know yet if I love it or hate it.
As I got bruised I started yearning for intel_legacy_engine_id
with a strong type.
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> struct intel_context *ce;
>
> ce = intel_context_create(ctx, engine);
> @@ -298,8 +299,8 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
> }
>
> e->engines[id] = ce;
> + e->num_engines = id + 1;
> }
> - e->num_engines = id;
>
> return e;
> }
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> index c41ae05988a5..83dbc54c3d2a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> @@ -142,8 +142,54 @@ const char *intel_engine_class_repr(u8 class)
> return uabi_names[class];
> }
>
> +struct legacy_ring {
> + struct intel_gt *gt;
> + u8 class;
> + u8 instance;
> +};
> +
> +static int legacy_ring_id(const struct legacy_ring *ring)
> +{
> + static const struct {
> + u8 base, max;
> + } map[] = {
> + [RENDER_CLASS] = { RCS0, 1 },
> + [COPY_ENGINE_CLASS] = { BCS0, 1 },
> + [VIDEO_DECODE_CLASS] = { VCS0, I915_MAX_VCS },
> + [VIDEO_ENHANCEMENT_CLASS] = { VECS0, I915_MAX_VECS },
> + };
> +
> + if (GEM_DEBUG_WARN_ON(ring->class >= ARRAY_SIZE(map)))
> + return -1;
> +
> + if (GEM_DEBUG_WARN_ON(ring->instance >= map[ring->class].max))
> + return -1;
> +
> + return map[ring->class].base + ring->instance;
> +}
> +
> +static void add_legacy_ring(struct legacy_ring *ring,
> + struct intel_engine_cs *engine)
> +{
> + int id;
> +
> + if (engine->gt != ring->gt || engine->class != ring->class) {
> + ring->gt = engine->gt;
> + ring->class = engine->class;
> + ring->instance = 0;
> + }
> +
> + id = legacy_ring_id(ring);
> + if (unlikely(id == -1))
> + return;
> +
> + ring->gt->engine[id] = engine;
> + ring->instance++;
> +}
> +
> void intel_engines_driver_register(struct drm_i915_private *i915)
> {
> + struct legacy_ring ring = {};
> u8 uabi_instances[4] = {};
> struct list_head *it, *next;
> struct rb_node **p, *prev;
> @@ -179,6 +225,9 @@ void intel_engines_driver_register(struct drm_i915_private *i915)
> engine->uabi_class,
> engine->uabi_instance) != engine);
>
> + /* Fix up the mapping to match default execbuf::user_map[] */
> + add_legacy_ring(&ring, engine);
> +
> prev = &engine->uabi_node;
> p = &prev->rb_right;
> }
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> index 5fd11e361d03..789102f4f46b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -78,6 +78,7 @@ struct intel_gt {
>
> u32 pm_guc_events;
>
> + struct intel_engine_cs *engine[I915_NUM_ENGINES];
> struct intel_engine_cs *engine_class[MAX_ENGINE_CLASS + 1]
> [MAX_ENGINE_INSTANCE + 1];
> };
> --
> 2.23.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Fix up the inverse mapping for default ctx->engines[]
2019-08-08 8:28 ` Mika Kuoppala
@ 2019-08-08 8:41 ` Chris Wilson
0 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2019-08-08 8:41 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2019-08-08 09:28:38)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > The order in which we store the engines inside default_engines() for the
> > legacy ctx->engines[] has to match the legacy I915_EXEC_RING selector
> > mapping in execbuf::user_map. If we present VCS2 as being the second
> > instance of the video engine, legacy userspace calls that I915_EXEC_BSD2
> > and so we need to insert it into the second video slot.
> >
> > Fixes: 2edda80db3d0 ("drm/i915: Rename engines to match their user interface")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> > Contract the static mapping table.
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 ++-
> > drivers/gpu/drm/i915/gt/intel_engine_user.c | 49 +++++++++++++++++++++
> > drivers/gpu/drm/i915/gt/intel_gt_types.h | 1 +
> > 3 files changed, 53 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > index 1c5bc21a80ff..fae8ca72e240 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > @@ -279,6 +279,7 @@ static void free_engines_rcu(struct rcu_head *rcu)
> >
> > static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
> > {
> > + const struct intel_gt *gt = &ctx->i915->gt;
> > struct intel_engine_cs *engine;
> > struct i915_gem_engines *e;
> > enum intel_engine_id id;
> > @@ -288,7 +289,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx)
> > return ERR_PTR(-ENOMEM);
> >
> > init_rcu_head(&e->rcu);
> > - for_each_engine(engine, ctx->i915, id) {
> > + for_each_engine(engine, gt, id) {
>
> Don't know yet if I love it or hate it.
Oh, i915->engine[] is being phased out. The plan wasn't to do this until
later!
> As I got bruised I started yearning for intel_legacy_engine_id
> with a strong type.
Yes, been bruised by the lack of legacy_id as well.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-08-08 8:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-08 7:19 [PATCH] drm/i915: Fix up the inverse mapping for default ctx->engines[] Chris Wilson
2019-08-08 7:21 ` Chris Wilson
2019-08-08 8:28 ` Mika Kuoppala
2019-08-08 8:41 ` Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.