Re: [PATCH v2 2/2] drm/imagination: Size page table preallocation by device address

[Gyeyoung Baek]

Hi Brajesh,

> > -             const u32 l1_start_idx = pvr_page_table_l2_idx(start_addr);
> > -             const u32 l1_end_idx = pvr_page_table_l2_idx(start_addr + size);
> > -             const u32 l1_count = l1_end_idx - l1_start_idx + 1;
> > -             const u32 l0_start_idx = pvr_page_table_l1_idx(start_addr);
> > -             const u32 l0_end_idx = pvr_page_table_l1_idx(start_addr + size);
> > -             const u32 l0_count = l0_end_idx - l0_start_idx + 1;
> > +             const u64 last_addr = device_addr + size - 1;
> > +             const u64 l1_count =
> > +                     (last_addr >> ROGUE_MMUCTRL_VADDR_PC_INDEX_SHIFT) -
> > +                     (device_addr >> ROGUE_MMUCTRL_VADDR_PC_INDEX_SHIFT) + 1;
> > +             const u64 l0_count =
> > +                     (last_addr >> ROGUE_MMUCTRL_VADDR_PD_INDEX_SHIFT) -
> > +                     (device_addr >> ROGUE_MMUCTRL_VADDR_PD_INDEX_SHIFT) + 1;
> Shouldn't we use 'device_addr + sgt_offset' instead of just 'device_addr' as
> start address for l0/l1 count calculation? Only account for requested mapping
> instead of whole memory.
>

This already counts only the requested mapping. `device_addr` is not
the start of the BO but the GPU address where this mapping starts, and
[device_addr, device_addr + size) is exactly the range pvr_mmu_map()
fills.

Adding sgt_offset does not narrow the range; it shifts it by
sgt_offset, which is an offset into the BO, not a GPU address. This
can leave too few preallocated tables, and VM_MAP then fails with
-ENOMEM.

-- 
Thanks,
Gyeyoung