Re: [PATCH] drm/i915/gem: Calculate object page offset for partial memory mapping

[Nirmoy Das <nirmoy.das@linux.intel.com>]

Hi Andi,

On 3/26/2024 12:12 PM, Andi Shyti wrote:
> Hi Nirmoy,
>
> ...
>
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>>> index a2195e28b625..57a2dda2c3cc 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>>> @@ -276,7 +276,7 @@ static vm_fault_t vm_fault_cpu(struct vm_fault *vmf)
>>>    	/* PTEs are revoked in obj->ops->put_pages() */
>>>    	err = remap_io_sg(area,
>>>    			  area->vm_start, area->vm_end - area->vm_start,
>>> -			  obj->mm.pages->sgl, iomap);
>>> +			  obj->mm.pages->sgl, 0, iomap);
>> Why don't we need partial mmap for CPU but only for GTT ?
> As far as I understood we don't. I have a version with the CPU
> offset as well in trybot[*]
>
> But without support for segmented buffer objects, I don't know
> how much this has any effect.

You confused me more :) Why segmented buffer object is needed for 
partial CPU mmap but not for GTT  ?

 From high level,  GTT and CPU both should support partial mmap unless I 
missing something here.

>
>> Sounds like this also need to be cover by a IGT tests.
> Yes, I it does need some igt work, working on it.
>
>> Don't we need "Fixes" tag for this?
> Why should we? I'm not fixing anything here,

If userspace  expects partial mmap to work then this is a bug/gap in 
i915 so we need to

backport this as far as possible. Need some information about the 
requirement about  why we need this patch suddenly?


Regards,

Nirmoy

>   I'm just
> recalculating the mapping not starting from the beginning of the
> scatter page.
>
> Andi
>
> [*] https://patchwork.freedesktop.org/patch/584474/?series=131539&rev=2