[Mesa-dev] [PATCH v2 02/13] intel/decoder: Engine parameter for instructions

[Toni Lönnberg]

On Tue, Nov 06, 2018 at 12:07:58PM +0000, Lionel Landwerlin wrote:
> On 31/10/2018 13:12, Toni Lönnberg 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
> > ---
> >   src/intel/common/gen_decoder.c | 11 +++++++++++
> >   src/intel/common/gen_decoder.h |  5 +++++
> >   2 files changed, 16 insertions(+)
> > 
> > diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
> > index 8148b2f1489..43e5b6e3561 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 = 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,14 @@ 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) {
> > +         if (strcmp(atts[i + 1], "render") == 0) {
> > +            group->engine = I915_ENGINE_CLASS_TO_MASK(I915_ENGINE_CLASS_RENDER);
> > +         } else if (strcmp(atts[i + 1], "video") == 0) {
> > +            group->engine = I915_ENGINE_CLASS_TO_MASK(I915_ENGINE_CLASS_VIDEO);
> > +         } else if (strcmp(atts[i + 1], "blitter") == 0) {
> > +            group->engine = I915_ENGINE_CLASS_TO_MASK(I915_ENGINE_CLASS_COPY);
> > +         }
> 
> 
> I think one feature we might want at some point is engine="render|video".
> 
> Maybe also printout a warning if the engine name isn't recognized.

Will add a printout to v3.

> >         }
> >      }
> > diff --git a/src/intel/common/gen_decoder.h b/src/intel/common/gen_decoder.h
> > index 4beed22d729..e9efa6d1451 100644
> > --- a/src/intel/common/gen_decoder.h
> > +++ b/src/intel/common/gen_decoder.h
> > @@ -31,6 +31,8 @@
> >   #include "dev/gen_device_info.h"
> >   #include "util/hash_table.h"
> > +#include "drm-uapi/i915_drm.h"
> > +
> >   #ifdef __cplusplus
> >   extern "C" {
> >   #endif
> > @@ -40,6 +42,8 @@ struct gen_group;
> >   struct gen_field;
> >   union gen_field_value;
> > +#define I915_ENGINE_CLASS_TO_MASK(x) (1 << (x))
> > +
> 
> 
> I would include util/bitset.h and
> 
> 
> #define I915_ENGINE_CLASS_TO_MASK(x) BITSET_BIT(x)

Didn't remember that macro, will update to v3.

> >   static inline uint32_t gen_make_gen(uint32_t major, uint32_t minor)
> >   {
> >      return (major << 8) | minor;
> > @@ -102,6 +106,7 @@ struct gen_group {
> >      struct gen_field *dword_length_field; /* <instruction> specific */
> >      uint32_t dw_length;
> > +   uint32_t engine; /* <instruction> specific */
> 
> 
> Maybe call this engine_mask so that there is no confusion.

In v3.

> >      uint32_t bias; /* <instruction> specific */
> >      uint32_t group_offset, group_count;
> >      uint32_t group_size;
> 
>