Hi Brajesh,
On Fri, 2026-09-18 at 16:11 +0530, Brajesh Gupta wrote:
> The GPU virtual start address wasn't included in the calculation for the
> amount of page tables required for mapping a BO object in map() interface.
> It resulted in map failure later due to not enough pages at L0/L1 level.
> Update pvr_mmu_op_context_create() interface to pass device address as well
> to allow correct calculation for page table memory.
>
> Miscalculation of page table pages for mapping a BO starting at a device
> address 0x8001b45000 of size 0x8ca000:
> old new
> L0 count 5 6
> L1 count 1 1
Suggestion for a simpler example, taken from the (wrong) example from Sashiko
and that might help calm it down: if L0 tables cover 2MB (0x200000), the range
defined by device address 0x80001ff000 (general heap at 2MB - 4KB) and size
0x2000 (two 4KB pages) requires two L0 pages to be mapped, but without the base
address a range of 0x2000 computes to a single L0 page which is not enough.
>
> Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
> Signed-off-by: Brajesh Gupta <[email protected]>
>
nit: Trailers should be a single block without empty lines and your signed off
line should come after everything else.
Thanks,
Alessio
> Reviewed-by: Alexandru Dadu <[email protected]>
> Reviewed-by: Alessio Belle <[email protected]>
> ---
> drivers/gpu/drm/imagination/pvr_mmu.c | 14 ++++++++------
> drivers/gpu/drm/imagination/pvr_mmu.h | 2 +-
> drivers/gpu/drm/imagination/pvr_vm.c | 4 ++--
> 3 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c
> b/drivers/gpu/drm/imagination/pvr_mmu.c
> index 23261d9ad3fd..62eae7fcd5a2 100644
> --- a/drivers/gpu/drm/imagination/pvr_mmu.c
> +++ b/drivers/gpu/drm/imagination/pvr_mmu.c
> @@ -2336,6 +2336,7 @@ void pvr_mmu_op_context_destroy(struct
> pvr_mmu_op_context *op_ctx)
> * pvr_mmu_op_context_create() - Create an MMU op context.
> * @ctx: MMU context associated with owning VM context.
> * @sgt: Scatter gather table containing pages pinned for use by this
> context.
> + * @device_addr: Virtual device address at the start of the requested
> mapping.
> * @sgt_offset: Start offset of the requested device-virtual memory mapping.
> * @size: Size in bytes of the requested device-virtual memory mapping. For
> an
> * unmapping, this should be zero so that no page tables are allocated.
> @@ -2347,8 +2348,9 @@ void pvr_mmu_op_context_destroy(struct
> pvr_mmu_op_context *op_ctx)
> */
> struct pvr_mmu_op_context *
> pvr_mmu_op_context_create(struct pvr_mmu_context *ctx, struct sg_table *sgt,
> - u64 sgt_offset, u64 size)
> + u64 device_addr, u64 sgt_offset, u64 size)
> {
> + u64 start_addr = device_addr + sgt_offset;
> int err;
>
> struct pvr_mmu_op_context *op_ctx = kzalloc_obj(*op_ctx);
> @@ -2364,16 +2366,16 @@ pvr_mmu_op_context_create(struct pvr_mmu_context
> *ctx, struct sg_table *sgt,
> if (size) {
> /*
> * The number of page table objects we need to prealloc is
> - * indicated by the mapping size, start offset and the sizes
> + * indicated by the mapping size, start address and the sizes
> * of the areas mapped per PT or PD. The range calculation is
> * identical to that for the index into a table for a device
> * address, so we reuse those functions here.
> */
> - const u32 l1_start_idx = pvr_page_table_l2_idx(sgt_offset);
> - const u32 l1_end_idx = pvr_page_table_l2_idx(sgt_offset + size);
> + 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(sgt_offset);
> - const u32 l0_end_idx = pvr_page_table_l1_idx(sgt_offset + size);
> + 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;
>
> /*
> diff --git a/drivers/gpu/drm/imagination/pvr_mmu.h
> b/drivers/gpu/drm/imagination/pvr_mmu.h
> index a8ecd460168d..2c02d61ba0a2 100644
> --- a/drivers/gpu/drm/imagination/pvr_mmu.h
> +++ b/drivers/gpu/drm/imagination/pvr_mmu.h
> @@ -99,7 +99,7 @@ dma_addr_t pvr_mmu_get_root_table_dma_addr(struct
> pvr_mmu_context *ctx);
> void pvr_mmu_op_context_destroy(struct pvr_mmu_op_context *op_ctx);
> struct pvr_mmu_op_context *
> pvr_mmu_op_context_create(struct pvr_mmu_context *ctx,
> - struct sg_table *sgt, u64 sgt_offset, u64 size);
> + struct sg_table *sgt, u64 device_addr, u64
> sgt_offset, u64 size);
>
> int pvr_mmu_map(struct pvr_mmu_op_context *op_ctx, u64 size, u64 flags,
> u64 device_addr);
> diff --git a/drivers/gpu/drm/imagination/pvr_vm.c
> b/drivers/gpu/drm/imagination/pvr_vm.c
> index 396d349fb6ce..867a4a44958a 100644
> --- a/drivers/gpu/drm/imagination/pvr_vm.c
> +++ b/drivers/gpu/drm/imagination/pvr_vm.c
> @@ -276,7 +276,7 @@ pvr_vm_bind_op_map_init(struct pvr_vm_bind_op *bind_op,
> goto err_bind_op_fini;
>
> bind_op->mmu_op_ctx =
> - pvr_mmu_op_context_create(vm_ctx->mmu_ctx, sgt, offset, size);
> + pvr_mmu_op_context_create(vm_ctx->mmu_ctx, sgt, device_addr,
> offset, size);
> err = PTR_ERR_OR_ZERO(bind_op->mmu_op_ctx);
> if (err) {
> bind_op->mmu_op_ctx = NULL;
> @@ -318,7 +318,7 @@ pvr_vm_bind_op_unmap_init(struct pvr_vm_bind_op *bind_op,
> }
>
> bind_op->mmu_op_ctx =
> - pvr_mmu_op_context_create(vm_ctx->mmu_ctx, NULL, 0, 0);
> + pvr_mmu_op_context_create(vm_ctx->mmu_ctx, NULL, device_addr,
> 0, 0);
> err = PTR_ERR_OR_ZERO(bind_op->mmu_op_ctx);
> if (err) {
> bind_op->mmu_op_ctx = NULL;
>
