Anuj Phogat <anuj.pho...@gmail.com> writes:
> 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 BSpec using a misleading name for something doesn't make it a good idea for us to use it. If you want to make it easy for people to find this information on the hardware docs it would be more useful to add a reference to the right BSpec page where you got this information. > "The way size per bank is not really documented in the BSpec so it > doesn't really give the reader any additional useful information." > The point I was trying to make is that the way size per bank is not documented consistently across generations (when at all) and you can only reconstruct it after pulling together information scattered over the whole BSpec. OTOH the total way size is readily available so arguing about the former only seemed like a way to confuse the reader, particularly in the context of BXT 2x6 that has a single L3 bank. > 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
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
