Re: [Mesa-dev] [PATCH v2 00/13] Engine parameter for instructions

[Toni Lönnberg]

On Fri, Nov 02, 2018 at 09:47:11AM -0500, Jason Ekstrand wrote:
> On Fri, Nov 2, 2018 at 6:05 AM Toni Lönnberg 
> wrote:
> 
> > On Fri, Nov 02, 2018 at 12:09:54AM -0500, Jason Ekstrand wrote:
> > > On Thu, Nov 1, 2018 at 5:51 AM Toni Lönnberg 
> > > wrote:
> > >
> > > > On Wed, Oct 31, 2018 at 01:18:11PM -0500, Jason Ekstrand wrote:
> > > > > On Wed, Oct 31, 2018 at 11:10 AM Toni Lönnberg <
> > toni.lonnb...@intel.com>
> > > > > wrote:
> > > > >
> > > > > > When we debug media or 3d+media workloads, we'd like to be able to
> > see
> > > > > > what is
> > > > > > in the aub dumps for those workloads. At the moment the decoder
> > can't
> > > > > > distinguish instructions which share the same opcode between the
> > render
> > > > > > and
> > > > > > video pipe, and thus aubinator outputs garbage on media
> > instructions.
> > > > > >
> > > > > > I was reluctant to make these changes into Mesa in the first place
> > > > since
> > > > > > the
> > > > > > work is related to media, not 3d, but as aubinator is now located
> > here,
> > > > > > here we
> > > > > > are.
> > > > > >
> > > > >
> > > > > That's fine.  It's fine if these changres live in mesa.
> > > > >
> > > > >
> > > > > > As far as I can see, these are the options:
> > > > > >
> > > > > > 1. Put these in Mesa as aubinator now resides here instead of IGT.
> > > > > >a) Put all instruction definitions in the current genxml files,
> > be
> > > > it
> > > > > > with a
> > > > > >   tag or an attribute, both methods have their advantages and
> > > > > > disadvantages.
> > > > > >b) Separate genxml files into common, render and video (and
> > > > blitter?)
> > > > > >
> > > > > > 2. Fork aubinator tools and related genxml infra to a completely
> > > > separate
> > > > > >project.
> > > > > >
> > > > > > If I have missed an option, feel free to suggest.
> > > > > >
> > > > >
> > > > > I wasn't suggesting we fork the tools and the XML.  I was just
> > wondering
> > > > > whether we wanted to do separate sections or an attribute.  I think
> > it
> > > > > should land in mesa either way.
> > > >
> > > > I'm not really a fan of any of the methods to be honest as each of them
> > > > have
> > > > disadvantages.
> > > >
> > > > Using the attribute makes it easier to add instructions which should be
> > > > handled
> > > > by a set of engines and not others but requires each instruction to
> > have
> > > > one,
> > > > except the common ones that are always shared which can be made to
> > default
> > > > to
> > > > all engines like in my version.
> > > >
> > > > The tag makes it easier to group things nicely but adds a new level to
> > the
> > > > xml
> > > > and requires duplicate definitions of the instruction if more than one
> > > > engine
> > > > uses that instruction but some engine doesn't.
> > > >
> > >
> > > Yeah, I think I'm convinced.
> > >
> > >
> > > > Splitting the genxml files into different ones for each engine also
> > > > organizes
> > > > the instructions nicely but has the same problem as the tag and
> > requires
> > > > loading
> > > > and parsing of multiple xml files.
> > > >
> > >
> > > I also experimented with splitting up the XML files.  There are times
> > when
> > > this is convenient but I'm not sure it's actually all that useful.  If we
> > > were to split it, we'd probably want to split it across other logical
> > lines
> > > like "MI" commands vs. "3DSTATE" commands and maybe a separate file for
> > > registers.  At least for now, we may as well keep it all together.
> >
> > Splitting the files seems convenient but coherence is important because we
> > don't
> > want to end up with hundreds of xml files just for the sake of splitting
> > stuff.
> > So I'd keep them as they are unless at some point we find a convincing
> > reason to
> > split them apart.
> >
> > > > To make things more icky, there are the instructions, like MI_FLUSH_DW,
> > > > which
> > > > are shared between engines but might use some of the bits for different
> > > > things.
> > > >
> > >
> > > Yeah.  When you mentioned this, I briefly considered the idea of putting
> > > engine tags on individual fields in that case.  That may be getting a bit
> > > nuts; I certainly don't want to recreate the insanity that is the bxml.
> >
> > I haven't checked how many instructions there are that are shared but have
> > some
> > bits enabled for some engines and not for others, can't be that many. If
> > there
> > were a lot of those, it might make more sense adding an engine-attribute
> > to
> > fields but as of now, that's a bit overkill. And I'm with you here about
> > bspec
> > :)
> >
> > > > In the end, I just went with how Lionel saw it because it was as good
> > of
> > > > an
> > > > option as any, and pretty straight forward to implement, so.. mehhh. I
> > > > don't
> > > > know if you'd prefer one way or the other.
> > > >
> > >
> > > Works for me.  I just wanted to make sure we'd thought it through before
> > > adding it.  I know I certainly hadn't g

Re: [Mesa-dev] [PATCH v2 00/13] Engine parameter for instructions

[Jason Ekstrand]

On Fri, Nov 2, 2018 at 6:05 AM Toni Lönnberg 
wrote:

> On Fri, Nov 02, 2018 at 12:09:54AM -0500, Jason Ekstrand wrote:
> > On Thu, Nov 1, 2018 at 5:51 AM Toni Lönnberg 
> > wrote:
> >
> > > On Wed, Oct 31, 2018 at 01:18:11PM -0500, Jason Ekstrand wrote:
> > > > On Wed, Oct 31, 2018 at 11:10 AM Toni Lönnberg <
> toni.lonnb...@intel.com>
> > > > wrote:
> > > >
> > > > > When we debug media or 3d+media workloads, we'd like to be able to
> see
> > > > > what is
> > > > > in the aub dumps for those workloads. At the moment the decoder
> can't
> > > > > distinguish instructions which share the same opcode between the
> render
> > > > > and
> > > > > video pipe, and thus aubinator outputs garbage on media
> instructions.
> > > > >
> > > > > I was reluctant to make these changes into Mesa in the first place
> > > since
> > > > > the
> > > > > work is related to media, not 3d, but as aubinator is now located
> here,
> > > > > here we
> > > > > are.
> > > > >
> > > >
> > > > That's fine.  It's fine if these changres live in mesa.
> > > >
> > > >
> > > > > As far as I can see, these are the options:
> > > > >
> > > > > 1. Put these in Mesa as aubinator now resides here instead of IGT.
> > > > >a) Put all instruction definitions in the current genxml files,
> be
> > > it
> > > > > with a
> > > > >   tag or an attribute, both methods have their advantages and
> > > > > disadvantages.
> > > > >b) Separate genxml files into common, render and video (and
> > > blitter?)
> > > > >
> > > > > 2. Fork aubinator tools and related genxml infra to a completely
> > > separate
> > > > >project.
> > > > >
> > > > > If I have missed an option, feel free to suggest.
> > > > >
> > > >
> > > > I wasn't suggesting we fork the tools and the XML.  I was just
> wondering
> > > > whether we wanted to do separate sections or an attribute.  I think
> it
> > > > should land in mesa either way.
> > >
> > > I'm not really a fan of any of the methods to be honest as each of them
> > > have
> > > disadvantages.
> > >
> > > Using the attribute makes it easier to add instructions which should be
> > > handled
> > > by a set of engines and not others but requires each instruction to
> have
> > > one,
> > > except the common ones that are always shared which can be made to
> default
> > > to
> > > all engines like in my version.
> > >
> > > The tag makes it easier to group things nicely but adds a new level to
> the
> > > xml
> > > and requires duplicate definitions of the instruction if more than one
> > > engine
> > > uses that instruction but some engine doesn't.
> > >
> >
> > Yeah, I think I'm convinced.
> >
> >
> > > Splitting the genxml files into different ones for each engine also
> > > organizes
> > > the instructions nicely but has the same problem as the tag and
> requires
> > > loading
> > > and parsing of multiple xml files.
> > >
> >
> > I also experimented with splitting up the XML files.  There are times
> when
> > this is convenient but I'm not sure it's actually all that useful.  If we
> > were to split it, we'd probably want to split it across other logical
> lines
> > like "MI" commands vs. "3DSTATE" commands and maybe a separate file for
> > registers.  At least for now, we may as well keep it all together.
>
> Splitting the files seems convenient but coherence is important because we
> don't
> want to end up with hundreds of xml files just for the sake of splitting
> stuff.
> So I'd keep them as they are unless at some point we find a convincing
> reason to
> split them apart.
>
> > > To make things more icky, there are the instructions, like MI_FLUSH_DW,
> > > which
> > > are shared between engines but might use some of the bits for different
> > > things.
> > >
> >
> > Yeah.  When you mentioned this, I briefly considered the idea of putting
> > engine tags on individual fields in that case.  That may be getting a bit
> > nuts; I certainly don't want to recreate the insanity that is the bxml.
>
> I haven't checked how many instructions there are that are shared but have
> some
> bits enabled for some engines and not for others, can't be that many. If
> there
> were a lot of those, it might make more sense adding an engine-attribute
> to
> fields but as of now, that's a bit overkill. And I'm with you here about
> bspec
> :)
>
> > > In the end, I just went with how Lionel saw it because it was as good
> of
> > > an
> > > option as any, and pretty straight forward to implement, so.. mehhh. I
> > > don't
> > > know if you'd prefer one way or the other.
> > >
> >
> > Works for me.  I just wanted to make sure we'd thought it through before
> > adding it.  I know I certainly hadn't given it all that much thought in
> my
> > brief experiments.  I think the attributes is the best solution for now.
>
> So we're good to go with the reviews?
>

Yes, if I understand the question correctly.  That doesn't mean I have
reviewed it; I haven't really looked at the patches in any detail
whatsoever.  It does mean that I'm happy with

Re: [Mesa-dev] [PATCH v2 00/13] Engine parameter for instructions

[Toni Lönnberg]

On Fri, Nov 02, 2018 at 12:09:54AM -0500, Jason Ekstrand wrote:
> On Thu, Nov 1, 2018 at 5:51 AM Toni Lönnberg 
> wrote:
> 
> > On Wed, Oct 31, 2018 at 01:18:11PM -0500, Jason Ekstrand wrote:
> > > On Wed, Oct 31, 2018 at 11:10 AM Toni Lönnberg 
> > > wrote:
> > >
> > > > When we debug media or 3d+media workloads, we'd like to be able to see
> > > > what is
> > > > in the aub dumps for those workloads. At the moment the decoder can't
> > > > distinguish instructions which share the same opcode between the render
> > > > and
> > > > video pipe, and thus aubinator outputs garbage on media instructions.
> > > >
> > > > I was reluctant to make these changes into Mesa in the first place
> > since
> > > > the
> > > > work is related to media, not 3d, but as aubinator is now located here,
> > > > here we
> > > > are.
> > > >
> > >
> > > That's fine.  It's fine if these changres live in mesa.
> > >
> > >
> > > > As far as I can see, these are the options:
> > > >
> > > > 1. Put these in Mesa as aubinator now resides here instead of IGT.
> > > >a) Put all instruction definitions in the current genxml files, be
> > it
> > > > with a
> > > >   tag or an attribute, both methods have their advantages and
> > > > disadvantages.
> > > >b) Separate genxml files into common, render and video (and
> > blitter?)
> > > >
> > > > 2. Fork aubinator tools and related genxml infra to a completely
> > separate
> > > >project.
> > > >
> > > > If I have missed an option, feel free to suggest.
> > > >
> > >
> > > I wasn't suggesting we fork the tools and the XML.  I was just wondering
> > > whether we wanted to do separate sections or an attribute.  I think it
> > > should land in mesa either way.
> >
> > I'm not really a fan of any of the methods to be honest as each of them
> > have
> > disadvantages.
> >
> > Using the attribute makes it easier to add instructions which should be
> > handled
> > by a set of engines and not others but requires each instruction to have
> > one,
> > except the common ones that are always shared which can be made to default
> > to
> > all engines like in my version.
> >
> > The tag makes it easier to group things nicely but adds a new level to the
> > xml
> > and requires duplicate definitions of the instruction if more than one
> > engine
> > uses that instruction but some engine doesn't.
> >
> 
> Yeah, I think I'm convinced.
> 
> 
> > Splitting the genxml files into different ones for each engine also
> > organizes
> > the instructions nicely but has the same problem as the tag and requires
> > loading
> > and parsing of multiple xml files.
> >
> 
> I also experimented with splitting up the XML files.  There are times when
> this is convenient but I'm not sure it's actually all that useful.  If we
> were to split it, we'd probably want to split it across other logical lines
> like "MI" commands vs. "3DSTATE" commands and maybe a separate file for
> registers.  At least for now, we may as well keep it all together.

Splitting the files seems convenient but coherence is important because we 
don't 
want to end up with hundreds of xml files just for the sake of splitting stuff. 
So I'd keep them as they are unless at some point we find a convincing reason 
to 
split them apart.

> > To make things more icky, there are the instructions, like MI_FLUSH_DW,
> > which
> > are shared between engines but might use some of the bits for different
> > things.
> >
> 
> Yeah.  When you mentioned this, I briefly considered the idea of putting
> engine tags on individual fields in that case.  That may be getting a bit
> nuts; I certainly don't want to recreate the insanity that is the bxml.

I haven't checked how many instructions there are that are shared but have some 
bits enabled for some engines and not for others, can't be that many. If there 
were a lot of those, it might make more sense adding an engine-attribute to 
fields but as of now, that's a bit overkill. And I'm with you here about bspec 
:)

> > In the end, I just went with how Lionel saw it because it was as good of
> > an
> > option as any, and pretty straight forward to implement, so.. mehhh. I
> > don't
> > know if you'd prefer one way or the other.
> >
> 
> Works for me.  I just wanted to make sure we'd thought it through before
> adding it.  I know I certainly hadn't given it all that much thought in my
> brief experiments.  I think the attributes is the best solution for now.

So we're good to go with the reviews?

> --Jason
> 
> 
> 
> > >
> > > --Jason
> > >
> > >
> > > > On Wed, Oct 31, 2018 at 09:20:39AM -0500, Jason Ekstrand wrote:
> > > > > Toni,
> > > > >
> > > > > I'm a bit curious where you're going with this.  I started on a
> > similar
> > > > > project a couple of years ago:
> > > > >
> > > > >
> > https://gitlab.freedesktop.org/jekstrand/mesa/commits/wip/genxml-engines
> > > > >
> > > > > Mine took a different (not necessarily better) approach of
> > surrounding
> > > > the
> > > > > instructions in an  tag.  I'm not

Re: [Mesa-dev] [PATCH v2 00/13] Engine parameter for instructions

[Jason Ekstrand]

On Thu, Nov 1, 2018 at 5:51 AM Toni Lönnberg 
wrote:

> On Wed, Oct 31, 2018 at 01:18:11PM -0500, Jason Ekstrand wrote:
> > On Wed, Oct 31, 2018 at 11:10 AM Toni Lönnberg 
> > wrote:
> >
> > > When we debug media or 3d+media workloads, we'd like to be able to see
> > > what is
> > > in the aub dumps for those workloads. At the moment the decoder can't
> > > distinguish instructions which share the same opcode between the render
> > > and
> > > video pipe, and thus aubinator outputs garbage on media instructions.
> > >
> > > I was reluctant to make these changes into Mesa in the first place
> since
> > > the
> > > work is related to media, not 3d, but as aubinator is now located here,
> > > here we
> > > are.
> > >
> >
> > That's fine.  It's fine if these changres live in mesa.
> >
> >
> > > As far as I can see, these are the options:
> > >
> > > 1. Put these in Mesa as aubinator now resides here instead of IGT.
> > >a) Put all instruction definitions in the current genxml files, be
> it
> > > with a
> > >   tag or an attribute, both methods have their advantages and
> > > disadvantages.
> > >b) Separate genxml files into common, render and video (and
> blitter?)
> > >
> > > 2. Fork aubinator tools and related genxml infra to a completely
> separate
> > >project.
> > >
> > > If I have missed an option, feel free to suggest.
> > >
> >
> > I wasn't suggesting we fork the tools and the XML.  I was just wondering
> > whether we wanted to do separate sections or an attribute.  I think it
> > should land in mesa either way.
>
> I'm not really a fan of any of the methods to be honest as each of them
> have
> disadvantages.
>
> Using the attribute makes it easier to add instructions which should be
> handled
> by a set of engines and not others but requires each instruction to have
> one,
> except the common ones that are always shared which can be made to default
> to
> all engines like in my version.
>
> The tag makes it easier to group things nicely but adds a new level to the
> xml
> and requires duplicate definitions of the instruction if more than one
> engine
> uses that instruction but some engine doesn't.
>

Yeah, I think I'm convinced.


> Splitting the genxml files into different ones for each engine also
> organizes
> the instructions nicely but has the same problem as the tag and requires
> loading
> and parsing of multiple xml files.
>

I also experimented with splitting up the XML files.  There are times when
this is convenient but I'm not sure it's actually all that useful.  If we
were to split it, we'd probably want to split it across other logical lines
like "MI" commands vs. "3DSTATE" commands and maybe a separate file for
registers.  At least for now, we may as well keep it all together.


> To make things more icky, there are the instructions, like MI_FLUSH_DW,
> which
> are shared between engines but might use some of the bits for different
> things.
>

Yeah.  When you mentioned this, I briefly considered the idea of putting
engine tags on individual fields in that case.  That may be getting a bit
nuts; I certainly don't want to recreate the insanity that is the bxml.


> In the end, I just went with how Lionel saw it because it was as good of
> an
> option as any, and pretty straight forward to implement, so.. mehhh. I
> don't
> know if you'd prefer one way or the other.
>

Works for me.  I just wanted to make sure we'd thought it through before
adding it.  I know I certainly hadn't given it all that much thought in my
brief experiments.  I think the attributes is the best solution for now.

--Jason



> >
> > --Jason
> >
> >
> > > On Wed, Oct 31, 2018 at 09:20:39AM -0500, Jason Ekstrand wrote:
> > > > Toni,
> > > >
> > > > I'm a bit curious where you're going with this.  I started on a
> similar
> > > > project a couple of years ago:
> > > >
> > > >
> https://gitlab.freedesktop.org/jekstrand/mesa/commits/wip/genxml-engines
> > > >
> > > > Mine took a different (not necessarily better) approach of
> surrounding
> > > the
> > > > instructions in an  tag.  I'm not sure if that's any better
> or
> > > > worse than an attribute.
> > > >
> > > > At the time, I was planning to port over the blitter code to genxml
> and
> > > get
> > > > aubinator decoding blit streams.  I canned the project because there
> are
> > > > few enough differences in hardware generations for the blitter to be
> > > worth
> > > > the re-compilation and I had better things to do.  I've always
> thought it
> > > > would be good to support other engines for no other reason than to
> make
> > > > aubinator for blits.  It would also likely be useful to have if we
> wanted
> > > > to start doing media in mesa for some reason.  What's your
> motivation?  I
> > > > ask because I can't really have an opinion on the approach unless I
> know
> > > > where it's headed.
> > > >
> > > > --Jason
> > > >
> > > > On Wed, Oct 31, 2018 at 8:12 AM Toni Lönnberg <
> toni.lonnb...@intel.com>
> > > > wrote:
> > > >
> > > > > Thes

Re: [Mesa-dev] [PATCH v2 00/13] Engine parameter for instructions

[Toni Lönnberg]

On Wed, Oct 31, 2018 at 01:18:11PM -0500, Jason Ekstrand wrote:
> On Wed, Oct 31, 2018 at 11:10 AM Toni Lönnberg 
> wrote:
> 
> > When we debug media or 3d+media workloads, we'd like to be able to see
> > what is
> > in the aub dumps for those workloads. At the moment the decoder can't
> > distinguish instructions which share the same opcode between the render
> > and
> > video pipe, and thus aubinator outputs garbage on media instructions.
> >
> > I was reluctant to make these changes into Mesa in the first place since
> > the
> > work is related to media, not 3d, but as aubinator is now located here,
> > here we
> > are.
> >
> 
> That's fine.  It's fine if these changres live in mesa.
> 
> 
> > As far as I can see, these are the options:
> >
> > 1. Put these in Mesa as aubinator now resides here instead of IGT.
> >a) Put all instruction definitions in the current genxml files, be it
> > with a
> >   tag or an attribute, both methods have their advantages and
> > disadvantages.
> >b) Separate genxml files into common, render and video (and blitter?)
> >
> > 2. Fork aubinator tools and related genxml infra to a completely separate
> >project.
> >
> > If I have missed an option, feel free to suggest.
> >
> 
> I wasn't suggesting we fork the tools and the XML.  I was just wondering
> whether we wanted to do separate sections or an attribute.  I think it
> should land in mesa either way.

I'm not really a fan of any of the methods to be honest as each of them have 
disadvantages.

Using the attribute makes it easier to add instructions which should be handled 
by a set of engines and not others but requires each instruction to have one, 
except the common ones that are always shared which can be made to default to 
all engines like in my version.

The tag makes it easier to group things nicely but adds a new level to the xml 
and requires duplicate definitions of the instruction if more than one engine 
uses that instruction but some engine doesn't.

Splitting the genxml files into different ones for each engine also organizes 
the instructions nicely but has the same problem as the tag and requires 
loading 
and parsing of multiple xml files.

To make things more icky, there are the instructions, like MI_FLUSH_DW, which 
are shared between engines but might use some of the bits for different things.

In the end, I just went with how Lionel saw it because it was as good of an 
option as any, and pretty straight forward to implement, so.. mehhh. I don't 
know if you'd prefer one way or the other.

> 
> --Jason
> 
> 
> > On Wed, Oct 31, 2018 at 09:20:39AM -0500, Jason Ekstrand wrote:
> > > Toni,
> > >
> > > I'm a bit curious where you're going with this.  I started on a similar
> > > project a couple of years ago:
> > >
> > > https://gitlab.freedesktop.org/jekstrand/mesa/commits/wip/genxml-engines
> > >
> > > Mine took a different (not necessarily better) approach of surrounding
> > the
> > > instructions in an  tag.  I'm not sure if that's any better or
> > > worse than an attribute.
> > >
> > > At the time, I was planning to port over the blitter code to genxml and
> > get
> > > aubinator decoding blit streams.  I canned the project because there are
> > > few enough differences in hardware generations for the blitter to be
> > worth
> > > the re-compilation and I had better things to do.  I've always thought it
> > > would be good to support other engines for no other reason than to make
> > > aubinator for blits.  It would also likely be useful to have if we wanted
> > > to start doing media in mesa for some reason.  What's your motivation?  I
> > > ask because I can't really have an opinion on the approach unless I know
> > > where it's headed.
> > >
> > > --Jason
> > >
> > > On Wed, Oct 31, 2018 at 8:12 AM Toni Lönnberg 
> > > wrote:
> > >
> > > > These patches add an engine parameter to the instructions defined in
> > the
> > > > genxml
> > > > files so that they can be distinguished when sending them to different
> > > > engines.
> > > > By default, an instruction is defined to be used by all engines and is
> > > > defined
> > > > for a specific engine by adding the parameter "engine" to the
> > definition.
> > > > Currently the supported engines are "render", "video" and "blitter".
> > > >
> > > > v2:
> > > >
> > > > * gen_engine enum removed and replaced with use of
> > > > drm_i915_gem_engine_class
> > > >
> > > > * The current engine being used is now saved in the decoder context
> > and is
> > > > not
> > > >   being passed through gen_print_batch().
> > > >
> > > > * Split the genxml changes into multiple patches
> > > >
> > > > Toni Lönnberg (13):
> > > >   intel/decoder: tools: gen_engine to drm_i915_gem_engine_class
> > > >   intel/decoder: Engine parameter for instructions
> > > >   intel/decoder: tools: Use engine for decoding batch instructions
> > > >   intel/genxml: Add engine definition to render engine instructions
> > > > (gen4)
> > > >   intel/genxml: Add engine def

Re: [Mesa-dev] [PATCH v2 00/13] Engine parameter for instructions

[Jason Ekstrand]

On Wed, Oct 31, 2018 at 9:34 AM Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:

> On 31/10/2018 14:20, Jason Ekstrand wrote:
>
> Toni,
>
> I'm a bit curious where you're going with this.  I started on a similar
> project a couple of years ago:
>
> https://gitlab.freedesktop.org/jekstrand/mesa/commits/wip/genxml-engines
>
> Mine took a different (not necessarily better) approach of surrounding the
> instructions in an  tag.  I'm not sure if that's any better or
> worse than an attribute.
>
>
> I suggested the attribute because it would avoid duplication of some MI
> instructions.
>

That makes sense.  We could do something like engines="a,b,c,d" if we
wanted to specify more than one but not all.  I guess that makes sense.

--Jason



>
> At the time, I was planning to port over the blitter code to genxml and
> get aubinator decoding blit streams.  I canned the project because there
> are few enough differences in hardware generations for the blitter to be
> worth the re-compilation and I had better things to do.  I've always
> thought it would be good to support other engines for no other reason than
> to make aubinator for blits.  It would also likely be useful to have if we
> wanted to start doing media in mesa for some reason.  What's your
> motivation?  I ask because I can't really have an opinion on the approach
> unless I know where it's headed.
>
> --Jason
>
> On Wed, Oct 31, 2018 at 8:12 AM Toni Lönnberg 
> wrote:
>
>> These patches add an engine parameter to the instructions defined in the
>> genxml
>> files so that they can be distinguished when sending them to different
>> engines.
>> By default, an instruction is defined to be used by all engines and is
>> defined
>> for a specific engine by adding the parameter "engine" to the definition.
>> Currently the supported engines are "render", "video" and "blitter".
>>
>> v2:
>>
>> * gen_engine enum removed and replaced with use of
>> drm_i915_gem_engine_class
>>
>> * The current engine being used is now saved in the decoder context and
>> is not
>>   being passed through gen_print_batch().
>>
>> * Split the genxml changes into multiple patches
>>
>> Toni Lönnberg (13):
>>   intel/decoder: tools: gen_engine to drm_i915_gem_engine_class
>>   intel/decoder: Engine parameter for instructions
>>   intel/decoder: tools: Use engine for decoding batch instructions
>>   intel/genxml: Add engine definition to render engine instructions
>> (gen4)
>>   intel/genxml: Add engine definition to render engine instructions
>> (gen45)
>>   intel/genxml: Add engine definition to render engine instructions
>> (gen5)
>>   intel/genxml: Add engine definition to render engine instructions
>> (gen6)
>>   intel/genxml: Add engine definition to render engine instructions
>> (gen7)
>>   intel/genxml: Add engine definition to render engine instructions
>> (gen75)
>>   intel/genxml: Add engine definition to render engine instructions
>> (gen8)
>>   intel/genxml: Add engine definition to render engine instructions
>> (gen9)
>>   intel/genxml: Add engine definition to render engine instructions
>> (gen10)
>>   intel/genxml: Add engine definition to render engine instructions
>> (gen11)
>>
>>  src/intel/common/gen_batch_decoder.c |  25 ++-
>>  src/intel/common/gen_decoder.c   |  18 +-
>>  src/intel/common/gen_decoder.h   |  11 +-
>>  src/intel/genxml/gen10.xml   | 206 +++---
>>  src/intel/genxml/gen11.xml   | 208 +++
>>  src/intel/genxml/gen4.xml|  36 ++--
>>  src/intel/genxml/gen45.xml   |  38 ++---
>>  src/intel/genxml/gen5.xml|  44 ++---
>>  src/intel/genxml/gen6.xml|  94 +-
>>  src/intel/genxml/gen7.xml| 154 -
>>  src/intel/genxml/gen75.xml   | 184 ++--
>>  src/intel/genxml/gen8.xml| 202 +++---
>>  src/intel/genxml/gen9.xml| 208 +++
>>  src/intel/tools/aub_read.c   |  22 +--
>>  src/intel/tools/aub_read.h   |  11 +-
>>  src/intel/tools/aubinator.c  |   8 +-
>>  src/intel/tools/aubinator_error_decode.c |  16 ++
>>  17 files changed, 763 insertions(+), 722 deletions(-)
>>
>> --
>> 2.17.1
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
> ___
> mesa-dev mailing 
> listmesa-dev@lists.freedesktop.orghttps://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 00/13] Engine parameter for instructions

[Jason Ekstrand]

On Wed, Oct 31, 2018 at 11:10 AM Toni Lönnberg 
wrote:

> When we debug media or 3d+media workloads, we'd like to be able to see
> what is
> in the aub dumps for those workloads. At the moment the decoder can't
> distinguish instructions which share the same opcode between the render
> and
> video pipe, and thus aubinator outputs garbage on media instructions.
>
> I was reluctant to make these changes into Mesa in the first place since
> the
> work is related to media, not 3d, but as aubinator is now located here,
> here we
> are.
>

That's fine.  It's fine if these changres live in mesa.


> As far as I can see, these are the options:
>
> 1. Put these in Mesa as aubinator now resides here instead of IGT.
>a) Put all instruction definitions in the current genxml files, be it
> with a
>   tag or an attribute, both methods have their advantages and
> disadvantages.
>b) Separate genxml files into common, render and video (and blitter?)
>
> 2. Fork aubinator tools and related genxml infra to a completely separate
>project.
>
> If I have missed an option, feel free to suggest.
>

I wasn't suggesting we fork the tools and the XML.  I was just wondering
whether we wanted to do separate sections or an attribute.  I think it
should land in mesa either way.

--Jason


> On Wed, Oct 31, 2018 at 09:20:39AM -0500, Jason Ekstrand wrote:
> > Toni,
> >
> > I'm a bit curious where you're going with this.  I started on a similar
> > project a couple of years ago:
> >
> > https://gitlab.freedesktop.org/jekstrand/mesa/commits/wip/genxml-engines
> >
> > Mine took a different (not necessarily better) approach of surrounding
> the
> > instructions in an  tag.  I'm not sure if that's any better or
> > worse than an attribute.
> >
> > At the time, I was planning to port over the blitter code to genxml and
> get
> > aubinator decoding blit streams.  I canned the project because there are
> > few enough differences in hardware generations for the blitter to be
> worth
> > the re-compilation and I had better things to do.  I've always thought it
> > would be good to support other engines for no other reason than to make
> > aubinator for blits.  It would also likely be useful to have if we wanted
> > to start doing media in mesa for some reason.  What's your motivation?  I
> > ask because I can't really have an opinion on the approach unless I know
> > where it's headed.
> >
> > --Jason
> >
> > On Wed, Oct 31, 2018 at 8:12 AM Toni Lönnberg 
> > wrote:
> >
> > > These patches add an engine parameter to the instructions defined in
> the
> > > genxml
> > > files so that they can be distinguished when sending them to different
> > > engines.
> > > By default, an instruction is defined to be used by all engines and is
> > > defined
> > > for a specific engine by adding the parameter "engine" to the
> definition.
> > > Currently the supported engines are "render", "video" and "blitter".
> > >
> > > v2:
> > >
> > > * gen_engine enum removed and replaced with use of
> > > drm_i915_gem_engine_class
> > >
> > > * The current engine being used is now saved in the decoder context
> and is
> > > not
> > >   being passed through gen_print_batch().
> > >
> > > * Split the genxml changes into multiple patches
> > >
> > > Toni Lönnberg (13):
> > >   intel/decoder: tools: gen_engine to drm_i915_gem_engine_class
> > >   intel/decoder: Engine parameter for instructions
> > >   intel/decoder: tools: Use engine for decoding batch instructions
> > >   intel/genxml: Add engine definition to render engine instructions
> > > (gen4)
> > >   intel/genxml: Add engine definition to render engine instructions
> > > (gen45)
> > >   intel/genxml: Add engine definition to render engine instructions
> > > (gen5)
> > >   intel/genxml: Add engine definition to render engine instructions
> > > (gen6)
> > >   intel/genxml: Add engine definition to render engine instructions
> > > (gen7)
> > >   intel/genxml: Add engine definition to render engine instructions
> > > (gen75)
> > >   intel/genxml: Add engine definition to render engine instructions
> > > (gen8)
> > >   intel/genxml: Add engine definition to render engine instructions
> > > (gen9)
> > >   intel/genxml: Add engine definition to render engine instructions
> > > (gen10)
> > >   intel/genxml: Add engine definition to render engine instructions
> > > (gen11)
> > >
> > >  src/intel/common/gen_batch_decoder.c |  25 ++-
> > >  src/intel/common/gen_decoder.c   |  18 +-
> > >  src/intel/common/gen_decoder.h   |  11 +-
> > >  src/intel/genxml/gen10.xml   | 206 +++---
> > >  src/intel/genxml/gen11.xml   | 208 +++
> > >  src/intel/genxml/gen4.xml|  36 ++--
> > >  src/intel/genxml/gen45.xml   |  38 ++---
> > >  src/intel/genxml/gen5.xml|  44 ++---
> > >  src/intel/genxml/gen6.xml|  94 +-
> > >  src/intel/genxml/gen7.xml   

Re: [Mesa-dev] [PATCH v2 00/13] Engine parameter for instructions

[Toni Lönnberg]

When we debug media or 3d+media workloads, we'd like to be able to see what is 
in the aub dumps for those workloads. At the moment the decoder can't 
distinguish instructions which share the same opcode between the render and 
video pipe, and thus aubinator outputs garbage on media instructions.

I was reluctant to make these changes into Mesa in the first place since the 
work is related to media, not 3d, but as aubinator is now located here, here we 
are.

As far as I can see, these are the options:

1. Put these in Mesa as aubinator now resides here instead of IGT.
   a) Put all instruction definitions in the current genxml files, be it with a 
  tag or an attribute, both methods have their advantages and disadvantages.
   b) Separate genxml files into common, render and video (and blitter?)

2. Fork aubinator tools and related genxml infra to a completely separate 
   project.

If I have missed an option, feel free to suggest.

On Wed, Oct 31, 2018 at 09:20:39AM -0500, Jason Ekstrand wrote:
> Toni,
> 
> I'm a bit curious where you're going with this.  I started on a similar
> project a couple of years ago:
> 
> https://gitlab.freedesktop.org/jekstrand/mesa/commits/wip/genxml-engines
> 
> Mine took a different (not necessarily better) approach of surrounding the
> instructions in an  tag.  I'm not sure if that's any better or
> worse than an attribute.
> 
> At the time, I was planning to port over the blitter code to genxml and get
> aubinator decoding blit streams.  I canned the project because there are
> few enough differences in hardware generations for the blitter to be worth
> the re-compilation and I had better things to do.  I've always thought it
> would be good to support other engines for no other reason than to make
> aubinator for blits.  It would also likely be useful to have if we wanted
> to start doing media in mesa for some reason.  What's your motivation?  I
> ask because I can't really have an opinion on the approach unless I know
> where it's headed.
> 
> --Jason
> 
> On Wed, Oct 31, 2018 at 8:12 AM Toni Lönnberg 
> wrote:
> 
> > These patches add an engine parameter to the instructions defined in the
> > genxml
> > files so that they can be distinguished when sending them to different
> > engines.
> > By default, an instruction is defined to be used by all engines and is
> > defined
> > for a specific engine by adding the parameter "engine" to the definition.
> > Currently the supported engines are "render", "video" and "blitter".
> >
> > v2:
> >
> > * gen_engine enum removed and replaced with use of
> > drm_i915_gem_engine_class
> >
> > * The current engine being used is now saved in the decoder context and is
> > not
> >   being passed through gen_print_batch().
> >
> > * Split the genxml changes into multiple patches
> >
> > Toni Lönnberg (13):
> >   intel/decoder: tools: gen_engine to drm_i915_gem_engine_class
> >   intel/decoder: Engine parameter for instructions
> >   intel/decoder: tools: Use engine for decoding batch instructions
> >   intel/genxml: Add engine definition to render engine instructions
> > (gen4)
> >   intel/genxml: Add engine definition to render engine instructions
> > (gen45)
> >   intel/genxml: Add engine definition to render engine instructions
> > (gen5)
> >   intel/genxml: Add engine definition to render engine instructions
> > (gen6)
> >   intel/genxml: Add engine definition to render engine instructions
> > (gen7)
> >   intel/genxml: Add engine definition to render engine instructions
> > (gen75)
> >   intel/genxml: Add engine definition to render engine instructions
> > (gen8)
> >   intel/genxml: Add engine definition to render engine instructions
> > (gen9)
> >   intel/genxml: Add engine definition to render engine instructions
> > (gen10)
> >   intel/genxml: Add engine definition to render engine instructions
> > (gen11)
> >
> >  src/intel/common/gen_batch_decoder.c |  25 ++-
> >  src/intel/common/gen_decoder.c   |  18 +-
> >  src/intel/common/gen_decoder.h   |  11 +-
> >  src/intel/genxml/gen10.xml   | 206 +++---
> >  src/intel/genxml/gen11.xml   | 208 +++
> >  src/intel/genxml/gen4.xml|  36 ++--
> >  src/intel/genxml/gen45.xml   |  38 ++---
> >  src/intel/genxml/gen5.xml|  44 ++---
> >  src/intel/genxml/gen6.xml|  94 +-
> >  src/intel/genxml/gen7.xml| 154 -
> >  src/intel/genxml/gen75.xml   | 184 ++--
> >  src/intel/genxml/gen8.xml| 202 +++---
> >  src/intel/genxml/gen9.xml| 208 +++
> >  src/intel/tools/aub_read.c   |  22 +--
> >  src/intel/tools/aub_read.h   |  11 +-
> >  src/intel/tools/aubinator.c  |   8 +-
> >  src/intel/tools/aubinator_error_decode.c |  16 ++
> >  17 files changed, 763 inse

Re: [Mesa-dev] [PATCH v2 00/13] Engine parameter for instructions

[Lionel Landwerlin]

On 31/10/2018 14:20, Jason Ekstrand wrote:

Toni,

I'm a bit curious where you're going with this.  I started on a 
similar project a couple of years ago:


https://gitlab.freedesktop.org/jekstrand/mesa/commits/wip/genxml-engines

Mine took a different (not necessarily better) approach of surrounding 
the instructions in an  tag. I'm not sure if that's any better 
or worse than an attribute.



I suggested the attribute because it would avoid duplication of some MI 
instructions.





At the time, I was planning to port over the blitter code to genxml 
and get aubinator decoding blit streams.  I canned the project because 
there are few enough differences in hardware generations for the 
blitter to be worth the re-compilation and I had better things to do.  
I've always thought it would be good to support other engines for no 
other reason than to make aubinator for blits.  It would also likely 
be useful to have if we wanted to start doing media in mesa for some 
reason.  What's your motivation?  I ask because I can't really have an 
opinion on the approach unless I know where it's headed.


--Jason

On Wed, Oct 31, 2018 at 8:12 AM Toni Lönnberg > wrote:


These patches add an engine parameter to the instructions defined
in the genxml
files so that they can be distinguished when sending them to
different engines.
By default, an instruction is defined to be used by all engines
and is defined
for a specific engine by adding the parameter "engine" to the
definition.
Currently the supported engines are "render", "video" and "blitter".

v2:

* gen_engine enum removed and replaced with use of
drm_i915_gem_engine_class

* The current engine being used is now saved in the decoder
context and is not
  being passed through gen_print_batch().

* Split the genxml changes into multiple patches

Toni Lönnberg (13):
  intel/decoder: tools: gen_engine to drm_i915_gem_engine_class
  intel/decoder: Engine parameter for instructions
  intel/decoder: tools: Use engine for decoding batch instructions
  intel/genxml: Add engine definition to render engine instructions
    (gen4)
  intel/genxml: Add engine definition to render engine instructions
    (gen45)
  intel/genxml: Add engine definition to render engine instructions
    (gen5)
  intel/genxml: Add engine definition to render engine instructions
    (gen6)
  intel/genxml: Add engine definition to render engine instructions
    (gen7)
  intel/genxml: Add engine definition to render engine instructions
    (gen75)
  intel/genxml: Add engine definition to render engine instructions
    (gen8)
  intel/genxml: Add engine definition to render engine instructions
    (gen9)
  intel/genxml: Add engine definition to render engine instructions
    (gen10)
  intel/genxml: Add engine definition to render engine instructions
    (gen11)

 src/intel/common/gen_batch_decoder.c     |  25 ++-
 src/intel/common/gen_decoder.c           |  18 +-
 src/intel/common/gen_decoder.h           |  11 +-
 src/intel/genxml/gen10.xml               | 206 +++---
 src/intel/genxml/gen11.xml               | 208
+++
 src/intel/genxml/gen4.xml                |  36 ++--
 src/intel/genxml/gen45.xml               |  38 ++---
 src/intel/genxml/gen5.xml                |  44 ++---
 src/intel/genxml/gen6.xml                |  94 +-
 src/intel/genxml/gen7.xml                | 154 -
 src/intel/genxml/gen75.xml               | 184 ++--
 src/intel/genxml/gen8.xml                | 202 +++---
 src/intel/genxml/gen9.xml                | 208
+++
 src/intel/tools/aub_read.c               |  22 +--
 src/intel/tools/aub_read.h               |  11 +-
 src/intel/tools/aubinator.c              |   8 +-
 src/intel/tools/aubinator_error_decode.c |  16 ++
 17 files changed, 763 insertions(+), 722 deletions(-)

-- 
2.17.1


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org 
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 00/13] Engine parameter for instructions

[Jason Ekstrand]

Toni,

I'm a bit curious where you're going with this.  I started on a similar
project a couple of years ago:

https://gitlab.freedesktop.org/jekstrand/mesa/commits/wip/genxml-engines

Mine took a different (not necessarily better) approach of surrounding the
instructions in an  tag.  I'm not sure if that's any better or
worse than an attribute.

At the time, I was planning to port over the blitter code to genxml and get
aubinator decoding blit streams.  I canned the project because there are
few enough differences in hardware generations for the blitter to be worth
the re-compilation and I had better things to do.  I've always thought it
would be good to support other engines for no other reason than to make
aubinator for blits.  It would also likely be useful to have if we wanted
to start doing media in mesa for some reason.  What's your motivation?  I
ask because I can't really have an opinion on the approach unless I know
where it's headed.

--Jason

On Wed, Oct 31, 2018 at 8:12 AM Toni Lönnberg 
wrote:

> These patches add an engine parameter to the instructions defined in the
> genxml
> files so that they can be distinguished when sending them to different
> engines.
> By default, an instruction is defined to be used by all engines and is
> defined
> for a specific engine by adding the parameter "engine" to the definition.
> Currently the supported engines are "render", "video" and "blitter".
>
> v2:
>
> * gen_engine enum removed and replaced with use of
> drm_i915_gem_engine_class
>
> * The current engine being used is now saved in the decoder context and is
> not
>   being passed through gen_print_batch().
>
> * Split the genxml changes into multiple patches
>
> Toni Lönnberg (13):
>   intel/decoder: tools: gen_engine to drm_i915_gem_engine_class
>   intel/decoder: Engine parameter for instructions
>   intel/decoder: tools: Use engine for decoding batch instructions
>   intel/genxml: Add engine definition to render engine instructions
> (gen4)
>   intel/genxml: Add engine definition to render engine instructions
> (gen45)
>   intel/genxml: Add engine definition to render engine instructions
> (gen5)
>   intel/genxml: Add engine definition to render engine instructions
> (gen6)
>   intel/genxml: Add engine definition to render engine instructions
> (gen7)
>   intel/genxml: Add engine definition to render engine instructions
> (gen75)
>   intel/genxml: Add engine definition to render engine instructions
> (gen8)
>   intel/genxml: Add engine definition to render engine instructions
> (gen9)
>   intel/genxml: Add engine definition to render engine instructions
> (gen10)
>   intel/genxml: Add engine definition to render engine instructions
> (gen11)
>
>  src/intel/common/gen_batch_decoder.c |  25 ++-
>  src/intel/common/gen_decoder.c   |  18 +-
>  src/intel/common/gen_decoder.h   |  11 +-
>  src/intel/genxml/gen10.xml   | 206 +++---
>  src/intel/genxml/gen11.xml   | 208 +++
>  src/intel/genxml/gen4.xml|  36 ++--
>  src/intel/genxml/gen45.xml   |  38 ++---
>  src/intel/genxml/gen5.xml|  44 ++---
>  src/intel/genxml/gen6.xml|  94 +-
>  src/intel/genxml/gen7.xml| 154 -
>  src/intel/genxml/gen75.xml   | 184 ++--
>  src/intel/genxml/gen8.xml| 202 +++---
>  src/intel/genxml/gen9.xml| 208 +++
>  src/intel/tools/aub_read.c   |  22 +--
>  src/intel/tools/aub_read.h   |  11 +-
>  src/intel/tools/aubinator.c  |   8 +-
>  src/intel/tools/aubinator_error_decode.c |  16 ++
>  17 files changed, 763 insertions(+), 722 deletions(-)
>
> --
> 2.17.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2 00/13] Engine parameter for instructions

[Toni Lönnberg]

These patches add an engine parameter to the instructions defined in the genxml
files so that they can be distinguished when sending them to different engines.
By default, an instruction is defined to be used by all engines and is defined
for a specific engine by adding the parameter "engine" to the definition.
Currently the supported engines are "render", "video" and "blitter".

v2:

* gen_engine enum removed and replaced with use of drm_i915_gem_engine_class

* The current engine being used is now saved in the decoder context and is not
  being passed through gen_print_batch().

* Split the genxml changes into multiple patches

Toni Lönnberg (13):
  intel/decoder: tools: gen_engine to drm_i915_gem_engine_class
  intel/decoder: Engine parameter for instructions
  intel/decoder: tools: Use engine for decoding batch instructions
  intel/genxml: Add engine definition to render engine instructions
(gen4)
  intel/genxml: Add engine definition to render engine instructions
(gen45)
  intel/genxml: Add engine definition to render engine instructions
(gen5)
  intel/genxml: Add engine definition to render engine instructions
(gen6)
  intel/genxml: Add engine definition to render engine instructions
(gen7)
  intel/genxml: Add engine definition to render engine instructions
(gen75)
  intel/genxml: Add engine definition to render engine instructions
(gen8)
  intel/genxml: Add engine definition to render engine instructions
(gen9)
  intel/genxml: Add engine definition to render engine instructions
(gen10)
  intel/genxml: Add engine definition to render engine instructions
(gen11)

 src/intel/common/gen_batch_decoder.c |  25 ++-
 src/intel/common/gen_decoder.c   |  18 +-
 src/intel/common/gen_decoder.h   |  11 +-
 src/intel/genxml/gen10.xml   | 206 +++---
 src/intel/genxml/gen11.xml   | 208 +++
 src/intel/genxml/gen4.xml|  36 ++--
 src/intel/genxml/gen45.xml   |  38 ++---
 src/intel/genxml/gen5.xml|  44 ++---
 src/intel/genxml/gen6.xml|  94 +-
 src/intel/genxml/gen7.xml| 154 -
 src/intel/genxml/gen75.xml   | 184 ++--
 src/intel/genxml/gen8.xml| 202 +++---
 src/intel/genxml/gen9.xml| 208 +++
 src/intel/tools/aub_read.c   |  22 +--
 src/intel/tools/aub_read.h   |  11 +-
 src/intel/tools/aubinator.c  |   8 +-
 src/intel/tools/aubinator_error_decode.c |  16 ++
 17 files changed, 763 insertions(+), 722 deletions(-)

-- 
2.17.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev