[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1a206b08485a1ebea0aeff5306c0f5b6eb7c315d.camel@imgtec.com>
Date: Wed, 16 Oct 2024 13:31:28 +0000
From: Frank Binns <Frank.Binns@...tec.com>
To: "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
Matt
Coster <Matt.Coster@...tec.com>
CC: Brendan King <Brendan.King@...tec.com>,
"daniel@...ll.ch"
<daniel@...ll.ch>,
"maarten.lankhorst@...ux.intel.com"
<maarten.lankhorst@...ux.intel.com>,
"mripard@...nel.org"
<mripard@...nel.org>,
"airlied@...il.com" <airlied@...il.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"tzimmermann@...e.de" <tzimmermann@...e.de>
Subject: Re: [PATCH v2 1/2] drm/imagination: Add a per-file PVR context list
On Mon, 2024-10-14 at 14:22 +0000, Matt Coster wrote:
> From: Brendan King <brendan.king@...tec.com>
>
> This adds a linked list of VM contexts which is needed for the next patch
> to be able to correctly track VM contexts for destruction on file close.
>
> It is only safe for VM contexts to be removed from the list and destroyed
> when not in interrupt context.
>
Reviewed-by: Frank Binns <frank.binns@...tec.com>
> Signed-off-by: Brendan King <brendan.king@...tec.com>
> Signed-off-by: Matt Coster <matt.coster@...tec.com>
> ---
> Changes in v1 -> v2:
> - Add justification to the commit message so it stands on its own (e.g.
> during bisect)
> ---
> drivers/gpu/drm/imagination/pvr_context.c | 14 ++++++++++++++
> drivers/gpu/drm/imagination/pvr_context.h | 3 +++
> drivers/gpu/drm/imagination/pvr_device.h | 10 ++++++++++
> drivers/gpu/drm/imagination/pvr_drv.c | 3 +++
> 4 files changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/imagination/pvr_context.c b/drivers/gpu/drm/imagination/pvr_context.c
> index eded5e955cc0..255c93582734 100644
> --- a/drivers/gpu/drm/imagination/pvr_context.c
> +++ b/drivers/gpu/drm/imagination/pvr_context.c
> @@ -17,10 +17,14 @@
>
> #include <drm/drm_auth.h>
> #include <drm/drm_managed.h>
> +
> +#include <linux/bug.h>
> #include <linux/errno.h>
> #include <linux/kernel.h>
> +#include <linux/list.h>
> #include <linux/sched.h>
> #include <linux/slab.h>
> +#include <linux/spinlock.h>
> #include <linux/string.h>
> #include <linux/types.h>
> #include <linux/xarray.h>
> @@ -354,6 +358,10 @@ int pvr_context_create(struct pvr_file *pvr_file, struct drm_pvr_ioctl_create_co
> return err;
> }
>
> + spin_lock(&pvr_dev->ctx_list_lock);
> + list_add_tail(&ctx->file_link, &pvr_file->contexts);
> + spin_unlock(&pvr_dev->ctx_list_lock);
> +
> return 0;
>
> err_destroy_fw_obj:
> @@ -380,6 +388,11 @@ pvr_context_release(struct kref *ref_count)
> container_of(ref_count, struct pvr_context, ref_count);
> struct pvr_device *pvr_dev = ctx->pvr_dev;
>
> + WARN_ON(in_interrupt());
> + spin_lock(&pvr_dev->ctx_list_lock);
> + list_del(&ctx->file_link);
> + spin_unlock(&pvr_dev->ctx_list_lock);
> +
> xa_erase(&pvr_dev->ctx_ids, ctx->ctx_id);
> pvr_context_destroy_queues(ctx);
> pvr_fw_object_destroy(ctx->fw_obj);
> @@ -451,6 +464,7 @@ void pvr_destroy_contexts_for_file(struct pvr_file *pvr_file)
> void pvr_context_device_init(struct pvr_device *pvr_dev)
> {
> xa_init_flags(&pvr_dev->ctx_ids, XA_FLAGS_ALLOC1);
> + spin_lock_init(&pvr_dev->ctx_list_lock);
> }
>
> /**
> diff --git a/drivers/gpu/drm/imagination/pvr_context.h b/drivers/gpu/drm/imagination/pvr_context.h
> index 0c7b97dfa6ba..a5b0a82a54a1 100644
> --- a/drivers/gpu/drm/imagination/pvr_context.h
> +++ b/drivers/gpu/drm/imagination/pvr_context.h
> @@ -85,6 +85,9 @@ struct pvr_context {
> /** @compute: Transfer queue. */
> struct pvr_queue *transfer;
> } queues;
> +
> + /** @file_link: pvr_file PVR context list link. */
> + struct list_head file_link;
> };
>
> static __always_inline struct pvr_queue *
> diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
> index b574e23d484b..6d0dfacb677b 100644
> --- a/drivers/gpu/drm/imagination/pvr_device.h
> +++ b/drivers/gpu/drm/imagination/pvr_device.h
> @@ -23,6 +23,7 @@
> #include <linux/kernel.h>
> #include <linux/math.h>
> #include <linux/mutex.h>
> +#include <linux/spinlock_types.h>
> #include <linux/timer.h>
> #include <linux/types.h>
> #include <linux/wait.h>
> @@ -293,6 +294,12 @@ struct pvr_device {
>
> /** @sched_wq: Workqueue for schedulers. */
> struct workqueue_struct *sched_wq;
> +
> + /**
> + * @ctx_list_lock: Lock to be held when accessing the context list in
> + * struct pvr_file.
> + */
> + spinlock_t ctx_list_lock;
> };
>
> /**
> @@ -344,6 +351,9 @@ struct pvr_file {
> * This array is used to allocate handles returned to userspace.
> */
> struct xarray vm_ctx_handles;
> +
> + /** @contexts: PVR context list. */
> + struct list_head contexts;
> };
>
> /**
> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
> index 1a0cb7aa9cea..fb17196e05f4 100644
> --- a/drivers/gpu/drm/imagination/pvr_drv.c
> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
> @@ -28,6 +28,7 @@
> #include <linux/export.h>
> #include <linux/fs.h>
> #include <linux/kernel.h>
> +#include <linux/list.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> @@ -1326,6 +1327,8 @@ pvr_drm_driver_open(struct drm_device *drm_dev, struct drm_file *file)
> */
> pvr_file->pvr_dev = pvr_dev;
>
> + INIT_LIST_HEAD(&pvr_file->contexts);
> +
> xa_init_flags(&pvr_file->ctx_handles, XA_FLAGS_ALLOC1);
> xa_init_flags(&pvr_file->free_list_handles, XA_FLAGS_ALLOC1);
> xa_init_flags(&pvr_file->hwrt_handles, XA_FLAGS_ALLOC1);
Powered by blists - more mailing lists