[Mesa-dev] [PATCH V2 2/2] i965: Add a variable for size increment per bank in get_l3_way_size()

[Anuj Phogat]

On Mon, Jun 19, 2017 at 2:18 PM, Francisco Jerez <currojerez at riseup.net> wrote:
> Anuj Phogat <anuj.phogat at gmail.com> writes:
>
>> Adding min_size_increment_per_bank variable better explains the
>> computation of L3 way size in the function.
>>
>> V2: Use const variable for min_size_increment_per_bank.
>>
>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>> Cc: Francisco Jerez <currojerez at riseup.net>
>> ---
>>  src/intel/common/gen_l3_config.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/intel/common/gen_l3_config.c b/src/intel/common/gen_l3_config.c
>> index 44a4b24..9a7771a 100644
>> --- a/src/intel/common/gen_l3_config.c
>> +++ b/src/intel/common/gen_l3_config.c
>> @@ -271,12 +271,11 @@ gen_get_l3_config(const struct gen_device_info *devinfo,
>>  static unsigned
>>  get_l3_way_size(const struct gen_device_info *devinfo)
>>  {
>> -   assert(devinfo->l3_banks);
>> -
>> -   if (devinfo->is_broxton)
>> -      return 4;
>> +   const unsigned min_size_increment_per_bank =
>
> I think the name you used in your previous revision (way_size_per_bank)
> was more descriptive.
>
I changed the variable name to better match with what we have in the docs
after seeing your comment  on [PATCH 1/2]:
"The way size per bank is not really documented in the BSpec so it
doesn't really give the reader any additional useful information."

I don't feel strongly about either of the names. So, I'll just make the
change and push the patch upstream.

>> +      (devinfo->gen >= 9 && devinfo->l3_banks == 1) ? 4 : 2;
>
> Redundant parenthesis.  With my (cosmetic) suggestions taken into
> account patch is:
>
> Reviewed-by: Francisco Jerez <currojerez at riseup.net>
>
>>
>> -   return 2 * devinfo->l3_banks;
>> +   assert(devinfo->l3_banks);
>> +   return min_size_increment_per_bank * devinfo->l3_banks;
>>  }
>>
>>  /**
>> --
>> 2.9.4