Joonas Lahtinen <joonas.lahti...@linux.intel.com> writes:

> On ma, 2016-10-31 at 17:55 +0200, Mika Kuoppala wrote:
>> @@ -712,13 +712,13 @@ static int gen8_48b_mm_switch(struct i915_hw_ppgtt 
>> *ppgtt,
>>   */
>>  static bool gen8_ppgtt_clear_pt(struct i915_address_space *vm,
>>                              struct i915_page_table *pt,
>> -                            uint64_t start,
>> -                            uint64_t length)
>> +                            const uint64_t start,
>> +                            const uint64_t length)
>>  {
>
> I think const for integers is bit much, with that logic we should make
> the pointers const too (not the pointer destination).
>

It is more of a compiler guard of accidentally changing these
in the body. Separation of variants and invariants. But
if this not preferred, I can change them back.

>> @@ -735,8 +737,8 @@ static bool gen8_ppgtt_clear_pt(struct 
>> i915_address_space *vm,
>>  
>>      pt_vaddr = kmap_px(pt);
>>  
>> -    for (pte = pte_start; pte < num_entries; pte++)
>> -            pt_vaddr[pte] = scratch_pte;
>> +    while (pte < pte_end)
>> +            pt_vaddr[pte++] = scratch_pte;
>
> I'd prefer the for loop still. Just fix "pte < pte_end".
>

I changed to this due to pte being already set and
used before the loop, to get rid of superfluous pte_start.
-Mika


> Regards, Joonas
> -- 
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to