[Intel-xe] [PATCH v3] drm/xe/bo: Return object bo size on create

[Thomas Hellström]

Hi!

On 6/26/23 20:06, Zbigniew Kempczyński wrote:
> On Mon, Jun 26, 2023 at 06:16:06PM +0200, Thomas Hellström wrote:
>> On 6/26/23 17:22, Zbigniew Kempczyński wrote:
>>> On Mon, Jun 26, 2023 at 04:55:29PM +0200, Thomas Hellström wrote:
>>>> Hi, Zbigniew,
>>>>
>>>> On 6/26/23 16:42, Zbigniew Kempczyński wrote:
>>>>> Driver may alter bo size requested by the user. Return real object
>>>>> size to make userspace aware how to arrange vm bindings.
>>>> I think for Xe VM_BIND, the space needed for a bo with given size must be
>>>>
>>>> ALIGN_TO(bo->size, xe_get_default_alignment())
>>>>
>>>> (xe_get_default_alignment() being an igt xe lib function).
>>> You mean I can't glue 4K system object with 64K one after another,
>>> for example:
>>>
>>> * bo on system: offset = 0x1000, size 0x1000
>>> * bo on vram:   offset = 0x2000, size 0x10000
>>>
>>> This won't work as vram object should be aligned on vm to 0x10000?
>> Yes that won't work, since a VRAM-capable bo needs 64K alignment.
>>
>> Although the explanation above ignores that system-only bo objects appears
>> to always allow 4K offset and size, which of course can be discussed.
>>
>> So with the current uAPI, it appears the bind alignment and size could
>> always be calculated using:
>>
>> if (bo_has_VRAM_placement() && xe_get_default_alignment() > 4K) {
>>     bind_size = ALIGN_TO(bo->size, xe_get_default_alignment());
>>     offset_alignment = xe_get_default_alignment();
>> } else {
>>     bind_size = bo->size;
>>     offset_alignment = 4K;
>> }
> offset_alignment = xe_get_default_alignment() is enough, as for
> integrated we got only one region and min_page_size is 4K. For discrete
> system objects just won't be adjacent (if I'm not wrong this lead to
> bigger ppgtt).
>
>> What I'm a bit afraid of is that if we return a size different from
>> bo->size, then UMD might start using that extra size for data, while it
>> really shouldn't. Would it be possible to return an alignment instead that
>> needs to be used by UMD for bind offset and bind size?
> Technically we got those alignments (for regions) - they can be get from
> min_page_size for some region. Problem I see is when user will create
> an object with flags = SYSTEM | VRAM. How to properly do vmbind in case?
> I mean sth like snippet below:
>
> size = 0x2000;
> bo = xe_bo_create_flags(fd, size, SYSTEM | VRAM);
>
> If I don't know on which region this object is created how can I prepare
> vm-bind range properly? May I use 0x2000 as size for object created
> in vram or this gpu mapping should be aligned to 64K?

Since KMD can in theory even move the object behind the back of UMD, it 
must be bound to the greatest alignment required by any of the regions. 
So if VRAM requires 64K, then any object that includes VRAM in one of 
the placements need to be aligned to 64K.


/Thomas


>
> --
> Zbigniew
>
>> /Thomas
>>
>>> --
>>> Zbigniew
>>>
>>>> Even the padded bo->size returned with this patch is lower than the value
>>>> above. If you use that value, will the value assigned below still be needed.
>>>>
>>>> /Thomas
>>>>
>>>>> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
>>>>> Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>
>>>>> Reviewed-by: Matthew Brost <matthew.brost at intel.com>
>>>>> ---
>>>>>     drivers/gpu/drm/xe/xe_bo.c | 1 +
>>>>>     1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
>>>>> index 71a14e1f25be..55dbb374a787 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_bo.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.c
>>>>> @@ -1744,6 +1744,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, void *data,
>>>>>     		return err;
>>>>>     	args->handle = handle;
>>>>> +	args->size = bo->size;
>>>>>     	return 0;
>>>>>     }