LinuxLists.cc - [PATCH v4 00/17] Venus QoL / maintainability fixes

[permalink] [raw]

Subject: [PATCH v4 02/17] media: venus: hfi_venus: Write to VIDC_CTRL_INIT after unmasking interrupts

The startup procedure shouldn't be started with interrupts masked, as that
may entail silent failures.

Kick off initialization only after the interrupts are unmasked.

Cc: [email protected] # v4.12+
Fixes: d96d3f30c0f2 ("[media] media: venus: hfi: add Venus HFI files")
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/hfi_venus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
index 918a283bd890..5506a0d196ef 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -453,7 +453,6 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
void __iomem *wrapper_base = hdev->core->wrapper_base;
int ret = 0;

- writel(BIT(VIDC_CTRL_INIT_CTRL_SHIFT), cpu_cs_base + VIDC_CTRL_INIT);
if (IS_V6(hdev->core)) {
mask_val = readl(wrapper_base + WRAPPER_INTR_MASK);
mask_val &= ~(WRAPPER_INTR_MASK_A2HWD_BASK_V6 |
@@ -464,6 +463,7 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
writel(mask_val, wrapper_base + WRAPPER_INTR_MASK);
writel(1, cpu_cs_base + CPU_CS_SCIACMDARG3);

+ writel(BIT(VIDC_CTRL_INIT_CTRL_SHIFT), cpu_cs_base + VIDC_CTRL_INIT);
while (!ctrl_status && count < max_tries) {
ctrl_status = readl(cpu_cs_base + CPU_CS_SCIACMDARG0);
if ((ctrl_status & CPU_CS_SCIACMDARG0_ERROR_STATUS_MASK) == 4) {

--
2.40.1

2023-05-30 12:42:28

[permalink] [raw]

Subject: [PATCH v4 14/17] media: venus: vdec: Sanitize vdec_set_work_route() per-VPU-version

Replace the general IS_V6 checks with more specific VPU version checks.

Reviewed-by: Bryan O'Donoghue <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/vdec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 12a2e99508f5..063a8b0d357b 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -727,7 +727,7 @@ static int vdec_set_work_route(struct venus_inst *inst)
u32 ptype = HFI_PROPERTY_PARAM_WORK_ROUTE;
struct hfi_video_work_route wr;

- if (!IS_V6(inst->core))
+ if (!(IS_IRIS2(inst->core) || IS_IRIS2_1(inst->core)))
return 0;

wr.video_work_route = inst->core->res->num_vpp_pipes;

--
2.40.1

2023-05-30 12:42:40

[permalink] [raw]

Subject: [PATCH v4 03/17] media: venus: Introduce VPU version distinction

The Video Processing Unit hardware version is the differentiator, based
on which the video driver should decide which code paths to take.

Up until now, we've relied on HFI versions instead, but that was just
a happy accident between recent SoCs. Add a field in the res struct
and add correlated definitions that will be used to account for the
aforementioned differences.

Reviewed-by: Vikash Garodia <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/core.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 250342d27a6f..4f8ad9659907 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -48,6 +48,14 @@ struct bw_tbl {
u32 peak_10bit;
};

+enum vpu_version {
+ VPU_VERSION_AR50,
+ VPU_VERSION_AR50_LITE,
+ VPU_VERSION_IRIS1,
+ VPU_VERSION_IRIS2,
+ VPU_VERSION_IRIS2_1,
+};
+
struct venus_resources {
u64 dma_mask;
const struct freq_tbl *freq_tbl;
@@ -71,6 +79,7 @@ struct venus_resources {
const char * const resets[VIDC_RESETS_NUM_MAX];
unsigned int resets_num;
enum hfi_version hfi_version;
+ enum vpu_version vpu_version;
u8 num_vpp_pipes;
u32 max_load;
unsigned int vmem_id;
@@ -503,6 +512,12 @@ struct venus_inst {
#define IS_V4(core) ((core)->res->hfi_version == HFI_VERSION_4XX)
#define IS_V6(core) ((core)->res->hfi_version == HFI_VERSION_6XX)

+#define IS_AR50(core) ((core)->res->vpu_version == VPU_VERSION_AR50)
+#define IS_AR50_LITE(core) ((core)->res->vpu_version == VPU_VERSION_AR50_LITE)
+#define IS_IRIS1(core) ((core)->res->vpu_version == VPU_VERSION_IRIS1)
+#define IS_IRIS2(core) ((core)->res->vpu_version == VPU_VERSION_IRIS2)
+#define IS_IRIS2_1(core) ((core)->res->vpu_version == VPU_VERSION_IRIS2_1)
+
#define ctrl_to_inst(ctrl) \
container_of((ctrl)->handler, struct venus_inst, ctrl_handler)

--
2.40.1

2023-05-30 12:46:01

[permalink] [raw]

Subject: [PATCH v4 08/17] media: venus: hfi_venus: Sanitize venus_halt_axi() per-VPU-version

Only IRIS2(_1) should enter the until-now-IS_V6() path and IRIS2_1
can be used instead of openly checking the number of VPP pipes.

Use VPU version comparison in both of these cases instead.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/hfi_venus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
index 9e36ef9076a0..60252d05781e 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -548,10 +548,10 @@ static int venus_halt_axi(struct venus_hfi_device *hdev)
u32 mask_val;
int ret;

- if (IS_V6(hdev->core)) {
+ if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
writel(0x3, cpu_cs_base + CPU_CS_X2RPMH_V6);

- if (hdev->core->res->num_vpp_pipes == 1)
+ if (IS_IRIS2_1(hdev->core))
goto skip_aon_mvp_noc;

writel(0x1, aon_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);

--
2.40.1

2023-05-30 12:46:05

[permalink] [raw]

Subject: [PATCH v4 07/17] media: venus: core: Assign registers based on VPU version

2023-05-30 12:46:05

[permalink] [raw]

Subject: [PATCH v4 06/17] media: venus: hfi_venus: Sanitize venus_boot_core() per-VPU-version

The current assumption of IS_V6 is overgeneralized. Adjust the logic
to take the VPU hardware version into account.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/hfi_venus.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
index 5506a0d196ef..9e36ef9076a0 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -447,19 +447,20 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
{
struct device *dev = hdev->core->dev;
static const unsigned int max_tries = 100;
- u32 ctrl_status = 0, mask_val;
+ u32 ctrl_status = 0, mask_val = 0;
unsigned int count = 0;
void __iomem *cpu_cs_base = hdev->core->cpu_cs_base;
void __iomem *wrapper_base = hdev->core->wrapper_base;
int ret = 0;

- if (IS_V6(hdev->core)) {
+ if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
mask_val = readl(wrapper_base + WRAPPER_INTR_MASK);
mask_val &= ~(WRAPPER_INTR_MASK_A2HWD_BASK_V6 |
WRAPPER_INTR_MASK_A2HCPU_MASK);
} else {
mask_val = WRAPPER_INTR_MASK_A2HVCODEC_MASK;
}
+
writel(mask_val, wrapper_base + WRAPPER_INTR_MASK);
writel(1, cpu_cs_base + CPU_CS_SCIACMDARG3);

@@ -479,7 +480,7 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
if (count >= max_tries)
ret = -ETIMEDOUT;

- if (IS_V6(hdev->core)) {
+ if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
writel(0x1, cpu_cs_base + CPU_CS_H2XSOFTINTEN_V6);
writel(0x0, cpu_cs_base + CPU_CS_X2RPMH_V6);
}

--
2.40.1

2023-05-30 12:47:24

[permalink] [raw]

Subject: [PATCH v4 04/17] media: venus: Add vpu_version to most SoCs

Add vpu_version where I was able to retrieve the information to
allow for more precise hardware-specific code path matching.

Reviewed-by: Dikshita Agarwal <[email protected]>
Reviewed-by: Vikash Garodia <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/core.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
index 2ae867cb4c48..01671dd23888 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -684,6 +684,7 @@ static const struct venus_resources sdm845_res = {
.vcodec_clks_num = 2,
.max_load = 3110400, /* 4096x2160@90 */
.hfi_version = HFI_VERSION_4XX,
+ .vpu_version = VPU_VERSION_AR50,
.vmem_id = VIDC_RESOURCE_NONE,
.vmem_size = 0,
.vmem_addr = 0,
@@ -709,6 +710,7 @@ static const struct venus_resources sdm845_res_v2 = {
.vcodec_num = 2,
.max_load = 3110400, /* 4096x2160@90 */
.hfi_version = HFI_VERSION_4XX,
+ .vpu_version = VPU_VERSION_AR50,
.vmem_id = VIDC_RESOURCE_NONE,
.vmem_size = 0,
.vmem_addr = 0,
@@ -756,6 +758,7 @@ static const struct venus_resources sc7180_res = {
.opp_pmdomain = (const char *[]) { "cx", NULL },
.vcodec_num = 1,
.hfi_version = HFI_VERSION_4XX,
+ .vpu_version = VPU_VERSION_AR50,
.vmem_id = VIDC_RESOURCE_NONE,
.vmem_size = 0,
.vmem_addr = 0,
@@ -809,6 +812,7 @@ static const struct venus_resources sm8250_res = {
.vcodec_num = 1,
.max_load = 7833600,
.hfi_version = HFI_VERSION_6XX,
+ .vpu_version = VPU_VERSION_IRIS2,
.num_vpp_pipes = 4,
.vmem_id = VIDC_RESOURCE_NONE,
.vmem_size = 0,
@@ -866,6 +870,7 @@ static const struct venus_resources sc7280_res = {
.opp_pmdomain = (const char *[]) { "cx", NULL },
.vcodec_num = 1,
.hfi_version = HFI_VERSION_6XX,
+ .vpu_version = VPU_VERSION_IRIS2_1,
.num_vpp_pipes = 1,
.vmem_id = VIDC_RESOURCE_NONE,
.vmem_size = 0,

--
2.40.1

2023-05-30 12:48:38

[permalink] [raw]

Subject: [PATCH v4 09/17] media: venus: hfi_venus: Sanitize venus_isr() per-VPU-version

Replace the general IS_V6 checks with more specific VPU version checks.

Reviewed-by: Dikshita Agarwal <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/hfi_venus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
index 60252d05781e..5e4b97b0a4ae 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -1109,7 +1109,7 @@ static irqreturn_t venus_isr(struct venus_core *core)
wrapper_base = hdev->core->wrapper_base;

status = readl(wrapper_base + WRAPPER_INTR_STATUS);
- if (IS_V6(core)) {
+ if (IS_IRIS2(core) || IS_IRIS2_1(core)) {
if (status & WRAPPER_INTR_STATUS_A2H_MASK ||
status & WRAPPER_INTR_STATUS_A2HWD_MASK_V6 ||
status & CPU_CS_SCIACMDARG0_INIT_IDLE_MSG_MASK)
@@ -1121,7 +1121,7 @@ static irqreturn_t venus_isr(struct venus_core *core)
hdev->irq_status = status;
}
writel(1, cpu_cs_base + CPU_CS_A2HSOFTINTCLR);
- if (!IS_V6(core))
+ if (!(IS_IRIS2(core) || IS_IRIS2_1(core)))
writel(status, wrapper_base + WRAPPER_INTR_CLEAR);

return IRQ_WAKE_THREAD;

--
2.40.1

2023-05-30 12:48:51

[permalink] [raw]

Subject: [PATCH v4 17/17] media: venus: hfi_venus: Restrict writing SCIACMDARG3 to Venus V1/V2

This write was last present on msm-3.10, which means before HFI3XX
platforms were introduced. Guard it with an appropriate if condition.

Does not seem to have any adverse effects on at least SM8250.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/hfi_venus.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
index 82854553f99e..19fc6575a489 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -462,7 +462,8 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
}

writel(mask_val, wrapper_base + WRAPPER_INTR_MASK);
- writel(1, cpu_cs_base + CPU_CS_SCIACMDARG3);
+ if (IS_V1(hdev->core))
+ writel(1, cpu_cs_base + CPU_CS_SCIACMDARG3);

writel(BIT(VIDC_CTRL_INIT_CTRL_SHIFT), cpu_cs_base + VIDC_CTRL_INIT);
while (!ctrl_status && count < max_tries) {

--
2.40.1

2023-05-30 12:49:49

[permalink] [raw]

Subject: [PATCH v4 01/17] media: venus: hfi_venus: Only consider sys_idle_indicator on V1

As per information from Qualcomm [1], this property is not really
supported beyond msm8916 (HFI V1) and some newer HFI versions really
dislike receiving it, going as far as crashing the device.

Only consider toggling it (via the module option) on HFIV1.
While at it, get rid of the global static variable (which defaulted
to zero) which was never explicitly assigned to for V1.

Note: [1] is a reply to the actual message in question, as lore did not
properly receive some of the emails..

[1] https://lore.kernel.org/lkml/[email protected]/
Fixes: 7ed9e0b3393c ("media: venus: hfi, vdec: v6 Add IS_V6() to existing IS_V4() if locations")
Fixes: d96d3f30c0f2 ("[media] media: venus: hfi: add Venus HFI files")
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/hfi_venus.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
index f0b46389e8d5..918a283bd890 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -131,7 +131,6 @@ struct venus_hfi_device {

static bool venus_pkt_debug;
int venus_fw_debug = HFI_DEBUG_MSG_ERROR | HFI_DEBUG_MSG_FATAL;
-static bool venus_sys_idle_indicator;
static bool venus_fw_low_power_mode = true;
static int venus_hw_rsp_timeout = 1000;
static bool venus_fw_coverage;
@@ -927,17 +926,12 @@ static int venus_sys_set_default_properties(struct venus_hfi_device *hdev)
if (ret)
dev_warn(dev, "setting fw debug msg ON failed (%d)\n", ret);

- /*
- * Idle indicator is disabled by default on some 4xx firmware versions,
- * enable it explicitly in order to make suspend functional by checking
- * WFI (wait-for-interrupt) bit.
- */
- if (IS_V4(hdev->core) || IS_V6(hdev->core))
- venus_sys_idle_indicator = true;
-
- ret = venus_sys_set_idle_message(hdev, venus_sys_idle_indicator);
- if (ret)
- dev_warn(dev, "setting idle response ON failed (%d)\n", ret);
+ /* HFI_PROPERTY_SYS_IDLE_INDICATOR is not supported beyond 8916 (HFI V1) */
+ if (IS_V1(hdev->core)) {
+ ret = venus_sys_set_idle_message(hdev, false);
+ if (ret)
+ dev_warn(dev, "setting idle response ON failed (%d)\n", ret);
+ }

ret = venus_sys_set_power_control(hdev, venus_fw_low_power_mode);
if (ret)

--
2.40.1

2023-05-30 12:50:07

[permalink] [raw]

Subject: [PATCH v4 12/17] media: venus: firmware: Sanitize per-VPU-version

Replace the general IS_V6 checks with more specific VPU version checks.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/firmware.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
index 572b649c56f3..ef07eea38d93 100644
--- a/drivers/media/platform/qcom/venus/firmware.c
+++ b/drivers/media/platform/qcom/venus/firmware.c
@@ -29,7 +29,7 @@ static void venus_reset_cpu(struct venus_core *core)
u32 fw_size = core->fw.mapped_mem_size;
void __iomem *wrapper_base;

- if (IS_V6(core))
+ if (IS_IRIS2_1(core))
wrapper_base = core->wrapper_tz_base;
else
wrapper_base = core->wrapper_base;
@@ -41,7 +41,7 @@ static void venus_reset_cpu(struct venus_core *core)
writel(fw_size, wrapper_base + WRAPPER_NONPIX_START_ADDR);
writel(fw_size, wrapper_base + WRAPPER_NONPIX_END_ADDR);

- if (IS_V6(core)) {
+ if (IS_IRIS2_1(core)) {
/* Bring XTSS out of reset */
writel(0, wrapper_base + WRAPPER_TZ_XTSS_SW_RESET);
} else {
@@ -67,7 +67,7 @@ int venus_set_hw_state(struct venus_core *core, bool resume)
if (resume) {
venus_reset_cpu(core);
} else {
- if (IS_V6(core))
+ if (IS_IRIS2_1(core))
writel(WRAPPER_XTSS_SW_RESET_BIT,
core->wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
else
@@ -179,7 +179,7 @@ static int venus_shutdown_no_tz(struct venus_core *core)
void __iomem *wrapper_base = core->wrapper_base;
void __iomem *wrapper_tz_base = core->wrapper_tz_base;

- if (IS_V6(core)) {
+ if (IS_IRIS2_1(core)) {
/* Assert the reset to XTSS */
reg = readl(wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
reg |= WRAPPER_XTSS_SW_RESET_BIT;

--
2.40.1

2023-05-30 12:51:35

[permalink] [raw]

Subject: [PATCH v4 16/17] media: venus: Use newly-introduced hfi_buffer_requirements accessors

Now that we have a way which is independent of the HFI version to set
the correct fields in hfi_buffer_requirements, use it!

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/helpers.c | 5 +++--
.../media/platform/qcom/venus/hfi_plat_bufs_v6.c | 22 +++++++++++-----------
2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index b70bd3dac4df..8295542e1a7c 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -668,6 +668,7 @@ int venus_helper_get_bufreq(struct venus_inst *inst, u32 type,
struct hfi_buffer_requirements *req)
{
u32 ptype = HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS;
+ enum hfi_version ver = inst->core->res->hfi_version;
union hfi_get_property hprop;
unsigned int i;
int ret;
@@ -675,12 +676,12 @@ int venus_helper_get_bufreq(struct venus_inst *inst, u32 type,
memset(req, 0, sizeof(*req));

if (type == HFI_BUFFER_OUTPUT || type == HFI_BUFFER_OUTPUT2)
- req->count_min = inst->fw_min_cnt;
+ hfi_bufreq_set_count_min(req, ver, inst->fw_min_cnt);

ret = platform_get_bufreq(inst, type, req);
if (!ret) {
if (type == HFI_BUFFER_OUTPUT || type == HFI_BUFFER_OUTPUT2)
- inst->fw_min_cnt = req->count_min;
+ inst->fw_min_cnt = hfi_bufreq_get_count_min(req, ver);
return 0;
}

diff --git a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
index e97ff8cf6d64..f5a655973c08 100644
--- a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
+++ b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
@@ -1215,24 +1215,24 @@ static int bufreq_dec(struct hfi_plat_buffers_params *params, u32 buftype,

out_min_count = output_buffer_count(VIDC_SESSION_TYPE_DEC, codec);
/* Max of driver and FW count */
- out_min_count = max(out_min_count, bufreq->count_min);
+ out_min_count = max(out_min_count, hfi_bufreq_get_count_min(bufreq, version));

bufreq->type = buftype;
bufreq->region_size = 0;
- bufreq->count_min = 1;
bufreq->count_actual = 1;
- bufreq->hold_count = 1;
+ hfi_bufreq_set_count_min(bufreq, version, 1);
+ hfi_bufreq_set_hold_count(bufreq, version, 1);
bufreq->contiguous = 1;
bufreq->alignment = 256;

if (buftype == HFI_BUFFER_INPUT) {
- bufreq->count_min = MIN_INPUT_BUFFERS;
+ hfi_bufreq_set_count_min(bufreq, version, MIN_INPUT_BUFFERS);
bufreq->size =
calculate_dec_input_frame_size(width, height, codec,
max_mbs_per_frame,
buffer_size_limit);
} else if (buftype == HFI_BUFFER_OUTPUT || buftype == HFI_BUFFER_OUTPUT2) {
- bufreq->count_min = out_min_count;
+ hfi_bufreq_set_count_min(bufreq, version, out_min_count);
bufreq->size =
venus_helper_get_framesz_raw(params->hfi_color_fmt,
out_width, out_height);
@@ -1269,7 +1269,7 @@ static int bufreq_enc(struct hfi_plat_buffers_params *params, u32 buftype,
u32 work_mode = params->enc.work_mode;
u32 rc_type = params->enc.rc_type;
u32 num_vpp_pipes = params->num_vpp_pipes;
- u32 num_ref;
+ u32 num_ref, count_min;

switch (codec) {
case V4L2_PIX_FMT_H264:
@@ -1289,21 +1289,21 @@ static int bufreq_enc(struct hfi_plat_buffers_params *params, u32 buftype,

bufreq->type = buftype;
bufreq->region_size = 0;
- bufreq->count_min = 1;
bufreq->count_actual = 1;
- bufreq->hold_count = 1;
+ hfi_bufreq_set_count_min(bufreq, version, 1);
+ hfi_bufreq_set_hold_count(bufreq, version, 1);
bufreq->contiguous = 1;
bufreq->alignment = 256;

if (buftype == HFI_BUFFER_INPUT) {
- bufreq->count_min = MIN_INPUT_BUFFERS;
+ hfi_bufreq_set_count_min(bufreq, version, MIN_INPUT_BUFFERS);
bufreq->size =
venus_helper_get_framesz_raw(params->hfi_color_fmt,
width, height);
} else if (buftype == HFI_BUFFER_OUTPUT ||
buftype == HFI_BUFFER_OUTPUT2) {
- bufreq->count_min =
- output_buffer_count(VIDC_SESSION_TYPE_ENC, codec);
+ count_min = output_buffer_count(VIDC_SESSION_TYPE_ENC, codec);
+ hfi_bufreq_set_count_min(bufreq, version, count_min);
bufreq->size = calculate_enc_output_frame_size(width, height,
rc_type);
} else if (buftype == HFI_BUFFER_INTERNAL_SCRATCH(version)) {

--
2.40.1

2023-05-30 12:53:16

[permalink] [raw]

Subject: [PATCH v4 15/17] media: venus: Introduce accessors for remapped hfi_buffer_reqs members

Currently we have macros to access these, but they don't provide a
way to override the remapped fields. Replace the macros with actual
get/set pairs to fix that.

Reviewed-by: Bryan O'Donoghue <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/helpers.c | 2 +-
drivers/media/platform/qcom/venus/hfi_helper.h | 61 ++++++++++++++++++++++----
drivers/media/platform/qcom/venus/hfi_msgs.c | 2 +-
drivers/media/platform/qcom/venus/vdec.c | 8 ++--
drivers/media/platform/qcom/venus/vdec_ctrls.c | 2 +-
drivers/media/platform/qcom/venus/venc.c | 4 +-
drivers/media/platform/qcom/venus/venc_ctrls.c | 2 +-
7 files changed, 63 insertions(+), 18 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 1822e85ab6bf..b70bd3dac4df 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -189,7 +189,7 @@ int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
if (ret)
return ret;

- count = HFI_BUFREQ_COUNT_MIN(&bufreq, ver);
+ count = hfi_bufreq_get_count_min(&bufreq, ver);

for (i = 0; i < count; i++) {
buf = kzalloc(sizeof(*buf), GFP_KERNEL);
diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
index 0abbc50c5864..e4c05d62cfc7 100644
--- a/drivers/media/platform/qcom/venus/hfi_helper.h
+++ b/drivers/media/platform/qcom/venus/hfi_helper.h
@@ -1170,14 +1170,6 @@ struct hfi_buffer_display_hold_count_actual {
u32 hold_count;
};

-/* HFI 4XX reorder the fields, use these macros */
-#define HFI_BUFREQ_HOLD_COUNT(bufreq, ver) \
- ((ver) == HFI_VERSION_4XX ? 0 : (bufreq)->hold_count)
-#define HFI_BUFREQ_COUNT_MIN(bufreq, ver) \
- ((ver) == HFI_VERSION_4XX ? (bufreq)->hold_count : (bufreq)->count_min)
-#define HFI_BUFREQ_COUNT_MIN_HOST(bufreq, ver) \
- ((ver) == HFI_VERSION_4XX ? (bufreq)->count_min : 0)
-
struct hfi_buffer_requirements {
u32 type;
u32 size;
@@ -1189,6 +1181,59 @@ struct hfi_buffer_requirements {
u32 alignment;
};

+/* On HFI 4XX, some of the struct members have been swapped. */
+static inline u32 hfi_bufreq_get_hold_count(struct hfi_buffer_requirements *req,
+ u32 ver)
+{
+ if (ver == HFI_VERSION_4XX)
+ return 0;
+
+ return req->hold_count;
+};
+
+static inline u32 hfi_bufreq_get_count_min(struct hfi_buffer_requirements *req,
+ u32 ver)
+{
+ if (ver == HFI_VERSION_4XX)
+ return req->hold_count;
+
+ return req->count_min;
+};
+
+static inline u32 hfi_bufreq_get_count_min_host(struct hfi_buffer_requirements *req,
+ u32 ver)
+{
+ if (ver == HFI_VERSION_4XX)
+ return req->count_min;
+
+ return 0;
+};
+
+static inline void hfi_bufreq_set_hold_count(struct hfi_buffer_requirements *req,
+ u32 ver, u32 val)
+{
+ if (ver == HFI_VERSION_4XX)
+ return;
+
+ req->hold_count = val;
+};
+
+static inline void hfi_bufreq_set_count_min(struct hfi_buffer_requirements *req,
+ u32 ver, u32 val)
+{
+ if (ver == HFI_VERSION_4XX)
+ req->hold_count = val;
+
+ req->count_min = val;
+};
+
+static inline void hfi_bufreq_set_count_min_host(struct hfi_buffer_requirements *req,
+ u32 ver, u32 val)
+{
+ if (ver == HFI_VERSION_4XX)
+ req->count_min = val;
+};
+
struct hfi_data_payload {
u32 size;
u8 data[1];
diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c
index 3d5dadfa1900..7cab685a2ec8 100644
--- a/drivers/media/platform/qcom/venus/hfi_msgs.c
+++ b/drivers/media/platform/qcom/venus/hfi_msgs.c
@@ -99,7 +99,7 @@ static void event_seq_changed(struct venus_core *core, struct venus_inst *inst,
case HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS:
data_ptr += sizeof(u32);
bufreq = (struct hfi_buffer_requirements *)data_ptr;
- event.buf_count = HFI_BUFREQ_COUNT_MIN(bufreq, ver);
+ event.buf_count = hfi_bufreq_get_count_min(bufreq, ver);
data_ptr += sizeof(*bufreq);
break;
case HFI_INDEX_EXTRADATA_INPUT_CROP:
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 063a8b0d357b..2a1e038f92cf 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -899,13 +899,13 @@ static int vdec_num_buffers(struct venus_inst *inst, unsigned int *in_num,
if (ret)
return ret;

- *in_num = HFI_BUFREQ_COUNT_MIN(&bufreq, ver);
+ *in_num = hfi_bufreq_get_count_min(&bufreq, ver);

ret = venus_helper_get_bufreq(inst, HFI_BUFFER_OUTPUT, &bufreq);
if (ret)
return ret;

- *out_num = HFI_BUFREQ_COUNT_MIN(&bufreq, ver);
+ *out_num = hfi_bufreq_get_count_min(&bufreq, ver);

return 0;
}
@@ -1019,14 +1019,14 @@ static int vdec_verify_conf(struct venus_inst *inst)
return ret;

if (inst->num_output_bufs < bufreq.count_actual ||
- inst->num_output_bufs < HFI_BUFREQ_COUNT_MIN(&bufreq, ver))
+ inst->num_output_bufs < hfi_bufreq_get_count_min(&bufreq, ver))
return -EINVAL;

ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, &bufreq);
if (ret)
return ret;

- if (inst->num_input_bufs < HFI_BUFREQ_COUNT_MIN(&bufreq, ver))
+ if (inst->num_input_bufs < hfi_bufreq_get_count_min(&bufreq, ver))
return -EINVAL;

return 0;
diff --git a/drivers/media/platform/qcom/venus/vdec_ctrls.c b/drivers/media/platform/qcom/venus/vdec_ctrls.c
index fbe12a608b21..7e0f29bf7fae 100644
--- a/drivers/media/platform/qcom/venus/vdec_ctrls.c
+++ b/drivers/media/platform/qcom/venus/vdec_ctrls.c
@@ -79,7 +79,7 @@ static int vdec_op_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
ret = venus_helper_get_bufreq(inst, HFI_BUFFER_OUTPUT, &bufreq);
if (!ret)
- ctrl->val = HFI_BUFREQ_COUNT_MIN(&bufreq, ver);
+ ctrl->val = hfi_bufreq_get_count_min(&bufreq, ver);
break;
default:
return -EINVAL;
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index b60772cc2cdc..d2e2d3108752 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -1207,7 +1207,7 @@ static int venc_verify_conf(struct venus_inst *inst)
return ret;

if (inst->num_output_bufs < bufreq.count_actual ||
- inst->num_output_bufs < HFI_BUFREQ_COUNT_MIN(&bufreq, ver))
+ inst->num_output_bufs < hfi_bufreq_get_count_min(&bufreq, ver))
return -EINVAL;

ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, &bufreq);
@@ -1215,7 +1215,7 @@ static int venc_verify_conf(struct venus_inst *inst)
return ret;

if (inst->num_input_bufs < bufreq.count_actual ||
- inst->num_input_bufs < HFI_BUFREQ_COUNT_MIN(&bufreq, ver))
+ inst->num_input_bufs < hfi_bufreq_get_count_min(&bufreq, ver))
return -EINVAL;

return 0;
diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c
index 7468e43800a9..d9d2a293f3ef 100644
--- a/drivers/media/platform/qcom/venus/venc_ctrls.c
+++ b/drivers/media/platform/qcom/venus/venc_ctrls.c
@@ -358,7 +358,7 @@ static int venc_op_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:
ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, &bufreq);
if (!ret)
- ctrl->val = HFI_BUFREQ_COUNT_MIN(&bufreq, ver);
+ ctrl->val = hfi_bufreq_get_count_min(&bufreq, ver);
break;
default:
return -EINVAL;

--
2.40.1

2023-05-30 12:54:01

[permalink] [raw]

Subject: [PATCH v4 05/17] media: venus: firmware: Leave a clue about obtaining CP VARs

The qcom_scm_mem_protect_video_var accepts two sets of <addr size>
pairs as arguments. They describe the virtual address ranges of the
CP (Content Protection) and CP_NONPIXEL regions. It is however not
immediately obvious how to obtain these values.

Leave a comment explaining how one can translate the vendor device
tree properties for use with the mainline driver.

Reviewed-by: Dikshita Agarwal <[email protected]>
Reviewed-by: Vikash Garodia <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/firmware.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
index cfb11c551167..572b649c56f3 100644
--- a/drivers/media/platform/qcom/venus/firmware.c
+++ b/drivers/media/platform/qcom/venus/firmware.c
@@ -241,6 +241,16 @@ int venus_boot(struct venus_core *core)
return ret;

if (core->use_tz && res->cp_size) {
+ /*
+ * Clues for porting using downstream data:
+ * cp_start = 0
+ * cp_size = venus_ns/virtual-addr-pool[0] - yes, address and not size!
+ * This works, as the non-secure context bank is placed
+ * contiguously right after the Content Protection region.
+ *
+ * cp_nonpixel_start = venus_sec_non_pixel/virtual-addr-pool[0]
+ * cp_nonpixel_size = venus_sec_non_pixel/virtual-addr-pool[1]
+ */
ret = qcom_scm_mem_protect_video_var(res->cp_start,
res->cp_size,
res->cp_nonpixel_start,

--
2.40.1

2023-05-30 12:54:03

[permalink] [raw]

Subject: [PATCH v4 10/17] media: venus: hfi_venus: Sanitize venus_cpu_and_video_core_idle() per-VPU-version

Replace the general IS_V6 checks with more specific VPU version checks.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/media/platform/qcom/venus/hfi_venus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
index 5e4b97b0a4ae..b5d7aab03bca 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -1516,7 +1516,7 @@ static bool venus_cpu_and_video_core_idle(struct venus_hfi_device *hdev)
void __iomem *cpu_cs_base = hdev->core->cpu_cs_base;
u32 ctrl_status, cpu_status;

- if (IS_V6(hdev->core))
+ if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core))
cpu_status = readl(wrapper_tz_base + WRAPPER_TZ_CPU_STATUS_V6);
else
cpu_status = readl(wrapper_base + WRAPPER_CPU_STATUS);

--
2.40.1

2023-06-01 09:19:02

[permalink] [raw]

Subject: Re: [PATCH v4 01/17] media: venus: hfi_venus: Only consider sys_idle_indicator on V1

On 5/30/2023 6:00 PM, Konrad Dybcio wrote:
> As per information from Qualcomm [1], this property is not really
> supported beyond msm8916 (HFI V1) and some newer HFI versions really
> dislike receiving it, going as far as crashing the device.
>
> Only consider toggling it (via the module option) on HFIV1.
> While at it, get rid of the global static variable (which defaulted
> to zero) which was never explicitly assigned to for V1.
>
> Note: [1] is a reply to the actual message in question, as lore did not
> properly receive some of the emails..
>
> [1] https://lore.kernel.org/lkml/[email protected]/
> Fixes: 7ed9e0b3393c ("media: venus: hfi, vdec: v6 Add IS_V6() to existing IS_V4() if locations")
> Fixes: d96d3f30c0f2 ("[media] media: venus: hfi: add Venus HFI files")
> Signed-off-by: Konrad Dybcio <[email protected]>

Reviewed-by: Vikash Garodia <[email protected]>

> ---
> drivers/media/platform/qcom/venus/hfi_venus.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
> index f0b46389e8d5..918a283bd890 100644
> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
> @@ -131,7 +131,6 @@ struct venus_hfi_device {
>
> static bool venus_pkt_debug;
> int venus_fw_debug = HFI_DEBUG_MSG_ERROR | HFI_DEBUG_MSG_FATAL;
> -static bool venus_sys_idle_indicator;
> static bool venus_fw_low_power_mode = true;
> static int venus_hw_rsp_timeout = 1000;
> static bool venus_fw_coverage;
> @@ -927,17 +926,12 @@ static int venus_sys_set_default_properties(struct venus_hfi_device *hdev)
> if (ret)
> dev_warn(dev, "setting fw debug msg ON failed (%d)\n", ret);
>
> - /*
> - * Idle indicator is disabled by default on some 4xx firmware versions,
> - * enable it explicitly in order to make suspend functional by checking
> - * WFI (wait-for-interrupt) bit.
> - */
> - if (IS_V4(hdev->core) || IS_V6(hdev->core))
> - venus_sys_idle_indicator = true;
> -
> - ret = venus_sys_set_idle_message(hdev, venus_sys_idle_indicator);
> - if (ret)
> - dev_warn(dev, "setting idle response ON failed (%d)\n", ret);
> + /* HFI_PROPERTY_SYS_IDLE_INDICATOR is not supported beyond 8916 (HFI V1) */
> + if (IS_V1(hdev->core)) {
> + ret = venus_sys_set_idle_message(hdev, false);
> + if (ret)
> + dev_warn(dev, "setting idle response ON failed (%d)\n", ret);
> + }
>
> ret = venus_sys_set_power_control(hdev, venus_fw_low_power_mode);
> if (ret)
>

2023-06-01 09:21:03

[permalink] [raw]

Subject: Re: [PATCH v4 08/17] media: venus: hfi_venus: Sanitize venus_halt_axi() per-VPU-version

On 5/30/2023 6:00 PM, Konrad Dybcio wrote:
> Only IRIS2(_1) should enter the until-now-IS_V6() path and IRIS2_1
> can be used instead of openly checking the number of VPP pipes.
>
> Use VPU version comparison in both of these cases instead.
>
> Signed-off-by: Konrad Dybcio <[email protected]>

Reviewed-by: Vikash Garodia <[email protected]>

> ---
> drivers/media/platform/qcom/venus/hfi_venus.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
> index 9e36ef9076a0..60252d05781e 100644
> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
> @@ -548,10 +548,10 @@ static int venus_halt_axi(struct venus_hfi_device *hdev)
> u32 mask_val;
> int ret;
>
> - if (IS_V6(hdev->core)) {
> + if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
> writel(0x3, cpu_cs_base + CPU_CS_X2RPMH_V6);
>
> - if (hdev->core->res->num_vpp_pipes == 1)
> + if (IS_IRIS2_1(hdev->core))
> goto skip_aon_mvp_noc;
>
> writel(0x1, aon_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
>

2023-06-01 09:27:49

[permalink] [raw]

Subject: Re: [PATCH v4 02/17] media: venus: hfi_venus: Write to VIDC_CTRL_INIT after unmasking interrupts

On 5/30/2023 6:00 PM, Konrad Dybcio wrote:
> The startup procedure shouldn't be started with interrupts masked, as that
> may entail silent failures.
>
> Kick off initialization only after the interrupts are unmasked.
>
> Cc: [email protected] # v4.12+
> Fixes: d96d3f30c0f2 ("[media] media: venus: hfi: add Venus HFI files")
> Signed-off-by: Konrad Dybcio <[email protected]>

Reviewed-by: Vikash Garodia <[email protected]>

> ---
> drivers/media/platform/qcom/venus/hfi_venus.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
> index 918a283bd890..5506a0d196ef 100644
> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
> @@ -453,7 +453,6 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
> void __iomem *wrapper_base = hdev->core->wrapper_base;
> int ret = 0;
>
> - writel(BIT(VIDC_CTRL_INIT_CTRL_SHIFT), cpu_cs_base + VIDC_CTRL_INIT);
> if (IS_V6(hdev->core)) {
> mask_val = readl(wrapper_base + WRAPPER_INTR_MASK);
> mask_val &= ~(WRAPPER_INTR_MASK_A2HWD_BASK_V6 |
> @@ -464,6 +463,7 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
> writel(mask_val, wrapper_base + WRAPPER_INTR_MASK);
> writel(1, cpu_cs_base + CPU_CS_SCIACMDARG3);
>
> + writel(BIT(VIDC_CTRL_INIT_CTRL_SHIFT), cpu_cs_base + VIDC_CTRL_INIT);
> while (!ctrl_status && count < max_tries) {
> ctrl_status = readl(cpu_cs_base + CPU_CS_SCIACMDARG0);
> if ((ctrl_status & CPU_CS_SCIACMDARG0_ERROR_STATUS_MASK) == 4) {
>

2023-06-01 09:28:05

[permalink] [raw]

Subject: Re: [PATCH v4 14/17] media: venus: vdec: Sanitize vdec_set_work_route() per-VPU-version

On 5/30/2023 6:00 PM, Konrad Dybcio wrote:
> Replace the general IS_V6 checks with more specific VPU version checks.
>
> Reviewed-by: Bryan O'Donoghue <[email protected]>
> Signed-off-by: Konrad Dybcio <[email protected]>

Reviewed-by: Vikash Garodia <[email protected]>

> ---
> drivers/media/platform/qcom/venus/vdec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 12a2e99508f5..063a8b0d357b 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -727,7 +727,7 @@ static int vdec_set_work_route(struct venus_inst *inst)
> u32 ptype = HFI_PROPERTY_PARAM_WORK_ROUTE;
> struct hfi_video_work_route wr;
>
> - if (!IS_V6(inst->core))
> + if (!(IS_IRIS2(inst->core) || IS_IRIS2_1(inst->core)))
> return 0;
>
> wr.video_work_route = inst->core->res->num_vpp_pipes;
>

2023-06-01 09:30:00

[permalink] [raw]

Subject: Re: [PATCH v4 10/17] media: venus: hfi_venus: Sanitize venus_cpu_and_video_core_idle() per-VPU-version

2023-06-01 09:31:07

[permalink] [raw]

Subject: Re: [PATCH v4 06/17] media: venus: hfi_venus: Sanitize venus_boot_core() per-VPU-version

On 5/30/2023 6:00 PM, Konrad Dybcio wrote:
> The current assumption of IS_V6 is overgeneralized. Adjust the logic
> to take the VPU hardware version into account.
>
> Signed-off-by: Konrad Dybcio <[email protected]>

Reviewed-by: Vikash Garodia <[email protected]>

> ---
> drivers/media/platform/qcom/venus/hfi_venus.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
> index 5506a0d196ef..9e36ef9076a0 100644
> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
> @@ -447,19 +447,20 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
> {
> struct device *dev = hdev->core->dev;
> static const unsigned int max_tries = 100;
> - u32 ctrl_status = 0, mask_val;
> + u32 ctrl_status = 0, mask_val = 0;
> unsigned int count = 0;
> void __iomem *cpu_cs_base = hdev->core->cpu_cs_base;
> void __iomem *wrapper_base = hdev->core->wrapper_base;
> int ret = 0;
>
> - if (IS_V6(hdev->core)) {
> + if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
> mask_val = readl(wrapper_base + WRAPPER_INTR_MASK);
> mask_val &= ~(WRAPPER_INTR_MASK_A2HWD_BASK_V6 |
> WRAPPER_INTR_MASK_A2HCPU_MASK);
> } else {
> mask_val = WRAPPER_INTR_MASK_A2HVCODEC_MASK;
> }
> +
> writel(mask_val, wrapper_base + WRAPPER_INTR_MASK);
> writel(1, cpu_cs_base + CPU_CS_SCIACMDARG3);
>
> @@ -479,7 +480,7 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
> if (count >= max_tries)
> ret = -ETIMEDOUT;
>
> - if (IS_V6(hdev->core)) {
> + if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
> writel(0x1, cpu_cs_base + CPU_CS_H2XSOFTINTEN_V6);
> writel(0x0, cpu_cs_base + CPU_CS_X2RPMH_V6);
> }
>

2023-06-01 09:33:59

[permalink] [raw]

Subject: Re: [PATCH v4 09/17] media: venus: hfi_venus: Sanitize venus_isr() per-VPU-version

On 5/30/2023 6:00 PM, Konrad Dybcio wrote:
> Replace the general IS_V6 checks with more specific VPU version checks.
>
> Reviewed-by: Dikshita Agarwal <[email protected]>
> Signed-off-by: Konrad Dybcio <[email protected]>

Reviewed-by: Vikash Garodia <[email protected]>

> ---
> drivers/media/platform/qcom/venus/hfi_venus.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
> index 60252d05781e..5e4b97b0a4ae 100644
> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
> @@ -1109,7 +1109,7 @@ static irqreturn_t venus_isr(struct venus_core *core)
> wrapper_base = hdev->core->wrapper_base;
>
> status = readl(wrapper_base + WRAPPER_INTR_STATUS);
> - if (IS_V6(core)) {
> + if (IS_IRIS2(core) || IS_IRIS2_1(core)) {
> if (status & WRAPPER_INTR_STATUS_A2H_MASK ||
> status & WRAPPER_INTR_STATUS_A2HWD_MASK_V6 ||
> status & CPU_CS_SCIACMDARG0_INIT_IDLE_MSG_MASK)
> @@ -1121,7 +1121,7 @@ static irqreturn_t venus_isr(struct venus_core *core)
> hdev->irq_status = status;
> }
> writel(1, cpu_cs_base + CPU_CS_A2HSOFTINTCLR);
> - if (!IS_V6(core))
> + if (!(IS_IRIS2(core) || IS_IRIS2_1(core)))
> writel(status, wrapper_base + WRAPPER_INTR_CLEAR);
>
> return IRQ_WAKE_THREAD;
>

2023-06-01 09:37:01

[permalink] [raw]

Subject: Re: [PATCH v4 12/17] media: venus: firmware: Sanitize per-VPU-version

On 5/30/2023 6:00 PM, Konrad Dybcio wrote:
> Replace the general IS_V6 checks with more specific VPU version checks.
>
> Signed-off-by: Konrad Dybcio <[email protected]>

Reviewed-by: Vikash Garodia <[email protected]>

> ---
> drivers/media/platform/qcom/venus/firmware.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> index 572b649c56f3..ef07eea38d93 100644
> --- a/drivers/media/platform/qcom/venus/firmware.c
> +++ b/drivers/media/platform/qcom/venus/firmware.c
> @@ -29,7 +29,7 @@ static void venus_reset_cpu(struct venus_core *core)
> u32 fw_size = core->fw.mapped_mem_size;
> void __iomem *wrapper_base;
>
> - if (IS_V6(core))
> + if (IS_IRIS2_1(core))
> wrapper_base = core->wrapper_tz_base;
> else
> wrapper_base = core->wrapper_base;
> @@ -41,7 +41,7 @@ static void venus_reset_cpu(struct venus_core *core)
> writel(fw_size, wrapper_base + WRAPPER_NONPIX_START_ADDR);
> writel(fw_size, wrapper_base + WRAPPER_NONPIX_END_ADDR);
>
> - if (IS_V6(core)) {
> + if (IS_IRIS2_1(core)) {
> /* Bring XTSS out of reset */
> writel(0, wrapper_base + WRAPPER_TZ_XTSS_SW_RESET);
> } else {
> @@ -67,7 +67,7 @@ int venus_set_hw_state(struct venus_core *core, bool resume)
> if (resume) {
> venus_reset_cpu(core);
> } else {
> - if (IS_V6(core))
> + if (IS_IRIS2_1(core))
> writel(WRAPPER_XTSS_SW_RESET_BIT,
> core->wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
> else
> @@ -179,7 +179,7 @@ static int venus_shutdown_no_tz(struct venus_core *core)
> void __iomem *wrapper_base = core->wrapper_base;
> void __iomem *wrapper_tz_base = core->wrapper_tz_base;
>
> - if (IS_V6(core)) {
> + if (IS_IRIS2_1(core)) {
> /* Assert the reset to XTSS */
> reg = readl(wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
> reg |= WRAPPER_XTSS_SW_RESET_BIT;
>

2023-06-01 09:38:08

[permalink] [raw]

Subject: Re: [PATCH v4 17/17] media: venus: hfi_venus: Restrict writing SCIACMDARG3 to Venus V1/V2

On 5/30/2023 6:00 PM, Konrad Dybcio wrote:
> This write was last present on msm-3.10, which means before HFI3XX
> platforms were introduced. Guard it with an appropriate if condition.
>
> Does not seem to have any adverse effects on at least SM8250.
>
> Signed-off-by: Konrad Dybcio <[email protected]>

Reviewed-by: Vikash Garodia <[email protected]>

> ---
> drivers/media/platform/qcom/venus/hfi_venus.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
> index 82854553f99e..19fc6575a489 100644
> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
> @@ -462,7 +462,8 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
> }
>
> writel(mask_val, wrapper_base + WRAPPER_INTR_MASK);
> - writel(1, cpu_cs_base + CPU_CS_SCIACMDARG3);
> + if (IS_V1(hdev->core))
> + writel(1, cpu_cs_base + CPU_CS_SCIACMDARG3);
>
> writel(BIT(VIDC_CTRL_INIT_CTRL_SHIFT), cpu_cs_base + VIDC_CTRL_INIT);
> while (!ctrl_status && count < max_tries) {
>

2023-06-01 09:39:51

[permalink] [raw]

Subject: Re: [PATCH v4 07/17] media: venus: core: Assign registers based on VPU version

2023-06-01 09:41:05

[permalink] [raw]

Subject: Re: [PATCH v4 12/17] media: venus: firmware: Sanitize per-VPU-version

On 5/30/2023 6:00 PM, Konrad Dybcio wrote:
> Replace the general IS_V6 checks with more specific VPU version checks.
>
> Signed-off-by: Konrad Dybcio <[email protected]>

Reviewed-by: Dikshita Agarwal <[email protected]>

> ---
> drivers/media/platform/qcom/venus/firmware.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> index 572b649c56f3..ef07eea38d93 100644
> --- a/drivers/media/platform/qcom/venus/firmware.c
> +++ b/drivers/media/platform/qcom/venus/firmware.c
> @@ -29,7 +29,7 @@ static void venus_reset_cpu(struct venus_core *core)
> u32 fw_size = core->fw.mapped_mem_size;
> void __iomem *wrapper_base;
>
> - if (IS_V6(core))
> + if (IS_IRIS2_1(core))
> wrapper_base = core->wrapper_tz_base;
> else
> wrapper_base = core->wrapper_base;
> @@ -41,7 +41,7 @@ static void venus_reset_cpu(struct venus_core *core)
> writel(fw_size, wrapper_base + WRAPPER_NONPIX_START_ADDR);
> writel(fw_size, wrapper_base + WRAPPER_NONPIX_END_ADDR);
>
> - if (IS_V6(core)) {
> + if (IS_IRIS2_1(core)) {
> /* Bring XTSS out of reset */
> writel(0, wrapper_base + WRAPPER_TZ_XTSS_SW_RESET);
> } else {
> @@ -67,7 +67,7 @@ int venus_set_hw_state(struct venus_core *core, bool resume)
> if (resume) {
> venus_reset_cpu(core);
> } else {
> - if (IS_V6(core))
> + if (IS_IRIS2_1(core))
> writel(WRAPPER_XTSS_SW_RESET_BIT,
> core->wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
> else
> @@ -179,7 +179,7 @@ static int venus_shutdown_no_tz(struct venus_core *core)
> void __iomem *wrapper_base = core->wrapper_base;
> void __iomem *wrapper_tz_base = core->wrapper_tz_base;
>
> - if (IS_V6(core)) {
> + if (IS_IRIS2_1(core)) {
> /* Assert the reset to XTSS */
> reg = readl(wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
> reg |= WRAPPER_XTSS_SW_RESET_BIT;
>

2023-06-01 09:49:31

[permalink] [raw]

Subject: Re: [PATCH v4 15/17] media: venus: Introduce accessors for remapped hfi_buffer_reqs members

On 5/30/2023 6:00 PM, Konrad Dybcio wrote:
> Currently we have macros to access these, but they don't provide a
> way to override the remapped fields. Replace the macros with actual
> get/set pairs to fix that.
>
> Reviewed-by: Bryan O'Donoghue <[email protected]>
> Signed-off-by: Konrad Dybcio <[email protected]>

Reviewed-by: Dikshita Agarwal <[email protected]>

> ---
> drivers/media/platform/qcom/venus/helpers.c | 2 +-
> drivers/media/platform/qcom/venus/hfi_helper.h | 61 ++++++++++++++++++++++----
> drivers/media/platform/qcom/venus/hfi_msgs.c | 2 +-
> drivers/media/platform/qcom/venus/vdec.c | 8 ++--
> drivers/media/platform/qcom/venus/vdec_ctrls.c | 2 +-
> drivers/media/platform/qcom/venus/venc.c | 4 +-
> drivers/media/platform/qcom/venus/venc_ctrls.c | 2 +-
> 7 files changed, 63 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index 1822e85ab6bf..b70bd3dac4df 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -189,7 +189,7 @@ int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
> if (ret)
> return ret;
>
> - count = HFI_BUFREQ_COUNT_MIN(&bufreq, ver);
> + count = hfi_bufreq_get_count_min(&bufreq, ver);
>
> for (i = 0; i < count; i++) {
> buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
> index 0abbc50c5864..e4c05d62cfc7 100644
> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
> @@ -1170,14 +1170,6 @@ struct hfi_buffer_display_hold_count_actual {
> u32 hold_count;
> };
>
> -/* HFI 4XX reorder the fields, use these macros */
> -#define HFI_BUFREQ_HOLD_COUNT(bufreq, ver) \
> - ((ver) == HFI_VERSION_4XX ? 0 : (bufreq)->hold_count)
> -#define HFI_BUFREQ_COUNT_MIN(bufreq, ver) \
> - ((ver) == HFI_VERSION_4XX ? (bufreq)->hold_count : (bufreq)->count_min)
> -#define HFI_BUFREQ_COUNT_MIN_HOST(bufreq, ver) \
> - ((ver) == HFI_VERSION_4XX ? (bufreq)->count_min : 0)
> -
> struct hfi_buffer_requirements {
> u32 type;
> u32 size;
> @@ -1189,6 +1181,59 @@ struct hfi_buffer_requirements {
> u32 alignment;
> };
>
> +/* On HFI 4XX, some of the struct members have been swapped. */
> +static inline u32 hfi_bufreq_get_hold_count(struct hfi_buffer_requirements *req,
> + u32 ver)
> +{
> + if (ver == HFI_VERSION_4XX)
> + return 0;
> +
> + return req->hold_count;
> +};
> +
> +static inline u32 hfi_bufreq_get_count_min(struct hfi_buffer_requirements *req,
> + u32 ver)
> +{
> + if (ver == HFI_VERSION_4XX)
> + return req->hold_count;
> +
> + return req->count_min;
> +};
> +
> +static inline u32 hfi_bufreq_get_count_min_host(struct hfi_buffer_requirements *req,
> + u32 ver)
> +{
> + if (ver == HFI_VERSION_4XX)
> + return req->count_min;
> +
> + return 0;
> +};
> +
> +static inline void hfi_bufreq_set_hold_count(struct hfi_buffer_requirements *req,
> + u32 ver, u32 val)
> +{
> + if (ver == HFI_VERSION_4XX)
> + return;
> +
> + req->hold_count = val;
> +};
> +
> +static inline void hfi_bufreq_set_count_min(struct hfi_buffer_requirements *req,
> + u32 ver, u32 val)
> +{
> + if (ver == HFI_VERSION_4XX)
> + req->hold_count = val;
> +
> + req->count_min = val;
> +};
> +
> +static inline void hfi_bufreq_set_count_min_host(struct hfi_buffer_requirements *req,
> + u32 ver, u32 val)
> +{
> + if (ver == HFI_VERSION_4XX)
> + req->count_min = val;
> +};
> +
> struct hfi_data_payload {
> u32 size;
> u8 data[1];
> diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c
> index 3d5dadfa1900..7cab685a2ec8 100644
> --- a/drivers/media/platform/qcom/venus/hfi_msgs.c
> +++ b/drivers/media/platform/qcom/venus/hfi_msgs.c
> @@ -99,7 +99,7 @@ static void event_seq_changed(struct venus_core *core, struct venus_inst *inst,
> case HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS:
> data_ptr += sizeof(u32);
> bufreq = (struct hfi_buffer_requirements *)data_ptr;
> - event.buf_count = HFI_BUFREQ_COUNT_MIN(bufreq, ver);
> + event.buf_count = hfi_bufreq_get_count_min(bufreq, ver);
> data_ptr += sizeof(*bufreq);
> break;
> case HFI_INDEX_EXTRADATA_INPUT_CROP:
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 063a8b0d357b..2a1e038f92cf 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -899,13 +899,13 @@ static int vdec_num_buffers(struct venus_inst *inst, unsigned int *in_num,
> if (ret)
> return ret;
>
> - *in_num = HFI_BUFREQ_COUNT_MIN(&bufreq, ver);
> + *in_num = hfi_bufreq_get_count_min(&bufreq, ver);
>
> ret = venus_helper_get_bufreq(inst, HFI_BUFFER_OUTPUT, &bufreq);
> if (ret)
> return ret;
>
> - *out_num = HFI_BUFREQ_COUNT_MIN(&bufreq, ver);
> + *out_num = hfi_bufreq_get_count_min(&bufreq, ver);
>
> return 0;
> }
> @@ -1019,14 +1019,14 @@ static int vdec_verify_conf(struct venus_inst *inst)
> return ret;
>
> if (inst->num_output_bufs < bufreq.count_actual ||
> - inst->num_output_bufs < HFI_BUFREQ_COUNT_MIN(&bufreq, ver))
> + inst->num_output_bufs < hfi_bufreq_get_count_min(&bufreq, ver))
> return -EINVAL;
>
> ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, &bufreq);
> if (ret)
> return ret;
>
> - if (inst->num_input_bufs < HFI_BUFREQ_COUNT_MIN(&bufreq, ver))
> + if (inst->num_input_bufs < hfi_bufreq_get_count_min(&bufreq, ver))
> return -EINVAL;
>
> return 0;
> diff --git a/drivers/media/platform/qcom/venus/vdec_ctrls.c b/drivers/media/platform/qcom/venus/vdec_ctrls.c
> index fbe12a608b21..7e0f29bf7fae 100644
> --- a/drivers/media/platform/qcom/venus/vdec_ctrls.c
> +++ b/drivers/media/platform/qcom/venus/vdec_ctrls.c
> @@ -79,7 +79,7 @@ static int vdec_op_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
> ret = venus_helper_get_bufreq(inst, HFI_BUFFER_OUTPUT, &bufreq);
> if (!ret)
> - ctrl->val = HFI_BUFREQ_COUNT_MIN(&bufreq, ver);
> + ctrl->val = hfi_bufreq_get_count_min(&bufreq, ver);
> break;
> default:
> return -EINVAL;
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index b60772cc2cdc..d2e2d3108752 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -1207,7 +1207,7 @@ static int venc_verify_conf(struct venus_inst *inst)
> return ret;
>
> if (inst->num_output_bufs < bufreq.count_actual ||
> - inst->num_output_bufs < HFI_BUFREQ_COUNT_MIN(&bufreq, ver))
> + inst->num_output_bufs < hfi_bufreq_get_count_min(&bufreq, ver))
> return -EINVAL;
>
> ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, &bufreq);
> @@ -1215,7 +1215,7 @@ static int venc_verify_conf(struct venus_inst *inst)
> return ret;
>
> if (inst->num_input_bufs < bufreq.count_actual ||
> - inst->num_input_bufs < HFI_BUFREQ_COUNT_MIN(&bufreq, ver))
> + inst->num_input_bufs < hfi_bufreq_get_count_min(&bufreq, ver))
> return -EINVAL;
>
> return 0;
> diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c
> index 7468e43800a9..d9d2a293f3ef 100644
> --- a/drivers/media/platform/qcom/venus/venc_ctrls.c
> +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c
> @@ -358,7 +358,7 @@ static int venc_op_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:
> ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, &bufreq);
> if (!ret)
> - ctrl->val = HFI_BUFREQ_COUNT_MIN(&bufreq, ver);
> + ctrl->val = hfi_bufreq_get_count_min(&bufreq, ver);
> break;
> default:
> return -EINVAL;
>

2023-06-01 09:52:30

[permalink] [raw]

Subject: Re: [PATCH v4 06/17] media: venus: hfi_venus: Sanitize venus_boot_core() per-VPU-version

On 5/30/2023 6:00 PM, Konrad Dybcio wrote:
> The current assumption of IS_V6 is overgeneralized. Adjust the logic
> to take the VPU hardware version into account.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/media/platform/qcom/venus/hfi_venus.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
> index 5506a0d196ef..9e36ef9076a0 100644
> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
> @@ -447,19 +447,20 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
> {
> struct device *dev = hdev->core->dev;
> static const unsigned int max_tries = 100;
> - u32 ctrl_status = 0, mask_val;
> + u32 ctrl_status = 0, mask_val = 0;
> unsigned int count = 0;
> void __iomem *cpu_cs_base = hdev->core->cpu_cs_base;
> void __iomem *wrapper_base = hdev->core->wrapper_base;
> int ret = 0;
>
> - if (IS_V6(hdev->core)) {
> + if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
> mask_val = readl(wrapper_base + WRAPPER_INTR_MASK);
> mask_val &= ~(WRAPPER_INTR_MASK_A2HWD_BASK_V6 |
> WRAPPER_INTR_MASK_A2HCPU_MASK);
> } else {
> mask_val = WRAPPER_INTR_MASK_A2HVCODEC_MASK;
> }
> +
> writel(mask_val, wrapper_base + WRAPPER_INTR_MASK);
> writel(1, cpu_cs_base + CPU_CS_SCIACMDARG3);
>
> @@ -479,7 +480,7 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
> if (count >= max_tries)
> ret = -ETIMEDOUT;
>
> - if (IS_V6(hdev->core)) {
> + if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
> writel(0x1, cpu_cs_base + CPU_CS_H2XSOFTINTEN_V6);
> writel(0x0, cpu_cs_base + CPU_CS_X2RPMH_V6);
> }
>
Reviewed-by: Dikshita Agarwal <[email protected]>

2023-06-01 09:53:14

[permalink] [raw]

Subject: Re: [PATCH v4 16/17] media: venus: Use newly-introduced hfi_buffer_requirements accessors

On 5/30/2023 6:00 PM, Konrad Dybcio wrote:
> Now that we have a way which is independent of the HFI version to set
> the correct fields in hfi_buffer_requirements, use it!
>
> Signed-off-by: Konrad Dybcio <[email protected]>

Reviewed-by: Dikshita Agarwal <[email protected]>

> ---
> drivers/media/platform/qcom/venus/helpers.c | 5 +++--
> .../media/platform/qcom/venus/hfi_plat_bufs_v6.c | 22 +++++++++++-----------
> 2 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index b70bd3dac4df..8295542e1a7c 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -668,6 +668,7 @@ int venus_helper_get_bufreq(struct venus_inst *inst, u32 type,
> struct hfi_buffer_requirements *req)
> {
> u32 ptype = HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS;
> + enum hfi_version ver = inst->core->res->hfi_version;
> union hfi_get_property hprop;
> unsigned int i;
> int ret;
> @@ -675,12 +676,12 @@ int venus_helper_get_bufreq(struct venus_inst *inst, u32 type,
> memset(req, 0, sizeof(*req));
>
> if (type == HFI_BUFFER_OUTPUT || type == HFI_BUFFER_OUTPUT2)
> - req->count_min = inst->fw_min_cnt;
> + hfi_bufreq_set_count_min(req, ver, inst->fw_min_cnt);
>
> ret = platform_get_bufreq(inst, type, req);
> if (!ret) {
> if (type == HFI_BUFFER_OUTPUT || type == HFI_BUFFER_OUTPUT2)
> - inst->fw_min_cnt = req->count_min;
> + inst->fw_min_cnt = hfi_bufreq_get_count_min(req, ver);
> return 0;
> }
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
> index e97ff8cf6d64..f5a655973c08 100644
> --- a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
> +++ b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
> @@ -1215,24 +1215,24 @@ static int bufreq_dec(struct hfi_plat_buffers_params *params, u32 buftype,
>
> out_min_count = output_buffer_count(VIDC_SESSION_TYPE_DEC, codec);
> /* Max of driver and FW count */
> - out_min_count = max(out_min_count, bufreq->count_min);
> + out_min_count = max(out_min_count, hfi_bufreq_get_count_min(bufreq, version));
>
> bufreq->type = buftype;
> bufreq->region_size = 0;
> - bufreq->count_min = 1;
> bufreq->count_actual = 1;
> - bufreq->hold_count = 1;
> + hfi_bufreq_set_count_min(bufreq, version, 1);
> + hfi_bufreq_set_hold_count(bufreq, version, 1);
> bufreq->contiguous = 1;
> bufreq->alignment = 256;
>
> if (buftype == HFI_BUFFER_INPUT) {
> - bufreq->count_min = MIN_INPUT_BUFFERS;
> + hfi_bufreq_set_count_min(bufreq, version, MIN_INPUT_BUFFERS);
> bufreq->size =
> calculate_dec_input_frame_size(width, height, codec,
> max_mbs_per_frame,
> buffer_size_limit);
> } else if (buftype == HFI_BUFFER_OUTPUT || buftype == HFI_BUFFER_OUTPUT2) {
> - bufreq->count_min = out_min_count;
> + hfi_bufreq_set_count_min(bufreq, version, out_min_count);
> bufreq->size =
> venus_helper_get_framesz_raw(params->hfi_color_fmt,
> out_width, out_height);
> @@ -1269,7 +1269,7 @@ static int bufreq_enc(struct hfi_plat_buffers_params *params, u32 buftype,
> u32 work_mode = params->enc.work_mode;
> u32 rc_type = params->enc.rc_type;
> u32 num_vpp_pipes = params->num_vpp_pipes;
> - u32 num_ref;
> + u32 num_ref, count_min;
>
> switch (codec) {
> case V4L2_PIX_FMT_H264:
> @@ -1289,21 +1289,21 @@ static int bufreq_enc(struct hfi_plat_buffers_params *params, u32 buftype,
>
> bufreq->type = buftype;
> bufreq->region_size = 0;
> - bufreq->count_min = 1;
> bufreq->count_actual = 1;
> - bufreq->hold_count = 1;
> + hfi_bufreq_set_count_min(bufreq, version, 1);
> + hfi_bufreq_set_hold_count(bufreq, version, 1);
> bufreq->contiguous = 1;
> bufreq->alignment = 256;
>
> if (buftype == HFI_BUFFER_INPUT) {
> - bufreq->count_min = MIN_INPUT_BUFFERS;
> + hfi_bufreq_set_count_min(bufreq, version, MIN_INPUT_BUFFERS);
> bufreq->size =
> venus_helper_get_framesz_raw(params->hfi_color_fmt,
> width, height);
> } else if (buftype == HFI_BUFFER_OUTPUT ||
> buftype == HFI_BUFFER_OUTPUT2) {
> - bufreq->count_min =
> - output_buffer_count(VIDC_SESSION_TYPE_ENC, codec);
> + count_min = output_buffer_count(VIDC_SESSION_TYPE_ENC, codec);
> + hfi_bufreq_set_count_min(bufreq, version, count_min);
> bufreq->size = calculate_enc_output_frame_size(width, height,
> rc_type);
> } else if (buftype == HFI_BUFFER_INTERNAL_SCRATCH(version)) {
>

2023-07-11 12:36:15