[Mesa-dev] [PATCH v3] intel/decoder: Engine parameter for instructions

[Toni Lönnberg]

On Thu, Nov 08, 2018 at 12:57:03PM +0200, andrey simiklit wrote:
> Hello,
> 
> Please find my comment below:
> 
> On Wed, Nov 7, 2018 at 4:49 PM Toni Lönnberg <toni.lonnberg at intel.com>
> wrote:
> 
> > Preliminary work for adding handling of different pipes to gen_decoder.
> > Each
> > instruction needs to have a definition describing which engine it is meant
> > for.
> > If left undefined, by default, the instruction is defined for all engines.
> >
> > v2: Changed to use the engine class definitions from UAPI
> >
> > v3: Changed I915_ENGINE_CLASS_TO_MASK to use BITSET_BIT, change engine to
> > engine_mask, added check for incorrect engine and added the possibility to
> > define an instruction to multiple engines using the "|" as a delimiter in
> > the
> > engine attribute.
> > ---
> >  src/intel/common/gen_decoder.c | 23 +++++++++++++++++++++++
> >  src/intel/common/gen_decoder.h |  6 ++++++
> >  2 files changed, 29 insertions(+)
> >
> > diff --git a/src/intel/common/gen_decoder.c
> > b/src/intel/common/gen_decoder.c
> > index 8148b2f1489..79404660f56 100644
> > --- a/src/intel/common/gen_decoder.c
> > +++ b/src/intel/common/gen_decoder.c
> > @@ -165,6 +165,9 @@ create_group(struct parser_context *ctx,
> >     group->fixed_length = fixed_length;
> >     group->dword_length_field = NULL;
> >     group->dw_length = 0;
> > +   group->engine_mask =
> > I915_ENGINE_CLASS_TO_MASK(I915_ENGINE_CLASS_RENDER) |
> > +
> > I915_ENGINE_CLASS_TO_MASK(I915_ENGINE_CLASS_VIDEO) |
> > +                        I915_ENGINE_CLASS_TO_MASK(I915_ENGINE_CLASS_COPY);
> >     group->bias = 1;
> >
> >     for (int i = 0; atts[i]; i += 2) {
> > @@ -173,6 +176,26 @@ create_group(struct parser_context *ctx,
> >           group->dw_length = strtoul(atts[i + 1], &p, 0);
> >        } else if (strcmp(atts[i], "bias") == 0) {
> >           group->bias = strtoul(atts[i + 1], &p, 0);
> > +      } else if (strcmp(atts[i], "engine") == 0) {
> > +         void *mem_ctx = ralloc_context(NULL);
> > +         char *tmp = ralloc_strdup(mem_ctx, atts[i + 1]);
> > +         char *save_ptr;
> > +         char *tok = strtok_r(tmp, "|", &save_ptr);
> > +
> > +         group->engine_mask = 0;
> > +         while (tok != NULL) {
> > +            if (strcmp(tok, "render") == 0) {
> > +               group->engine_mask |=
> > I915_ENGINE_CLASS_TO_MASK(I915_ENGINE_CLASS_RENDER);
> > +            } else if (strcmp(tok, "video") == 0) {
> > +               group->engine_mask |=
> > I915_ENGINE_CLASS_TO_MASK(I915_ENGINE_CLASS_VIDEO);
> > +            } else if (strcmp(tok, "blitter") == 0) {
> > +               group->engine_mask |=
> > I915_ENGINE_CLASS_TO_MASK(I915_ENGINE_CLASS_COPY);
> > +            } else {
> > +               fprintf(stderr, "unknown engine class defined for
> > instruction \"%s\": %s\n", name, atts[i + 1]);
> > +            }
> > +
> > +            tok = strtok_r(NULL, "|", &save_ptr);
> > +         }
> >
> 
> Seems like 'ralloc_free(mem_ctx)' is missing here :-)

Yes it is :) Thanks for pointing that out.

> Regards,
> - Andrii
> 
> 
> >        }
> >     }
> >
> > diff --git a/src/intel/common/gen_decoder.h
> > b/src/intel/common/gen_decoder.h
> > index 4beed22d729..4311f58b4eb 100644
> > --- a/src/intel/common/gen_decoder.h
> > +++ b/src/intel/common/gen_decoder.h
> > @@ -30,6 +30,9 @@
> >
> >  #include "dev/gen_device_info.h"
> >  #include "util/hash_table.h"
> > +#include "util/bitset.h"
> > +
> > +#include "drm-uapi/i915_drm.h"
> >
> >  #ifdef __cplusplus
> >  extern "C" {
> > @@ -40,6 +43,8 @@ struct gen_group;
> >  struct gen_field;
> >  union gen_field_value;
> >
> > +#define I915_ENGINE_CLASS_TO_MASK(x) BITSET_BIT(x)
> > +
> >  static inline uint32_t gen_make_gen(uint32_t major, uint32_t minor)
> >  {
> >     return (major << 8) | minor;
> > @@ -102,6 +107,7 @@ struct gen_group {
> >     struct gen_field *dword_length_field; /* <instruction> specific */
> >
> >     uint32_t dw_length;
> > +   uint32_t engine_mask; /* <instruction> specific */
> >     uint32_t bias; /* <instruction> specific */
> >     uint32_t group_offset, group_count;
> >     uint32_t group_size;
> > --
> > 2.17.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >