[igt-dev] [PATCH i-g-t 13/89] lib/i915/gem_engine_topology: Add an iterator for intel_ctx_t (v2)
Jason Ekstrand
jason at jlekstrand.net
Wed Jun 9 14:37:03 UTC 2021
On Tue, May 18, 2021 at 3:12 AM Zbigniew Kempczyński
<zbigniew.kempczynski at intel.com> wrote:
>
> On Fri, Apr 23, 2021 at 04:47:37PM -0500, Jason Ekstrand wrote:
> > v2 (Jason Ekstrand):
> > - Add documentation
> >
> > Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> > Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > ---
> > lib/i915/gem_engine_topology.c | 69 ++++++++++++++++++++++++++++++++++
> > lib/i915/gem_engine_topology.h | 29 +++++++++++++-
> > 2 files changed, 96 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c
> > index 6bceb909..ac707cb3 100644
> > --- a/lib/i915/gem_engine_topology.c
> > +++ b/lib/i915/gem_engine_topology.c
> > @@ -39,6 +39,47 @@
> > *
> > * This helper library contains functions used for querying and dealing
> > * with engines in GEM contexts.
> > + *
> > + * Combined with intel_ctx_t, these helpers give a pretty standard pattern
> > + * for testing every engine in a device:
> > + * |[<!-- language="C" -->
> > + * const struct intel_execution_engine2 *e;
> > + * const intel_ctx_t *ctx = intel_ctx_create_all_physical(fd);
> > + *
> > + * igt_subtest_with_dynamic("basic") {
> > + * for_each_ctx_engine(fd, ctx, e) {
> > + * igt_dynamic_f("%s", e->name)
> > + * run_ctx_test(fd, ctx, e);
> > + * }
> > + * }
> > + * ]|
> > + * This pattern works regardless of whether or not the engines topology API
> > + * is available and regardless of whether or not your platform supports
> > + * contexts. If engines are unavailable, it falls back to a legacy context
> > + * and if contexts are unavailable, intel_ctx_create_all_physial() will
>
> ^^^^^^^ - typo
Fixed.
> > + * return a wrapper around ctx0.
> > + *
> > + * If, for some reason, you want to create a second identical context to
> > + * use with your engine iterator, duplicating the context is easy:
> > + * |[<!-- language="C" -->
> > + * const intel_ctx_t *ctx2 = intel_ctx_create(fd, &ctx->cfg);
> > + * ]|
> > + *
> > + * If you want each subtest to always create its own contexts, there are
> > + * also iterators which work only on a context config. As long as all
> > + * contexts are created from that config, or from one with an identical set
> > + * of engines, the iterator will be valid for those contexts.
> > + * |[<!-- language="C" -->
> > + * const struct intel_execution_engine2 *e;
> > + * intel_ctx_cfg_t cfg = intel_ctx_cfg_all_physical(fd);
> > + *
> > + * igt_subtest_with_dynamic("basic") {
> > + * for_each_ctx_cfg_engine(fd, &cfg, e) {
> > + * igt_dynamic_f("%s", e->name)
> > + * run_ctx_cfg_test(fd, &cfg, e);
> > + * }
> > + * }
> > + * ]|
> > */
> >
> > /*
> > @@ -256,6 +297,34 @@ struct intel_engine_data intel_engine_list_of_physical(int fd)
> > return intel_engine_list_for_static(fd);
> > }
> >
> > +/**
> > + * intel_engine_list_for_ctx_cfg:
> > + * @fd: open i915 drm file descriptor
> > + * @cfg: Context config
> > + *
> > + * Returns the list of all engines in the context config
> > + */
> > +struct intel_engine_data
> > +intel_engine_list_for_ctx_cfg(int fd, const intel_ctx_cfg_t *cfg)
> > +{
>
> I would add igt_assert(cfg) here especially we expect not-null
> cfg pointer here.
Done.
> > + if (fd >= 0 && cfg->num_engines) {
> > + struct intel_engine_data engine_data = { };
> > + int i;
> > +
> > + engine_data.nengines = cfg->num_engines;
> > + for (i = 0; i < cfg->num_engines; i++)
> > + init_engine(&engine_data.engines[i],
> > + cfg->engines[i].engine_class,
> > + cfg->engines[i].engine_instance,
> > + i);
>
> What's the reason of passing incrementing value here? It's confusing
> especially it is not correspond to legacy engine ids.
The final parameter to init_engine() is the "flags" parameter to use
with it. For userspace-specified engines, the flags parameter is an
engine index, hence i. For legacy contexts it's something like
I915_EXEC_RENDER or I915_EXEC_BLT.
--Jason
> --
> Zbigniew
>
> > +
> > + return engine_data;
> > + } else {
> > + /* This is a legacy context */
> > + return intel_engine_list_for_static(fd);
> > + }
> > +}
> > +
> > static int gem_topology_get_param(int fd,
> > struct drm_i915_gem_context_param *p)
> > {
> > diff --git a/lib/i915/gem_engine_topology.h b/lib/i915/gem_engine_topology.h
> > index bd93e410..92d9a479 100644
> > --- a/lib/i915/gem_engine_topology.h
> > +++ b/lib/i915/gem_engine_topology.h
> > @@ -26,8 +26,7 @@
> >
> > #include "igt_gt.h"
> > #include "i915_drm.h"
> > -
> > -#define GEM_MAX_ENGINES I915_EXEC_RING_MASK + 1
> > +#include "intel_ctx.h"
> >
> > int __gem_query_engines(int fd,
> > struct drm_i915_query_engine_info *query_engines,
> > @@ -51,6 +50,7 @@ struct intel_engine_data {
> >
> > bool gem_has_engine_topology(int fd);
> > struct intel_engine_data intel_engine_list_of_physical(int fd);
> > +struct intel_engine_data intel_engine_list_for_ctx_cfg(int fd, const intel_ctx_cfg_t *cfg);
> > struct intel_engine_data intel_init_engine_list(int fd, uint32_t ctx_id);
> >
> > /* iteration functions */
> > @@ -81,6 +81,31 @@ struct intel_execution_engine2 gem_eb_flags_to_engine(unsigned int flags);
> > #define __for_each_static_engine(e__) \
> > for ((e__) = intel_execution_engines2; (e__)->name[0]; (e__)++)
> >
> > +/**
> > + * for_each_ctx_cfg_engine
> > + * @fd__: open i915 drm file descriptor
> > + * @ctx_cfg__: Intel context config
> > + * @e__: struct intel_execution_engine2 iterator
> > + *
> > + * Iterates over each physical engine in the context config
> > + */
> > +#define for_each_ctx_cfg_engine(fd__, ctx_cfg__, e__) \
> > + for (struct intel_engine_data i__##e__ = \
> > + intel_engine_list_for_ctx_cfg(fd__, ctx_cfg__); \
> > + ((e__) = intel_get_current_engine(&i__##e__)); \
> > + intel_next_engine(&i__##e__))
> > +
> > +/**
> > + * for_each_ctx_engine
> > + * @fd__: open i915 drm file descriptor
> > + * @ctx__: Intel context wrapper
> > + * @e__: struct intel_execution_engine2 iterator
> > + *
> > + * Iterates over each physical engine in the context
> > + */
> > +#define for_each_ctx_engine(fd__, ctx__, e__) \
> > + for_each_ctx_cfg_engine(fd__, &(ctx__)->cfg, e__)
> > +
> > #define for_each_context_engine(fd__, ctx__, e__) \
> > for (struct intel_engine_data i__ = intel_init_engine_list(fd__, ctx__); \
> > ((e__) = intel_get_current_engine(&i__)); \
> > --
> > 2.31.1
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
More information about the igt-dev
mailing list