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

[Brajesh Gupta]

On Fri, 2026-10-02 at 10:08 +0900, Gyeyoung Baek wrote:
Hi Gyeyoung,
> 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.
> 
Make sense.

Reviewed-by: Brajesh Gupta <brajesh.gupta@xxxxxxxxxx>

Thanks,
Brajesh