Overall I agree this probably what we should do rather than this long
special case switch in gen_group_get_length().
I have one suggestion below.
On 26/10/2018 14:07, Toni Lönnberg wrote:
Use the 'DWord Length' and 'bias' fields from the instruction definition to
parse the packet length from the command stream when possible. The hardcoded
mechanism is used whenever an instruction doesn't have this field.
---
src/intel/common/gen_decoder.c | 16 ++++++++++++++--
src/intel/common/gen_decoder.h | 1 +
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
index 5f6e7a0b808..2d58708f5dd 100644
--- a/src/intel/common/gen_decoder.c
+++ b/src/intel/common/gen_decoder.c
@@ -163,11 +163,15 @@ create_group(struct parser_context *ctx,
group->spec = ctx->spec;
group->variable = false;
group->fixed_length = fixed_length;
+ group->dw_length = 0;
+ group->bias = 1;
for (int i = 0; atts[i]; i += 2) {
char *p;
if (strcmp(atts[i], "length") == 0) {
group->dw_length = strtoul(atts[i + 1], &p, 0);
+ } else if (strcmp(atts[i], "bias") == 0) {
+ group->bias = strtoul(atts[i + 1], &p, 0);
}
}
@@ -743,8 +747,16 @@ gen_group_find_field(struct gen_group *group, const char *name)
int
gen_group_get_length(struct gen_group *group, const uint32_t *p)
{
- if (group && group->fixed_length)
- return group->dw_length;
+ if (group) {
+ if (group->fixed_length)
+ return group->dw_length;
+ else {
+ struct gen_field *field = gen_group_find_field(group, "DWord Length");
Since "DWord Length" is a field we're going to use quite often, I would
put a pointer to the field in gen_group.
You can do that in the create_field() function and save your self the
find_field() for every instruction.
+ if (field) {
+ return field_value(p[0], field->start, field->end) + group->bias;
}
+ }
+ }
If we had tests (I'm meant to write some), I would go as far as removing
the whole thing below and just return 1.
It means we don't have a group and in that case we're likely going to
just iterate dword by dword.
uint32_t h = p[0];
uint32_t type = field_value(h, 29, 31);
diff --git a/src/intel/common/gen_decoder.h b/src/intel/common/gen_decoder.h
index a80c50b6647..0b33eb32cfc 100644
--- a/src/intel/common/gen_decoder.h
+++ b/src/intel/common/gen_decoder.h
@@ -101,6 +101,7 @@ struct gen_group {
struct gen_field *fields; /* linked list of fields */
uint32_t dw_length;
+ uint32_t bias; /* <instruction> specific */
uint32_t group_offset, group_count;
uint32_t group_size;
bool variable; /* <group> specific */
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
