Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> writes:
> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
>
>> The radix levels of each page directory are easily determined so replace
>> the numerous hardcoded constants with precomputed derived constants.
>>
>> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 39 +++++++++++++++++++++++++++++
>> 1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 2fc60e8acd9a..271305705c1c 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -868,6 +868,45 @@ static int gen8_ppgtt_notify_vgt(struct i915_ppgtt *ppgtt, bool create)
>> return 0;
>> }
>>
>> +/* Index shifts into the pagetable are offset by GEN8_PTE_SHIFT [12] */
>> +#define gen8_pd_shift(lvl) ((lvl) * ilog2(I915_PDES))
>
> Could be just (lvl) * 9. But looking at ilog2() it will be
> so both are fine.
>
>> +#define gen8_pd_index(i, lvl) i915_pde_index((i), gen8_pd_shift(lvl))
>> +#define __gen8_pte_shift(lvl) (GEN8_PTE_SHIFT + gen8_pd_shift(lvl))
>> +#define __gen8_pte_index(a, lvl) i915_pde_index((a), __gen8_pte_shift(lvl))
>> +
>> +static inline unsigned int
>> +gen8_pd_range(u64 addr, u64 end, int lvl, unsigned int *idx)
>
> I was enough confused (even tho the last function reveals
> it clearly) that in irc we concluded that addr as a
> first parameter is misleading and converged on 'start'
>
>> +{
>> + const int shift = gen8_pd_shift(lvl);
>> + const u64 mask = ~0ull << gen8_pd_shift(lvl + 1);
>> +
>> + GEM_BUG_ON(addr >= end);
>> + end += ~mask >> gen8_pd_shift(1);
>> +
>> + *idx = i915_pde_index(addr, shift);
>> + if ((addr ^ end) & mask)
>> + return I915_PDES - *idx;
>> + else
>> + return i915_pde_index(end, shift) - *idx;
>> +}
>> +
>> +static inline bool gen8_pd_subsumes(u64 addr, u64 end, int lvl)
>> +{
>
> Just a suggestion gen8_pd_contains() for emphasis on exclusivity.
> But well, this is fine too. I guess what reads better in callsite,
> (which we dont see yet!)
>
>> + const u64 mask = ~0ull << gen8_pd_shift(lvl + 1);
>> +
>> + GEM_BUG_ON(addr >= end);
>> + return (addr ^ end) & mask && (addr & ~mask) == 0;
>> +}
>> +
>> +static inline unsigned int gen8_pt_count(u64 addr, u64 end)
>> +{
>> + GEM_BUG_ON(addr >= end);
>> + if ((addr ^ end) & ~I915_PDE_MASK)
>> + return I915_PDES - (addr & I915_PDE_MASK);
>
> Ok, I yield on 512. I915_PDES is fine as it atleast
> couples it to mask :O
>
> With s/addr/start,
>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@mika.kuoppala@xxxxxxxxxxxxxxx>
not long till vacation, hanging there but it starts to show...
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
>
>> + else
>> + return end - addr;
>> +}
>> +
>> static void gen8_free_page_tables(struct i915_address_space *vm,
>> struct i915_page_directory *pd)
>> {
>> --
>> 2.20.1
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
