On 10/28/2025 9:39 PM, Jeff Hugo wrote:
> On 10/28/2025 1:06 AM, Karol Wachowski wrote:
>> From: Jacek Lawrynowicz <[email protected]>
>>
>> Add support for creating buffer objects from user pointers
>> in the Intel NPU driver.
>
> This feels redundant. $SUBJECT already mentions adding the support,
> and in the Intel NPU driver is a given.
Removed.
>
>> Introduce a new ioctl `drm_ivpu_bo_create_from_userptr` that allows
>> users to create a GEM buffer object from a user pointer to a memory
>> region.
>> The user pointer must be page-aligned and the memory region must remain
>> valid for the lifetime of the buffer object.
>
> This seems good, but perhaps incomplete. Why do this? What benifit
> would this new IOCTL give the user? Increases performance when the
> user has a specific usecase?
I have added paragraph describing that this allows to limit memory used
and simplifies NPU usage for user applications allowing to mmap files
directly into NPU's buffer objects.
>
>> diff --git a/drivers/accel/ivpu/ivpu_gem_userptr.c
>> b/drivers/accel/ivpu/ivpu_gem_userptr.c
>> new file mode 100644
>> index 000000000000..c5b64e0f2b13
>> --- /dev/null
>> +++ b/drivers/accel/ivpu/ivpu_gem_userptr.c
>> @@ -0,0 +1,207 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2020-2025 Intel Corporation
>> + */
>> +
>> +#include <linux/dma-buf.h>
>> +#include <linux/err.h>
>> +#include <linux/highmem.h>
>> +#include <linux/mm.h>
>> +#include <linux/mman.h>
>> +#include <linux/scatterlist.h>
>> +#include <linux/slab.h>
>> +#include <linux/capability.h>
>> +
>> +#include <drm/drm_device.h>
>> +#include <drm/drm_file.h>
>> +#include <drm/drm_gem.h>
>> +
>> +#include "ivpu_drv.h"
>> +#include "ivpu_gem.h"
>> +
>> +#define IVPU_BO_USERPTR_MAX_SIZE (4ULL * SZ_1G)
>
> Is this not the same as SZ_4G?
After consideration I have decided to remove this limitation. So I have
removed this define and check completely.
Maximum memory usage for both userptr and generic buffer object is
limited by driver's defined ranges anyway.
>
>> +/**
>> + * struct drm_ivpu_bo_create_from_userptr - Create dma-buf from user
>> pointer
>> + *
>> + * Create a GEM buffer object from a user pointer to a memory region.
>> + */
>> +struct drm_ivpu_bo_create_from_userptr {
>> + /** @user_ptr: User pointer to memory region (must be page
>> aligned) */
>> + __u64 user_ptr;
>> +
>> + /** @size: Size of the memory region in bytes (must be page
>> aligned) */
>
> Mention max size?
As above, I have removed the limitation.
>
>> + __u64 size;
>> +
>> + /**
>> + * @flags:
>> + *
>> + * Supported flags:
>> + *
>> + * %DRM_IVPU_BO_HIGH_MEM:
>> + *
>> + * Allocate VPU address from >4GB range.
>> + *
>> + * %DRM_IVPU_BO_DMA_MEM:
>> + *
>> + * Allocate from DMA memory range accessible by hardware DMA.
>> + *
>> + * %DRM_IVPU_BO_READ_ONLY:
>> + *
>> + * Allocate as a read-only buffer object.
>> + */
>> + __u32 flags;
>> +
>> + /** @handle: Returned GEM object handle */
>> + __u32 handle;
>> +
>> + /** @vpu_addr: Returned VPU virtual address */
>> + __u64 vpu_addr;
>> +};
>> +
>> /**
>> * struct drm_ivpu_bo_info - Query buffer object info
>> */
>
Thank you for the review, all above are addressed in v2.
-Karol
