[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0b212187-168b-4757-b5a5-afe3ff0922e7@oss.qualcomm.com>
Date: Tue, 6 Jan 2026 10:17:50 -0800
From: Vijay Kumar Tumati <vijay.tumati@....qualcomm.com>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
Hangxiang Ma <hangxiang.ma@....qualcomm.com>,
Loic Poulain <loic.poulain@....qualcomm.com>,
Robert Foss
<rfoss@...nel.org>, Andi Shyti <andi.shyti@...nel.org>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Todor Tomov <todor.too@...il.com>,
Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: linux-i2c@...r.kernel.org, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org,
jeyaprakash.soundrapandian@....qualcomm.com,
Atiya Kailany <atiya.kailany@....qualcomm.com>
Subject: Re: [PATCH 6/7] media: qcom: camss: vfe: Add support for VFE gen4
On 11/27/2025 2:04 AM, Bryan O'Donoghue wrote:
> On 26/11/2025 09:38, Hangxiang Ma wrote:
>> Add support for Video Front End (VFE) that is on the SM8750 SoCs. The
>> bus_wr configuration and the registers offsets closely match with the
>> driver that had been added for Kaanapali. Hence, rename the previously
>> added driver as 'gen4' and use that for both to avoid redundancy. Handle
>> the minor differences in the driver using the chipset version.
>
> Specify you are renaming a file and dropping the 1080 postfix in its
> naming convention.
We renamed this already in Kaanapali patches. Will rebase this on that.
>
>>
>> This change limits SM8750 VFE output lines to 3 for now as constrained
>> by the CAMSS driver framework.
>
> What does that mean ?
This is coming from
enum vfe_line_id {
VFE_LINE_NONE = -1,
VFE_LINE_RDI0 = 0,
VFE_LINE_RDI1 = 1,
VFE_LINE_RDI2 = 2,
VFE_LINE_PIX = 3,
VFE_LINE_NUM_MAX = 4
};
The way the VFE driver is currently implemented, expanding the RDIX
enums might break previous generations if they were using
'VFE_LINE_PIX'. If there is a need for more than 3 RDIs, we need to
cleanly fix this, probably in a dedicated patch. Please advise. Thanks.
>
>>
>> Co-developed-by: Atiya Kailany <atiya.kailany@....qualcomm.com>
>> Signed-off-by: Atiya Kailany <atiya.kailany@....qualcomm.com>
>> Signed-off-by: Hangxiang Ma <hangxiang.ma@....qualcomm.com>
>> ---
>> drivers/media/platform/qcom/camss/Makefile | 4 +-
>> .../camss/{camss-vfe-1080.c => camss-vfe-gen4.c} | 60 +++++----
>> drivers/media/platform/qcom/camss/camss-vfe.c | 2 +
>> drivers/media/platform/qcom/camss/camss-vfe.h | 2 +-
>> drivers/media/platform/qcom/camss/camss.c | 150
>> ++++++++++++++++++++-
>> 5 files changed, 182 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/Makefile
>> b/drivers/media/platform/qcom/camss/Makefile
>> index 74e12ec65427..6e54d2d11ed3 100644
>> --- a/drivers/media/platform/qcom/camss/Makefile
>> +++ b/drivers/media/platform/qcom/camss/Makefile
>> @@ -23,9 +23,9 @@ qcom-camss-objs += \
>> camss-vfe-340.o \
>> camss-vfe-480.o \
>> camss-vfe-680.o \
>> - camss-vfe-1080.o \
>> - camss-vfe-gen3.o \
>> camss-vfe-gen1.o \
>> + camss-vfe-gen3.o \
>> + camss-vfe-gen4.o \
>> camss-vfe-vbif.o \
>> camss-vfe.o \
>> camss-video.o \
>> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-1080.c
>> b/drivers/media/platform/qcom/camss/camss-vfe-gen4.c
>> similarity index 75%
>> rename from drivers/media/platform/qcom/camss/camss-vfe-1080.c
>> rename to drivers/media/platform/qcom/camss/camss-vfe-gen4.c
>> index 9ad3dee2e80b..d0218950c05c 100644
>> --- a/drivers/media/platform/qcom/camss/camss-vfe-1080.c
>> +++ b/drivers/media/platform/qcom/camss/camss-vfe-gen4.c
>> @@ -1,8 +1,8 @@
>> // SPDX-License-Identifier: GPL-2.0
>> /*
>> - * camss-vfe-1080.c
>> + * camss-vfe-gen4.c
>> *
>> - * Qualcomm MSM Camera Subsystem - VFE (Video Front End) Module v1080
>> + * Qualcomm MSM Camera Subsystem - VFE (Video Front End) Module gen4
>> *
>> * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> */
>> @@ -13,8 +13,12 @@
>> #include "camss.h"
>> #include "camss-vfe.h"
>> -/* VFE-1080 Bus Register Base Addresses */
>> -#define BUS_REG_BASE (vfe_is_lite(vfe) ? 0x800 : 0x1000)
>> +#define IS_VFE_980(vfe) ((vfe)->camss->res->version == CAMSS_8750)
>> +
>> +#define BUS_REG_BASE_980 (vfe_is_lite(vfe) ? 0x200 : 0x800)
>> +#define BUS_REG_BASE_1080 (vfe_is_lite(vfe) ? 0x800 : 0x1000)
>> +#define BUS_REG_BASE \
>> + (IS_VFE_980(vfe) ? BUS_REG_BASE_980 : BUS_REG_BASE_1080)
>> #define VFE_BUS_WM_CGC_OVERRIDE (BUS_REG_BASE + 0x08)
>> #define WM_CGC_OVERRIDE_ALL (0x7FFFFFF)
>> @@ -55,7 +59,7 @@
>> * DISPLAY_DS2_C 6
>> * FD_Y 7
>> * FD_C 8
>> - * PIXEL_RAW 9
>> + * RAW_OUT(1080)/IR_OUT(980) 9
>> * STATS_AEC_BG 10
>> * STATS_AEC_BHIST 11
>> * STATS_TINTLESS_BG 12
>> @@ -86,7 +90,7 @@
>> */
>> #define RDI_WM(n) ((vfe_is_lite(vfe) ? 0x0 : 0x17) + (n))
>> -static void vfe_wm_start_1080(struct vfe_device *vfe, u8 wm,
>> struct vfe_line *line)
>> +static void vfe_wm_start(struct vfe_device *vfe, u8 wm, struct
>> vfe_line *line)
>> {
>> struct v4l2_pix_format_mplane *pix =
>> &line->video_out.active_fmt.fmt.pix_mp;
>> @@ -121,14 +125,14 @@ static void vfe_wm_start_1080(struct vfe_device
>> *vfe, u8 wm, struct vfe_line *li
>> writel(WM_CFG_EN | WM_CFG_MODE, vfe->base + VFE_BUS_WM_CFG(wm));
>> }
>> -static void vfe_wm_stop_1080(struct vfe_device *vfe, u8 wm)
>> +static void vfe_wm_stop(struct vfe_device *vfe, u8 wm)
>> {
>> wm = RDI_WM(wm);
>> writel(0, vfe->base + VFE_BUS_WM_CFG(wm));
>> }
>> -static void vfe_wm_update_1080(struct vfe_device *vfe, u8 wm, u32
>> addr,
>> - struct vfe_line *line)
>> +static void vfe_wm_update(struct vfe_device *vfe, u8 wm, u32 addr,
>> + struct vfe_line *line)
>> {
>> wm = RDI_WM(wm);
>> writel(addr >> 8, vfe->base + VFE_BUS_WM_IMAGE_ADDR(wm));
>> @@ -136,62 +140,62 @@ static void vfe_wm_update_1080(struct
>> vfe_device *vfe, u8 wm, u32 addr,
>> dev_dbg(vfe->camss->dev, "wm:%d, image buf addr:0x%x\n", wm,
>> addr);
>> }
>> -static void vfe_reg_update_1080(struct vfe_device *vfe, enum
>> vfe_line_id line_id)
>> +static void vfe_reg_update(struct vfe_device *vfe, enum vfe_line_id
>> line_id)
>> {
>> int port_id = line_id;
>> camss_reg_update(vfe->camss, vfe->id, port_id, false);
>> }
>> -static inline void vfe_reg_update_clear_1080(struct vfe_device *vfe,
>> - enum vfe_line_id line_id)
>> +static inline void vfe_reg_update_clear(struct vfe_device *vfe,
>> + enum vfe_line_id line_id)
>> {
>> int port_id = line_id;
>> camss_reg_update(vfe->camss, vfe->id, port_id, true);
>> }
>> -static const struct camss_video_ops vfe_video_ops_1080 = {
>> +static const struct camss_video_ops vfe_video_ops = {
>> .queue_buffer = vfe_queue_buffer_v2,
>> .flush_buffers = vfe_flush_buffers,
>> };
>> -static void vfe_subdev_init_1080(struct device *dev, struct
>> vfe_device *vfe)
>> +static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
>> {
>> - vfe->video_ops = vfe_video_ops_1080;
>> + vfe->video_ops = vfe_video_ops;
>> }
>> -static void vfe_global_reset_1080(struct vfe_device *vfe)
>> +static void vfe_global_reset(struct vfe_device *vfe)
>> {
>> vfe_isr_reset_ack(vfe);
>> }
>> -static irqreturn_t vfe_isr_1080(int irq, void *dev)
>> +static irqreturn_t vfe_isr(int irq, void *dev)
>> {
>> /* nop */
>> return IRQ_HANDLED;
>> }
>> -static int vfe_halt_1080(struct vfe_device *vfe)
>> +static int vfe_halt(struct vfe_device *vfe)
>> {
>> /* rely on vfe_disable_output() to stop the VFE */
>> return 0;
>> }
>> -const struct vfe_hw_ops vfe_ops_1080 = {
>> - .global_reset = vfe_global_reset_1080,
>> +const struct vfe_hw_ops vfe_ops_gen4 = {
>> + .global_reset = vfe_global_reset,
>> .hw_version = vfe_hw_version,
>> - .isr = vfe_isr_1080,
>> + .isr = vfe_isr,
>> .pm_domain_off = vfe_pm_domain_off,
>> .pm_domain_on = vfe_pm_domain_on,
>> - .reg_update = vfe_reg_update_1080,
>> - .reg_update_clear = vfe_reg_update_clear_1080,
>> - .subdev_init = vfe_subdev_init_1080,
>> + .reg_update = vfe_reg_update,
>> + .reg_update_clear = vfe_reg_update_clear,
>> + .subdev_init = vfe_subdev_init,
>> .vfe_disable = vfe_disable,
>> .vfe_enable = vfe_enable_v2,
>> - .vfe_halt = vfe_halt_1080,
>> - .vfe_wm_start = vfe_wm_start_1080,
>> - .vfe_wm_stop = vfe_wm_stop_1080,
>> + .vfe_halt = vfe_halt,
>> + .vfe_wm_start = vfe_wm_start,
>> + .vfe_wm_stop = vfe_wm_stop,
>> .vfe_buf_done = vfe_buf_done,
>> - .vfe_wm_update = vfe_wm_update_1080,
>> + .vfe_wm_update = vfe_wm_update,
>> };
>> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c
>> b/drivers/media/platform/qcom/camss/camss-vfe.c
>> index 399be8b70fed..b8aa4b7d1a8d 100644
>> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
>> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
>> @@ -350,6 +350,7 @@ static u32 vfe_src_pad_code(struct vfe_line
>> *line, u32 sink_code,
>> case CAMSS_845:
>> case CAMSS_8550:
>> case CAMSS_8650:
>> + case CAMSS_8750:
>> case CAMSS_8775P:
>> case CAMSS_KAANAPALI:
>> case CAMSS_X1E80100:
>> @@ -2012,6 +2013,7 @@ static int vfe_bpl_align(struct vfe_device *vfe)
>> case CAMSS_845:
>> case CAMSS_8550:
>> case CAMSS_8650:
>> + case CAMSS_8750:
>> case CAMSS_8775P:
>> case CAMSS_KAANAPALI:
>> case CAMSS_X1E80100:
>> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h
>> b/drivers/media/platform/qcom/camss/camss-vfe.h
>> index 118cac5daf37..c402ef170c81 100644
>> --- a/drivers/media/platform/qcom/camss/camss-vfe.h
>> +++ b/drivers/media/platform/qcom/camss/camss-vfe.h
>> @@ -249,8 +249,8 @@ extern const struct vfe_hw_ops vfe_ops_170;
>> extern const struct vfe_hw_ops vfe_ops_340;
>> extern const struct vfe_hw_ops vfe_ops_480;
>> extern const struct vfe_hw_ops vfe_ops_680;
>> -extern const struct vfe_hw_ops vfe_ops_1080;
>> extern const struct vfe_hw_ops vfe_ops_gen3;
>> +extern const struct vfe_hw_ops vfe_ops_gen4;
>> int vfe_get(struct vfe_device *vfe);
>> void vfe_put(struct vfe_device *vfe);
>> diff --git a/drivers/media/platform/qcom/camss/camss.c
>> b/drivers/media/platform/qcom/camss/camss.c
>> index 9dea343c1ac5..48d8f282d780 100644
>> --- a/drivers/media/platform/qcom/camss/camss.c
>> +++ b/drivers/media/platform/qcom/camss/camss.c
>> @@ -245,7 +245,7 @@ static const struct camss_subdev_resources
>> vfe_res_kaanapali[] = {
>> .reg_update_after_csid_config = true,
>> .has_pd = true,
>> .pd_name = "vfe0",
>> - .hw_ops = &vfe_ops_1080,
>> + .hw_ops = &vfe_ops_gen4,
>> .formats_rdi = &vfe_formats_rdi_845,
>> .formats_pix = &vfe_formats_pix_845
>> }
>> @@ -274,7 +274,7 @@ static const struct camss_subdev_resources
>> vfe_res_kaanapali[] = {
>> .reg_update_after_csid_config = true,
>> .has_pd = true,
>> .pd_name = "vfe1",
>> - .hw_ops = &vfe_ops_1080,
>> + .hw_ops = &vfe_ops_gen4,
>> .formats_rdi = &vfe_formats_rdi_845,
>> .formats_pix = &vfe_formats_pix_845
>> }
>> @@ -303,7 +303,7 @@ static const struct camss_subdev_resources
>> vfe_res_kaanapali[] = {
>> .reg_update_after_csid_config = true,
>> .has_pd = true,
>> .pd_name = "vfe2",
>> - .hw_ops = &vfe_ops_1080,
>> + .hw_ops = &vfe_ops_gen4,
>> .formats_rdi = &vfe_formats_rdi_845,
>> .formats_pix = &vfe_formats_pix_845
>> }
>> @@ -327,7 +327,7 @@ static const struct camss_subdev_resources
>> vfe_res_kaanapali[] = {
>> .line_num = 4,
>> .is_lite = true,
>> .reg_update_after_csid_config = true,
>> - .hw_ops = &vfe_ops_1080,
>> + .hw_ops = &vfe_ops_gen4,
>> .formats_rdi = &vfe_formats_rdi_845,
>> .formats_pix = &vfe_formats_pix_845
>> }
>> @@ -351,7 +351,7 @@ static const struct camss_subdev_resources
>> vfe_res_kaanapali[] = {
>> .line_num = 4,
>> .is_lite = true,
>> .reg_update_after_csid_config = true,
>> - .hw_ops = &vfe_ops_1080,
>> + .hw_ops = &vfe_ops_gen4,
>> .formats_rdi = &vfe_formats_rdi_845,
>> .formats_pix = &vfe_formats_pix_845
>> }
>> @@ -4053,6 +4053,144 @@ static const struct camss_subdev_resources
>> csid_res_8750[] = {
>> }
>> };
>> +static const struct camss_subdev_resources vfe_res_8750[] = {
>> + /* VFE0 - TFE Full */
>> + {
>> + .regulators = {},
>> + .clock = { "gcc_hf_axi", "vfe0_fast_ahb", "vfe0",
>> + "camnoc_rt_vfe0", "camnoc_rt_vfe1", "camnoc_rt_vfe2",
>> + "camnoc_rt_axi", "camnoc_nrt_axi", "qdss_debug_xo" },
>> + .clock_rate = { { 0 },
>> + { 0 },
>> + { 360280000, 480000000, 630000000, 716000000,
>> + 833000000 },
>> + { 0 },
>> + { 0 },
>> + { 0 },
>> + { 200000000, 300000000, 400000000, 480000000 },
>> + { 0 },
>> + { 0 } },
>> + .reg = { "vfe0" },
>> + .interrupt = { "vfe0" },
>> + .vfe = {
>> + .line_num = 3,
>> + .is_lite = false,
>> + .reg_update_after_csid_config = true,
>> + .has_pd = true,
>> + .pd_name = "vfe0",
>> + .hw_ops = &vfe_ops_gen4,
>> + .formats_rdi = &vfe_formats_rdi_845,
>> + .formats_pix = &vfe_formats_pix_845
>> + }
>> + },
>> + /* VFE1 - TFE Full */
>> + {
>> + .regulators = {},
>> + .clock = { "gcc_hf_axi", "vfe1_fast_ahb", "vfe1",
>> + "camnoc_rt_vfe0", "camnoc_rt_vfe1", "camnoc_rt_vfe2",
>> + "camnoc_rt_axi", "camnoc_nrt_axi", "qdss_debug_xo" },
>> + .clock_rate = { { 0 },
>> + { 0 },
>> + { 360280000, 480000000, 630000000, 716000000,
>> + 833000000 },
>> + { 0 },
>> + { 0 },
>> + { 0 },
>> + { 200000000, 300000000, 400000000, 480000000 },
>> + { 0 },
>> + { 0 } },
>> + .reg = { "vfe1" },
>> + .interrupt = { "vfe1" },
>> + .vfe = {
>> + .line_num = 3,
>> + .is_lite = false,
>> + .reg_update_after_csid_config = true,
>> + .has_pd = true,
>> + .pd_name = "vfe1",
>> + .hw_ops = &vfe_ops_gen4,
>> + .formats_rdi = &vfe_formats_rdi_845,
>> + .formats_pix = &vfe_formats_pix_845
>> + }
>> + },
>> + /* VFE2 - TFE Full */
>> + {
>> + .regulators = {},
>> + .clock = { "gcc_hf_axi", "vfe2_fast_ahb", "vfe2",
>> + "camnoc_rt_vfe0", "camnoc_rt_vfe1", "camnoc_rt_vfe2",
>> + "camnoc_rt_axi", "camnoc_nrt_axi", "qdss_debug_xo" },
>> + .clock_rate = { { 0 },
>> + { 0 },
>> + { 360280000, 480000000, 630000000, 716000000,
>> + 833000000 },
>> + { 0 },
>> + { 0 },
>> + { 0 },
>> + { 200000000, 300000000, 400000000, 480000000 },
>> + { 0 },
>> + { 0 } },
>> + .reg = { "vfe2" },
>> + .interrupt = { "vfe2" },
>> + .vfe = {
>> + .line_num = 3,
>> + .is_lite = false,
>> + .reg_update_after_csid_config = true,
>> + .has_pd = true,
>> + .pd_name = "vfe2",
>> + .hw_ops = &vfe_ops_gen4,
>> + .formats_rdi = &vfe_formats_rdi_845,
>> + .formats_pix = &vfe_formats_pix_845
>> + }
>> + },
>> + /* VFE_LITE0 */
>> + {
>> + .regulators = {},
>> + .clock = { "gcc_hf_axi", "vfe_lite_ahb", "vfe_lite",
>> + "camnoc_rt_vfe_lite", "camnoc_rt_axi",
>> + "camnoc_nrt_axi", "qdss_debug_xo" },
>> + .clock_rate = { { 0 },
>> + { 0 },
>> + { 266666667, 400000000, 480000000 },
>> + { 0 },
>> + { 200000000, 300000000, 400000000, 480000000 },
>> + { 0 },
>> + { 0 } },
>> + .reg = { "vfe_lite0" },
>> + .interrupt = { "vfe_lite0" },
>> + .vfe = {
>> + .line_num = 4,
>> + .is_lite = true,
>> + .reg_update_after_csid_config = true,
>> + .hw_ops = &vfe_ops_gen4,
>> + .formats_rdi = &vfe_formats_rdi_845,
>> + .formats_pix = &vfe_formats_pix_845
>> + }
>> + },
>> + /* VFE_LITE1 */
>> + {
>> + .regulators = {},
>> + .clock = { "gcc_hf_axi", "vfe_lite_ahb", "vfe_lite",
>> + "camnoc_rt_vfe_lite", "camnoc_rt_axi",
>> + "camnoc_nrt_axi", "qdss_debug_xo" },
>> + .clock_rate = { { 0 },
>> + { 0 },
>> + { 266666667, 400000000, 480000000 },
>> + { 0 },
>> + { 200000000, 300000000, 400000000, 480000000 },
>> + { 0 },
>> + { 0 } },
>> + .reg = { "vfe_lite1" },
>> + .interrupt = { "vfe_lite1" },
>> + .vfe = {
>> + .line_num = 4,
>> + .is_lite = true,
>> + .reg_update_after_csid_config = true,
>> + .hw_ops = &vfe_ops_gen4,
>> + .formats_rdi = &vfe_formats_rdi_845,
>> + .formats_pix = &vfe_formats_pix_845
>> + }
>> + }
>> +};
>> +
>> static const struct resources_icc icc_res_sm8750[] = {
>> {
>> .name = "ahb",
>> @@ -5485,9 +5623,11 @@ static const struct camss_resources
>> sm8750_resources = {
>> .pd_name = "top",
>> .csiphy_res = csiphy_res_8750,
>> .csid_res = csid_res_8750,
>> + .vfe_res = vfe_res_8750,
>> .icc_res = icc_res_sm8750,
>> .csiphy_num = ARRAY_SIZE(csiphy_res_8750),
>> .csid_num = ARRAY_SIZE(csid_res_8750),
>> + .vfe_num = ARRAY_SIZE(vfe_res_8750),
>> .icc_path_num = ARRAY_SIZE(icc_res_sm8750),
>> };
>>
>
> Once done.
>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
Powered by blists - more mailing lists