[v5] media: venus: Add QCM2290 support with AR50_LITE core

[PATCH v5 2/5] media: venus: vdec: AR50_LITE video core support

Posted by Jorge Ramirez-Ortiz 3 months, 2 weeks ago

The AR50_LITE is a streamlined variant of the AR50 video core, designed
for power and cost-efficient platforms.

It supports hardware-accelerated decoding of H.264, HEVC, and VP9
formats.

Co-developed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/venus/core.c      | 11 ++-
 drivers/media/platform/qcom/venus/core.h      | 11 ++-
 drivers/media/platform/qcom/venus/firmware.c  |  8 +-
 drivers/media/platform/qcom/venus/helpers.c   | 80 +++++++++++++++++++
 drivers/media/platform/qcom/venus/helpers.h   |  2 +
 .../media/platform/qcom/venus/hfi_helper.h    | 10 ++-
 drivers/media/platform/qcom/venus/hfi_venus.c | 14 ++--
 .../media/platform/qcom/venus/pm_helpers.c    |  1 +
 drivers/media/platform/qcom/venus/vdec.c      | 15 ++--
 9 files changed, 128 insertions(+), 24 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
index d305d74bb152..736ef53d988d 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -254,14 +254,19 @@ static int venus_enumerate_codecs(struct venus_core *core, u32 type)
 
 static void venus_assign_register_offsets(struct venus_core *core)
 {
-	if (IS_IRIS2(core) || IS_IRIS2_1(core)) {
-		core->vbif_base = core->base + VBIF_BASE;
+	if (IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
 		core->cpu_base = core->base + CPU_BASE_V6;
 		core->cpu_cs_base = core->base + CPU_CS_BASE_V6;
 		core->cpu_ic_base = core->base + CPU_IC_BASE_V6;
 		core->wrapper_base = core->base + WRAPPER_BASE_V6;
 		core->wrapper_tz_base = core->base + WRAPPER_TZ_BASE_V6;
-		core->aon_base = core->base + AON_BASE_V6;
+		if (IS_AR50_LITE(core)) {
+			core->vbif_base = NULL;
+			core->aon_base = NULL;
+		} else {
+			core->vbif_base = core->base + VBIF_BASE;
+			core->aon_base = core->base + AON_BASE_V6;
+		}
 	} else {
 		core->vbif_base = core->base + VBIF_BASE;
 		core->cpu_base = core->base + CPU_BASE;
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index b412e0c5515a..e755a28e919b 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -382,6 +382,7 @@ enum venus_inst_modes {
  * @lock:	instance lock
  * @core:	a reference to the core struct
  * @clk_data:	clock data per core ID
+ * @eosbufs:	a lit of EOS buffers
  * @dpbbufs:	a list of decoded picture buffers
  * @internalbufs:	a list of internal bufferes
  * @registeredbufs:	a list of registered capture bufferes
@@ -450,6 +451,7 @@ struct venus_inst {
 	struct mutex lock;
 	struct venus_core *core;
 	struct clock_data clk_data;
+	struct list_head eosbufs;
 	struct list_head dpbbufs;
 	struct list_head internalbufs;
 	struct list_head registeredbufs;
@@ -520,7 +522,14 @@ struct venus_inst {
 #define IS_V1(core)	((core)->res->hfi_version == HFI_VERSION_1XX)
 #define IS_V3(core)	((core)->res->hfi_version == HFI_VERSION_3XX)
 #define IS_V4(core)	((core)->res->hfi_version == HFI_VERSION_4XX)
-#define IS_V6(core)	((core)->res->hfi_version == HFI_VERSION_6XX)
+static inline bool IS_V6(struct venus_core *core)
+{
+	if (WARN_ON_ONCE(!core))
+		return false;
+
+	return core->res->hfi_version == HFI_VERSION_6XX ||
+	       core->res->hfi_version == HFI_VERSION_6XX_LITE;
+}
 
 #define IS_AR50(core)		((core)->res->vpu_version == VPU_VERSION_AR50)
 #define IS_AR50_LITE(core)	((core)->res->vpu_version == VPU_VERSION_AR50_LITE)
diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
index 66a18830e66d..f8dcef0426ac 100644
--- a/drivers/media/platform/qcom/venus/firmware.c
+++ b/drivers/media/platform/qcom/venus/firmware.c
@@ -30,7 +30,7 @@ static void venus_reset_cpu(struct venus_core *core)
 	u32 fw_size = core->fw.mapped_mem_size;
 	void __iomem *wrapper_base;
 
-	if (IS_IRIS2_1(core))
+	if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
 		wrapper_base = core->wrapper_tz_base;
 	else
 		wrapper_base = core->wrapper_base;
@@ -42,7 +42,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_IRIS2_1(core)) {
+	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
 		/* Bring XTSS out of reset */
 		writel(0, wrapper_base + WRAPPER_TZ_XTSS_SW_RESET);
 	} else {
@@ -68,7 +68,7 @@ int venus_set_hw_state(struct venus_core *core, bool resume)
 	if (resume) {
 		venus_reset_cpu(core);
 	} else {
-		if (IS_IRIS2_1(core))
+		if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
 			writel(WRAPPER_XTSS_SW_RESET_BIT,
 			       core->wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
 		else
@@ -181,7 +181,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_IRIS2_1(core)) {
+	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
 		/* Assert the reset to XTSS */
 		reg = readl(wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
 		reg |= WRAPPER_XTSS_SW_RESET_BIT;
diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 8295542e1a7c..812bec9a05be 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -230,6 +230,79 @@ int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
 }
 EXPORT_SYMBOL_GPL(venus_helper_alloc_dpb_bufs);
 
+static void free_eos_buf(struct venus_inst *inst, struct intbuf *buf)
+{
+	list_del_init(&buf->list);
+	dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da,
+		       buf->attrs);
+	kfree(buf);
+}
+
+int venus_helper_free_eos_bufs(struct venus_inst *inst)
+{
+	struct intbuf *buf, *n;
+
+	list_for_each_entry_safe(buf, n, &inst->eosbufs, list) {
+		free_eos_buf(inst, buf);
+	}
+
+	if (list_empty(&inst->eosbufs))
+		INIT_LIST_HEAD(&inst->eosbufs);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(venus_helper_free_eos_bufs);
+
+int venus_helper_alloc_eos_buf(struct venus_inst *inst,
+			       struct hfi_frame_data *data)
+{
+	struct venus_core *core = inst->core;
+	struct device *dev = core->dev;
+	struct intbuf *buf;
+	int ret = 0;
+
+	memset(data, 0, sizeof(*data));
+
+	data->buffer_type = HFI_BUFFER_INPUT;
+	data->flags = HFI_BUFFERFLAG_EOS;
+
+	if (IS_AR50_LITE(inst->core)) {
+		/* We must send valid sizes and addresses */
+		buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+		if (!buf) {
+			ret = -ENOMEM;
+			goto fail;
+		}
+
+		buf->type = HFI_BUFFER_INPUT;
+		buf->size = SZ_4K;
+		buf->attrs = DMA_ATTR_NO_KERNEL_MAPPING;
+		buf->va = dma_alloc_attrs(dev, buf->size, &buf->da, GFP_KERNEL,
+					  buf->attrs);
+		if (!buf->va) {
+			ret = -ENOMEM;
+			goto fail;
+		}
+
+		list_add_tail(&buf->list, &inst->eosbufs);
+
+		data->alloc_len = buf->size;
+		data->device_addr = buf->da;
+
+	} else if (IS_V6(inst->core) &&
+		   is_fw_rev_or_older(inst->core, 1, 0, 87)) {
+		data->device_addr = 0;
+	} else {
+		data->device_addr = 0xdeadb000;
+	}
+
+	return 0;
+fail:
+	kfree(buf);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(venus_helper_alloc_eos_buf);
+
 static int intbufs_set_buffer(struct venus_inst *inst, u32 type)
 {
 	struct venus_core *core = inst->core;
@@ -630,6 +703,13 @@ static int platform_get_bufreq(struct venus_inst *inst, u32 buftype,
 	if (!hfi_plat || !hfi_plat->bufreq)
 		return -EINVAL;
 
+	/* Firmware buffer requirements for internal buffers only */
+	if (IS_AR50_LITE(inst->core))
+		if (buftype != HFI_BUFFER_INPUT &&
+		    buftype != HFI_BUFFER_OUTPUT &&
+		    buftype != HFI_BUFFER_OUTPUT2)
+			return -EINVAL;
+
 	params.version = version;
 	params.num_vpp_pipes = inst->core->res->num_vpp_pipes;
 
diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
index 358e4f39c9c0..bf55fe3b8747 100644
--- a/drivers/media/platform/qcom/venus/helpers.h
+++ b/drivers/media/platform/qcom/venus/helpers.h
@@ -58,6 +58,8 @@ int venus_helper_get_out_fmts(struct venus_inst *inst, u32 fmt, u32 *out_fmt,
 bool venus_helper_check_format(struct venus_inst *inst, u32 v4l2_pixfmt);
 int venus_helper_alloc_dpb_bufs(struct venus_inst *inst);
 int venus_helper_free_dpb_bufs(struct venus_inst *inst);
+int venus_helper_alloc_eos_buf(struct venus_inst *inst, struct hfi_frame_data *data);
+int venus_helper_free_eos_bufs(struct venus_inst *inst);
 int venus_helper_intbufs_alloc(struct venus_inst *inst);
 int venus_helper_intbufs_free(struct venus_inst *inst);
 int venus_helper_intbufs_realloc(struct venus_inst *inst);
diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
index f44059f19505..128ddf8e3cd5 100644
--- a/drivers/media/platform/qcom/venus/hfi_helper.h
+++ b/drivers/media/platform/qcom/venus/hfi_helper.h
@@ -397,13 +397,16 @@
 #define HFI_BUFFER_INTERNAL_PERSIST_1		0x5
 #define HFI_BUFFER_INTERNAL_SCRATCH(ver)	\
 	(((ver) == HFI_VERSION_4XX ||		\
-	(ver) == HFI_VERSION_6XX) ? 0x6 : 0x1000001)
+	(ver) == HFI_VERSION_6XX || (ver) == HFI_VERSION_6XX_LITE) \
+	? 0x6 : 0x1000001)
 #define HFI_BUFFER_INTERNAL_SCRATCH_1(ver)	\
 	(((ver) == HFI_VERSION_4XX ||		\
-	(ver) == HFI_VERSION_6XX) ? 0x7 : 0x1000005)
+	(ver) == HFI_VERSION_6XX || (ver) == HFI_VERSION_6XX_LITE) \
+	? 0x7 : 0x1000005)
 #define HFI_BUFFER_INTERNAL_SCRATCH_2(ver)	\
 	(((ver) == HFI_VERSION_4XX ||		\
-	(ver) == HFI_VERSION_6XX) ? 0x8 : 0x1000006)
+	(ver) == HFI_VERSION_6XX || (ver) == HFI_VERSION_6XX_LITE) \
+	? 0x8 : 0x1000006)
 #define HFI_BUFFER_EXTRADATA_INPUT(ver)		\
 	(((ver) == HFI_VERSION_4XX) ? 0xc : 0x1000002)
 #define HFI_BUFFER_EXTRADATA_OUTPUT(ver)	\
@@ -561,6 +564,7 @@ enum hfi_version {
 	HFI_VERSION_3XX,
 	HFI_VERSION_4XX,
 	HFI_VERSION_6XX,
+	HFI_VERSION_6XX_LITE,
 };
 
 struct hfi_buffer_info {
diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
index b5f2ea879950..302776bf8fe6 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -497,7 +497,7 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
 	if (count >= max_tries)
 		ret = -ETIMEDOUT;
 
-	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
+	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core) || IS_AR50_LITE(hdev->core)) {
 		writel(0x1, cpu_cs_base + CPU_CS_H2XSOFTINTEN_V6);
 		writel(0x0, cpu_cs_base + CPU_CS_X2RPMH_V6);
 	}
@@ -565,6 +565,9 @@ static int venus_halt_axi(struct venus_hfi_device *hdev)
 	u32 mask_val;
 	int ret;
 
+	if (IS_AR50_LITE(hdev->core))
+		return 0;
+
 	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
 		writel(0x3, cpu_cs_base + CPU_CS_X2RPMH_V6);
 
@@ -1134,7 +1137,8 @@ static irqreturn_t venus_isr(struct venus_core *core)
 	wrapper_base = hdev->core->wrapper_base;
 
 	status = readl(wrapper_base + WRAPPER_INTR_STATUS);
-	if (IS_IRIS2(core) || IS_IRIS2_1(core)) {
+
+	if (IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
 		if (status & WRAPPER_INTR_STATUS_A2H_MASK ||
 		    status & WRAPPER_INTR_STATUS_A2HWD_MASK_V6 ||
 		    status & CPU_CS_SCIACMDARG0_INIT_IDLE_MSG_MASK)
@@ -1146,7 +1150,7 @@ static irqreturn_t venus_isr(struct venus_core *core)
 			hdev->irq_status = status;
 	}
 	writel(1, cpu_cs_base + CPU_CS_A2HSOFTINTCLR);
-	if (!(IS_IRIS2(core) || IS_IRIS2_1(core)))
+	if (!(IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)))
 		writel(status, wrapper_base + WRAPPER_INTR_CLEAR);
 
 	return IRQ_WAKE_THREAD;
@@ -1531,7 +1535,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_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core))
+	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core) || IS_AR50_LITE(hdev->core))
 		cpu_status = readl(wrapper_tz_base + WRAPPER_TZ_CPU_STATUS_V6);
 	else
 		cpu_status = readl(wrapper_base + WRAPPER_CPU_STATUS);
@@ -1551,7 +1555,7 @@ static bool venus_cpu_idle_and_pc_ready(struct venus_hfi_device *hdev)
 	void __iomem *cpu_cs_base = hdev->core->cpu_cs_base;
 	u32 ctrl_status, cpu_status;
 
-	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core))
+	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core) || IS_AR50_LITE(hdev->core))
 		cpu_status = readl(wrapper_tz_base + WRAPPER_TZ_CPU_STATUS_V6);
 	else
 		cpu_status = readl(wrapper_base + WRAPPER_CPU_STATUS);
diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
index 409aa9bd0b5d..5d9dfe3fd043 100644
--- a/drivers/media/platform/qcom/venus/pm_helpers.c
+++ b/drivers/media/platform/qcom/venus/pm_helpers.c
@@ -1168,6 +1168,7 @@ const struct venus_pm_ops *venus_pm_get(enum hfi_version version)
 		return &pm_ops_v3;
 	case HFI_VERSION_4XX:
 	case HFI_VERSION_6XX:
+	case HFI_VERSION_6XX_LITE:
 		return &pm_ops_v4;
 	}
 
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 99ce5fd41577..87c7901b280e 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -550,7 +550,7 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
 {
 	struct venus_inst *inst = to_inst(file);
 	struct vb2_queue *dst_vq;
-	struct hfi_frame_data fdata = {0};
+	struct hfi_frame_data fdata;
 	int ret;
 
 	ret = v4l2_m2m_ioctl_try_decoder_cmd(file, fh, cmd);
@@ -561,18 +561,15 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
 
 	if (cmd->cmd == V4L2_DEC_CMD_STOP) {
 		/*
-		 * Implement V4L2_DEC_CMD_STOP by enqueue an empty buffer on
+		 * Implement V4L2_DEC_CMD_STOP by enqueue a buffer on
 		 * decoder input to signal EOS.
 		 */
 		if (!(inst->streamon_out && inst->streamon_cap))
 			goto unlock;
 
-		fdata.buffer_type = HFI_BUFFER_INPUT;
-		fdata.flags |= HFI_BUFFERFLAG_EOS;
-		if (IS_V6(inst->core) && is_fw_rev_or_older(inst->core, 1, 0, 87))
-			fdata.device_addr = 0;
-		else
-			fdata.device_addr = 0xdeadb000;
+		ret = venus_helper_alloc_eos_buf(inst, &fdata);
+		if (ret)
+			goto unlock;
 
 		ret = hfi_session_process_buf(inst, &fdata);
 
@@ -1332,6 +1329,7 @@ static void vdec_session_release(struct venus_inst *inst)
 		hfi_session_abort(inst);
 
 	venus_helper_free_dpb_bufs(inst);
+	venus_helper_free_eos_bufs(inst);
 	venus_pm_load_scale(inst);
 	INIT_LIST_HEAD(&inst->registeredbufs);
 	mutex_unlock(&inst->lock);
@@ -1682,6 +1680,7 @@ static int vdec_open(struct file *file)
 	if (!inst)
 		return -ENOMEM;
 
+	INIT_LIST_HEAD(&inst->eosbufs);
 	INIT_LIST_HEAD(&inst->dpbbufs);
 	INIT_LIST_HEAD(&inst->registeredbufs);
 	INIT_LIST_HEAD(&inst->internalbufs);
-- 
2.34.1

Re: [PATCH v5 2/5] media: venus: vdec: AR50_LITE video core support

Posted by Dikshita Agarwal 3 months, 1 week ago


On 6/26/2025 7:29 PM, Jorge Ramirez-Ortiz wrote:
> The AR50_LITE is a streamlined variant of the AR50 video core, designed
> for power and cost-efficient platforms.
> 
> It supports hardware-accelerated decoding of H.264, HEVC, and VP9
> formats.
> 
> Co-developed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/core.c      | 11 ++-
>  drivers/media/platform/qcom/venus/core.h      | 11 ++-
>  drivers/media/platform/qcom/venus/firmware.c  |  8 +-
>  drivers/media/platform/qcom/venus/helpers.c   | 80 +++++++++++++++++++
>  drivers/media/platform/qcom/venus/helpers.h   |  2 +
>  .../media/platform/qcom/venus/hfi_helper.h    | 10 ++-
>  drivers/media/platform/qcom/venus/hfi_venus.c | 14 ++--
>  .../media/platform/qcom/venus/pm_helpers.c    |  1 +
>  drivers/media/platform/qcom/venus/vdec.c      | 15 ++--
>  9 files changed, 128 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index d305d74bb152..736ef53d988d 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -254,14 +254,19 @@ static int venus_enumerate_codecs(struct venus_core *core, u32 type)
>  
>  static void venus_assign_register_offsets(struct venus_core *core)
>  {
> -	if (IS_IRIS2(core) || IS_IRIS2_1(core)) {
> -		core->vbif_base = core->base + VBIF_BASE;
> +	if (IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
>  		core->cpu_base = core->base + CPU_BASE_V6;
>  		core->cpu_cs_base = core->base + CPU_CS_BASE_V6;
>  		core->cpu_ic_base = core->base + CPU_IC_BASE_V6;
>  		core->wrapper_base = core->base + WRAPPER_BASE_V6;
>  		core->wrapper_tz_base = core->base + WRAPPER_TZ_BASE_V6;
> -		core->aon_base = core->base + AON_BASE_V6;
> +		if (IS_AR50_LITE(core)) {
> +			core->vbif_base = NULL;
> +			core->aon_base = NULL;
> +		} else {
> +			core->vbif_base = core->base + VBIF_BASE;
> +			core->aon_base = core->base + AON_BASE_V6;
> +		}
>  	} else {
>  		core->vbif_base = core->base + VBIF_BASE;
>  		core->cpu_base = core->base + CPU_BASE;
> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> index b412e0c5515a..e755a28e919b 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -382,6 +382,7 @@ enum venus_inst_modes {
>   * @lock:	instance lock
>   * @core:	a reference to the core struct
>   * @clk_data:	clock data per core ID
> + * @eosbufs:	a lit of EOS buffers
>   * @dpbbufs:	a list of decoded picture buffers
>   * @internalbufs:	a list of internal bufferes
>   * @registeredbufs:	a list of registered capture bufferes
> @@ -450,6 +451,7 @@ struct venus_inst {
>  	struct mutex lock;
>  	struct venus_core *core;
>  	struct clock_data clk_data;
> +	struct list_head eosbufs;
>  	struct list_head dpbbufs;
>  	struct list_head internalbufs;
>  	struct list_head registeredbufs;
> @@ -520,7 +522,14 @@ struct venus_inst {
>  #define IS_V1(core)	((core)->res->hfi_version == HFI_VERSION_1XX)
>  #define IS_V3(core)	((core)->res->hfi_version == HFI_VERSION_3XX)
>  #define IS_V4(core)	((core)->res->hfi_version == HFI_VERSION_4XX)
> -#define IS_V6(core)	((core)->res->hfi_version == HFI_VERSION_6XX)
> +static inline bool IS_V6(struct venus_core *core)
> +{
> +	if (WARN_ON_ONCE(!core))
> +		return false;
> +
> +	return core->res->hfi_version == HFI_VERSION_6XX ||
> +	       core->res->hfi_version == HFI_VERSION_6XX_LITE;
> +}
It should be HFI_VERSION_4XX_LITE for AR50_LITE. 4XX represents SC7280 and
SDM845 which are AR50.
>  
>  #define IS_AR50(core)		((core)->res->vpu_version == VPU_VERSION_AR50)
>  #define IS_AR50_LITE(core)	((core)->res->vpu_version == VPU_VERSION_AR50_LITE)
> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> index 66a18830e66d..f8dcef0426ac 100644
> --- a/drivers/media/platform/qcom/venus/firmware.c
> +++ b/drivers/media/platform/qcom/venus/firmware.c
> @@ -30,7 +30,7 @@ static void venus_reset_cpu(struct venus_core *core)
>  	u32 fw_size = core->fw.mapped_mem_size;
>  	void __iomem *wrapper_base;
>  
> -	if (IS_IRIS2_1(core))
> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
>  		wrapper_base = core->wrapper_tz_base;
>  	else
>  		wrapper_base = core->wrapper_base;
> @@ -42,7 +42,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_IRIS2_1(core)) {
> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
>  		/* Bring XTSS out of reset */
>  		writel(0, wrapper_base + WRAPPER_TZ_XTSS_SW_RESET);
>  	} else {
> @@ -68,7 +68,7 @@ int venus_set_hw_state(struct venus_core *core, bool resume)
>  	if (resume) {
>  		venus_reset_cpu(core);
>  	} else {
> -		if (IS_IRIS2_1(core))
> +		if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
>  			writel(WRAPPER_XTSS_SW_RESET_BIT,
>  			       core->wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
>  		else
> @@ -181,7 +181,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_IRIS2_1(core)) {
> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
>  		/* Assert the reset to XTSS */
>  		reg = readl(wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
No need to handle no-tz case. Pls drop.
>  		reg |= WRAPPER_XTSS_SW_RESET_BIT;
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index 8295542e1a7c..812bec9a05be 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -230,6 +230,79 @@ int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
>  }
>  EXPORT_SYMBOL_GPL(venus_helper_alloc_dpb_bufs);
>  
> +static void free_eos_buf(struct venus_inst *inst, struct intbuf *buf)
> +{
> +	list_del_init(&buf->list);
> +	dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da,
> +		       buf->attrs);
> +	kfree(buf);
> +}
> +
> +int venus_helper_free_eos_bufs(struct venus_inst *inst)
> +{
> +	struct intbuf *buf, *n;
> +
> +	list_for_each_entry_safe(buf, n, &inst->eosbufs, list) {
> +		free_eos_buf(inst, buf);
> +	}
> +
> +	if (list_empty(&inst->eosbufs))
> +		INIT_LIST_HEAD(&inst->eosbufs);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(venus_helper_free_eos_bufs);
> +
> +int venus_helper_alloc_eos_buf(struct venus_inst *inst,
> +			       struct hfi_frame_data *data)
> +{
> +	struct venus_core *core = inst->core;
> +	struct device *dev = core->dev;
> +	struct intbuf *buf;
> +	int ret = 0;
> +
> +	memset(data, 0, sizeof(*data));
> +
> +	data->buffer_type = HFI_BUFFER_INPUT;
> +	data->flags = HFI_BUFFERFLAG_EOS;
> +
> +	if (IS_AR50_LITE(inst->core)) {
> +		/* We must send valid sizes and addresses */
> +		buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> +		if (!buf) {
> +			ret = -ENOMEM;
> +			goto fail;
> +		}
> +
> +		buf->type = HFI_BUFFER_INPUT;
> +		buf->size = SZ_4K;
> +		buf->attrs = DMA_ATTR_NO_KERNEL_MAPPING;
> +		buf->va = dma_alloc_attrs(dev, buf->size, &buf->da, GFP_KERNEL,
> +					  buf->attrs);
> +		if (!buf->va) {
> +			ret = -ENOMEM;
> +			goto fail;
> +		}
> +
> +		list_add_tail(&buf->list, &inst->eosbufs);
> +
> +		data->alloc_len = buf->size;
> +		data->device_addr = buf->da;
> +
why this special handling for eos buffer is needed for AR50_LITE?
> +	} else if (IS_V6(inst->core) &&
> +		   is_fw_rev_or_older(inst->core, 1, 0, 87)) {
> +		data->device_addr = 0;
> +	} else {
> +		data->device_addr = 0xdeadb000;
> +	}
> +
> +	return 0;
> +fail:
> +	kfree(buf);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(venus_helper_alloc_eos_buf);
> +
>  static int intbufs_set_buffer(struct venus_inst *inst, u32 type)
>  {
>  	struct venus_core *core = inst->core;
> @@ -630,6 +703,13 @@ static int platform_get_bufreq(struct venus_inst *inst, u32 buftype,
>  	if (!hfi_plat || !hfi_plat->bufreq)
>  		return -EINVAL;
>  
> +	/* Firmware buffer requirements for internal buffers only */
> +	if (IS_AR50_LITE(inst->core))
> +		if (buftype != HFI_BUFFER_INPUT &&
> +		    buftype != HFI_BUFFER_OUTPUT &&
> +		    buftype != HFI_BUFFER_OUTPUT2)
> +			return -EINVAL;
> +
why this check is needed here? Buffer requirements for all buffer types
should come from firmware for all V4 based targets.
>  	params.version = version;
>  	params.num_vpp_pipes = inst->core->res->num_vpp_pipes;
>  
> diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
> index 358e4f39c9c0..bf55fe3b8747 100644
> --- a/drivers/media/platform/qcom/venus/helpers.h
> +++ b/drivers/media/platform/qcom/venus/helpers.h
> @@ -58,6 +58,8 @@ int venus_helper_get_out_fmts(struct venus_inst *inst, u32 fmt, u32 *out_fmt,
>  bool venus_helper_check_format(struct venus_inst *inst, u32 v4l2_pixfmt);
>  int venus_helper_alloc_dpb_bufs(struct venus_inst *inst);
>  int venus_helper_free_dpb_bufs(struct venus_inst *inst);
> +int venus_helper_alloc_eos_buf(struct venus_inst *inst, struct hfi_frame_data *data);
> +int venus_helper_free_eos_bufs(struct venus_inst *inst);
>  int venus_helper_intbufs_alloc(struct venus_inst *inst);
>  int venus_helper_intbufs_free(struct venus_inst *inst);
>  int venus_helper_intbufs_realloc(struct venus_inst *inst);
> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
> index f44059f19505..128ddf8e3cd5 100644
> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
> @@ -397,13 +397,16 @@
>  #define HFI_BUFFER_INTERNAL_PERSIST_1		0x5
>  #define HFI_BUFFER_INTERNAL_SCRATCH(ver)	\
>  	(((ver) == HFI_VERSION_4XX ||		\
> -	(ver) == HFI_VERSION_6XX) ? 0x6 : 0x1000001)
> +	(ver) == HFI_VERSION_6XX || (ver) == HFI_VERSION_6XX_LITE) \
> +	? 0x6 : 0x1000001)
>  #define HFI_BUFFER_INTERNAL_SCRATCH_1(ver)	\
>  	(((ver) == HFI_VERSION_4XX ||		\
> -	(ver) == HFI_VERSION_6XX) ? 0x7 : 0x1000005)
> +	(ver) == HFI_VERSION_6XX || (ver) == HFI_VERSION_6XX_LITE) \
> +	? 0x7 : 0x1000005)
>  #define HFI_BUFFER_INTERNAL_SCRATCH_2(ver)	\
>  	(((ver) == HFI_VERSION_4XX ||		\
> -	(ver) == HFI_VERSION_6XX) ? 0x8 : 0x1000006)
> +	(ver) == HFI_VERSION_6XX || (ver) == HFI_VERSION_6XX_LITE) \
> +	? 0x8 : 0x1000006)
>  #define HFI_BUFFER_EXTRADATA_INPUT(ver)		\
>  	(((ver) == HFI_VERSION_4XX) ? 0xc : 0x1000002)
>  #define HFI_BUFFER_EXTRADATA_OUTPUT(ver)	\
> @@ -561,6 +564,7 @@ enum hfi_version {
>  	HFI_VERSION_3XX,
>  	HFI_VERSION_4XX,
>  	HFI_VERSION_6XX,
> +	HFI_VERSION_6XX_LITE,
s/HFI_VERSION_6XX_LITE/HFI_VERSION_4XX_LITE.
update all applicable places.
>  };
>  
>  struct hfi_buffer_info {
> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
> index b5f2ea879950..302776bf8fe6 100644
> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
> @@ -497,7 +497,7 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
>  	if (count >= max_tries)
>  		ret = -ETIMEDOUT;
>  
> -	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
> +	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core) || IS_AR50_LITE(hdev->core)) {
>  		writel(0x1, cpu_cs_base + CPU_CS_H2XSOFTINTEN_V6);
>  		writel(0x0, cpu_cs_base + CPU_CS_X2RPMH_V6);
CPU_CS_X2RPMH_V6 is not needed for AR50_LITE, pls drop.
>  	}
> @@ -565,6 +565,9 @@ static int venus_halt_axi(struct venus_hfi_device *hdev)
>  	u32 mask_val;
>  	int ret;
>  
> +	if (IS_AR50_LITE(hdev->core))
> +		return 0;
> +
>  	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
>  		writel(0x3, cpu_cs_base + CPU_CS_X2RPMH_V6);
>  
> @@ -1134,7 +1137,8 @@ static irqreturn_t venus_isr(struct venus_core *core)
>  	wrapper_base = hdev->core->wrapper_base;
>  
>  	status = readl(wrapper_base + WRAPPER_INTR_STATUS);
> -	if (IS_IRIS2(core) || IS_IRIS2_1(core)) {
> +
> +	if (IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
>  		if (status & WRAPPER_INTR_STATUS_A2H_MASK ||
>  		    status & WRAPPER_INTR_STATUS_A2HWD_MASK_V6 ||
WD mask is incorrect, pls fix.
>  		    status & CPU_CS_SCIACMDARG0_INIT_IDLE_MSG_MASK)
> @@ -1146,7 +1150,7 @@ static irqreturn_t venus_isr(struct venus_core *core)
>  			hdev->irq_status = status;
>  	}
>  	writel(1, cpu_cs_base + CPU_CS_A2HSOFTINTCLR);
> -	if (!(IS_IRIS2(core) || IS_IRIS2_1(core)))
> +	if (!(IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)))
>  		writel(status, wrapper_base + WRAPPER_INTR_CLEAR);
>  
>  	return IRQ_WAKE_THREAD;
> @@ -1531,7 +1535,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_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core))
> +	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core) || IS_AR50_LITE(hdev->core))
>  		cpu_status = readl(wrapper_tz_base + WRAPPER_TZ_CPU_STATUS_V6);
>  	else
>  		cpu_status = readl(wrapper_base + WRAPPER_CPU_STATUS);
> @@ -1551,7 +1555,7 @@ static bool venus_cpu_idle_and_pc_ready(struct venus_hfi_device *hdev)
>  	void __iomem *cpu_cs_base = hdev->core->cpu_cs_base;
>  	u32 ctrl_status, cpu_status;
>  
> -	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core))
> +	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core) || IS_AR50_LITE(hdev->core))
>  		cpu_status = readl(wrapper_tz_base + WRAPPER_TZ_CPU_STATUS_V6);
>  	else
>  		cpu_status = readl(wrapper_base + WRAPPER_CPU_STATUS);
> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
> index 409aa9bd0b5d..5d9dfe3fd043 100644
> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
> @@ -1168,6 +1168,7 @@ const struct venus_pm_ops *venus_pm_get(enum hfi_version version)
>  		return &pm_ops_v3;
>  	case HFI_VERSION_4XX:
>  	case HFI_VERSION_6XX:
> +	case HFI_VERSION_6XX_LITE:
s/HFI_VERSION_6XX_LITE/HFI_VERSION_4XX_LITE

Thanks,
Dikshita
>  		return &pm_ops_v4;
>  	}
>  
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 99ce5fd41577..87c7901b280e 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -550,7 +550,7 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
>  {
>  	struct venus_inst *inst = to_inst(file);
>  	struct vb2_queue *dst_vq;
> -	struct hfi_frame_data fdata = {0};
> +	struct hfi_frame_data fdata;
>  	int ret;
>  
>  	ret = v4l2_m2m_ioctl_try_decoder_cmd(file, fh, cmd);
> @@ -561,18 +561,15 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
>  
>  	if (cmd->cmd == V4L2_DEC_CMD_STOP) {
>  		/*
> -		 * Implement V4L2_DEC_CMD_STOP by enqueue an empty buffer on
> +		 * Implement V4L2_DEC_CMD_STOP by enqueue a buffer on
>  		 * decoder input to signal EOS.
>  		 */
>  		if (!(inst->streamon_out && inst->streamon_cap))
>  			goto unlock;
>  
> -		fdata.buffer_type = HFI_BUFFER_INPUT;
> -		fdata.flags |= HFI_BUFFERFLAG_EOS;
> -		if (IS_V6(inst->core) && is_fw_rev_or_older(inst->core, 1, 0, 87))
> -			fdata.device_addr = 0;
> -		else
> -			fdata.device_addr = 0xdeadb000;
> +		ret = venus_helper_alloc_eos_buf(inst, &fdata);
> +		if (ret)
> +			goto unlock;
>  
>  		ret = hfi_session_process_buf(inst, &fdata);
>  
> @@ -1332,6 +1329,7 @@ static void vdec_session_release(struct venus_inst *inst)
>  		hfi_session_abort(inst);
>  
>  	venus_helper_free_dpb_bufs(inst);
> +	venus_helper_free_eos_bufs(inst);
>  	venus_pm_load_scale(inst);
>  	INIT_LIST_HEAD(&inst->registeredbufs);
>  	mutex_unlock(&inst->lock);
> @@ -1682,6 +1680,7 @@ static int vdec_open(struct file *file)
>  	if (!inst)
>  		return -ENOMEM;
>  
> +	INIT_LIST_HEAD(&inst->eosbufs);
>  	INIT_LIST_HEAD(&inst->dpbbufs);
>  	INIT_LIST_HEAD(&inst->registeredbufs);
>  	INIT_LIST_HEAD(&inst->internalbufs);

Re: [PATCH v5 2/5] media: venus: vdec: AR50_LITE video core support

Posted by Jorge Ramirez 3 months, 1 week ago

On 27/06/25 18:17:27, Dikshita Agarwal wrote:
> 
> 
> On 6/26/2025 7:29 PM, Jorge Ramirez-Ortiz wrote:
> > The AR50_LITE is a streamlined variant of the AR50 video core, designed
> > for power and cost-efficient platforms.
> > 
> > It supports hardware-accelerated decoding of H.264, HEVC, and VP9
> > formats.
> > 
> > Co-developed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > ---
> >  drivers/media/platform/qcom/venus/core.c      | 11 ++-
> >  drivers/media/platform/qcom/venus/core.h      | 11 ++-
> >  drivers/media/platform/qcom/venus/firmware.c  |  8 +-
> >  drivers/media/platform/qcom/venus/helpers.c   | 80 +++++++++++++++++++
> >  drivers/media/platform/qcom/venus/helpers.h   |  2 +
> >  .../media/platform/qcom/venus/hfi_helper.h    | 10 ++-
> >  drivers/media/platform/qcom/venus/hfi_venus.c | 14 ++--
> >  .../media/platform/qcom/venus/pm_helpers.c    |  1 +
> >  drivers/media/platform/qcom/venus/vdec.c      | 15 ++--
> >  9 files changed, 128 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> > index d305d74bb152..736ef53d988d 100644
> > --- a/drivers/media/platform/qcom/venus/core.c
> > +++ b/drivers/media/platform/qcom/venus/core.c
> > @@ -254,14 +254,19 @@ static int venus_enumerate_codecs(struct venus_core *core, u32 type)
> >  
> >  static void venus_assign_register_offsets(struct venus_core *core)
> >  {
> > -	if (IS_IRIS2(core) || IS_IRIS2_1(core)) {
> > -		core->vbif_base = core->base + VBIF_BASE;
> > +	if (IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
> >  		core->cpu_base = core->base + CPU_BASE_V6;
> >  		core->cpu_cs_base = core->base + CPU_CS_BASE_V6;
> >  		core->cpu_ic_base = core->base + CPU_IC_BASE_V6;
> >  		core->wrapper_base = core->base + WRAPPER_BASE_V6;
> >  		core->wrapper_tz_base = core->base + WRAPPER_TZ_BASE_V6;
> > -		core->aon_base = core->base + AON_BASE_V6;
> > +		if (IS_AR50_LITE(core)) {
> > +			core->vbif_base = NULL;
> > +			core->aon_base = NULL;
> > +		} else {
> > +			core->vbif_base = core->base + VBIF_BASE;
> > +			core->aon_base = core->base + AON_BASE_V6;
> > +		}
> >  	} else {
> >  		core->vbif_base = core->base + VBIF_BASE;
> >  		core->cpu_base = core->base + CPU_BASE;
> > diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> > index b412e0c5515a..e755a28e919b 100644
> > --- a/drivers/media/platform/qcom/venus/core.h
> > +++ b/drivers/media/platform/qcom/venus/core.h
> > @@ -382,6 +382,7 @@ enum venus_inst_modes {
> >   * @lock:	instance lock
> >   * @core:	a reference to the core struct
> >   * @clk_data:	clock data per core ID
> > + * @eosbufs:	a lit of EOS buffers
> >   * @dpbbufs:	a list of decoded picture buffers
> >   * @internalbufs:	a list of internal bufferes
> >   * @registeredbufs:	a list of registered capture bufferes
> > @@ -450,6 +451,7 @@ struct venus_inst {
> >  	struct mutex lock;
> >  	struct venus_core *core;
> >  	struct clock_data clk_data;
> > +	struct list_head eosbufs;
> >  	struct list_head dpbbufs;
> >  	struct list_head internalbufs;
> >  	struct list_head registeredbufs;
> > @@ -520,7 +522,14 @@ struct venus_inst {
> >  #define IS_V1(core)	((core)->res->hfi_version == HFI_VERSION_1XX)
> >  #define IS_V3(core)	((core)->res->hfi_version == HFI_VERSION_3XX)
> >  #define IS_V4(core)	((core)->res->hfi_version == HFI_VERSION_4XX)
> > -#define IS_V6(core)	((core)->res->hfi_version == HFI_VERSION_6XX)
> > +static inline bool IS_V6(struct venus_core *core)
> > +{
> > +	if (WARN_ON_ONCE(!core))
> > +		return false;
> > +
> > +	return core->res->hfi_version == HFI_VERSION_6XX ||
> > +	       core->res->hfi_version == HFI_VERSION_6XX_LITE;
> > +}
> It should be HFI_VERSION_4XX_LITE for AR50_LITE. 4XX represents SC7280 and
> SDM845 which are AR50.

ah good information - where is this documented? I never found this
information... I'd appreciate if you could confirm with some document
for future reference.

> >  
> >  #define IS_AR50(core)		((core)->res->vpu_version == VPU_VERSION_AR50)
> >  #define IS_AR50_LITE(core)	((core)->res->vpu_version == VPU_VERSION_AR50_LITE)
> > diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> > index 66a18830e66d..f8dcef0426ac 100644
> > --- a/drivers/media/platform/qcom/venus/firmware.c
> > +++ b/drivers/media/platform/qcom/venus/firmware.c
> > @@ -30,7 +30,7 @@ static void venus_reset_cpu(struct venus_core *core)
> >  	u32 fw_size = core->fw.mapped_mem_size;
> >  	void __iomem *wrapper_base;
> >  
> > -	if (IS_IRIS2_1(core))
> > +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
> >  		wrapper_base = core->wrapper_tz_base;
> >  	else
> >  		wrapper_base = core->wrapper_base;
> > @@ -42,7 +42,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_IRIS2_1(core)) {
> > +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
> >  		/* Bring XTSS out of reset */
> >  		writel(0, wrapper_base + WRAPPER_TZ_XTSS_SW_RESET);
> >  	} else {
> > @@ -68,7 +68,7 @@ int venus_set_hw_state(struct venus_core *core, bool resume)
> >  	if (resume) {
> >  		venus_reset_cpu(core);
> >  	} else {
> > -		if (IS_IRIS2_1(core))
> > +		if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
> >  			writel(WRAPPER_XTSS_SW_RESET_BIT,
> >  			       core->wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
> >  		else
> > @@ -181,7 +181,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_IRIS2_1(core)) {
> > +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
> >  		/* Assert the reset to XTSS */
> >  		reg = readl(wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
> No need to handle no-tz case. Pls drop.

ok

> >  		reg |= WRAPPER_XTSS_SW_RESET_BIT;
> > diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> > index 8295542e1a7c..812bec9a05be 100644
> > --- a/drivers/media/platform/qcom/venus/helpers.c
> > +++ b/drivers/media/platform/qcom/venus/helpers.c
> > @@ -230,6 +230,79 @@ int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
> >  }
> >  EXPORT_SYMBOL_GPL(venus_helper_alloc_dpb_bufs);
> >  
> > +static void free_eos_buf(struct venus_inst *inst, struct intbuf *buf)
> > +{
> > +	list_del_init(&buf->list);
> > +	dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da,
> > +		       buf->attrs);
> > +	kfree(buf);
> > +}
> > +
> > +int venus_helper_free_eos_bufs(struct venus_inst *inst)
> > +{
> > +	struct intbuf *buf, *n;
> > +
> > +	list_for_each_entry_safe(buf, n, &inst->eosbufs, list) {
> > +		free_eos_buf(inst, buf);
> > +	}
> > +
> > +	if (list_empty(&inst->eosbufs))
> > +		INIT_LIST_HEAD(&inst->eosbufs);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(venus_helper_free_eos_bufs);
> > +
> > +int venus_helper_alloc_eos_buf(struct venus_inst *inst,
> > +			       struct hfi_frame_data *data)
> > +{
> > +	struct venus_core *core = inst->core;
> > +	struct device *dev = core->dev;
> > +	struct intbuf *buf;
> > +	int ret = 0;
> > +
> > +	memset(data, 0, sizeof(*data));
> > +
> > +	data->buffer_type = HFI_BUFFER_INPUT;
> > +	data->flags = HFI_BUFFERFLAG_EOS;
> > +
> > +	if (IS_AR50_LITE(inst->core)) {
> > +		/* We must send valid sizes and addresses */
> > +		buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> > +		if (!buf) {
> > +			ret = -ENOMEM;
> > +			goto fail;
> > +		}
> > +
> > +		buf->type = HFI_BUFFER_INPUT;
> > +		buf->size = SZ_4K;
> > +		buf->attrs = DMA_ATTR_NO_KERNEL_MAPPING;
> > +		buf->va = dma_alloc_attrs(dev, buf->size, &buf->da, GFP_KERNEL,
> > +					  buf->attrs);
> > +		if (!buf->va) {
> > +			ret = -ENOMEM;
> > +			goto fail;
> > +		}
> > +
> > +		list_add_tail(&buf->list, &inst->eosbufs);
> > +
> > +		data->alloc_len = buf->size;
> > +		data->device_addr = buf->da;
> > +
> why this special handling for eos buffer is needed for AR50_LITE?

this _fix_ was develope through testing: without it there is no EOS and
errors are reporting upon killing the player


> > +	} else if (IS_V6(inst->core) &&
> > +		   is_fw_rev_or_older(inst->core, 1, 0, 87)) {
> > +		data->device_addr = 0;
> > +	} else {
> > +		data->device_addr = 0xdeadb000;
> > +	}
> > +
> > +	return 0;
> > +fail:
> > +	kfree(buf);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(venus_helper_alloc_eos_buf);
> > +
> >  static int intbufs_set_buffer(struct venus_inst *inst, u32 type)
> >  {
> >  	struct venus_core *core = inst->core;
> > @@ -630,6 +703,13 @@ static int platform_get_bufreq(struct venus_inst *inst, u32 buftype,
> >  	if (!hfi_plat || !hfi_plat->bufreq)
> >  		return -EINVAL;
> >  
> > +	/* Firmware buffer requirements for internal buffers only */
> > +	if (IS_AR50_LITE(inst->core))
> > +		if (buftype != HFI_BUFFER_INPUT &&
> > +		    buftype != HFI_BUFFER_OUTPUT &&
> > +		    buftype != HFI_BUFFER_OUTPUT2)
> > +			return -EINVAL;
> > +
> why this check is needed here? Buffer requirements for all buffer types
> should come from firmware for all V4 based targets.

because I used V6 as you saw earlier.

> >  	params.version = version;
> >  	params.num_vpp_pipes = inst->core->res->num_vpp_pipes;
> >  
> > diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
> > index 358e4f39c9c0..bf55fe3b8747 100644
> > --- a/drivers/media/platform/qcom/venus/helpers.h
> > +++ b/drivers/media/platform/qcom/venus/helpers.h
> > @@ -58,6 +58,8 @@ int venus_helper_get_out_fmts(struct venus_inst *inst, u32 fmt, u32 *out_fmt,
> >  bool venus_helper_check_format(struct venus_inst *inst, u32 v4l2_pixfmt);
> >  int venus_helper_alloc_dpb_bufs(struct venus_inst *inst);
> >  int venus_helper_free_dpb_bufs(struct venus_inst *inst);
> > +int venus_helper_alloc_eos_buf(struct venus_inst *inst, struct hfi_frame_data *data);
> > +int venus_helper_free_eos_bufs(struct venus_inst *inst);
> >  int venus_helper_intbufs_alloc(struct venus_inst *inst);
> >  int venus_helper_intbufs_free(struct venus_inst *inst);
> >  int venus_helper_intbufs_realloc(struct venus_inst *inst);
> > diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
> > index f44059f19505..128ddf8e3cd5 100644
> > --- a/drivers/media/platform/qcom/venus/hfi_helper.h
> > +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
> > @@ -397,13 +397,16 @@
> >  #define HFI_BUFFER_INTERNAL_PERSIST_1		0x5
> >  #define HFI_BUFFER_INTERNAL_SCRATCH(ver)	\
> >  	(((ver) == HFI_VERSION_4XX ||		\
> > -	(ver) == HFI_VERSION_6XX) ? 0x6 : 0x1000001)
> > +	(ver) == HFI_VERSION_6XX || (ver) == HFI_VERSION_6XX_LITE) \
> > +	? 0x6 : 0x1000001)
> >  #define HFI_BUFFER_INTERNAL_SCRATCH_1(ver)	\
> >  	(((ver) == HFI_VERSION_4XX ||		\
> > -	(ver) == HFI_VERSION_6XX) ? 0x7 : 0x1000005)
> > +	(ver) == HFI_VERSION_6XX || (ver) == HFI_VERSION_6XX_LITE) \
> > +	? 0x7 : 0x1000005)
> >  #define HFI_BUFFER_INTERNAL_SCRATCH_2(ver)	\
> >  	(((ver) == HFI_VERSION_4XX ||		\
> > -	(ver) == HFI_VERSION_6XX) ? 0x8 : 0x1000006)
> > +	(ver) == HFI_VERSION_6XX || (ver) == HFI_VERSION_6XX_LITE) \
> > +	? 0x8 : 0x1000006)
> >  #define HFI_BUFFER_EXTRADATA_INPUT(ver)		\
> >  	(((ver) == HFI_VERSION_4XX) ? 0xc : 0x1000002)
> >  #define HFI_BUFFER_EXTRADATA_OUTPUT(ver)	\
> > @@ -561,6 +564,7 @@ enum hfi_version {
> >  	HFI_VERSION_3XX,
> >  	HFI_VERSION_4XX,
> >  	HFI_VERSION_6XX,
> > +	HFI_VERSION_6XX_LITE,
> s/HFI_VERSION_6XX_LITE/HFI_VERSION_4XX_LITE.
> update all applicable places.

sure, I'll validate that - I remember starting with V4 but it didnt
work so switched to 6xx. I'll try again.


> >  };
> >  
> >  struct hfi_buffer_info {
> > diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
> > index b5f2ea879950..302776bf8fe6 100644
> > --- a/drivers/media/platform/qcom/venus/hfi_venus.c
> > +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
> > @@ -497,7 +497,7 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
> >  	if (count >= max_tries)
> >  		ret = -ETIMEDOUT;
> >  
> > -	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
> > +	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core) || IS_AR50_LITE(hdev->core)) {
> >  		writel(0x1, cpu_cs_base + CPU_CS_H2XSOFTINTEN_V6);
> >  		writel(0x0, cpu_cs_base + CPU_CS_X2RPMH_V6);
> CPU_CS_X2RPMH_V6 is not needed for AR50_LITE, pls drop.
> >  	}
> > @@ -565,6 +565,9 @@ static int venus_halt_axi(struct venus_hfi_device *hdev)
> >  	u32 mask_val;
> >  	int ret;
> >  
> > +	if (IS_AR50_LITE(hdev->core))
> > +		return 0;
> > +
> >  	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
> >  		writel(0x3, cpu_cs_base + CPU_CS_X2RPMH_V6);
> >  
> > @@ -1134,7 +1137,8 @@ static irqreturn_t venus_isr(struct venus_core *core)
> >  	wrapper_base = hdev->core->wrapper_base;
> >  
> >  	status = readl(wrapper_base + WRAPPER_INTR_STATUS);
> > -	if (IS_IRIS2(core) || IS_IRIS2_1(core)) {
> > +
> > +	if (IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
> >  		if (status & WRAPPER_INTR_STATUS_A2H_MASK ||
> >  		    status & WRAPPER_INTR_STATUS_A2HWD_MASK_V6 ||
> WD mask is incorrect, pls fix.
> >  		    status & CPU_CS_SCIACMDARG0_INIT_IDLE_MSG_MASK)
> > @@ -1146,7 +1150,7 @@ static irqreturn_t venus_isr(struct venus_core *core)
> >  			hdev->irq_status = status;
> >  	}
> >  	writel(1, cpu_cs_base + CPU_CS_A2HSOFTINTCLR);
> > -	if (!(IS_IRIS2(core) || IS_IRIS2_1(core)))
> > +	if (!(IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)))
> >  		writel(status, wrapper_base + WRAPPER_INTR_CLEAR);
> >  
> >  	return IRQ_WAKE_THREAD;
> > @@ -1531,7 +1535,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_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core))
> > +	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core) || IS_AR50_LITE(hdev->core))
> >  		cpu_status = readl(wrapper_tz_base + WRAPPER_TZ_CPU_STATUS_V6);
> >  	else
> >  		cpu_status = readl(wrapper_base + WRAPPER_CPU_STATUS);
> > @@ -1551,7 +1555,7 @@ static bool venus_cpu_idle_and_pc_ready(struct venus_hfi_device *hdev)
> >  	void __iomem *cpu_cs_base = hdev->core->cpu_cs_base;
> >  	u32 ctrl_status, cpu_status;
> >  
> > -	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core))
> > +	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core) || IS_AR50_LITE(hdev->core))
> >  		cpu_status = readl(wrapper_tz_base + WRAPPER_TZ_CPU_STATUS_V6);
> >  	else
> >  		cpu_status = readl(wrapper_base + WRAPPER_CPU_STATUS);
> > diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
> > index 409aa9bd0b5d..5d9dfe3fd043 100644
> > --- a/drivers/media/platform/qcom/venus/pm_helpers.c
> > +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
> > @@ -1168,6 +1168,7 @@ const struct venus_pm_ops *venus_pm_get(enum hfi_version version)
> >  		return &pm_ops_v3;
> >  	case HFI_VERSION_4XX:
> >  	case HFI_VERSION_6XX:
> > +	case HFI_VERSION_6XX_LITE:
> s/HFI_VERSION_6XX_LITE/HFI_VERSION_4XX_LITE

yep

> 
> Thanks,
> Dikshita
> >  		return &pm_ops_v4;
> >  	}
> >  
> > diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> > index 99ce5fd41577..87c7901b280e 100644
> > --- a/drivers/media/platform/qcom/venus/vdec.c
> > +++ b/drivers/media/platform/qcom/venus/vdec.c
> > @@ -550,7 +550,7 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
> >  {
> >  	struct venus_inst *inst = to_inst(file);
> >  	struct vb2_queue *dst_vq;
> > -	struct hfi_frame_data fdata = {0};
> > +	struct hfi_frame_data fdata;
> >  	int ret;
> >  
> >  	ret = v4l2_m2m_ioctl_try_decoder_cmd(file, fh, cmd);
> > @@ -561,18 +561,15 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
> >  
> >  	if (cmd->cmd == V4L2_DEC_CMD_STOP) {
> >  		/*
> > -		 * Implement V4L2_DEC_CMD_STOP by enqueue an empty buffer on
> > +		 * Implement V4L2_DEC_CMD_STOP by enqueue a buffer on
> >  		 * decoder input to signal EOS.
> >  		 */
> >  		if (!(inst->streamon_out && inst->streamon_cap))
> >  			goto unlock;
> >  
> > -		fdata.buffer_type = HFI_BUFFER_INPUT;
> > -		fdata.flags |= HFI_BUFFERFLAG_EOS;
> > -		if (IS_V6(inst->core) && is_fw_rev_or_older(inst->core, 1, 0, 87))
> > -			fdata.device_addr = 0;
> > -		else
> > -			fdata.device_addr = 0xdeadb000;
> > +		ret = venus_helper_alloc_eos_buf(inst, &fdata);
> > +		if (ret)
> > +			goto unlock;
> >  
> >  		ret = hfi_session_process_buf(inst, &fdata);
> >  
> > @@ -1332,6 +1329,7 @@ static void vdec_session_release(struct venus_inst *inst)
> >  		hfi_session_abort(inst);
> >  
> >  	venus_helper_free_dpb_bufs(inst);
> > +	venus_helper_free_eos_bufs(inst);
> >  	venus_pm_load_scale(inst);
> >  	INIT_LIST_HEAD(&inst->registeredbufs);
> >  	mutex_unlock(&inst->lock);
> > @@ -1682,6 +1680,7 @@ static int vdec_open(struct file *file)
> >  	if (!inst)
> >  		return -ENOMEM;
> >  
> > +	INIT_LIST_HEAD(&inst->eosbufs);
> >  	INIT_LIST_HEAD(&inst->dpbbufs);
> >  	INIT_LIST_HEAD(&inst->registeredbufs);
> >  	INIT_LIST_HEAD(&inst->internalbufs);

Re: [PATCH v5 2/5] media: venus: vdec: AR50_LITE video core support

Posted by Dikshita Agarwal 3 months, 1 week ago


On 6/27/2025 8:48 PM, Jorge Ramirez wrote:
> On 27/06/25 18:17:27, Dikshita Agarwal wrote:
>>
>>
>> On 6/26/2025 7:29 PM, Jorge Ramirez-Ortiz wrote:
>>> The AR50_LITE is a streamlined variant of the AR50 video core, designed
>>> for power and cost-efficient platforms.
>>>
>>> It supports hardware-accelerated decoding of H.264, HEVC, and VP9
>>> formats.
>>>
>>> Co-developed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> ---
>>>  drivers/media/platform/qcom/venus/core.c      | 11 ++-
>>>  drivers/media/platform/qcom/venus/core.h      | 11 ++-
>>>  drivers/media/platform/qcom/venus/firmware.c  |  8 +-
>>>  drivers/media/platform/qcom/venus/helpers.c   | 80 +++++++++++++++++++
>>>  drivers/media/platform/qcom/venus/helpers.h   |  2 +
>>>  .../media/platform/qcom/venus/hfi_helper.h    | 10 ++-
>>>  drivers/media/platform/qcom/venus/hfi_venus.c | 14 ++--
>>>  .../media/platform/qcom/venus/pm_helpers.c    |  1 +
>>>  drivers/media/platform/qcom/venus/vdec.c      | 15 ++--
>>>  9 files changed, 128 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
>>> index d305d74bb152..736ef53d988d 100644
>>> --- a/drivers/media/platform/qcom/venus/core.c
>>> +++ b/drivers/media/platform/qcom/venus/core.c
>>> @@ -254,14 +254,19 @@ static int venus_enumerate_codecs(struct venus_core *core, u32 type)
>>>  
>>>  static void venus_assign_register_offsets(struct venus_core *core)
>>>  {
>>> -	if (IS_IRIS2(core) || IS_IRIS2_1(core)) {
>>> -		core->vbif_base = core->base + VBIF_BASE;
>>> +	if (IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
>>>  		core->cpu_base = core->base + CPU_BASE_V6;
>>>  		core->cpu_cs_base = core->base + CPU_CS_BASE_V6;
>>>  		core->cpu_ic_base = core->base + CPU_IC_BASE_V6;
>>>  		core->wrapper_base = core->base + WRAPPER_BASE_V6;
>>>  		core->wrapper_tz_base = core->base + WRAPPER_TZ_BASE_V6;
>>> -		core->aon_base = core->base + AON_BASE_V6;
>>> +		if (IS_AR50_LITE(core)) {
>>> +			core->vbif_base = NULL;
>>> +			core->aon_base = NULL;
>>> +		} else {
>>> +			core->vbif_base = core->base + VBIF_BASE;
>>> +			core->aon_base = core->base + AON_BASE_V6;
>>> +		}
>>>  	} else {
>>>  		core->vbif_base = core->base + VBIF_BASE;
>>>  		core->cpu_base = core->base + CPU_BASE;
>>> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
>>> index b412e0c5515a..e755a28e919b 100644
>>> --- a/drivers/media/platform/qcom/venus/core.h
>>> +++ b/drivers/media/platform/qcom/venus/core.h
>>> @@ -382,6 +382,7 @@ enum venus_inst_modes {
>>>   * @lock:	instance lock
>>>   * @core:	a reference to the core struct
>>>   * @clk_data:	clock data per core ID
>>> + * @eosbufs:	a lit of EOS buffers
>>>   * @dpbbufs:	a list of decoded picture buffers
>>>   * @internalbufs:	a list of internal bufferes
>>>   * @registeredbufs:	a list of registered capture bufferes
>>> @@ -450,6 +451,7 @@ struct venus_inst {
>>>  	struct mutex lock;
>>>  	struct venus_core *core;
>>>  	struct clock_data clk_data;
>>> +	struct list_head eosbufs;
>>>  	struct list_head dpbbufs;
>>>  	struct list_head internalbufs;
>>>  	struct list_head registeredbufs;
>>> @@ -520,7 +522,14 @@ struct venus_inst {
>>>  #define IS_V1(core)	((core)->res->hfi_version == HFI_VERSION_1XX)
>>>  #define IS_V3(core)	((core)->res->hfi_version == HFI_VERSION_3XX)
>>>  #define IS_V4(core)	((core)->res->hfi_version == HFI_VERSION_4XX)
>>> -#define IS_V6(core)	((core)->res->hfi_version == HFI_VERSION_6XX)
>>> +static inline bool IS_V6(struct venus_core *core)
>>> +{
>>> +	if (WARN_ON_ONCE(!core))
>>> +		return false;
>>> +
>>> +	return core->res->hfi_version == HFI_VERSION_6XX ||
>>> +	       core->res->hfi_version == HFI_VERSION_6XX_LITE;
>>> +}
>> It should be HFI_VERSION_4XX_LITE for AR50_LITE. 4XX represents SC7280 and
>> SDM845 which are AR50.
> 
> ah good information - where is this documented? I never found this
> information... I'd appreciate if you could confirm with some document
> for future reference.
> 
>>>  
>>>  #define IS_AR50(core)		((core)->res->vpu_version == VPU_VERSION_AR50)
>>>  #define IS_AR50_LITE(core)	((core)->res->vpu_version == VPU_VERSION_AR50_LITE)
>>> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
>>> index 66a18830e66d..f8dcef0426ac 100644
>>> --- a/drivers/media/platform/qcom/venus/firmware.c
>>> +++ b/drivers/media/platform/qcom/venus/firmware.c
>>> @@ -30,7 +30,7 @@ static void venus_reset_cpu(struct venus_core *core)
>>>  	u32 fw_size = core->fw.mapped_mem_size;
>>>  	void __iomem *wrapper_base;
>>>  
>>> -	if (IS_IRIS2_1(core))
>>> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
>>>  		wrapper_base = core->wrapper_tz_base;
>>>  	else
>>>  		wrapper_base = core->wrapper_base;
>>> @@ -42,7 +42,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_IRIS2_1(core)) {
>>> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
>>>  		/* Bring XTSS out of reset */
>>>  		writel(0, wrapper_base + WRAPPER_TZ_XTSS_SW_RESET);
>>>  	} else {
>>> @@ -68,7 +68,7 @@ int venus_set_hw_state(struct venus_core *core, bool resume)
>>>  	if (resume) {
>>>  		venus_reset_cpu(core);
>>>  	} else {
>>> -		if (IS_IRIS2_1(core))
>>> +		if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
>>>  			writel(WRAPPER_XTSS_SW_RESET_BIT,
>>>  			       core->wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
>>>  		else
>>> @@ -181,7 +181,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_IRIS2_1(core)) {
>>> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
>>>  		/* Assert the reset to XTSS */
>>>  		reg = readl(wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
>> No need to handle no-tz case. Pls drop.
> 
> ok
> 
>>>  		reg |= WRAPPER_XTSS_SW_RESET_BIT;
>>> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
>>> index 8295542e1a7c..812bec9a05be 100644
>>> --- a/drivers/media/platform/qcom/venus/helpers.c
>>> +++ b/drivers/media/platform/qcom/venus/helpers.c
>>> @@ -230,6 +230,79 @@ int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
>>>  }
>>>  EXPORT_SYMBOL_GPL(venus_helper_alloc_dpb_bufs);
>>>  
>>> +static void free_eos_buf(struct venus_inst *inst, struct intbuf *buf)
>>> +{
>>> +	list_del_init(&buf->list);
>>> +	dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da,
>>> +		       buf->attrs);
>>> +	kfree(buf);
>>> +}
>>> +
>>> +int venus_helper_free_eos_bufs(struct venus_inst *inst)
>>> +{
>>> +	struct intbuf *buf, *n;
>>> +
>>> +	list_for_each_entry_safe(buf, n, &inst->eosbufs, list) {
>>> +		free_eos_buf(inst, buf);
>>> +	}
>>> +
>>> +	if (list_empty(&inst->eosbufs))
>>> +		INIT_LIST_HEAD(&inst->eosbufs);
>>> +
>>> +	return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(venus_helper_free_eos_bufs);
>>> +
>>> +int venus_helper_alloc_eos_buf(struct venus_inst *inst,
>>> +			       struct hfi_frame_data *data)
>>> +{
>>> +	struct venus_core *core = inst->core;
>>> +	struct device *dev = core->dev;
>>> +	struct intbuf *buf;
>>> +	int ret = 0;
>>> +
>>> +	memset(data, 0, sizeof(*data));
>>> +
>>> +	data->buffer_type = HFI_BUFFER_INPUT;
>>> +	data->flags = HFI_BUFFERFLAG_EOS;
>>> +
>>> +	if (IS_AR50_LITE(inst->core)) {
>>> +		/* We must send valid sizes and addresses */
>>> +		buf = kzalloc(sizeof(*buf), GFP_KERNEL);
>>> +		if (!buf) {
>>> +			ret = -ENOMEM;
>>> +			goto fail;
>>> +		}
>>> +
>>> +		buf->type = HFI_BUFFER_INPUT;
>>> +		buf->size = SZ_4K;
>>> +		buf->attrs = DMA_ATTR_NO_KERNEL_MAPPING;
>>> +		buf->va = dma_alloc_attrs(dev, buf->size, &buf->da, GFP_KERNEL,
>>> +					  buf->attrs);
>>> +		if (!buf->va) {
>>> +			ret = -ENOMEM;
>>> +			goto fail;
>>> +		}
>>> +
>>> +		list_add_tail(&buf->list, &inst->eosbufs);
>>> +
>>> +		data->alloc_len = buf->size;
>>> +		data->device_addr = buf->da;
>>> +
>> why this special handling for eos buffer is needed for AR50_LITE?
> 
> this _fix_ was develope through testing: without it there is no EOS and
> errors are reporting upon killing the player
> 
Would be better to see why there is no EOS from firmware,
there shouldn't be the need to have a dma allocation for this dummy
buffers, as there is no useful info in the buffer. Having the device
address as 0 or 0xdeadb000 should be enough.

> 
>>> +	} else if (IS_V6(inst->core) &&
>>> +		   is_fw_rev_or_older(inst->core, 1, 0, 87)) {
>>> +		data->device_addr = 0;
>>> +	} else {
>>> +		data->device_addr = 0xdeadb000;
>>> +	}
>>> +
>>> +	return 0;
>>> +fail:
>>> +	kfree(buf);
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(venus_helper_alloc_eos_buf);
>>> +
>>>  static int intbufs_set_buffer(struct venus_inst *inst, u32 type)
>>>  {
>>>  	struct venus_core *core = inst->core;
>>> @@ -630,6 +703,13 @@ static int platform_get_bufreq(struct venus_inst *inst, u32 buftype,
>>>  	if (!hfi_plat || !hfi_plat->bufreq)
>>>  		return -EINVAL;
>>>  
>>> +	/* Firmware buffer requirements for internal buffers only */
>>> +	if (IS_AR50_LITE(inst->core))
>>> +		if (buftype != HFI_BUFFER_INPUT &&
>>> +		    buftype != HFI_BUFFER_OUTPUT &&
>>> +		    buftype != HFI_BUFFER_OUTPUT2)
>>> +			return -EINVAL;
>>> +
>> why this check is needed here? Buffer requirements for all buffer types
>> should come from firmware for all V4 based targets.
> 
> because I used V6 as you saw earlier.
Yes, pls use v4, and in that case all buffer requirements will come from
firmware.

Thanks,
Dikshita
> 
>>>  	params.version = version;
>>>  	params.num_vpp_pipes = inst->core->res->num_vpp_pipes;
>>>  
>>> diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
>>> index 358e4f39c9c0..bf55fe3b8747 100644
>>> --- a/drivers/media/platform/qcom/venus/helpers.h
>>> +++ b/drivers/media/platform/qcom/venus/helpers.h
>>> @@ -58,6 +58,8 @@ int venus_helper_get_out_fmts(struct venus_inst *inst, u32 fmt, u32 *out_fmt,
>>>  bool venus_helper_check_format(struct venus_inst *inst, u32 v4l2_pixfmt);
>>>  int venus_helper_alloc_dpb_bufs(struct venus_inst *inst);
>>>  int venus_helper_free_dpb_bufs(struct venus_inst *inst);
>>> +int venus_helper_alloc_eos_buf(struct venus_inst *inst, struct hfi_frame_data *data);
>>> +int venus_helper_free_eos_bufs(struct venus_inst *inst);
>>>  int venus_helper_intbufs_alloc(struct venus_inst *inst);
>>>  int venus_helper_intbufs_free(struct venus_inst *inst);
>>>  int venus_helper_intbufs_realloc(struct venus_inst *inst);
>>> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
>>> index f44059f19505..128ddf8e3cd5 100644
>>> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
>>> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
>>> @@ -397,13 +397,16 @@
>>>  #define HFI_BUFFER_INTERNAL_PERSIST_1		0x5
>>>  #define HFI_BUFFER_INTERNAL_SCRATCH(ver)	\
>>>  	(((ver) == HFI_VERSION_4XX ||		\
>>> -	(ver) == HFI_VERSION_6XX) ? 0x6 : 0x1000001)
>>> +	(ver) == HFI_VERSION_6XX || (ver) == HFI_VERSION_6XX_LITE) \
>>> +	? 0x6 : 0x1000001)
>>>  #define HFI_BUFFER_INTERNAL_SCRATCH_1(ver)	\
>>>  	(((ver) == HFI_VERSION_4XX ||		\
>>> -	(ver) == HFI_VERSION_6XX) ? 0x7 : 0x1000005)
>>> +	(ver) == HFI_VERSION_6XX || (ver) == HFI_VERSION_6XX_LITE) \
>>> +	? 0x7 : 0x1000005)
>>>  #define HFI_BUFFER_INTERNAL_SCRATCH_2(ver)	\
>>>  	(((ver) == HFI_VERSION_4XX ||		\
>>> -	(ver) == HFI_VERSION_6XX) ? 0x8 : 0x1000006)
>>> +	(ver) == HFI_VERSION_6XX || (ver) == HFI_VERSION_6XX_LITE) \
>>> +	? 0x8 : 0x1000006)
>>>  #define HFI_BUFFER_EXTRADATA_INPUT(ver)		\
>>>  	(((ver) == HFI_VERSION_4XX) ? 0xc : 0x1000002)
>>>  #define HFI_BUFFER_EXTRADATA_OUTPUT(ver)	\
>>> @@ -561,6 +564,7 @@ enum hfi_version {
>>>  	HFI_VERSION_3XX,
>>>  	HFI_VERSION_4XX,
>>>  	HFI_VERSION_6XX,
>>> +	HFI_VERSION_6XX_LITE,
>> s/HFI_VERSION_6XX_LITE/HFI_VERSION_4XX_LITE.
>> update all applicable places.
> 
> sure, I'll validate that - I remember starting with V4 but it didnt
> work so switched to 6xx. I'll try again.
> 
> 
>>>  };
>>>  
>>>  struct hfi_buffer_info {
>>> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
>>> index b5f2ea879950..302776bf8fe6 100644
>>> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
>>> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
>>> @@ -497,7 +497,7 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
>>>  	if (count >= max_tries)
>>>  		ret = -ETIMEDOUT;
>>>  
>>> -	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
>>> +	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core) || IS_AR50_LITE(hdev->core)) {
>>>  		writel(0x1, cpu_cs_base + CPU_CS_H2XSOFTINTEN_V6);
>>>  		writel(0x0, cpu_cs_base + CPU_CS_X2RPMH_V6);
>> CPU_CS_X2RPMH_V6 is not needed for AR50_LITE, pls drop.
>>>  	}
>>> @@ -565,6 +565,9 @@ static int venus_halt_axi(struct venus_hfi_device *hdev)
>>>  	u32 mask_val;
>>>  	int ret;
>>>  
>>> +	if (IS_AR50_LITE(hdev->core))
>>> +		return 0;
>>> +
>>>  	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
>>>  		writel(0x3, cpu_cs_base + CPU_CS_X2RPMH_V6);
>>>  
>>> @@ -1134,7 +1137,8 @@ static irqreturn_t venus_isr(struct venus_core *core)
>>>  	wrapper_base = hdev->core->wrapper_base;
>>>  
>>>  	status = readl(wrapper_base + WRAPPER_INTR_STATUS);
>>> -	if (IS_IRIS2(core) || IS_IRIS2_1(core)) {
>>> +
>>> +	if (IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
>>>  		if (status & WRAPPER_INTR_STATUS_A2H_MASK ||
>>>  		    status & WRAPPER_INTR_STATUS_A2HWD_MASK_V6 ||
>> WD mask is incorrect, pls fix.
>>>  		    status & CPU_CS_SCIACMDARG0_INIT_IDLE_MSG_MASK)
>>> @@ -1146,7 +1150,7 @@ static irqreturn_t venus_isr(struct venus_core *core)
>>>  			hdev->irq_status = status;
>>>  	}
>>>  	writel(1, cpu_cs_base + CPU_CS_A2HSOFTINTCLR);
>>> -	if (!(IS_IRIS2(core) || IS_IRIS2_1(core)))
>>> +	if (!(IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)))
>>>  		writel(status, wrapper_base + WRAPPER_INTR_CLEAR);
>>>  
>>>  	return IRQ_WAKE_THREAD;
>>> @@ -1531,7 +1535,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_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core))
>>> +	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core) || IS_AR50_LITE(hdev->core))
>>>  		cpu_status = readl(wrapper_tz_base + WRAPPER_TZ_CPU_STATUS_V6);
>>>  	else
>>>  		cpu_status = readl(wrapper_base + WRAPPER_CPU_STATUS);
>>> @@ -1551,7 +1555,7 @@ static bool venus_cpu_idle_and_pc_ready(struct venus_hfi_device *hdev)
>>>  	void __iomem *cpu_cs_base = hdev->core->cpu_cs_base;
>>>  	u32 ctrl_status, cpu_status;
>>>  
>>> -	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core))
>>> +	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core) || IS_AR50_LITE(hdev->core))
>>>  		cpu_status = readl(wrapper_tz_base + WRAPPER_TZ_CPU_STATUS_V6);
>>>  	else
>>>  		cpu_status = readl(wrapper_base + WRAPPER_CPU_STATUS);
>>> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
>>> index 409aa9bd0b5d..5d9dfe3fd043 100644
>>> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
>>> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
>>> @@ -1168,6 +1168,7 @@ const struct venus_pm_ops *venus_pm_get(enum hfi_version version)
>>>  		return &pm_ops_v3;
>>>  	case HFI_VERSION_4XX:
>>>  	case HFI_VERSION_6XX:
>>> +	case HFI_VERSION_6XX_LITE:
>> s/HFI_VERSION_6XX_LITE/HFI_VERSION_4XX_LITE
> 
> yep
> 
>>
>> Thanks,
>> Dikshita
>>>  		return &pm_ops_v4;
>>>  	}
>>>  
>>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>>> index 99ce5fd41577..87c7901b280e 100644
>>> --- a/drivers/media/platform/qcom/venus/vdec.c
>>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>>> @@ -550,7 +550,7 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
>>>  {
>>>  	struct venus_inst *inst = to_inst(file);
>>>  	struct vb2_queue *dst_vq;
>>> -	struct hfi_frame_data fdata = {0};
>>> +	struct hfi_frame_data fdata;
>>>  	int ret;
>>>  
>>>  	ret = v4l2_m2m_ioctl_try_decoder_cmd(file, fh, cmd);
>>> @@ -561,18 +561,15 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
>>>  
>>>  	if (cmd->cmd == V4L2_DEC_CMD_STOP) {
>>>  		/*
>>> -		 * Implement V4L2_DEC_CMD_STOP by enqueue an empty buffer on
>>> +		 * Implement V4L2_DEC_CMD_STOP by enqueue a buffer on
>>>  		 * decoder input to signal EOS.
>>>  		 */
>>>  		if (!(inst->streamon_out && inst->streamon_cap))
>>>  			goto unlock;
>>>  
>>> -		fdata.buffer_type = HFI_BUFFER_INPUT;
>>> -		fdata.flags |= HFI_BUFFERFLAG_EOS;
>>> -		if (IS_V6(inst->core) && is_fw_rev_or_older(inst->core, 1, 0, 87))
>>> -			fdata.device_addr = 0;
>>> -		else
>>> -			fdata.device_addr = 0xdeadb000;
>>> +		ret = venus_helper_alloc_eos_buf(inst, &fdata);
>>> +		if (ret)
>>> +			goto unlock;
>>>  
>>>  		ret = hfi_session_process_buf(inst, &fdata);
>>>  
>>> @@ -1332,6 +1329,7 @@ static void vdec_session_release(struct venus_inst *inst)
>>>  		hfi_session_abort(inst);
>>>  
>>>  	venus_helper_free_dpb_bufs(inst);
>>> +	venus_helper_free_eos_bufs(inst);
>>>  	venus_pm_load_scale(inst);
>>>  	INIT_LIST_HEAD(&inst->registeredbufs);
>>>  	mutex_unlock(&inst->lock);
>>> @@ -1682,6 +1680,7 @@ static int vdec_open(struct file *file)
>>>  	if (!inst)
>>>  		return -ENOMEM;
>>>  
>>> +	INIT_LIST_HEAD(&inst->eosbufs);
>>>  	INIT_LIST_HEAD(&inst->dpbbufs);
>>>  	INIT_LIST_HEAD(&inst->registeredbufs);
>>>  	INIT_LIST_HEAD(&inst->internalbufs);

Re: [PATCH v5 2/5] media: venus: vdec: AR50_LITE video core support

Posted by Jorge Ramirez 3 months ago

On 30/06/25 12:17:32, Dikshita Agarwal wrote:
> 
> 
> On 6/27/2025 8:48 PM, Jorge Ramirez wrote:
> > On 27/06/25 18:17:27, Dikshita Agarwal wrote:
> >>
> >>
> >> On 6/26/2025 7:29 PM, Jorge Ramirez-Ortiz wrote:
> >>> The AR50_LITE is a streamlined variant of the AR50 video core, designed
> >>> for power and cost-efficient platforms.
> >>>
> >>> It supports hardware-accelerated decoding of H.264, HEVC, and VP9
> >>> formats.
> >>>
> >>> Co-developed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> >>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> >>> ---
> >>>  drivers/media/platform/qcom/venus/core.c      | 11 ++-
> >>>  drivers/media/platform/qcom/venus/core.h      | 11 ++-
> >>>  drivers/media/platform/qcom/venus/firmware.c  |  8 +-
> >>>  drivers/media/platform/qcom/venus/helpers.c   | 80 +++++++++++++++++++
> >>>  drivers/media/platform/qcom/venus/helpers.h   |  2 +
> >>>  .../media/platform/qcom/venus/hfi_helper.h    | 10 ++-
> >>>  drivers/media/platform/qcom/venus/hfi_venus.c | 14 ++--
> >>>  .../media/platform/qcom/venus/pm_helpers.c    |  1 +
> >>>  drivers/media/platform/qcom/venus/vdec.c      | 15 ++--
> >>>  9 files changed, 128 insertions(+), 24 deletions(-)
> >>>
> >>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> >>> index d305d74bb152..736ef53d988d 100644
> >>> --- a/drivers/media/platform/qcom/venus/core.c
> >>> +++ b/drivers/media/platform/qcom/venus/core.c
> >>> @@ -254,14 +254,19 @@ static int venus_enumerate_codecs(struct venus_core *core, u32 type)
> >>>  
> >>>  static void venus_assign_register_offsets(struct venus_core *core)
> >>>  {
> >>> -	if (IS_IRIS2(core) || IS_IRIS2_1(core)) {
> >>> -		core->vbif_base = core->base + VBIF_BASE;
> >>> +	if (IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
> >>>  		core->cpu_base = core->base + CPU_BASE_V6;
> >>>  		core->cpu_cs_base = core->base + CPU_CS_BASE_V6;
> >>>  		core->cpu_ic_base = core->base + CPU_IC_BASE_V6;
> >>>  		core->wrapper_base = core->base + WRAPPER_BASE_V6;
> >>>  		core->wrapper_tz_base = core->base + WRAPPER_TZ_BASE_V6;
> >>> -		core->aon_base = core->base + AON_BASE_V6;
> >>> +		if (IS_AR50_LITE(core)) {
> >>> +			core->vbif_base = NULL;
> >>> +			core->aon_base = NULL;
> >>> +		} else {
> >>> +			core->vbif_base = core->base + VBIF_BASE;
> >>> +			core->aon_base = core->base + AON_BASE_V6;
> >>> +		}
> >>>  	} else {
> >>>  		core->vbif_base = core->base + VBIF_BASE;
> >>>  		core->cpu_base = core->base + CPU_BASE;
> >>> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> >>> index b412e0c5515a..e755a28e919b 100644
> >>> --- a/drivers/media/platform/qcom/venus/core.h
> >>> +++ b/drivers/media/platform/qcom/venus/core.h
> >>> @@ -382,6 +382,7 @@ enum venus_inst_modes {
> >>>   * @lock:	instance lock
> >>>   * @core:	a reference to the core struct
> >>>   * @clk_data:	clock data per core ID
> >>> + * @eosbufs:	a lit of EOS buffers
> >>>   * @dpbbufs:	a list of decoded picture buffers
> >>>   * @internalbufs:	a list of internal bufferes
> >>>   * @registeredbufs:	a list of registered capture bufferes
> >>> @@ -450,6 +451,7 @@ struct venus_inst {
> >>>  	struct mutex lock;
> >>>  	struct venus_core *core;
> >>>  	struct clock_data clk_data;
> >>> +	struct list_head eosbufs;
> >>>  	struct list_head dpbbufs;
> >>>  	struct list_head internalbufs;
> >>>  	struct list_head registeredbufs;
> >>> @@ -520,7 +522,14 @@ struct venus_inst {
> >>>  #define IS_V1(core)	((core)->res->hfi_version == HFI_VERSION_1XX)
> >>>  #define IS_V3(core)	((core)->res->hfi_version == HFI_VERSION_3XX)
> >>>  #define IS_V4(core)	((core)->res->hfi_version == HFI_VERSION_4XX)
> >>> -#define IS_V6(core)	((core)->res->hfi_version == HFI_VERSION_6XX)
> >>> +static inline bool IS_V6(struct venus_core *core)
> >>> +{
> >>> +	if (WARN_ON_ONCE(!core))
> >>> +		return false;
> >>> +
> >>> +	return core->res->hfi_version == HFI_VERSION_6XX ||
> >>> +	       core->res->hfi_version == HFI_VERSION_6XX_LITE;
> >>> +}
> >> It should be HFI_VERSION_4XX_LITE for AR50_LITE. 4XX represents SC7280 and
> >> SDM845 which are AR50.
> > 
> > ah good information - where is this documented? I never found this
> > information... I'd appreciate if you could confirm with some document
> > for future reference.
> > 
> >>>  
> >>>  #define IS_AR50(core)		((core)->res->vpu_version == VPU_VERSION_AR50)
> >>>  #define IS_AR50_LITE(core)	((core)->res->vpu_version == VPU_VERSION_AR50_LITE)
> >>> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> >>> index 66a18830e66d..f8dcef0426ac 100644
> >>> --- a/drivers/media/platform/qcom/venus/firmware.c
> >>> +++ b/drivers/media/platform/qcom/venus/firmware.c
> >>> @@ -30,7 +30,7 @@ static void venus_reset_cpu(struct venus_core *core)
> >>>  	u32 fw_size = core->fw.mapped_mem_size;
> >>>  	void __iomem *wrapper_base;
> >>>  
> >>> -	if (IS_IRIS2_1(core))
> >>> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
> >>>  		wrapper_base = core->wrapper_tz_base;
> >>>  	else
> >>>  		wrapper_base = core->wrapper_base;
> >>> @@ -42,7 +42,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_IRIS2_1(core)) {
> >>> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
> >>>  		/* Bring XTSS out of reset */
> >>>  		writel(0, wrapper_base + WRAPPER_TZ_XTSS_SW_RESET);
> >>>  	} else {
> >>> @@ -68,7 +68,7 @@ int venus_set_hw_state(struct venus_core *core, bool resume)
> >>>  	if (resume) {
> >>>  		venus_reset_cpu(core);
> >>>  	} else {
> >>> -		if (IS_IRIS2_1(core))
> >>> +		if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
> >>>  			writel(WRAPPER_XTSS_SW_RESET_BIT,
> >>>  			       core->wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
> >>>  		else
> >>> @@ -181,7 +181,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_IRIS2_1(core)) {
> >>> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
> >>>  		/* Assert the reset to XTSS */
> >>>  		reg = readl(wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
> >> No need to handle no-tz case. Pls drop.
> > 
> > ok
> > 
> >>>  		reg |= WRAPPER_XTSS_SW_RESET_BIT;
> >>> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> >>> index 8295542e1a7c..812bec9a05be 100644
> >>> --- a/drivers/media/platform/qcom/venus/helpers.c
> >>> +++ b/drivers/media/platform/qcom/venus/helpers.c
> >>> @@ -230,6 +230,79 @@ int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
> >>>  }
> >>>  EXPORT_SYMBOL_GPL(venus_helper_alloc_dpb_bufs);
> >>>  
> >>> +static void free_eos_buf(struct venus_inst *inst, struct intbuf *buf)
> >>> +{
> >>> +	list_del_init(&buf->list);
> >>> +	dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da,
> >>> +		       buf->attrs);
> >>> +	kfree(buf);
> >>> +}
> >>> +
> >>> +int venus_helper_free_eos_bufs(struct venus_inst *inst)
> >>> +{
> >>> +	struct intbuf *buf, *n;
> >>> +
> >>> +	list_for_each_entry_safe(buf, n, &inst->eosbufs, list) {
> >>> +		free_eos_buf(inst, buf);
> >>> +	}
> >>> +
> >>> +	if (list_empty(&inst->eosbufs))
> >>> +		INIT_LIST_HEAD(&inst->eosbufs);
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(venus_helper_free_eos_bufs);
> >>> +
> >>> +int venus_helper_alloc_eos_buf(struct venus_inst *inst,
> >>> +			       struct hfi_frame_data *data)
> >>> +{
> >>> +	struct venus_core *core = inst->core;
> >>> +	struct device *dev = core->dev;
> >>> +	struct intbuf *buf;
> >>> +	int ret = 0;
> >>> +
> >>> +	memset(data, 0, sizeof(*data));
> >>> +
> >>> +	data->buffer_type = HFI_BUFFER_INPUT;
> >>> +	data->flags = HFI_BUFFERFLAG_EOS;
> >>> +
> >>> +	if (IS_AR50_LITE(inst->core)) {
> >>> +		/* We must send valid sizes and addresses */
> >>> +		buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> >>> +		if (!buf) {
> >>> +			ret = -ENOMEM;
> >>> +			goto fail;
> >>> +		}
> >>> +
> >>> +		buf->type = HFI_BUFFER_INPUT;
> >>> +		buf->size = SZ_4K;
> >>> +		buf->attrs = DMA_ATTR_NO_KERNEL_MAPPING;
> >>> +		buf->va = dma_alloc_attrs(dev, buf->size, &buf->da, GFP_KERNEL,
> >>> +					  buf->attrs);
> >>> +		if (!buf->va) {
> >>> +			ret = -ENOMEM;
> >>> +			goto fail;
> >>> +		}
> >>> +
> >>> +		list_add_tail(&buf->list, &inst->eosbufs);
> >>> +
> >>> +		data->alloc_len = buf->size;
> >>> +		data->device_addr = buf->da;
> >>> +
> >> why this special handling for eos buffer is needed for AR50_LITE?
> > 
> > this _fix_ was develope through testing: without it there is no EOS and
> > errors are reporting upon killing the player
> > 
> Would be better to see why there is no EOS from firmware,
> there shouldn't be the need to have a dma allocation for this dummy
> buffers, as there is no useful info in the buffer. Having the device
> address as 0 or 0xdeadb000 should be enough.
> 

hi dikshita,

I am still keeping this on v6 as per our internal discussions and
because v6 is quite different from v5 so wanted to provide early access
to users.

if the firwmare is fixed to address this issue on time, I might revert
the EOS page buffer. 

TIA
jorge

Re: [PATCH v5 2/5] media: venus: vdec: AR50_LITE video core support

Posted by Dikshita Agarwal 3 months ago


On 7/9/2025 12:44 AM, Jorge Ramirez wrote:
> On 30/06/25 12:17:32, Dikshita Agarwal wrote:
>>
>>
>> On 6/27/2025 8:48 PM, Jorge Ramirez wrote:
>>> On 27/06/25 18:17:27, Dikshita Agarwal wrote:
>>>>
>>>>
>>>> On 6/26/2025 7:29 PM, Jorge Ramirez-Ortiz wrote:
>>>>> The AR50_LITE is a streamlined variant of the AR50 video core, designed
>>>>> for power and cost-efficient platforms.
>>>>>
>>>>> It supports hardware-accelerated decoding of H.264, HEVC, and VP9
>>>>> formats.
>>>>>
>>>>> Co-developed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
>>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>>> ---
>>>>>  drivers/media/platform/qcom/venus/core.c      | 11 ++-
>>>>>  drivers/media/platform/qcom/venus/core.h      | 11 ++-
>>>>>  drivers/media/platform/qcom/venus/firmware.c  |  8 +-
>>>>>  drivers/media/platform/qcom/venus/helpers.c   | 80 +++++++++++++++++++
>>>>>  drivers/media/platform/qcom/venus/helpers.h   |  2 +
>>>>>  .../media/platform/qcom/venus/hfi_helper.h    | 10 ++-
>>>>>  drivers/media/platform/qcom/venus/hfi_venus.c | 14 ++--
>>>>>  .../media/platform/qcom/venus/pm_helpers.c    |  1 +
>>>>>  drivers/media/platform/qcom/venus/vdec.c      | 15 ++--
>>>>>  9 files changed, 128 insertions(+), 24 deletions(-)
>>>>>
>>>>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
>>>>> index d305d74bb152..736ef53d988d 100644
>>>>> --- a/drivers/media/platform/qcom/venus/core.c
>>>>> +++ b/drivers/media/platform/qcom/venus/core.c
>>>>> @@ -254,14 +254,19 @@ static int venus_enumerate_codecs(struct venus_core *core, u32 type)
>>>>>  
>>>>>  static void venus_assign_register_offsets(struct venus_core *core)
>>>>>  {
>>>>> -	if (IS_IRIS2(core) || IS_IRIS2_1(core)) {
>>>>> -		core->vbif_base = core->base + VBIF_BASE;
>>>>> +	if (IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
>>>>>  		core->cpu_base = core->base + CPU_BASE_V6;
>>>>>  		core->cpu_cs_base = core->base + CPU_CS_BASE_V6;
>>>>>  		core->cpu_ic_base = core->base + CPU_IC_BASE_V6;
>>>>>  		core->wrapper_base = core->base + WRAPPER_BASE_V6;
>>>>>  		core->wrapper_tz_base = core->base + WRAPPER_TZ_BASE_V6;
>>>>> -		core->aon_base = core->base + AON_BASE_V6;
>>>>> +		if (IS_AR50_LITE(core)) {
>>>>> +			core->vbif_base = NULL;
>>>>> +			core->aon_base = NULL;
>>>>> +		} else {
>>>>> +			core->vbif_base = core->base + VBIF_BASE;
>>>>> +			core->aon_base = core->base + AON_BASE_V6;
>>>>> +		}
>>>>>  	} else {
>>>>>  		core->vbif_base = core->base + VBIF_BASE;
>>>>>  		core->cpu_base = core->base + CPU_BASE;
>>>>> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
>>>>> index b412e0c5515a..e755a28e919b 100644
>>>>> --- a/drivers/media/platform/qcom/venus/core.h
>>>>> +++ b/drivers/media/platform/qcom/venus/core.h
>>>>> @@ -382,6 +382,7 @@ enum venus_inst_modes {
>>>>>   * @lock:	instance lock
>>>>>   * @core:	a reference to the core struct
>>>>>   * @clk_data:	clock data per core ID
>>>>> + * @eosbufs:	a lit of EOS buffers
>>>>>   * @dpbbufs:	a list of decoded picture buffers
>>>>>   * @internalbufs:	a list of internal bufferes
>>>>>   * @registeredbufs:	a list of registered capture bufferes
>>>>> @@ -450,6 +451,7 @@ struct venus_inst {
>>>>>  	struct mutex lock;
>>>>>  	struct venus_core *core;
>>>>>  	struct clock_data clk_data;
>>>>> +	struct list_head eosbufs;
>>>>>  	struct list_head dpbbufs;
>>>>>  	struct list_head internalbufs;
>>>>>  	struct list_head registeredbufs;
>>>>> @@ -520,7 +522,14 @@ struct venus_inst {
>>>>>  #define IS_V1(core)	((core)->res->hfi_version == HFI_VERSION_1XX)
>>>>>  #define IS_V3(core)	((core)->res->hfi_version == HFI_VERSION_3XX)
>>>>>  #define IS_V4(core)	((core)->res->hfi_version == HFI_VERSION_4XX)
>>>>> -#define IS_V6(core)	((core)->res->hfi_version == HFI_VERSION_6XX)
>>>>> +static inline bool IS_V6(struct venus_core *core)
>>>>> +{
>>>>> +	if (WARN_ON_ONCE(!core))
>>>>> +		return false;
>>>>> +
>>>>> +	return core->res->hfi_version == HFI_VERSION_6XX ||
>>>>> +	       core->res->hfi_version == HFI_VERSION_6XX_LITE;
>>>>> +}
>>>> It should be HFI_VERSION_4XX_LITE for AR50_LITE. 4XX represents SC7280 and
>>>> SDM845 which are AR50.
>>>
>>> ah good information - where is this documented? I never found this
>>> information... I'd appreciate if you could confirm with some document
>>> for future reference.
>>>
>>>>>  
>>>>>  #define IS_AR50(core)		((core)->res->vpu_version == VPU_VERSION_AR50)
>>>>>  #define IS_AR50_LITE(core)	((core)->res->vpu_version == VPU_VERSION_AR50_LITE)
>>>>> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
>>>>> index 66a18830e66d..f8dcef0426ac 100644
>>>>> --- a/drivers/media/platform/qcom/venus/firmware.c
>>>>> +++ b/drivers/media/platform/qcom/venus/firmware.c
>>>>> @@ -30,7 +30,7 @@ static void venus_reset_cpu(struct venus_core *core)
>>>>>  	u32 fw_size = core->fw.mapped_mem_size;
>>>>>  	void __iomem *wrapper_base;
>>>>>  
>>>>> -	if (IS_IRIS2_1(core))
>>>>> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
>>>>>  		wrapper_base = core->wrapper_tz_base;
>>>>>  	else
>>>>>  		wrapper_base = core->wrapper_base;
>>>>> @@ -42,7 +42,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_IRIS2_1(core)) {
>>>>> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
>>>>>  		/* Bring XTSS out of reset */
>>>>>  		writel(0, wrapper_base + WRAPPER_TZ_XTSS_SW_RESET);
>>>>>  	} else {
>>>>> @@ -68,7 +68,7 @@ int venus_set_hw_state(struct venus_core *core, bool resume)
>>>>>  	if (resume) {
>>>>>  		venus_reset_cpu(core);
>>>>>  	} else {
>>>>> -		if (IS_IRIS2_1(core))
>>>>> +		if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
>>>>>  			writel(WRAPPER_XTSS_SW_RESET_BIT,
>>>>>  			       core->wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
>>>>>  		else
>>>>> @@ -181,7 +181,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_IRIS2_1(core)) {
>>>>> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
>>>>>  		/* Assert the reset to XTSS */
>>>>>  		reg = readl(wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
>>>> No need to handle no-tz case. Pls drop.
>>>
>>> ok
>>>
>>>>>  		reg |= WRAPPER_XTSS_SW_RESET_BIT;
>>>>> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
>>>>> index 8295542e1a7c..812bec9a05be 100644
>>>>> --- a/drivers/media/platform/qcom/venus/helpers.c
>>>>> +++ b/drivers/media/platform/qcom/venus/helpers.c
>>>>> @@ -230,6 +230,79 @@ int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
>>>>>  }
>>>>>  EXPORT_SYMBOL_GPL(venus_helper_alloc_dpb_bufs);
>>>>>  
>>>>> +static void free_eos_buf(struct venus_inst *inst, struct intbuf *buf)
>>>>> +{
>>>>> +	list_del_init(&buf->list);
>>>>> +	dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da,
>>>>> +		       buf->attrs);
>>>>> +	kfree(buf);
>>>>> +}
>>>>> +
>>>>> +int venus_helper_free_eos_bufs(struct venus_inst *inst)
>>>>> +{
>>>>> +	struct intbuf *buf, *n;
>>>>> +
>>>>> +	list_for_each_entry_safe(buf, n, &inst->eosbufs, list) {
>>>>> +		free_eos_buf(inst, buf);
>>>>> +	}
>>>>> +
>>>>> +	if (list_empty(&inst->eosbufs))
>>>>> +		INIT_LIST_HEAD(&inst->eosbufs);
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(venus_helper_free_eos_bufs);
>>>>> +
>>>>> +int venus_helper_alloc_eos_buf(struct venus_inst *inst,
>>>>> +			       struct hfi_frame_data *data)
>>>>> +{
>>>>> +	struct venus_core *core = inst->core;
>>>>> +	struct device *dev = core->dev;
>>>>> +	struct intbuf *buf;
>>>>> +	int ret = 0;
>>>>> +
>>>>> +	memset(data, 0, sizeof(*data));
>>>>> +
>>>>> +	data->buffer_type = HFI_BUFFER_INPUT;
>>>>> +	data->flags = HFI_BUFFERFLAG_EOS;
>>>>> +
>>>>> +	if (IS_AR50_LITE(inst->core)) {
>>>>> +		/* We must send valid sizes and addresses */
>>>>> +		buf = kzalloc(sizeof(*buf), GFP_KERNEL);
>>>>> +		if (!buf) {
>>>>> +			ret = -ENOMEM;
>>>>> +			goto fail;
>>>>> +		}
>>>>> +
>>>>> +		buf->type = HFI_BUFFER_INPUT;
>>>>> +		buf->size = SZ_4K;
>>>>> +		buf->attrs = DMA_ATTR_NO_KERNEL_MAPPING;
>>>>> +		buf->va = dma_alloc_attrs(dev, buf->size, &buf->da, GFP_KERNEL,
>>>>> +					  buf->attrs);
>>>>> +		if (!buf->va) {
>>>>> +			ret = -ENOMEM;
>>>>> +			goto fail;
>>>>> +		}
>>>>> +
>>>>> +		list_add_tail(&buf->list, &inst->eosbufs);
>>>>> +
>>>>> +		data->alloc_len = buf->size;
>>>>> +		data->device_addr = buf->da;
>>>>> +
>>>> why this special handling for eos buffer is needed for AR50_LITE?
>>>
>>> this _fix_ was develope through testing: without it there is no EOS and
>>> errors are reporting upon killing the player
>>>
>> Would be better to see why there is no EOS from firmware,
>> there shouldn't be the need to have a dma allocation for this dummy
>> buffers, as there is no useful info in the buffer. Having the device
>> address as 0 or 0xdeadb000 should be enough.
>>
> 
> hi dikshita,
> 
> I am still keeping this on v6 as per our internal discussions and
> because v6 is quite different from v5 so wanted to provide early access
> to users.
> 
> if the firwmare is fixed to address this issue on time, I might revert
> the EOS page buffer. 
> 
I'd prefer to resolve this via correct EOS handling or gain clarity on why
AR50_LITE requires special treatment, instead of proceeding with new patch
sets built around this design.

Thanks,
Dikshita
> TIA
> jorge

Re: [PATCH v5 2/5] media: venus: vdec: AR50_LITE video core support

Posted by Jorge Ramirez 2 months, 4 weeks ago

On 10/07/25 14:25:33, Dikshita Agarwal wrote:
> 
> 
> On 7/9/2025 12:44 AM, Jorge Ramirez wrote:
> > On 30/06/25 12:17:32, Dikshita Agarwal wrote:
> >>
> >>
> >> On 6/27/2025 8:48 PM, Jorge Ramirez wrote:
> >>> On 27/06/25 18:17:27, Dikshita Agarwal wrote:
> >>>>
> >>>>
> >>>> On 6/26/2025 7:29 PM, Jorge Ramirez-Ortiz wrote:
> >>>>> The AR50_LITE is a streamlined variant of the AR50 video core, designed
> >>>>> for power and cost-efficient platforms.
> >>>>>
> >>>>> It supports hardware-accelerated decoding of H.264, HEVC, and VP9
> >>>>> formats.
> >>>>>
> >>>>> Co-developed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> >>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> >>>>> ---
> >>>>>  drivers/media/platform/qcom/venus/core.c      | 11 ++-
> >>>>>  drivers/media/platform/qcom/venus/core.h      | 11 ++-
> >>>>>  drivers/media/platform/qcom/venus/firmware.c  |  8 +-
> >>>>>  drivers/media/platform/qcom/venus/helpers.c   | 80 +++++++++++++++++++
> >>>>>  drivers/media/platform/qcom/venus/helpers.h   |  2 +
> >>>>>  .../media/platform/qcom/venus/hfi_helper.h    | 10 ++-
> >>>>>  drivers/media/platform/qcom/venus/hfi_venus.c | 14 ++--
> >>>>>  .../media/platform/qcom/venus/pm_helpers.c    |  1 +
> >>>>>  drivers/media/platform/qcom/venus/vdec.c      | 15 ++--
> >>>>>  9 files changed, 128 insertions(+), 24 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> >>>>> index d305d74bb152..736ef53d988d 100644
> >>>>> --- a/drivers/media/platform/qcom/venus/core.c
> >>>>> +++ b/drivers/media/platform/qcom/venus/core.c
> >>>>> @@ -254,14 +254,19 @@ static int venus_enumerate_codecs(struct venus_core *core, u32 type)
> >>>>>  
> >>>>>  static void venus_assign_register_offsets(struct venus_core *core)
> >>>>>  {
> >>>>> -	if (IS_IRIS2(core) || IS_IRIS2_1(core)) {
> >>>>> -		core->vbif_base = core->base + VBIF_BASE;
> >>>>> +	if (IS_IRIS2(core) || IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
> >>>>>  		core->cpu_base = core->base + CPU_BASE_V6;
> >>>>>  		core->cpu_cs_base = core->base + CPU_CS_BASE_V6;
> >>>>>  		core->cpu_ic_base = core->base + CPU_IC_BASE_V6;
> >>>>>  		core->wrapper_base = core->base + WRAPPER_BASE_V6;
> >>>>>  		core->wrapper_tz_base = core->base + WRAPPER_TZ_BASE_V6;
> >>>>> -		core->aon_base = core->base + AON_BASE_V6;
> >>>>> +		if (IS_AR50_LITE(core)) {
> >>>>> +			core->vbif_base = NULL;
> >>>>> +			core->aon_base = NULL;
> >>>>> +		} else {
> >>>>> +			core->vbif_base = core->base + VBIF_BASE;
> >>>>> +			core->aon_base = core->base + AON_BASE_V6;
> >>>>> +		}
> >>>>>  	} else {
> >>>>>  		core->vbif_base = core->base + VBIF_BASE;
> >>>>>  		core->cpu_base = core->base + CPU_BASE;
> >>>>> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> >>>>> index b412e0c5515a..e755a28e919b 100644
> >>>>> --- a/drivers/media/platform/qcom/venus/core.h
> >>>>> +++ b/drivers/media/platform/qcom/venus/core.h
> >>>>> @@ -382,6 +382,7 @@ enum venus_inst_modes {
> >>>>>   * @lock:	instance lock
> >>>>>   * @core:	a reference to the core struct
> >>>>>   * @clk_data:	clock data per core ID
> >>>>> + * @eosbufs:	a lit of EOS buffers
> >>>>>   * @dpbbufs:	a list of decoded picture buffers
> >>>>>   * @internalbufs:	a list of internal bufferes
> >>>>>   * @registeredbufs:	a list of registered capture bufferes
> >>>>> @@ -450,6 +451,7 @@ struct venus_inst {
> >>>>>  	struct mutex lock;
> >>>>>  	struct venus_core *core;
> >>>>>  	struct clock_data clk_data;
> >>>>> +	struct list_head eosbufs;
> >>>>>  	struct list_head dpbbufs;
> >>>>>  	struct list_head internalbufs;
> >>>>>  	struct list_head registeredbufs;
> >>>>> @@ -520,7 +522,14 @@ struct venus_inst {
> >>>>>  #define IS_V1(core)	((core)->res->hfi_version == HFI_VERSION_1XX)
> >>>>>  #define IS_V3(core)	((core)->res->hfi_version == HFI_VERSION_3XX)
> >>>>>  #define IS_V4(core)	((core)->res->hfi_version == HFI_VERSION_4XX)
> >>>>> -#define IS_V6(core)	((core)->res->hfi_version == HFI_VERSION_6XX)
> >>>>> +static inline bool IS_V6(struct venus_core *core)
> >>>>> +{
> >>>>> +	if (WARN_ON_ONCE(!core))
> >>>>> +		return false;
> >>>>> +
> >>>>> +	return core->res->hfi_version == HFI_VERSION_6XX ||
> >>>>> +	       core->res->hfi_version == HFI_VERSION_6XX_LITE;
> >>>>> +}
> >>>> It should be HFI_VERSION_4XX_LITE for AR50_LITE. 4XX represents SC7280 and
> >>>> SDM845 which are AR50.
> >>>
> >>> ah good information - where is this documented? I never found this
> >>> information... I'd appreciate if you could confirm with some document
> >>> for future reference.
> >>>
> >>>>>  
> >>>>>  #define IS_AR50(core)		((core)->res->vpu_version == VPU_VERSION_AR50)
> >>>>>  #define IS_AR50_LITE(core)	((core)->res->vpu_version == VPU_VERSION_AR50_LITE)
> >>>>> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> >>>>> index 66a18830e66d..f8dcef0426ac 100644
> >>>>> --- a/drivers/media/platform/qcom/venus/firmware.c
> >>>>> +++ b/drivers/media/platform/qcom/venus/firmware.c
> >>>>> @@ -30,7 +30,7 @@ static void venus_reset_cpu(struct venus_core *core)
> >>>>>  	u32 fw_size = core->fw.mapped_mem_size;
> >>>>>  	void __iomem *wrapper_base;
> >>>>>  
> >>>>> -	if (IS_IRIS2_1(core))
> >>>>> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
> >>>>>  		wrapper_base = core->wrapper_tz_base;
> >>>>>  	else
> >>>>>  		wrapper_base = core->wrapper_base;
> >>>>> @@ -42,7 +42,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_IRIS2_1(core)) {
> >>>>> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
> >>>>>  		/* Bring XTSS out of reset */
> >>>>>  		writel(0, wrapper_base + WRAPPER_TZ_XTSS_SW_RESET);
> >>>>>  	} else {
> >>>>> @@ -68,7 +68,7 @@ int venus_set_hw_state(struct venus_core *core, bool resume)
> >>>>>  	if (resume) {
> >>>>>  		venus_reset_cpu(core);
> >>>>>  	} else {
> >>>>> -		if (IS_IRIS2_1(core))
> >>>>> +		if (IS_IRIS2_1(core) || IS_AR50_LITE(core))
> >>>>>  			writel(WRAPPER_XTSS_SW_RESET_BIT,
> >>>>>  			       core->wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
> >>>>>  		else
> >>>>> @@ -181,7 +181,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_IRIS2_1(core)) {
> >>>>> +	if (IS_IRIS2_1(core) || IS_AR50_LITE(core)) {
> >>>>>  		/* Assert the reset to XTSS */
> >>>>>  		reg = readl(wrapper_tz_base + WRAPPER_TZ_XTSS_SW_RESET);
> >>>> No need to handle no-tz case. Pls drop.
> >>>
> >>> ok
> >>>
> >>>>>  		reg |= WRAPPER_XTSS_SW_RESET_BIT;
> >>>>> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> >>>>> index 8295542e1a7c..812bec9a05be 100644
> >>>>> --- a/drivers/media/platform/qcom/venus/helpers.c
> >>>>> +++ b/drivers/media/platform/qcom/venus/helpers.c
> >>>>> @@ -230,6 +230,79 @@ int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
> >>>>>  }
> >>>>>  EXPORT_SYMBOL_GPL(venus_helper_alloc_dpb_bufs);
> >>>>>  
> >>>>> +static void free_eos_buf(struct venus_inst *inst, struct intbuf *buf)
> >>>>> +{
> >>>>> +	list_del_init(&buf->list);
> >>>>> +	dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da,
> >>>>> +		       buf->attrs);
> >>>>> +	kfree(buf);
> >>>>> +}
> >>>>> +
> >>>>> +int venus_helper_free_eos_bufs(struct venus_inst *inst)
> >>>>> +{
> >>>>> +	struct intbuf *buf, *n;
> >>>>> +
> >>>>> +	list_for_each_entry_safe(buf, n, &inst->eosbufs, list) {
> >>>>> +		free_eos_buf(inst, buf);
> >>>>> +	}
> >>>>> +
> >>>>> +	if (list_empty(&inst->eosbufs))
> >>>>> +		INIT_LIST_HEAD(&inst->eosbufs);
> >>>>> +
> >>>>> +	return 0;
> >>>>> +}
> >>>>> +EXPORT_SYMBOL_GPL(venus_helper_free_eos_bufs);
> >>>>> +
> >>>>> +int venus_helper_alloc_eos_buf(struct venus_inst *inst,
> >>>>> +			       struct hfi_frame_data *data)
> >>>>> +{
> >>>>> +	struct venus_core *core = inst->core;
> >>>>> +	struct device *dev = core->dev;
> >>>>> +	struct intbuf *buf;
> >>>>> +	int ret = 0;
> >>>>> +
> >>>>> +	memset(data, 0, sizeof(*data));
> >>>>> +
> >>>>> +	data->buffer_type = HFI_BUFFER_INPUT;
> >>>>> +	data->flags = HFI_BUFFERFLAG_EOS;
> >>>>> +
> >>>>> +	if (IS_AR50_LITE(inst->core)) {
> >>>>> +		/* We must send valid sizes and addresses */
> >>>>> +		buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> >>>>> +		if (!buf) {
> >>>>> +			ret = -ENOMEM;
> >>>>> +			goto fail;
> >>>>> +		}
> >>>>> +
> >>>>> +		buf->type = HFI_BUFFER_INPUT;
> >>>>> +		buf->size = SZ_4K;
> >>>>> +		buf->attrs = DMA_ATTR_NO_KERNEL_MAPPING;
> >>>>> +		buf->va = dma_alloc_attrs(dev, buf->size, &buf->da, GFP_KERNEL,
> >>>>> +					  buf->attrs);
> >>>>> +		if (!buf->va) {
> >>>>> +			ret = -ENOMEM;
> >>>>> +			goto fail;
> >>>>> +		}
> >>>>> +
> >>>>> +		list_add_tail(&buf->list, &inst->eosbufs);
> >>>>> +
> >>>>> +		data->alloc_len = buf->size;
> >>>>> +		data->device_addr = buf->da;
> >>>>> +
> >>>> why this special handling for eos buffer is needed for AR50_LITE?
> >>>
> >>> this _fix_ was develope through testing: without it there is no EOS and
> >>> errors are reporting upon killing the player
> >>>
> >> Would be better to see why there is no EOS from firmware,
> >> there shouldn't be the need to have a dma allocation for this dummy
> >> buffers, as there is no useful info in the buffer. Having the device
> >> address as 0 or 0xdeadb000 should be enough.
> >>
> > 
> > hi dikshita,
> > 
> > I am still keeping this on v6 as per our internal discussions and
> > because v6 is quite different from v5 so wanted to provide early access
> > to users.
> > 
> > if the firwmare is fixed to address this issue on time, I might revert
> > the EOS page buffer. 
> > 
> I'd prefer to resolve this via correct EOS handling or gain clarity on why
> AR50_LITE requires special treatment, instead of proceeding with new patch
> sets built around this design.
>
Fully agree.

However this patch is the actual proper implementation - it follows the
HFI spec - while the current code upstream is not.

We should revert over time the current implementation to avoid hitting
issues when the firmware stops checking for things like 0xdeadb000.

[PATCH v5 1/5] media: dt-bindings: venus: Add qcm2290 dt schema
[PATCH v5 2/5] media: venus: vdec: AR50_LITE video core support
[PATCH v5 3/5] media: venus: hfi_plat_v6_lite: Populate decode capabilities
[PATCH v5 4/5] media: venus: core: Add qcm2290 DT compatible and resource data
[PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node