Re: [PATCH 02/74] media: qcom: camss: vfe: Separate VFE 690 from VFE 780/880

[Gjorgji Rosikopulos (Consultant)]

Hi Bryan,

On 10/5/2026 8:13 PM, Bryan O'Donoghue wrote:
> Looking at the write-master index for VFE 690 and VFE 780/880 we can see a
> discontinuity between 690 and 780/880. Extending out the functionality in
> these files will result in spaghettification of the code for no good
> purpose.
> 
> The WM index difference is indication enough that the silicon should live
> in separate files.
> 
> Disjoin now.
> 
> Signed-off-by: Bryan O'Donoghue <bod@xxxxxxxxxx>
> ---
>  drivers/media/platform/qcom/camss/Makefile        |   1 +
>  drivers/media/platform/qcom/camss/camss-vfe-690.c | 168 ++++++++++++++++++++++
>  drivers/media/platform/qcom/camss/camss-vfe-780.c |  53 ++-----
>  drivers/media/platform/qcom/camss/camss-vfe.h     |   1 +
>  drivers/media/platform/qcom/camss/camss.c         |  14 +-
>  5 files changed, 187 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile
> index e82bc8141c241..4dcb08a0e25aa 100644
> --- a/drivers/media/platform/qcom/camss/Makefile
> +++ b/drivers/media/platform/qcom/camss/Makefile
> @@ -25,6 +25,7 @@ qcom-camss-objs += \
>  		camss-vfe-340.o \
>  		camss-vfe-480.o \
>  		camss-vfe-680.o \
> +		camss-vfe-690.o \
>  		camss-vfe-780.o \
>  		camss-vfe-gen1.o \
>  		camss-vfe-vbif.o \
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-690.c b/drivers/media/platform/qcom/camss/camss-vfe-690.c
> new file mode 100644
> index 0000000000000..6f90134d5ed64
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-690.c
> @@ -0,0 +1,168 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Qualcomm MSM Camera Subsystem - VFE (Video Front End) Module 690
> + *
> + * Copyright (c) 2024 Qualcomm Technologies, Inc.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +
> +#include "camss.h"
> +#include "camss-vfe.h"
> +
> +#define BUS_REG_BASE (vfe_is_lite(vfe) ? 0x480 : 0x400)
> +
> +#define VFE_TOP_CORE_CFG (0x24)
> +#define VFE_DISABLE_DSCALING_DS4  BIT(21)
> +#define VFE_DISABLE_DSCALING_DS16 BIT(22)
> +
> +#define VFE_BUS_WM_TEST_BUS_CTRL (BUS_REG_BASE + 0xFC)
> +/*
> + * Bus client mapping:
> + *
> + * Full VFE:
> + * VFE_690: 16 = RDI0, 17 = RDI1, 18 = RDI2
> + *
> + * VFE LITE:
> + * VFE_690 : 0 = RDI0, 1 = RDI1, 2 = RDI2, 3 = RDI3, 4 = RDI4, 5 = RDI5
> + */
> +#define RDI_WM(n)	((vfe_is_lite(vfe) ? 0x0 : 0x10) + (n))
> +
> +#define VFE_BUS_WM_CGC_OVERRIDE		(BUS_REG_BASE + 0x08)
> +#define		WM_CGC_OVERRIDE_ALL		(0x7FFFFFF)
> +
> +#define VFE_BUS_WM_CFG(n)		(BUS_REG_BASE + 0x200 + (n) * 0x100)
> +#define		WM_CFG_EN			BIT(0)
> +#define		WM_VIR_FRM_EN			BIT(1)
> +#define		WM_CFG_MODE			BIT(16)
> +#define VFE_BUS_WM_IMAGE_ADDR(n)	(BUS_REG_BASE + 0x204 + (n) * 0x100)
> +#define VFE_BUS_WM_FRAME_INCR(n)	(BUS_REG_BASE + 0x208 + (n) * 0x100)
> +#define VFE_BUS_WM_IMAGE_CFG_0(n)	(BUS_REG_BASE + 0x20c + (n) * 0x100)
> +#define		WM_IMAGE_CFG_0_DEFAULT_WIDTH	(0xFFFF)
> +#define VFE_BUS_WM_IMAGE_CFG_2(n)	(BUS_REG_BASE + 0x214 + (n) * 0x100)
> +#define		WM_IMAGE_CFG_2_DEFAULT_STRIDE	(0xFFFF)
> +#define VFE_BUS_WM_PACKER_CFG(n)	(BUS_REG_BASE + 0x218 + (n) * 0x100)
> +
> +#define VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD(n)	(BUS_REG_BASE + 0x230 + (n) * 0x100)
> +#define VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN(n)	(BUS_REG_BASE + 0x234 + (n) * 0x100)
> +#define VFE_BUS_WM_FRAMEDROP_PERIOD(n)		(BUS_REG_BASE + 0x238 + (n) * 0x100)
> +#define VFE_BUS_WM_FRAMEDROP_PATTERN(n)		(BUS_REG_BASE + 0x23c + (n) * 0x100)
> +
> +#define VFE_BUS_WM_MMU_PREFETCH_CFG(n)		(BUS_REG_BASE + 0x260 + (n) * 0x100)
> +#define VFE_BUS_WM_MMU_PREFETCH_MAX_OFFSET(n)	(BUS_REG_BASE + 0x264 + (n) * 0x100)

So copy paste entire file just to get different wm indexes i dont think is good to have.
You can look in the series posted for pixel path enablement we have different
bus version implementations and different descriptors for wm indexes. I think
that fits better and it is more extensible.

Different isp versions share same modules (some of them are different) but the idea
to have sub-module abstraction is to address those issues. That was main reason
to go with new implementation.

Just speaking to technical debt new ife sub-device vs duplication of the code
for each version, is still matter of choice.

~Gjorgji