-----Original Message-----
From: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
Sent: Wednesday, August 7, 2024 3:46 AM
To: intel-gfx <intel-gfx@xxxxxxxxxxxxxxxxxxxxx>; dri-devel <dri-devel@xxxxxxxxxxxxxxxxxxxxx>
Cc: Niemiec, Krzysztof <krzysztof.niemiec@xxxxxxxxx>; Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>; Cavitt, Jonathan <jonathan.cavitt@xxxxxxxxx>
Subject: [PATCH] drm/i915/gem: Improve pfn calculation readability in vm_fault_gtt()
>
> By moving the pfn calculation to the set_address_limits()
> function we improve code readability. This way,
> set_address_limits() is responsible for calculating all memory
> mapping paramenters: "start", "end" and "pfn".
>
> This suggestion from Jonathan was made during the review of
> commit 8bdd9ef7e9b1 ("drm/i915/gem: Fix Virtual Memory mapping
> boundaries calculation"), which I liked, but it got lost on the
> way.
>
> Suggested-by: Jonathan Cavitt <Jonathan.cavitt@xxxxxxxxx>
> Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
Reviewed-by: Jonathan Cavitt <jonathan.cavitt@xxxxxxxxx>
-Jonathan Cavitt
> ---
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index cac6d4184506..e9b2424156f0 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -293,8 +293,10 @@ static vm_fault_t vm_fault_cpu(struct vm_fault *vmf)
> static void set_address_limits(struct vm_area_struct *area,
> struct i915_vma *vma,
> unsigned long obj_offset,
> + resource_size_t gmadr_start,
> unsigned long *start_vaddr,
> - unsigned long *end_vaddr)
> + unsigned long *end_vaddr,
> + unsigned long *pfn)
> {
> unsigned long vm_start, vm_end, vma_size; /* user's memory parameters */
> long start, end; /* memory boundaries */
> @@ -323,6 +325,10 @@ static void set_address_limits(struct vm_area_struct *area,
> /* Let's move back into the "<< PAGE_SHIFT" domain */
> *start_vaddr = (unsigned long)start << PAGE_SHIFT;
> *end_vaddr = (unsigned long)end << PAGE_SHIFT;
> +
> + *pfn = (gmadr_start + i915_ggtt_offset(vma)) >> PAGE_SHIFT;
> + *pfn += (*start_vaddr - area->vm_start) >> PAGE_SHIFT;
> + *pfn += obj_offset - vma->gtt_view.partial.offset;
> }
>
> static vm_fault_t vm_fault_gtt(struct vm_fault *vmf)
> @@ -441,11 +447,13 @@ static vm_fault_t vm_fault_gtt(struct vm_fault *vmf)
> if (ret)
> goto err_unpin;
>
> - set_address_limits(area, vma, obj_offset, &start, &end);
> -
> - pfn = (ggtt->gmadr.start + i915_ggtt_offset(vma)) >> PAGE_SHIFT;
> - pfn += (start - area->vm_start) >> PAGE_SHIFT;
> - pfn += obj_offset - vma->gtt_view.partial.offset;
> + /*
> + * Dump all the necessary parameters in this function to perform the
> + * arithmetic calculation for the virtual address start and end and
> + * the PFN (Page Frame Number).
> + */
> + set_address_limits(area, vma, obj_offset, ggtt->gmadr.start,
> + &start, &end, &pfn);
>
> /* Finally, remap it using the new GTT offset */
> ret = remap_io_mapping(area, start, pfn, end - start, &ggtt->iomap);
> --
> 2.45.2
>
>
