Re: [igt-dev] [PATCH i-g-t 10/21] gem_wsim: Extract str to engine lookup

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 10/05/2019 14:20, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-08 13:10:47)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   benchmarks/gem_wsim.c | 34 +++++++++++++++++++++-------------
>>   1 file changed, 21 insertions(+), 13 deletions(-)
>>
>> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
>> index 5245692df6eb..f654decb24cc 100644
>> --- a/benchmarks/gem_wsim.c
>> +++ b/benchmarks/gem_wsim.c
>> @@ -318,6 +318,18 @@ wsim_err(const char *fmt, ...)
>>          } \
>>   }
>>   
>> +static int str_to_engine(const char *str)
>> +{
>> +       unsigned int i;
>> +
>> +       for (i = 0; i < ARRAY_SIZE(ring_str_map); i++) {
>> +               if (!strcasecmp(str, ring_str_map[i]))
>> +                       return i;
>> +       }
>> +
>> +       return -1;
>> +}
>> +
>>   static struct workload *
>>   parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w)
>>   {
>> @@ -480,22 +492,18 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w)
>>                  }
>>   
>>                  if ((field = strtok_r(fstart, ".", &fctx)) != NULL) {
>> -                       unsigned int old_valid = valid;
>> -
>>                          fstart = NULL;
>>   
>> -                       for (i = 0; i < ARRAY_SIZE(ring_str_map); i++) {
>> -                               if (!strcasecmp(field, ring_str_map[i])) {
>> -                                       step.engine = i;
>> -                                       if (step.engine == BCS)
>> -                                               bcs_used = true;
>> -                                       valid++;
>> -                                       break;
>> -                               }
>> -                       }
>> -
>> -                       check_arg(old_valid == valid,
>> +                       i = str_to_engine(field);
>> +                       check_arg(i < 0,
>>                                    "Invalid engine id at step %u!\n", nr_steps);
>> +                       if (i >= 0)
>> +                               valid++;
> 
> check_arg() returned already for all i < 0, no?

Yes, and it looks the very next patch removes the if. I'll pull it here.

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks!

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev