mbox series

[0/5] media: venus: Add QCM2290 support with AR50_LITE core

Message ID 20250613140402.3619465-1-jorge.ramirez@oss.qualcomm.com
Headers show
Series media: venus: Add QCM2290 support with AR50_LITE core | expand

Message

Jorge Ramirez-Ortiz June 13, 2025, 2:03 p.m. UTC
This patch series adds support for the Venus video decoder/encoder block
present on the Qualcomm QCM2290.

The QCM2290 integrates an AR50_LITE core, a low-power implementation of
Venus supporting H.264, HEVC (H.265), and VP9 decoding.

The series includes:
  • DT binding schema for qcom,qcm2290-venus
  • SoC integration via qcm2290.dtsi
  • Resource table definitions and frequency scaling
  • Platform capability registration for the AR50_LITE core

Decoding was verified on the QCOM RB1 platform using GStreamer with V4L2-based
decode plugins. The following pipelines were used for playback 1280x720 and
1920x1080 H.264, HVEC and VP9 videos from https://www.elecard.com/videos.

[H.264]
gst-launch-1.0 filesrc location=videos/xxxxx.mp4 \
  ! qtdemux name=demux demux.video_0 ! queue ! h264parse ! v4l2h264dec \
  ! videoconvert ! autovideosink

[H.265]
gst-launch-1.0 filesrc location=videos/xxxxx.mp4 \
  ! qtdemux name=demux demux.video_0 ! queue ! h265parse ! v4l2h265dec \
  ! videoconvert ! autovideosink

[VP9]
gst-launch-1.0 filesrc location=videos/xxxxx.webm \
  ! matroskademus ! queue ! v4l2vp8dec \
  ! videoconvert ! autovideosink


---

Jorge Ramirez-Ortiz (5):
  dt-bindings: media: venus: Add qcm2290 dt schema
  arm64: dts: qcom: qcm2290: Add venus video node
  media: venus: vdec: AR50_LITE video core support
  media: venus: hfi_plat_v6_lite: Populate decode capabilities
  media: venus: core: Add qcm2290 DT compatible and resource data

---

Comments

Bryan O'Donoghue June 13, 2025, 2:06 p.m. UTC | #1
On 13/06/2025 15:03, Jorge Ramirez-Ortiz wrote:
> Add DT entries for the qcm2290 venus encoder/decoder.
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> ---
>   arch/arm64/boot/dts/qcom/qcm2290.dtsi | 53 +++++++++++++++++++++++++++
>   1 file changed, 53 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcm2290.dtsi b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
> index f49ac1c1f8a3..af2c1f66fe07 100644
> --- a/arch/arm64/boot/dts/qcom/qcm2290.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
> @@ -1628,6 +1628,59 @@ adreno_smmu: iommu@59a0000 {
>   			#iommu-cells = <2>;
>   		};
>   
> +		venus: video-codec@5a00000 {
> +			compatible = "qcom,qcm2290-venus";
> +			reg = <0 0x5a00000 0 0xff000>;
> +			interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			power-domains = <&gcc GCC_VENUS_GDSC>,
> +					<&gcc GCC_VCODEC0_GDSC>,
> +					<&rpmpd QCM2290_VDDCX>;
> +			power-domain-names = "venus", "vcodec0", "cx";
> +			operating-points-v2 = <&venus_opp_table>;
> +
> +			clocks = <&gcc GCC_VIDEO_VENUS_CTL_CLK>,
> +				 <&gcc GCC_VIDEO_AHB_CLK>,
> +				 <&gcc GCC_VENUS_CTL_AXI_CLK>,
> +				 <&gcc GCC_VIDEO_THROTTLE_CORE_CLK>,
> +				 <&gcc GCC_VIDEO_VCODEC0_SYS_CLK>,
> +				 <&gcc GCC_VCODEC0_AXI_CLK>;
> +			clock-names = "core", "iface", "bus", "throttle",
> +				      "vcodec0_core", "vcodec0_bus";
> +
> +			memory-region = <&pil_video_mem>;
> +			iommus = <&apps_smmu 0x860 0x0>,
> +				 <&apps_smmu 0x880 0x0>,
> +				 <&apps_smmu 0x861 0x04>,
> +				 <&apps_smmu 0x863 0x0>,
> +				 <&apps_smmu 0x804 0xE0>;
> +
> +			interconnects = <&mmnrt_virt MASTER_VIDEO_P0 0 &bimc SLAVE_EBI1 0>,
> +					<&bimc MASTER_APPSS_PROC 0 &config_noc SLAVE_VENUS_CFG 0>;
> +			interconnect-names = "video-mem", "cpu-cfg";
> +
> +			venus_opp_table: opp-table {
> +				compatible = "operating-points-v2";
> +				opp-133000000 {
> +					opp-hz = /bits/ 64 <133000000>;
> +					required-opps = <&rpmpd_opp_low_svs>;
> +				};
> +
> +				opp-240000000 {
> +					opp-hz = /bits/ 64 <240000000>;
> +					required-opps = <&rpmpd_opp_svs>;
> +				};
> +			};
> +
> +			video-decoder {
> +				compatible = "venus-decoder";
> +			};
> +
> +			video-encoder {
> +				compatible = "venus-encoder";
> +			};

These should be dropped in favour of static config in the driver.

> +		};
> +
>   		mdss: display-subsystem@5e00000 {
>   			compatible = "qcom,qcm2290-mdss";
>   			reg = <0x0 0x05e00000 0x0 0x1000>;

---
bod
Bryan O'Donoghue June 13, 2025, 2:18 p.m. UTC | #2
On 13/06/2025 15:04, 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.
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/venus/core.c      | 11 ++-
>   drivers/media/platform/qcom/venus/core.h      |  5 +-
>   drivers/media/platform/qcom/venus/firmware.c  |  8 +-
>   drivers/media/platform/qcom/venus/helpers.c   | 81 +++++++++++++++++++
>   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, 123 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)) {

Is there a property that IS_IRIS_2_1 || IS_AR50_LITE that we could use 
instead of expanding this IS_THING list ?


>   		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..122441f9600a 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,8 @@ 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)
> +#define IS_V6(core)     (((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))

For example here.

>   		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..ae89369c6a07 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -230,6 +230,80 @@ 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);

These EOS buf things look like they are not specific to ar50, could you 
add these in a preceding patch to this ?

> +
> +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);

I think the series would be nicer if the EOS buf decomposition stuff 
went into a patch that preceded this one, and then you add the 
IS_AR50_LITE() in the next patch progressively.

> +
>   static int intbufs_set_buffer(struct venus_inst *inst, u32 type)
>   {
>   	struct venus_core *core = inst->core;
> @@ -630,6 +704,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);
Otherwise LGTM.

---
bod
Jorge Ramirez-Ortiz June 15, 2025, 11:18 a.m. UTC | #3
On 13/06/25 15:06:23, Bryan O'Donoghue wrote:
> On 13/06/2025 15:03, Jorge Ramirez-Ortiz wrote:
> > Add DT entries for the qcm2290 venus encoder/decoder.
> > 
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > ---
> >   arch/arm64/boot/dts/qcom/qcm2290.dtsi | 53 +++++++++++++++++++++++++++
> >   1 file changed, 53 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/qcm2290.dtsi b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
> > index f49ac1c1f8a3..af2c1f66fe07 100644
> > --- a/arch/arm64/boot/dts/qcom/qcm2290.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
> > @@ -1628,6 +1628,59 @@ adreno_smmu: iommu@59a0000 {
> >   			#iommu-cells = <2>;
> >   		};
> > +		venus: video-codec@5a00000 {
> > +			compatible = "qcom,qcm2290-venus";
> > +			reg = <0 0x5a00000 0 0xff000>;
> > +			interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > +			power-domains = <&gcc GCC_VENUS_GDSC>,
> > +					<&gcc GCC_VCODEC0_GDSC>,
> > +					<&rpmpd QCM2290_VDDCX>;
> > +			power-domain-names = "venus", "vcodec0", "cx";
> > +			operating-points-v2 = <&venus_opp_table>;
> > +
> > +			clocks = <&gcc GCC_VIDEO_VENUS_CTL_CLK>,
> > +				 <&gcc GCC_VIDEO_AHB_CLK>,
> > +				 <&gcc GCC_VENUS_CTL_AXI_CLK>,
> > +				 <&gcc GCC_VIDEO_THROTTLE_CORE_CLK>,
> > +				 <&gcc GCC_VIDEO_VCODEC0_SYS_CLK>,
> > +				 <&gcc GCC_VCODEC0_AXI_CLK>;
> > +			clock-names = "core", "iface", "bus", "throttle",
> > +				      "vcodec0_core", "vcodec0_bus";
> > +
> > +			memory-region = <&pil_video_mem>;
> > +			iommus = <&apps_smmu 0x860 0x0>,
> > +				 <&apps_smmu 0x880 0x0>,
> > +				 <&apps_smmu 0x861 0x04>,
> > +				 <&apps_smmu 0x863 0x0>,
> > +				 <&apps_smmu 0x804 0xE0>;
> > +
> > +			interconnects = <&mmnrt_virt MASTER_VIDEO_P0 0 &bimc SLAVE_EBI1 0>,
> > +					<&bimc MASTER_APPSS_PROC 0 &config_noc SLAVE_VENUS_CFG 0>;
> > +			interconnect-names = "video-mem", "cpu-cfg";
> > +
> > +			venus_opp_table: opp-table {
> > +				compatible = "operating-points-v2";
> > +				opp-133000000 {
> > +					opp-hz = /bits/ 64 <133000000>;
> > +					required-opps = <&rpmpd_opp_low_svs>;
> > +				};
> > +
> > +				opp-240000000 {
> > +					opp-hz = /bits/ 64 <240000000>;
> > +					required-opps = <&rpmpd_opp_svs>;
> > +				};
> > +			};
> > +
> > +			video-decoder {
> > +				compatible = "venus-decoder";
> > +			};
> > +
> > +			video-encoder {
> > +				compatible = "venus-encoder";
> > +			};
> 
> These should be dropped in favour of static config in the driver.
>
OK

> > +		};
> > +
> >   		mdss: display-subsystem@5e00000 {
> >   			compatible = "qcom,qcm2290-mdss";
> >   			reg = <0x0 0x05e00000 0x0 0x1000>;
> 
> ---
> bod
Krzysztof Kozlowski June 16, 2025, 8:22 a.m. UTC | #4
On 13/06/2025 16:03, Jorge Ramirez-Ortiz wrote:
> Add DT entries for the qcm2290 venus encoder/decoder.
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/qcm2290.dtsi | 53 +++++++++++++++++++++++++++

DTS cannot be a dependency for driver. Order your patches correctly (see
dts coding style, soc maintainer rules or internal guidelines).

>  1 file changed, 53 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcm2290.dtsi b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
> index f49ac1c1f8a3..af2c1f66fe07 100644
> --- a/arch/arm64/boot/dts/qcom/qcm2290.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
> @@ -1628,6 +1628,59 @@ adreno_smmu: iommu@59a0000 {
>  			#iommu-cells = <2>;
>  		};
>  
> +		venus: video-codec@5a00000 {
> +			compatible = "qcom,qcm2290-venus";
> +			reg = <0 0x5a00000 0 0xff000>;
> +			interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			power-domains = <&gcc GCC_VENUS_GDSC>,
> +					<&gcc GCC_VCODEC0_GDSC>,
> +					<&rpmpd QCM2290_VDDCX>;
> +			power-domain-names = "venus", "vcodec0", "cx";
> +			operating-points-v2 = <&venus_opp_table>;
> +
> +			clocks = <&gcc GCC_VIDEO_VENUS_CTL_CLK>,
> +				 <&gcc GCC_VIDEO_AHB_CLK>,
> +				 <&gcc GCC_VENUS_CTL_AXI_CLK>,
> +				 <&gcc GCC_VIDEO_THROTTLE_CORE_CLK>,
> +				 <&gcc GCC_VIDEO_VCODEC0_SYS_CLK>,
> +				 <&gcc GCC_VCODEC0_AXI_CLK>;
> +			clock-names = "core", "iface", "bus", "throttle",
> +				      "vcodec0_core", "vcodec0_bus";
> +
> +			memory-region = <&pil_video_mem>;
> +			iommus = <&apps_smmu 0x860 0x0>,
> +				 <&apps_smmu 0x880 0x0>,
> +				 <&apps_smmu 0x861 0x04>,
> +				 <&apps_smmu 0x863 0x0>,
> +				 <&apps_smmu 0x804 0xE0>;
> +
> +			interconnects = <&mmnrt_virt MASTER_VIDEO_P0 0 &bimc SLAVE_EBI1 0>,
> +					<&bimc MASTER_APPSS_PROC 0 &config_noc SLAVE_VENUS_CFG 0>;
> +			interconnect-names = "video-mem", "cpu-cfg";
> +
> +			venus_opp_table: opp-table {
> +				compatible = "operating-points-v2";
> +				opp-133000000 {
> +					opp-hz = /bits/ 64 <133000000>;
> +					required-opps = <&rpmpd_opp_low_svs>;
> +				};
> +
> +				opp-240000000 {
> +					opp-hz = /bits/ 64 <240000000>;
> +					required-opps = <&rpmpd_opp_svs>;
> +				};
> +			};
> +
> +			video-decoder {
> +				compatible = "venus-decoder";

Don't add deprecated properties.

Best regards,
Krzysztof
Jorge Ramirez-Ortiz June 16, 2025, 12:56 p.m. UTC | #5
On 16/06/25 09:13:08, Bryan O'Donoghue wrote:
> On 15/06/2025 12:38, Jorge Ramirez wrote:
> > On 13/06/25 15:18:02, Bryan O'Donoghue wrote:
> > > On 13/06/2025 15:04, 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.
> > > > 
> > > > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> > > > ---
> > > >    drivers/media/platform/qcom/venus/core.c      | 11 ++-
> > > >    drivers/media/platform/qcom/venus/core.h      |  5 +-
> > > >    drivers/media/platform/qcom/venus/firmware.c  |  8 +-
> > > >    drivers/media/platform/qcom/venus/helpers.c   | 81 +++++++++++++++++++
> > > >    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, 123 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)) {
> > > 
> > > Is there a property that IS_IRIS_2_1 || IS_AR50_LITE that we could use
> > > instead of expanding this IS_THING list ?
> > 
> > I know what you mean but I think looking for possible comonalities
> > between cores will just make the partitioning more obscure.
> > 
> > we could perhaps define a different type of macro ie, something like
> > IS_CORE(core, IRIS2, IRIS2_1, AR50_LITE) ? shall I go ahead with this
> > type of macro instead?
> 
> It feels like a nit-picky point to be making but, I'm just thinking about
> adding another say two or three SoCs that need this conditional.
> 
> In that case this statement would start to look long and ungainley. It would
> be nice to come up with some common conditional for it.

what about

#define IS_VPU_MATCH(core, type)	IS_##type(core)
#define IS_VERSION_MATCH(core, type)	IS_##type(core)

#define MATCH_1(CMD, core, a) \
	IS_##CMD##_MATCH(core, a)
#define MATCH_2(CMD, core, a, b) \
	(IS_##CMD##_MATCH(core, a) || IS_##CMD##_MATCH(core, b))
#define MATCH_3(CMD, core, a, b, c) \
	(IS_##CMD##_MATCH(core, a) || IS_##CMD##_MATCH(core, b) || \
	IS_##CMD##_MATCH(core, c))
#define MATCH_4(CMD, core, a, b, c, d) \
	(IS_##CMD##_MATCH(core, a) || IS_##CMD##_MATCH(core, b) || \
	IS_##CMD##_MATCH(core, c) || IS_##CMD##_MATCH(core, d))
#define GET_MACRO(_1, _2, _3, _4, NAME, ...) NAME

#define IS_DISPATCH(CMD, core, ...) \
	GET_MACRO(__VA_ARGS__, MATCH_4, MATCH_3, MATCH_2, MATCH_1)\
	(CMD, core, __VA_ARGS__)

#define IS_VPU(core, ...)	IS_DISPATCH(VPU, core, __VA_ARGS__)
#define IS_HFI(core, ...)	IS_DISPATCH(VERSION, core, __VA_ARGS__)

then when there are three or more conditions (not much point for less
than 3 IMO)  we can just have
 if (IS_VPU(core, AR50_LITE, IRIS_2, IRIS_2_1))

would be the same for IS_HFI(core, V3, V4, V6)..

shall I post this as a separate patch and then just use it?

> 
> > > 
> > > 
> > > >    		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..122441f9600a 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,8 @@ 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)
> > > > +#define IS_V6(core)     (((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))
> > > 
> > > For example here.
> > > 
> > > >    		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..ae89369c6a07 100644
> > > > --- a/drivers/media/platform/qcom/venus/helpers.c
> > > > +++ b/drivers/media/platform/qcom/venus/helpers.c
> > > > @@ -230,6 +230,80 @@ 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);
> > > 
> > > These EOS buf things look like they are not specific to ar50, could you add
> > > these in a preceding patch to this ?
> > 
> > um, previous cores seemed to tolerate passing empty pointers to the VPU
> > firmware without issue. Unfortunately, with the AR50_LITE, this results
> > in segmentation faults in the _firmware_. The purpose of this function is
> > to add support for AR50_LITE while preserving the existing behaviour for
> > other cores...introducing a  dedicated function felt cleaner than
> > further complicating the existing conditional.
> > Given that the previous conditional was just an if/else I don’t really
> > see a lot of benefit in refactoring the earlier conditionals _before_
> > adding the AR50_LITE logic. Would you be OK if we keep it as proposed?
> 
> ok
> 
> > 
> > > 
> > > > +
> > > > +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);
> > > 
> > > I think the series would be nicer if the EOS buf decomposition stuff went
> > > into a patch that preceded this one, and then you add the IS_AR50_LITE() in
> > > the next patch progressively.
> > 
> > see above
> > 
> > > 
> > > > +
> > > >    static int intbufs_set_buffer(struct venus_inst *inst, u32 type)
> > > >    {
> > > >    	struct venus_core *core = inst->core;
> > > > @@ -630,6 +704,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);
> > > Otherwise LGTM.
> > > 
> > > ---
> > > bod
> > > 
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>