[PATCH] drm/i915/gem: Improve pfn calculation readability in vm_fault_gtt()

[Cavitt, Jonathan]

-----Original Message-----
From: Andi Shyti <andi.shyti at linux.intel.com> 
Sent: Wednesday, August 7, 2024 3:46 AM
To: intel-gfx <intel-gfx at lists.freedesktop.org>; dri-devel <dri-devel at lists.freedesktop.org>
Cc: Niemiec, Krzysztof <krzysztof.niemiec at intel.com>; Andi Shyti <andi.shyti at linux.intel.com>; Cavitt, Jonathan <jonathan.cavitt at intel.com>
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 at intel.com>
> Signed-off-by: Andi Shyti <andi.shyti at linux.intel.com>

Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
-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
> 
>