[Mesa-dev] [PATCH 1/3] intel/gen_decoder: Fix length for Media State/Object commands

[Lionel Landwerlin]

On 04/04/17 00:22, Jordan Justen wrote:
>  From BDW PRM, Volume 6: Command Stream Programming, 'Render Command
> Header Format'.
>
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> ---
>   src/intel/common/gen_decoder.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
> index 1ae78c88e3f..1244f4c4480 100644
> --- a/src/intel/common/gen_decoder.c
> +++ b/src/intel/common/gen_decoder.c
> @@ -697,8 +697,16 @@ gen_group_get_length(struct gen_group *group, const uint32_t *p)
>            return field(h, 0, 7) + 2;
>         case 1:
>            return 1;
> -      case 2:
> -         return 2;
> +      case 2: {
> +         uint32_t opcode = field(h, 24, 26);
> +         assert(opcode < 3 /* 3 and above currently reserved */);
> +         if (opcode == 0)
> +            return field(h, 0, 7) + 2;

It's a bit hard to read (so I might be wrong), but this doesn't seem 
correct.
For example 3DSTATE_AA_LINE_PARAMETERS has an opcode=1, yet it's 
DWordLength field is 0-7bits.

At this point I'm wondering whether it's not better off using looking 
for the actual instruction and getting the "DWord Length" to know how to 
read the length right.
Potentially printing a warning to tell genxml might need to be updated 
if we're missing the instruction in genxml.
What do you think?

> +         else if (opcode < 3)
> +            return field(h, 0, 15) + 2;
> +         else
> +            return 1; /* FIXME: if more opcodes are added */
> +      }
>         case 3:
>            return field(h, 0, 7) + 2;
>         }