* [PATCH 01/14] accel/ivpu: Separate DB ID and CMDQ ID allocations from CMDQ allocation
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-09 8:22 ` Jacek Lawrynowicz
2025-01-07 17:32 ` [PATCH 02/14] accel/ivpu: Add API for command queue create/destroy/submit Maciej Falkowski
` (14 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel
Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Karol Wachowski,
Maciej Falkowski
From: Karol Wachowski <karol.wachowski@intel.com>
Move doorbell ID and command queue ID XArray allocations from command
queue memory allocation function. This will allows IDs allocations to be
done without the need for actual memory allocation.
Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_job.c | 88 +++++++++++++++++++++++++----------
1 file changed, 64 insertions(+), 24 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index 7149312f16e1..98e53cb38ecd 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -83,23 +83,9 @@ static struct ivpu_cmdq *ivpu_cmdq_alloc(struct ivpu_file_priv *file_priv)
if (!cmdq)
return NULL;
- ret = xa_alloc_cyclic(&vdev->db_xa, &cmdq->db_id, NULL, vdev->db_limit, &vdev->db_next,
- GFP_KERNEL);
- if (ret < 0) {
- ivpu_err(vdev, "Failed to allocate doorbell id: %d\n", ret);
- goto err_free_cmdq;
- }
-
- ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &cmdq->id, cmdq, file_priv->cmdq_limit,
- &file_priv->cmdq_id_next, GFP_KERNEL);
- if (ret < 0) {
- ivpu_err(vdev, "Failed to allocate command queue id: %d\n", ret);
- goto err_erase_db_xa;
- }
-
cmdq->mem = ivpu_bo_create_global(vdev, SZ_4K, DRM_IVPU_BO_WC | DRM_IVPU_BO_MAPPABLE);
if (!cmdq->mem)
- goto err_erase_cmdq_xa;
+ goto err_free_cmdq;
ret = ivpu_preemption_buffers_create(vdev, file_priv, cmdq);
if (ret)
@@ -107,10 +93,6 @@ static struct ivpu_cmdq *ivpu_cmdq_alloc(struct ivpu_file_priv *file_priv)
return cmdq;
-err_erase_cmdq_xa:
- xa_erase(&file_priv->cmdq_xa, cmdq->id);
-err_erase_db_xa:
- xa_erase(&vdev->db_xa, cmdq->db_id);
err_free_cmdq:
kfree(cmdq);
return NULL;
@@ -233,30 +215,88 @@ static int ivpu_cmdq_fini(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cm
return 0;
}
+static int ivpu_db_id_alloc(struct ivpu_device *vdev, u32 *db_id)
+{
+ int ret;
+ u32 id;
+
+ ret = xa_alloc_cyclic(&vdev->db_xa, &id, NULL, vdev->db_limit, &vdev->db_next, GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+
+ *db_id = id;
+ return 0;
+}
+
+static int ivpu_cmdq_id_alloc(struct ivpu_file_priv *file_priv, u32 *cmdq_id)
+{
+ int ret;
+ u32 id;
+
+ ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &id, NULL, file_priv->cmdq_limit,
+ &file_priv->cmdq_id_next, GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+
+ *cmdq_id = id;
+ return 0;
+}
+
static struct ivpu_cmdq *ivpu_cmdq_acquire(struct ivpu_file_priv *file_priv, u8 priority)
{
+ struct ivpu_device *vdev = file_priv->vdev;
struct ivpu_cmdq *cmdq;
- unsigned long cmdq_id;
+ unsigned long id;
int ret;
lockdep_assert_held(&file_priv->lock);
- xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
+ xa_for_each(&file_priv->cmdq_xa, id, cmdq)
if (cmdq->priority == priority)
break;
if (!cmdq) {
cmdq = ivpu_cmdq_alloc(file_priv);
- if (!cmdq)
+ if (!cmdq) {
+ ivpu_err(vdev, "Failed to allocate command queue\n");
return NULL;
+ }
+
+ ret = ivpu_db_id_alloc(vdev, &cmdq->db_id);
+ if (ret) {
+ ivpu_err(file_priv->vdev, "Failed to allocate doorbell ID: %d\n", ret);
+ goto err_free_cmdq;
+ }
+
+ ret = ivpu_cmdq_id_alloc(file_priv, &cmdq->id);
+ if (ret) {
+ ivpu_err(vdev, "Failed to allocate command queue ID: %d\n", ret);
+ goto err_erase_db_id;
+ }
+
cmdq->priority = priority;
+ ret = xa_err(xa_store(&file_priv->cmdq_xa, cmdq->id, cmdq, GFP_KERNEL));
+ if (ret) {
+ ivpu_err(vdev, "Failed to store command queue in cmdq_xa: %d\n", ret);
+ goto err_erase_cmdq_id;
+ }
}
ret = ivpu_cmdq_init(file_priv, cmdq, priority);
- if (ret)
- return NULL;
+ if (ret) {
+ ivpu_err(vdev, "Failed to initialize command queue: %d\n", ret);
+ goto err_free_cmdq;
+ }
return cmdq;
+
+err_erase_cmdq_id:
+ xa_erase(&file_priv->cmdq_xa, cmdq->id);
+err_erase_db_id:
+ xa_erase(&vdev->db_xa, cmdq->db_id);
+err_free_cmdq:
+ ivpu_cmdq_free(file_priv, cmdq);
+ return NULL;
}
void ivpu_cmdq_release_all_locked(struct ivpu_file_priv *file_priv)
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 01/14] accel/ivpu: Separate DB ID and CMDQ ID allocations from CMDQ allocation
2025-01-07 17:32 ` [PATCH 01/14] accel/ivpu: Separate DB ID and CMDQ ID allocations from CMDQ allocation Maciej Falkowski
@ 2025-01-09 8:22 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:22 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo, Karol Wachowski
This will allows -> This will allow
Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> From: Karol Wachowski <karol.wachowski@intel.com>
>
> Move doorbell ID and command queue ID XArray allocations from command
> queue memory allocation function. This will allows IDs allocations to be
> done without the need for actual memory allocation.
>
> Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_job.c | 88 +++++++++++++++++++++++++----------
> 1 file changed, 64 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
> index 7149312f16e1..98e53cb38ecd 100644
> --- a/drivers/accel/ivpu/ivpu_job.c
> +++ b/drivers/accel/ivpu/ivpu_job.c
> @@ -83,23 +83,9 @@ static struct ivpu_cmdq *ivpu_cmdq_alloc(struct ivpu_file_priv *file_priv)
> if (!cmdq)
> return NULL;
>
> - ret = xa_alloc_cyclic(&vdev->db_xa, &cmdq->db_id, NULL, vdev->db_limit, &vdev->db_next,
> - GFP_KERNEL);
> - if (ret < 0) {
> - ivpu_err(vdev, "Failed to allocate doorbell id: %d\n", ret);
> - goto err_free_cmdq;
> - }
> -
> - ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &cmdq->id, cmdq, file_priv->cmdq_limit,
> - &file_priv->cmdq_id_next, GFP_KERNEL);
> - if (ret < 0) {
> - ivpu_err(vdev, "Failed to allocate command queue id: %d\n", ret);
> - goto err_erase_db_xa;
> - }
> -
> cmdq->mem = ivpu_bo_create_global(vdev, SZ_4K, DRM_IVPU_BO_WC | DRM_IVPU_BO_MAPPABLE);
> if (!cmdq->mem)
> - goto err_erase_cmdq_xa;
> + goto err_free_cmdq;
>
> ret = ivpu_preemption_buffers_create(vdev, file_priv, cmdq);
> if (ret)
> @@ -107,10 +93,6 @@ static struct ivpu_cmdq *ivpu_cmdq_alloc(struct ivpu_file_priv *file_priv)
>
> return cmdq;
>
> -err_erase_cmdq_xa:
> - xa_erase(&file_priv->cmdq_xa, cmdq->id);
> -err_erase_db_xa:
> - xa_erase(&vdev->db_xa, cmdq->db_id);
> err_free_cmdq:
> kfree(cmdq);
> return NULL;
> @@ -233,30 +215,88 @@ static int ivpu_cmdq_fini(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cm
> return 0;
> }
>
> +static int ivpu_db_id_alloc(struct ivpu_device *vdev, u32 *db_id)
> +{
> + int ret;
> + u32 id;
> +
> + ret = xa_alloc_cyclic(&vdev->db_xa, &id, NULL, vdev->db_limit, &vdev->db_next, GFP_KERNEL);
> + if (ret < 0)
> + return ret;
> +
> + *db_id = id;
> + return 0;
> +}
> +
> +static int ivpu_cmdq_id_alloc(struct ivpu_file_priv *file_priv, u32 *cmdq_id)
> +{
> + int ret;
> + u32 id;
> +
> + ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &id, NULL, file_priv->cmdq_limit,
> + &file_priv->cmdq_id_next, GFP_KERNEL);
> + if (ret < 0)
> + return ret;
> +
> + *cmdq_id = id;
> + return 0;
> +}
> +
> static struct ivpu_cmdq *ivpu_cmdq_acquire(struct ivpu_file_priv *file_priv, u8 priority)
> {
> + struct ivpu_device *vdev = file_priv->vdev;
> struct ivpu_cmdq *cmdq;
> - unsigned long cmdq_id;
> + unsigned long id;
> int ret;
>
> lockdep_assert_held(&file_priv->lock);
>
> - xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
> + xa_for_each(&file_priv->cmdq_xa, id, cmdq)
> if (cmdq->priority == priority)
> break;
>
> if (!cmdq) {
> cmdq = ivpu_cmdq_alloc(file_priv);
> - if (!cmdq)
> + if (!cmdq) {
> + ivpu_err(vdev, "Failed to allocate command queue\n");
> return NULL;
> + }
> +
> + ret = ivpu_db_id_alloc(vdev, &cmdq->db_id);
> + if (ret) {
> + ivpu_err(file_priv->vdev, "Failed to allocate doorbell ID: %d\n", ret);
> + goto err_free_cmdq;
> + }
> +
> + ret = ivpu_cmdq_id_alloc(file_priv, &cmdq->id);
> + if (ret) {
> + ivpu_err(vdev, "Failed to allocate command queue ID: %d\n", ret);
> + goto err_erase_db_id;
> + }
> +
> cmdq->priority = priority;
> + ret = xa_err(xa_store(&file_priv->cmdq_xa, cmdq->id, cmdq, GFP_KERNEL));
> + if (ret) {
> + ivpu_err(vdev, "Failed to store command queue in cmdq_xa: %d\n", ret);
> + goto err_erase_cmdq_id;
> + }
> }
>
> ret = ivpu_cmdq_init(file_priv, cmdq, priority);
> - if (ret)
> - return NULL;
> + if (ret) {
> + ivpu_err(vdev, "Failed to initialize command queue: %d\n", ret);
> + goto err_free_cmdq;
> + }
>
> return cmdq;
> +
> +err_erase_cmdq_id:
> + xa_erase(&file_priv->cmdq_xa, cmdq->id);
> +err_erase_db_id:
> + xa_erase(&vdev->db_xa, cmdq->db_id);
> +err_free_cmdq:
> + ivpu_cmdq_free(file_priv, cmdq);
> + return NULL;
> }
>
> void ivpu_cmdq_release_all_locked(struct ivpu_file_priv *file_priv)
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 02/14] accel/ivpu: Add API for command queue create/destroy/submit
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
2025-01-07 17:32 ` [PATCH 01/14] accel/ivpu: Separate DB ID and CMDQ ID allocations from CMDQ allocation Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-09 8:22 ` Jacek Lawrynowicz
2025-01-07 17:32 ` [PATCH 03/14] accel/ivpu: Abort all jobs after command queue unregister Maciej Falkowski
` (13 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel
Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Karol Wachowski,
Maciej Falkowski
From: Karol Wachowski <karol.wachowski@intel.com>
Implement support for explicit command queue management.
To allow more flexible control over command queues add capabilities
to create, destroy and submit jobs to specific command queues.
Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_drv.c | 6 +
drivers/accel/ivpu/ivpu_job.c | 369 +++++++++++++++++++++-------------
drivers/accel/ivpu/ivpu_job.h | 5 +-
include/uapi/drm/ivpu_accel.h | 84 ++++++++
4 files changed, 319 insertions(+), 145 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index 1e8ffbe25eee..f4171536640b 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -137,6 +137,9 @@ static int ivpu_get_capabilities(struct ivpu_device *vdev, struct drm_ivpu_param
case DRM_IVPU_CAP_DMA_MEMORY_RANGE:
args->value = 1;
break;
+ case DRM_IVPU_CAP_MANAGE_CMDQ:
+ args->value = 1;
+ break;
default:
return -EINVAL;
}
@@ -310,6 +313,9 @@ static const struct drm_ioctl_desc ivpu_drm_ioctls[] = {
DRM_IOCTL_DEF_DRV(IVPU_METRIC_STREAMER_GET_DATA, ivpu_ms_get_data_ioctl, 0),
DRM_IOCTL_DEF_DRV(IVPU_METRIC_STREAMER_STOP, ivpu_ms_stop_ioctl, 0),
DRM_IOCTL_DEF_DRV(IVPU_METRIC_STREAMER_GET_INFO, ivpu_ms_get_info_ioctl, 0),
+ DRM_IOCTL_DEF_DRV(IVPU_CMDQ_CREATE, ivpu_cmdq_create_ioctl, 0),
+ DRM_IOCTL_DEF_DRV(IVPU_CMDQ_DESTROY, ivpu_cmdq_destroy_ioctl, 0),
+ DRM_IOCTL_DEF_DRV(IVPU_CMDQ_SUBMIT, ivpu_cmdq_submit_ioctl, 0),
};
static int ivpu_wait_for_ready(struct ivpu_device *vdev)
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index 98e53cb38ecd..43507d3aea51 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -100,15 +100,43 @@ static struct ivpu_cmdq *ivpu_cmdq_alloc(struct ivpu_file_priv *file_priv)
static void ivpu_cmdq_free(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
{
- if (!cmdq)
- return;
-
ivpu_preemption_buffers_free(file_priv->vdev, file_priv, cmdq);
ivpu_bo_free(cmdq->mem);
- xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
kfree(cmdq);
}
+static struct ivpu_cmdq *ivpu_cmdq_create(struct ivpu_file_priv *file_priv, u8 priority,
+ bool is_legacy)
+{
+ struct ivpu_device *vdev = file_priv->vdev;
+ struct ivpu_cmdq *cmdq = NULL;
+ int ret;
+
+ lockdep_assert_held(&file_priv->lock);
+
+ cmdq = ivpu_cmdq_alloc(file_priv);
+ if (!cmdq) {
+ ivpu_err(vdev, "Failed to allocate command queue\n");
+ return NULL;
+ }
+
+ cmdq->priority = priority;
+ cmdq->is_legacy = is_legacy;
+
+ ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &cmdq->id, cmdq, file_priv->cmdq_limit,
+ &file_priv->cmdq_id_next, GFP_KERNEL);
+ if (ret < 0) {
+ ivpu_err(vdev, "Failed to allocate command queue ID: %d\n", ret);
+ goto err_free_cmdq;
+ }
+
+ return cmdq;
+
+err_free_cmdq:
+ ivpu_cmdq_free(file_priv, cmdq);
+ return NULL;
+}
+
static int ivpu_hws_cmdq_init(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq, u16 engine,
u8 priority)
{
@@ -134,6 +162,13 @@ static int ivpu_register_db(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *
struct ivpu_device *vdev = file_priv->vdev;
int ret;
+ ret = xa_alloc_cyclic(&vdev->db_xa, &cmdq->db_id, NULL, vdev->db_limit, &vdev->db_next,
+ GFP_KERNEL);
+ if (ret < 0) {
+ ivpu_err(vdev, "Failed to allocate doorbell ID: %d\n", ret);
+ return ret;
+ }
+
if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW)
ret = ivpu_jsm_hws_register_db(vdev, file_priv->ctx.id, cmdq->id, cmdq->db_id,
cmdq->mem->vpu_addr, ivpu_bo_size(cmdq->mem));
@@ -142,41 +177,52 @@ static int ivpu_register_db(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *
cmdq->mem->vpu_addr, ivpu_bo_size(cmdq->mem));
if (!ret)
- ivpu_dbg(vdev, JOB, "DB %d registered to cmdq %d ctx %d\n",
- cmdq->db_id, cmdq->id, file_priv->ctx.id);
+ ivpu_dbg(vdev, JOB, "DB %d registered to cmdq %d ctx %d priority %d\n",
+ cmdq->db_id, cmdq->id, file_priv->ctx.id, cmdq->priority);
+ else
+ xa_erase(&vdev->db_xa, cmdq->db_id);
return ret;
}
-static int
-ivpu_cmdq_init(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq, u8 priority)
+static void ivpu_cmdq_jobq_init(struct ivpu_device *vdev, struct vpu_job_queue *jobq)
+{
+ jobq->header.engine_idx = VPU_ENGINE_COMPUTE;
+ jobq->header.head = 0;
+ jobq->header.tail = 0;
+
+ if (ivpu_test_mode & IVPU_TEST_MODE_TURBO) {
+ ivpu_dbg(vdev, JOB, "Turbo mode enabled");
+ jobq->header.flags = VPU_JOB_QUEUE_FLAGS_TURBO_MODE;
+ }
+
+ wmb(); /* Flush WC buffer for jobq->header */
+}
+
+static inline u32 ivpu_cmdq_get_entry_count(struct ivpu_cmdq *cmdq)
+{
+ size_t size = ivpu_bo_size(cmdq->mem) - sizeof(struct vpu_job_queue_header);
+
+ return size / sizeof(struct vpu_job_queue_entry);
+}
+
+static int ivpu_cmdq_register(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
{
struct ivpu_device *vdev = file_priv->vdev;
- struct vpu_job_queue_header *jobq_header;
int ret;
lockdep_assert_held(&file_priv->lock);
- if (cmdq->db_registered)
+ if (cmdq->db_id)
return 0;
- cmdq->entry_count = (u32)((ivpu_bo_size(cmdq->mem) - sizeof(struct vpu_job_queue_header)) /
- sizeof(struct vpu_job_queue_entry));
-
+ cmdq->entry_count = ivpu_cmdq_get_entry_count(cmdq);
cmdq->jobq = (struct vpu_job_queue *)ivpu_bo_vaddr(cmdq->mem);
- jobq_header = &cmdq->jobq->header;
- jobq_header->engine_idx = VPU_ENGINE_COMPUTE;
- jobq_header->head = 0;
- jobq_header->tail = 0;
- if (ivpu_test_mode & IVPU_TEST_MODE_TURBO) {
- ivpu_dbg(vdev, JOB, "Turbo mode enabled");
- jobq_header->flags = VPU_JOB_QUEUE_FLAGS_TURBO_MODE;
- }
- wmb(); /* Flush WC buffer for jobq->header */
+ ivpu_cmdq_jobq_init(vdev, cmdq->jobq);
if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW) {
- ret = ivpu_hws_cmdq_init(file_priv, cmdq, VPU_ENGINE_COMPUTE, priority);
+ ret = ivpu_hws_cmdq_init(file_priv, cmdq, VPU_ENGINE_COMPUTE, cmdq->priority);
if (ret)
return ret;
}
@@ -185,23 +231,19 @@ ivpu_cmdq_init(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq, u8 prio
if (ret)
return ret;
- cmdq->db_registered = true;
-
return 0;
}
-static int ivpu_cmdq_fini(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
+static int ivpu_cmdq_unregister(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
{
struct ivpu_device *vdev = file_priv->vdev;
int ret;
lockdep_assert_held(&file_priv->lock);
- if (!cmdq->db_registered)
+ if (!cmdq->db_id)
return 0;
- cmdq->db_registered = false;
-
if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW) {
ret = ivpu_jsm_hws_destroy_cmdq(vdev, file_priv->ctx.id, cmdq->id);
if (!ret)
@@ -212,91 +254,61 @@ static int ivpu_cmdq_fini(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cm
if (!ret)
ivpu_dbg(vdev, JOB, "DB %d unregistered\n", cmdq->db_id);
+ xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
+ cmdq->db_id = 0;
+
return 0;
}
-static int ivpu_db_id_alloc(struct ivpu_device *vdev, u32 *db_id)
+static inline u8 ivpu_job_to_jsm_priority(u8 priority)
{
- int ret;
- u32 id;
-
- ret = xa_alloc_cyclic(&vdev->db_xa, &id, NULL, vdev->db_limit, &vdev->db_next, GFP_KERNEL);
- if (ret < 0)
- return ret;
+ if (priority == DRM_IVPU_JOB_PRIORITY_DEFAULT)
+ return VPU_JOB_SCHEDULING_PRIORITY_BAND_NORMAL;
- *db_id = id;
- return 0;
+ return priority - 1;
}
-static int ivpu_cmdq_id_alloc(struct ivpu_file_priv *file_priv, u32 *cmdq_id)
+static void ivpu_cmdq_destroy(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
{
- int ret;
- u32 id;
-
- ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &id, NULL, file_priv->cmdq_limit,
- &file_priv->cmdq_id_next, GFP_KERNEL);
- if (ret < 0)
- return ret;
-
- *cmdq_id = id;
- return 0;
+ ivpu_cmdq_unregister(file_priv, cmdq);
+ xa_erase(&file_priv->cmdq_xa, cmdq->id);
+ ivpu_cmdq_free(file_priv, cmdq);
}
-static struct ivpu_cmdq *ivpu_cmdq_acquire(struct ivpu_file_priv *file_priv, u8 priority)
+static struct ivpu_cmdq *ivpu_cmdq_acquire_legacy(struct ivpu_file_priv *file_priv, u8 priority)
{
- struct ivpu_device *vdev = file_priv->vdev;
struct ivpu_cmdq *cmdq;
unsigned long id;
- int ret;
lockdep_assert_held(&file_priv->lock);
xa_for_each(&file_priv->cmdq_xa, id, cmdq)
- if (cmdq->priority == priority)
+ if (cmdq->is_legacy && cmdq->priority == priority)
break;
if (!cmdq) {
- cmdq = ivpu_cmdq_alloc(file_priv);
- if (!cmdq) {
- ivpu_err(vdev, "Failed to allocate command queue\n");
+ cmdq = ivpu_cmdq_create(file_priv, priority, true);
+ if (!cmdq)
return NULL;
- }
+ }
- ret = ivpu_db_id_alloc(vdev, &cmdq->db_id);
- if (ret) {
- ivpu_err(file_priv->vdev, "Failed to allocate doorbell ID: %d\n", ret);
- goto err_free_cmdq;
- }
+ return cmdq;
+}
- ret = ivpu_cmdq_id_alloc(file_priv, &cmdq->id);
- if (ret) {
- ivpu_err(vdev, "Failed to allocate command queue ID: %d\n", ret);
- goto err_erase_db_id;
- }
+static struct ivpu_cmdq *ivpu_cmdq_acquire(struct ivpu_file_priv *file_priv, u32 cmdq_id)
+{
+ struct ivpu_device *vdev = file_priv->vdev;
+ struct ivpu_cmdq *cmdq;
- cmdq->priority = priority;
- ret = xa_err(xa_store(&file_priv->cmdq_xa, cmdq->id, cmdq, GFP_KERNEL));
- if (ret) {
- ivpu_err(vdev, "Failed to store command queue in cmdq_xa: %d\n", ret);
- goto err_erase_cmdq_id;
- }
- }
+ lockdep_assert_held(&file_priv->lock);
- ret = ivpu_cmdq_init(file_priv, cmdq, priority);
- if (ret) {
- ivpu_err(vdev, "Failed to initialize command queue: %d\n", ret);
- goto err_free_cmdq;
+ cmdq = xa_load(&file_priv->cmdq_xa, cmdq_id);
+ if (!cmdq) {
+ ivpu_err(vdev, "Failed to find command queue with ID: %u\n", cmdq_id);
+ return NULL;
}
return cmdq;
-
-err_erase_cmdq_id:
- xa_erase(&file_priv->cmdq_xa, cmdq->id);
-err_erase_db_id:
- xa_erase(&vdev->db_xa, cmdq->db_id);
-err_free_cmdq:
- ivpu_cmdq_free(file_priv, cmdq);
- return NULL;
}
void ivpu_cmdq_release_all_locked(struct ivpu_file_priv *file_priv)
@@ -306,11 +318,8 @@ void ivpu_cmdq_release_all_locked(struct ivpu_file_priv *file_priv)
lockdep_assert_held(&file_priv->lock);
- xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq) {
- xa_erase(&file_priv->cmdq_xa, cmdq_id);
- ivpu_cmdq_fini(file_priv, cmdq);
- ivpu_cmdq_free(file_priv, cmdq);
- }
+ xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
+ ivpu_cmdq_destroy(file_priv, cmdq);
}
/*
@@ -326,8 +335,10 @@ static void ivpu_cmdq_reset(struct ivpu_file_priv *file_priv)
mutex_lock(&file_priv->lock);
- xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
- cmdq->db_registered = false;
+ xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq) {
+ xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
+ cmdq->db_id = 0;
+ }
mutex_unlock(&file_priv->lock);
}
@@ -345,13 +356,13 @@ void ivpu_cmdq_reset_all_contexts(struct ivpu_device *vdev)
mutex_unlock(&vdev->context_list_lock);
}
-static void ivpu_cmdq_fini_all(struct ivpu_file_priv *file_priv)
+static void ivpu_cmdq_unregister_all(struct ivpu_file_priv *file_priv)
{
struct ivpu_cmdq *cmdq;
unsigned long cmdq_id;
xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
- ivpu_cmdq_fini(file_priv, cmdq);
+ ivpu_cmdq_unregister(file_priv, cmdq);
}
void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv)
@@ -360,7 +371,7 @@ void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv)
lockdep_assert_held(&file_priv->lock);
- ivpu_cmdq_fini_all(file_priv);
+ ivpu_cmdq_unregister_all(file_priv);
if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_OS)
ivpu_jsm_context_release(vdev, file_priv->ctx.id);
@@ -549,7 +560,7 @@ void ivpu_jobs_abort_all(struct ivpu_device *vdev)
ivpu_job_signal_and_destroy(vdev, id, DRM_IVPU_JOB_STATUS_ABORTED);
}
-static int ivpu_job_submit(struct ivpu_job *job, u8 priority)
+static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
{
struct ivpu_file_priv *file_priv = job->file_priv;
struct ivpu_device *vdev = job->vdev;
@@ -563,14 +574,22 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority)
mutex_lock(&file_priv->lock);
- cmdq = ivpu_cmdq_acquire(file_priv, priority);
+ if (cmdq_id == 0)
+ cmdq = ivpu_cmdq_acquire_legacy(file_priv, priority);
+ else
+ cmdq = ivpu_cmdq_acquire(file_priv, cmdq_id);
if (!cmdq) {
- ivpu_warn_ratelimited(vdev, "Failed to get job queue, ctx %d engine %d prio %d\n",
- file_priv->ctx.id, job->engine_idx, priority);
+ ivpu_warn_ratelimited(vdev, "Failed to get job queue, ctx %d\n", file_priv->ctx.id);
ret = -EINVAL;
goto err_unlock_file_priv;
}
+ ret = ivpu_cmdq_register(file_priv, cmdq);
+ if (ret) {
+ ivpu_err(vdev, "Failed to register command queue: %d\n", ret);
+ goto err_unlock_file_priv;
+ }
+
xa_lock(&vdev->submitted_jobs_xa);
is_first_job = xa_empty(&vdev->submitted_jobs_xa);
ret = __xa_alloc_cyclic(&vdev->submitted_jobs_xa, &job->job_id, job, file_priv->job_limit,
@@ -599,7 +618,7 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority)
trace_job("submit", job);
ivpu_dbg(vdev, JOB, "Job submitted: id %3u ctx %2d engine %d prio %d addr 0x%llx next %d\n",
- job->job_id, file_priv->ctx.id, job->engine_idx, priority,
+ job->job_id, file_priv->ctx.id, job->engine_idx, cmdq->priority,
job->cmd_buf_vpu_addr, cmdq->jobq->header.tail);
xa_unlock(&vdev->submitted_jobs_xa);
@@ -625,7 +644,7 @@ static int
ivpu_job_prepare_bos_for_submit(struct drm_file *file, struct ivpu_job *job, u32 *buf_handles,
u32 buf_count, u32 commands_offset)
{
- struct ivpu_file_priv *file_priv = file->driver_priv;
+ struct ivpu_file_priv *file_priv = job->file_priv;
struct ivpu_device *vdev = file_priv->vdev;
struct ww_acquire_ctx acquire_ctx;
enum dma_resv_usage usage;
@@ -687,49 +706,20 @@ ivpu_job_prepare_bos_for_submit(struct drm_file *file, struct ivpu_job *job, u32
return ret;
}
-static inline u8 ivpu_job_to_hws_priority(struct ivpu_file_priv *file_priv, u8 priority)
+static int ivpu_submit(struct drm_file *file, struct ivpu_file_priv *file_priv, u32 cmdq_id,
+ u32 buffer_count, u32 engine, void __user *buffers_ptr, u32 cmds_offset,
+ u8 priority)
{
- if (priority == DRM_IVPU_JOB_PRIORITY_DEFAULT)
- return DRM_IVPU_JOB_PRIORITY_NORMAL;
-
- return priority - 1;
-}
-
-int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
-{
- struct ivpu_file_priv *file_priv = file->driver_priv;
struct ivpu_device *vdev = file_priv->vdev;
- struct drm_ivpu_submit *params = data;
struct ivpu_job *job;
u32 *buf_handles;
int idx, ret;
- u8 priority;
-
- if (params->engine != DRM_IVPU_ENGINE_COMPUTE)
- return -EINVAL;
-
- if (params->priority > DRM_IVPU_JOB_PRIORITY_REALTIME)
- return -EINVAL;
-
- if (params->buffer_count == 0 || params->buffer_count > JOB_MAX_BUFFER_COUNT)
- return -EINVAL;
-
- if (!IS_ALIGNED(params->commands_offset, 8))
- return -EINVAL;
- if (!file_priv->ctx.id)
- return -EINVAL;
-
- if (file_priv->has_mmu_faults)
- return -EBADFD;
-
- buf_handles = kcalloc(params->buffer_count, sizeof(u32), GFP_KERNEL);
+ buf_handles = kcalloc(buffer_count, sizeof(u32), GFP_KERNEL);
if (!buf_handles)
return -ENOMEM;
- ret = copy_from_user(buf_handles,
- (void __user *)params->buffers_ptr,
- params->buffer_count * sizeof(u32));
+ ret = copy_from_user(buf_handles, buffers_ptr, buffer_count * sizeof(u32));
if (ret) {
ret = -EFAULT;
goto err_free_handles;
@@ -740,27 +730,23 @@ int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
goto err_free_handles;
}
- ivpu_dbg(vdev, JOB, "Submit ioctl: ctx %u buf_count %u\n",
- file_priv->ctx.id, params->buffer_count);
+ ivpu_dbg(vdev, JOB, "Submit ioctl: ctx %u buf_count %u\n", file_priv->ctx.id, buffer_count);
- job = ivpu_job_create(file_priv, params->engine, params->buffer_count);
+ job = ivpu_job_create(file_priv, engine, buffer_count);
if (!job) {
ivpu_err(vdev, "Failed to create job\n");
ret = -ENOMEM;
goto err_exit_dev;
}
- ret = ivpu_job_prepare_bos_for_submit(file, job, buf_handles, params->buffer_count,
- params->commands_offset);
+ ret = ivpu_job_prepare_bos_for_submit(file, job, buf_handles, buffer_count, cmds_offset);
if (ret) {
ivpu_err(vdev, "Failed to prepare job: %d\n", ret);
goto err_destroy_job;
}
- priority = ivpu_job_to_hws_priority(file_priv, params->priority);
-
down_read(&vdev->pm->reset_lock);
- ret = ivpu_job_submit(job, priority);
+ ret = ivpu_job_submit(job, priority, cmdq_id);
up_read(&vdev->pm->reset_lock);
if (ret)
goto err_signal_fence;
@@ -780,6 +766,101 @@ int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
return ret;
}
+int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
+{
+ struct ivpu_file_priv *file_priv = file->driver_priv;
+ struct drm_ivpu_submit *args = data;
+ u8 priority;
+
+ if (args->engine != DRM_IVPU_ENGINE_COMPUTE)
+ return -EINVAL;
+
+ if (args->priority > DRM_IVPU_JOB_PRIORITY_REALTIME)
+ return -EINVAL;
+
+ if (args->buffer_count == 0 || args->buffer_count > JOB_MAX_BUFFER_COUNT)
+ return -EINVAL;
+
+ if (!IS_ALIGNED(args->commands_offset, 8))
+ return -EINVAL;
+
+ if (!file_priv->ctx.id)
+ return -EINVAL;
+
+ if (file_priv->has_mmu_faults)
+ return -EBADFD;
+
+ priority = ivpu_job_to_jsm_priority(args->priority);
+
+ return ivpu_submit(file, file_priv, 0, args->buffer_count, args->engine,
+ (void __user *)args->buffers_ptr, args->commands_offset, priority);
+}
+
+int ivpu_cmdq_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
+{
+ struct ivpu_file_priv *file_priv = file->driver_priv;
+ struct drm_ivpu_cmdq_submit *args = data;
+
+ if (args->cmdq_id < IVPU_CMDQ_MIN_ID || args->cmdq_id > IVPU_CMDQ_MAX_ID)
+ return -EINVAL;
+
+ if (args->buffer_count == 0 || args->buffer_count > JOB_MAX_BUFFER_COUNT)
+ return -EINVAL;
+
+ if (!IS_ALIGNED(args->commands_offset, 8))
+ return -EINVAL;
+
+ if (!file_priv->ctx.id)
+ return -EINVAL;
+
+ if (file_priv->has_mmu_faults)
+ return -EBADFD;
+
+ return ivpu_submit(file, file_priv, args->cmdq_id, args->buffer_count, VPU_ENGINE_COMPUTE,
+ (void __user *)args->buffers_ptr, args->commands_offset, 0);
+}
+
+int ivpu_cmdq_create_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
+{
+ struct ivpu_file_priv *file_priv = file->driver_priv;
+ struct drm_ivpu_cmdq_create *args = data;
+ struct ivpu_cmdq *cmdq;
+
+ if (args->priority > DRM_IVPU_JOB_PRIORITY_REALTIME)
+ return -EINVAL;
+
+ mutex_lock(&file_priv->lock);
+
+ cmdq = ivpu_cmdq_create(file_priv, ivpu_job_to_jsm_priority(args->priority), false);
+ if (cmdq)
+ args->cmdq_id = cmdq->id;
+
+ mutex_unlock(&file_priv->lock);
+
+ return cmdq ? 0 : -ENOMEM;
+}
+
+int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
+{
+ struct ivpu_file_priv *file_priv = file->driver_priv;
+ struct drm_ivpu_cmdq_destroy *args = data;
+ struct ivpu_cmdq *cmdq;
+ int ret = 0;
+
+ mutex_lock(&file_priv->lock);
+
+ cmdq = xa_load(&file_priv->cmdq_xa, args->cmdq_id);
+ if (!cmdq || cmdq->is_legacy) {
+ ret = -ENOENT;
+ goto unlock;
+ }
+
+ ivpu_cmdq_destroy(file_priv, cmdq);
+unlock:
+ mutex_unlock(&file_priv->lock);
+ return ret;
+}
+
static void
ivpu_job_done_callback(struct ivpu_device *vdev, struct ivpu_ipc_hdr *ipc_hdr,
struct vpu_jsm_msg *jsm_msg)
diff --git a/drivers/accel/ivpu/ivpu_job.h b/drivers/accel/ivpu/ivpu_job.h
index 8b19e3f8b4cf..4d31277bcc41 100644
--- a/drivers/accel/ivpu/ivpu_job.h
+++ b/drivers/accel/ivpu/ivpu_job.h
@@ -30,8 +30,8 @@ struct ivpu_cmdq {
u32 entry_count;
u32 id;
u32 db_id;
- bool db_registered;
u8 priority;
+ bool is_legacy;
};
/**
@@ -58,6 +58,9 @@ struct ivpu_job {
};
int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file);
+int ivpu_cmdq_create_ioctl(struct drm_device *dev, void *data, struct drm_file *file);
+int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file *file);
+int ivpu_cmdq_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file);
void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv);
diff --git a/include/uapi/drm/ivpu_accel.h b/include/uapi/drm/ivpu_accel.h
index a35b97b097bf..746c43bd3eb6 100644
--- a/include/uapi/drm/ivpu_accel.h
+++ b/include/uapi/drm/ivpu_accel.h
@@ -22,6 +22,9 @@ extern "C" {
#define DRM_IVPU_METRIC_STREAMER_STOP 0x08
#define DRM_IVPU_METRIC_STREAMER_GET_DATA 0x09
#define DRM_IVPU_METRIC_STREAMER_GET_INFO 0x0a
+#define DRM_IVPU_CMDQ_CREATE 0x0b
+#define DRM_IVPU_CMDQ_DESTROY 0x0c
+#define DRM_IVPU_CMDQ_SUBMIT 0x0d
#define DRM_IOCTL_IVPU_GET_PARAM \
DRM_IOWR(DRM_COMMAND_BASE + DRM_IVPU_GET_PARAM, struct drm_ivpu_param)
@@ -57,6 +60,15 @@ extern "C" {
DRM_IOWR(DRM_COMMAND_BASE + DRM_IVPU_METRIC_STREAMER_GET_INFO, \
struct drm_ivpu_metric_streamer_get_data)
+#define DRM_IOCTL_IVPU_CMDQ_CREATE \
+ DRM_IOWR(DRM_COMMAND_BASE + DRM_IVPU_CMDQ_CREATE, struct drm_ivpu_cmdq_create)
+
+#define DRM_IOCTL_IVPU_CMDQ_DESTROY \
+ DRM_IOW(DRM_COMMAND_BASE + DRM_IVPU_CMDQ_DESTROY, struct drm_ivpu_cmdq_destroy)
+
+#define DRM_IOCTL_IVPU_CMDQ_SUBMIT \
+ DRM_IOW(DRM_COMMAND_BASE + DRM_IVPU_CMDQ_SUBMIT, struct drm_ivpu_cmdq_submit)
+
/**
* DOC: contexts
*
@@ -107,6 +119,13 @@ extern "C" {
* accessible by hardware DMA.
*/
#define DRM_IVPU_CAP_DMA_MEMORY_RANGE 2
+/**
+ * DRM_IVPU_CAP_MANAGE_CMDQ
+ *
+ * Driver supports explicit command queue operations like command queue create,
+ * command queue destroy and submit job on specific command queue.
+ */
+#define DRM_IVPU_CAP_MANAGE_CMDQ 3
/**
* struct drm_ivpu_param - Get/Set VPU parameters
@@ -316,6 +335,44 @@ struct drm_ivpu_submit {
__u32 priority;
};
+/**
+ * struct drm_ivpu_cmdq_submit - Submit commands to the VPU using explicit command queue
+ *
+ * Execute a single command buffer on a given command queue.
+ * Handles to all referenced buffer objects have to be provided in @buffers_ptr.
+ *
+ * User space may wait on job completion using %DRM_IVPU_BO_WAIT ioctl.
+ */
+struct drm_ivpu_cmdq_submit {
+ /**
+ * @buffers_ptr:
+ *
+ * A pointer to an u32 array of GEM handles of the BOs required for this job.
+ * The number of elements in the array must be equal to the value given by @buffer_count.
+ *
+ * The first BO is the command buffer. The rest of array has to contain all
+ * BOs referenced from the command buffer.
+ */
+ __u64 buffers_ptr;
+
+ /** @buffer_count: Number of elements in the @buffers_ptr */
+ __u32 buffer_count;
+
+ /** @cmdq_id: ID for the command queue where job will be submitted */
+ __u32 cmdq_id;
+
+ /** @flags: Reserved for future use - must be zero */
+ __u32 flags;
+
+ /**
+ * @commands_offset:
+ *
+ * Offset inside the first buffer in @buffers_ptr containing commands
+ * to be executed. The offset has to be 8-byte aligned.
+ */
+ __u32 commands_offset;
+};
+
/* drm_ivpu_bo_wait job status codes */
#define DRM_IVPU_JOB_STATUS_SUCCESS 0
#define DRM_IVPU_JOB_STATUS_ABORTED 256
@@ -388,6 +445,33 @@ struct drm_ivpu_metric_streamer_get_data {
__u64 data_size;
};
+/**
+ * struct drm_ivpu_cmdq_create - Create command queue for job submission
+ */
+struct drm_ivpu_cmdq_create {
+ /** @cmdq_id: Returned ID of created command queue */
+ __u32 cmdq_id;
+ /**
+ * @priority:
+ *
+ * Priority to be set for related job command queue, can be one of the following:
+ * %DRM_IVPU_JOB_PRIORITY_DEFAULT
+ * %DRM_IVPU_JOB_PRIORITY_IDLE
+ * %DRM_IVPU_JOB_PRIORITY_NORMAL
+ * %DRM_IVPU_JOB_PRIORITY_FOCUS
+ * %DRM_IVPU_JOB_PRIORITY_REALTIME
+ */
+ __u32 priority;
+};
+
+/**
+ * struct drm_ivpu_cmdq_destroy - Destroy a command queue
+ */
+struct drm_ivpu_cmdq_destroy {
+ /** @cmdq_id: ID of command queue to destroy */
+ __u32 cmdq_id;
+};
+
/**
* struct drm_ivpu_metric_streamer_stop - Stop collecting metric data
*/
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 02/14] accel/ivpu: Add API for command queue create/destroy/submit
2025-01-07 17:32 ` [PATCH 02/14] accel/ivpu: Add API for command queue create/destroy/submit Maciej Falkowski
@ 2025-01-09 8:22 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:22 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo, Karol Wachowski
Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> From: Karol Wachowski <karol.wachowski@intel.com>
>
> Implement support for explicit command queue management.
> To allow more flexible control over command queues add capabilities
> to create, destroy and submit jobs to specific command queues.
>
> Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_drv.c | 6 +
> drivers/accel/ivpu/ivpu_job.c | 369 +++++++++++++++++++++-------------
> drivers/accel/ivpu/ivpu_job.h | 5 +-
> include/uapi/drm/ivpu_accel.h | 84 ++++++++
> 4 files changed, 319 insertions(+), 145 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
> index 1e8ffbe25eee..f4171536640b 100644
> --- a/drivers/accel/ivpu/ivpu_drv.c
> +++ b/drivers/accel/ivpu/ivpu_drv.c
> @@ -137,6 +137,9 @@ static int ivpu_get_capabilities(struct ivpu_device *vdev, struct drm_ivpu_param
> case DRM_IVPU_CAP_DMA_MEMORY_RANGE:
> args->value = 1;
> break;
> + case DRM_IVPU_CAP_MANAGE_CMDQ:
> + args->value = 1;
> + break;
> default:
> return -EINVAL;
> }
> @@ -310,6 +313,9 @@ static const struct drm_ioctl_desc ivpu_drm_ioctls[] = {
> DRM_IOCTL_DEF_DRV(IVPU_METRIC_STREAMER_GET_DATA, ivpu_ms_get_data_ioctl, 0),
> DRM_IOCTL_DEF_DRV(IVPU_METRIC_STREAMER_STOP, ivpu_ms_stop_ioctl, 0),
> DRM_IOCTL_DEF_DRV(IVPU_METRIC_STREAMER_GET_INFO, ivpu_ms_get_info_ioctl, 0),
> + DRM_IOCTL_DEF_DRV(IVPU_CMDQ_CREATE, ivpu_cmdq_create_ioctl, 0),
> + DRM_IOCTL_DEF_DRV(IVPU_CMDQ_DESTROY, ivpu_cmdq_destroy_ioctl, 0),
> + DRM_IOCTL_DEF_DRV(IVPU_CMDQ_SUBMIT, ivpu_cmdq_submit_ioctl, 0),
> };
>
> static int ivpu_wait_for_ready(struct ivpu_device *vdev)
> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
> index 98e53cb38ecd..43507d3aea51 100644
> --- a/drivers/accel/ivpu/ivpu_job.c
> +++ b/drivers/accel/ivpu/ivpu_job.c
> @@ -100,15 +100,43 @@ static struct ivpu_cmdq *ivpu_cmdq_alloc(struct ivpu_file_priv *file_priv)
>
> static void ivpu_cmdq_free(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
> {
> - if (!cmdq)
> - return;
> -
> ivpu_preemption_buffers_free(file_priv->vdev, file_priv, cmdq);
> ivpu_bo_free(cmdq->mem);
> - xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
> kfree(cmdq);
> }
>
> +static struct ivpu_cmdq *ivpu_cmdq_create(struct ivpu_file_priv *file_priv, u8 priority,
> + bool is_legacy)
> +{
> + struct ivpu_device *vdev = file_priv->vdev;
> + struct ivpu_cmdq *cmdq = NULL;
> + int ret;
> +
> + lockdep_assert_held(&file_priv->lock);
> +
> + cmdq = ivpu_cmdq_alloc(file_priv);
> + if (!cmdq) {
> + ivpu_err(vdev, "Failed to allocate command queue\n");
> + return NULL;
> + }
> +
> + cmdq->priority = priority;
> + cmdq->is_legacy = is_legacy;
> +
> + ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &cmdq->id, cmdq, file_priv->cmdq_limit,
> + &file_priv->cmdq_id_next, GFP_KERNEL);
> + if (ret < 0) {
> + ivpu_err(vdev, "Failed to allocate command queue ID: %d\n", ret);
> + goto err_free_cmdq;
> + }
> +
> + return cmdq;
> +
> +err_free_cmdq:
> + ivpu_cmdq_free(file_priv, cmdq);
> + return NULL;
> +}
> +
> static int ivpu_hws_cmdq_init(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq, u16 engine,
> u8 priority)
> {
> @@ -134,6 +162,13 @@ static int ivpu_register_db(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *
> struct ivpu_device *vdev = file_priv->vdev;
> int ret;
>
> + ret = xa_alloc_cyclic(&vdev->db_xa, &cmdq->db_id, NULL, vdev->db_limit, &vdev->db_next,
> + GFP_KERNEL);
> + if (ret < 0) {
> + ivpu_err(vdev, "Failed to allocate doorbell ID: %d\n", ret);
> + return ret;
> + }
> +
> if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW)
> ret = ivpu_jsm_hws_register_db(vdev, file_priv->ctx.id, cmdq->id, cmdq->db_id,
> cmdq->mem->vpu_addr, ivpu_bo_size(cmdq->mem));
> @@ -142,41 +177,52 @@ static int ivpu_register_db(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *
> cmdq->mem->vpu_addr, ivpu_bo_size(cmdq->mem));
>
> if (!ret)
> - ivpu_dbg(vdev, JOB, "DB %d registered to cmdq %d ctx %d\n",
> - cmdq->db_id, cmdq->id, file_priv->ctx.id);
> + ivpu_dbg(vdev, JOB, "DB %d registered to cmdq %d ctx %d priority %d\n",
> + cmdq->db_id, cmdq->id, file_priv->ctx.id, cmdq->priority);
> + else
> + xa_erase(&vdev->db_xa, cmdq->db_id);
>
> return ret;
> }
>
> -static int
> -ivpu_cmdq_init(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq, u8 priority)
> +static void ivpu_cmdq_jobq_init(struct ivpu_device *vdev, struct vpu_job_queue *jobq)
> +{
> + jobq->header.engine_idx = VPU_ENGINE_COMPUTE;
> + jobq->header.head = 0;
> + jobq->header.tail = 0;
> +
> + if (ivpu_test_mode & IVPU_TEST_MODE_TURBO) {
> + ivpu_dbg(vdev, JOB, "Turbo mode enabled");
> + jobq->header.flags = VPU_JOB_QUEUE_FLAGS_TURBO_MODE;
> + }
> +
> + wmb(); /* Flush WC buffer for jobq->header */
> +}
> +
> +static inline u32 ivpu_cmdq_get_entry_count(struct ivpu_cmdq *cmdq)
> +{
> + size_t size = ivpu_bo_size(cmdq->mem) - sizeof(struct vpu_job_queue_header);
> +
> + return size / sizeof(struct vpu_job_queue_entry);
> +}
> +
> +static int ivpu_cmdq_register(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
> {
> struct ivpu_device *vdev = file_priv->vdev;
> - struct vpu_job_queue_header *jobq_header;
> int ret;
>
> lockdep_assert_held(&file_priv->lock);
>
> - if (cmdq->db_registered)
> + if (cmdq->db_id)
> return 0;
>
> - cmdq->entry_count = (u32)((ivpu_bo_size(cmdq->mem) - sizeof(struct vpu_job_queue_header)) /
> - sizeof(struct vpu_job_queue_entry));
> -
> + cmdq->entry_count = ivpu_cmdq_get_entry_count(cmdq);
> cmdq->jobq = (struct vpu_job_queue *)ivpu_bo_vaddr(cmdq->mem);
> - jobq_header = &cmdq->jobq->header;
> - jobq_header->engine_idx = VPU_ENGINE_COMPUTE;
> - jobq_header->head = 0;
> - jobq_header->tail = 0;
> - if (ivpu_test_mode & IVPU_TEST_MODE_TURBO) {
> - ivpu_dbg(vdev, JOB, "Turbo mode enabled");
> - jobq_header->flags = VPU_JOB_QUEUE_FLAGS_TURBO_MODE;
> - }
>
> - wmb(); /* Flush WC buffer for jobq->header */
> + ivpu_cmdq_jobq_init(vdev, cmdq->jobq);
>
> if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW) {
> - ret = ivpu_hws_cmdq_init(file_priv, cmdq, VPU_ENGINE_COMPUTE, priority);
> + ret = ivpu_hws_cmdq_init(file_priv, cmdq, VPU_ENGINE_COMPUTE, cmdq->priority);
> if (ret)
> return ret;
> }
> @@ -185,23 +231,19 @@ ivpu_cmdq_init(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq, u8 prio
> if (ret)
> return ret;
>
> - cmdq->db_registered = true;
> -
> return 0;
> }
>
> -static int ivpu_cmdq_fini(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
> +static int ivpu_cmdq_unregister(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
> {
> struct ivpu_device *vdev = file_priv->vdev;
> int ret;
>
> lockdep_assert_held(&file_priv->lock);
>
> - if (!cmdq->db_registered)
> + if (!cmdq->db_id)
> return 0;
>
> - cmdq->db_registered = false;
> -
> if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW) {
> ret = ivpu_jsm_hws_destroy_cmdq(vdev, file_priv->ctx.id, cmdq->id);
> if (!ret)
> @@ -212,91 +254,61 @@ static int ivpu_cmdq_fini(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cm
> if (!ret)
> ivpu_dbg(vdev, JOB, "DB %d unregistered\n", cmdq->db_id);
>
> + xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
> + cmdq->db_id = 0;
> +
> return 0;
> }
>
> -static int ivpu_db_id_alloc(struct ivpu_device *vdev, u32 *db_id)
> +static inline u8 ivpu_job_to_jsm_priority(u8 priority)
> {
> - int ret;
> - u32 id;
> -
> - ret = xa_alloc_cyclic(&vdev->db_xa, &id, NULL, vdev->db_limit, &vdev->db_next, GFP_KERNEL);
> - if (ret < 0)
> - return ret;
> + if (priority == DRM_IVPU_JOB_PRIORITY_DEFAULT)
> + return VPU_JOB_SCHEDULING_PRIORITY_BAND_NORMAL;
>
> - *db_id = id;
> - return 0;
> + return priority - 1;
> }
>
> -static int ivpu_cmdq_id_alloc(struct ivpu_file_priv *file_priv, u32 *cmdq_id)
> +static void ivpu_cmdq_destroy(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
> {
> - int ret;
> - u32 id;
> -
> - ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &id, NULL, file_priv->cmdq_limit,
> - &file_priv->cmdq_id_next, GFP_KERNEL);
> - if (ret < 0)
> - return ret;
> -
> - *cmdq_id = id;
> - return 0;
> + ivpu_cmdq_unregister(file_priv, cmdq);
> + xa_erase(&file_priv->cmdq_xa, cmdq->id);
> + ivpu_cmdq_free(file_priv, cmdq);
> }
>
> -static struct ivpu_cmdq *ivpu_cmdq_acquire(struct ivpu_file_priv *file_priv, u8 priority)
> +static struct ivpu_cmdq *ivpu_cmdq_acquire_legacy(struct ivpu_file_priv *file_priv, u8 priority)
> {
> - struct ivpu_device *vdev = file_priv->vdev;
> struct ivpu_cmdq *cmdq;
> unsigned long id;
> - int ret;
>
> lockdep_assert_held(&file_priv->lock);
>
> xa_for_each(&file_priv->cmdq_xa, id, cmdq)
> - if (cmdq->priority == priority)
> + if (cmdq->is_legacy && cmdq->priority == priority)
> break;
>
> if (!cmdq) {
> - cmdq = ivpu_cmdq_alloc(file_priv);
> - if (!cmdq) {
> - ivpu_err(vdev, "Failed to allocate command queue\n");
> + cmdq = ivpu_cmdq_create(file_priv, priority, true);
> + if (!cmdq)
> return NULL;
> - }
> + }
>
> - ret = ivpu_db_id_alloc(vdev, &cmdq->db_id);
> - if (ret) {
> - ivpu_err(file_priv->vdev, "Failed to allocate doorbell ID: %d\n", ret);
> - goto err_free_cmdq;
> - }
> + return cmdq;
> +}
>
> - ret = ivpu_cmdq_id_alloc(file_priv, &cmdq->id);
> - if (ret) {
> - ivpu_err(vdev, "Failed to allocate command queue ID: %d\n", ret);
> - goto err_erase_db_id;
> - }
> +static struct ivpu_cmdq *ivpu_cmdq_acquire(struct ivpu_file_priv *file_priv, u32 cmdq_id)
> +{
> + struct ivpu_device *vdev = file_priv->vdev;
> + struct ivpu_cmdq *cmdq;
>
> - cmdq->priority = priority;
> - ret = xa_err(xa_store(&file_priv->cmdq_xa, cmdq->id, cmdq, GFP_KERNEL));
> - if (ret) {
> - ivpu_err(vdev, "Failed to store command queue in cmdq_xa: %d\n", ret);
> - goto err_erase_cmdq_id;
> - }
> - }
> + lockdep_assert_held(&file_priv->lock);
>
> - ret = ivpu_cmdq_init(file_priv, cmdq, priority);
> - if (ret) {
> - ivpu_err(vdev, "Failed to initialize command queue: %d\n", ret);
> - goto err_free_cmdq;
> + cmdq = xa_load(&file_priv->cmdq_xa, cmdq_id);
> + if (!cmdq) {
> + ivpu_err(vdev, "Failed to find command queue with ID: %u\n", cmdq_id);
> + return NULL;
> }
>
> return cmdq;
> -
> -err_erase_cmdq_id:
> - xa_erase(&file_priv->cmdq_xa, cmdq->id);
> -err_erase_db_id:
> - xa_erase(&vdev->db_xa, cmdq->db_id);
> -err_free_cmdq:
> - ivpu_cmdq_free(file_priv, cmdq);
> - return NULL;
> }
>
> void ivpu_cmdq_release_all_locked(struct ivpu_file_priv *file_priv)
> @@ -306,11 +318,8 @@ void ivpu_cmdq_release_all_locked(struct ivpu_file_priv *file_priv)
>
> lockdep_assert_held(&file_priv->lock);
>
> - xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq) {
> - xa_erase(&file_priv->cmdq_xa, cmdq_id);
> - ivpu_cmdq_fini(file_priv, cmdq);
> - ivpu_cmdq_free(file_priv, cmdq);
> - }
> + xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
> + ivpu_cmdq_destroy(file_priv, cmdq);
> }
>
> /*
> @@ -326,8 +335,10 @@ static void ivpu_cmdq_reset(struct ivpu_file_priv *file_priv)
>
> mutex_lock(&file_priv->lock);
>
> - xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
> - cmdq->db_registered = false;
> + xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq) {
> + xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
> + cmdq->db_id = 0;
> + }
>
> mutex_unlock(&file_priv->lock);
> }
> @@ -345,13 +356,13 @@ void ivpu_cmdq_reset_all_contexts(struct ivpu_device *vdev)
> mutex_unlock(&vdev->context_list_lock);
> }
>
> -static void ivpu_cmdq_fini_all(struct ivpu_file_priv *file_priv)
> +static void ivpu_cmdq_unregister_all(struct ivpu_file_priv *file_priv)
> {
> struct ivpu_cmdq *cmdq;
> unsigned long cmdq_id;
>
> xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
> - ivpu_cmdq_fini(file_priv, cmdq);
> + ivpu_cmdq_unregister(file_priv, cmdq);
> }
>
> void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv)
> @@ -360,7 +371,7 @@ void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv)
>
> lockdep_assert_held(&file_priv->lock);
>
> - ivpu_cmdq_fini_all(file_priv);
> + ivpu_cmdq_unregister_all(file_priv);
>
> if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_OS)
> ivpu_jsm_context_release(vdev, file_priv->ctx.id);
> @@ -549,7 +560,7 @@ void ivpu_jobs_abort_all(struct ivpu_device *vdev)
> ivpu_job_signal_and_destroy(vdev, id, DRM_IVPU_JOB_STATUS_ABORTED);
> }
>
> -static int ivpu_job_submit(struct ivpu_job *job, u8 priority)
> +static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
> {
> struct ivpu_file_priv *file_priv = job->file_priv;
> struct ivpu_device *vdev = job->vdev;
> @@ -563,14 +574,22 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority)
>
> mutex_lock(&file_priv->lock);
>
> - cmdq = ivpu_cmdq_acquire(file_priv, priority);
> + if (cmdq_id == 0)
> + cmdq = ivpu_cmdq_acquire_legacy(file_priv, priority);
> + else
> + cmdq = ivpu_cmdq_acquire(file_priv, cmdq_id);
> if (!cmdq) {
> - ivpu_warn_ratelimited(vdev, "Failed to get job queue, ctx %d engine %d prio %d\n",
> - file_priv->ctx.id, job->engine_idx, priority);
> + ivpu_warn_ratelimited(vdev, "Failed to get job queue, ctx %d\n", file_priv->ctx.id);
> ret = -EINVAL;
> goto err_unlock_file_priv;
> }
>
> + ret = ivpu_cmdq_register(file_priv, cmdq);
> + if (ret) {
> + ivpu_err(vdev, "Failed to register command queue: %d\n", ret);
> + goto err_unlock_file_priv;
> + }
> +
> xa_lock(&vdev->submitted_jobs_xa);
> is_first_job = xa_empty(&vdev->submitted_jobs_xa);
> ret = __xa_alloc_cyclic(&vdev->submitted_jobs_xa, &job->job_id, job, file_priv->job_limit,
> @@ -599,7 +618,7 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority)
>
> trace_job("submit", job);
> ivpu_dbg(vdev, JOB, "Job submitted: id %3u ctx %2d engine %d prio %d addr 0x%llx next %d\n",
> - job->job_id, file_priv->ctx.id, job->engine_idx, priority,
> + job->job_id, file_priv->ctx.id, job->engine_idx, cmdq->priority,
> job->cmd_buf_vpu_addr, cmdq->jobq->header.tail);
>
> xa_unlock(&vdev->submitted_jobs_xa);
> @@ -625,7 +644,7 @@ static int
> ivpu_job_prepare_bos_for_submit(struct drm_file *file, struct ivpu_job *job, u32 *buf_handles,
> u32 buf_count, u32 commands_offset)
> {
> - struct ivpu_file_priv *file_priv = file->driver_priv;
> + struct ivpu_file_priv *file_priv = job->file_priv;
> struct ivpu_device *vdev = file_priv->vdev;
> struct ww_acquire_ctx acquire_ctx;
> enum dma_resv_usage usage;
> @@ -687,49 +706,20 @@ ivpu_job_prepare_bos_for_submit(struct drm_file *file, struct ivpu_job *job, u32
> return ret;
> }
>
> -static inline u8 ivpu_job_to_hws_priority(struct ivpu_file_priv *file_priv, u8 priority)
> +static int ivpu_submit(struct drm_file *file, struct ivpu_file_priv *file_priv, u32 cmdq_id,
> + u32 buffer_count, u32 engine, void __user *buffers_ptr, u32 cmds_offset,
> + u8 priority)
> {
> - if (priority == DRM_IVPU_JOB_PRIORITY_DEFAULT)
> - return DRM_IVPU_JOB_PRIORITY_NORMAL;
> -
> - return priority - 1;
> -}
> -
> -int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> -{
> - struct ivpu_file_priv *file_priv = file->driver_priv;
> struct ivpu_device *vdev = file_priv->vdev;
> - struct drm_ivpu_submit *params = data;
> struct ivpu_job *job;
> u32 *buf_handles;
> int idx, ret;
> - u8 priority;
> -
> - if (params->engine != DRM_IVPU_ENGINE_COMPUTE)
> - return -EINVAL;
> -
> - if (params->priority > DRM_IVPU_JOB_PRIORITY_REALTIME)
> - return -EINVAL;
> -
> - if (params->buffer_count == 0 || params->buffer_count > JOB_MAX_BUFFER_COUNT)
> - return -EINVAL;
> -
> - if (!IS_ALIGNED(params->commands_offset, 8))
> - return -EINVAL;
>
> - if (!file_priv->ctx.id)
> - return -EINVAL;
> -
> - if (file_priv->has_mmu_faults)
> - return -EBADFD;
> -
> - buf_handles = kcalloc(params->buffer_count, sizeof(u32), GFP_KERNEL);
> + buf_handles = kcalloc(buffer_count, sizeof(u32), GFP_KERNEL);
> if (!buf_handles)
> return -ENOMEM;
>
> - ret = copy_from_user(buf_handles,
> - (void __user *)params->buffers_ptr,
> - params->buffer_count * sizeof(u32));
> + ret = copy_from_user(buf_handles, buffers_ptr, buffer_count * sizeof(u32));
> if (ret) {
> ret = -EFAULT;
> goto err_free_handles;
> @@ -740,27 +730,23 @@ int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> goto err_free_handles;
> }
>
> - ivpu_dbg(vdev, JOB, "Submit ioctl: ctx %u buf_count %u\n",
> - file_priv->ctx.id, params->buffer_count);
> + ivpu_dbg(vdev, JOB, "Submit ioctl: ctx %u buf_count %u\n", file_priv->ctx.id, buffer_count);
>
> - job = ivpu_job_create(file_priv, params->engine, params->buffer_count);
> + job = ivpu_job_create(file_priv, engine, buffer_count);
> if (!job) {
> ivpu_err(vdev, "Failed to create job\n");
> ret = -ENOMEM;
> goto err_exit_dev;
> }
>
> - ret = ivpu_job_prepare_bos_for_submit(file, job, buf_handles, params->buffer_count,
> - params->commands_offset);
> + ret = ivpu_job_prepare_bos_for_submit(file, job, buf_handles, buffer_count, cmds_offset);
> if (ret) {
> ivpu_err(vdev, "Failed to prepare job: %d\n", ret);
> goto err_destroy_job;
> }
>
> - priority = ivpu_job_to_hws_priority(file_priv, params->priority);
> -
> down_read(&vdev->pm->reset_lock);
> - ret = ivpu_job_submit(job, priority);
> + ret = ivpu_job_submit(job, priority, cmdq_id);
> up_read(&vdev->pm->reset_lock);
> if (ret)
> goto err_signal_fence;
> @@ -780,6 +766,101 @@ int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> return ret;
> }
>
> +int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> +{
> + struct ivpu_file_priv *file_priv = file->driver_priv;
> + struct drm_ivpu_submit *args = data;
> + u8 priority;
> +
> + if (args->engine != DRM_IVPU_ENGINE_COMPUTE)
> + return -EINVAL;
> +
> + if (args->priority > DRM_IVPU_JOB_PRIORITY_REALTIME)
> + return -EINVAL;
> +
> + if (args->buffer_count == 0 || args->buffer_count > JOB_MAX_BUFFER_COUNT)
> + return -EINVAL;
> +
> + if (!IS_ALIGNED(args->commands_offset, 8))
> + return -EINVAL;
> +
> + if (!file_priv->ctx.id)
> + return -EINVAL;
> +
> + if (file_priv->has_mmu_faults)
> + return -EBADFD;
> +
> + priority = ivpu_job_to_jsm_priority(args->priority);
> +
> + return ivpu_submit(file, file_priv, 0, args->buffer_count, args->engine,
> + (void __user *)args->buffers_ptr, args->commands_offset, priority);
> +}
> +
> +int ivpu_cmdq_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> +{
> + struct ivpu_file_priv *file_priv = file->driver_priv;
> + struct drm_ivpu_cmdq_submit *args = data;
> +
> + if (args->cmdq_id < IVPU_CMDQ_MIN_ID || args->cmdq_id > IVPU_CMDQ_MAX_ID)
> + return -EINVAL;
> +
> + if (args->buffer_count == 0 || args->buffer_count > JOB_MAX_BUFFER_COUNT)
> + return -EINVAL;
> +
> + if (!IS_ALIGNED(args->commands_offset, 8))
> + return -EINVAL;
> +
> + if (!file_priv->ctx.id)
> + return -EINVAL;
> +
> + if (file_priv->has_mmu_faults)
> + return -EBADFD;
> +
> + return ivpu_submit(file, file_priv, args->cmdq_id, args->buffer_count, VPU_ENGINE_COMPUTE,
> + (void __user *)args->buffers_ptr, args->commands_offset, 0);
> +}
> +
> +int ivpu_cmdq_create_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> +{
> + struct ivpu_file_priv *file_priv = file->driver_priv;
> + struct drm_ivpu_cmdq_create *args = data;
> + struct ivpu_cmdq *cmdq;
> +
> + if (args->priority > DRM_IVPU_JOB_PRIORITY_REALTIME)
> + return -EINVAL;
> +
> + mutex_lock(&file_priv->lock);
> +
> + cmdq = ivpu_cmdq_create(file_priv, ivpu_job_to_jsm_priority(args->priority), false);
> + if (cmdq)
> + args->cmdq_id = cmdq->id;
> +
> + mutex_unlock(&file_priv->lock);
> +
> + return cmdq ? 0 : -ENOMEM;
> +}
> +
> +int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> +{
> + struct ivpu_file_priv *file_priv = file->driver_priv;
> + struct drm_ivpu_cmdq_destroy *args = data;
> + struct ivpu_cmdq *cmdq;
> + int ret = 0;
> +
> + mutex_lock(&file_priv->lock);
> +
> + cmdq = xa_load(&file_priv->cmdq_xa, args->cmdq_id);
> + if (!cmdq || cmdq->is_legacy) {
> + ret = -ENOENT;
> + goto unlock;
> + }
> +
> + ivpu_cmdq_destroy(file_priv, cmdq);
> +unlock:
> + mutex_unlock(&file_priv->lock);
> + return ret;
> +}
> +
> static void
> ivpu_job_done_callback(struct ivpu_device *vdev, struct ivpu_ipc_hdr *ipc_hdr,
> struct vpu_jsm_msg *jsm_msg)
> diff --git a/drivers/accel/ivpu/ivpu_job.h b/drivers/accel/ivpu/ivpu_job.h
> index 8b19e3f8b4cf..4d31277bcc41 100644
> --- a/drivers/accel/ivpu/ivpu_job.h
> +++ b/drivers/accel/ivpu/ivpu_job.h
> @@ -30,8 +30,8 @@ struct ivpu_cmdq {
> u32 entry_count;
> u32 id;
> u32 db_id;
> - bool db_registered;
> u8 priority;
> + bool is_legacy;
> };
>
> /**
> @@ -58,6 +58,9 @@ struct ivpu_job {
> };
>
> int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file);
> +int ivpu_cmdq_create_ioctl(struct drm_device *dev, void *data, struct drm_file *file);
> +int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file *file);
> +int ivpu_cmdq_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file);
>
> void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv);
>
> diff --git a/include/uapi/drm/ivpu_accel.h b/include/uapi/drm/ivpu_accel.h
> index a35b97b097bf..746c43bd3eb6 100644
> --- a/include/uapi/drm/ivpu_accel.h
> +++ b/include/uapi/drm/ivpu_accel.h
> @@ -22,6 +22,9 @@ extern "C" {
> #define DRM_IVPU_METRIC_STREAMER_STOP 0x08
> #define DRM_IVPU_METRIC_STREAMER_GET_DATA 0x09
> #define DRM_IVPU_METRIC_STREAMER_GET_INFO 0x0a
> +#define DRM_IVPU_CMDQ_CREATE 0x0b
> +#define DRM_IVPU_CMDQ_DESTROY 0x0c
> +#define DRM_IVPU_CMDQ_SUBMIT 0x0d
>
> #define DRM_IOCTL_IVPU_GET_PARAM \
> DRM_IOWR(DRM_COMMAND_BASE + DRM_IVPU_GET_PARAM, struct drm_ivpu_param)
> @@ -57,6 +60,15 @@ extern "C" {
> DRM_IOWR(DRM_COMMAND_BASE + DRM_IVPU_METRIC_STREAMER_GET_INFO, \
> struct drm_ivpu_metric_streamer_get_data)
>
> +#define DRM_IOCTL_IVPU_CMDQ_CREATE \
> + DRM_IOWR(DRM_COMMAND_BASE + DRM_IVPU_CMDQ_CREATE, struct drm_ivpu_cmdq_create)
> +
> +#define DRM_IOCTL_IVPU_CMDQ_DESTROY \
> + DRM_IOW(DRM_COMMAND_BASE + DRM_IVPU_CMDQ_DESTROY, struct drm_ivpu_cmdq_destroy)
> +
> +#define DRM_IOCTL_IVPU_CMDQ_SUBMIT \
> + DRM_IOW(DRM_COMMAND_BASE + DRM_IVPU_CMDQ_SUBMIT, struct drm_ivpu_cmdq_submit)
> +
> /**
> * DOC: contexts
> *
> @@ -107,6 +119,13 @@ extern "C" {
> * accessible by hardware DMA.
> */
> #define DRM_IVPU_CAP_DMA_MEMORY_RANGE 2
> +/**
> + * DRM_IVPU_CAP_MANAGE_CMDQ
> + *
> + * Driver supports explicit command queue operations like command queue create,
> + * command queue destroy and submit job on specific command queue.
> + */
> +#define DRM_IVPU_CAP_MANAGE_CMDQ 3
>
> /**
> * struct drm_ivpu_param - Get/Set VPU parameters
> @@ -316,6 +335,44 @@ struct drm_ivpu_submit {
> __u32 priority;
> };
>
> +/**
> + * struct drm_ivpu_cmdq_submit - Submit commands to the VPU using explicit command queue
> + *
> + * Execute a single command buffer on a given command queue.
> + * Handles to all referenced buffer objects have to be provided in @buffers_ptr.
> + *
> + * User space may wait on job completion using %DRM_IVPU_BO_WAIT ioctl.
> + */
> +struct drm_ivpu_cmdq_submit {
> + /**
> + * @buffers_ptr:
> + *
> + * A pointer to an u32 array of GEM handles of the BOs required for this job.
> + * The number of elements in the array must be equal to the value given by @buffer_count.
> + *
> + * The first BO is the command buffer. The rest of array has to contain all
> + * BOs referenced from the command buffer.
> + */
> + __u64 buffers_ptr;
> +
> + /** @buffer_count: Number of elements in the @buffers_ptr */
> + __u32 buffer_count;
> +
> + /** @cmdq_id: ID for the command queue where job will be submitted */
> + __u32 cmdq_id;
> +
> + /** @flags: Reserved for future use - must be zero */
> + __u32 flags;
> +
> + /**
> + * @commands_offset:
> + *
> + * Offset inside the first buffer in @buffers_ptr containing commands
> + * to be executed. The offset has to be 8-byte aligned.
> + */
> + __u32 commands_offset;
> +};
> +
> /* drm_ivpu_bo_wait job status codes */
> #define DRM_IVPU_JOB_STATUS_SUCCESS 0
> #define DRM_IVPU_JOB_STATUS_ABORTED 256
> @@ -388,6 +445,33 @@ struct drm_ivpu_metric_streamer_get_data {
> __u64 data_size;
> };
>
> +/**
> + * struct drm_ivpu_cmdq_create - Create command queue for job submission
> + */
> +struct drm_ivpu_cmdq_create {
> + /** @cmdq_id: Returned ID of created command queue */
> + __u32 cmdq_id;
> + /**
> + * @priority:
> + *
> + * Priority to be set for related job command queue, can be one of the following:
> + * %DRM_IVPU_JOB_PRIORITY_DEFAULT
> + * %DRM_IVPU_JOB_PRIORITY_IDLE
> + * %DRM_IVPU_JOB_PRIORITY_NORMAL
> + * %DRM_IVPU_JOB_PRIORITY_FOCUS
> + * %DRM_IVPU_JOB_PRIORITY_REALTIME
> + */
> + __u32 priority;
> +};
> +
> +/**
> + * struct drm_ivpu_cmdq_destroy - Destroy a command queue
> + */
> +struct drm_ivpu_cmdq_destroy {
> + /** @cmdq_id: ID of command queue to destroy */
> + __u32 cmdq_id;
> +};
> +
> /**
> * struct drm_ivpu_metric_streamer_stop - Stop collecting metric data
> */
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 03/14] accel/ivpu: Abort all jobs after command queue unregister
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
2025-01-07 17:32 ` [PATCH 01/14] accel/ivpu: Separate DB ID and CMDQ ID allocations from CMDQ allocation Maciej Falkowski
2025-01-07 17:32 ` [PATCH 02/14] accel/ivpu: Add API for command queue create/destroy/submit Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-09 8:23 ` Jacek Lawrynowicz
2025-01-07 17:32 ` [PATCH 04/14] accel/ivpu: Expose NPU memory utilization info in sysfs Maciej Falkowski
` (12 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel
Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Karol Wachowski,
Maciej Falkowski
From: Karol Wachowski <karol.wachowski@intel.com>
With hardware scheduler it is not expected to receive JOB_DONE
notifications from NPU FW for the jobs aborted due to command queue destroy
JSM command.
Remove jobs submitted to unregistered command queue from submitted_jobs_xa
to avoid triggering a TDR in such case.
Add explicit submitted_jobs_lock that protects access to list of submitted
jobs which is now used to find jobs to abort.
Move context abort procedure to separate work queue not to slow down
handling of IPCs or DCT requests in case where job abort takes longer,
especially when destruction of the last job of a specific context results
in context release.
Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_drv.c | 32 ++------
drivers/accel/ivpu/ivpu_drv.h | 2 +
drivers/accel/ivpu/ivpu_job.c | 137 ++++++++++++++++++++++++--------
drivers/accel/ivpu/ivpu_job.h | 4 +
drivers/accel/ivpu/ivpu_mmu.c | 3 +-
drivers/accel/ivpu/ivpu_sysfs.c | 5 +-
6 files changed, 121 insertions(+), 62 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index f4171536640b..300eea8c305f 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -36,8 +36,6 @@
#define DRIVER_VERSION_STR "1.0.0 " UTS_RELEASE
#endif
-static struct lock_class_key submitted_jobs_xa_lock_class_key;
-
int ivpu_dbg_mask;
module_param_named(dbg_mask, ivpu_dbg_mask, int, 0644);
MODULE_PARM_DESC(dbg_mask, "Driver debug mask. See IVPU_DBG_* macros.");
@@ -467,26 +465,6 @@ static const struct drm_driver driver = {
.major = 1,
};
-static void ivpu_context_abort_invalid(struct ivpu_device *vdev)
-{
- struct ivpu_file_priv *file_priv;
- unsigned long ctx_id;
-
- mutex_lock(&vdev->context_list_lock);
-
- xa_for_each(&vdev->context_xa, ctx_id, file_priv) {
- if (!file_priv->has_mmu_faults || file_priv->aborted)
- continue;
-
- mutex_lock(&file_priv->lock);
- ivpu_context_abort_locked(file_priv);
- file_priv->aborted = true;
- mutex_unlock(&file_priv->lock);
- }
-
- mutex_unlock(&vdev->context_list_lock);
-}
-
static irqreturn_t ivpu_irq_thread_handler(int irq, void *arg)
{
struct ivpu_device *vdev = arg;
@@ -500,9 +478,6 @@ static irqreturn_t ivpu_irq_thread_handler(int irq, void *arg)
case IVPU_HW_IRQ_SRC_IPC:
ivpu_ipc_irq_thread_handler(vdev);
break;
- case IVPU_HW_IRQ_SRC_MMU_EVTQ:
- ivpu_context_abort_invalid(vdev);
- break;
case IVPU_HW_IRQ_SRC_DCT:
ivpu_pm_dct_irq_thread_handler(vdev);
break;
@@ -619,16 +594,21 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
xa_init_flags(&vdev->context_xa, XA_FLAGS_ALLOC | XA_FLAGS_LOCK_IRQ);
xa_init_flags(&vdev->submitted_jobs_xa, XA_FLAGS_ALLOC1);
xa_init_flags(&vdev->db_xa, XA_FLAGS_ALLOC1);
- lockdep_set_class(&vdev->submitted_jobs_xa.xa_lock, &submitted_jobs_xa_lock_class_key);
INIT_LIST_HEAD(&vdev->bo_list);
vdev->db_limit.min = IVPU_MIN_DB;
vdev->db_limit.max = IVPU_MAX_DB;
+ INIT_WORK(&vdev->context_abort_work, ivpu_context_abort_thread_handler);
+
ret = drmm_mutex_init(&vdev->drm, &vdev->context_list_lock);
if (ret)
goto err_xa_destroy;
+ ret = drmm_mutex_init(&vdev->drm, &vdev->submitted_jobs_lock);
+ if (ret)
+ goto err_xa_destroy;
+
ret = drmm_mutex_init(&vdev->drm, &vdev->bo_list_lock);
if (ret)
goto err_xa_destroy;
diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
index 3fdff3f6cffd..ebfcf3e42a3d 100644
--- a/drivers/accel/ivpu/ivpu_drv.h
+++ b/drivers/accel/ivpu/ivpu_drv.h
@@ -137,6 +137,7 @@ struct ivpu_device {
struct mutex context_list_lock; /* Protects user context addition/removal */
struct xarray context_xa;
struct xa_limit context_xa_limit;
+ struct work_struct context_abort_work;
struct xarray db_xa;
struct xa_limit db_limit;
@@ -145,6 +146,7 @@ struct ivpu_device {
struct mutex bo_list_lock; /* Protects bo_list */
struct list_head bo_list;
+ struct mutex submitted_jobs_lock; /* Protects submitted_jobs */
struct xarray submitted_jobs_xa;
struct ivpu_ipc_consumer job_done_consumer;
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index 43507d3aea51..7fed3c8406ee 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -122,6 +122,7 @@ static struct ivpu_cmdq *ivpu_cmdq_create(struct ivpu_file_priv *file_priv, u8 p
cmdq->priority = priority;
cmdq->is_legacy = is_legacy;
+ cmdq->is_valid = true;
ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &cmdq->id, cmdq, file_priv->cmdq_limit,
&file_priv->cmdq_id_next, GFP_KERNEL);
@@ -130,6 +131,7 @@ static struct ivpu_cmdq *ivpu_cmdq_create(struct ivpu_file_priv *file_priv, u8 p
goto err_free_cmdq;
}
+ ivpu_dbg(vdev, JOB, "Command queue %d created, ctx %d\n", cmdq->id, file_priv->ctx.id);
return cmdq;
err_free_cmdq:
@@ -247,7 +249,8 @@ static int ivpu_cmdq_unregister(struct ivpu_file_priv *file_priv, struct ivpu_cm
if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW) {
ret = ivpu_jsm_hws_destroy_cmdq(vdev, file_priv->ctx.id, cmdq->id);
if (!ret)
- ivpu_dbg(vdev, JOB, "Command queue %d destroyed\n", cmdq->id);
+ ivpu_dbg(vdev, JOB, "Command queue %d destroyed, ctx %d\n",
+ cmdq->id, file_priv->ctx.id);
}
ret = ivpu_jsm_unregister_db(vdev, cmdq->db_id);
@@ -303,8 +306,8 @@ static struct ivpu_cmdq *ivpu_cmdq_acquire(struct ivpu_file_priv *file_priv, u32
lockdep_assert_held(&file_priv->lock);
cmdq = xa_load(&file_priv->cmdq_xa, cmdq_id);
- if (!cmdq) {
- ivpu_err(vdev, "Failed to find command queue with ID: %u\n", cmdq_id);
+ if (!cmdq || !cmdq->is_valid) {
+ ivpu_warn_ratelimited(vdev, "Failed to find command queue with ID: %u\n", cmdq_id);
return NULL;
}
@@ -356,25 +359,21 @@ void ivpu_cmdq_reset_all_contexts(struct ivpu_device *vdev)
mutex_unlock(&vdev->context_list_lock);
}
-static void ivpu_cmdq_unregister_all(struct ivpu_file_priv *file_priv)
-{
- struct ivpu_cmdq *cmdq;
- unsigned long cmdq_id;
-
- xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
- ivpu_cmdq_unregister(file_priv, cmdq);
-}
-
void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv)
{
struct ivpu_device *vdev = file_priv->vdev;
+ struct ivpu_cmdq *cmdq;
+ unsigned long cmdq_id;
lockdep_assert_held(&file_priv->lock);
- ivpu_cmdq_unregister_all(file_priv);
+ xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
+ ivpu_cmdq_unregister(file_priv, cmdq);
if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_OS)
ivpu_jsm_context_release(vdev, file_priv->ctx.id);
+
+ file_priv->aborted = true;
}
static int ivpu_cmdq_push_job(struct ivpu_cmdq *cmdq, struct ivpu_job *job)
@@ -513,16 +512,14 @@ static struct ivpu_job *ivpu_job_remove_from_submitted_jobs(struct ivpu_device *
{
struct ivpu_job *job;
- xa_lock(&vdev->submitted_jobs_xa);
- job = __xa_erase(&vdev->submitted_jobs_xa, job_id);
+ lockdep_assert_held(&vdev->submitted_jobs_lock);
+ job = xa_erase(&vdev->submitted_jobs_xa, job_id);
if (xa_empty(&vdev->submitted_jobs_xa) && job) {
vdev->busy_time = ktime_add(ktime_sub(ktime_get(), vdev->busy_start_ts),
vdev->busy_time);
}
- xa_unlock(&vdev->submitted_jobs_xa);
-
return job;
}
@@ -530,6 +527,8 @@ static int ivpu_job_signal_and_destroy(struct ivpu_device *vdev, u32 job_id, u32
{
struct ivpu_job *job;
+ lockdep_assert_held(&vdev->submitted_jobs_lock);
+
job = ivpu_job_remove_from_submitted_jobs(vdev, job_id);
if (!job)
return -ENOENT;
@@ -548,6 +547,10 @@ static int ivpu_job_signal_and_destroy(struct ivpu_device *vdev, u32 job_id, u32
ivpu_stop_job_timeout_detection(vdev);
ivpu_rpm_put(vdev);
+
+ if (!xa_empty(&vdev->submitted_jobs_xa))
+ ivpu_start_job_timeout_detection(vdev);
+
return 0;
}
@@ -556,8 +559,26 @@ void ivpu_jobs_abort_all(struct ivpu_device *vdev)
struct ivpu_job *job;
unsigned long id;
+ mutex_lock(&vdev->submitted_jobs_lock);
+
xa_for_each(&vdev->submitted_jobs_xa, id, job)
ivpu_job_signal_and_destroy(vdev, id, DRM_IVPU_JOB_STATUS_ABORTED);
+
+ mutex_unlock(&vdev->submitted_jobs_lock);
+}
+
+void ivpu_cmdq_abort_all_jobs(struct ivpu_device *vdev, u32 ctx_id, u32 cmdq_id)
+{
+ struct ivpu_job *job;
+ unsigned long id;
+
+ mutex_lock(&vdev->submitted_jobs_lock);
+
+ xa_for_each(&vdev->submitted_jobs_xa, id, job)
+ if (job->file_priv->ctx.id == ctx_id && job->cmdq_id == cmdq_id)
+ ivpu_job_signal_and_destroy(vdev, id, DRM_IVPU_JOB_STATUS_ABORTED);
+
+ mutex_unlock(&vdev->submitted_jobs_lock);
}
static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
@@ -590,15 +611,18 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
goto err_unlock_file_priv;
}
- xa_lock(&vdev->submitted_jobs_xa);
+ job->cmdq_id = cmdq->id;
+
+ mutex_lock(&vdev->submitted_jobs_lock);
+
is_first_job = xa_empty(&vdev->submitted_jobs_xa);
- ret = __xa_alloc_cyclic(&vdev->submitted_jobs_xa, &job->job_id, job, file_priv->job_limit,
- &file_priv->job_id_next, GFP_KERNEL);
+ ret = xa_alloc_cyclic(&vdev->submitted_jobs_xa, &job->job_id, job, file_priv->job_limit,
+ &file_priv->job_id_next, GFP_KERNEL);
if (ret < 0) {
ivpu_dbg(vdev, JOB, "Too many active jobs in ctx %d\n",
file_priv->ctx.id);
ret = -EBUSY;
- goto err_unlock_submitted_jobs_xa;
+ goto err_unlock_submitted_jobs;
}
ret = ivpu_cmdq_push_job(cmdq, job);
@@ -621,19 +645,21 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
job->job_id, file_priv->ctx.id, job->engine_idx, cmdq->priority,
job->cmd_buf_vpu_addr, cmdq->jobq->header.tail);
- xa_unlock(&vdev->submitted_jobs_xa);
-
+ mutex_unlock(&vdev->submitted_jobs_lock);
mutex_unlock(&file_priv->lock);
- if (unlikely(ivpu_test_mode & IVPU_TEST_MODE_NULL_HW))
+ if (unlikely(ivpu_test_mode & IVPU_TEST_MODE_NULL_HW)) {
+ mutex_lock(&vdev->submitted_jobs_lock);
ivpu_job_signal_and_destroy(vdev, job->job_id, VPU_JSM_STATUS_SUCCESS);
+ mutex_unlock(&vdev->submitted_jobs_lock);
+ }
return 0;
err_erase_xa:
- __xa_erase(&vdev->submitted_jobs_xa, job->job_id);
-err_unlock_submitted_jobs_xa:
- xa_unlock(&vdev->submitted_jobs_xa);
+ xa_erase(&vdev->submitted_jobs_xa, job->job_id);
+err_unlock_submitted_jobs:
+ mutex_unlock(&vdev->submitted_jobs_lock);
err_unlock_file_priv:
mutex_unlock(&file_priv->lock);
ivpu_rpm_put(vdev);
@@ -843,19 +869,33 @@ int ivpu_cmdq_create_ioctl(struct drm_device *dev, void *data, struct drm_file *
int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
{
struct ivpu_file_priv *file_priv = file->driver_priv;
+ struct ivpu_device *vdev = file_priv->vdev;
struct drm_ivpu_cmdq_destroy *args = data;
struct ivpu_cmdq *cmdq;
+ u32 cmdq_id;
int ret = 0;
mutex_lock(&file_priv->lock);
cmdq = xa_load(&file_priv->cmdq_xa, args->cmdq_id);
- if (!cmdq || cmdq->is_legacy) {
+ if (!cmdq || !cmdq->is_valid || cmdq->is_legacy) {
ret = -ENOENT;
goto unlock;
}
+ /*
+ * There is no way to stop executing jobs per command queue
+ * in OS scheduling mode, mark command queue as invalid instead
+ * and it will be freed together with context release.
+ */
+ if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_OS) {
+ cmdq->is_valid = false;
+ goto unlock;
+ }
+
+ cmdq_id = cmdq->id;
ivpu_cmdq_destroy(file_priv, cmdq);
+ ivpu_cmdq_abort_all_jobs(vdev, file_priv->ctx.id, cmdq_id);
unlock:
mutex_unlock(&file_priv->lock);
return ret;
@@ -866,7 +906,6 @@ ivpu_job_done_callback(struct ivpu_device *vdev, struct ivpu_ipc_hdr *ipc_hdr,
struct vpu_jsm_msg *jsm_msg)
{
struct vpu_ipc_msg_payload_job_done *payload;
- int ret;
if (!jsm_msg) {
ivpu_err(vdev, "IPC message has no JSM payload\n");
@@ -879,9 +918,10 @@ ivpu_job_done_callback(struct ivpu_device *vdev, struct ivpu_ipc_hdr *ipc_hdr,
}
payload = (struct vpu_ipc_msg_payload_job_done *)&jsm_msg->payload;
- ret = ivpu_job_signal_and_destroy(vdev, payload->job_id, payload->job_status);
- if (!ret && !xa_empty(&vdev->submitted_jobs_xa))
- ivpu_start_job_timeout_detection(vdev);
+
+ mutex_lock(&vdev->submitted_jobs_lock);
+ ivpu_job_signal_and_destroy(vdev, payload->job_id, payload->job_status);
+ mutex_unlock(&vdev->submitted_jobs_lock);
}
void ivpu_job_done_consumer_init(struct ivpu_device *vdev)
@@ -894,3 +934,36 @@ void ivpu_job_done_consumer_fini(struct ivpu_device *vdev)
{
ivpu_ipc_consumer_del(vdev, &vdev->job_done_consumer);
}
+
+void ivpu_context_abort_thread_handler(struct work_struct *work)
+{
+ struct ivpu_device *vdev = container_of(work, struct ivpu_device, context_abort_work);
+ struct ivpu_file_priv *file_priv;
+ unsigned long ctx_id;
+ struct ivpu_job *job;
+ unsigned long id;
+
+ mutex_lock(&vdev->context_list_lock);
+ xa_for_each(&vdev->context_xa, ctx_id, file_priv) {
+ if (!file_priv->has_mmu_faults || file_priv->aborted)
+ continue;
+
+ mutex_lock(&file_priv->lock);
+ ivpu_context_abort_locked(file_priv);
+ mutex_unlock(&file_priv->lock);
+ }
+ mutex_unlock(&vdev->context_list_lock);
+
+ if (vdev->fw->sched_mode != VPU_SCHEDULING_MODE_HW)
+ return;
+ /*
+ * In hardware scheduling mode NPU already has stopped processing jobs
+ * and won't send us any further notifications, thus we have to free job related resources
+ * and notify userspace
+ */
+ mutex_lock(&vdev->submitted_jobs_lock);
+ xa_for_each(&vdev->submitted_jobs_xa, id, job)
+ if (job->file_priv->aborted)
+ ivpu_job_signal_and_destroy(vdev, job->job_id, DRM_IVPU_JOB_STATUS_ABORTED);
+ mutex_unlock(&vdev->submitted_jobs_lock);
+}
diff --git a/drivers/accel/ivpu/ivpu_job.h b/drivers/accel/ivpu/ivpu_job.h
index 4d31277bcc41..fef8aed1fc88 100644
--- a/drivers/accel/ivpu/ivpu_job.h
+++ b/drivers/accel/ivpu/ivpu_job.h
@@ -31,6 +31,7 @@ struct ivpu_cmdq {
u32 id;
u32 db_id;
u8 priority;
+ bool is_valid;
bool is_legacy;
};
@@ -51,6 +52,7 @@ struct ivpu_job {
struct ivpu_file_priv *file_priv;
struct dma_fence *done_fence;
u64 cmd_buf_vpu_addr;
+ u32 cmdq_id;
u32 job_id;
u32 engine_idx;
size_t bo_count;
@@ -66,9 +68,11 @@ void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv);
void ivpu_cmdq_release_all_locked(struct ivpu_file_priv *file_priv);
void ivpu_cmdq_reset_all_contexts(struct ivpu_device *vdev);
+void ivpu_cmdq_abort_all_jobs(struct ivpu_device *vdev, u32 ctx_id, u32 cmdq_id);
void ivpu_job_done_consumer_init(struct ivpu_device *vdev);
void ivpu_job_done_consumer_fini(struct ivpu_device *vdev);
+void ivpu_context_abort_thread_handler(struct work_struct *work);
void ivpu_jobs_abort_all(struct ivpu_device *vdev);
diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c
index 26ef52fbb93e..21f820dd0c65 100644
--- a/drivers/accel/ivpu/ivpu_mmu.c
+++ b/drivers/accel/ivpu/ivpu_mmu.c
@@ -890,8 +890,7 @@ void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev)
REGV_WR32(IVPU_MMU_REG_EVTQ_CONS_SEC, vdev->mmu->evtq.cons);
}
- if (!kfifo_put(&vdev->hw->irq.fifo, IVPU_HW_IRQ_SRC_MMU_EVTQ))
- ivpu_err_ratelimited(vdev, "IRQ FIFO full\n");
+ queue_work(system_wq, &vdev->context_abort_work);
}
void ivpu_mmu_evtq_dump(struct ivpu_device *vdev)
diff --git a/drivers/accel/ivpu/ivpu_sysfs.c b/drivers/accel/ivpu/ivpu_sysfs.c
index 616477fc17fa..8a616791c32f 100644
--- a/drivers/accel/ivpu/ivpu_sysfs.c
+++ b/drivers/accel/ivpu/ivpu_sysfs.c
@@ -30,11 +30,12 @@ npu_busy_time_us_show(struct device *dev, struct device_attribute *attr, char *b
struct ivpu_device *vdev = to_ivpu_device(drm);
ktime_t total, now = 0;
- xa_lock(&vdev->submitted_jobs_xa);
+ mutex_lock(&vdev->submitted_jobs_lock);
+
total = vdev->busy_time;
if (!xa_empty(&vdev->submitted_jobs_xa))
now = ktime_sub(ktime_get(), vdev->busy_start_ts);
- xa_unlock(&vdev->submitted_jobs_xa);
+ mutex_unlock(&vdev->submitted_jobs_lock);
return sysfs_emit(buf, "%lld\n", ktime_to_us(ktime_add(total, now)));
}
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 03/14] accel/ivpu: Abort all jobs after command queue unregister
2025-01-07 17:32 ` [PATCH 03/14] accel/ivpu: Abort all jobs after command queue unregister Maciej Falkowski
@ 2025-01-09 8:23 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:23 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo, Karol Wachowski
Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> From: Karol Wachowski <karol.wachowski@intel.com>
>
> With hardware scheduler it is not expected to receive JOB_DONE
> notifications from NPU FW for the jobs aborted due to command queue destroy
> JSM command.
>
> Remove jobs submitted to unregistered command queue from submitted_jobs_xa
> to avoid triggering a TDR in such case.
>
> Add explicit submitted_jobs_lock that protects access to list of submitted
> jobs which is now used to find jobs to abort.
>
> Move context abort procedure to separate work queue not to slow down
> handling of IPCs or DCT requests in case where job abort takes longer,
> especially when destruction of the last job of a specific context results
> in context release.
>
> Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_drv.c | 32 ++------
> drivers/accel/ivpu/ivpu_drv.h | 2 +
> drivers/accel/ivpu/ivpu_job.c | 137 ++++++++++++++++++++++++--------
> drivers/accel/ivpu/ivpu_job.h | 4 +
> drivers/accel/ivpu/ivpu_mmu.c | 3 +-
> drivers/accel/ivpu/ivpu_sysfs.c | 5 +-
> 6 files changed, 121 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
> index f4171536640b..300eea8c305f 100644
> --- a/drivers/accel/ivpu/ivpu_drv.c
> +++ b/drivers/accel/ivpu/ivpu_drv.c
> @@ -36,8 +36,6 @@
> #define DRIVER_VERSION_STR "1.0.0 " UTS_RELEASE
> #endif
>
> -static struct lock_class_key submitted_jobs_xa_lock_class_key;
> -
> int ivpu_dbg_mask;
> module_param_named(dbg_mask, ivpu_dbg_mask, int, 0644);
> MODULE_PARM_DESC(dbg_mask, "Driver debug mask. See IVPU_DBG_* macros.");
> @@ -467,26 +465,6 @@ static const struct drm_driver driver = {
> .major = 1,
> };
>
> -static void ivpu_context_abort_invalid(struct ivpu_device *vdev)
> -{
> - struct ivpu_file_priv *file_priv;
> - unsigned long ctx_id;
> -
> - mutex_lock(&vdev->context_list_lock);
> -
> - xa_for_each(&vdev->context_xa, ctx_id, file_priv) {
> - if (!file_priv->has_mmu_faults || file_priv->aborted)
> - continue;
> -
> - mutex_lock(&file_priv->lock);
> - ivpu_context_abort_locked(file_priv);
> - file_priv->aborted = true;
> - mutex_unlock(&file_priv->lock);
> - }
> -
> - mutex_unlock(&vdev->context_list_lock);
> -}
> -
> static irqreturn_t ivpu_irq_thread_handler(int irq, void *arg)
> {
> struct ivpu_device *vdev = arg;
> @@ -500,9 +478,6 @@ static irqreturn_t ivpu_irq_thread_handler(int irq, void *arg)
> case IVPU_HW_IRQ_SRC_IPC:
> ivpu_ipc_irq_thread_handler(vdev);
> break;
> - case IVPU_HW_IRQ_SRC_MMU_EVTQ:
> - ivpu_context_abort_invalid(vdev);
> - break;
> case IVPU_HW_IRQ_SRC_DCT:
> ivpu_pm_dct_irq_thread_handler(vdev);
> break;
> @@ -619,16 +594,21 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
> xa_init_flags(&vdev->context_xa, XA_FLAGS_ALLOC | XA_FLAGS_LOCK_IRQ);
> xa_init_flags(&vdev->submitted_jobs_xa, XA_FLAGS_ALLOC1);
> xa_init_flags(&vdev->db_xa, XA_FLAGS_ALLOC1);
> - lockdep_set_class(&vdev->submitted_jobs_xa.xa_lock, &submitted_jobs_xa_lock_class_key);
> INIT_LIST_HEAD(&vdev->bo_list);
>
> vdev->db_limit.min = IVPU_MIN_DB;
> vdev->db_limit.max = IVPU_MAX_DB;
>
> + INIT_WORK(&vdev->context_abort_work, ivpu_context_abort_thread_handler);
> +
> ret = drmm_mutex_init(&vdev->drm, &vdev->context_list_lock);
> if (ret)
> goto err_xa_destroy;
>
> + ret = drmm_mutex_init(&vdev->drm, &vdev->submitted_jobs_lock);
> + if (ret)
> + goto err_xa_destroy;
> +
> ret = drmm_mutex_init(&vdev->drm, &vdev->bo_list_lock);
> if (ret)
> goto err_xa_destroy;
> diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
> index 3fdff3f6cffd..ebfcf3e42a3d 100644
> --- a/drivers/accel/ivpu/ivpu_drv.h
> +++ b/drivers/accel/ivpu/ivpu_drv.h
> @@ -137,6 +137,7 @@ struct ivpu_device {
> struct mutex context_list_lock; /* Protects user context addition/removal */
> struct xarray context_xa;
> struct xa_limit context_xa_limit;
> + struct work_struct context_abort_work;
>
> struct xarray db_xa;
> struct xa_limit db_limit;
> @@ -145,6 +146,7 @@ struct ivpu_device {
> struct mutex bo_list_lock; /* Protects bo_list */
> struct list_head bo_list;
>
> + struct mutex submitted_jobs_lock; /* Protects submitted_jobs */
> struct xarray submitted_jobs_xa;
> struct ivpu_ipc_consumer job_done_consumer;
>
> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
> index 43507d3aea51..7fed3c8406ee 100644
> --- a/drivers/accel/ivpu/ivpu_job.c
> +++ b/drivers/accel/ivpu/ivpu_job.c
> @@ -122,6 +122,7 @@ static struct ivpu_cmdq *ivpu_cmdq_create(struct ivpu_file_priv *file_priv, u8 p
>
> cmdq->priority = priority;
> cmdq->is_legacy = is_legacy;
> + cmdq->is_valid = true;
>
> ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &cmdq->id, cmdq, file_priv->cmdq_limit,
> &file_priv->cmdq_id_next, GFP_KERNEL);
> @@ -130,6 +131,7 @@ static struct ivpu_cmdq *ivpu_cmdq_create(struct ivpu_file_priv *file_priv, u8 p
> goto err_free_cmdq;
> }
>
> + ivpu_dbg(vdev, JOB, "Command queue %d created, ctx %d\n", cmdq->id, file_priv->ctx.id);
> return cmdq;
>
> err_free_cmdq:
> @@ -247,7 +249,8 @@ static int ivpu_cmdq_unregister(struct ivpu_file_priv *file_priv, struct ivpu_cm
> if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW) {
> ret = ivpu_jsm_hws_destroy_cmdq(vdev, file_priv->ctx.id, cmdq->id);
> if (!ret)
> - ivpu_dbg(vdev, JOB, "Command queue %d destroyed\n", cmdq->id);
> + ivpu_dbg(vdev, JOB, "Command queue %d destroyed, ctx %d\n",
> + cmdq->id, file_priv->ctx.id);
> }
>
> ret = ivpu_jsm_unregister_db(vdev, cmdq->db_id);
> @@ -303,8 +306,8 @@ static struct ivpu_cmdq *ivpu_cmdq_acquire(struct ivpu_file_priv *file_priv, u32
> lockdep_assert_held(&file_priv->lock);
>
> cmdq = xa_load(&file_priv->cmdq_xa, cmdq_id);
> - if (!cmdq) {
> - ivpu_err(vdev, "Failed to find command queue with ID: %u\n", cmdq_id);
> + if (!cmdq || !cmdq->is_valid) {
> + ivpu_warn_ratelimited(vdev, "Failed to find command queue with ID: %u\n", cmdq_id);
> return NULL;
> }
>
> @@ -356,25 +359,21 @@ void ivpu_cmdq_reset_all_contexts(struct ivpu_device *vdev)
> mutex_unlock(&vdev->context_list_lock);
> }
>
> -static void ivpu_cmdq_unregister_all(struct ivpu_file_priv *file_priv)
> -{
> - struct ivpu_cmdq *cmdq;
> - unsigned long cmdq_id;
> -
> - xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
> - ivpu_cmdq_unregister(file_priv, cmdq);
> -}
> -
> void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv)
> {
> struct ivpu_device *vdev = file_priv->vdev;
> + struct ivpu_cmdq *cmdq;
> + unsigned long cmdq_id;
>
> lockdep_assert_held(&file_priv->lock);
>
> - ivpu_cmdq_unregister_all(file_priv);
> + xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
> + ivpu_cmdq_unregister(file_priv, cmdq);
>
> if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_OS)
> ivpu_jsm_context_release(vdev, file_priv->ctx.id);
> +
> + file_priv->aborted = true;
> }
>
> static int ivpu_cmdq_push_job(struct ivpu_cmdq *cmdq, struct ivpu_job *job)
> @@ -513,16 +512,14 @@ static struct ivpu_job *ivpu_job_remove_from_submitted_jobs(struct ivpu_device *
> {
> struct ivpu_job *job;
>
> - xa_lock(&vdev->submitted_jobs_xa);
> - job = __xa_erase(&vdev->submitted_jobs_xa, job_id);
> + lockdep_assert_held(&vdev->submitted_jobs_lock);
>
> + job = xa_erase(&vdev->submitted_jobs_xa, job_id);
> if (xa_empty(&vdev->submitted_jobs_xa) && job) {
> vdev->busy_time = ktime_add(ktime_sub(ktime_get(), vdev->busy_start_ts),
> vdev->busy_time);
> }
>
> - xa_unlock(&vdev->submitted_jobs_xa);
> -
> return job;
> }
>
> @@ -530,6 +527,8 @@ static int ivpu_job_signal_and_destroy(struct ivpu_device *vdev, u32 job_id, u32
> {
> struct ivpu_job *job;
>
> + lockdep_assert_held(&vdev->submitted_jobs_lock);
> +
> job = ivpu_job_remove_from_submitted_jobs(vdev, job_id);
> if (!job)
> return -ENOENT;
> @@ -548,6 +547,10 @@ static int ivpu_job_signal_and_destroy(struct ivpu_device *vdev, u32 job_id, u32
> ivpu_stop_job_timeout_detection(vdev);
>
> ivpu_rpm_put(vdev);
> +
> + if (!xa_empty(&vdev->submitted_jobs_xa))
> + ivpu_start_job_timeout_detection(vdev);
> +
> return 0;
> }
>
> @@ -556,8 +559,26 @@ void ivpu_jobs_abort_all(struct ivpu_device *vdev)
> struct ivpu_job *job;
> unsigned long id;
>
> + mutex_lock(&vdev->submitted_jobs_lock);
> +
> xa_for_each(&vdev->submitted_jobs_xa, id, job)
> ivpu_job_signal_and_destroy(vdev, id, DRM_IVPU_JOB_STATUS_ABORTED);
> +
> + mutex_unlock(&vdev->submitted_jobs_lock);
> +}
> +
> +void ivpu_cmdq_abort_all_jobs(struct ivpu_device *vdev, u32 ctx_id, u32 cmdq_id)
> +{
> + struct ivpu_job *job;
> + unsigned long id;
> +
> + mutex_lock(&vdev->submitted_jobs_lock);
> +
> + xa_for_each(&vdev->submitted_jobs_xa, id, job)
> + if (job->file_priv->ctx.id == ctx_id && job->cmdq_id == cmdq_id)
> + ivpu_job_signal_and_destroy(vdev, id, DRM_IVPU_JOB_STATUS_ABORTED);
> +
> + mutex_unlock(&vdev->submitted_jobs_lock);
> }
>
> static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
> @@ -590,15 +611,18 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
> goto err_unlock_file_priv;
> }
>
> - xa_lock(&vdev->submitted_jobs_xa);
> + job->cmdq_id = cmdq->id;
> +
> + mutex_lock(&vdev->submitted_jobs_lock);
> +
> is_first_job = xa_empty(&vdev->submitted_jobs_xa);
> - ret = __xa_alloc_cyclic(&vdev->submitted_jobs_xa, &job->job_id, job, file_priv->job_limit,
> - &file_priv->job_id_next, GFP_KERNEL);
> + ret = xa_alloc_cyclic(&vdev->submitted_jobs_xa, &job->job_id, job, file_priv->job_limit,
> + &file_priv->job_id_next, GFP_KERNEL);
> if (ret < 0) {
> ivpu_dbg(vdev, JOB, "Too many active jobs in ctx %d\n",
> file_priv->ctx.id);
> ret = -EBUSY;
> - goto err_unlock_submitted_jobs_xa;
> + goto err_unlock_submitted_jobs;
> }
>
> ret = ivpu_cmdq_push_job(cmdq, job);
> @@ -621,19 +645,21 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
> job->job_id, file_priv->ctx.id, job->engine_idx, cmdq->priority,
> job->cmd_buf_vpu_addr, cmdq->jobq->header.tail);
>
> - xa_unlock(&vdev->submitted_jobs_xa);
> -
> + mutex_unlock(&vdev->submitted_jobs_lock);
> mutex_unlock(&file_priv->lock);
>
> - if (unlikely(ivpu_test_mode & IVPU_TEST_MODE_NULL_HW))
> + if (unlikely(ivpu_test_mode & IVPU_TEST_MODE_NULL_HW)) {
> + mutex_lock(&vdev->submitted_jobs_lock);
> ivpu_job_signal_and_destroy(vdev, job->job_id, VPU_JSM_STATUS_SUCCESS);
> + mutex_unlock(&vdev->submitted_jobs_lock);
> + }
>
> return 0;
>
> err_erase_xa:
> - __xa_erase(&vdev->submitted_jobs_xa, job->job_id);
> -err_unlock_submitted_jobs_xa:
> - xa_unlock(&vdev->submitted_jobs_xa);
> + xa_erase(&vdev->submitted_jobs_xa, job->job_id);
> +err_unlock_submitted_jobs:
> + mutex_unlock(&vdev->submitted_jobs_lock);
> err_unlock_file_priv:
> mutex_unlock(&file_priv->lock);
> ivpu_rpm_put(vdev);
> @@ -843,19 +869,33 @@ int ivpu_cmdq_create_ioctl(struct drm_device *dev, void *data, struct drm_file *
> int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> {
> struct ivpu_file_priv *file_priv = file->driver_priv;
> + struct ivpu_device *vdev = file_priv->vdev;
> struct drm_ivpu_cmdq_destroy *args = data;
> struct ivpu_cmdq *cmdq;
> + u32 cmdq_id;
> int ret = 0;
>
> mutex_lock(&file_priv->lock);
>
> cmdq = xa_load(&file_priv->cmdq_xa, args->cmdq_id);
> - if (!cmdq || cmdq->is_legacy) {
> + if (!cmdq || !cmdq->is_valid || cmdq->is_legacy) {
> ret = -ENOENT;
> goto unlock;
> }
>
> + /*
> + * There is no way to stop executing jobs per command queue
> + * in OS scheduling mode, mark command queue as invalid instead
> + * and it will be freed together with context release.
> + */
> + if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_OS) {
> + cmdq->is_valid = false;
> + goto unlock;
> + }
> +
> + cmdq_id = cmdq->id;
> ivpu_cmdq_destroy(file_priv, cmdq);
> + ivpu_cmdq_abort_all_jobs(vdev, file_priv->ctx.id, cmdq_id);
> unlock:
> mutex_unlock(&file_priv->lock);
> return ret;
> @@ -866,7 +906,6 @@ ivpu_job_done_callback(struct ivpu_device *vdev, struct ivpu_ipc_hdr *ipc_hdr,
> struct vpu_jsm_msg *jsm_msg)
> {
> struct vpu_ipc_msg_payload_job_done *payload;
> - int ret;
>
> if (!jsm_msg) {
> ivpu_err(vdev, "IPC message has no JSM payload\n");
> @@ -879,9 +918,10 @@ ivpu_job_done_callback(struct ivpu_device *vdev, struct ivpu_ipc_hdr *ipc_hdr,
> }
>
> payload = (struct vpu_ipc_msg_payload_job_done *)&jsm_msg->payload;
> - ret = ivpu_job_signal_and_destroy(vdev, payload->job_id, payload->job_status);
> - if (!ret && !xa_empty(&vdev->submitted_jobs_xa))
> - ivpu_start_job_timeout_detection(vdev);
> +
> + mutex_lock(&vdev->submitted_jobs_lock);
> + ivpu_job_signal_and_destroy(vdev, payload->job_id, payload->job_status);
> + mutex_unlock(&vdev->submitted_jobs_lock);
> }
>
> void ivpu_job_done_consumer_init(struct ivpu_device *vdev)
> @@ -894,3 +934,36 @@ void ivpu_job_done_consumer_fini(struct ivpu_device *vdev)
> {
> ivpu_ipc_consumer_del(vdev, &vdev->job_done_consumer);
> }
> +
> +void ivpu_context_abort_thread_handler(struct work_struct *work)
> +{
> + struct ivpu_device *vdev = container_of(work, struct ivpu_device, context_abort_work);
> + struct ivpu_file_priv *file_priv;
> + unsigned long ctx_id;
> + struct ivpu_job *job;
> + unsigned long id;
> +
> + mutex_lock(&vdev->context_list_lock);
> + xa_for_each(&vdev->context_xa, ctx_id, file_priv) {
> + if (!file_priv->has_mmu_faults || file_priv->aborted)
> + continue;
> +
> + mutex_lock(&file_priv->lock);
> + ivpu_context_abort_locked(file_priv);
> + mutex_unlock(&file_priv->lock);
> + }
> + mutex_unlock(&vdev->context_list_lock);
> +
> + if (vdev->fw->sched_mode != VPU_SCHEDULING_MODE_HW)
> + return;
> + /*
> + * In hardware scheduling mode NPU already has stopped processing jobs
> + * and won't send us any further notifications, thus we have to free job related resources
> + * and notify userspace
> + */
> + mutex_lock(&vdev->submitted_jobs_lock);
> + xa_for_each(&vdev->submitted_jobs_xa, id, job)
> + if (job->file_priv->aborted)
> + ivpu_job_signal_and_destroy(vdev, job->job_id, DRM_IVPU_JOB_STATUS_ABORTED);
> + mutex_unlock(&vdev->submitted_jobs_lock);
> +}
> diff --git a/drivers/accel/ivpu/ivpu_job.h b/drivers/accel/ivpu/ivpu_job.h
> index 4d31277bcc41..fef8aed1fc88 100644
> --- a/drivers/accel/ivpu/ivpu_job.h
> +++ b/drivers/accel/ivpu/ivpu_job.h
> @@ -31,6 +31,7 @@ struct ivpu_cmdq {
> u32 id;
> u32 db_id;
> u8 priority;
> + bool is_valid;
> bool is_legacy;
> };
>
> @@ -51,6 +52,7 @@ struct ivpu_job {
> struct ivpu_file_priv *file_priv;
> struct dma_fence *done_fence;
> u64 cmd_buf_vpu_addr;
> + u32 cmdq_id;
> u32 job_id;
> u32 engine_idx;
> size_t bo_count;
> @@ -66,9 +68,11 @@ void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv);
>
> void ivpu_cmdq_release_all_locked(struct ivpu_file_priv *file_priv);
> void ivpu_cmdq_reset_all_contexts(struct ivpu_device *vdev);
> +void ivpu_cmdq_abort_all_jobs(struct ivpu_device *vdev, u32 ctx_id, u32 cmdq_id);
>
> void ivpu_job_done_consumer_init(struct ivpu_device *vdev);
> void ivpu_job_done_consumer_fini(struct ivpu_device *vdev);
> +void ivpu_context_abort_thread_handler(struct work_struct *work);
>
> void ivpu_jobs_abort_all(struct ivpu_device *vdev);
>
> diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c
> index 26ef52fbb93e..21f820dd0c65 100644
> --- a/drivers/accel/ivpu/ivpu_mmu.c
> +++ b/drivers/accel/ivpu/ivpu_mmu.c
> @@ -890,8 +890,7 @@ void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev)
> REGV_WR32(IVPU_MMU_REG_EVTQ_CONS_SEC, vdev->mmu->evtq.cons);
> }
>
> - if (!kfifo_put(&vdev->hw->irq.fifo, IVPU_HW_IRQ_SRC_MMU_EVTQ))
> - ivpu_err_ratelimited(vdev, "IRQ FIFO full\n");
> + queue_work(system_wq, &vdev->context_abort_work);
> }
>
> void ivpu_mmu_evtq_dump(struct ivpu_device *vdev)
> diff --git a/drivers/accel/ivpu/ivpu_sysfs.c b/drivers/accel/ivpu/ivpu_sysfs.c
> index 616477fc17fa..8a616791c32f 100644
> --- a/drivers/accel/ivpu/ivpu_sysfs.c
> +++ b/drivers/accel/ivpu/ivpu_sysfs.c
> @@ -30,11 +30,12 @@ npu_busy_time_us_show(struct device *dev, struct device_attribute *attr, char *b
> struct ivpu_device *vdev = to_ivpu_device(drm);
> ktime_t total, now = 0;
>
> - xa_lock(&vdev->submitted_jobs_xa);
> + mutex_lock(&vdev->submitted_jobs_lock);
> +
> total = vdev->busy_time;
> if (!xa_empty(&vdev->submitted_jobs_xa))
> now = ktime_sub(ktime_get(), vdev->busy_start_ts);
> - xa_unlock(&vdev->submitted_jobs_xa);
> + mutex_unlock(&vdev->submitted_jobs_lock);
>
> return sysfs_emit(buf, "%lld\n", ktime_to_us(ktime_add(total, now)));
> }
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 04/14] accel/ivpu: Expose NPU memory utilization info in sysfs
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
` (2 preceding siblings ...)
2025-01-07 17:32 ` [PATCH 03/14] accel/ivpu: Abort all jobs after command queue unregister Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-08 19:53 ` Lizhi Hou
2025-01-09 8:24 ` Jacek Lawrynowicz
2025-01-07 17:32 ` [PATCH 05/14] accel/ivpu: Use workqueue for IRQ handling Maciej Falkowski
` (11 subsequent siblings)
15 siblings, 2 replies; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel; +Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Maciej Falkowski
Expose NPU memory utilization info in sysfs in bytes
to show total memory used by NPU (FW + runtime).
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_sysfs.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/accel/ivpu/ivpu_sysfs.c b/drivers/accel/ivpu/ivpu_sysfs.c
index 8a616791c32f..97102feaf8dd 100644
--- a/drivers/accel/ivpu/ivpu_sysfs.c
+++ b/drivers/accel/ivpu/ivpu_sysfs.c
@@ -7,11 +7,14 @@
#include <linux/err.h>
#include "ivpu_drv.h"
+#include "ivpu_gem.h"
#include "ivpu_fw.h"
#include "ivpu_hw.h"
#include "ivpu_sysfs.h"
-/*
+/**
+ * DOC: npu_busy_time_us
+ *
* npu_busy_time_us is the time that the device spent executing jobs.
* The time is counted when and only when there are jobs submitted to firmware.
*
@@ -42,6 +45,30 @@ npu_busy_time_us_show(struct device *dev, struct device_attribute *attr, char *b
static DEVICE_ATTR_RO(npu_busy_time_us);
+/**
+ * DOC: npu_memory_utilization
+ *
+ * The npu_memory_utilization is used to report in bytes a current NPU memory utilization.
+ *
+ */
+static ssize_t
+npu_memory_utilization_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct drm_device *drm = dev_get_drvdata(dev);
+ struct ivpu_device *vdev = to_ivpu_device(drm);
+ struct ivpu_bo *bo;
+ u64 total_npu_memory = 0;
+
+ mutex_lock(&vdev->bo_list_lock);
+ list_for_each_entry(bo, &vdev->bo_list, bo_list_node)
+ total_npu_memory += bo->base.base.size;
+ mutex_unlock(&vdev->bo_list_lock);
+
+ return sysfs_emit(buf, "%lld\n", total_npu_memory);
+}
+
+static DEVICE_ATTR_RO(npu_memory_utilization);
+
/**
* DOC: sched_mode
*
@@ -65,6 +92,7 @@ static DEVICE_ATTR_RO(sched_mode);
static struct attribute *ivpu_dev_attrs[] = {
&dev_attr_npu_busy_time_us.attr,
+ &dev_attr_npu_memory_utilization.attr,
&dev_attr_sched_mode.attr,
NULL,
};
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 04/14] accel/ivpu: Expose NPU memory utilization info in sysfs
2025-01-07 17:32 ` [PATCH 04/14] accel/ivpu: Expose NPU memory utilization info in sysfs Maciej Falkowski
@ 2025-01-08 19:53 ` Lizhi Hou
2025-01-09 8:19 ` Jacek Lawrynowicz
2025-01-09 8:24 ` Jacek Lawrynowicz
1 sibling, 1 reply; 33+ messages in thread
From: Lizhi Hou @ 2025-01-08 19:53 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz
On 1/7/25 09:32, Maciej Falkowski wrote:
> Expose NPU memory utilization info in sysfs in bytes
> to show total memory used by NPU (FW + runtime).
>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_sysfs.c | 30 +++++++++++++++++++++++++++++-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_sysfs.c b/drivers/accel/ivpu/ivpu_sysfs.c
> index 8a616791c32f..97102feaf8dd 100644
> --- a/drivers/accel/ivpu/ivpu_sysfs.c
> +++ b/drivers/accel/ivpu/ivpu_sysfs.c
> @@ -7,11 +7,14 @@
> #include <linux/err.h>
>
> #include "ivpu_drv.h"
> +#include "ivpu_gem.h"
> #include "ivpu_fw.h"
> #include "ivpu_hw.h"
> #include "ivpu_sysfs.h"
>
> -/*
> +/**
> + * DOC: npu_busy_time_us
> + *
> * npu_busy_time_us is the time that the device spent executing jobs.
> * The time is counted when and only when there are jobs submitted to firmware.
> *
> @@ -42,6 +45,30 @@ npu_busy_time_us_show(struct device *dev, struct device_attribute *attr, char *b
>
> static DEVICE_ATTR_RO(npu_busy_time_us);
>
> +/**
> + * DOC: npu_memory_utilization
> + *
> + * The npu_memory_utilization is used to report in bytes a current NPU memory utilization.
> + *
> + */
> +static ssize_t
> +npu_memory_utilization_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct drm_device *drm = dev_get_drvdata(dev);
> + struct ivpu_device *vdev = to_ivpu_device(drm);
> + struct ivpu_bo *bo;
> + u64 total_npu_memory = 0;
> +
> + mutex_lock(&vdev->bo_list_lock);
> + list_for_each_entry(bo, &vdev->bo_list, bo_list_node)
> + total_npu_memory += bo->base.base.size;
> + mutex_unlock(&vdev->bo_list_lock);
> +
> + return sysfs_emit(buf, "%lld\n", total_npu_memory);
> +}
> +
> +static DEVICE_ATTR_RO(npu_memory_utilization);
> +
Instead of adding sysfs, maybe implementing show_fdinfo callback and
call drm_show_memory_stats()?
Thanks,
Lizhi
> /**
> * DOC: sched_mode
> *
> @@ -65,6 +92,7 @@ static DEVICE_ATTR_RO(sched_mode);
>
> static struct attribute *ivpu_dev_attrs[] = {
> &dev_attr_npu_busy_time_us.attr,
> + &dev_attr_npu_memory_utilization.attr,
> &dev_attr_sched_mode.attr,
> NULL,
> };
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 04/14] accel/ivpu: Expose NPU memory utilization info in sysfs
2025-01-08 19:53 ` Lizhi Hou
@ 2025-01-09 8:19 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:19 UTC (permalink / raw)
To: Lizhi Hou, Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo
Hi,
On 1/8/2025 8:53 PM, Lizhi Hou wrote:
>
> On 1/7/25 09:32, Maciej Falkowski wrote:
>> Expose NPU memory utilization info in sysfs in bytes
>> to show total memory used by NPU (FW + runtime).
>>
>> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
>> ---
>> drivers/accel/ivpu/ivpu_sysfs.c | 30 +++++++++++++++++++++++++++++-
>> 1 file changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/accel/ivpu/ivpu_sysfs.c b/drivers/accel/ivpu/ivpu_sysfs.c
>> index 8a616791c32f..97102feaf8dd 100644
>> --- a/drivers/accel/ivpu/ivpu_sysfs.c
>> +++ b/drivers/accel/ivpu/ivpu_sysfs.c
>> @@ -7,11 +7,14 @@
>> #include <linux/err.h>
>> #include "ivpu_drv.h"
>> +#include "ivpu_gem.h"
>> #include "ivpu_fw.h"
>> #include "ivpu_hw.h"
>> #include "ivpu_sysfs.h"
>> -/*
>> +/**
>> + * DOC: npu_busy_time_us
>> + *
>> * npu_busy_time_us is the time that the device spent executing jobs.
>> * The time is counted when and only when there are jobs submitted to firmware.
>> *
>> @@ -42,6 +45,30 @@ npu_busy_time_us_show(struct device *dev, struct device_attribute *attr, char *b
>> static DEVICE_ATTR_RO(npu_busy_time_us);
>> +/**
>> + * DOC: npu_memory_utilization
>> + *
>> + * The npu_memory_utilization is used to report in bytes a current NPU memory utilization.
>> + *
>> + */
>> +static ssize_t
>> +npu_memory_utilization_show(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> + struct drm_device *drm = dev_get_drvdata(dev);
>> + struct ivpu_device *vdev = to_ivpu_device(drm);
>> + struct ivpu_bo *bo;
>> + u64 total_npu_memory = 0;
>> +
>> + mutex_lock(&vdev->bo_list_lock);
>> + list_for_each_entry(bo, &vdev->bo_list, bo_list_node)
>> + total_npu_memory += bo->base.base.size;
>> + mutex_unlock(&vdev->bo_list_lock);
>> +
>> + return sysfs_emit(buf, "%lld\n", total_npu_memory);
>> +}
>> +
>> +static DEVICE_ATTR_RO(npu_memory_utilization);
>> +
>
> Instead of adding sysfs, maybe implementing show_fdinfo callback and call drm_show_memory_stats()?
show_fdinfo cannot be used for global device stats. We are considering adding per context stats using show_fdinfo but for the moment total device memory usage is our priority.
Regards,
Jacek
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 04/14] accel/ivpu: Expose NPU memory utilization info in sysfs
2025-01-07 17:32 ` [PATCH 04/14] accel/ivpu: Expose NPU memory utilization info in sysfs Maciej Falkowski
2025-01-08 19:53 ` Lizhi Hou
@ 2025-01-09 8:24 ` Jacek Lawrynowicz
1 sibling, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:24 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo
Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> Expose NPU memory utilization info in sysfs in bytes
> to show total memory used by NPU (FW + runtime).
>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_sysfs.c | 30 +++++++++++++++++++++++++++++-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_sysfs.c b/drivers/accel/ivpu/ivpu_sysfs.c
> index 8a616791c32f..97102feaf8dd 100644
> --- a/drivers/accel/ivpu/ivpu_sysfs.c
> +++ b/drivers/accel/ivpu/ivpu_sysfs.c
> @@ -7,11 +7,14 @@
> #include <linux/err.h>
>
> #include "ivpu_drv.h"
> +#include "ivpu_gem.h"
> #include "ivpu_fw.h"
> #include "ivpu_hw.h"
> #include "ivpu_sysfs.h"
>
> -/*
> +/**
> + * DOC: npu_busy_time_us
> + *
> * npu_busy_time_us is the time that the device spent executing jobs.
> * The time is counted when and only when there are jobs submitted to firmware.
> *
> @@ -42,6 +45,30 @@ npu_busy_time_us_show(struct device *dev, struct device_attribute *attr, char *b
>
> static DEVICE_ATTR_RO(npu_busy_time_us);
>
> +/**
> + * DOC: npu_memory_utilization
> + *
> + * The npu_memory_utilization is used to report in bytes a current NPU memory utilization.
> + *
> + */
> +static ssize_t
> +npu_memory_utilization_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct drm_device *drm = dev_get_drvdata(dev);
> + struct ivpu_device *vdev = to_ivpu_device(drm);
> + struct ivpu_bo *bo;
> + u64 total_npu_memory = 0;
> +
> + mutex_lock(&vdev->bo_list_lock);
> + list_for_each_entry(bo, &vdev->bo_list, bo_list_node)
> + total_npu_memory += bo->base.base.size;
> + mutex_unlock(&vdev->bo_list_lock);
> +
> + return sysfs_emit(buf, "%lld\n", total_npu_memory);
> +}
> +
> +static DEVICE_ATTR_RO(npu_memory_utilization);
> +
> /**
> * DOC: sched_mode
> *
> @@ -65,6 +92,7 @@ static DEVICE_ATTR_RO(sched_mode);
>
> static struct attribute *ivpu_dev_attrs[] = {
> &dev_attr_npu_busy_time_us.attr,
> + &dev_attr_npu_memory_utilization.attr,
> &dev_attr_sched_mode.attr,
> NULL,
> };
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 05/14] accel/ivpu: Use workqueue for IRQ handling
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
` (3 preceding siblings ...)
2025-01-07 17:32 ` [PATCH 04/14] accel/ivpu: Expose NPU memory utilization info in sysfs Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-09 8:26 ` Jacek Lawrynowicz
2025-01-07 17:32 ` [PATCH 06/14] accel/ivpu: Dump only first MMU fault from single context Maciej Falkowski
` (10 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel; +Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Maciej Falkowski
Convert IRQ bottom half from the thread handler into workqueue.
This increases a stability in rare scenarios where driver on
debugging/hardening kernels processes IRQ too slow and misses
some interrupts due to it.
Workqueue handler also gives a very minor performance increase.
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_drv.c | 39 ++++++++-----------------------
drivers/accel/ivpu/ivpu_drv.h | 5 +++-
drivers/accel/ivpu/ivpu_hw.c | 5 ----
drivers/accel/ivpu/ivpu_hw.h | 9 -------
drivers/accel/ivpu/ivpu_hw_btrs.c | 3 +--
drivers/accel/ivpu/ivpu_ipc.c | 7 +++---
drivers/accel/ivpu/ivpu_ipc.h | 2 +-
drivers/accel/ivpu/ivpu_job.c | 2 +-
drivers/accel/ivpu/ivpu_job.h | 2 +-
drivers/accel/ivpu/ivpu_pm.c | 3 ++-
drivers/accel/ivpu/ivpu_pm.h | 2 +-
11 files changed, 24 insertions(+), 55 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index 300eea8c305f..9b0d99873fb3 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -7,6 +7,7 @@
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/pm_runtime.h>
+#include <linux/workqueue.h>
#include <generated/utsrelease.h>
#include <drm/drm_accel.h>
@@ -421,6 +422,9 @@ void ivpu_prepare_for_reset(struct ivpu_device *vdev)
{
ivpu_hw_irq_disable(vdev);
disable_irq(vdev->irq);
+ cancel_work_sync(&vdev->irq_ipc_work);
+ cancel_work_sync(&vdev->irq_dct_work);
+ cancel_work_sync(&vdev->context_abort_work);
ivpu_ipc_disable(vdev);
ivpu_mmu_disable(vdev);
}
@@ -465,31 +469,6 @@ static const struct drm_driver driver = {
.major = 1,
};
-static irqreturn_t ivpu_irq_thread_handler(int irq, void *arg)
-{
- struct ivpu_device *vdev = arg;
- u8 irq_src;
-
- if (kfifo_is_empty(&vdev->hw->irq.fifo))
- return IRQ_NONE;
-
- while (kfifo_get(&vdev->hw->irq.fifo, &irq_src)) {
- switch (irq_src) {
- case IVPU_HW_IRQ_SRC_IPC:
- ivpu_ipc_irq_thread_handler(vdev);
- break;
- case IVPU_HW_IRQ_SRC_DCT:
- ivpu_pm_dct_irq_thread_handler(vdev);
- break;
- default:
- ivpu_err_ratelimited(vdev, "Unknown IRQ source: %u\n", irq_src);
- break;
- }
- }
-
- return IRQ_HANDLED;
-}
-
static int ivpu_irq_init(struct ivpu_device *vdev)
{
struct pci_dev *pdev = to_pci_dev(vdev->drm.dev);
@@ -501,12 +480,16 @@ static int ivpu_irq_init(struct ivpu_device *vdev)
return ret;
}
+ INIT_WORK(&vdev->irq_ipc_work, ivpu_ipc_irq_work_fn);
+ INIT_WORK(&vdev->irq_dct_work, ivpu_pm_irq_dct_work_fn);
+ INIT_WORK(&vdev->context_abort_work, ivpu_context_abort_work_fn);
+
ivpu_irq_handlers_init(vdev);
vdev->irq = pci_irq_vector(pdev, 0);
- ret = devm_request_threaded_irq(vdev->drm.dev, vdev->irq, ivpu_hw_irq_handler,
- ivpu_irq_thread_handler, IRQF_NO_AUTOEN, DRIVER_NAME, vdev);
+ ret = devm_request_irq(vdev->drm.dev, vdev->irq, ivpu_hw_irq_handler,
+ IRQF_NO_AUTOEN, DRIVER_NAME, vdev);
if (ret)
ivpu_err(vdev, "Failed to request an IRQ %d\n", ret);
@@ -599,8 +582,6 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
vdev->db_limit.min = IVPU_MIN_DB;
vdev->db_limit.max = IVPU_MAX_DB;
- INIT_WORK(&vdev->context_abort_work, ivpu_context_abort_thread_handler);
-
ret = drmm_mutex_init(&vdev->drm, &vdev->context_list_lock);
if (ret)
goto err_xa_destroy;
diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
index ebfcf3e42a3d..b57d878f2fcd 100644
--- a/drivers/accel/ivpu/ivpu_drv.h
+++ b/drivers/accel/ivpu/ivpu_drv.h
@@ -137,12 +137,15 @@ struct ivpu_device {
struct mutex context_list_lock; /* Protects user context addition/removal */
struct xarray context_xa;
struct xa_limit context_xa_limit;
- struct work_struct context_abort_work;
struct xarray db_xa;
struct xa_limit db_limit;
u32 db_next;
+ struct work_struct irq_ipc_work;
+ struct work_struct irq_dct_work;
+ struct work_struct context_abort_work;
+
struct mutex bo_list_lock; /* Protects bo_list */
struct list_head bo_list;
diff --git a/drivers/accel/ivpu/ivpu_hw.c b/drivers/accel/ivpu/ivpu_hw.c
index 4e1054f3466e..1b691375ee4d 100644
--- a/drivers/accel/ivpu/ivpu_hw.c
+++ b/drivers/accel/ivpu/ivpu_hw.c
@@ -285,8 +285,6 @@ void ivpu_hw_profiling_freq_drive(struct ivpu_device *vdev, bool enable)
void ivpu_irq_handlers_init(struct ivpu_device *vdev)
{
- INIT_KFIFO(vdev->hw->irq.fifo);
-
if (ivpu_hw_ip_gen(vdev) == IVPU_HW_IP_37XX)
vdev->hw->irq.ip_irq_handler = ivpu_hw_ip_irq_handler_37xx;
else
@@ -300,7 +298,6 @@ void ivpu_irq_handlers_init(struct ivpu_device *vdev)
void ivpu_hw_irq_enable(struct ivpu_device *vdev)
{
- kfifo_reset(&vdev->hw->irq.fifo);
ivpu_hw_ip_irq_enable(vdev);
ivpu_hw_btrs_irq_enable(vdev);
}
@@ -327,8 +324,6 @@ irqreturn_t ivpu_hw_irq_handler(int irq, void *ptr)
/* Re-enable global interrupts to re-trigger MSI for pending interrupts */
ivpu_hw_btrs_global_int_enable(vdev);
- if (!kfifo_is_empty(&vdev->hw->irq.fifo))
- return IRQ_WAKE_THREAD;
if (ip_handled || btrs_handled)
return IRQ_HANDLED;
return IRQ_NONE;
diff --git a/drivers/accel/ivpu/ivpu_hw.h b/drivers/accel/ivpu/ivpu_hw.h
index fc4dbfc980c8..fbef9816b9d0 100644
--- a/drivers/accel/ivpu/ivpu_hw.h
+++ b/drivers/accel/ivpu/ivpu_hw.h
@@ -6,18 +6,10 @@
#ifndef __IVPU_HW_H__
#define __IVPU_HW_H__
-#include <linux/kfifo.h>
-
#include "ivpu_drv.h"
#include "ivpu_hw_btrs.h"
#include "ivpu_hw_ip.h"
-#define IVPU_HW_IRQ_FIFO_LENGTH 1024
-
-#define IVPU_HW_IRQ_SRC_IPC 1
-#define IVPU_HW_IRQ_SRC_MMU_EVTQ 2
-#define IVPU_HW_IRQ_SRC_DCT 3
-
struct ivpu_addr_range {
resource_size_t start;
resource_size_t end;
@@ -27,7 +19,6 @@ struct ivpu_hw_info {
struct {
bool (*btrs_irq_handler)(struct ivpu_device *vdev, int irq);
bool (*ip_irq_handler)(struct ivpu_device *vdev, int irq);
- DECLARE_KFIFO(fifo, u8, IVPU_HW_IRQ_FIFO_LENGTH);
} irq;
struct {
struct ivpu_addr_range global;
diff --git a/drivers/accel/ivpu/ivpu_hw_btrs.c b/drivers/accel/ivpu/ivpu_hw_btrs.c
index 3212c99f3682..3753b00ed2d6 100644
--- a/drivers/accel/ivpu/ivpu_hw_btrs.c
+++ b/drivers/accel/ivpu/ivpu_hw_btrs.c
@@ -630,8 +630,7 @@ bool ivpu_hw_btrs_irq_handler_lnl(struct ivpu_device *vdev, int irq)
if (REG_TEST_FLD(VPU_HW_BTRS_LNL_INTERRUPT_STAT, SURV_ERR, status)) {
ivpu_dbg(vdev, IRQ, "Survivability IRQ\n");
- if (!kfifo_put(&vdev->hw->irq.fifo, IVPU_HW_IRQ_SRC_DCT))
- ivpu_err_ratelimited(vdev, "IRQ FIFO full\n");
+ queue_work(system_wq, &vdev->irq_dct_work);
}
if (REG_TEST_FLD(VPU_HW_BTRS_LNL_INTERRUPT_STAT, FREQ_CHANGE, status))
diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c
index 01ebf88fe6ef..0e096fd9b95d 100644
--- a/drivers/accel/ivpu/ivpu_ipc.c
+++ b/drivers/accel/ivpu/ivpu_ipc.c
@@ -459,13 +459,12 @@ void ivpu_ipc_irq_handler(struct ivpu_device *vdev)
}
}
- if (!list_empty(&ipc->cb_msg_list))
- if (!kfifo_put(&vdev->hw->irq.fifo, IVPU_HW_IRQ_SRC_IPC))
- ivpu_err_ratelimited(vdev, "IRQ FIFO full\n");
+ queue_work(system_wq, &vdev->irq_ipc_work);
}
-void ivpu_ipc_irq_thread_handler(struct ivpu_device *vdev)
+void ivpu_ipc_irq_work_fn(struct work_struct *work)
{
+ struct ivpu_device *vdev = container_of(work, struct ivpu_device, irq_ipc_work);
struct ivpu_ipc_info *ipc = vdev->ipc;
struct ivpu_ipc_rx_msg *rx_msg, *r;
struct list_head cb_msg_list;
diff --git a/drivers/accel/ivpu/ivpu_ipc.h b/drivers/accel/ivpu/ivpu_ipc.h
index b4dfb504679b..b524a1985b9d 100644
--- a/drivers/accel/ivpu/ivpu_ipc.h
+++ b/drivers/accel/ivpu/ivpu_ipc.h
@@ -90,7 +90,7 @@ void ivpu_ipc_disable(struct ivpu_device *vdev);
void ivpu_ipc_reset(struct ivpu_device *vdev);
void ivpu_ipc_irq_handler(struct ivpu_device *vdev);
-void ivpu_ipc_irq_thread_handler(struct ivpu_device *vdev);
+void ivpu_ipc_irq_work_fn(struct work_struct *work);
void ivpu_ipc_consumer_add(struct ivpu_device *vdev, struct ivpu_ipc_consumer *cons,
u32 channel, ivpu_ipc_rx_callback_t callback);
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index 7fed3c8406ee..c678dcddb8d8 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -935,7 +935,7 @@ void ivpu_job_done_consumer_fini(struct ivpu_device *vdev)
ivpu_ipc_consumer_del(vdev, &vdev->job_done_consumer);
}
-void ivpu_context_abort_thread_handler(struct work_struct *work)
+void ivpu_context_abort_work_fn(struct work_struct *work)
{
struct ivpu_device *vdev = container_of(work, struct ivpu_device, context_abort_work);
struct ivpu_file_priv *file_priv;
diff --git a/drivers/accel/ivpu/ivpu_job.h b/drivers/accel/ivpu/ivpu_job.h
index fef8aed1fc88..ff77ee1fcee2 100644
--- a/drivers/accel/ivpu/ivpu_job.h
+++ b/drivers/accel/ivpu/ivpu_job.h
@@ -72,7 +72,7 @@ void ivpu_cmdq_abort_all_jobs(struct ivpu_device *vdev, u32 ctx_id, u32 cmdq_id)
void ivpu_job_done_consumer_init(struct ivpu_device *vdev);
void ivpu_job_done_consumer_fini(struct ivpu_device *vdev);
-void ivpu_context_abort_thread_handler(struct work_struct *work);
+void ivpu_context_abort_work_fn(struct work_struct *work);
void ivpu_jobs_abort_all(struct ivpu_device *vdev);
diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
index 6821051dfa3a..f41b3bfe40af 100644
--- a/drivers/accel/ivpu/ivpu_pm.c
+++ b/drivers/accel/ivpu/ivpu_pm.c
@@ -452,8 +452,9 @@ int ivpu_pm_dct_disable(struct ivpu_device *vdev)
return 0;
}
-void ivpu_pm_dct_irq_thread_handler(struct ivpu_device *vdev)
+void ivpu_pm_irq_dct_work_fn(struct work_struct *work)
{
+ struct ivpu_device *vdev = container_of(work, struct ivpu_device, irq_dct_work);
bool enable;
int ret;
diff --git a/drivers/accel/ivpu/ivpu_pm.h b/drivers/accel/ivpu/ivpu_pm.h
index b70efe6c36e4..89b264cc0e3e 100644
--- a/drivers/accel/ivpu/ivpu_pm.h
+++ b/drivers/accel/ivpu/ivpu_pm.h
@@ -45,6 +45,6 @@ void ivpu_stop_job_timeout_detection(struct ivpu_device *vdev);
int ivpu_pm_dct_init(struct ivpu_device *vdev);
int ivpu_pm_dct_enable(struct ivpu_device *vdev, u8 active_percent);
int ivpu_pm_dct_disable(struct ivpu_device *vdev);
-void ivpu_pm_dct_irq_thread_handler(struct ivpu_device *vdev);
+void ivpu_pm_irq_dct_work_fn(struct work_struct *work);
#endif /* __IVPU_PM_H__ */
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 05/14] accel/ivpu: Use workqueue for IRQ handling
2025-01-07 17:32 ` [PATCH 05/14] accel/ivpu: Use workqueue for IRQ handling Maciej Falkowski
@ 2025-01-09 8:26 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:26 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo
Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> Convert IRQ bottom half from the thread handler into workqueue.
> This increases a stability in rare scenarios where driver on
> debugging/hardening kernels processes IRQ too slow and misses
> some interrupts due to it.
> Workqueue handler also gives a very minor performance increase.
>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_drv.c | 39 ++++++++-----------------------
> drivers/accel/ivpu/ivpu_drv.h | 5 +++-
> drivers/accel/ivpu/ivpu_hw.c | 5 ----
> drivers/accel/ivpu/ivpu_hw.h | 9 -------
> drivers/accel/ivpu/ivpu_hw_btrs.c | 3 +--
> drivers/accel/ivpu/ivpu_ipc.c | 7 +++---
> drivers/accel/ivpu/ivpu_ipc.h | 2 +-
> drivers/accel/ivpu/ivpu_job.c | 2 +-
> drivers/accel/ivpu/ivpu_job.h | 2 +-
> drivers/accel/ivpu/ivpu_pm.c | 3 ++-
> drivers/accel/ivpu/ivpu_pm.h | 2 +-
> 11 files changed, 24 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
> index 300eea8c305f..9b0d99873fb3 100644
> --- a/drivers/accel/ivpu/ivpu_drv.c
> +++ b/drivers/accel/ivpu/ivpu_drv.c
> @@ -7,6 +7,7 @@
> #include <linux/module.h>
> #include <linux/pci.h>
> #include <linux/pm_runtime.h>
> +#include <linux/workqueue.h>
> #include <generated/utsrelease.h>
>
> #include <drm/drm_accel.h>
> @@ -421,6 +422,9 @@ void ivpu_prepare_for_reset(struct ivpu_device *vdev)
> {
> ivpu_hw_irq_disable(vdev);
> disable_irq(vdev->irq);
> + cancel_work_sync(&vdev->irq_ipc_work);
> + cancel_work_sync(&vdev->irq_dct_work);
> + cancel_work_sync(&vdev->context_abort_work);
> ivpu_ipc_disable(vdev);
> ivpu_mmu_disable(vdev);
> }
> @@ -465,31 +469,6 @@ static const struct drm_driver driver = {
> .major = 1,
> };
>
> -static irqreturn_t ivpu_irq_thread_handler(int irq, void *arg)
> -{
> - struct ivpu_device *vdev = arg;
> - u8 irq_src;
> -
> - if (kfifo_is_empty(&vdev->hw->irq.fifo))
> - return IRQ_NONE;
> -
> - while (kfifo_get(&vdev->hw->irq.fifo, &irq_src)) {
> - switch (irq_src) {
> - case IVPU_HW_IRQ_SRC_IPC:
> - ivpu_ipc_irq_thread_handler(vdev);
> - break;
> - case IVPU_HW_IRQ_SRC_DCT:
> - ivpu_pm_dct_irq_thread_handler(vdev);
> - break;
> - default:
> - ivpu_err_ratelimited(vdev, "Unknown IRQ source: %u\n", irq_src);
> - break;
> - }
> - }
> -
> - return IRQ_HANDLED;
> -}
> -
> static int ivpu_irq_init(struct ivpu_device *vdev)
> {
> struct pci_dev *pdev = to_pci_dev(vdev->drm.dev);
> @@ -501,12 +480,16 @@ static int ivpu_irq_init(struct ivpu_device *vdev)
> return ret;
> }
>
> + INIT_WORK(&vdev->irq_ipc_work, ivpu_ipc_irq_work_fn);
> + INIT_WORK(&vdev->irq_dct_work, ivpu_pm_irq_dct_work_fn);
> + INIT_WORK(&vdev->context_abort_work, ivpu_context_abort_work_fn);
> +
> ivpu_irq_handlers_init(vdev);
>
> vdev->irq = pci_irq_vector(pdev, 0);
>
> - ret = devm_request_threaded_irq(vdev->drm.dev, vdev->irq, ivpu_hw_irq_handler,
> - ivpu_irq_thread_handler, IRQF_NO_AUTOEN, DRIVER_NAME, vdev);
> + ret = devm_request_irq(vdev->drm.dev, vdev->irq, ivpu_hw_irq_handler,
> + IRQF_NO_AUTOEN, DRIVER_NAME, vdev);
> if (ret)
> ivpu_err(vdev, "Failed to request an IRQ %d\n", ret);
>
> @@ -599,8 +582,6 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
> vdev->db_limit.min = IVPU_MIN_DB;
> vdev->db_limit.max = IVPU_MAX_DB;
>
> - INIT_WORK(&vdev->context_abort_work, ivpu_context_abort_thread_handler);
> -
> ret = drmm_mutex_init(&vdev->drm, &vdev->context_list_lock);
> if (ret)
> goto err_xa_destroy;
> diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
> index ebfcf3e42a3d..b57d878f2fcd 100644
> --- a/drivers/accel/ivpu/ivpu_drv.h
> +++ b/drivers/accel/ivpu/ivpu_drv.h
> @@ -137,12 +137,15 @@ struct ivpu_device {
> struct mutex context_list_lock; /* Protects user context addition/removal */
> struct xarray context_xa;
> struct xa_limit context_xa_limit;
> - struct work_struct context_abort_work;
>
> struct xarray db_xa;
> struct xa_limit db_limit;
> u32 db_next;
>
> + struct work_struct irq_ipc_work;
> + struct work_struct irq_dct_work;
> + struct work_struct context_abort_work;
> +
> struct mutex bo_list_lock; /* Protects bo_list */
> struct list_head bo_list;
>
> diff --git a/drivers/accel/ivpu/ivpu_hw.c b/drivers/accel/ivpu/ivpu_hw.c
> index 4e1054f3466e..1b691375ee4d 100644
> --- a/drivers/accel/ivpu/ivpu_hw.c
> +++ b/drivers/accel/ivpu/ivpu_hw.c
> @@ -285,8 +285,6 @@ void ivpu_hw_profiling_freq_drive(struct ivpu_device *vdev, bool enable)
>
> void ivpu_irq_handlers_init(struct ivpu_device *vdev)
> {
> - INIT_KFIFO(vdev->hw->irq.fifo);
> -
> if (ivpu_hw_ip_gen(vdev) == IVPU_HW_IP_37XX)
> vdev->hw->irq.ip_irq_handler = ivpu_hw_ip_irq_handler_37xx;
> else
> @@ -300,7 +298,6 @@ void ivpu_irq_handlers_init(struct ivpu_device *vdev)
>
> void ivpu_hw_irq_enable(struct ivpu_device *vdev)
> {
> - kfifo_reset(&vdev->hw->irq.fifo);
> ivpu_hw_ip_irq_enable(vdev);
> ivpu_hw_btrs_irq_enable(vdev);
> }
> @@ -327,8 +324,6 @@ irqreturn_t ivpu_hw_irq_handler(int irq, void *ptr)
> /* Re-enable global interrupts to re-trigger MSI for pending interrupts */
> ivpu_hw_btrs_global_int_enable(vdev);
>
> - if (!kfifo_is_empty(&vdev->hw->irq.fifo))
> - return IRQ_WAKE_THREAD;
> if (ip_handled || btrs_handled)
> return IRQ_HANDLED;
> return IRQ_NONE;
> diff --git a/drivers/accel/ivpu/ivpu_hw.h b/drivers/accel/ivpu/ivpu_hw.h
> index fc4dbfc980c8..fbef9816b9d0 100644
> --- a/drivers/accel/ivpu/ivpu_hw.h
> +++ b/drivers/accel/ivpu/ivpu_hw.h
> @@ -6,18 +6,10 @@
> #ifndef __IVPU_HW_H__
> #define __IVPU_HW_H__
>
> -#include <linux/kfifo.h>
> -
> #include "ivpu_drv.h"
> #include "ivpu_hw_btrs.h"
> #include "ivpu_hw_ip.h"
>
> -#define IVPU_HW_IRQ_FIFO_LENGTH 1024
> -
> -#define IVPU_HW_IRQ_SRC_IPC 1
> -#define IVPU_HW_IRQ_SRC_MMU_EVTQ 2
> -#define IVPU_HW_IRQ_SRC_DCT 3
> -
> struct ivpu_addr_range {
> resource_size_t start;
> resource_size_t end;
> @@ -27,7 +19,6 @@ struct ivpu_hw_info {
> struct {
> bool (*btrs_irq_handler)(struct ivpu_device *vdev, int irq);
> bool (*ip_irq_handler)(struct ivpu_device *vdev, int irq);
> - DECLARE_KFIFO(fifo, u8, IVPU_HW_IRQ_FIFO_LENGTH);
> } irq;
> struct {
> struct ivpu_addr_range global;
> diff --git a/drivers/accel/ivpu/ivpu_hw_btrs.c b/drivers/accel/ivpu/ivpu_hw_btrs.c
> index 3212c99f3682..3753b00ed2d6 100644
> --- a/drivers/accel/ivpu/ivpu_hw_btrs.c
> +++ b/drivers/accel/ivpu/ivpu_hw_btrs.c
> @@ -630,8 +630,7 @@ bool ivpu_hw_btrs_irq_handler_lnl(struct ivpu_device *vdev, int irq)
>
> if (REG_TEST_FLD(VPU_HW_BTRS_LNL_INTERRUPT_STAT, SURV_ERR, status)) {
> ivpu_dbg(vdev, IRQ, "Survivability IRQ\n");
> - if (!kfifo_put(&vdev->hw->irq.fifo, IVPU_HW_IRQ_SRC_DCT))
> - ivpu_err_ratelimited(vdev, "IRQ FIFO full\n");
> + queue_work(system_wq, &vdev->irq_dct_work);
> }
>
> if (REG_TEST_FLD(VPU_HW_BTRS_LNL_INTERRUPT_STAT, FREQ_CHANGE, status))
> diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c
> index 01ebf88fe6ef..0e096fd9b95d 100644
> --- a/drivers/accel/ivpu/ivpu_ipc.c
> +++ b/drivers/accel/ivpu/ivpu_ipc.c
> @@ -459,13 +459,12 @@ void ivpu_ipc_irq_handler(struct ivpu_device *vdev)
> }
> }
>
> - if (!list_empty(&ipc->cb_msg_list))
> - if (!kfifo_put(&vdev->hw->irq.fifo, IVPU_HW_IRQ_SRC_IPC))
> - ivpu_err_ratelimited(vdev, "IRQ FIFO full\n");
> + queue_work(system_wq, &vdev->irq_ipc_work);
> }
>
> -void ivpu_ipc_irq_thread_handler(struct ivpu_device *vdev)
> +void ivpu_ipc_irq_work_fn(struct work_struct *work)
> {
> + struct ivpu_device *vdev = container_of(work, struct ivpu_device, irq_ipc_work);
> struct ivpu_ipc_info *ipc = vdev->ipc;
> struct ivpu_ipc_rx_msg *rx_msg, *r;
> struct list_head cb_msg_list;
> diff --git a/drivers/accel/ivpu/ivpu_ipc.h b/drivers/accel/ivpu/ivpu_ipc.h
> index b4dfb504679b..b524a1985b9d 100644
> --- a/drivers/accel/ivpu/ivpu_ipc.h
> +++ b/drivers/accel/ivpu/ivpu_ipc.h
> @@ -90,7 +90,7 @@ void ivpu_ipc_disable(struct ivpu_device *vdev);
> void ivpu_ipc_reset(struct ivpu_device *vdev);
>
> void ivpu_ipc_irq_handler(struct ivpu_device *vdev);
> -void ivpu_ipc_irq_thread_handler(struct ivpu_device *vdev);
> +void ivpu_ipc_irq_work_fn(struct work_struct *work);
>
> void ivpu_ipc_consumer_add(struct ivpu_device *vdev, struct ivpu_ipc_consumer *cons,
> u32 channel, ivpu_ipc_rx_callback_t callback);
> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
> index 7fed3c8406ee..c678dcddb8d8 100644
> --- a/drivers/accel/ivpu/ivpu_job.c
> +++ b/drivers/accel/ivpu/ivpu_job.c
> @@ -935,7 +935,7 @@ void ivpu_job_done_consumer_fini(struct ivpu_device *vdev)
> ivpu_ipc_consumer_del(vdev, &vdev->job_done_consumer);
> }
>
> -void ivpu_context_abort_thread_handler(struct work_struct *work)
> +void ivpu_context_abort_work_fn(struct work_struct *work)
> {
> struct ivpu_device *vdev = container_of(work, struct ivpu_device, context_abort_work);
> struct ivpu_file_priv *file_priv;
> diff --git a/drivers/accel/ivpu/ivpu_job.h b/drivers/accel/ivpu/ivpu_job.h
> index fef8aed1fc88..ff77ee1fcee2 100644
> --- a/drivers/accel/ivpu/ivpu_job.h
> +++ b/drivers/accel/ivpu/ivpu_job.h
> @@ -72,7 +72,7 @@ void ivpu_cmdq_abort_all_jobs(struct ivpu_device *vdev, u32 ctx_id, u32 cmdq_id)
>
> void ivpu_job_done_consumer_init(struct ivpu_device *vdev);
> void ivpu_job_done_consumer_fini(struct ivpu_device *vdev);
> -void ivpu_context_abort_thread_handler(struct work_struct *work);
> +void ivpu_context_abort_work_fn(struct work_struct *work);
>
> void ivpu_jobs_abort_all(struct ivpu_device *vdev);
>
> diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
> index 6821051dfa3a..f41b3bfe40af 100644
> --- a/drivers/accel/ivpu/ivpu_pm.c
> +++ b/drivers/accel/ivpu/ivpu_pm.c
> @@ -452,8 +452,9 @@ int ivpu_pm_dct_disable(struct ivpu_device *vdev)
> return 0;
> }
>
> -void ivpu_pm_dct_irq_thread_handler(struct ivpu_device *vdev)
> +void ivpu_pm_irq_dct_work_fn(struct work_struct *work)
> {
> + struct ivpu_device *vdev = container_of(work, struct ivpu_device, irq_dct_work);
> bool enable;
> int ret;
>
> diff --git a/drivers/accel/ivpu/ivpu_pm.h b/drivers/accel/ivpu/ivpu_pm.h
> index b70efe6c36e4..89b264cc0e3e 100644
> --- a/drivers/accel/ivpu/ivpu_pm.h
> +++ b/drivers/accel/ivpu/ivpu_pm.h
> @@ -45,6 +45,6 @@ void ivpu_stop_job_timeout_detection(struct ivpu_device *vdev);
> int ivpu_pm_dct_init(struct ivpu_device *vdev);
> int ivpu_pm_dct_enable(struct ivpu_device *vdev, u8 active_percent);
> int ivpu_pm_dct_disable(struct ivpu_device *vdev);
> -void ivpu_pm_dct_irq_thread_handler(struct ivpu_device *vdev);
> +void ivpu_pm_irq_dct_work_fn(struct work_struct *work);
>
> #endif /* __IVPU_PM_H__ */
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 06/14] accel/ivpu: Dump only first MMU fault from single context
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
` (4 preceding siblings ...)
2025-01-07 17:32 ` [PATCH 05/14] accel/ivpu: Use workqueue for IRQ handling Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-09 8:26 ` Jacek Lawrynowicz
2025-01-07 17:32 ` [PATCH 07/14] accel/ivpu: Move parts of MMU event IRQ handling to thread handler Maciej Falkowski
` (9 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel
Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Karol Wachowski,
Maciej Falkowski
From: Karol Wachowski <karol.wachowski@intel.com>
Stop dumping consecutive faults from an already faulty context immediately,
instead of waiting for the context abort thread handler (IRQ handler bottom
half) to abort currently executing jobs.
Remove 'R' (record events) bit from context descriptor of a faulty
context to prevent future faults generation.
This change speeds up the IRQ handler by eliminating the need to print the
fault content repeatedly. Additionally, it prevents flooding dmesg with
errors, which was occurring due to the delay in the bottom half of the
handler stopping fault-generating jobs.
Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_mmu.c | 51 ++++++++++++++++++++++++---
drivers/accel/ivpu/ivpu_mmu_context.c | 13 -------
drivers/accel/ivpu/ivpu_mmu_context.h | 2 --
3 files changed, 46 insertions(+), 20 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c
index 21f820dd0c65..5ee4df892b3e 100644
--- a/drivers/accel/ivpu/ivpu_mmu.c
+++ b/drivers/accel/ivpu/ivpu_mmu.c
@@ -870,23 +870,64 @@ static u32 *ivpu_mmu_get_event(struct ivpu_device *vdev)
return evt;
}
+static int ivpu_mmu_disable_events(struct ivpu_device *vdev, u32 ssid)
+{
+ struct ivpu_mmu_info *mmu = vdev->mmu;
+ struct ivpu_mmu_cdtab *cdtab = &mmu->cdtab;
+ u64 *entry;
+ u64 val;
+
+ if (ssid > IVPU_MMU_CDTAB_ENT_COUNT)
+ return -EINVAL;
+
+ entry = cdtab->base + (ssid * IVPU_MMU_CDTAB_ENT_SIZE);
+
+ val = READ_ONCE(entry[0]);
+ val &= ~IVPU_MMU_CD_0_R;
+ WRITE_ONCE(entry[0], val);
+
+ if (!ivpu_is_force_snoop_enabled(vdev))
+ clflush_cache_range(entry, IVPU_MMU_CDTAB_ENT_SIZE);
+
+ ivpu_mmu_cmdq_write_cfgi_all(vdev);
+
+ return 0;
+}
+
void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev)
{
+ struct ivpu_file_priv *file_priv;
+ u32 last_ssid = -1;
u32 *event;
u32 ssid;
ivpu_dbg(vdev, IRQ, "MMU event queue\n");
- while ((event = ivpu_mmu_get_event(vdev)) != NULL) {
- ivpu_mmu_dump_event(vdev, event);
-
+ while ((event = ivpu_mmu_get_event(vdev))) {
ssid = FIELD_GET(IVPU_MMU_EVT_SSID_MASK, event[0]);
+
+ if (ssid == last_ssid)
+ continue;
+
+ xa_lock(&vdev->context_xa);
+ file_priv = xa_load(&vdev->context_xa, ssid);
+ if (file_priv) {
+ if (file_priv->has_mmu_faults) {
+ event = NULL;
+ } else {
+ ivpu_mmu_disable_events(vdev, ssid);
+ file_priv->has_mmu_faults = true;
+ }
+ }
+ xa_unlock(&vdev->context_xa);
+
+ if (event)
+ ivpu_mmu_dump_event(vdev, event);
+
if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID) {
ivpu_pm_trigger_recovery(vdev, "MMU event");
return;
}
-
- ivpu_mmu_user_context_mark_invalid(vdev, ssid);
REGV_WR32(IVPU_MMU_REG_EVTQ_CONS_SEC, vdev->mmu->evtq.cons);
}
diff --git a/drivers/accel/ivpu/ivpu_mmu_context.c b/drivers/accel/ivpu/ivpu_mmu_context.c
index 891967a95bc3..d373443bbc83 100644
--- a/drivers/accel/ivpu/ivpu_mmu_context.c
+++ b/drivers/accel/ivpu/ivpu_mmu_context.c
@@ -631,16 +631,3 @@ void ivpu_mmu_reserved_context_fini(struct ivpu_device *vdev)
ivpu_mmu_cd_clear(vdev, vdev->rctx.id);
ivpu_mmu_context_fini(vdev, &vdev->rctx);
}
-
-void ivpu_mmu_user_context_mark_invalid(struct ivpu_device *vdev, u32 ssid)
-{
- struct ivpu_file_priv *file_priv;
-
- xa_lock(&vdev->context_xa);
-
- file_priv = xa_load(&vdev->context_xa, ssid);
- if (file_priv)
- file_priv->has_mmu_faults = true;
-
- xa_unlock(&vdev->context_xa);
-}
diff --git a/drivers/accel/ivpu/ivpu_mmu_context.h b/drivers/accel/ivpu/ivpu_mmu_context.h
index 8042fc067062..f255310968cf 100644
--- a/drivers/accel/ivpu/ivpu_mmu_context.h
+++ b/drivers/accel/ivpu/ivpu_mmu_context.h
@@ -37,8 +37,6 @@ void ivpu_mmu_global_context_fini(struct ivpu_device *vdev);
int ivpu_mmu_reserved_context_init(struct ivpu_device *vdev);
void ivpu_mmu_reserved_context_fini(struct ivpu_device *vdev);
-void ivpu_mmu_user_context_mark_invalid(struct ivpu_device *vdev, u32 ssid);
-
int ivpu_mmu_context_insert_node(struct ivpu_mmu_context *ctx, const struct ivpu_addr_range *range,
u64 size, struct drm_mm_node *node);
void ivpu_mmu_context_remove_node(struct ivpu_mmu_context *ctx, struct drm_mm_node *node);
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 06/14] accel/ivpu: Dump only first MMU fault from single context
2025-01-07 17:32 ` [PATCH 06/14] accel/ivpu: Dump only first MMU fault from single context Maciej Falkowski
@ 2025-01-09 8:26 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:26 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo, Karol Wachowski
Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> From: Karol Wachowski <karol.wachowski@intel.com>
>
> Stop dumping consecutive faults from an already faulty context immediately,
> instead of waiting for the context abort thread handler (IRQ handler bottom
> half) to abort currently executing jobs.
>
> Remove 'R' (record events) bit from context descriptor of a faulty
> context to prevent future faults generation.
>
> This change speeds up the IRQ handler by eliminating the need to print the
> fault content repeatedly. Additionally, it prevents flooding dmesg with
> errors, which was occurring due to the delay in the bottom half of the
> handler stopping fault-generating jobs.
>
> Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_mmu.c | 51 ++++++++++++++++++++++++---
> drivers/accel/ivpu/ivpu_mmu_context.c | 13 -------
> drivers/accel/ivpu/ivpu_mmu_context.h | 2 --
> 3 files changed, 46 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c
> index 21f820dd0c65..5ee4df892b3e 100644
> --- a/drivers/accel/ivpu/ivpu_mmu.c
> +++ b/drivers/accel/ivpu/ivpu_mmu.c
> @@ -870,23 +870,64 @@ static u32 *ivpu_mmu_get_event(struct ivpu_device *vdev)
> return evt;
> }
>
> +static int ivpu_mmu_disable_events(struct ivpu_device *vdev, u32 ssid)
> +{
> + struct ivpu_mmu_info *mmu = vdev->mmu;
> + struct ivpu_mmu_cdtab *cdtab = &mmu->cdtab;
> + u64 *entry;
> + u64 val;
> +
> + if (ssid > IVPU_MMU_CDTAB_ENT_COUNT)
> + return -EINVAL;
> +
> + entry = cdtab->base + (ssid * IVPU_MMU_CDTAB_ENT_SIZE);
> +
> + val = READ_ONCE(entry[0]);
> + val &= ~IVPU_MMU_CD_0_R;
> + WRITE_ONCE(entry[0], val);
> +
> + if (!ivpu_is_force_snoop_enabled(vdev))
> + clflush_cache_range(entry, IVPU_MMU_CDTAB_ENT_SIZE);
> +
> + ivpu_mmu_cmdq_write_cfgi_all(vdev);
> +
> + return 0;
> +}
> +
> void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev)
> {
> + struct ivpu_file_priv *file_priv;
> + u32 last_ssid = -1;
> u32 *event;
> u32 ssid;
>
> ivpu_dbg(vdev, IRQ, "MMU event queue\n");
>
> - while ((event = ivpu_mmu_get_event(vdev)) != NULL) {
> - ivpu_mmu_dump_event(vdev, event);
> -
> + while ((event = ivpu_mmu_get_event(vdev))) {
> ssid = FIELD_GET(IVPU_MMU_EVT_SSID_MASK, event[0]);
> +
> + if (ssid == last_ssid)
> + continue;
> +
> + xa_lock(&vdev->context_xa);
> + file_priv = xa_load(&vdev->context_xa, ssid);
> + if (file_priv) {
> + if (file_priv->has_mmu_faults) {
> + event = NULL;
> + } else {
> + ivpu_mmu_disable_events(vdev, ssid);
> + file_priv->has_mmu_faults = true;
> + }
> + }
> + xa_unlock(&vdev->context_xa);
> +
> + if (event)
> + ivpu_mmu_dump_event(vdev, event);
> +
> if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID) {
> ivpu_pm_trigger_recovery(vdev, "MMU event");
> return;
> }
> -
> - ivpu_mmu_user_context_mark_invalid(vdev, ssid);
> REGV_WR32(IVPU_MMU_REG_EVTQ_CONS_SEC, vdev->mmu->evtq.cons);
> }
>
> diff --git a/drivers/accel/ivpu/ivpu_mmu_context.c b/drivers/accel/ivpu/ivpu_mmu_context.c
> index 891967a95bc3..d373443bbc83 100644
> --- a/drivers/accel/ivpu/ivpu_mmu_context.c
> +++ b/drivers/accel/ivpu/ivpu_mmu_context.c
> @@ -631,16 +631,3 @@ void ivpu_mmu_reserved_context_fini(struct ivpu_device *vdev)
> ivpu_mmu_cd_clear(vdev, vdev->rctx.id);
> ivpu_mmu_context_fini(vdev, &vdev->rctx);
> }
> -
> -void ivpu_mmu_user_context_mark_invalid(struct ivpu_device *vdev, u32 ssid)
> -{
> - struct ivpu_file_priv *file_priv;
> -
> - xa_lock(&vdev->context_xa);
> -
> - file_priv = xa_load(&vdev->context_xa, ssid);
> - if (file_priv)
> - file_priv->has_mmu_faults = true;
> -
> - xa_unlock(&vdev->context_xa);
> -}
> diff --git a/drivers/accel/ivpu/ivpu_mmu_context.h b/drivers/accel/ivpu/ivpu_mmu_context.h
> index 8042fc067062..f255310968cf 100644
> --- a/drivers/accel/ivpu/ivpu_mmu_context.h
> +++ b/drivers/accel/ivpu/ivpu_mmu_context.h
> @@ -37,8 +37,6 @@ void ivpu_mmu_global_context_fini(struct ivpu_device *vdev);
> int ivpu_mmu_reserved_context_init(struct ivpu_device *vdev);
> void ivpu_mmu_reserved_context_fini(struct ivpu_device *vdev);
>
> -void ivpu_mmu_user_context_mark_invalid(struct ivpu_device *vdev, u32 ssid);
> -
> int ivpu_mmu_context_insert_node(struct ivpu_mmu_context *ctx, const struct ivpu_addr_range *range,
> u64 size, struct drm_mm_node *node);
> void ivpu_mmu_context_remove_node(struct ivpu_mmu_context *ctx, struct drm_mm_node *node);
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 07/14] accel/ivpu: Move parts of MMU event IRQ handling to thread handler
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
` (5 preceding siblings ...)
2025-01-07 17:32 ` [PATCH 06/14] accel/ivpu: Dump only first MMU fault from single context Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-09 8:27 ` Jacek Lawrynowicz
2025-01-07 17:32 ` [PATCH 08/14] accel/ivpu: Fix missing MMU events from reserved SSID Maciej Falkowski
` (8 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel
Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Karol Wachowski,
Maciej Falkowski
From: Karol Wachowski <karol.wachowski@intel.com>
To prevent looping infinitely in MMU event handler we stop
generating new events by removing 'R' (record) bit from context
descriptor, but to ensure this change has effect KMD has to perform
configuration invalidation followed by sync command.
Because of that move parts of the interrupt handler that can take longer
to a thread not to block in interrupt handler for too long.
This includes:
* disabling event queue for the time KMD updates MMU event queue consumer
to ensure proper synchronization between MMU and KMD
* removal of 'R' (record) bit from context descriptor to ensure no more
faults are recorded until that context is destroyed
Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_job.c | 7 ++-
drivers/accel/ivpu/ivpu_mmu.c | 93 +++++++++++++++++++++++------------
drivers/accel/ivpu/ivpu_mmu.h | 2 +
3 files changed, 69 insertions(+), 33 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index c678dcddb8d8..c55de9736d84 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -17,6 +17,7 @@
#include "ivpu_ipc.h"
#include "ivpu_job.h"
#include "ivpu_jsm_msg.h"
+#include "ivpu_mmu.h"
#include "ivpu_pm.h"
#include "ivpu_trace.h"
#include "vpu_boot_api.h"
@@ -366,6 +367,7 @@ void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv)
unsigned long cmdq_id;
lockdep_assert_held(&file_priv->lock);
+ ivpu_dbg(vdev, JOB, "Context ID: %u abort\n", file_priv->ctx.id);
xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
ivpu_cmdq_unregister(file_priv, cmdq);
@@ -373,6 +375,9 @@ void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv)
if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_OS)
ivpu_jsm_context_release(vdev, file_priv->ctx.id);
+ ivpu_mmu_disable_ssid_events(vdev, file_priv->ctx.id);
+ ivpu_mmu_discard_events(vdev);
+
file_priv->aborted = true;
}
@@ -939,8 +944,8 @@ void ivpu_context_abort_work_fn(struct work_struct *work)
{
struct ivpu_device *vdev = container_of(work, struct ivpu_device, context_abort_work);
struct ivpu_file_priv *file_priv;
- unsigned long ctx_id;
struct ivpu_job *job;
+ unsigned long ctx_id;
unsigned long id;
mutex_lock(&vdev->context_list_lock);
diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c
index 5ee4df892b3e..ae1dcd04051c 100644
--- a/drivers/accel/ivpu/ivpu_mmu.c
+++ b/drivers/accel/ivpu/ivpu_mmu.c
@@ -20,6 +20,12 @@
#define IVPU_MMU_REG_CR0 0x00200020u
#define IVPU_MMU_REG_CR0ACK 0x00200024u
#define IVPU_MMU_REG_CR0ACK_VAL_MASK GENMASK(31, 0)
+#define IVPU_MMU_REG_CR0_ATSCHK_MASK BIT(4)
+#define IVPU_MMU_REG_CR0_CMDQEN_MASK BIT(3)
+#define IVPU_MMU_REG_CR0_EVTQEN_MASK BIT(2)
+#define IVPU_MMU_REG_CR0_PRIQEN_MASK BIT(1)
+#define IVPU_MMU_REG_CR0_SMMUEN_MASK BIT(0)
+
#define IVPU_MMU_REG_CR1 0x00200028u
#define IVPU_MMU_REG_CR2 0x0020002cu
#define IVPU_MMU_REG_IRQ_CTRL 0x00200050u
@@ -141,12 +147,6 @@
#define IVPU_MMU_IRQ_EVTQ_EN BIT(2)
#define IVPU_MMU_IRQ_GERROR_EN BIT(0)
-#define IVPU_MMU_CR0_ATSCHK BIT(4)
-#define IVPU_MMU_CR0_CMDQEN BIT(3)
-#define IVPU_MMU_CR0_EVTQEN BIT(2)
-#define IVPU_MMU_CR0_PRIQEN BIT(1)
-#define IVPU_MMU_CR0_SMMUEN BIT(0)
-
#define IVPU_MMU_CR1_TABLE_SH GENMASK(11, 10)
#define IVPU_MMU_CR1_TABLE_OC GENMASK(9, 8)
#define IVPU_MMU_CR1_TABLE_IC GENMASK(7, 6)
@@ -596,7 +596,7 @@ static int ivpu_mmu_reset(struct ivpu_device *vdev)
REGV_WR32(IVPU_MMU_REG_CMDQ_PROD, 0);
REGV_WR32(IVPU_MMU_REG_CMDQ_CONS, 0);
- val = IVPU_MMU_CR0_CMDQEN;
+ val = REG_SET_FLD(IVPU_MMU_REG_CR0, CMDQEN, 0);
ret = ivpu_mmu_reg_write_cr0(vdev, val);
if (ret)
return ret;
@@ -617,12 +617,12 @@ static int ivpu_mmu_reset(struct ivpu_device *vdev)
REGV_WR32(IVPU_MMU_REG_EVTQ_PROD_SEC, 0);
REGV_WR32(IVPU_MMU_REG_EVTQ_CONS_SEC, 0);
- val |= IVPU_MMU_CR0_EVTQEN;
+ val = REG_SET_FLD(IVPU_MMU_REG_CR0, EVTQEN, val);
ret = ivpu_mmu_reg_write_cr0(vdev, val);
if (ret)
return ret;
- val |= IVPU_MMU_CR0_ATSCHK;
+ val = REG_SET_FLD(IVPU_MMU_REG_CR0, ATSCHK, val);
ret = ivpu_mmu_reg_write_cr0(vdev, val);
if (ret)
return ret;
@@ -631,7 +631,7 @@ static int ivpu_mmu_reset(struct ivpu_device *vdev)
if (ret)
return ret;
- val |= IVPU_MMU_CR0_SMMUEN;
+ val = REG_SET_FLD(IVPU_MMU_REG_CR0, SMMUEN, val);
return ivpu_mmu_reg_write_cr0(vdev, val);
}
@@ -870,7 +870,47 @@ static u32 *ivpu_mmu_get_event(struct ivpu_device *vdev)
return evt;
}
-static int ivpu_mmu_disable_events(struct ivpu_device *vdev, u32 ssid)
+static int ivpu_mmu_evtq_set(struct ivpu_device *vdev, bool enable)
+{
+ u32 val = REGV_RD32(IVPU_MMU_REG_CR0);
+
+ if (enable)
+ val = REG_SET_FLD(IVPU_MMU_REG_CR0, EVTQEN, val);
+ else
+ val = REG_CLR_FLD(IVPU_MMU_REG_CR0, EVTQEN, val);
+ REGV_WR32(IVPU_MMU_REG_CR0, val);
+
+ return REGV_POLL_FLD(IVPU_MMU_REG_CR0ACK, VAL, val, IVPU_MMU_REG_TIMEOUT_US);
+}
+
+static int ivpu_mmu_evtq_enable(struct ivpu_device *vdev)
+{
+ return ivpu_mmu_evtq_set(vdev, true);
+}
+
+static int ivpu_mmu_evtq_disable(struct ivpu_device *vdev)
+{
+ return ivpu_mmu_evtq_set(vdev, false);
+}
+
+void ivpu_mmu_discard_events(struct ivpu_device *vdev)
+{
+ /*
+ * Disable event queue (stop MMU from updating the producer)
+ * to allow synchronization of consumer and producer indexes
+ */
+ ivpu_mmu_evtq_disable(vdev);
+
+ vdev->mmu->evtq.cons = REGV_RD32(IVPU_MMU_REG_EVTQ_PROD_SEC);
+ REGV_WR32(IVPU_MMU_REG_EVTQ_CONS_SEC, vdev->mmu->evtq.cons);
+ vdev->mmu->evtq.prod = REGV_RD32(IVPU_MMU_REG_EVTQ_PROD_SEC);
+
+ ivpu_mmu_evtq_enable(vdev);
+
+ drm_WARN_ON_ONCE(&vdev->drm, vdev->mmu->evtq.cons != vdev->mmu->evtq.prod);
+}
+
+int ivpu_mmu_disable_ssid_events(struct ivpu_device *vdev, u32 ssid)
{
struct ivpu_mmu_info *mmu = vdev->mmu;
struct ivpu_mmu_cdtab *cdtab = &mmu->cdtab;
@@ -890,6 +930,7 @@ static int ivpu_mmu_disable_events(struct ivpu_device *vdev, u32 ssid)
clflush_cache_range(entry, IVPU_MMU_CDTAB_ENT_SIZE);
ivpu_mmu_cmdq_write_cfgi_all(vdev);
+ ivpu_mmu_cmdq_sync(vdev);
return 0;
}
@@ -897,38 +938,26 @@ static int ivpu_mmu_disable_events(struct ivpu_device *vdev, u32 ssid)
void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev)
{
struct ivpu_file_priv *file_priv;
- u32 last_ssid = -1;
u32 *event;
u32 ssid;
ivpu_dbg(vdev, IRQ, "MMU event queue\n");
while ((event = ivpu_mmu_get_event(vdev))) {
- ssid = FIELD_GET(IVPU_MMU_EVT_SSID_MASK, event[0]);
-
- if (ssid == last_ssid)
- continue;
+ ssid = FIELD_GET(IVPU_MMU_EVT_SSID_MASK, *event);
+ if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID) {
+ ivpu_mmu_dump_event(vdev, event);
+ ivpu_pm_trigger_recovery(vdev, "MMU event");
+ return;
+ }
- xa_lock(&vdev->context_xa);
file_priv = xa_load(&vdev->context_xa, ssid);
if (file_priv) {
- if (file_priv->has_mmu_faults) {
- event = NULL;
- } else {
- ivpu_mmu_disable_events(vdev, ssid);
- file_priv->has_mmu_faults = true;
+ if (!READ_ONCE(file_priv->has_mmu_faults)) {
+ ivpu_mmu_dump_event(vdev, event);
+ WRITE_ONCE(file_priv->has_mmu_faults, true);
}
}
- xa_unlock(&vdev->context_xa);
-
- if (event)
- ivpu_mmu_dump_event(vdev, event);
-
- if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID) {
- ivpu_pm_trigger_recovery(vdev, "MMU event");
- return;
- }
- REGV_WR32(IVPU_MMU_REG_EVTQ_CONS_SEC, vdev->mmu->evtq.cons);
}
queue_work(system_wq, &vdev->context_abort_work);
diff --git a/drivers/accel/ivpu/ivpu_mmu.h b/drivers/accel/ivpu/ivpu_mmu.h
index 7afea9cd8731..1ce7529746ad 100644
--- a/drivers/accel/ivpu/ivpu_mmu.h
+++ b/drivers/accel/ivpu/ivpu_mmu.h
@@ -47,5 +47,7 @@ int ivpu_mmu_invalidate_tlb(struct ivpu_device *vdev, u16 ssid);
void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev);
void ivpu_mmu_irq_gerr_handler(struct ivpu_device *vdev);
void ivpu_mmu_evtq_dump(struct ivpu_device *vdev);
+void ivpu_mmu_discard_events(struct ivpu_device *vdev);
+int ivpu_mmu_disable_ssid_events(struct ivpu_device *vdev, u32 ssid);
#endif /* __IVPU_MMU_H__ */
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 07/14] accel/ivpu: Move parts of MMU event IRQ handling to thread handler
2025-01-07 17:32 ` [PATCH 07/14] accel/ivpu: Move parts of MMU event IRQ handling to thread handler Maciej Falkowski
@ 2025-01-09 8:27 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:27 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo, Karol Wachowski
Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> From: Karol Wachowski <karol.wachowski@intel.com>
>
> To prevent looping infinitely in MMU event handler we stop
> generating new events by removing 'R' (record) bit from context
> descriptor, but to ensure this change has effect KMD has to perform
> configuration invalidation followed by sync command.
>
> Because of that move parts of the interrupt handler that can take longer
> to a thread not to block in interrupt handler for too long.
> This includes:
> * disabling event queue for the time KMD updates MMU event queue consumer
> to ensure proper synchronization between MMU and KMD
>
> * removal of 'R' (record) bit from context descriptor to ensure no more
> faults are recorded until that context is destroyed
>
> Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_job.c | 7 ++-
> drivers/accel/ivpu/ivpu_mmu.c | 93 +++++++++++++++++++++++------------
> drivers/accel/ivpu/ivpu_mmu.h | 2 +
> 3 files changed, 69 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
> index c678dcddb8d8..c55de9736d84 100644
> --- a/drivers/accel/ivpu/ivpu_job.c
> +++ b/drivers/accel/ivpu/ivpu_job.c
> @@ -17,6 +17,7 @@
> #include "ivpu_ipc.h"
> #include "ivpu_job.h"
> #include "ivpu_jsm_msg.h"
> +#include "ivpu_mmu.h"
> #include "ivpu_pm.h"
> #include "ivpu_trace.h"
> #include "vpu_boot_api.h"
> @@ -366,6 +367,7 @@ void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv)
> unsigned long cmdq_id;
>
> lockdep_assert_held(&file_priv->lock);
> + ivpu_dbg(vdev, JOB, "Context ID: %u abort\n", file_priv->ctx.id);
>
> xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq)
> ivpu_cmdq_unregister(file_priv, cmdq);
> @@ -373,6 +375,9 @@ void ivpu_context_abort_locked(struct ivpu_file_priv *file_priv)
> if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_OS)
> ivpu_jsm_context_release(vdev, file_priv->ctx.id);
>
> + ivpu_mmu_disable_ssid_events(vdev, file_priv->ctx.id);
> + ivpu_mmu_discard_events(vdev);
> +
> file_priv->aborted = true;
> }
>
> @@ -939,8 +944,8 @@ void ivpu_context_abort_work_fn(struct work_struct *work)
> {
> struct ivpu_device *vdev = container_of(work, struct ivpu_device, context_abort_work);
> struct ivpu_file_priv *file_priv;
> - unsigned long ctx_id;
> struct ivpu_job *job;
> + unsigned long ctx_id;
> unsigned long id;
>
> mutex_lock(&vdev->context_list_lock);
> diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c
> index 5ee4df892b3e..ae1dcd04051c 100644
> --- a/drivers/accel/ivpu/ivpu_mmu.c
> +++ b/drivers/accel/ivpu/ivpu_mmu.c
> @@ -20,6 +20,12 @@
> #define IVPU_MMU_REG_CR0 0x00200020u
> #define IVPU_MMU_REG_CR0ACK 0x00200024u
> #define IVPU_MMU_REG_CR0ACK_VAL_MASK GENMASK(31, 0)
> +#define IVPU_MMU_REG_CR0_ATSCHK_MASK BIT(4)
> +#define IVPU_MMU_REG_CR0_CMDQEN_MASK BIT(3)
> +#define IVPU_MMU_REG_CR0_EVTQEN_MASK BIT(2)
> +#define IVPU_MMU_REG_CR0_PRIQEN_MASK BIT(1)
> +#define IVPU_MMU_REG_CR0_SMMUEN_MASK BIT(0)
> +
> #define IVPU_MMU_REG_CR1 0x00200028u
> #define IVPU_MMU_REG_CR2 0x0020002cu
> #define IVPU_MMU_REG_IRQ_CTRL 0x00200050u
> @@ -141,12 +147,6 @@
> #define IVPU_MMU_IRQ_EVTQ_EN BIT(2)
> #define IVPU_MMU_IRQ_GERROR_EN BIT(0)
>
> -#define IVPU_MMU_CR0_ATSCHK BIT(4)
> -#define IVPU_MMU_CR0_CMDQEN BIT(3)
> -#define IVPU_MMU_CR0_EVTQEN BIT(2)
> -#define IVPU_MMU_CR0_PRIQEN BIT(1)
> -#define IVPU_MMU_CR0_SMMUEN BIT(0)
> -
> #define IVPU_MMU_CR1_TABLE_SH GENMASK(11, 10)
> #define IVPU_MMU_CR1_TABLE_OC GENMASK(9, 8)
> #define IVPU_MMU_CR1_TABLE_IC GENMASK(7, 6)
> @@ -596,7 +596,7 @@ static int ivpu_mmu_reset(struct ivpu_device *vdev)
> REGV_WR32(IVPU_MMU_REG_CMDQ_PROD, 0);
> REGV_WR32(IVPU_MMU_REG_CMDQ_CONS, 0);
>
> - val = IVPU_MMU_CR0_CMDQEN;
> + val = REG_SET_FLD(IVPU_MMU_REG_CR0, CMDQEN, 0);
> ret = ivpu_mmu_reg_write_cr0(vdev, val);
> if (ret)
> return ret;
> @@ -617,12 +617,12 @@ static int ivpu_mmu_reset(struct ivpu_device *vdev)
> REGV_WR32(IVPU_MMU_REG_EVTQ_PROD_SEC, 0);
> REGV_WR32(IVPU_MMU_REG_EVTQ_CONS_SEC, 0);
>
> - val |= IVPU_MMU_CR0_EVTQEN;
> + val = REG_SET_FLD(IVPU_MMU_REG_CR0, EVTQEN, val);
> ret = ivpu_mmu_reg_write_cr0(vdev, val);
> if (ret)
> return ret;
>
> - val |= IVPU_MMU_CR0_ATSCHK;
> + val = REG_SET_FLD(IVPU_MMU_REG_CR0, ATSCHK, val);
> ret = ivpu_mmu_reg_write_cr0(vdev, val);
> if (ret)
> return ret;
> @@ -631,7 +631,7 @@ static int ivpu_mmu_reset(struct ivpu_device *vdev)
> if (ret)
> return ret;
>
> - val |= IVPU_MMU_CR0_SMMUEN;
> + val = REG_SET_FLD(IVPU_MMU_REG_CR0, SMMUEN, val);
> return ivpu_mmu_reg_write_cr0(vdev, val);
> }
>
> @@ -870,7 +870,47 @@ static u32 *ivpu_mmu_get_event(struct ivpu_device *vdev)
> return evt;
> }
>
> -static int ivpu_mmu_disable_events(struct ivpu_device *vdev, u32 ssid)
> +static int ivpu_mmu_evtq_set(struct ivpu_device *vdev, bool enable)
> +{
> + u32 val = REGV_RD32(IVPU_MMU_REG_CR0);
> +
> + if (enable)
> + val = REG_SET_FLD(IVPU_MMU_REG_CR0, EVTQEN, val);
> + else
> + val = REG_CLR_FLD(IVPU_MMU_REG_CR0, EVTQEN, val);
> + REGV_WR32(IVPU_MMU_REG_CR0, val);
> +
> + return REGV_POLL_FLD(IVPU_MMU_REG_CR0ACK, VAL, val, IVPU_MMU_REG_TIMEOUT_US);
> +}
> +
> +static int ivpu_mmu_evtq_enable(struct ivpu_device *vdev)
> +{
> + return ivpu_mmu_evtq_set(vdev, true);
> +}
> +
> +static int ivpu_mmu_evtq_disable(struct ivpu_device *vdev)
> +{
> + return ivpu_mmu_evtq_set(vdev, false);
> +}
> +
> +void ivpu_mmu_discard_events(struct ivpu_device *vdev)
> +{
> + /*
> + * Disable event queue (stop MMU from updating the producer)
> + * to allow synchronization of consumer and producer indexes
> + */
> + ivpu_mmu_evtq_disable(vdev);
> +
> + vdev->mmu->evtq.cons = REGV_RD32(IVPU_MMU_REG_EVTQ_PROD_SEC);
> + REGV_WR32(IVPU_MMU_REG_EVTQ_CONS_SEC, vdev->mmu->evtq.cons);
> + vdev->mmu->evtq.prod = REGV_RD32(IVPU_MMU_REG_EVTQ_PROD_SEC);
> +
> + ivpu_mmu_evtq_enable(vdev);
> +
> + drm_WARN_ON_ONCE(&vdev->drm, vdev->mmu->evtq.cons != vdev->mmu->evtq.prod);
> +}
> +
> +int ivpu_mmu_disable_ssid_events(struct ivpu_device *vdev, u32 ssid)
> {
> struct ivpu_mmu_info *mmu = vdev->mmu;
> struct ivpu_mmu_cdtab *cdtab = &mmu->cdtab;
> @@ -890,6 +930,7 @@ static int ivpu_mmu_disable_events(struct ivpu_device *vdev, u32 ssid)
> clflush_cache_range(entry, IVPU_MMU_CDTAB_ENT_SIZE);
>
> ivpu_mmu_cmdq_write_cfgi_all(vdev);
> + ivpu_mmu_cmdq_sync(vdev);
>
> return 0;
> }
> @@ -897,38 +938,26 @@ static int ivpu_mmu_disable_events(struct ivpu_device *vdev, u32 ssid)
> void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev)
> {
> struct ivpu_file_priv *file_priv;
> - u32 last_ssid = -1;
> u32 *event;
> u32 ssid;
>
> ivpu_dbg(vdev, IRQ, "MMU event queue\n");
>
> while ((event = ivpu_mmu_get_event(vdev))) {
> - ssid = FIELD_GET(IVPU_MMU_EVT_SSID_MASK, event[0]);
> -
> - if (ssid == last_ssid)
> - continue;
> + ssid = FIELD_GET(IVPU_MMU_EVT_SSID_MASK, *event);
> + if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID) {
> + ivpu_mmu_dump_event(vdev, event);
> + ivpu_pm_trigger_recovery(vdev, "MMU event");
> + return;
> + }
>
> - xa_lock(&vdev->context_xa);
> file_priv = xa_load(&vdev->context_xa, ssid);
> if (file_priv) {
> - if (file_priv->has_mmu_faults) {
> - event = NULL;
> - } else {
> - ivpu_mmu_disable_events(vdev, ssid);
> - file_priv->has_mmu_faults = true;
> + if (!READ_ONCE(file_priv->has_mmu_faults)) {
> + ivpu_mmu_dump_event(vdev, event);
> + WRITE_ONCE(file_priv->has_mmu_faults, true);
> }
> }
> - xa_unlock(&vdev->context_xa);
> -
> - if (event)
> - ivpu_mmu_dump_event(vdev, event);
> -
> - if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID) {
> - ivpu_pm_trigger_recovery(vdev, "MMU event");
> - return;
> - }
> - REGV_WR32(IVPU_MMU_REG_EVTQ_CONS_SEC, vdev->mmu->evtq.cons);
> }
>
> queue_work(system_wq, &vdev->context_abort_work);
> diff --git a/drivers/accel/ivpu/ivpu_mmu.h b/drivers/accel/ivpu/ivpu_mmu.h
> index 7afea9cd8731..1ce7529746ad 100644
> --- a/drivers/accel/ivpu/ivpu_mmu.h
> +++ b/drivers/accel/ivpu/ivpu_mmu.h
> @@ -47,5 +47,7 @@ int ivpu_mmu_invalidate_tlb(struct ivpu_device *vdev, u16 ssid);
> void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev);
> void ivpu_mmu_irq_gerr_handler(struct ivpu_device *vdev);
> void ivpu_mmu_evtq_dump(struct ivpu_device *vdev);
> +void ivpu_mmu_discard_events(struct ivpu_device *vdev);
> +int ivpu_mmu_disable_ssid_events(struct ivpu_device *vdev, u32 ssid);
>
> #endif /* __IVPU_MMU_H__ */
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 08/14] accel/ivpu: Fix missing MMU events from reserved SSID
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
` (6 preceding siblings ...)
2025-01-07 17:32 ` [PATCH 07/14] accel/ivpu: Move parts of MMU event IRQ handling to thread handler Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-09 8:27 ` Jacek Lawrynowicz
2025-01-07 17:32 ` [PATCH 09/14] accel/ivpu: Set command queue management capability based on HWS Maciej Falkowski
` (7 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel
Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Karol Wachowski,
Maciej Falkowski
From: Karol Wachowski <karol.wachowski@intel.com>
Generate recovery when fault from reserved context is detected.
Add Abort (A) bit to reserved (1) SSID to ensure NPU also receives a fault.
There is no way to create a file_priv with reserved SSID
but it is still possible to receive MMU faults from that SSID
as it is a default NPU HW setting. Such situation will occur if
FW freed context related resources but still performed access to DRAM.
Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_mmu.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c
index ae1dcd04051c..b80bdded9fd7 100644
--- a/drivers/accel/ivpu/ivpu_mmu.c
+++ b/drivers/accel/ivpu/ivpu_mmu.c
@@ -725,8 +725,8 @@ static int ivpu_mmu_cdtab_entry_set(struct ivpu_device *vdev, u32 ssid, u64 cd_d
cd[2] = 0;
cd[3] = 0x0000000000007444;
- /* For global context generate memory fault on VPU */
- if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID)
+ /* For global and reserved contexts generate memory fault on VPU */
+ if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID || ssid == IVPU_RESERVED_CONTEXT_MMU_SSID)
cd[0] |= IVPU_MMU_CD_0_A;
if (valid)
@@ -945,7 +945,8 @@ void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev)
while ((event = ivpu_mmu_get_event(vdev))) {
ssid = FIELD_GET(IVPU_MMU_EVT_SSID_MASK, *event);
- if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID) {
+ if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID ||
+ ssid == IVPU_RESERVED_CONTEXT_MMU_SSID) {
ivpu_mmu_dump_event(vdev, event);
ivpu_pm_trigger_recovery(vdev, "MMU event");
return;
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 08/14] accel/ivpu: Fix missing MMU events from reserved SSID
2025-01-07 17:32 ` [PATCH 08/14] accel/ivpu: Fix missing MMU events from reserved SSID Maciej Falkowski
@ 2025-01-09 8:27 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:27 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo, Karol Wachowski
Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> From: Karol Wachowski <karol.wachowski@intel.com>
>
> Generate recovery when fault from reserved context is detected.
> Add Abort (A) bit to reserved (1) SSID to ensure NPU also receives a fault.
>
> There is no way to create a file_priv with reserved SSID
> but it is still possible to receive MMU faults from that SSID
> as it is a default NPU HW setting. Such situation will occur if
> FW freed context related resources but still performed access to DRAM.
>
> Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_mmu.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c
> index ae1dcd04051c..b80bdded9fd7 100644
> --- a/drivers/accel/ivpu/ivpu_mmu.c
> +++ b/drivers/accel/ivpu/ivpu_mmu.c
> @@ -725,8 +725,8 @@ static int ivpu_mmu_cdtab_entry_set(struct ivpu_device *vdev, u32 ssid, u64 cd_d
> cd[2] = 0;
> cd[3] = 0x0000000000007444;
>
> - /* For global context generate memory fault on VPU */
> - if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID)
> + /* For global and reserved contexts generate memory fault on VPU */
> + if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID || ssid == IVPU_RESERVED_CONTEXT_MMU_SSID)
> cd[0] |= IVPU_MMU_CD_0_A;
>
> if (valid)
> @@ -945,7 +945,8 @@ void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev)
>
> while ((event = ivpu_mmu_get_event(vdev))) {
> ssid = FIELD_GET(IVPU_MMU_EVT_SSID_MASK, *event);
> - if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID) {
> + if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID ||
> + ssid == IVPU_RESERVED_CONTEXT_MMU_SSID) {
> ivpu_mmu_dump_event(vdev, event);
> ivpu_pm_trigger_recovery(vdev, "MMU event");
> return;
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 09/14] accel/ivpu: Set command queue management capability based on HWS
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
` (7 preceding siblings ...)
2025-01-07 17:32 ` [PATCH 08/14] accel/ivpu: Fix missing MMU events from reserved SSID Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-09 8:28 ` Jacek Lawrynowicz
2025-01-07 17:32 ` [PATCH 10/14] accel/ivpu: Fix locking order in ivpu_cmdq_destroy_ioctl Maciej Falkowski
` (6 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel
Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Karol Wachowski,
Maciej Falkowski
From: Karol Wachowski <karol.wachowski@intel.com>
Control explicit command queue management capability bit based on
scheduling mode. Capability will be available only when hardware
scheduling mode is set.
There is no point of allowing user space to create and destroy command
queues with OS schedling mode because FW does not support all required
functionalities for correct command queue management with OS scheduling.
Return -ENODEV from command queue create/destroy/submit IOCTLs.
Remove is_valid field from struct ivpu_job_cmdq
Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_drv.c | 19 +++++++------------
drivers/accel/ivpu/ivpu_drv.h | 1 +
drivers/accel/ivpu/ivpu_job.c | 24 +++++++++++-------------
drivers/accel/ivpu/ivpu_job.h | 1 -
4 files changed, 19 insertions(+), 26 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index 9b0d99873fb3..6a80d626d609 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -127,23 +127,18 @@ void ivpu_file_priv_put(struct ivpu_file_priv **link)
kref_put(&file_priv->ref, file_priv_release);
}
-static int ivpu_get_capabilities(struct ivpu_device *vdev, struct drm_ivpu_param *args)
+bool ivpu_is_capable(struct ivpu_device *vdev, u32 capability)
{
- switch (args->index) {
+ switch (capability) {
case DRM_IVPU_CAP_METRIC_STREAMER:
- args->value = 1;
- break;
+ return true;
case DRM_IVPU_CAP_DMA_MEMORY_RANGE:
- args->value = 1;
- break;
+ return true;
case DRM_IVPU_CAP_MANAGE_CMDQ:
- args->value = 1;
- break;
+ return vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW;
default:
- return -EINVAL;
+ return false;
}
-
- return 0;
}
static int ivpu_get_param_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
@@ -203,7 +198,7 @@ static int ivpu_get_param_ioctl(struct drm_device *dev, void *data, struct drm_f
args->value = vdev->hw->sku;
break;
case DRM_IVPU_PARAM_CAPABILITIES:
- ret = ivpu_get_capabilities(vdev, args);
+ args->value = ivpu_is_capable(vdev, args->index);
break;
default:
ret = -EINVAL;
diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
index b57d878f2fcd..d53902b34070 100644
--- a/drivers/accel/ivpu/ivpu_drv.h
+++ b/drivers/accel/ivpu/ivpu_drv.h
@@ -213,6 +213,7 @@ void ivpu_file_priv_put(struct ivpu_file_priv **link);
int ivpu_boot(struct ivpu_device *vdev);
int ivpu_shutdown(struct ivpu_device *vdev);
void ivpu_prepare_for_reset(struct ivpu_device *vdev);
+bool ivpu_is_capable(struct ivpu_device *vdev, u32 capability);
static inline u8 ivpu_revision(struct ivpu_device *vdev)
{
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index c55de9736d84..37ea92eb4b25 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -123,7 +123,6 @@ static struct ivpu_cmdq *ivpu_cmdq_create(struct ivpu_file_priv *file_priv, u8 p
cmdq->priority = priority;
cmdq->is_legacy = is_legacy;
- cmdq->is_valid = true;
ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &cmdq->id, cmdq, file_priv->cmdq_limit,
&file_priv->cmdq_id_next, GFP_KERNEL);
@@ -307,7 +306,7 @@ static struct ivpu_cmdq *ivpu_cmdq_acquire(struct ivpu_file_priv *file_priv, u32
lockdep_assert_held(&file_priv->lock);
cmdq = xa_load(&file_priv->cmdq_xa, cmdq_id);
- if (!cmdq || !cmdq->is_valid) {
+ if (!cmdq) {
ivpu_warn_ratelimited(vdev, "Failed to find command queue with ID: %u\n", cmdq_id);
return NULL;
}
@@ -832,6 +831,9 @@ int ivpu_cmdq_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *
struct ivpu_file_priv *file_priv = file->driver_priv;
struct drm_ivpu_cmdq_submit *args = data;
+ if (!ivpu_is_capable(file_priv->vdev, DRM_IVPU_CAP_MANAGE_CMDQ))
+ return -ENODEV;
+
if (args->cmdq_id < IVPU_CMDQ_MIN_ID || args->cmdq_id > IVPU_CMDQ_MAX_ID)
return -EINVAL;
@@ -857,6 +859,9 @@ int ivpu_cmdq_create_ioctl(struct drm_device *dev, void *data, struct drm_file *
struct drm_ivpu_cmdq_create *args = data;
struct ivpu_cmdq *cmdq;
+ if (!ivpu_is_capable(file_priv->vdev, DRM_IVPU_CAP_MANAGE_CMDQ))
+ return -ENODEV;
+
if (args->priority > DRM_IVPU_JOB_PRIORITY_REALTIME)
return -EINVAL;
@@ -880,24 +885,17 @@ int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file
u32 cmdq_id;
int ret = 0;
+ if (!ivpu_is_capable(vdev, DRM_IVPU_CAP_MANAGE_CMDQ))
+ return -ENODEV;
+
mutex_lock(&file_priv->lock);
cmdq = xa_load(&file_priv->cmdq_xa, args->cmdq_id);
- if (!cmdq || !cmdq->is_valid || cmdq->is_legacy) {
+ if (!cmdq || cmdq->is_legacy) {
ret = -ENOENT;
goto unlock;
}
- /*
- * There is no way to stop executing jobs per command queue
- * in OS scheduling mode, mark command queue as invalid instead
- * and it will be freed together with context release.
- */
- if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_OS) {
- cmdq->is_valid = false;
- goto unlock;
- }
-
cmdq_id = cmdq->id;
ivpu_cmdq_destroy(file_priv, cmdq);
ivpu_cmdq_abort_all_jobs(vdev, file_priv->ctx.id, cmdq_id);
diff --git a/drivers/accel/ivpu/ivpu_job.h b/drivers/accel/ivpu/ivpu_job.h
index ff77ee1fcee2..2e301c2eea7b 100644
--- a/drivers/accel/ivpu/ivpu_job.h
+++ b/drivers/accel/ivpu/ivpu_job.h
@@ -31,7 +31,6 @@ struct ivpu_cmdq {
u32 id;
u32 db_id;
u8 priority;
- bool is_valid;
bool is_legacy;
};
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 09/14] accel/ivpu: Set command queue management capability based on HWS
2025-01-07 17:32 ` [PATCH 09/14] accel/ivpu: Set command queue management capability based on HWS Maciej Falkowski
@ 2025-01-09 8:28 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:28 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo, Karol Wachowski
Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> From: Karol Wachowski <karol.wachowski@intel.com>
>
> Control explicit command queue management capability bit based on
> scheduling mode. Capability will be available only when hardware
> scheduling mode is set.
>
> There is no point of allowing user space to create and destroy command
> queues with OS schedling mode because FW does not support all required
> functionalities for correct command queue management with OS scheduling.
>
> Return -ENODEV from command queue create/destroy/submit IOCTLs.
>
> Remove is_valid field from struct ivpu_job_cmdq
>
> Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_drv.c | 19 +++++++------------
> drivers/accel/ivpu/ivpu_drv.h | 1 +
> drivers/accel/ivpu/ivpu_job.c | 24 +++++++++++-------------
> drivers/accel/ivpu/ivpu_job.h | 1 -
> 4 files changed, 19 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
> index 9b0d99873fb3..6a80d626d609 100644
> --- a/drivers/accel/ivpu/ivpu_drv.c
> +++ b/drivers/accel/ivpu/ivpu_drv.c
> @@ -127,23 +127,18 @@ void ivpu_file_priv_put(struct ivpu_file_priv **link)
> kref_put(&file_priv->ref, file_priv_release);
> }
>
> -static int ivpu_get_capabilities(struct ivpu_device *vdev, struct drm_ivpu_param *args)
> +bool ivpu_is_capable(struct ivpu_device *vdev, u32 capability)
> {
> - switch (args->index) {
> + switch (capability) {
> case DRM_IVPU_CAP_METRIC_STREAMER:
> - args->value = 1;
> - break;
> + return true;
> case DRM_IVPU_CAP_DMA_MEMORY_RANGE:
> - args->value = 1;
> - break;
> + return true;
> case DRM_IVPU_CAP_MANAGE_CMDQ:
> - args->value = 1;
> - break;
> + return vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW;
> default:
> - return -EINVAL;
> + return false;
> }
> -
> - return 0;
> }
>
> static int ivpu_get_param_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> @@ -203,7 +198,7 @@ static int ivpu_get_param_ioctl(struct drm_device *dev, void *data, struct drm_f
> args->value = vdev->hw->sku;
> break;
> case DRM_IVPU_PARAM_CAPABILITIES:
> - ret = ivpu_get_capabilities(vdev, args);
> + args->value = ivpu_is_capable(vdev, args->index);
> break;
> default:
> ret = -EINVAL;
> diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
> index b57d878f2fcd..d53902b34070 100644
> --- a/drivers/accel/ivpu/ivpu_drv.h
> +++ b/drivers/accel/ivpu/ivpu_drv.h
> @@ -213,6 +213,7 @@ void ivpu_file_priv_put(struct ivpu_file_priv **link);
> int ivpu_boot(struct ivpu_device *vdev);
> int ivpu_shutdown(struct ivpu_device *vdev);
> void ivpu_prepare_for_reset(struct ivpu_device *vdev);
> +bool ivpu_is_capable(struct ivpu_device *vdev, u32 capability);
>
> static inline u8 ivpu_revision(struct ivpu_device *vdev)
> {
> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
> index c55de9736d84..37ea92eb4b25 100644
> --- a/drivers/accel/ivpu/ivpu_job.c
> +++ b/drivers/accel/ivpu/ivpu_job.c
> @@ -123,7 +123,6 @@ static struct ivpu_cmdq *ivpu_cmdq_create(struct ivpu_file_priv *file_priv, u8 p
>
> cmdq->priority = priority;
> cmdq->is_legacy = is_legacy;
> - cmdq->is_valid = true;
>
> ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &cmdq->id, cmdq, file_priv->cmdq_limit,
> &file_priv->cmdq_id_next, GFP_KERNEL);
> @@ -307,7 +306,7 @@ static struct ivpu_cmdq *ivpu_cmdq_acquire(struct ivpu_file_priv *file_priv, u32
> lockdep_assert_held(&file_priv->lock);
>
> cmdq = xa_load(&file_priv->cmdq_xa, cmdq_id);
> - if (!cmdq || !cmdq->is_valid) {
> + if (!cmdq) {
> ivpu_warn_ratelimited(vdev, "Failed to find command queue with ID: %u\n", cmdq_id);
> return NULL;
> }
> @@ -832,6 +831,9 @@ int ivpu_cmdq_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *
> struct ivpu_file_priv *file_priv = file->driver_priv;
> struct drm_ivpu_cmdq_submit *args = data;
>
> + if (!ivpu_is_capable(file_priv->vdev, DRM_IVPU_CAP_MANAGE_CMDQ))
> + return -ENODEV;
> +
> if (args->cmdq_id < IVPU_CMDQ_MIN_ID || args->cmdq_id > IVPU_CMDQ_MAX_ID)
> return -EINVAL;
>
> @@ -857,6 +859,9 @@ int ivpu_cmdq_create_ioctl(struct drm_device *dev, void *data, struct drm_file *
> struct drm_ivpu_cmdq_create *args = data;
> struct ivpu_cmdq *cmdq;
>
> + if (!ivpu_is_capable(file_priv->vdev, DRM_IVPU_CAP_MANAGE_CMDQ))
> + return -ENODEV;
> +
> if (args->priority > DRM_IVPU_JOB_PRIORITY_REALTIME)
> return -EINVAL;
>
> @@ -880,24 +885,17 @@ int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file
> u32 cmdq_id;
> int ret = 0;
>
> + if (!ivpu_is_capable(vdev, DRM_IVPU_CAP_MANAGE_CMDQ))
> + return -ENODEV;
> +
> mutex_lock(&file_priv->lock);
>
> cmdq = xa_load(&file_priv->cmdq_xa, args->cmdq_id);
> - if (!cmdq || !cmdq->is_valid || cmdq->is_legacy) {
> + if (!cmdq || cmdq->is_legacy) {
> ret = -ENOENT;
> goto unlock;
> }
>
> - /*
> - * There is no way to stop executing jobs per command queue
> - * in OS scheduling mode, mark command queue as invalid instead
> - * and it will be freed together with context release.
> - */
> - if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_OS) {
> - cmdq->is_valid = false;
> - goto unlock;
> - }
> -
> cmdq_id = cmdq->id;
> ivpu_cmdq_destroy(file_priv, cmdq);
> ivpu_cmdq_abort_all_jobs(vdev, file_priv->ctx.id, cmdq_id);
> diff --git a/drivers/accel/ivpu/ivpu_job.h b/drivers/accel/ivpu/ivpu_job.h
> index ff77ee1fcee2..2e301c2eea7b 100644
> --- a/drivers/accel/ivpu/ivpu_job.h
> +++ b/drivers/accel/ivpu/ivpu_job.h
> @@ -31,7 +31,6 @@ struct ivpu_cmdq {
> u32 id;
> u32 db_id;
> u8 priority;
> - bool is_valid;
> bool is_legacy;
> };
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 10/14] accel/ivpu: Fix locking order in ivpu_cmdq_destroy_ioctl
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
` (8 preceding siblings ...)
2025-01-07 17:32 ` [PATCH 09/14] accel/ivpu: Set command queue management capability based on HWS Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-09 8:28 ` Jacek Lawrynowicz
2025-01-07 17:32 ` [PATCH 11/14] accel/ivpu: Fix locking order in ivpu_job_submit Maciej Falkowski
` (5 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel
Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Karol Wachowski,
Maciej Falkowski
From: Karol Wachowski <karol.wachowski@intel.com>
Fix deadlock caused by inversed locking order in ivpu_job_submit()
and ivpu_cmdq_destroy_ioctl(). Both functions operate locking
file_priv->lock and submitted_jobs_lock.
Unlock file_priv->lock in ivpu_cmdq_destroy_ioctl() before calling
ivpu_cmdq_abort_all_jobs() function which locks submitted_jobs_lock.
That way locking order is maintained:
1) global submitted_jobs_lock first
2) per context file_priv->lock second
Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_job.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index 37ea92eb4b25..c694822a14bf 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -883,7 +883,7 @@ int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file
struct drm_ivpu_cmdq_destroy *args = data;
struct ivpu_cmdq *cmdq;
u32 cmdq_id;
- int ret = 0;
+ int ret;
if (!ivpu_is_capable(vdev, DRM_IVPU_CAP_MANAGE_CMDQ))
return -ENODEV;
@@ -893,13 +893,16 @@ int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file
cmdq = xa_load(&file_priv->cmdq_xa, args->cmdq_id);
if (!cmdq || cmdq->is_legacy) {
ret = -ENOENT;
- goto unlock;
+ goto err_unlock;
}
cmdq_id = cmdq->id;
ivpu_cmdq_destroy(file_priv, cmdq);
+ mutex_unlock(&file_priv->lock);
ivpu_cmdq_abort_all_jobs(vdev, file_priv->ctx.id, cmdq_id);
-unlock:
+ return 0;
+
+err_unlock:
mutex_unlock(&file_priv->lock);
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 10/14] accel/ivpu: Fix locking order in ivpu_cmdq_destroy_ioctl
2025-01-07 17:32 ` [PATCH 10/14] accel/ivpu: Fix locking order in ivpu_cmdq_destroy_ioctl Maciej Falkowski
@ 2025-01-09 8:28 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:28 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo, Karol Wachowski
Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> From: Karol Wachowski <karol.wachowski@intel.com>
>
> Fix deadlock caused by inversed locking order in ivpu_job_submit()
> and ivpu_cmdq_destroy_ioctl(). Both functions operate locking
> file_priv->lock and submitted_jobs_lock.
>
> Unlock file_priv->lock in ivpu_cmdq_destroy_ioctl() before calling
> ivpu_cmdq_abort_all_jobs() function which locks submitted_jobs_lock.
> That way locking order is maintained:
> 1) global submitted_jobs_lock first
> 2) per context file_priv->lock second
>
> Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_job.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
> index 37ea92eb4b25..c694822a14bf 100644
> --- a/drivers/accel/ivpu/ivpu_job.c
> +++ b/drivers/accel/ivpu/ivpu_job.c
> @@ -883,7 +883,7 @@ int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file
> struct drm_ivpu_cmdq_destroy *args = data;
> struct ivpu_cmdq *cmdq;
> u32 cmdq_id;
> - int ret = 0;
> + int ret;
>
> if (!ivpu_is_capable(vdev, DRM_IVPU_CAP_MANAGE_CMDQ))
> return -ENODEV;
> @@ -893,13 +893,16 @@ int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file
> cmdq = xa_load(&file_priv->cmdq_xa, args->cmdq_id);
> if (!cmdq || cmdq->is_legacy) {
> ret = -ENOENT;
> - goto unlock;
> + goto err_unlock;
> }
>
> cmdq_id = cmdq->id;
> ivpu_cmdq_destroy(file_priv, cmdq);
> + mutex_unlock(&file_priv->lock);
> ivpu_cmdq_abort_all_jobs(vdev, file_priv->ctx.id, cmdq_id);
> -unlock:
> + return 0;
> +
> +err_unlock:
> mutex_unlock(&file_priv->lock);
> return ret;
> }
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 11/14] accel/ivpu: Fix locking order in ivpu_job_submit
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
` (9 preceding siblings ...)
2025-01-07 17:32 ` [PATCH 10/14] accel/ivpu: Fix locking order in ivpu_cmdq_destroy_ioctl Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-09 8:28 ` Jacek Lawrynowicz
2025-01-07 17:32 ` [PATCH 12/14] accel/ivpu: Add handling of VPU_JSM_STATUS_MVNCI_CONTEXT_VIOLATION_HW Maciej Falkowski
` (4 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel
Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Karol Wachowski,
Maciej Falkowski
From: Karol Wachowski <karol.wachowski@intel.com>
Fix deadlock in job submission and abort handling.
When a thread aborts currently executing jobs due to a fault,
it first locks the global lock protecting submitted_jobs (#1).
After the last job is destroyed, it proceeds to release the related context
and locks file_priv (#2). Meanwhile, in the job submission thread,
the file_priv lock (#2) is taken first, and then the submitted_jobs
lock (#1) is obtained when a job is added to the submitted jobs list.
CPU0 CPU1
---- ----
(for example due to a fault) (jobs submissions keep coming)
lock(&vdev->submitted_jobs_lock) #1
ivpu_jobs_abort_all()
job_destroy()
lock(&file_priv->lock) #2
lock(&vdev->submitted_jobs_lock) #1
file_priv_release()
lock(&vdev->context_list_lock)
lock(&file_priv->lock) #2
This order of locking causes a deadlock. To resolve this issue,
change the order of locking in ivpu_job_submit().
Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_job.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index c694822a14bf..c93ea37062d7 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -597,6 +597,7 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
if (ret < 0)
return ret;
+ mutex_lock(&vdev->submitted_jobs_lock);
mutex_lock(&file_priv->lock);
if (cmdq_id == 0)
@@ -606,19 +607,17 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
if (!cmdq) {
ivpu_warn_ratelimited(vdev, "Failed to get job queue, ctx %d\n", file_priv->ctx.id);
ret = -EINVAL;
- goto err_unlock_file_priv;
+ goto err_unlock;
}
ret = ivpu_cmdq_register(file_priv, cmdq);
if (ret) {
ivpu_err(vdev, "Failed to register command queue: %d\n", ret);
- goto err_unlock_file_priv;
+ goto err_unlock;
}
job->cmdq_id = cmdq->id;
- mutex_lock(&vdev->submitted_jobs_lock);
-
is_first_job = xa_empty(&vdev->submitted_jobs_xa);
ret = xa_alloc_cyclic(&vdev->submitted_jobs_xa, &job->job_id, job, file_priv->job_limit,
&file_priv->job_id_next, GFP_KERNEL);
@@ -626,7 +625,7 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
ivpu_dbg(vdev, JOB, "Too many active jobs in ctx %d\n",
file_priv->ctx.id);
ret = -EBUSY;
- goto err_unlock_submitted_jobs;
+ goto err_unlock;
}
ret = ivpu_cmdq_push_job(cmdq, job);
@@ -649,22 +648,20 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
job->job_id, file_priv->ctx.id, job->engine_idx, cmdq->priority,
job->cmd_buf_vpu_addr, cmdq->jobq->header.tail);
- mutex_unlock(&vdev->submitted_jobs_lock);
mutex_unlock(&file_priv->lock);
if (unlikely(ivpu_test_mode & IVPU_TEST_MODE_NULL_HW)) {
- mutex_lock(&vdev->submitted_jobs_lock);
ivpu_job_signal_and_destroy(vdev, job->job_id, VPU_JSM_STATUS_SUCCESS);
- mutex_unlock(&vdev->submitted_jobs_lock);
}
+ mutex_unlock(&vdev->submitted_jobs_lock);
+
return 0;
err_erase_xa:
xa_erase(&vdev->submitted_jobs_xa, job->job_id);
-err_unlock_submitted_jobs:
+err_unlock:
mutex_unlock(&vdev->submitted_jobs_lock);
-err_unlock_file_priv:
mutex_unlock(&file_priv->lock);
ivpu_rpm_put(vdev);
return ret;
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 11/14] accel/ivpu: Fix locking order in ivpu_job_submit
2025-01-07 17:32 ` [PATCH 11/14] accel/ivpu: Fix locking order in ivpu_job_submit Maciej Falkowski
@ 2025-01-09 8:28 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:28 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo, Karol Wachowski
Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> From: Karol Wachowski <karol.wachowski@intel.com>
>
> Fix deadlock in job submission and abort handling.
> When a thread aborts currently executing jobs due to a fault,
> it first locks the global lock protecting submitted_jobs (#1).
>
> After the last job is destroyed, it proceeds to release the related context
> and locks file_priv (#2). Meanwhile, in the job submission thread,
> the file_priv lock (#2) is taken first, and then the submitted_jobs
> lock (#1) is obtained when a job is added to the submitted jobs list.
>
> CPU0 CPU1
> ---- ----
> (for example due to a fault) (jobs submissions keep coming)
>
> lock(&vdev->submitted_jobs_lock) #1
> ivpu_jobs_abort_all()
> job_destroy()
> lock(&file_priv->lock) #2
> lock(&vdev->submitted_jobs_lock) #1
> file_priv_release()
> lock(&vdev->context_list_lock)
> lock(&file_priv->lock) #2
>
> This order of locking causes a deadlock. To resolve this issue,
> change the order of locking in ivpu_job_submit().
>
> Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_job.c | 17 +++++++----------
> 1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
> index c694822a14bf..c93ea37062d7 100644
> --- a/drivers/accel/ivpu/ivpu_job.c
> +++ b/drivers/accel/ivpu/ivpu_job.c
> @@ -597,6 +597,7 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
> if (ret < 0)
> return ret;
>
> + mutex_lock(&vdev->submitted_jobs_lock);
> mutex_lock(&file_priv->lock);
>
> if (cmdq_id == 0)
> @@ -606,19 +607,17 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
> if (!cmdq) {
> ivpu_warn_ratelimited(vdev, "Failed to get job queue, ctx %d\n", file_priv->ctx.id);
> ret = -EINVAL;
> - goto err_unlock_file_priv;
> + goto err_unlock;
> }
>
> ret = ivpu_cmdq_register(file_priv, cmdq);
> if (ret) {
> ivpu_err(vdev, "Failed to register command queue: %d\n", ret);
> - goto err_unlock_file_priv;
> + goto err_unlock;
> }
>
> job->cmdq_id = cmdq->id;
>
> - mutex_lock(&vdev->submitted_jobs_lock);
> -
> is_first_job = xa_empty(&vdev->submitted_jobs_xa);
> ret = xa_alloc_cyclic(&vdev->submitted_jobs_xa, &job->job_id, job, file_priv->job_limit,
> &file_priv->job_id_next, GFP_KERNEL);
> @@ -626,7 +625,7 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
> ivpu_dbg(vdev, JOB, "Too many active jobs in ctx %d\n",
> file_priv->ctx.id);
> ret = -EBUSY;
> - goto err_unlock_submitted_jobs;
> + goto err_unlock;
> }
>
> ret = ivpu_cmdq_push_job(cmdq, job);
> @@ -649,22 +648,20 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
> job->job_id, file_priv->ctx.id, job->engine_idx, cmdq->priority,
> job->cmd_buf_vpu_addr, cmdq->jobq->header.tail);
>
> - mutex_unlock(&vdev->submitted_jobs_lock);
> mutex_unlock(&file_priv->lock);
>
> if (unlikely(ivpu_test_mode & IVPU_TEST_MODE_NULL_HW)) {
> - mutex_lock(&vdev->submitted_jobs_lock);
> ivpu_job_signal_and_destroy(vdev, job->job_id, VPU_JSM_STATUS_SUCCESS);
> - mutex_unlock(&vdev->submitted_jobs_lock);
> }
>
> + mutex_unlock(&vdev->submitted_jobs_lock);
> +
> return 0;
>
> err_erase_xa:
> xa_erase(&vdev->submitted_jobs_xa, job->job_id);
> -err_unlock_submitted_jobs:
> +err_unlock:
> mutex_unlock(&vdev->submitted_jobs_lock);
> -err_unlock_file_priv:
> mutex_unlock(&file_priv->lock);
> ivpu_rpm_put(vdev);
> return ret;
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 12/14] accel/ivpu: Add handling of VPU_JSM_STATUS_MVNCI_CONTEXT_VIOLATION_HW
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
` (10 preceding siblings ...)
2025-01-07 17:32 ` [PATCH 11/14] accel/ivpu: Fix locking order in ivpu_job_submit Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-09 8:29 ` Jacek Lawrynowicz
2025-01-07 17:32 ` [PATCH 13/14] accel/ivpu: Add platform detection for presilicon Maciej Falkowski
` (3 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel
Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Karol Wachowski,
Maciej Falkowski
From: Karol Wachowski <karol.wachowski@intel.com>
Mark as invalid context of a job that returned HW context violation
error and queue work that aborts jobs from faulty context.
Add engine reset to the context abort thread handler to not only abort
currently executing jobs but also to ensure NPU invalid state recovery.
Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_job.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index c93ea37062d7..3c162ac41a1d 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -533,6 +533,26 @@ static int ivpu_job_signal_and_destroy(struct ivpu_device *vdev, u32 job_id, u32
lockdep_assert_held(&vdev->submitted_jobs_lock);
+ job = xa_load(&vdev->submitted_jobs_xa, job_id);
+ if (!job)
+ return -ENOENT;
+
+ if (job_status == VPU_JSM_STATUS_MVNCI_CONTEXT_VIOLATION_HW) {
+ guard(mutex)(&job->file_priv->lock);
+
+ if (job->file_priv->has_mmu_faults)
+ return 0;
+
+ /*
+ * Mark context as faulty and defer destruction of the job to jobs abort thread
+ * handler to synchronize between both faults and jobs returning context violation
+ * status and ensure both are handled in the same way
+ */
+ job->file_priv->has_mmu_faults = true;
+ queue_work(system_wq, &vdev->context_abort_work);
+ return 0;
+ }
+
job = ivpu_job_remove_from_submitted_jobs(vdev, job_id);
if (!job)
return -ENOENT;
@@ -946,6 +966,9 @@ void ivpu_context_abort_work_fn(struct work_struct *work)
unsigned long ctx_id;
unsigned long id;
+ if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW)
+ ivpu_jsm_reset_engine(vdev, 0);
+
mutex_lock(&vdev->context_list_lock);
xa_for_each(&vdev->context_xa, ctx_id, file_priv) {
if (!file_priv->has_mmu_faults || file_priv->aborted)
@@ -959,6 +982,8 @@ void ivpu_context_abort_work_fn(struct work_struct *work)
if (vdev->fw->sched_mode != VPU_SCHEDULING_MODE_HW)
return;
+
+ ivpu_jsm_hws_resume_engine(vdev, 0);
/*
* In hardware scheduling mode NPU already has stopped processing jobs
* and won't send us any further notifications, thus we have to free job related resources
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 12/14] accel/ivpu: Add handling of VPU_JSM_STATUS_MVNCI_CONTEXT_VIOLATION_HW
2025-01-07 17:32 ` [PATCH 12/14] accel/ivpu: Add handling of VPU_JSM_STATUS_MVNCI_CONTEXT_VIOLATION_HW Maciej Falkowski
@ 2025-01-09 8:29 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:29 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo, Karol Wachowski
Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> From: Karol Wachowski <karol.wachowski@intel.com>
>
> Mark as invalid context of a job that returned HW context violation
> error and queue work that aborts jobs from faulty context.
> Add engine reset to the context abort thread handler to not only abort
> currently executing jobs but also to ensure NPU invalid state recovery.
>
> Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_job.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
> index c93ea37062d7..3c162ac41a1d 100644
> --- a/drivers/accel/ivpu/ivpu_job.c
> +++ b/drivers/accel/ivpu/ivpu_job.c
> @@ -533,6 +533,26 @@ static int ivpu_job_signal_and_destroy(struct ivpu_device *vdev, u32 job_id, u32
>
> lockdep_assert_held(&vdev->submitted_jobs_lock);
>
> + job = xa_load(&vdev->submitted_jobs_xa, job_id);
> + if (!job)
> + return -ENOENT;
> +
> + if (job_status == VPU_JSM_STATUS_MVNCI_CONTEXT_VIOLATION_HW) {
> + guard(mutex)(&job->file_priv->lock);
> +
> + if (job->file_priv->has_mmu_faults)
> + return 0;
> +
> + /*
> + * Mark context as faulty and defer destruction of the job to jobs abort thread
> + * handler to synchronize between both faults and jobs returning context violation
> + * status and ensure both are handled in the same way
> + */
> + job->file_priv->has_mmu_faults = true;
> + queue_work(system_wq, &vdev->context_abort_work);
> + return 0;
> + }
> +
> job = ivpu_job_remove_from_submitted_jobs(vdev, job_id);
> if (!job)
> return -ENOENT;
> @@ -946,6 +966,9 @@ void ivpu_context_abort_work_fn(struct work_struct *work)
> unsigned long ctx_id;
> unsigned long id;
>
> + if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW)
> + ivpu_jsm_reset_engine(vdev, 0);
> +
> mutex_lock(&vdev->context_list_lock);
> xa_for_each(&vdev->context_xa, ctx_id, file_priv) {
> if (!file_priv->has_mmu_faults || file_priv->aborted)
> @@ -959,6 +982,8 @@ void ivpu_context_abort_work_fn(struct work_struct *work)
>
> if (vdev->fw->sched_mode != VPU_SCHEDULING_MODE_HW)
> return;
> +
> + ivpu_jsm_hws_resume_engine(vdev, 0);
> /*
> * In hardware scheduling mode NPU already has stopped processing jobs
> * and won't send us any further notifications, thus we have to free job related resources
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 13/14] accel/ivpu: Add platform detection for presilicon
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
` (11 preceding siblings ...)
2025-01-07 17:32 ` [PATCH 12/14] accel/ivpu: Add handling of VPU_JSM_STATUS_MVNCI_CONTEXT_VIOLATION_HW Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-09 8:29 ` Jacek Lawrynowicz
2025-01-07 17:32 ` [PATCH 14/14] accel/ivpu: Enable HWS by default on all platforms Maciej Falkowski
` (2 subsequent siblings)
15 siblings, 1 reply; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel
Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Karol Wachowski,
Maciej Falkowski
From: Karol Wachowski <karol.wachowski@intel.com>
Use highest buttress VPU_STATUS register bits(15:13) that encode
platform type as follows:
0 - Silicon
2 - Simics
3 - FPGA
4 - Hybrid SLE
Remove old DMI based method.
Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_drv.h | 4 ++-
drivers/accel/ivpu/ivpu_hw.c | 41 ++++++++++-------------
drivers/accel/ivpu/ivpu_hw_btrs.c | 7 ++++
drivers/accel/ivpu/ivpu_hw_btrs.h | 1 +
drivers/accel/ivpu/ivpu_hw_btrs_lnl_reg.h | 1 +
5 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
index d53902b34070..ca21102ca366 100644
--- a/drivers/accel/ivpu/ivpu_drv.h
+++ b/drivers/accel/ivpu/ivpu_drv.h
@@ -58,6 +58,7 @@
#define IVPU_PLATFORM_SILICON 0
#define IVPU_PLATFORM_SIMICS 2
#define IVPU_PLATFORM_FPGA 3
+#define IVPU_PLATFORM_HSLE 4
#define IVPU_PLATFORM_INVALID 8
#define IVPU_SCHED_MODE_AUTO -1
@@ -288,7 +289,8 @@ static inline bool ivpu_is_simics(struct ivpu_device *vdev)
static inline bool ivpu_is_fpga(struct ivpu_device *vdev)
{
- return ivpu_get_platform(vdev) == IVPU_PLATFORM_FPGA;
+ return ivpu_get_platform(vdev) == IVPU_PLATFORM_FPGA ||
+ ivpu_get_platform(vdev) == IVPU_PLATFORM_HSLE;
}
static inline bool ivpu_is_force_snoop_enabled(struct ivpu_device *vdev)
diff --git a/drivers/accel/ivpu/ivpu_hw.c b/drivers/accel/ivpu/ivpu_hw.c
index 1b691375ee4d..e332f19ab51d 100644
--- a/drivers/accel/ivpu/ivpu_hw.c
+++ b/drivers/accel/ivpu/ivpu_hw.c
@@ -19,38 +19,31 @@ static char *platform_to_str(u32 platform)
return "SIMICS";
case IVPU_PLATFORM_FPGA:
return "FPGA";
+ case IVPU_PLATFORM_HSLE:
+ return "HSLE";
default:
return "Invalid platform";
}
}
-static const struct dmi_system_id dmi_platform_simulation[] = {
- {
- .ident = "Intel Simics",
- .matches = {
- DMI_MATCH(DMI_BOARD_NAME, "lnlrvp"),
- DMI_MATCH(DMI_BOARD_VERSION, "1.0"),
- DMI_MATCH(DMI_BOARD_SERIAL, "123456789"),
- },
- },
- {
- .ident = "Intel Simics",
- .matches = {
- DMI_MATCH(DMI_BOARD_NAME, "Simics"),
- },
- },
- { }
-};
-
static void platform_init(struct ivpu_device *vdev)
{
- if (dmi_check_system(dmi_platform_simulation))
- vdev->platform = IVPU_PLATFORM_SIMICS;
- else
- vdev->platform = IVPU_PLATFORM_SILICON;
+ int platform = ivpu_hw_btrs_platform_read(vdev);
+
+ ivpu_dbg(vdev, MISC, "Platform type: %s (%d)\n", platform_to_str(platform), platform);
+
+ switch (platform) {
+ case IVPU_PLATFORM_SILICON:
+ case IVPU_PLATFORM_SIMICS:
+ case IVPU_PLATFORM_FPGA:
+ case IVPU_PLATFORM_HSLE:
+ vdev->platform = platform;
+ break;
- ivpu_dbg(vdev, MISC, "Platform type: %s (%d)\n",
- platform_to_str(vdev->platform), vdev->platform);
+ default:
+ ivpu_err(vdev, "Invalid platform type: %d\n", platform);
+ break;
+ }
}
static void wa_init(struct ivpu_device *vdev)
diff --git a/drivers/accel/ivpu/ivpu_hw_btrs.c b/drivers/accel/ivpu/ivpu_hw_btrs.c
index 3753b00ed2d6..56c56012b980 100644
--- a/drivers/accel/ivpu/ivpu_hw_btrs.c
+++ b/drivers/accel/ivpu/ivpu_hw_btrs.c
@@ -887,3 +887,10 @@ void ivpu_hw_btrs_diagnose_failure(struct ivpu_device *vdev)
else
return diagnose_failure_lnl(vdev);
}
+
+int ivpu_hw_btrs_platform_read(struct ivpu_device *vdev)
+{
+ u32 reg = REGB_RD32(VPU_HW_BTRS_LNL_VPU_STATUS);
+
+ return REG_GET_FLD(VPU_HW_BTRS_LNL_VPU_STATUS, PLATFORM, reg);
+}
diff --git a/drivers/accel/ivpu/ivpu_hw_btrs.h b/drivers/accel/ivpu/ivpu_hw_btrs.h
index 04f14f50fed6..1fd71b4d4ab0 100644
--- a/drivers/accel/ivpu/ivpu_hw_btrs.h
+++ b/drivers/accel/ivpu/ivpu_hw_btrs.h
@@ -46,5 +46,6 @@ void ivpu_hw_btrs_global_int_disable(struct ivpu_device *vdev);
void ivpu_hw_btrs_irq_enable(struct ivpu_device *vdev);
void ivpu_hw_btrs_irq_disable(struct ivpu_device *vdev);
void ivpu_hw_btrs_diagnose_failure(struct ivpu_device *vdev);
+int ivpu_hw_btrs_platform_read(struct ivpu_device *vdev);
#endif /* __IVPU_HW_BTRS_H__ */
diff --git a/drivers/accel/ivpu/ivpu_hw_btrs_lnl_reg.h b/drivers/accel/ivpu/ivpu_hw_btrs_lnl_reg.h
index fc51f3098f97..fff2ef2cada6 100644
--- a/drivers/accel/ivpu/ivpu_hw_btrs_lnl_reg.h
+++ b/drivers/accel/ivpu/ivpu_hw_btrs_lnl_reg.h
@@ -86,6 +86,7 @@
#define VPU_HW_BTRS_LNL_VPU_STATUS_POWER_RESOURCE_OWN_ACK_MASK BIT_MASK(7)
#define VPU_HW_BTRS_LNL_VPU_STATUS_PERF_CLK_MASK BIT_MASK(11)
#define VPU_HW_BTRS_LNL_VPU_STATUS_DISABLE_CLK_RELINQUISH_MASK BIT_MASK(12)
+#define VPU_HW_BTRS_LNL_VPU_STATUS_PLATFORM_MASK GENMASK(31, 29)
#define VPU_HW_BTRS_LNL_IP_RESET 0x00000160u
#define VPU_HW_BTRS_LNL_IP_RESET_TRIGGER_MASK BIT_MASK(0)
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 13/14] accel/ivpu: Add platform detection for presilicon
2025-01-07 17:32 ` [PATCH 13/14] accel/ivpu: Add platform detection for presilicon Maciej Falkowski
@ 2025-01-09 8:29 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:29 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo, Karol Wachowski
Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> From: Karol Wachowski <karol.wachowski@intel.com>
>
> Use highest buttress VPU_STATUS register bits(15:13) that encode
> platform type as follows:
> 0 - Silicon
> 2 - Simics
> 3 - FPGA
> 4 - Hybrid SLE
>
> Remove old DMI based method.
>
> Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_drv.h | 4 ++-
> drivers/accel/ivpu/ivpu_hw.c | 41 ++++++++++-------------
> drivers/accel/ivpu/ivpu_hw_btrs.c | 7 ++++
> drivers/accel/ivpu/ivpu_hw_btrs.h | 1 +
> drivers/accel/ivpu/ivpu_hw_btrs_lnl_reg.h | 1 +
> 5 files changed, 29 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
> index d53902b34070..ca21102ca366 100644
> --- a/drivers/accel/ivpu/ivpu_drv.h
> +++ b/drivers/accel/ivpu/ivpu_drv.h
> @@ -58,6 +58,7 @@
> #define IVPU_PLATFORM_SILICON 0
> #define IVPU_PLATFORM_SIMICS 2
> #define IVPU_PLATFORM_FPGA 3
> +#define IVPU_PLATFORM_HSLE 4
> #define IVPU_PLATFORM_INVALID 8
>
> #define IVPU_SCHED_MODE_AUTO -1
> @@ -288,7 +289,8 @@ static inline bool ivpu_is_simics(struct ivpu_device *vdev)
>
> static inline bool ivpu_is_fpga(struct ivpu_device *vdev)
> {
> - return ivpu_get_platform(vdev) == IVPU_PLATFORM_FPGA;
> + return ivpu_get_platform(vdev) == IVPU_PLATFORM_FPGA ||
> + ivpu_get_platform(vdev) == IVPU_PLATFORM_HSLE;
> }
>
> static inline bool ivpu_is_force_snoop_enabled(struct ivpu_device *vdev)
> diff --git a/drivers/accel/ivpu/ivpu_hw.c b/drivers/accel/ivpu/ivpu_hw.c
> index 1b691375ee4d..e332f19ab51d 100644
> --- a/drivers/accel/ivpu/ivpu_hw.c
> +++ b/drivers/accel/ivpu/ivpu_hw.c
> @@ -19,38 +19,31 @@ static char *platform_to_str(u32 platform)
> return "SIMICS";
> case IVPU_PLATFORM_FPGA:
> return "FPGA";
> + case IVPU_PLATFORM_HSLE:
> + return "HSLE";
> default:
> return "Invalid platform";
> }
> }
>
> -static const struct dmi_system_id dmi_platform_simulation[] = {
> - {
> - .ident = "Intel Simics",
> - .matches = {
> - DMI_MATCH(DMI_BOARD_NAME, "lnlrvp"),
> - DMI_MATCH(DMI_BOARD_VERSION, "1.0"),
> - DMI_MATCH(DMI_BOARD_SERIAL, "123456789"),
> - },
> - },
> - {
> - .ident = "Intel Simics",
> - .matches = {
> - DMI_MATCH(DMI_BOARD_NAME, "Simics"),
> - },
> - },
> - { }
> -};
> -
> static void platform_init(struct ivpu_device *vdev)
> {
> - if (dmi_check_system(dmi_platform_simulation))
> - vdev->platform = IVPU_PLATFORM_SIMICS;
> - else
> - vdev->platform = IVPU_PLATFORM_SILICON;
> + int platform = ivpu_hw_btrs_platform_read(vdev);
> +
> + ivpu_dbg(vdev, MISC, "Platform type: %s (%d)\n", platform_to_str(platform), platform);
> +
> + switch (platform) {
> + case IVPU_PLATFORM_SILICON:
> + case IVPU_PLATFORM_SIMICS:
> + case IVPU_PLATFORM_FPGA:
> + case IVPU_PLATFORM_HSLE:
> + vdev->platform = platform;
> + break;
>
> - ivpu_dbg(vdev, MISC, "Platform type: %s (%d)\n",
> - platform_to_str(vdev->platform), vdev->platform);
> + default:
> + ivpu_err(vdev, "Invalid platform type: %d\n", platform);
> + break;
> + }
> }
>
> static void wa_init(struct ivpu_device *vdev)
> diff --git a/drivers/accel/ivpu/ivpu_hw_btrs.c b/drivers/accel/ivpu/ivpu_hw_btrs.c
> index 3753b00ed2d6..56c56012b980 100644
> --- a/drivers/accel/ivpu/ivpu_hw_btrs.c
> +++ b/drivers/accel/ivpu/ivpu_hw_btrs.c
> @@ -887,3 +887,10 @@ void ivpu_hw_btrs_diagnose_failure(struct ivpu_device *vdev)
> else
> return diagnose_failure_lnl(vdev);
> }
> +
> +int ivpu_hw_btrs_platform_read(struct ivpu_device *vdev)
> +{
> + u32 reg = REGB_RD32(VPU_HW_BTRS_LNL_VPU_STATUS);
> +
> + return REG_GET_FLD(VPU_HW_BTRS_LNL_VPU_STATUS, PLATFORM, reg);
> +}
> diff --git a/drivers/accel/ivpu/ivpu_hw_btrs.h b/drivers/accel/ivpu/ivpu_hw_btrs.h
> index 04f14f50fed6..1fd71b4d4ab0 100644
> --- a/drivers/accel/ivpu/ivpu_hw_btrs.h
> +++ b/drivers/accel/ivpu/ivpu_hw_btrs.h
> @@ -46,5 +46,6 @@ void ivpu_hw_btrs_global_int_disable(struct ivpu_device *vdev);
> void ivpu_hw_btrs_irq_enable(struct ivpu_device *vdev);
> void ivpu_hw_btrs_irq_disable(struct ivpu_device *vdev);
> void ivpu_hw_btrs_diagnose_failure(struct ivpu_device *vdev);
> +int ivpu_hw_btrs_platform_read(struct ivpu_device *vdev);
>
> #endif /* __IVPU_HW_BTRS_H__ */
> diff --git a/drivers/accel/ivpu/ivpu_hw_btrs_lnl_reg.h b/drivers/accel/ivpu/ivpu_hw_btrs_lnl_reg.h
> index fc51f3098f97..fff2ef2cada6 100644
> --- a/drivers/accel/ivpu/ivpu_hw_btrs_lnl_reg.h
> +++ b/drivers/accel/ivpu/ivpu_hw_btrs_lnl_reg.h
> @@ -86,6 +86,7 @@
> #define VPU_HW_BTRS_LNL_VPU_STATUS_POWER_RESOURCE_OWN_ACK_MASK BIT_MASK(7)
> #define VPU_HW_BTRS_LNL_VPU_STATUS_PERF_CLK_MASK BIT_MASK(11)
> #define VPU_HW_BTRS_LNL_VPU_STATUS_DISABLE_CLK_RELINQUISH_MASK BIT_MASK(12)
> +#define VPU_HW_BTRS_LNL_VPU_STATUS_PLATFORM_MASK GENMASK(31, 29)
>
> #define VPU_HW_BTRS_LNL_IP_RESET 0x00000160u
> #define VPU_HW_BTRS_LNL_IP_RESET_TRIGGER_MASK BIT_MASK(0)
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 14/14] accel/ivpu: Enable HWS by default on all platforms
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
` (12 preceding siblings ...)
2025-01-07 17:32 ` [PATCH 13/14] accel/ivpu: Add platform detection for presilicon Maciej Falkowski
@ 2025-01-07 17:32 ` Maciej Falkowski
2025-01-08 18:19 ` [PATCH 00/14] accel/ivpu: Changes for 6.14 Simona Vetter
2025-01-09 8:43 ` Jacek Lawrynowicz
15 siblings, 0 replies; 33+ messages in thread
From: Maciej Falkowski @ 2025-01-07 17:32 UTC (permalink / raw)
To: dri-devel; +Cc: oded.gabbay, quic_jhugo, jacek.lawrynowicz, Maciej Falkowski
From: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
Enable HWS on selected platforms if FW API version is above 3.19.
Signed-off-by: Karol Wachowski <karol.wachowski@intel.com>
Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
drivers/accel/ivpu/ivpu_fw.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/accel/ivpu/ivpu_fw.c b/drivers/accel/ivpu/ivpu_fw.c
index 6037ec0b3096..6cf1fb826d1b 100644
--- a/drivers/accel/ivpu/ivpu_fw.c
+++ b/drivers/accel/ivpu/ivpu_fw.c
@@ -145,7 +145,18 @@ ivpu_fw_sched_mode_select(struct ivpu_device *vdev, const struct vpu_firmware_he
if (ivpu_sched_mode != IVPU_SCHED_MODE_AUTO)
return ivpu_sched_mode;
- return VPU_SCHEDULING_MODE_OS;
+ if (IVPU_FW_CHECK_API_VER_LT(vdev, fw_hdr, JSM, 3, 24))
+ return VPU_SCHEDULING_MODE_OS;
+
+ switch (ivpu_device_id(vdev)) {
+ case PCI_DEVICE_ID_MTL:
+ case PCI_DEVICE_ID_ARL:
+ case PCI_DEVICE_ID_LNL:
+ case PCI_DEVICE_ID_PTL_P:
+ return VPU_SCHEDULING_MODE_HW;
+ default:
+ return VPU_SCHEDULING_MODE_OS;
+ }
}
static int ivpu_fw_parse(struct ivpu_device *vdev)
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 00/14] accel/ivpu: Changes for 6.14
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
` (13 preceding siblings ...)
2025-01-07 17:32 ` [PATCH 14/14] accel/ivpu: Enable HWS by default on all platforms Maciej Falkowski
@ 2025-01-08 18:19 ` Simona Vetter
2025-01-09 8:14 ` Jacek Lawrynowicz
2025-01-09 8:43 ` Jacek Lawrynowicz
15 siblings, 1 reply; 33+ messages in thread
From: Simona Vetter @ 2025-01-08 18:19 UTC (permalink / raw)
To: Maciej Falkowski; +Cc: dri-devel, oded.gabbay, quic_jhugo, jacek.lawrynowicz
On Tue, Jan 07, 2025 at 06:32:23PM +0100, Maciej Falkowski wrote:
> This patchset contains latest changes for Intel NPU driver
> including enabling HWS by default, adding memory utilization,
> MMU optimizations, and others.
Note that we generally cut feauture merges around -rc6 for drm subtrees,
so this is cutting it awfully close. But I guess should be ok because big
holidays and all that. In general would be better to send patches as you
go instead of a big dump before the next merge window.
-Sima
>
> Jacek Lawrynowicz (1):
> accel/ivpu: Enable HWS by default on all platforms
>
> Karol Wachowski (11):
> accel/ivpu: Separate DB ID and CMDQ ID allocations from CMDQ
> allocation
> accel/ivpu: Add API for command queue create/destroy/submit
> accel/ivpu: Abort all jobs after command queue unregister
> accel/ivpu: Dump only first MMU fault from single context
> accel/ivpu: Move parts of MMU event IRQ handling to thread handler
> accel/ivpu: Fix missing MMU events from reserved SSID
> accel/ivpu: Set command queue management capability based on HWS
> accel/ivpu: Fix locking order in ivpu_cmdq_destroy_ioctl
> accel/ivpu: Fix locking order in ivpu_job_submit
> accel/ivpu: Add handling of VPU_JSM_STATUS_MVNCI_CONTEXT_VIOLATION_HW
> accel/ivpu: Add platform detection for presilicon
>
> Maciej Falkowski (2):
> accel/ivpu: Expose NPU memory utilization info in sysfs
> accel/ivpu: Use workqueue for IRQ handling
>
> drivers/accel/ivpu/ivpu_drv.c | 88 ++--
> drivers/accel/ivpu/ivpu_drv.h | 10 +-
> drivers/accel/ivpu/ivpu_fw.c | 13 +-
> drivers/accel/ivpu/ivpu_hw.c | 46 +-
> drivers/accel/ivpu/ivpu_hw.h | 9 -
> drivers/accel/ivpu/ivpu_hw_btrs.c | 10 +-
> drivers/accel/ivpu/ivpu_hw_btrs.h | 1 +
> drivers/accel/ivpu/ivpu_hw_btrs_lnl_reg.h | 1 +
> drivers/accel/ivpu/ivpu_ipc.c | 7 +-
> drivers/accel/ivpu/ivpu_ipc.h | 2 +-
> drivers/accel/ivpu/ivpu_job.c | 508 ++++++++++++++++------
> drivers/accel/ivpu/ivpu_job.h | 8 +-
> drivers/accel/ivpu/ivpu_mmu.c | 112 ++++-
> drivers/accel/ivpu/ivpu_mmu.h | 2 +
> drivers/accel/ivpu/ivpu_mmu_context.c | 13 -
> drivers/accel/ivpu/ivpu_mmu_context.h | 2 -
> drivers/accel/ivpu/ivpu_pm.c | 3 +-
> drivers/accel/ivpu/ivpu_pm.h | 2 +-
> drivers/accel/ivpu/ivpu_sysfs.c | 35 +-
> include/uapi/drm/ivpu_accel.h | 84 ++++
> 20 files changed, 661 insertions(+), 295 deletions(-)
>
> --
> 2.43.0
>
--
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 00/14] accel/ivpu: Changes for 6.14
2025-01-08 18:19 ` [PATCH 00/14] accel/ivpu: Changes for 6.14 Simona Vetter
@ 2025-01-09 8:14 ` Jacek Lawrynowicz
0 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:14 UTC (permalink / raw)
To: Simona Vetter, Maciej Falkowski; +Cc: dri-devel, oded.gabbay, quic_jhugo
Hi,
On 1/8/2025 7:19 PM, Simona Vetter wrote:
> On Tue, Jan 07, 2025 at 06:32:23PM +0100, Maciej Falkowski wrote:
>> This patchset contains latest changes for Intel NPU driver
>> including enabling HWS by default, adding memory utilization,
>> MMU optimizations, and others.
>
> Note that we generally cut feauture merges around -rc6 for drm subtrees,
> so this is cutting it awfully close. But I guess should be ok because big
> holidays and all that. In general would be better to send patches as you
> go instead of a big dump before the next merge window.
> -Sima
Yeah, sorry about this. We had a 3 week delay due to holidays.
We will try to post patches weekly this year.
Regards,
Jacek
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 00/14] accel/ivpu: Changes for 6.14
2025-01-07 17:32 [PATCH 00/14] accel/ivpu: Changes for 6.14 Maciej Falkowski
` (14 preceding siblings ...)
2025-01-08 18:19 ` [PATCH 00/14] accel/ivpu: Changes for 6.14 Simona Vetter
@ 2025-01-09 8:43 ` Jacek Lawrynowicz
15 siblings, 0 replies; 33+ messages in thread
From: Jacek Lawrynowicz @ 2025-01-09 8:43 UTC (permalink / raw)
To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, quic_jhugo
Applied to drm-misc-next
On 1/7/2025 6:32 PM, Maciej Falkowski wrote:
> This patchset contains latest changes for Intel NPU driver
> including enabling HWS by default, adding memory utilization,
> MMU optimizations, and others.
>
> Jacek Lawrynowicz (1):
> accel/ivpu: Enable HWS by default on all platforms
>
> Karol Wachowski (11):
> accel/ivpu: Separate DB ID and CMDQ ID allocations from CMDQ
> allocation
> accel/ivpu: Add API for command queue create/destroy/submit
> accel/ivpu: Abort all jobs after command queue unregister
> accel/ivpu: Dump only first MMU fault from single context
> accel/ivpu: Move parts of MMU event IRQ handling to thread handler
> accel/ivpu: Fix missing MMU events from reserved SSID
> accel/ivpu: Set command queue management capability based on HWS
> accel/ivpu: Fix locking order in ivpu_cmdq_destroy_ioctl
> accel/ivpu: Fix locking order in ivpu_job_submit
> accel/ivpu: Add handling of VPU_JSM_STATUS_MVNCI_CONTEXT_VIOLATION_HW
> accel/ivpu: Add platform detection for presilicon
>
> Maciej Falkowski (2):
> accel/ivpu: Expose NPU memory utilization info in sysfs
> accel/ivpu: Use workqueue for IRQ handling
>
> drivers/accel/ivpu/ivpu_drv.c | 88 ++--
> drivers/accel/ivpu/ivpu_drv.h | 10 +-
> drivers/accel/ivpu/ivpu_fw.c | 13 +-
> drivers/accel/ivpu/ivpu_hw.c | 46 +-
> drivers/accel/ivpu/ivpu_hw.h | 9 -
> drivers/accel/ivpu/ivpu_hw_btrs.c | 10 +-
> drivers/accel/ivpu/ivpu_hw_btrs.h | 1 +
> drivers/accel/ivpu/ivpu_hw_btrs_lnl_reg.h | 1 +
> drivers/accel/ivpu/ivpu_ipc.c | 7 +-
> drivers/accel/ivpu/ivpu_ipc.h | 2 +-
> drivers/accel/ivpu/ivpu_job.c | 508 ++++++++++++++++------
> drivers/accel/ivpu/ivpu_job.h | 8 +-
> drivers/accel/ivpu/ivpu_mmu.c | 112 ++++-
> drivers/accel/ivpu/ivpu_mmu.h | 2 +
> drivers/accel/ivpu/ivpu_mmu_context.c | 13 -
> drivers/accel/ivpu/ivpu_mmu_context.h | 2 -
> drivers/accel/ivpu/ivpu_pm.c | 3 +-
> drivers/accel/ivpu/ivpu_pm.h | 2 +-
> drivers/accel/ivpu/ivpu_sysfs.c | 35 +-
> include/uapi/drm/ivpu_accel.h | 84 ++++
> 20 files changed, 661 insertions(+), 295 deletions(-)
>
^ permalink raw reply [flat|nested] 33+ messages in thread