Re: [PATCH 06/11] accel/ivpu: Change test_mode module param to bitmask
On Mon, Oct 30, 2023 at 08:05:28AM -0600, Jeffrey Hugo wrote: > On 10/28/2023 2:18 AM, Stanislaw Gruszka wrote: > > On Fri, Oct 27, 2023 at 08:47:11AM -0600, Jeffrey Hugo wrote: > > > On 10/25/2023 3:43 AM, Stanislaw Gruszka wrote: > > > > From: Karol Wachowski > > > > > > > > Change meaning of test_mode module parameter from integer value > > > > to bitmask allowing setting different test features with corresponding > > > > bits. > > > > > > > > Signed-off-by: Karol Wachowski > > > > Reviewed-by: Stanislaw Gruszka > > > > Signed-off-by: Stanislaw Gruszka > > > > > > Seems like this changes the uAPI. You still haven't made a release of the > > > userspace, correct? > > > > Yes the user space is not yet released. However I think module parameter > > is not considered part of the linux kernel uAPI and there are no guaranties > > regarding not changing or removing or change the semantics. > > Patch 3 of [1] seems to suggest otherwise (module parameters are part of the > uAPI) I hope it will not be applied :-) Will be quite burden to maintain module parameters compatibility. Regards Stanislaw
Re: [PATCH 06/11] accel/ivpu: Change test_mode module param to bitmask
On 10/28/2023 2:18 AM, Stanislaw Gruszka wrote: On Fri, Oct 27, 2023 at 08:47:11AM -0600, Jeffrey Hugo wrote: On 10/25/2023 3:43 AM, Stanislaw Gruszka wrote: From: Karol Wachowski Change meaning of test_mode module parameter from integer value to bitmask allowing setting different test features with corresponding bits. Signed-off-by: Karol Wachowski Reviewed-by: Stanislaw Gruszka Signed-off-by: Stanislaw Gruszka Seems like this changes the uAPI. You still haven't made a release of the userspace, correct? Yes the user space is not yet released. However I think module parameter is not considered part of the linux kernel uAPI and there are no guaranties regarding not changing or removing or change the semantics. Patch 3 of [1] seems to suggest otherwise (module parameters are part of the uAPI) [1]: https://lore.kernel.org/all/20231027193016.27516-1-quic_joh...@quicinc.com/
Re: [PATCH 06/11] accel/ivpu: Change test_mode module param to bitmask
On Fri, Oct 27, 2023 at 08:47:11AM -0600, Jeffrey Hugo wrote: > On 10/25/2023 3:43 AM, Stanislaw Gruszka wrote: > > From: Karol Wachowski > > > > Change meaning of test_mode module parameter from integer value > > to bitmask allowing setting different test features with corresponding > > bits. > > > > Signed-off-by: Karol Wachowski > > Reviewed-by: Stanislaw Gruszka > > Signed-off-by: Stanislaw Gruszka > > Seems like this changes the uAPI. You still haven't made a release of the > userspace, correct? Yes the user space is not yet released. However I think module parameter is not considered part of the linux kernel uAPI and there are no guaranties regarding not changing or removing or change the semantics. Obviously we don't want to brake anyone config file or script, but that's more like courtesy than hardcoded rule. Currently nobody except Intel and it's partners are using intel_vpu and all user should be aware of the change. Regards Stanislaw
Re: [PATCH 06/11] accel/ivpu: Change test_mode module param to bitmask
On 10/25/2023 3:43 AM, Stanislaw Gruszka wrote: From: Karol Wachowski Change meaning of test_mode module parameter from integer value to bitmask allowing setting different test features with corresponding bits. Signed-off-by: Karol Wachowski Reviewed-by: Stanislaw Gruszka Signed-off-by: Stanislaw Gruszka Seems like this changes the uAPI. You still haven't made a release of the userspace, correct? If so - Reviewed-by: Jeffrey Hugo
[PATCH 06/11] accel/ivpu: Change test_mode module param to bitmask
From: Karol Wachowski
Change meaning of test_mode module parameter from integer value
to bitmask allowing setting different test features with corresponding
bits.
Signed-off-by: Karol Wachowski
Reviewed-by: Stanislaw Gruszka
Signed-off-by: Stanislaw Gruszka
---
drivers/accel/ivpu/ivpu_drv.c | 4 ++--
drivers/accel/ivpu/ivpu_drv.h | 7 +++
drivers/accel/ivpu/ivpu_job.c | 4 ++--
3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index 2a58fb1e2fcc..4dbfd05680ce 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -39,7 +39,7 @@ MODULE_PARM_DESC(dbg_mask, "Driver debug mask. See IVPU_DBG_*
macros.");
int ivpu_test_mode;
module_param_named_unsafe(test_mode, ivpu_test_mode, int, 0644);
-MODULE_PARM_DESC(test_mode, "Test mode: 0 - disabled , 1 - fw unit test, 2 -
null hw, 3 - null submission");
+MODULE_PARM_DESC(test_mode, "Test mode mask. See IVPU_TEST_MODE_* macros.");
u8 ivpu_pll_min_ratio;
module_param_named(pll_min_ratio, ivpu_pll_min_ratio, byte, 0644);
@@ -315,7 +315,7 @@ static int ivpu_wait_for_ready(struct ivpu_device *vdev)
unsigned long timeout;
int ret;
- if (ivpu_test_mode == IVPU_TEST_MODE_FW_TEST)
+ if (ivpu_test_mode & IVPU_TEST_MODE_FW_TEST)
return 0;
ivpu_ipc_consumer_add(vdev, , IVPU_IPC_CHAN_BOOT_MSG);
diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
index 267e03a5edf4..5432b5ee90df 100644
--- a/drivers/accel/ivpu/ivpu_drv.h
+++ b/drivers/accel/ivpu/ivpu_drv.h
@@ -148,10 +148,9 @@ extern u8 ivpu_pll_min_ratio;
extern u8 ivpu_pll_max_ratio;
extern bool ivpu_disable_mmu_cont_pages;
-#define IVPU_TEST_MODE_DISABLED0
-#define IVPU_TEST_MODE_FW_TEST 1
-#define IVPU_TEST_MODE_NULL_HW 2
-#define IVPU_TEST_MODE_NULL_SUBMISSION 3
+#define IVPU_TEST_MODE_FW_TEST BIT(0)
+#define IVPU_TEST_MODE_NULL_HW BIT(1)
+#define IVPU_TEST_MODE_NULL_SUBMISSION BIT(2)
extern int ivpu_test_mode;
struct ivpu_file_priv *ivpu_file_priv_get(struct ivpu_file_priv *file_priv);
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index 646b8f812901..6e96c921547d 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -196,7 +196,7 @@ static int ivpu_cmdq_push_job(struct ivpu_cmdq *cmdq,
struct ivpu_job *job)
entry->batch_buf_addr = job->cmd_buf_vpu_addr;
entry->job_id = job->job_id;
entry->flags = 0;
- if (unlikely(ivpu_test_mode == IVPU_TEST_MODE_NULL_SUBMISSION))
+ if (unlikely(ivpu_test_mode & IVPU_TEST_MODE_NULL_SUBMISSION))
entry->flags = VPU_JOB_FLAGS_NULL_SUBMISSION_MASK;
wmb(); /* Ensure that tail is updated after filling entry */
header->tail = next_entry;
@@ -404,7 +404,7 @@ static int ivpu_direct_job_submission(struct ivpu_job *job)
job->job_id, job->cmd_buf_vpu_addr, file_priv->ctx.id,
job->engine_idx, cmdq->jobq->header.tail);
- if (ivpu_test_mode == IVPU_TEST_MODE_NULL_HW) {
+ if (ivpu_test_mode & IVPU_TEST_MODE_NULL_HW) {
ivpu_job_done(vdev, job->job_id, VPU_JSM_STATUS_SUCCESS);
cmdq->jobq->header.head = cmdq->jobq->header.tail;
wmb(); /* Flush WC buffer for jobq header */
--
2.25.1
