mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Brajesh Gupta <Brajesh.Gupta@imgtec.com>
To: Luigi Santivetti <Luigi.Santivetti@imgtec.com>,
	"tzimmermann@suse.de" <tzimmermann@suse.de>,
	"simona@ffwll.ch" <simona@ffwll.ch>,
	"airlied@gmail.com" <airlied@gmail.com>,
	Alessio Belle <Alessio.Belle@imgtec.com>,
	"maarten.lankhorst@linux.intel.com"
	<maarten.lankhorst@linux.intel.com>,
	"mripard@kernel.org" <mripard@kernel.org>,
	"gye976@gmail.com" <gye976@gmail.com>
Cc: "dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"imagination@lists.freedesktop.org"
	<imagination@lists.freedesktop.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/3] drm/imagination: Size page table preallocation by device address
Date: Mon, 28 Sep 2026 09:03:42 +0000	[thread overview]
Message-ID: <26a436bd27cc66407d776947c5f91ea7ae518600.camel@imgtec.com> (raw)
In-Reply-To: <20260927-pvr-fixes-a-v1-3-7f5b18ab989a@gmail.com>

On Sun, 2026-09-27 at 17:25 +0900, Gyeyoung Baek wrote:
Hi Gyeyoung,
> pvr_mmu_op_context_create() preallocates the page tables a mapping
> needs. How many is set by the device-virtual address being mapped, not
> by the offset into the object's pages, which it used instead.
> 
> Size the preallocation from the device address instead.
> 
This issue is recently fixed under
https://patchwork.freedesktop.org/patch/755274/. Please have a look.

Thanks,
Brajesh
> Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
> Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
> ---
>  drivers/gpu/drm/imagination/pvr_mmu.c | 27 ++++++++++++++-------------
>  drivers/gpu/drm/imagination/pvr_mmu.h |  3 ++-
>  drivers/gpu/drm/imagination/pvr_vm.c  |  5 +++--
>  3 files changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c b/drivers/gpu/drm/imagination/pvr_mmu.c
> index 3e494bfa936..dd5f6d32864 100644
> --- a/drivers/gpu/drm/imagination/pvr_mmu.c
> +++ b/drivers/gpu/drm/imagination/pvr_mmu.c
> @@ -2335,7 +2335,8 @@ 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.
> - * @sgt_offset: Start offset of the requested device-virtual memory mapping.
> + * @sgt_offset: Offset into @sgt of the start of the requested mapping.
> + * @device_addr: Device-virtual address at the start of the requested 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.
>   *
> @@ -2346,7 +2347,7 @@ 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 sgt_offset, u64 device_addr, u64 size)
>  {
>  	int err;
>  
> @@ -2362,18 +2363,18 @@ 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
> -		 * 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.
> +		 * The page tables needed are set by the device-virtual range
> +		 * being mapped: one level 1 table per 1GiB region and one
> +		 * level 0 table per 2MiB region the range touches. Tables that
> +		 * already exist are not consumed, so this is an upper bound.
>  		 */
> -		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_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_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;
>  
>  		/*
>  		 * Alloc and push page table entries until we have enough of
> diff --git a/drivers/gpu/drm/imagination/pvr_mmu.h b/drivers/gpu/drm/imagination/pvr_mmu.h
> index a8ecd460168..7088f80805e 100644
> --- a/drivers/gpu/drm/imagination/pvr_mmu.h
> +++ b/drivers/gpu/drm/imagination/pvr_mmu.h
> @@ -99,7 +99,8 @@ 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 sgt_offset,
> +			  u64 device_addr, 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 c5ae0b79fe6..7bb58ca14e4 100644
> --- a/drivers/gpu/drm/imagination/pvr_vm.c
> +++ b/drivers/gpu/drm/imagination/pvr_vm.c
> @@ -276,7 +276,8 @@ 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, offset,
> +					  device_addr, size);
>  	err = PTR_ERR_OR_ZERO(bind_op->mmu_op_ctx);
>  	if (err) {
>  		bind_op->mmu_op_ctx = NULL;
> @@ -318,7 +319,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, 0, 0, 0);
>  	err = PTR_ERR_OR_ZERO(bind_op->mmu_op_ctx);
>  	if (err) {
>  		bind_op->mmu_op_ctx = NULL;
> 


  reply	other threads:[~2026-09-28  9:04 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-27  8:25 [PATCH 0/3] drm/imagination: Fix pre-existing bugs flagged by Sashiko's review of a VM_BIND series Gyeyoung Baek
2026-09-27  8:25 ` [PATCH 1/3] drm/imagination: Fix reference and vm_bo handling in remap() Gyeyoung Baek
2026-09-28  9:10   ` Brajesh Gupta
2026-09-27  8:25 ` [PATCH 2/3] drm/imagination: Fix pvr_mmu_map_sgl() overwriting its own error Gyeyoung Baek
2026-09-28  9:01   ` Brajesh Gupta
2026-09-28  9:19     ` Gyeyoung Baek
2026-09-27  8:25 ` [PATCH 3/3] drm/imagination: Size page table preallocation by device address Gyeyoung Baek
2026-09-28  9:03   ` Brajesh Gupta [this message]
2026-10-01  4:47     ` Brajesh Gupta
2026-10-01  6:00       ` Gyeyoung Baek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=26a436bd27cc66407d776947c5f91ea7ae518600.camel@imgtec.com \
    --to=brajesh.gupta@imgtec.com \
    --cc=Alessio.Belle@imgtec.com \
    --cc=Luigi.Santivetti@imgtec.com \
    --cc=airlied@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gye976@gmail.com \
    --cc=imagination@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®