[Mesa-dev] [PATCH] intel: Future-proof ring names for aubinator_error_decode

[Chris Wilson]

Quoting Michel Thierry (2018-01-18 16:44:18)
> On 1/17/2018 7:47 AM, Chris Wilson wrote:
> > The kernel is moving to a $class$instance naming scheme in preparation
> > for accommodating more rings in the future in a consistent manner. It is
> > already using the naming scheme internally, and now we are looking at
> > updating some soft-ABI such as the error state to use the new naming
> > scheme. This of course means we need to teach aubinator_error_decode how
> > to map both sets of ring names onto its register maps.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Michel Thierry <michel.thierry at intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> > Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> > Cc: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >   src/intel/tools/aubinator_error_decode.c | 122 +++++++++++++++++++++++++------
> >   1 file changed, 98 insertions(+), 24 deletions(-)
> > 
> > diff --git a/src/intel/tools/aubinator_error_decode.c b/src/intel/tools/aubinator_error_decode.c
> > index 9dd70790e1..01c6a7a365 100644
> > --- a/src/intel/tools/aubinator_error_decode.c
> > +++ b/src/intel/tools/aubinator_error_decode.c
> > @@ -74,40 +74,95 @@ print_register(struct gen_spec *spec, const char *name, uint32_t reg)
> >   }
> >   
> >   struct ring_register_mapping {
> > -   const char *ring_name;
> > +   unsigned ring_class;
> > +   unsigned ring_instance;
> >      const char *register_name;
> >   };
> >   
> > +enum {
> > +   RCS,
> > +   BCS,
> > +   VCS,
> > +   VECS,
> > +};
> > +
> >   static const struct ring_register_mapping acthd_registers[] = {
> > -   { "blt", "BCS_ACTHD_UDW" },
> > -   { "bsd", "VCS_ACTHD_UDW" },
> > -   { "bsd2", "VCS2_ACTHD_UDW" },
> > -   { "render", "ACTHD_UDW" },
> > -   { "vebox", "VECS_ACTHD_UDW" },
> > +   { BCS, 0, "BCS_ACTHD_UDW" },
> > +   { VCS, 0, "VCS_ACTHD_UDW" },
> > +   { VCS, 1, "VCS2_ACTHD_UDW" },
> > +   { RCS, 0, "ACTHD_UDW" },
> > +   { VECS, 0, "VECS_ACTHD_UDW" },
> >   };
> >   
> >   static const struct ring_register_mapping ctl_registers[] = {
> > -   { "blt", "BCS_RING_BUFFER_CTL" },
> > -   { "bsd", "VCS_RING_BUFFER_CTL" },
> > -   { "bsd2", "VCS2_RING_BUFFER_CTL" },
> > -   { "render", "RCS_RING_BUFFER_CTL" },
> > -   { "vebox", "VECS_RING_BUFFER_CTL" },
> > +   { BCS, 0, "BCS_RING_BUFFER_CTL" },
> > +   { VCS, 0, "VCS_RING_BUFFER_CTL" },
> > +   { VCS, 1, "VCS2_RING_BUFFER_CTL" },
> > +   { RCS, 0, "RCS_RING_BUFFER_CTL" },
> > +   { VECS, 0,  "VECS_RING_BUFFER_CTL" },
> >   };
> >   
> >   static const struct ring_register_mapping fault_registers[] = {
> > -   { "blt", "BCS_FAULT_REG" },
> > -   { "bsd", "VCS_FAULT_REG" },
> > -   { "render", "RCS_FAULT_REG" },
> > -   { "vebox", "VECS_FAULT_REG" },
> > +   { BCS, 0, "BCS_FAULT_REG" },
> > +   { VCS, 0, "VCS_FAULT_REG" },
> > +   { RCS, 0, "RCS_FAULT_REG" },
> > +   { VECS, 0, "VECS_FAULT_REG" },
> >   };
> >   
> > +static int ring_name_to_class(const char *ring_name,
> > +                              unsigned int *class)
> > +{
> > +   static const char *class_names[] = {
> > +      [RCS] = "rcs",
> > +      [BCS] = "bcs",
> > +      [VCS] = "vcs",
> > +      [VECS] = "vecs",
> > +   };
> 
> This will match the new names in the error state, e.g.
> 
> rcs0 command stream:
> ...
> bcs0 command stream:
> ...
> vcs0 command stream:
> ...
> vcs1 command stream:
> ...
> vecs0 command stream:
> ...
> 
> > +   for (size_t i = 0; i < ARRAY_SIZE(class_names); i++) {
> > +      if (strcmp(ring_name, class_names[i]))
> > +         continue;
> > +
> > +      *class = i;
> > +      return atoi(ring_name + strlen(class_names[i]));
> > +   }
> > +
> > +   static const struct {
> > +      const char *name;
> > +      unsigned int class;
> > +      int instance;
> > +   } legacy_names[] = {
> > +      { "render", RCS, 0 },
> > +      { "blt", BCS, 0 },
> > +      { "bsd", VCS, 0 },
> > +      { "bsd2", VCS, 1 },
> > +      { "vebox", VECS, 0 },
> > +   };
> 
> And these are the current ones, so also
> 
> Reviewed-by: Michel Thierry <michel.thierry at intel.com>

Thanks for sanity checking my string handling, pushed.
We should be ready now to use the new naming scheme in the error state,
I believe.
-Chris