On Mon, Jun 19, 2017 at 2:18 PM, Francisco Jerez <curroje...@riseup.net> wrote:
> Anuj Phogat <anuj.pho...@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.pho...@gmail.com>
>> Cc: Francisco Jerez <curroje...@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 <curroje...@riseup.net>
>
>>
>> - return 2 * devinfo->l3_banks;
>> + assert(devinfo->l3_banks);
>> + return min_size_increment_per_bank * devinfo->l3_banks;
>> }
>>
>> /**
>> --
>> 2.9.4
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
