| Message ID | 20180117154705.24628-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wednesday, January 17, 2018 7:47:05 AM PST 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@chris-wilson.co.uk> > Cc: Michel Thierry <michel.thierry@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Thanks Chris! Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
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@chris-wilson.co.uk> > Cc: Michel Thierry <michel.thierry@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Cc: Kenneth Graunke <kenneth@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@intel.com> > + for (size_t i = 0; i < ARRAY_SIZE(legacy_names); i++) { > + if (strcmp(ring_name, legacy_names[i].name)) > + continue; > + > + *class = legacy_names[i].class; > + return legacy_names[i].instance; > + } > + > + return -1; > +} > + > static const char * > register_name_from_ring(const struct ring_register_mapping *mapping, > unsigned nb_mapping, > const char *ring_name) > { > + unsigned int class; > + int instance; > + > + instance = ring_name_to_class(ring_name, &class); > + if (instance < 0) > + return NULL; > + > for (unsigned i = 0; i < nb_mapping; i++) { > - if (strcmp(mapping[i].ring_name, ring_name) == 0) > + if (mapping[i].ring_class == class && > + mapping[i].ring_instance == instance) > return mapping[i].register_name; > } > return NULL; > @@ -117,16 +172,35 @@ static const char * > instdone_register_for_ring(const struct gen_device_info *devinfo, > const char *ring_name) > { > - if (strcmp(ring_name, "blt") == 0) > - return "BCS_INSTDONE"; > - else if (strcmp(ring_name, "vebox") == 0) > - return "VECS_INSTDONE"; > - else if (strcmp(ring_name, "bsd") == 0) > - return "VCS_INSTDONE"; > - else if (strcmp(ring_name, "render") == 0) { > + unsigned int class; > + int instance; > + > + instance = ring_name_to_class(ring_name, &class); > + if (instance < 0) > + return NULL; > + > + switch (class) { > + case RCS: > if (devinfo->gen == 6) > return "INSTDONE_2"; > - return "INSTDONE_1"; > + else > + return "INSTDONE_1"; > + > + case BCS: > + return "BCS_INSTDONE"; > + > + case VCS: > + switch (instance) { > + case 0: > + return "VCS_INSTDONE"; > + case 1: > + return "VCS2_INSTDONE"; > + default: > + return NULL; > + } > + > + case VECS: > + return "VECS_INSTDONE"; > } > > return NULL; >
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@chris-wilson.co.uk> > > Cc: Michel Thierry <michel.thierry@intel.com> > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > Cc: Kenneth Graunke <kenneth@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@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
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", + }; + 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 }, + }; + for (size_t i = 0; i < ARRAY_SIZE(legacy_names); i++) { + if (strcmp(ring_name, legacy_names[i].name)) + continue; + + *class = legacy_names[i].class; + return legacy_names[i].instance; + } + + return -1; +} + static const char * register_name_from_ring(const struct ring_register_mapping *mapping, unsigned nb_mapping, const char *ring_name) { + unsigned int class; + int instance; + + instance = ring_name_to_class(ring_name, &class); + if (instance < 0) + return NULL; + for (unsigned i = 0; i < nb_mapping; i++) { - if (strcmp(mapping[i].ring_name, ring_name) == 0) + if (mapping[i].ring_class == class && + mapping[i].ring_instance == instance) return mapping[i].register_name; } return NULL; @@ -117,16 +172,35 @@ static const char * instdone_register_for_ring(const struct gen_device_info *devinfo, const char *ring_name) { - if (strcmp(ring_name, "blt") == 0) - return "BCS_INSTDONE"; - else if (strcmp(ring_name, "vebox") == 0) - return "VECS_INSTDONE"; - else if (strcmp(ring_name, "bsd") == 0) - return "VCS_INSTDONE"; - else if (strcmp(ring_name, "render") == 0) { + unsigned int class; + int instance; + + instance = ring_name_to_class(ring_name, &class); + if (instance < 0) + return NULL; + + switch (class) { + case RCS: if (devinfo->gen == 6) return "INSTDONE_2"; - return "INSTDONE_1"; + else + return "INSTDONE_1"; + + case BCS: + return "BCS_INSTDONE"; + + case VCS: + switch (instance) { + case 0: + return "VCS_INSTDONE"; + case 1: + return "VCS2_INSTDONE"; + default: + return NULL; + } + + case VECS: + return "VECS_INSTDONE"; } return NULL;
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@chris-wilson.co.uk> Cc: Michel Thierry <michel.thierry@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Cc: Kenneth Graunke <kenneth@whitecape.org> --- src/intel/tools/aubinator_error_decode.c | 122 +++++++++++++++++++++++++------ 1 file changed, 98 insertions(+), 24 deletions(-)