Re: [PATCH v3 11/14] drm/panthor: Add the driver frontend block
On Wed, 20 Dec 2023 16:24:17 + Liviu Dudau wrote: > > +/** > > + * panthor_get_uobj_array() - Copy a user object array into a kernel > > accessible object array. > > + * @in: The object array to copy. > > + * @min_stride: Minimum array stride. > > + * @obj_size: Kernel object size. > > + * > > + * Helper automating user -> kernel object copies. > > + * > > + * Don't use this function directly, use PANTHOR_UOBJ_ARRAY_GET() instead. > > > > Looks like the macro is called PANTHOR_UOBJ_GET_ARRAY(). Will fix. Thanks! Boris
Re: [PATCH v3 11/14] drm/panthor: Add the driver frontend block
On Mon, Dec 04, 2023 at 06:33:04PM +0100, Boris Brezillon wrote:
> This is the last piece missing to expose the driver to the outside
> world.
>
> This is basically a wrapper between the ioctls and the other logical
> blocks.
>
> v3:
> - Add acks for the MIT/GPL2 relicensing
> - Fix 32-bit support
> - Account for panthor_vm and panthor_sched changes
> - Simplify the resv preparation/update logic
> - Use a linked list rather than xarray for list of signals.
> - Simplify panthor_get_uobj_array by returning the newly allocated
> array.
> - Drop the "DOC" for job submission helpers and move the relevant
> comments to panthor_ioctl_group_submit().
> - Add helpers sync_op_is_signal()/sync_op_is_wait().
> - Simplify return type of panthor_submit_ctx_add_sync_signal() and
> panthor_submit_ctx_get_sync_signal().
> - Drop WARN_ON from panthor_submit_ctx_add_job().
> - Fix typos in comments.
>
> Signed-off-by: Boris Brezillon
> Signed-off-by: Steven Price
> Acked-by: Steven Price # MIT+GPL2 relicensing,Arm
> Acked-by: Grant Likely # MIT+GPL2 relicensing,Linaro
> Acked-by: Boris Brezillon # MIT+GPL2
> relicensing,Collabora
Hello,
Just some small name mismatch spotted here.
Otherwise,
Reviewed-by: Liviu Dudau
> ---
> drivers/gpu/drm/panthor/panthor_drv.c | 1454 +
> 1 file changed, 1454 insertions(+)
> create mode 100644 drivers/gpu/drm/panthor/panthor_drv.c
>
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c
> b/drivers/gpu/drm/panthor/panthor_drv.c
> new file mode 100644
> index ..9447a4e90018
> --- /dev/null
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -0,0 +1,1454 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +/* Copyright 2018 Marty E. Plummer */
> +/* Copyright 2019 Linaro, Ltd., Rob Herring */
> +/* Copyright 2019 Collabora ltd. */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "panthor_sched.h"
> +#include "panthor_device.h"
> +#include "panthor_gem.h"
> +#include "panthor_heap.h"
> +#include "panthor_fw.h"
> +#include "panthor_mmu.h"
> +#include "panthor_gpu.h"
> +#include "panthor_regs.h"
> +
> +/**
> + * DOC: user <-> kernel object copy helpers.
> + */
> +
> +/**
> + * panthor_set_uobj() - Copy kernel object to user object.
> + * @usr_ptr: Users pointer.
> + * @usr_size: Size of the user object.
> + * @min_size: Minimum size for this object.
> + * @kern_size: Size of the kernel object.
> + * @in: Address of the kernel object to copy.
> + *
> + * Helper automating kernel -> user object copies.
> + *
> + * Don't use this function directly, use PANTHOR_UOBJ_SET() instead.
> + *
> + * Return: 0 on success, a negative error code otherwise.
> + */
> +static int
> +panthor_set_uobj(u64 usr_ptr, u32 usr_size, u32 min_size, u32 kern_size,
> const void *in)
> +{
> + /* User size shouldn't be smaller than the minimal object size. */
> + if (usr_size < min_size)
> + return -EINVAL;
> +
> + if (copy_to_user(u64_to_user_ptr(usr_ptr), in, min_t(u32, usr_size,
> kern_size)))
> + return -EFAULT;
> +
> + /* When the kernel object is smaller than the user object, we fill the
> gap with
> + * zeros.
> + */
> + if (usr_size > kern_size &&
> + clear_user(u64_to_user_ptr(usr_ptr + kern_size), usr_size -
> kern_size)) {
> + return -EFAULT;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * panthor_get_uobj_array() - Copy a user object array into a kernel
> accessible object array.
> + * @in: The object array to copy.
> + * @min_stride: Minimum array stride.
> + * @obj_size: Kernel object size.
> + *
> + * Helper automating user -> kernel object copies.
> + *
> + * Don't use this function directly, use PANTHOR_UOBJ_ARRAY_GET() instead.
Looks like the macro is called PANTHOR_UOBJ_GET_ARRAY().
Best regards,
Liviu
> + *
> + * Return: newly allocated object array or an ERR_PTR on error.
> + */
> +static void *
> +panthor_get_uobj_array(const struct drm_panthor_obj_array *in, u32
> min_stride,
> +u32 obj_size)
> +{
> + int ret = 0;
> + void *out_alloc;
> +
> + /* User stride must be at least the minimum object size, otherwise it
> might
> + * lack useful information.
> + */
> + if (in->stride < min_stride)
> + return ERR_PTR(-EINVAL);
> +
> + if (!in->count)
> + return NULL;
> +
> + out_alloc = kvmalloc_array(in->count, obj_size, GFP_KERNEL);
> + if (!out_alloc)
> + return ERR_PTR(-ENOMEM);
> +
> + if (obj_size == in->stride) {
> + /* Fast path when user/kernel have the same uAPI header
> version. */
> + if (copy_from_user(out_alloc, u64_to_user_ptr(in->array),
> +(unsigned long)obj_size * in->count))
> + ret = -EFAULT;
> + } else {
> + void
Re: [PATCH v3 11/14] drm/panthor: Add the driver frontend block
On 04/12/2023 17:33, Boris Brezillon wrote:
> This is the last piece missing to expose the driver to the outside
> world.
>
> This is basically a wrapper between the ioctls and the other logical
> blocks.
>
> v3:
> - Add acks for the MIT/GPL2 relicensing
> - Fix 32-bit support
> - Account for panthor_vm and panthor_sched changes
> - Simplify the resv preparation/update logic
> - Use a linked list rather than xarray for list of signals.
> - Simplify panthor_get_uobj_array by returning the newly allocated
> array.
> - Drop the "DOC" for job submission helpers and move the relevant
> comments to panthor_ioctl_group_submit().
> - Add helpers sync_op_is_signal()/sync_op_is_wait().
> - Simplify return type of panthor_submit_ctx_add_sync_signal() and
> panthor_submit_ctx_get_sync_signal().
> - Drop WARN_ON from panthor_submit_ctx_add_job().
> - Fix typos in comments.
>
> Signed-off-by: Boris Brezillon
> Signed-off-by: Steven Price
> Acked-by: Steven Price # MIT+GPL2 relicensing,Arm
> Acked-by: Grant Likely # MIT+GPL2 relicensing,Linaro
> Acked-by: Boris Brezillon # MIT+GPL2
> relicensing,Collabora
A typo and I think the cleanup in panthor_init() is backwards, but with
that fixed:
Reviewed-by: Steven Price
> ---
> drivers/gpu/drm/panthor/panthor_drv.c | 1454 +
> 1 file changed, 1454 insertions(+)
> create mode 100644 drivers/gpu/drm/panthor/panthor_drv.c
>
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c
> b/drivers/gpu/drm/panthor/panthor_drv.c
> new file mode 100644
> index ..9447a4e90018
> --- /dev/null
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> +
> +/**
> + * panthor_submit_ctx_push_jobs() - Push jobs to their scheduling entities.
> + * @ctx: Submit context.
> + * @upd_resvs: Callback used to update reservation objects that were
> previously
> + * preapred.
NIT: s/preapred/prepared/
> + */
> +static void
> +panthor_submit_ctx_push_jobs(struct panthor_submit_ctx *ctx,
> + void (*upd_resvs)(struct drm_exec *, struct
> drm_sched_job *))
> +{
> + for (u32 i = 0; i < ctx->job_count; i++) {
> + upd_resvs(>exec, ctx->jobs[i].job);
> + drm_sched_entity_push_job(ctx->jobs[i].job);
> +
> + /* Job is owned by the scheduler now. */
> + ctx->jobs[i].job = NULL;
> + }
> +
> + panthor_submit_ctx_push_fences(ctx);
> +}
> +
> +/*
> + * Workqueue used to cleanup stuff.
> + *
> + * We create a dedicated workqueue so we can drain on unplug and
> + * make sure all resources are freed before the module is unloaded.
> + */
> +struct workqueue_struct *panthor_cleanup_wq;
> +
> +static int __init panthor_init(void)
> +{
> + int ret;
> +
> + ret = panthor_mmu_pt_cache_init();
> + if (ret)
> + return ret;
> +
> + panthor_cleanup_wq = alloc_workqueue("panthor-cleanup", WQ_UNBOUND, 0);
> + if (!panthor_cleanup_wq) {
> + pr_err("panthor: Failed to allocate the workqueues");
> + ret = -ENOMEM;
> + goto err_mmu_pt_cache_fini;
> + }
> +
> + ret = platform_driver_register(_driver);
> + if (ret)
> + goto err_destroy_cleanup_wq;
Here we skip the call to panthor_mmu_pt_cache_fini() which doesn't look
right. I think the cleanup below is just backwards.
Steve
> +
> + return ret;
> +
> +err_mmu_pt_cache_fini:
> + panthor_mmu_pt_cache_fini();
> +
> +err_destroy_cleanup_wq:
> + destroy_workqueue(panthor_cleanup_wq);
> + return ret;
> +}
> +module_init(panthor_init);
> +
> +static void __exit panthor_exit(void)
> +{
> + platform_driver_unregister(_driver);
> + destroy_workqueue(panthor_cleanup_wq);
> + panthor_mmu_pt_cache_fini();
> +}
> +module_exit(panthor_exit);
> +
> +MODULE_AUTHOR("Panthor Project Developers");
> +MODULE_DESCRIPTION("Panthor DRM Driver");
> +MODULE_LICENSE("Dual MIT/GPL");
[PATCH v3 11/14] drm/panthor: Add the driver frontend block
This is the last piece missing to expose the driver to the outside
world.
This is basically a wrapper between the ioctls and the other logical
blocks.
v3:
- Add acks for the MIT/GPL2 relicensing
- Fix 32-bit support
- Account for panthor_vm and panthor_sched changes
- Simplify the resv preparation/update logic
- Use a linked list rather than xarray for list of signals.
- Simplify panthor_get_uobj_array by returning the newly allocated
array.
- Drop the "DOC" for job submission helpers and move the relevant
comments to panthor_ioctl_group_submit().
- Add helpers sync_op_is_signal()/sync_op_is_wait().
- Simplify return type of panthor_submit_ctx_add_sync_signal() and
panthor_submit_ctx_get_sync_signal().
- Drop WARN_ON from panthor_submit_ctx_add_job().
- Fix typos in comments.
Signed-off-by: Boris Brezillon
Signed-off-by: Steven Price
Acked-by: Steven Price # MIT+GPL2 relicensing,Arm
Acked-by: Grant Likely # MIT+GPL2 relicensing,Linaro
Acked-by: Boris Brezillon # MIT+GPL2
relicensing,Collabora
---
drivers/gpu/drm/panthor/panthor_drv.c | 1454 +
1 file changed, 1454 insertions(+)
create mode 100644 drivers/gpu/drm/panthor/panthor_drv.c
diff --git a/drivers/gpu/drm/panthor/panthor_drv.c
b/drivers/gpu/drm/panthor/panthor_drv.c
new file mode 100644
index ..9447a4e90018
--- /dev/null
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -0,0 +1,1454 @@
+// SPDX-License-Identifier: GPL-2.0 or MIT
+/* Copyright 2018 Marty E. Plummer */
+/* Copyright 2019 Linaro, Ltd., Rob Herring */
+/* Copyright 2019 Collabora ltd. */
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "panthor_sched.h"
+#include "panthor_device.h"
+#include "panthor_gem.h"
+#include "panthor_heap.h"
+#include "panthor_fw.h"
+#include "panthor_mmu.h"
+#include "panthor_gpu.h"
+#include "panthor_regs.h"
+
+/**
+ * DOC: user <-> kernel object copy helpers.
+ */
+
+/**
+ * panthor_set_uobj() - Copy kernel object to user object.
+ * @usr_ptr: Users pointer.
+ * @usr_size: Size of the user object.
+ * @min_size: Minimum size for this object.
+ * @kern_size: Size of the kernel object.
+ * @in: Address of the kernel object to copy.
+ *
+ * Helper automating kernel -> user object copies.
+ *
+ * Don't use this function directly, use PANTHOR_UOBJ_SET() instead.
+ *
+ * Return: 0 on success, a negative error code otherwise.
+ */
+static int
+panthor_set_uobj(u64 usr_ptr, u32 usr_size, u32 min_size, u32 kern_size, const
void *in)
+{
+ /* User size shouldn't be smaller than the minimal object size. */
+ if (usr_size < min_size)
+ return -EINVAL;
+
+ if (copy_to_user(u64_to_user_ptr(usr_ptr), in, min_t(u32, usr_size,
kern_size)))
+ return -EFAULT;
+
+ /* When the kernel object is smaller than the user object, we fill the
gap with
+* zeros.
+*/
+ if (usr_size > kern_size &&
+ clear_user(u64_to_user_ptr(usr_ptr + kern_size), usr_size -
kern_size)) {
+ return -EFAULT;
+ }
+
+ return 0;
+}
+
+/**
+ * panthor_get_uobj_array() - Copy a user object array into a kernel
accessible object array.
+ * @in: The object array to copy.
+ * @min_stride: Minimum array stride.
+ * @obj_size: Kernel object size.
+ *
+ * Helper automating user -> kernel object copies.
+ *
+ * Don't use this function directly, use PANTHOR_UOBJ_ARRAY_GET() instead.
+ *
+ * Return: newly allocated object array or an ERR_PTR on error.
+ */
+static void *
+panthor_get_uobj_array(const struct drm_panthor_obj_array *in, u32 min_stride,
+ u32 obj_size)
+{
+ int ret = 0;
+ void *out_alloc;
+
+ /* User stride must be at least the minimum object size, otherwise it
might
+* lack useful information.
+*/
+ if (in->stride < min_stride)
+ return ERR_PTR(-EINVAL);
+
+ if (!in->count)
+ return NULL;
+
+ out_alloc = kvmalloc_array(in->count, obj_size, GFP_KERNEL);
+ if (!out_alloc)
+ return ERR_PTR(-ENOMEM);
+
+ if (obj_size == in->stride) {
+ /* Fast path when user/kernel have the same uAPI header
version. */
+ if (copy_from_user(out_alloc, u64_to_user_ptr(in->array),
+ (unsigned long)obj_size * in->count))
+ ret = -EFAULT;
+ } else {
+ void __user *in_ptr = u64_to_user_ptr(in->array);
+ void *out_ptr = out_alloc;
+
+ /* If the sizes differ, we need to copy elements one by one. */
+ for (u32 i = 0; i < in->count; i++) {
+ ret = copy_struct_from_user(out_ptr, obj_size, in_ptr,
in->stride);
+ if (ret)
+ break;
+
+ out_ptr += obj_size;
+ in_ptr += in->stride;
+
