CAMSS: SM8250 support (and some fixes)
Subsystems (5)
cover CAMSS: SM8250 support (and some fixes)
This adds initial support for SM8250 and its 4 VFEs and 6 CSIPHYs. The only big change is the added camss-vfe-480.c to support the Titan 480 VFE.
Jonathan Marek (17):
media: camss: csiphy-3ph: don't print HW version as an error
media: camss: csiphy-3ph: disable interrupts
media: camss: csiphy-3ph: add support for SM8250 CSI DPHY
media: camss: csid-170: fix non-10bit formats
media: camss: csid-170: don't enable unused irqs
media: camss: csid-170: remove stray comment
media: camss: csid-170: support more than one lite vfe
media: camss: csid-170: set the right HALF_CMD when disabled
media: camss: csid: allow csid to work without a regulator
media: camss: remove vdda-csiN from sdm845 resources
media: camss: fix VFE irq name
media: camss: vfe-170: clean up some dead code
media: camss: vfe-170: fix "VFE halt timeout" error
media: camss: Add initial support for VFE hardware version Titan 480
media: camss: add support for V4L2_PIX_FMT_GREY for sdm845 HW
media: camss: add support for SM8250 camss
media: dt-bindings: media: camss: Add qcom,sm8250-camss binding
.../bindings/media/qcom,sm8250-camss.yaml | 398 +++++++++++++ drivers/media/platform/qcom/camss/Makefile | 1 + .../platform/qcom/camss/camss-csid-170.c | 31 +- .../media/platform/qcom/camss/camss-csid.c | 39 +- .../qcom/camss/camss-csiphy-3ph-1-0.c | 181 ++++-- .../media/platform/qcom/camss/camss-csiphy.c | 9 +- .../media/platform/qcom/camss/camss-vfe-170.c | 65 +- .../media/platform/qcom/camss/camss-vfe-480.c | 554 ++++++++++++++++++ drivers/media/platform/qcom/camss/camss-vfe.c | 13 +- drivers/media/platform/qcom/camss/camss-vfe.h | 1 + .../media/platform/qcom/camss/camss-video.c | 5 +- drivers/media/platform/qcom/camss/camss.c | 202 ++++++- drivers/media/platform/qcom/camss/camss.h | 1 + 13 files changed, 1352 insertions(+), 148 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-480.c
Signature · 1 line
2.26.1
PATCH 1/17 media: camss: csiphy-3ph: don't print HW version as an error
Avoid unnecessary noise in normal usage (it prints every time CSIPHY is powered on).
--- drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +1 -1
@@ -163,7 +163,7 @@ static void csiphy_hw_version_read(struct csiphy_device *csiphy, hw_version |=readl_relaxed (csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_STATUSn (15 )) <<24 ; -dev_err (dev,"CSIPHY 3PH HW Version = 0x%08x\n" , hw_version); +dev_dbg (dev,"CSIPHY 3PH HW Version = 0x%08x\n" , hw_version); }/*
Signature · 1 line
2.26.1
Re: [PATCH 01/17] media: camss: csiphy-3ph: don't print HW version as an error
Hey Jonathan,
Thanks for submitting this series, and including some cleanups and fixes.
Quoted 14 lines · Jonathan Marek
Avoid unnecessary noise in normal usage (it prints every time CSIPHY is powered on).
Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +0
-0
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c static void csiphy_hw_version_read(struct csiphy_device *csiphy, · line 169
Quoted 8 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +1
-1@@ -163,7 +163,7 @@ static void csiphy_hw_version_read(struct csiphy_device *csiphy, hw_version |=readl_relaxed (csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_STATUSn (15 )) <<24 ; -dev_err (dev,"CSIPHY 3PH HW Version = 0x%08x\n" , hw_version); +dev_dbg (dev,"CSIPHY 3PH HW Version = 0x%08x\n" , hw_version); }/*
PATCH 2/17 media: camss: csiphy-3ph: disable interrupts
In reply to Jonathan MarekThe driver does nothing with the interrupts, so set the irq mask registers to zero to avoid wasting CPU time for thing.
--- .../qcom/camss/camss-csiphy-3ph-1-0.c | 35 ++----------------- 1 file changed, 3 insertions(+), 32 deletions(-)
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +3 -32
@@ -352,38 +352,9 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,else if (csiphy->camss->version ==CAMSS_845 )csiphy_gen2_config_lanes (csiphy, settle_cnt); - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (11 )); - - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (12 )); - - val =0xfb ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (13 )); - - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (14 )); - - val =0x7f ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (15 )); - - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (16 )); - - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (17 )); - - val =0xef ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (18 )); - - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (19 )); - - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (20 )); - - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (21 )); +/* IRQ_MASK registers - disable all interrupts */ +for (i =11 ; i <22 ; i++) +writel_relaxed (0 , csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (i)); } static void csiphy_lanes_disable(struct csiphy_device *csiphy,
Signature · 1 line
2.26.1
Re: [PATCH 02/17] media: camss: csiphy-3ph: disable interrupts
Quoted 45 lines · Jonathan Marek
The driver does nothing with the interrupts, so set the irq mask registers to zero to avoid wasting CPU time for thing.
Signed-off-by Jonathan Marek <[email protected]>--- .../qcom/camss/camss-csiphy-3ph-1-0.c | 35 ++----------------- 1 file changed, 3 insertions(+), 32 deletions(-)drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +0
-28@@ -352,38 +352,9 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,else if (csiphy->camss->version ==CAMSS_845 )csiphy_gen2_config_lanes (csiphy, settle_cnt); - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (11 )); - - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (12 )); - - val =0xfb ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (13 )); - - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (14 )); - - val =0x7f ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (15 )); - - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (16 )); - - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (17 )); - - val =0xef ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (18 )); - - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (19 )); - - val =0xff ;
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c static void csiphy_lanes_enable(struct csiphy_device *csiphy, · line 360
Quoted 10 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +3
-4-writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (20 )); - - val =0xff ; -writel_relaxed (val, csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (21 )); +/* IRQ_MASK registers - disable all interrupts */ +for (i =11 ; i <22 ; i++) +writel_relaxed (0 , csiphy->base +CSIPHY_3PH_CMN_CSI_COMMON_CTRLn (i)); } static void csiphy_lanes_disable(struct csiphy_device *csiphy,
PATCH 3/17 media: camss: csiphy-3ph: add support for SM8250 CSI DPHY
In reply to Jonathan MarekAdd support for CSIPHY (2PH/DPHY mode) found on SM8250 hardware.
--- .../qcom/camss/camss-csiphy-3ph-1-0.c | 144 +++++++++++++++++- 1 file changed, 137 insertions(+), 7 deletions(-)
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +137 -7
@@ -62,6 +62,7 @@ struct csiphy_reg_t {u32 csiphy_param_type; }; +/* GEN2 1.0 2PH */ static const struct csiphy_reg_t lane_regs_sdm845[5 ][14 ] = { {
@@ -146,6 +147,121 @@ csiphy_reg_t lane_regs_sdm845[5][14] = { }, }; +/* GEN2 1.2.1 2PH */ +static const struct +csiphy_reg_t lane_regs_sm8250[5 ][20 ] = { + { + {0x0030 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0900 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0908 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0904 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0904 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0004 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x002C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0034 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0010 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x001C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x003C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0008 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0000 ,0x8D ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x000c ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0038 ,0xFE ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0014 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0028 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0024 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, + { + {0x0730 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C80 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C88 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C84 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C84 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0704 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x072C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0734 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0710 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x071C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x073C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0708 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0700 ,0x80 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x070c ,0xA5 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0738 ,0x1F ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0714 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0728 ,0x04 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0724 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, + { + {0x0230 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0A00 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0A08 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0A04 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0A04 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0204 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x022C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0234 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0210 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x021C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x023C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0208 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0200 ,0x8D ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x020c ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0238 ,0xFE ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0214 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0228 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0224 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, + { + {0x0430 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0B00 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0B08 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0B04 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0B04 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0404 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x042C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0434 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0410 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x041C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x043C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0408 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0400 ,0x8D ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x040c ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0438 ,0xFE ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0414 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0428 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0424 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, + { + {0x0630 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C00 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C08 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C04 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C04 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0604 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x062C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0634 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0610 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x061C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x063C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0608 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0600 ,0x8D ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x060c ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0638 ,0xFE ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0614 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0628 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0624 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, +}; +static void csiphy_hw_version_read (struct csiphy_device *csiphy,struct device *dev) {
@@ -298,13 +414,23 @@ static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy,static void csiphy_gen2_config_lanes (struct csiphy_device *csiphy,u8 settle_cnt) { -int i, l; -u32 val; +const struct csiphy_reg_t *r; +int i, l, array_size; +u32 val, lane_enable; + +switch (csiphy->camss->version) { +case CAMSS_845 : + r = &lane_regs_sdm845[0 ][0 ]; + array_size =ARRAY_SIZE (lane_regs_sdm845[0 ]); +break ; +case CAMSS_8250 : + r = &lane_regs_sm8250[0 ][0 ]; + array_size =ARRAY_SIZE (lane_regs_sm8250[0 ]); +break ; + }for (l =0 ; l <5 ; l++) { -for (i =0 ; i <14 ; i++) { -const struct csiphy_reg_t *r = &lane_regs_sdm845[l][i]; - +for (i =0 ; i < array_size; i++, r++) {switch (r->csiphy_param_type) {case CSIPHY_SETTLE_CNT_LOWER_BYTE : val = settle_cnt &0xff ;
@@ -331,7 +457,10 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy, settle_cnt =csiphy_settle_cnt_calc (link_freq, csiphy->timer_clk_rate); - val =BIT (c->clk.pos); +if (csiphy->camss->version ==CAMSS_8250 ) + val =BIT (7 ); +else + val =BIT (c->clk.pos);for (i =0 ; i < c->num_data; i++) val |=BIT (c->data[i].pos *2 );
@@ -349,7 +478,8 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,if (csiphy->camss->version == CAMSS_8x16 || csiphy->camss->version == CAMSS_8x96)csiphy_gen1_config_lanes (csiphy, cfg, settle_cnt); -else if (csiphy->camss->version ==CAMSS_845 ) +else if (csiphy->camss->version ==CAMSS_845 || + csiphy->camss->version ==CAMSS_8250 )csiphy_gen2_config_lanes (csiphy, settle_cnt);/* IRQ_MASK registers - disable all interrupts */
Signature · 1 line
2.26.1
Re: [PATCH 03/17] media: camss: csiphy-3ph: add support for SM8250 CSI DPHY
Quoted 148 lines · Jonathan Marek
Add support for CSIPHY (2PH/DPHY mode) found on SM8250 hardware.
Signed-off-by Jonathan Marek <[email protected]>--- .../qcom/camss/camss-csiphy-3ph-1-0.c | 144 +++++++++++++++++- 1 file changed, 137 insertions(+), 7 deletions(-)drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +116
-2@@ -62,6 +62,7 @@ struct csiphy_reg_t {u32 csiphy_param_type; }; +/* GEN2 1.0 2PH */ static const struct csiphy_reg_t lane_regs_sdm845[5 ][14 ] = { {@@ -146,6 +147,121 @@ csiphy_reg_t lane_regs_sdm845[5][14] = { }, }; +/* GEN2 1.2.1 2PH */ +static const struct +csiphy_reg_t lane_regs_sm8250[5 ][20 ] = { + { + {0x0030 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0900 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0908 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0904 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0904 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0004 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x002C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0034 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0010 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x001C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x003C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0008 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0000 ,0x8D ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x000c ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0038 ,0xFE ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0014 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0028 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0024 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, + { + {0x0730 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C80 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C88 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C84 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C84 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0704 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x072C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0734 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0710 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x071C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x073C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0708 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0700 ,0x80 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x070c ,0xA5 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0738 ,0x1F ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0714 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0728 ,0x04 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0724 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, + { + {0x0230 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0A00 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0A08 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0A04 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0A04 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0204 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x022C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0234 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0210 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x021C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x023C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0208 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0200 ,0x8D ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x020c ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0238 ,0xFE ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0214 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0228 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0224 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, + { + {0x0430 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0B00 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0B08 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0B04 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0B04 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0404 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x042C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0434 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0410 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x041C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x043C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0408 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0400 ,0x8D ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x040c ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0438 ,0xFE ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0414 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0428 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0424 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, + { + {0x0630 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C00 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C08 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C04 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C04 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0604 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x062C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0634 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0610 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x061C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x063C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0608 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0600 ,0x8D ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x060c ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0638 ,0xFE ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0614 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0628 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0624 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, +}; +static void csiphy_hw_version_read (struct csiphy_device *csiphy,struct device *dev) {@@ -298,13 +414,23 @@ static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy,static void csiphy_gen2_config_lanes (struct csiphy_device *csiphy,u8 settle_cnt) { -int i, l; -u32 val;
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy, · line 426
Quoted 10 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +10
-0+const struct csiphy_reg_t *r; +int i, l, array_size; +u32 val, lane_enable; + +switch (csiphy->camss->version) { +case CAMSS_845 : + r = &lane_regs_sdm845[0 ][0 ]; + array_size =ARRAY_SIZE (lane_regs_sdm845[0 ]); +break ; +case CAMSS_8250 :
CAMSS_8250 is not defined until later in the series, in "media: camss:
add support for SM8250 camss".
Quoted 14 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +5
-3+ r = &lane_regs_sm8250[0 ][0 ]; + array_size =ARRAY_SIZE (lane_regs_sm8250[0 ]); +break ; + }for (l =0 ; l <5 ; l++) { -for (i =0 ; i <14 ; i++) { -const struct csiphy_reg_t *r = &lane_regs_sdm845[l][i]; - +for (i =0 ; i < array_size; i++, r++) {switch (r->csiphy_param_type) {case CSIPHY_SETTLE_CNT_LOWER_BYTE : val = settle_cnt &0xff ;
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c static void csiphy_lanes_enable(struct csiphy_device *csiphy, · line 463
Quoted 8 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +4
-1@@ -331,7 +457,10 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy, settle_cnt =csiphy_settle_cnt_calc (link_freq, csiphy->timer_clk_rate); - val =BIT (c->clk.pos); +if (csiphy->camss->version ==CAMSS_8250 ) + val =BIT (7 ); +else + val =BIT (c->clk.pos);
sdm845 and sm8250 behave the same way, and I think this chunk should reflect that. With that being said the docs for camss-sdm845 mention that the only valid lane for the clock is #7. I don't know if it is preferred to enforce the restriction in the driver, yaml or both.
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c static void csiphy_lanes_enable(struct csiphy_device *csiphy, · line 485
Quoted 13 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +2
-1for (i =0 ; i < c->num_data; i++) val |=BIT (c->data[i].pos *2 );@@ -349,7 +478,8 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,if (csiphy->camss->version == CAMSS_8x16 || csiphy->camss->version == CAMSS_8x96)csiphy_gen1_config_lanes (csiphy, cfg, settle_cnt); -else if (csiphy->camss->version ==CAMSS_845 ) +else if (csiphy->camss->version ==CAMSS_845 || + csiphy->camss->version ==CAMSS_8250 )csiphy_gen2_config_lanes (csiphy, settle_cnt);/* IRQ_MASK registers - disable all interrupts */
With the above issues fixed;
Re: [PATCH 03/17] media: camss: csiphy-3ph: add support for SM8250 CSI DPHY
In reply to Jonathan MarekHi Jonathan,
Thank you for the patchset!
Quoted 147 lines · Jonathan Marek
Add support for CSIPHY (2PH/DPHY mode) found on SM8250 hardware.
Signed-off-by Jonathan Marek <[email protected]>--- .../qcom/camss/camss-csiphy-3ph-1-0.c | 144 +++++++++++++++++- 1 file changed, 137 insertions(+), 7 deletions(-)drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +116
-2@@ -62,6 +62,7 @@ struct csiphy_reg_t {u32 csiphy_param_type; }; +/* GEN2 1.0 2PH */ static const struct csiphy_reg_t lane_regs_sdm845[5 ][14 ] = { {@@ -146,6 +147,121 @@ csiphy_reg_t lane_regs_sdm845[5][14] = { }, }; +/* GEN2 1.2.1 2PH */ +static const struct +csiphy_reg_t lane_regs_sm8250[5 ][20 ] = { + { + {0x0030 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0900 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0908 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0904 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0904 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0004 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x002C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0034 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0010 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x001C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x003C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0008 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0000 ,0x8D ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x000c ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0038 ,0xFE ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0014 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0028 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0024 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, + { + {0x0730 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C80 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C88 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C84 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C84 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0704 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x072C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0734 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0710 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x071C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x073C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0708 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0700 ,0x80 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x070c ,0xA5 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0738 ,0x1F ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0714 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0728 ,0x04 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0724 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, + { + {0x0230 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0A00 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0A08 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0A04 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0A04 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0204 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x022C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0234 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0210 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x021C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x023C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0208 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0200 ,0x8D ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x020c ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0238 ,0xFE ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0214 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0228 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0224 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, + { + {0x0430 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0B00 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0B08 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0B04 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0B04 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0404 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x042C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0434 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0410 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x041C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x043C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0408 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0400 ,0x8D ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x040c ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0438 ,0xFE ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0414 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0428 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0424 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, + { + {0x0630 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C00 ,0x05 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C08 ,0x10 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C04 ,0x00 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0C04 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0604 ,0x0C ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x062C ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0634 ,0x07 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0610 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x061C ,0x08 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x063C ,0xB8 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0608 ,0x10 ,0x00 ,CSIPHY_SETTLE_CNT_LOWER_BYTE }, + {0x0600 ,0x8D ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x060c ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0638 ,0xFE ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0614 ,0x60 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0628 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0624 ,0x00 ,0x00 ,CSIPHY_DNP_PARAMS }, + {0x0800 ,0x02 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + {0x0884 ,0x01 ,0x00 ,CSIPHY_DEFAULT_PARAMS }, + }, +}; +static void csiphy_hw_version_read (struct csiphy_device *csiphy,struct device *dev) {@@ -298,13 +414,23 @@ static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy,static void csiphy_gen2_config_lanes (struct csiphy_device *csiphy,u8 settle_cnt) { -int i, l; -u32 val;
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy, · line 426
Quoted 10 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +10
-0+const struct csiphy_reg_t *r; +int i, l, array_size; +u32 val, lane_enable; + +switch (csiphy->camss->version) { +case CAMSS_845 : + r = &lane_regs_sdm845[0 ][0 ]; + array_size =ARRAY_SIZE (lane_regs_sdm845[0 ]); +break ; +case CAMSS_8250 :
CAMSS_8250 is only introduced in "[PATCH 16_17] media: camss: add support for SM8250 camss", and this breaks bisecting.
Thanks,
Andrey
Quoted 36 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +11
-5+ r = &lane_regs_sm8250[0 ][0 ]; + array_size =ARRAY_SIZE (lane_regs_sm8250[0 ]); +break ; + }for (l =0 ; l <5 ; l++) { -for (i =0 ; i <14 ; i++) { -const struct csiphy_reg_t *r = &lane_regs_sdm845[l][i]; - +for (i =0 ; i < array_size; i++, r++) {switch (r->csiphy_param_type) {case CSIPHY_SETTLE_CNT_LOWER_BYTE : val = settle_cnt &0xff ;@@ -331,7 +457,10 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy, settle_cnt =csiphy_settle_cnt_calc (link_freq, csiphy->timer_clk_rate); - val =BIT (c->clk.pos); +if (csiphy->camss->version ==CAMSS_8250 ) + val =BIT (7 ); +else + val =BIT (c->clk.pos);for (i =0 ; i < c->num_data; i++) val |=BIT (c->data[i].pos *2 );@@ -349,7 +478,8 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,if (csiphy->camss->version == CAMSS_8x16 || csiphy->camss->version == CAMSS_8x96)csiphy_gen1_config_lanes (csiphy, cfg, settle_cnt); -else if (csiphy->camss->version ==CAMSS_845 ) +else if (csiphy->camss->version ==CAMSS_845 || + csiphy->camss->version ==CAMSS_8250 )csiphy_gen2_config_lanes (csiphy, settle_cnt);/* IRQ_MASK registers - disable all interrupts */
Re: [PATCH 03/17] media: camss: csiphy-3ph: add support for SM8250 CSI DPHY
Hi Jonathan,
Couple more minor issues.
Quoted 145 lines · Andrey Konovalov
Hi Jonathan,
Thank you for the patchset!
Quoted 140 lines · Jonathan Marek
Add support for CSIPHY (2PH/DPHY mode) found on SM8250 hardware.
Signed-off-by Jonathan Marek <[email protected]>---
.../qcom/camss/camss-csiphy-3ph-1-0.c | 144 +++++++++++++++++-1 file changed, 137 insertions(+), 7 deletions(-)drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +1
-0@@ -62,6 +62,7 @@ struct csiphy_reg_t { u32 csiphy_param_type; }; +/* GEN2 1.0 2PH */ static const struct
csiphy_reg_tlane_regs_sdm845[5][14] = {
{drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +115
-0@@ -146,6 +147,121 @@ csiphy_reg_t lane_regs_sdm845[5][14] = { }, }; +/* GEN2 1.2.1 2PH */ +static const struct +csiphy_reg_t lane_regs_sm8250[5 ][20 ] = { + { + {0x0030, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0900, 0x05, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0908, 0x10, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0904, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0904, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0004, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x002C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0034, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0010, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x001C, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x003C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0008, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE}, + {0x0000, 0x8D, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x000c, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0038, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0014, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0028, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0024, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0884, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + }, + { + {0x0730, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C80, 0x05, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C88, 0x10, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C84, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C84, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0704, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x072C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0734, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0710, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x071C, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x073C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0708, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE}, + {0x0700, 0x80, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x070c, 0xA5, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0738, 0x1F, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0714, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0728, 0x04, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0724, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0884, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + }, + { + {0x0230, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0A00, 0x05, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0A08, 0x10, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0A04, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0A04, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0204, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x022C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0234, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0210, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x021C, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x023C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0208, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE}, + {0x0200, 0x8D, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x020c, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0238, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0214, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0228, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0224, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0884, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + }, + { + {0x0430, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0B00, 0x05, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0B08, 0x10, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0B04, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0B04, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0404, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x042C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0434, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0410, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x041C, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x043C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0408, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE}, + {0x0400, 0x8D, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x040c, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0438, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0414, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0428, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0424, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0884, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + }, + { + {0x0630, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C00, 0x05, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C08, 0x10, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C04, 0x00, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0C04, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0604, 0x0C, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x062C, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0634, 0x07, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0610, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x061C, 0x08, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x063C, 0xB8, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0608, 0x10, 0x00, CSIPHY_SETTLE_CNT_LOWER_BYTE}, + {0x0600, 0x8D, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x060c, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0638, 0xFE, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0614, 0x60, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0628, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0624, 0x00, 0x00, CSIPHY_DNP_PARAMS}, + {0x0800, 0x02, 0x00, CSIPHY_DEFAULT_PARAMS}, + {0x0884, 0x01, 0x00, CSIPHY_DEFAULT_PARAMS}, + }, +}; + static void csiphy_hw_version_read(struct csiphy_device *csiphy, struct device *dev) {@@ -298,13 +414,23 @@ static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy,
Quoted 8 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy, · line 419
Quoted 8 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +3
-2static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy, u8 settle_cnt) { - int i, l; - u32 val; + const struct csiphy_reg_t *r; + int i, l, array_size; + u32 val, lane_enable;
lane_enable is not used in this patch ("warning: unused variable ‘lane_enable’").
Quoted 2 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy, · line 421
Quoted 2 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +2
-0+ + switch (csiphy->camss->version) {
This switch statement doesn't have the "default:" clause which leads to several warnings of "enumeration value ‘CAMSS_8x16’ not handled in switch" kind.
Thanks,
Andrey
Quoted 43 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy, · line 426
Quoted 5 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +5
-0+ case CAMSS_845: + r = &lane_regs_sdm845[0][0]; + array_size = ARRAY_SIZE(lane_regs_sdm845[0]); + break; + case CAMSS_8250:
CAMSS_8250is only introduced in "[PATCH 16_17] media: camss: add support for SM8250 camss", and this breaks bisecting.Thanks,
AndreyQuoted 31 lines
drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +5
-3+ r = &lane_regs_sm8250[0][0]; + array_size = ARRAY_SIZE(lane_regs_sm8250[0]); + break; + } for (l = 0; l < 5; l++) { - for (i = 0; i < 14; i++) { - const struct csiphy_reg_t *r = &lane_regs_sdm845[l][i]; - + for (i = 0; i < array_size; i++, r++) { switch (r->csiphy_param_type) {case
CSIPHY_SETTLE_CNT_LOWER_BYTE:
val =settle_cnt&0xff;drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c +4
-1@@ -331,7 +457,10 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy, settle_cnt = csiphy_settle_cnt_calc(link_freq, csiphy->timer_clk_rate); - val = BIT(c->clk.pos); + if (csiphy->camss->version == CAMSS_8250) + val = BIT(7); + else + val = BIT(c->clk.pos); for (i = 0; i < c->num_data; i++) val |= BIT(c->data[i].pos * 2);@@ -349,7 +478,8 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,if (
csiphy->camss->version==CAMSS_8x16||csiphy->camss->version==CAMSS_8x96)csiphy_gen1_config_lanes(csiphy, cfg, settle_cnt); - else if (csiphy->camss->version==CAMSS_845) + else if (csiphy->camss->version==CAMSS_845|| +csiphy->camss->version==CAMSS_8250)csiphy_gen2_config_lanes(csiphy, settle_cnt); /*IRQ_MASKregisters - disable all interrupts */
PATCH 4/17 media: camss: csid-170: fix non-10bit formats
In reply to Jonathan MarekUse the decode_format/data_type from the "format" struct instead of a hardcoded 10-bit format.
--- drivers/media/platform/qcom/camss/camss-csid-170.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/media/platform/qcom/camss/camss-csid-170.c +3 -3
@@ -366,7 +366,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val |= input_format->width &0x1fff << TPG_DT_n_CFG_0_FRAME_WIDTH;writel_relaxed (val, csid->base +CSID_TPG_DT_n_CFG_0 (0 )); - val =DATA_TYPE_RAW_10BIT << TPG_DT_n_CFG_1_DATA_TYPE; + val = format->data_type << TPG_DT_n_CFG_1_DATA_TYPE;writel_relaxed (val, csid->base +CSID_TPG_DT_n_CFG_1 (0 )); val = tg->mode << TPG_DT_n_CFG_2_PAYLOAD_MODE;
@@ -382,8 +382,8 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val =1 <<RDI_CFG0_BYTE_CNTR_EN ; val |=1 <<RDI_CFG0_FORMAT_MEASURE_EN ; val |=1 <<RDI_CFG0_TIMESTAMP_EN ; - val |=DECODE_FORMAT_PAYLOAD_ONLY <<RDI_CFG0_DECODE_FORMAT ; - val |=DATA_TYPE_RAW_10BIT <<RDI_CFG0_DATA_TYPE ; + val |= format->decode_format <<RDI_CFG0_DECODE_FORMAT ; + val |= format->data_type <<RDI_CFG0_DATA_TYPE ; val |= vc <<RDI_CFG0_VIRTUAL_CHANNEL ; val |= dt_id <<RDI_CFG0_DT_ID ;writel_relaxed (val, csid->base +CSID_RDI_CFG0 (0 ));
Signature · 1 line
2.26.1
Re: [PATCH 04/17] media: camss: csid-170: fix non-10bit formats
Hey Jonathan,
Thanks for catching this.
Quoted 24 lines · Jonathan Marek
Use the decode_format/data_type from the "format" struct instead of a hardcoded 10-bit format.
Fixes eebe6d00e9bf ("media: camss: Add support for CSID hardware version Titan 170")Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/camss-csid-170.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)drivers/media/platform/qcom/camss/camss-csid-170.c +1
-1@@ -366,7 +366,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val |= input_format->width &0x1fff << TPG_DT_n_CFG_0_FRAME_WIDTH;writel_relaxed (val, csid->base +CSID_TPG_DT_n_CFG_0 (0 )); - val =DATA_TYPE_RAW_10BIT << TPG_DT_n_CFG_1_DATA_TYPE; + val = format->data_type << TPG_DT_n_CFG_1_DATA_TYPE;writel_relaxed (val, csid->base +CSID_TPG_DT_n_CFG_1 (0 )); val = tg->mode << TPG_DT_n_CFG_2_PAYLOAD_MODE;
drivers/media/platform/qcom/camss/camss-csid-170.c static void csid_configure_stream(struct csid_device *csid, u8 enable) · line 389
Quoted 10 lines
drivers/media/platform/qcom/camss/camss-csid-170.c +2
-2@@ -382,8 +382,8 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val =1 <<RDI_CFG0_BYTE_CNTR_EN ; val |=1 <<RDI_CFG0_FORMAT_MEASURE_EN ; val |=1 <<RDI_CFG0_TIMESTAMP_EN ; - val |=DECODE_FORMAT_PAYLOAD_ONLY <<RDI_CFG0_DECODE_FORMAT ; - val |=DATA_TYPE_RAW_10BIT <<RDI_CFG0_DATA_TYPE ; + val |= format->decode_format <<RDI_CFG0_DECODE_FORMAT ; + val |= format->data_type <<RDI_CFG0_DATA_TYPE ; val |= vc <<RDI_CFG0_VIRTUAL_CHANNEL ; val |= dt_id <<RDI_CFG0_DT_ID ;writel_relaxed (val, csid->base +CSID_RDI_CFG0 (0 ));
Re: [PATCH 04/17] media: camss: csid-170: fix non-10bit formats
In reply to Jonathan MarekHi Jonathan,
Thank you for your patch!
Quoted 23 lines · Jonathan Marek
Use the decode_format/data_type from the "format" struct instead of a hardcoded 10-bit format.
Fixes eebe6d00e9bf ("media: camss: Add support for CSID hardware version Titan 170")Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/camss-csid-170.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)drivers/media/platform/qcom/camss/camss-csid-170.c +1
-1@@ -366,7 +366,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val |= input_format->width &0x1fff << TPG_DT_n_CFG_0_FRAME_WIDTH;writel_relaxed (val, csid->base +CSID_TPG_DT_n_CFG_0 (0 )); - val =DATA_TYPE_RAW_10BIT << TPG_DT_n_CFG_1_DATA_TYPE; + val = format->data_type << TPG_DT_n_CFG_1_DATA_TYPE;writel_relaxed (val, csid->base +CSID_TPG_DT_n_CFG_1 (0 )); val = tg->mode << TPG_DT_n_CFG_2_PAYLOAD_MODE;
drivers/media/platform/qcom/camss/camss-csid-170.c static void csid_configure_stream(struct csid_device *csid, u8 enable) · line 386
Quoted 7 lines
drivers/media/platform/qcom/camss/camss-csid-170.c +2
-2@@ -382,8 +382,8 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val =1 <<RDI_CFG0_BYTE_CNTR_EN ; val |=1 <<RDI_CFG0_FORMAT_MEASURE_EN ; val |=1 <<RDI_CFG0_TIMESTAMP_EN ; - val |=DECODE_FORMAT_PAYLOAD_ONLY <<RDI_CFG0_DECODE_FORMAT ; - val |=DATA_TYPE_RAW_10BIT <<RDI_CFG0_DATA_TYPE ; + val |= format->decode_format <<RDI_CFG0_DECODE_FORMAT ; + val |= format->data_type <<RDI_CFG0_DATA_TYPE ;
I've given it a try on RB3 board (aka db845c plus the navigation mezzanine), which uses ov8856 camera sensor (its output format is SGRBG10_1X10).
The above change doesn't work for me because format->decode_format has the value of 0x02 (which is DECODE_FORMAT_UNCOMPRESSED_10_BIT). format->data_type has the expected value of 0x2b (DATA_TYPE_RAW_10BIT).
Thanks,
Andrey
Quoted 4 lines
drivers/media/platform/qcom/camss/camss-csid-170.c +0
-0val |= vc <<RDI_CFG0_VIRTUAL_CHANNEL ; val |= dt_id <<RDI_CFG0_DT_ID ;writel_relaxed (val, csid->base +CSID_RDI_CFG0 (0 ));
Re: [PATCH 04/17] media: camss: csid-170: fix non-10bit formats
Quoted 39 lines · Andrey Konovalov
Hi Jonathan,
Thank you for your patch!
Quoted 22 lines · Jonathan Marek
Use the decode_format/data_type from the "format" struct instead of a hardcoded 10-bit format.
Fixes eebe6d00e9bf ("media: camss: Add support for CSID hardware version Titan 170")Signed-off-by Jonathan Marek <[email protected]>---
drivers/media/platform/qcom/camss/camss-csid-170.c| 6 +++---1 file changed, 3 insertions(+), 3 deletions(-)drivers/media/platform/qcom/camss/camss-csid-170.c +1
-1@@ -366,7 +366,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val |= input_format->width & 0x1fff << TPG_DT_n_CFG_0_FRAME_WIDTH; writel_relaxed(val, csid->base + CSID_TPG_DT_n_CFG_0(0)); - val = DATA_TYPE_RAW_10BIT << TPG_DT_n_CFG_1_DATA_TYPE; + val = format->data_type << TPG_DT_n_CFG_1_DATA_TYPE; writel_relaxed(val, csid->base + CSID_TPG_DT_n_CFG_1(0)); val = tg->mode << TPG_DT_n_CFG_2_PAYLOAD_MODE;drivers/media/platform/qcom/camss/camss-csid-170.c static void csid_configure_stream(struct · line 387
Quoted 8 lines
drivers/media/platform/qcom/camss/camss-csid-170.c +2
-2@@ -382,8 +382,8 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val = 1 << RDI_CFG0_BYTE_CNTR_EN; val |= 1 << RDI_CFG0_FORMAT_MEASURE_EN; val |= 1 << RDI_CFG0_TIMESTAMP_EN; - val |= DECODE_FORMAT_PAYLOAD_ONLY << RDI_CFG0_DECODE_FORMAT; - val |= DATA_TYPE_RAW_10BIT << RDI_CFG0_DATA_TYPE; + val |= format->decode_format << RDI_CFG0_DECODE_FORMAT; + val |= format->data_type << RDI_CFG0_DATA_TYPE;I've given it a try on RB3 board (aka db845c plus the navigation mezzanine), which uses ov8856 camera sensor (its output format is
SGRBG10_1X10).
Quoted 3 lines
The above change doesn't work for me because
format->decode_formathas the value of0x02(which isDECODE_FORMAT_UNCOMPRESSED_10_BIT).format->data_typehas the expected value of0x2b(DATA_TYPE_RAW_10BIT).
I will change it back to using DECODE_FORMAT_PAYLOAD_ONLY for the v2, since it does seem like this is the correct value for the RDI path.
(but IIRC, using DECODE_FORMAT_UNCOMPRESSED_10_BIT worked on RB3 with the ov8856 camera last year when I brought it up. maybe the VFE or another register is configured differently)
Quoted 7 lines
Thanks,
AndreyQuoted 4 lines
drivers/media/platform/qcom/camss/camss-csid-170.c +0
-0val |= vc << RDI_CFG0_VIRTUAL_CHANNEL; val |= dt_id << RDI_CFG0_DT_ID;
writel_relaxed(val, csid->base + CSID_RDI_CFG0(0));
Re: [PATCH 04/17] media: camss: csid-170: fix non-10bit formats
Hi Jonathan,
Quoted 38 lines · Jonathan Marek
Quoted 37 lines · Andrey Konovalov
Hi Jonathan,
Thank you for your patch!
Quoted 21 lines · Jonathan Marek
Use the decode_format/data_type from the "format" struct instead of a hardcoded 10-bit format.
Fixes eebe6d00e9bf ("media: camss: Add support for CSID hardware version Titan 170")Signed-off-by Jonathan Marek <[email protected]>---
drivers/media/platform/qcom/camss/camss-csid-170.c| 6 +++---1 file changed, 3 insertions(+), 3 deletions(-)drivers/media/platform/qcom/camss/camss-csid-170.c +1
-1@@ -366,7 +366,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val |= input_format->width & 0x1fff << TPG_DT_n_CFG_0_FRAME_WIDTH; writel_relaxed(val, csid->base + CSID_TPG_DT_n_CFG_0(0)); - val = DATA_TYPE_RAW_10BIT << TPG_DT_n_CFG_1_DATA_TYPE; + val = format->data_type << TPG_DT_n_CFG_1_DATA_TYPE; writel_relaxed(val, csid->base + CSID_TPG_DT_n_CFG_1(0)); val = tg->mode << TPG_DT_n_CFG_2_PAYLOAD_MODE;drivers/media/platform/qcom/camss/camss-csid-170.c static void csid_configure_stream(struct csid_device *csid, u8 enable) · line 386
Quoted 7 lines
drivers/media/platform/qcom/camss/camss-csid-170.c +2
-2@@ -382,8 +382,8 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val = 1 << RDI_CFG0_BYTE_CNTR_EN; val |= 1 << RDI_CFG0_FORMAT_MEASURE_EN; val |= 1 << RDI_CFG0_TIMESTAMP_EN; - val |= DECODE_FORMAT_PAYLOAD_ONLY << RDI_CFG0_DECODE_FORMAT; - val |= DATA_TYPE_RAW_10BIT << RDI_CFG0_DATA_TYPE; + val |= format->decode_format << RDI_CFG0_DECODE_FORMAT; + val |= format->data_type << RDI_CFG0_DATA_TYPE;I've given it a try on RB3 board (aka db845c plus the navigation mezzanine), which uses ov8856 camera sensor (its output format is
SGRBG10_1X10).
Quoted 5 lines
Quoted 3 lines
The above change doesn't work for me because
format->decode_formathas the value of0x02(which isDECODE_FORMAT_UNCOMPRESSED_10_BIT).format->data_typehas the expected value of0x2b(DATA_TYPE_RAW_10BIT).I will change it back to using
DECODE_FORMAT_PAYLOAD_ONLYfor the v2, since it does seem like this is the correct value for the RDI path.
Sounds good. Just in case, I've tried
drivers/media/platform/qcom/camss/camss-csid-170.c +1 -1
@@ -390,7 +390,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val =1 <<RDI_CFG0_BYTE_CNTR_EN ; val |=1 <<RDI_CFG0_FORMAT_MEASURE_EN ; val |=1 <<RDI_CFG0_TIMESTAMP_EN ; - val |= format->decode_format <<RDI_CFG0_DECODE_FORMAT ; + val |=DECODE_FORMAT_PAYLOAD_ONLY <<RDI_CFG0_DECODE_FORMAT ; val |= format->data_type <<RDI_CFG0_DATA_TYPE ; val |= vc <<RDI_CFG0_VIRTUAL_CHANNEL ; val |= dt_id <<RDI_CFG0_DT_ID ;
on top of v1 patchset, and it worked OK on RB3 with the ov8856 camera.
I planned to check the other RB3's camera too - the "tracking" ov7251 one, but it turned out that I don't have the information on which particular SOC's MIPI lanes are connected to this camera. I've tried a few clock-lanes/data-lanes combinations, but none worked (ov7251 is controlled OK via the i2c bus, but no data are captured).
Quoted 1 line
(but IIRC, using
DECODE_FORMAT_UNCOMPRESSED_10_BITworked on RB3 with the ov8856 camera last year when I brought it up. maybe the VFE or another register is configured differently)
OK. I only used the current upstream camss driver, so have nothing to compare it against.
I also noticed that with ov8856 camera, only 3264x2448 and 1632x1224 modes work correctly on RB3 (used the current media_tree). But with the below change all the other modes (3280x2464 and 1640x1232) start working OK too:
drivers/media/platform/qcom/camss/camss-vfe.c +1 -1
@@ -1575,7 +1575,7 @@ int msm_vfe_register_entities(struct vfe_device *vfe, } video_out->ops = &vfe->video_ops; - video_out->bpl_alignment =8 ; + video_out->bpl_alignment =16 ; video_out->line_based =0 ;if (i ==VFE_LINE_PIX ) { video_out->bpl_alignment =16 ;
And (with the increased size of the capture buffer) I did see that the data captured from camss were padded to the next 16-byte boundary with the data from the previous 16-byte chunk. E.g. in the 3280x2464 case 12 padding bytes were added.
Personally I don't have access to the datasheet, but I've been told that as per the docs, 16-byte alignment should not be needed in this case (RAW10P camera connected through rdi).
Does it ring any bell for you?
Thanks,
Andrey
Quoted 7 lines
Quoted 7 lines
Thanks,
AndreyQuoted 4 lines
drivers/media/platform/qcom/camss/camss-csid-170.c +0
-0val |= vc << RDI_CFG0_VIRTUAL_CHANNEL; val |= dt_id << RDI_CFG0_DT_ID; writel_relaxed(val, csid->base + CSID_RDI_CFG0(0));
PATCH 5/17 media: camss: csid-170: don't enable unused irqs
In reply to Jonathan Marekcsid_isr() only checks for the reset irq, so enabling any other irqs doesn't make sense. The "RDI irq" comment is also wrong, the register should be CSID_CSI2_RDIN_IRQ_MASK. Without this fix there may be an excessive amount of irqs.
--- drivers/media/platform/qcom/camss/camss-csid-170.c | 6 ------ 1 file changed, 6 deletions(-)
drivers/media/platform/qcom/camss/camss-csid-170.c +0 -6
@@ -443,12 +443,6 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val |=1 <<CSI2_RX_CFG1_MISR_EN ;writel_relaxed (val, csid->base +CSID_CSI2_RX_CFG1 );// csi2_vc_mode_shift_val ? -/* error irqs start at BIT(11) */ -writel_relaxed (~0u , csid->base +CSID_CSI2_RX_IRQ_MASK ); - -/* RDI irq */ -writel_relaxed (~0u , csid->base +CSID_TOP_IRQ_MASK ); - val =1 <<RDI_CTRL_HALT_CMD ;writel_relaxed (val, csid->base +CSID_RDI_CTRL (0 )); }
Signature · 1 line
2.26.1
Re: [PATCH 05/17] media: camss: csid-170: don't enable unused irqs
Quoted 19 lines · Jonathan Marek
csid_isr()only checks for the reset irq, so enabling any other irqs doesn't make sense. The "RDI irq" comment is also wrong, the register should beCSID_CSI2_RDIN_IRQ_MASK. Without this fix there may be an excessive amount of irqs.Fixes eebe6d00e9bf ("media: camss: Add support for CSID hardware version Titan 170")Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/camss-csid-170.c | 6 ------ 1 file changed, 6 deletions(-)drivers/media/platform/qcom/camss/camss-csid-170.c +0
-0@@ -443,12 +443,6 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val |=1 <<CSI2_RX_CFG1_MISR_EN ;writel_relaxed (val, csid->base +CSID_CSI2_RX_CFG1 );// csi2_vc_mode_shift_val ?
drivers/media/platform/qcom/camss/camss-csid-170.c static void csid_configure_stream(struct csid_device *csid, u8 enable) · line 448
Quoted 10 lines
drivers/media/platform/qcom/camss/camss-csid-170.c +0
-6-/* error irqs start at BIT(11) */ -writel_relaxed (~0u , csid->base +CSID_CSI2_RX_IRQ_MASK ); - -/* RDI irq */ -writel_relaxed (~0u , csid->base +CSID_TOP_IRQ_MASK ); - val =1 <<RDI_CTRL_HALT_CMD ;writel_relaxed (val, csid->base +CSID_RDI_CTRL (0 )); }
PATCH 6/17 media: camss: csid-170: remove stray comment
In reply to Jonathan MarekThis is a leftover from my original patches, it doesn't serve any purpose. (it was a reminder to figure out how downstream sets a particular field in the register).
--- drivers/media/platform/qcom/camss/camss-csid-170.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/platform/qcom/camss/camss-csid-170.c +1 -1
@@ -441,7 +441,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val =1 <<CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN ; val |=1 <<CSI2_RX_CFG1_MISR_EN ; -writel_relaxed (val, csid->base +CSID_CSI2_RX_CFG1 );// csi2_vc_mode_shift_val ? +writel_relaxed (val, csid->base +CSID_CSI2_RX_CFG1 ); val =1 <<RDI_CTRL_HALT_CMD ;writel_relaxed (val, csid->base +CSID_RDI_CTRL (0 ));
Signature · 1 line
2.26.1
Re: [PATCH 06/17] media: camss: csid-170: remove stray comment
Quoted 16 lines · Jonathan Marek
This is a leftover from my original patches, it doesn't serve any purpose. (it was a reminder to figure out how downstream sets a particular field in the register).
Fixes eebe6d00e9bf ("media: camss: Add support for CSID hardware version Titan 170")Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/camss-csid-170.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/media/platform/qcom/camss/camss-csid-170.c +0
-0
drivers/media/platform/qcom/camss/camss-csid-170.c static void csid_configure_stream(struct csid_device *csid, u8 enable) · line 447
Quoted 8 lines
drivers/media/platform/qcom/camss/camss-csid-170.c +1
-1@@ -441,7 +441,7 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val =1 <<CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN ; val |=1 <<CSI2_RX_CFG1_MISR_EN ; -writel_relaxed (val, csid->base +CSID_CSI2_RX_CFG1 );// csi2_vc_mode_shift_val ? +writel_relaxed (val, csid->base +CSID_CSI2_RX_CFG1 ); val =1 <<RDI_CTRL_HALT_CMD ;writel_relaxed (val, csid->base +CSID_RDI_CTRL (0 ));
PATCH 7/17 media: camss: csid-170: support more than one lite vfe
In reply to Jonathan MarekChange the IS_LITE condition so that it returns true for the second lite vfe found on titan 480 hardware (8250), which will have id == 3.
--- drivers/media/platform/qcom/camss/camss-csid-170.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/platform/qcom/camss/camss-csid-170.c +1 -1
@@ -21,7 +21,7 @@* interface support. As a result of that it has an * alternate register layout. */ -#define IS_LITE (csid->id == 2 ? 1 : 0) +#define IS_LITE (csid->id >= 2 ? 1 : 0)#define CSID_HW_VERSION 0x0#define HW_VERSION_STEPPING 0
Signature · 1 line
2.26.1
Re: [PATCH 07/17] media: camss: csid-170: support more than one lite vfe
Quoted 14 lines · Jonathan Marek
Change the
IS_LITEcondition so that it returns true for the second lite vfe found on titan 480 hardware (8250), which will have id == 3.Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/camss-csid-170.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/media/platform/qcom/camss/camss-csid-170.c +0
-0
drivers/media/platform/qcom/camss/camss-csid-170.c line 27
Quoted 8 lines
drivers/media/platform/qcom/camss/camss-csid-170.c +1
-1@@ -21,7 +21,7 @@* interface support. As a result of that it has an * alternate register layout. */ -#define IS_LITE (csid->id == 2 ? 1 : 0) +#define IS_LITE (csid->id >= 2 ? 1 : 0)#define CSID_HW_VERSION 0x0#define HW_VERSION_STEPPING 0
PATCH 8/17 media: camss: csid-170: set the right HALF_CMD when disabled
In reply to Jonathan MarekUse the "HALT_CMD_RESUME_AT_FRAME_BOUNDARY" define instead of a "1" which is otherwise confusing, and add a "HALT_CMD_HALT_AT_FRAME_BOUNDARY" which is set when disabling.
--- drivers/media/platform/qcom/camss/camss-csid-170.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/media/platform/qcom/camss/camss-csid-170.c +6 -2
@@ -105,7 +105,8 @@#define CSID_RDI_CTRL (rdi) ((IS_LITE ? 0x208 : 0x308)\ + 0x100 * (rdi))#define RDI_CTRL_HALT_CMD 0 -#define ALT_CMD_RESUME_AT_FRAME_BOUNDARY 1 +#define HALT_CMD_HALT_AT_FRAME_BOUNDARY 0 +#define HALT_CMD_RESUME_AT_FRAME_BOUNDARY 1#define RDI_CTRL_HALT_MODE 2#define CSID_RDI_FRM_DROP_PATTERN (rdi) ((IS_LITE ? 0x20C : 0x30C)\
@@ -443,7 +444,10 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val |=1 <<CSI2_RX_CFG1_MISR_EN ;writel_relaxed (val, csid->base +CSID_CSI2_RX_CFG1 ); - val =1 <<RDI_CTRL_HALT_CMD ; +if (enable) + val =HALT_CMD_RESUME_AT_FRAME_BOUNDARY <<RDI_CTRL_HALT_CMD ; +else + val =HALT_CMD_HALT_AT_FRAME_BOUNDARY <<RDI_CTRL_HALT_CMD ;writel_relaxed (val, csid->base +CSID_RDI_CTRL (0 )); }
Signature · 1 line
2.26.1
Re: [PATCH 08/17] media: camss: csid-170: set the right HALF_CMD when disabled
Hey Jonathan,
Quoted 26 lines · Jonathan Marek
Use the "
HALT_CMD_RESUME_AT_FRAME_BOUNDARY" define instead of a "1" which is otherwise confusing, and add a "HALT_CMD_HALT_AT_FRAME_BOUNDARY" which is set when disabling.Fixes eebe6d00e9bf ("media: camss: Add support for CSID hardware version Titan 170")Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/camss-csid-170.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)drivers/media/platform/qcom/camss/camss-csid-170.c +2
-1@@ -105,7 +105,8 @@#define CSID_RDI_CTRL (rdi) ((IS_LITE ? 0x208 : 0x308)\ + 0x100 * (rdi))#define RDI_CTRL_HALT_CMD 0 -#define ALT_CMD_RESUME_AT_FRAME_BOUNDARY 1 +#define HALT_CMD_HALT_AT_FRAME_BOUNDARY 0 +#define HALT_CMD_RESUME_AT_FRAME_BOUNDARY 1#define RDI_CTRL_HALT_MODE 2#define CSID_RDI_FRM_DROP_PATTERN (rdi) ((IS_LITE ? 0x20C : 0x30C)\
drivers/media/platform/qcom/camss/camss-csid-170.c static void csid_configure_stream(struct csid_device *csid, u8 enable) · line 451
Quoted 9 lines
drivers/media/platform/qcom/camss/camss-csid-170.c +4
-1@@ -443,7 +444,10 @@ static void csid_configure_stream(struct csid_device *csid, u8 enable) val |=1 <<CSI2_RX_CFG1_MISR_EN ;writel_relaxed (val, csid->base +CSID_CSI2_RX_CFG1 ); - val =1 <<RDI_CTRL_HALT_CMD ; +if (enable) + val =HALT_CMD_RESUME_AT_FRAME_BOUNDARY <<RDI_CTRL_HALT_CMD ; +else + val =HALT_CMD_HALT_AT_FRAME_BOUNDARY <<RDI_CTRL_HALT_CMD ;writel_relaxed (val, csid->base +CSID_RDI_CTRL (0 ));
PATCH 9/17 media: camss: csid: allow csid to work without a regulator
In reply to Jonathan MarekAt least for titan HW, CSID don't have an associated regulator. This change is necessary to be able to model this in the CSID resources.
--- drivers/media/platform/qcom/camss/camss-csid.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
drivers/media/platform/qcom/camss/camss-csid.c +11 -6
@@ -162,7 +162,7 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)return ret; } - ret =regulator_enable (csid->vdda); + ret = csid->vdda ?regulator_enable (csid->vdda) :0 ;if (ret <0 ) {pm_runtime_put_sync (dev);return ret;
@@ -170,14 +170,16 @@ static int csid_set_power(struct v4l2_subdev *sd, int on) ret =csid_set_clock_rates (csid);if (ret <0 ) { -regulator_disable (csid->vdda); +if (csid->vdda) +regulator_disable (csid->vdda);pm_runtime_put_sync (dev);return ret; } ret =camss_enable_clocks (csid->nclocks, csid->clock, dev);if (ret <0 ) { -regulator_disable (csid->vdda); +if (csid->vdda) +regulator_disable (csid->vdda);pm_runtime_put_sync (dev);return ret; }
@@ -188,7 +190,8 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)if (ret <0 ) {disable_irq (csid->irq);camss_disable_clocks (csid->nclocks, csid->clock); -regulator_disable (csid->vdda); +if (csid->vdda) +regulator_disable (csid->vdda);pm_runtime_put_sync (dev);return ret; }
@@ -197,7 +200,7 @@ static int csid_set_power(struct v4l2_subdev *sd, int on) }else {disable_irq (csid->irq);camss_disable_clocks (csid->nclocks, csid->clock); - ret =regulator_disable (csid->vdda); + ret = csid->vdda ?regulator_disable (csid->vdda) :0 ;pm_runtime_put_sync (dev); }
@@ -634,7 +637,9 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid,/* Regulator */ - csid->vdda =devm_regulator_get (dev, res->regulator[0 ]); + csid->vdda =NULL ; +if (res->regulator[0 ]) + csid->vdda =devm_regulator_get (dev, res->regulator[0 ]);if (IS_ERR (csid->vdda)) {dev_err (dev,"could not get regulator\n" );return PTR_ERR (csid->vdda);
Signature · 1 line
2.26.1
Re: [PATCH 09/17] media: camss: csid: allow csid to work without a regulator
Quoted 42 lines · Jonathan Marek
At least for titan HW, CSID don't have an associated regulator. This change is necessary to be able to model this in the CSID resources.
Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/camss-csid.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)drivers/media/platform/qcom/camss/camss-csid.c +5
-3@@ -162,7 +162,7 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)return ret; } - ret =regulator_enable (csid->vdda); + ret = csid->vdda ?regulator_enable (csid->vdda) :0 ;if (ret <0 ) {pm_runtime_put_sync (dev);return ret;@@ -170,14 +170,16 @@ static int csid_set_power(struct v4l2_subdev *sd, int on) ret =csid_set_clock_rates (csid);if (ret <0 ) { -regulator_disable (csid->vdda); +if (csid->vdda) +regulator_disable (csid->vdda);pm_runtime_put_sync (dev);return ret; } ret =camss_enable_clocks (csid->nclocks, csid->clock, dev);if (ret <0 ) { -regulator_disable (csid->vdda); +if (csid->vdda) +regulator_disable (csid->vdda);pm_runtime_put_sync (dev);return ret; }
drivers/media/platform/qcom/camss/camss-csid.c static int csid_set_power(struct v4l2_subdev *sd, int on) · line 196
Quoted 8 lines
drivers/media/platform/qcom/camss/camss-csid.c +2
-1@@ -188,7 +190,8 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)if (ret <0 ) {disable_irq (csid->irq);camss_disable_clocks (csid->nclocks, csid->clock); -regulator_disable (csid->vdda); +if (csid->vdda) +regulator_disable (csid->vdda);pm_runtime_put_sync (dev);return ret;
Since this & the previous chunks of failure cleanups are growing larger, maybe it is time to extract all of the failure cleanups into gotos. That should probably go into a seperate patch though.
Quoted 11 lines
drivers/media/platform/qcom/camss/camss-csid.c +1
-1}@@ -197,7 +200,7 @@ static int csid_set_power(struct v4l2_subdev *sd, int on) }else {disable_irq (csid->irq);camss_disable_clocks (csid->nclocks, csid->clock); - ret =regulator_disable (csid->vdda); + ret = csid->vdda ?regulator_disable (csid->vdda) :0 ;pm_runtime_put_sync (dev); }
drivers/media/platform/qcom/camss/camss-csid.c int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid, · line 645
Quoted 10 lines
drivers/media/platform/qcom/camss/camss-csid.c +3
-1@@ -634,7 +637,9 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid,/* Regulator */ - csid->vdda =devm_regulator_get (dev, res->regulator[0 ]); + csid->vdda =NULL ; +if (res->regulator[0 ]) + csid->vdda =devm_regulator_get (dev, res->regulator[0 ]);if (IS_ERR (csid->vdda)) {dev_err (dev,"could not get regulator\n" );return PTR_ERR (csid->vdda);
PATCH 10/17 media: camss: remove vdda-csiN from sdm845 resources
In reply to Jonathan MarekThis isn't used and only works because devm_regulator_get() returns a dummy regulator.
--- drivers/media/platform/qcom/camss/camss.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/media/platform/qcom/camss/camss.c +3 -3
@@ -542,7 +542,7 @@ static const struct resources csiphy_res_845[] = {static const struct resources csid_res_845[] = {/* CSID0 */ { - .regulator = {"vdda-csi0" }, + .regulator = {NULL }, .clock = {"cpas_ahb" ,"cphy_rx_src" ,"slow_ahb_src" ,"soc_ahb" ,"vfe0" ,"vfe0_src" ,"vfe0_cphy_rx" ,"csi0" ,
@@ -562,7 +562,7 @@ static const struct resources csid_res_845[] = {/* CSID1 */ { - .regulator = { "vdda-csi1" }, + .regulator = { NULL }, .clock = { "cpas_ahb", "cphy_rx_src", "slow_ahb_src","soc_ahb" ,"vfe1" ,"vfe1_src" , "vfe1_cphy_rx", "csi1",
@@ -582,7 +582,7 @@ static const struct resources csid_res_845[] = {/* CSID2 */ { - .regulator = { "vdda-csi2" }, + .regulator = { NULL }, .clock = { "cpas_ahb", "cphy_rx_src", "slow_ahb_src","soc_ahb" ,"vfe_lite" ,"vfe_lite_src" , "vfe_lite_cphy_rx", "csi2",
Signature · 1 line
2.26.1
Re: [PATCH 10/17] media: camss: remove vdda-csiN from sdm845 resources
Quoted 32 lines · Jonathan Marek
This isn't used and only works because
devm_regulator_get()returns a dummy regulator.Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/camss.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)drivers/media/platform/qcom/camss/camss.c +2
-2@@ -542,7 +542,7 @@ static const struct resources csiphy_res_845[] = {static const struct resources csid_res_845[] = {/* CSID0 */ { - .regulator = {"vdda-csi0" }, + .regulator = {NULL }, .clock = {"cpas_ahb" ,"cphy_rx_src" ,"slow_ahb_src" ,"soc_ahb" ,"vfe0" ,"vfe0_src" ,"vfe0_cphy_rx" ,"csi0" ,@@ -562,7 +562,7 @@ static const struct resources csid_res_845[] = {/* CSID1 */ { - .regulator = { "vdda-csi1" }, + .regulator = { NULL }, .clock = { "cpas_ahb", "cphy_rx_src", "slow_ahb_src","soc_ahb" ,"vfe1" ,"vfe1_src" , "vfe1_cphy_rx", "csi1",
drivers/media/platform/qcom/camss/camss.c static const struct resources csid_res_845[] = { · line 588
Quoted 8 lines
drivers/media/platform/qcom/camss/camss.c +1
-1@@ -582,7 +582,7 @@ static const struct resources csid_res_845[] = {/* CSID2 */ { - .regulator = { "vdda-csi2" }, + .regulator = { NULL }, .clock = { "cpas_ahb", "cphy_rx_src", "slow_ahb_src","soc_ahb" ,"vfe_lite" ,"vfe_lite_src" , "vfe_lite_cphy_rx", "csi2",
PATCH 11/17 media: camss: fix VFE irq name
In reply to Jonathan Marekvfe->id isn't set yet, so use "id" instead here.
--- drivers/media/platform/qcom/camss/camss-vfe.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/media/platform/qcom/camss/camss-vfe.c +1 -2
@@ -1290,7 +1290,6 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,case CAMSS_660 : vfe->ops = &vfe_ops_4_8;break ; -case CAMSS_845 : vfe->ops = &vfe_ops_170;break ;
@@ -1319,7 +1318,7 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe, vfe->irq = r->start;snprintf (vfe->irq_name,sizeof (vfe->irq_name),"%s_%s%d" , -dev_name (dev),MSM_VFE_NAME , vfe->id); +dev_name (dev),MSM_VFE_NAME , id); ret =devm_request_irq (dev, vfe->irq, vfe->ops->isr,IRQF_TRIGGER_RISING , vfe->irq_name, vfe);if (ret <0 ) {
Signature · 1 line
2.26.1
Re: [PATCH 11/17] media: camss: fix VFE irq name
Quoted 21 lines · Jonathan Marek
vfe->idisn't set yet, so use "id" instead here.Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/camss-vfe.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/media/platform/qcom/camss/camss-vfe.c +0
-1@@ -1290,7 +1290,6 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,case CAMSS_660 : vfe->ops = &vfe_ops_4_8;break ; -case CAMSS_845 : vfe->ops = &vfe_ops_170;break ;
drivers/media/platform/qcom/camss/camss-vfe.c int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe, · line 1324
Quoted 8 lines
drivers/media/platform/qcom/camss/camss-vfe.c +1
-1@@ -1319,7 +1318,7 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe, vfe->irq = r->start;snprintf (vfe->irq_name,sizeof (vfe->irq_name),"%s_%s%d" , -dev_name (dev),MSM_VFE_NAME , vfe->id); +dev_name (dev),MSM_VFE_NAME , id); ret =devm_request_irq (dev, vfe->irq, vfe->ops->isr,IRQF_TRIGGER_RISING , vfe->irq_name, vfe);if (ret <0 ) {
PATCH 12/17 media: camss: vfe-170: clean up some dead code
In reply to Jonathan Marekvfe_isr_read()/vfe_isr_halt_ack()/vfe_reg_clr() are never called.
vfe_isr_sof() does nothing, remove it.
The only vfe_reg_set() usage can be easily replaced with a writel.
--- .../media/platform/qcom/camss/camss-vfe-170.c | 53 +------------------ 1 file changed, 2 insertions(+), 51 deletions(-)
drivers/media/platform/qcom/camss/camss-vfe-170.c +2 -51
@@ -188,20 +188,6 @@ static void vfe_hw_version_read(struct vfe_device *vfe, struct device *dev)dev_err (dev,"VFE HW Version = %u.%u.%u\n" , gen, rev, step); } -static inline void vfe_reg_clr (struct vfe_device *vfe,u32 reg,u32 clr_bits) -{ -u32 bits =readl_relaxed (vfe->base + reg); - -writel_relaxed (bits & ~clr_bits, vfe->base + reg); -} - -static inline void vfe_reg_set (struct vfe_device *vfe,u32 reg,u32 set_bits) -{ -u32 bits =readl_relaxed (vfe->base + reg); - -writel_relaxed (bits | set_bits, vfe->base + reg); -} -static void vfe_global_reset (struct vfe_device *vfe) { u32 reset_bits = GLOBAL_RESET_CMD_CORE |
@@ -305,32 +291,14 @@ static inline void vfe_reg_update_clear(struct vfe_device *vfe,static void vfe_enable_irq_common (struct vfe_device *vfe) { -vfe_reg_set (vfe,VFE_IRQ_MASK_0 , ~0u ); -vfe_reg_set (vfe,VFE_IRQ_MASK_1 , ~0u ); +writel_relaxed (~0u , vfe->base +VFE_IRQ_MASK_0 ); +writel_relaxed (~0u , vfe->base +VFE_IRQ_MASK_1 );writel_relaxed (~0u , vfe->base +VFE_BUS_IRQ_MASK (0 ));writel_relaxed (~0u , vfe->base +VFE_BUS_IRQ_MASK (1 ));writel_relaxed (~0u , vfe->base +VFE_BUS_IRQ_MASK (2 )); } -static void vfe_isr_halt_ack (struct vfe_device *vfe) -{ -complete (&vfe->halt_complete); -} - -static void vfe_isr_read (struct vfe_device *vfe,u32 *status0,u32 *status1) -{ - *status0 =readl_relaxed (vfe->base +VFE_IRQ_STATUS_0 ); - *status1 =readl_relaxed (vfe->base +VFE_IRQ_STATUS_1 ); - -writel_relaxed (*status0, vfe->base +VFE_IRQ_CLEAR_0 ); -writel_relaxed (*status1, vfe->base +VFE_IRQ_CLEAR_1 ); - -/* Enforce ordering between IRQ Clear and Global IRQ Clear */ -wmb (); -writel_relaxed (CMD_GLOBAL_CLEAR , vfe->base +VFE_IRQ_CMD ); -} -static void vfe_violation_read (struct vfe_device *vfe) {u32 violation =readl_relaxed (vfe->base +VFE_VIOLATION_STATUS );
@@ -375,10 +343,6 @@ static irqreturn_t vfe_isr(int irq, void *dev)if (status0 &STATUS_0_RDI_REG_UPDATE (i)) vfe->isr_ops.reg_update (vfe, i); -for (i =VFE_LINE_RDI0 ; i <=VFE_LINE_RDI2 ; i++) -if (status0 &STATUS_1_RDI_SOF (i)) - vfe->isr_ops.sof (vfe, i); -for (i =0 ; i <MSM_VFE_COMPOSITE_IRQ_NUM ; i++)if (vfe_bus_status[0 ] &STATUS0_COMP_BUF_DONE (i)) vfe->isr_ops.comp_done (vfe, i);
@@ -607,16 +571,6 @@ static int vfe_disable(struct vfe_line *line)return 0 ; } -/* -* vfe_isr_sof - Process start of frame interrupt -* @vfe: VFE Device -* @line_id: VFE line -*/ -static void vfe_isr_sof (struct vfe_device *vfe,enum vfe_line_id line_id) -{ -/* nop */ -} -/* * vfe_isr_reg_update - Process reg update interrupt * @vfe: VFE Device
@@ -749,9 +703,7 @@ static int vfe_queue_buffer(struct camss_video *vid,static const struct vfe_isr_ops vfe_isr_ops_170 = { .reset_ack = vfe_isr_reset_ack, - .halt_ack = vfe_isr_halt_ack, .reg_update = vfe_isr_reg_update, - .sof = vfe_isr_sof, .comp_done = vfe_isr_comp_done, .wm_done = vfe_isr_wm_done, };
@@ -772,7 +724,6 @@ static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)const struct vfe_hw_ops vfe_ops_170 = { .global_reset = vfe_global_reset, .hw_version_read = vfe_hw_version_read, - .isr_read = vfe_isr_read, .isr = vfe_isr, .pm_domain_off = vfe_pm_domain_off, .pm_domain_on = vfe_pm_domain_on,
Signature · 1 line
2.26.1
Re: [PATCH 12/17] media: camss: vfe-170: clean up some dead code
Quoted 112 lines · Jonathan Marek
vfe_isr_read()/vfe_isr_halt_ack()/vfe_reg_clr() are never called.
vfe_isr_sof()does nothing, remove it.The only
vfe_reg_set()usage can be easily replaced with a writel.Fixes 7319cdf189bb ("media: camss: Add support for VFE hardware version Titan 170")Signed-off-by Jonathan Marek <[email protected]>--- .../media/platform/qcom/camss/camss-vfe-170.c | 53 +------------------ 1 file changed, 2 insertions(+), 51 deletions(-)drivers/media/platform/qcom/camss/camss-vfe-170.c +2
-50@@ -188,20 +188,6 @@ static void vfe_hw_version_read(struct vfe_device *vfe, struct device *dev)dev_err (dev,"VFE HW Version = %u.%u.%u\n" , gen, rev, step); } -static inline void vfe_reg_clr (struct vfe_device *vfe,u32 reg,u32 clr_bits) -{ -u32 bits =readl_relaxed (vfe->base + reg); - -writel_relaxed (bits & ~clr_bits, vfe->base + reg); -} - -static inline void vfe_reg_set (struct vfe_device *vfe,u32 reg,u32 set_bits) -{ -u32 bits =readl_relaxed (vfe->base + reg); - -writel_relaxed (bits | set_bits, vfe->base + reg); -} -static void vfe_global_reset (struct vfe_device *vfe) { u32 reset_bits = GLOBAL_RESET_CMD_CORE |@@ -305,32 +291,14 @@ static inline void vfe_reg_update_clear(struct vfe_device *vfe,static void vfe_enable_irq_common (struct vfe_device *vfe) { -vfe_reg_set (vfe,VFE_IRQ_MASK_0 , ~0u ); -vfe_reg_set (vfe,VFE_IRQ_MASK_1 , ~0u ); +writel_relaxed (~0u , vfe->base +VFE_IRQ_MASK_0 ); +writel_relaxed (~0u , vfe->base +VFE_IRQ_MASK_1 );writel_relaxed (~0u , vfe->base +VFE_BUS_IRQ_MASK (0 ));writel_relaxed (~0u , vfe->base +VFE_BUS_IRQ_MASK (1 ));writel_relaxed (~0u , vfe->base +VFE_BUS_IRQ_MASK (2 )); } -static void vfe_isr_halt_ack (struct vfe_device *vfe) -{ -complete (&vfe->halt_complete); -} - -static void vfe_isr_read (struct vfe_device *vfe,u32 *status0,u32 *status1) -{ - *status0 =readl_relaxed (vfe->base +VFE_IRQ_STATUS_0 ); - *status1 =readl_relaxed (vfe->base +VFE_IRQ_STATUS_1 ); - -writel_relaxed (*status0, vfe->base +VFE_IRQ_CLEAR_0 ); -writel_relaxed (*status1, vfe->base +VFE_IRQ_CLEAR_1 ); - -/* Enforce ordering between IRQ Clear and Global IRQ Clear */ -wmb (); -writel_relaxed (CMD_GLOBAL_CLEAR , vfe->base +VFE_IRQ_CMD ); -} -static void vfe_violation_read (struct vfe_device *vfe) {u32 violation =readl_relaxed (vfe->base +VFE_VIOLATION_STATUS );@@ -375,10 +343,6 @@ static irqreturn_t vfe_isr(int irq, void *dev)if (status0 &STATUS_0_RDI_REG_UPDATE (i)) vfe->isr_ops.reg_update (vfe, i); -for (i =VFE_LINE_RDI0 ; i <=VFE_LINE_RDI2 ; i++) -if (status0 &STATUS_1_RDI_SOF (i)) - vfe->isr_ops.sof (vfe, i); -for (i =0 ; i <MSM_VFE_COMPOSITE_IRQ_NUM ; i++)if (vfe_bus_status[0 ] &STATUS0_COMP_BUF_DONE (i)) vfe->isr_ops.comp_done (vfe, i);@@ -607,16 +571,6 @@ static int vfe_disable(struct vfe_line *line)return 0 ; } -/* -* vfe_isr_sof - Process start of frame interrupt -* @vfe: VFE Device -* @line_id: VFE line -*/ -static void vfe_isr_sof (struct vfe_device *vfe,enum vfe_line_id line_id) -{ -/* nop */ -} -/* * vfe_isr_reg_update - Process reg update interrupt * @vfe: VFE Device @@ -749,9 +703,7 @@ static int vfe_queue_buffer(struct camss_video *vid,static const struct vfe_isr_ops vfe_isr_ops_170 = { .reset_ack = vfe_isr_reset_ack, - .halt_ack = vfe_isr_halt_ack, .reg_update = vfe_isr_reg_update, - .sof = vfe_isr_sof, .comp_done = vfe_isr_comp_done, .wm_done = vfe_isr_wm_done, };
drivers/media/platform/qcom/camss/camss-vfe-170.c static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe) · line 729
Quoted 7 lines
drivers/media/platform/qcom/camss/camss-vfe-170.c +0
-1@@ -772,7 +724,6 @@ static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)const struct vfe_hw_ops vfe_ops_170 = { .global_reset = vfe_global_reset, .hw_version_read = vfe_hw_version_read, - .isr_read = vfe_isr_read, .isr = vfe_isr, .pm_domain_off = vfe_pm_domain_off, .pm_domain_on = vfe_pm_domain_on,
Gen1 & gen2 seem to diverge in terms of which ISR & helpers are needed. Should the vfe_isr_ops & vfe_hw_ops structs have generation based versions? I'm fine with a no here. There are diminishing returns to this pedantism.
With ^^^ covered, feel free to add my r-b.
PATCH 13/17 media: camss: vfe-170: fix "VFE halt timeout" error
In reply to Jonathan MarekThis function waits for halt_complete but doesn't do anything to cause it to complete, and always hits the "VFE halt timeout" error. Just delete this code for now.
--- drivers/media/platform/qcom/camss/camss-vfe-170.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)
drivers/media/platform/qcom/camss/camss-vfe-170.c +1 -11
@@ -363,17 +363,7 @@ static irqreturn_t vfe_isr(int irq, void *dev)*/ static int vfe_halt (struct vfe_device *vfe) { -unsigned long time; - -reinit_completion (&vfe->halt_complete); - - time =wait_for_completion_timeout (&vfe->halt_complete, -msecs_to_jiffies (VFE_HALT_TIMEOUT_MS )); -if (!time) { -dev_err (vfe->camss->dev,"VFE halt timeout\n" ); -return -EIO ; - } - +/* rely on vfe_disable_output() to stop the VFE */ return 0 ; }
Signature · 1 line
2.26.1
Re: [PATCH 13/17] media: camss: vfe-170: fix "VFE halt timeout" error
Quoted 23 lines · Jonathan Marek
This function waits for
halt_completebut doesn't do anything to cause it to complete, and always hits the "VFE halt timeout" error. Just delete this code for now.Fixes 7319cdf189bb ("media: camss: Add support for VFE hardware version Titan 170")Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/camss-vfe-170.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)drivers/media/platform/qcom/camss/camss-vfe-170.c +0
-4@@ -363,17 +363,7 @@ static irqreturn_t vfe_isr(int irq, void *dev)*/ static int vfe_halt (struct vfe_device *vfe) { -unsigned long time; - -reinit_completion (&vfe->halt_complete); -
drivers/media/platform/qcom/camss/camss-vfe-170.c static irqreturn_t vfe_isr(int irq, void *dev) · line 368
Quoted 10 lines
drivers/media/platform/qcom/camss/camss-vfe-170.c +1
-7- time =wait_for_completion_timeout (&vfe->halt_complete, -msecs_to_jiffies (VFE_HALT_TIMEOUT_MS )); -if (!time) { -dev_err (vfe->camss->dev,"VFE halt timeout\n" ); -return -EIO ; - } - +/* rely on vfe_disable_output() to stop the VFE */ return 0 ; }
PATCH 14/17 media: camss: Add initial support for VFE hardware version Titan 480
In reply to Jonathan MarekAdd support for VFE found on SM8250 (Titan 480). This implementation is based on the titan 170 implementation. It supports the normal and lite VFE, and only supports the RDI0 capture path.
--- drivers/media/platform/qcom/camss/Makefile | 1 + .../media/platform/qcom/camss/camss-vfe-480.c | 554 ++++++++++++++++++ drivers/media/platform/qcom/camss/camss-vfe.h | 1 + 3 files changed, 556 insertions(+) create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-480.c
drivers/media/platform/qcom/camss/Makefile +1 -0
@@ -15,6 +15,7 @@ qcom-camss-objs += \ camss-vfe-4-7.o \ camss-vfe-4-8.o \ camss-vfe-170.o \ + camss-vfe-480.o \ camss-vfe-gen1.o \ camss-vfe.o \ camss-video.o \
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +554 -0
554 lines not on this page · raw message
drivers/media/platform/qcom/camss/camss-vfe.h +1 -0
@@ -201,5 +201,6 @@ extern const struct vfe_hw_ops vfe_ops_4_1;extern const struct vfe_hw_ops vfe_ops_4_7;extern const struct vfe_hw_ops vfe_ops_4_8;extern const struct vfe_hw_ops vfe_ops_170; +extern const struct vfe_hw_ops vfe_ops_480;#endif /* QC_MSM_CAMSS_VFE_H */
Signature · 1 line
2.26.1
Re: [PATCH 14/17] media: camss: Add initial support for VFE hardware version Titan 480
Hey Jonathan,
Thanks for sending this out.
There are a few checkpatch --strict warnings/etc. in this patch. I won't cover them individually below.
Quoted 101 lines · Jonathan Marek
Add support for VFE found on SM8250 (Titan 480). This implementation is based on the titan 170 implementation. It supports the normal and lite VFE, and only supports the RDI0 capture path.
Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/Makefile | 1 + .../media/platform/qcom/camss/camss-vfe-480.c | 554 ++++++++++++++++++ drivers/media/platform/qcom/camss/camss-vfe.h | 1 + 3 files changed, 556 insertions(+) create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-480.cdrivers/media/platform/qcom/camss/Makefile +1
-0@@ -15,6 +15,7 @@ qcom-camss-objs += \ camss-vfe-4-7.o \ camss-vfe-4-8.o \ camss-vfe-170.o \ + camss-vfe-480.o \ camss-vfe-gen1.o \ camss-vfe.o \ camss-video.o \drivers/media/platform/qcom/camss/camss-vfe-480.c new file +70
-0@@ -0,0 +1,554 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* camss-vfe-480.c +* +* Qualcomm MSM Camera Subsystem - VFE (Video Front End) Module v480 (SM8250) +* +* Copyright (C) 2020-2021 Linaro Ltd. +* Copyright (C) 2021 Jonathan Marek +*/ + +#include <linux/delay.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/iopoll.h> + +#include "camss.h" +#include "camss-vfe.h" + +/* VFE 2/3 are lite and have a different register layout */ +#define IS_LITE (vfe->id >= 2 ? 1 : 0) + +#define VFE_HW_VERSION (0x00) + +#define VFE_GLOBAL_RESET_CMD (IS_LITE ? 0x0c : 0x1c) +#define GLOBAL_RESET_HW_AND_REG (IS_LITE ? BIT(1) : BIT(0)) + +#define VFE_REG_UPDATE_CMD (IS_LITE ? 0x20 : 0x34) +#define REG_UPDATE_RDI (n) (IS_LITE ? BIT(n) : BIT(1 + (n))) +#define VFE_IRQ_CMD (IS_LITE ? 0x24 : 0x38) +#define IRQ_CMD_GLOBAL_CLEAR BIT(0) + +#define VFE_IRQ_MASK (n) ((IS_LITE ? 0x28 : 0x3c) + (n) * 4) +#define IRQ_MASK_0_RESET_ACK (IS_LITE ? BIT(17) : BIT(0)) +#define IRQ_MASK_0_BUS_TOP_IRQ (IS_LITE ? BIT(4) : BIT(7)) +#define VFE_IRQ_CLEAR (n) ((IS_LITE ? 0x34 : 0x48) + (n) * 4) +#define VFE_IRQ_STATUS (n) ((IS_LITE ? 0x40 : 0x54) + (n) * 4) + +#define BUS_REG_BASE (IS_LITE ? 0x1a00 : 0xaa00) + +#define VFE_BUS_WM_CGC_OVERRIDE (BUS_REG_BASE + 0x08) +#define WM_CGC_OVERRIDE_ALL (0x3FFFFFF) + +#define VFE_BUS_WM_TEST_BUS_CTRL (BUS_REG_BASE + 0xdc) + +#define VFE_BUS_IRQ_MASK (n) (BUS_REG_BASE + 0x18 + (n) * 4) +#define BUS_IRQ_MASK_0_RDI_RUP (n) (IS_LITE ? BIT(n) : BIT(3 + (n))) +#define BUS_IRQ_MASK_0_COMP_DONE (n) (IS_LITE ? BIT(4 + (n)) : BIT(6 + (n))) +#define VFE_BUS_IRQ_CLEAR (n) (BUS_REG_BASE + 0x20 + (n) * 4) +#define VFE_BUS_IRQ_STATUS (n) (BUS_REG_BASE + 0x28 + (n) * 4) +#define VFE_BUS_IRQ_CLEAR_GLOBAL (BUS_REG_BASE + 0x30) + +#define VFE_BUS_WM_CFG (n) (BUS_REG_BASE + 0x200 + (n) * 0x100) +#define WM_CFG_EN (0) +#define WM_CFG_MODE (16) +#define MODE_QCOM_PLAIN (0) +#define MODE_MIPI_RAW (1) +#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_1 (n) (BUS_REG_BASE + 0x210 + (n) * 0x100) +#define VFE_BUS_WM_IMAGE_CFG_2 (n) (BUS_REG_BASE + 0x214 + (n) * 0x100) +#define VFE_BUS_WM_PACKER_CFG (n) (BUS_REG_BASE + 0x218 + (n) * 0x100) +#define VFE_BUS_WM_HEADER_ADDR (n) (BUS_REG_BASE + 0x220 + (n) * 0x100) +#define VFE_BUS_WM_HEADER_INCR (n) (BUS_REG_BASE + 0x224 + (n) * 0x100) +#define VFE_BUS_WM_HEADER_CFG (n) (BUS_REG_BASE + 0x228 + (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)
drivers/media/platform/qcom/camss/camss-vfe-480.c line 80
Quoted 10 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +10
-0+#define VFE_BUS_WM_FRAMEDROP_PATTERN (n) (BUS_REG_BASE + 0x23c + (n) * 0x100) + +#define VFE_BUS_WM_SYSTEM_CACHE_CFG (n) (BUS_REG_BASE + 0x260 + (n) * 0x100) +#define VFE_BUS_WM_BURST_LIMIT (n) (BUS_REG_BASE + 0x264 + (n) * 0x100) + +/* for titan 480, each bus client is hardcoded to a specific path +* and each bus client is part of a hardcoded "comp group" +*/ +#define RDI_WM (n) ((IS_LITE ? 0 : 23) + n) +#define RDI_COMP_GROUP (n) ((IS_LITE ? 0 : 11) + n)
The indentation of the different types of defines above differ from vfe170, but I kind of prefer this style. Feel free to change either.
Quoted 28 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +28
-0+ +static void vfe_hw_version_read (struct vfe_device *vfe,struct device *dev) +{ +u32 hw_version =readl_relaxed (vfe->base +VFE_HW_VERSION ); + +u32 gen = (hw_version >>28 ) &0xF ; +u32 rev = (hw_version >>16 ) &0xFFF ; +u32 step = hw_version &0xFFFF ; + +dev_dbg (dev,"VFE HW Version = %u.%u.%u\n" , gen, rev, step); +} + +static void vfe_global_reset (struct vfe_device *vfe) +{ +writel_relaxed (IRQ_MASK_0_RESET_ACK , vfe->base +VFE_IRQ_MASK (0 )); +writel_relaxed (GLOBAL_RESET_HW_AND_REG , vfe->base +VFE_GLOBAL_RESET_CMD ); +} + +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; + + wm =RDI_WM (wm);/* map to actual WM used (from wm=RDI index) */ + +/* no clock gating at bus input */ +writel_relaxed (WM_CGC_OVERRIDE_ALL , vfe->base +VFE_BUS_WM_CGC_OVERRIDE ); +
drivers/media/platform/qcom/camss/camss-vfe-480.c line 118
Quoted 10 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +10
-0+writel_relaxed (0x0 , vfe->base +VFE_BUS_WM_TEST_BUS_CTRL ); + +writel_relaxed (pix->plane_fmt[0 ].bytesperline * pix->height, + vfe->base +VFE_BUS_WM_FRAME_INCR (wm)); +writel_relaxed (0xf , vfe->base +VFE_BUS_WM_BURST_LIMIT (wm)); +writel_relaxed (WM_IMAGE_CFG_0_DEFAULT_WIDTH , + vfe->base +VFE_BUS_WM_IMAGE_CFG_0 (wm)); +writel_relaxed (pix->plane_fmt[0 ].bytesperline, + vfe->base +VFE_BUS_WM_IMAGE_CFG_2 (wm)); +writel_relaxed (0 , vfe->base +VFE_BUS_WM_PACKER_CFG (wm));
^^^ using more of the 130char line length is probably better for legibility.
Quoted 65 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +65
-0+ +/* no dropped frames, one irq per frame */ +writel_relaxed (0 , vfe->base +VFE_BUS_WM_FRAMEDROP_PERIOD (wm)); +writel_relaxed (1 , vfe->base +VFE_BUS_WM_FRAMEDROP_PATTERN (wm)); +writel_relaxed (0 , vfe->base +VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD (wm)); +writel_relaxed (1 , vfe->base +VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN (wm)); + +writel_relaxed (1 <<WM_CFG_EN |MODE_MIPI_RAW <<WM_CFG_MODE , + vfe->base +VFE_BUS_WM_CFG (wm)); +} + +static void vfe_wm_stop (struct vfe_device *vfe,u8 wm) +{ + wm =RDI_WM (wm);/* map to actual WM used (from wm=RDI index) */ +writel_relaxed (0 , vfe->base +VFE_BUS_WM_CFG (wm)); +} + +static void vfe_wm_update (struct vfe_device *vfe,u8 wm,u32 addr, +struct vfe_line *line) +{ + wm =RDI_WM (wm);/* map to actual WM used (from wm=RDI index) */ +writel_relaxed (addr, vfe->base +VFE_BUS_WM_IMAGE_ADDR (wm)); +} + +static void vfe_reg_update (struct vfe_device *vfe,enum vfe_line_id line_id) +{ + vfe->reg_update |=REG_UPDATE_RDI (line_id); +writel_relaxed (vfe->reg_update, vfe->base +VFE_REG_UPDATE_CMD ); +} + +static inline void vfe_reg_update_clear (struct vfe_device *vfe, +enum vfe_line_id line_id) +{ + vfe->reg_update &= ~REG_UPDATE_RDI (line_id); +} + +static void vfe_enable_irq_common (struct vfe_device *vfe) +{ +/* enable only the IRQs used: rup and comp_done irqs for RDI0 */ +writel_relaxed (IRQ_MASK_0_RESET_ACK |IRQ_MASK_0_BUS_TOP_IRQ , + vfe->base +VFE_IRQ_MASK (0 )); +writel_relaxed (BUS_IRQ_MASK_0_RDI_RUP (0 ) | +BUS_IRQ_MASK_0_COMP_DONE (RDI_COMP_GROUP (0 )), + vfe->base +VFE_BUS_IRQ_MASK (0 )); +} + +/* +* vfe_isr - VFE module interrupt handler +* @irq: Interrupt line +* @dev: VFE device +* +* Return IRQ_HANDLED on success +*/ +static irqreturn_t vfe_isr (int irq,void *dev) +{ +struct vfe_device *vfe = dev; +u32 status; + + status =readl_relaxed (vfe->base +VFE_IRQ_STATUS (0 )); +writel_relaxed (status, vfe->base +VFE_IRQ_CLEAR (0 )); +writel_relaxed (IRQ_CMD_GLOBAL_CLEAR , vfe->base +VFE_IRQ_CMD ); + +if (status &IRQ_MASK_0_RESET_ACK ) + vfe->isr_ops.reset_ack (vfe); +
drivers/media/platform/qcom/camss/camss-vfe-480.c line 193
Quoted 10 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +10
-0+if (status &IRQ_MASK_0_BUS_TOP_IRQ ) { +u32 status =readl_relaxed (vfe->base +VFE_BUS_IRQ_STATUS (0 )); +writel_relaxed (status, vfe->base +VFE_BUS_IRQ_CLEAR (0 )); +writel_relaxed (1 , vfe->base +VFE_BUS_IRQ_CLEAR_GLOBAL ); + +if (status &BUS_IRQ_MASK_0_RDI_RUP (0 )) + vfe->isr_ops.reg_update (vfe,0 ); + +if (status &BUS_IRQ_MASK_0_COMP_DONE (RDI_COMP_GROUP (0 ))) + vfe->isr_ops.wm_done (vfe,0 );
COMP_DONE is signalled in the status register, but wm_done() is called. comp_done() seems to never be called.
Quoted 351 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +351
-0+ } + +return IRQ_HANDLED ; +} + +/* +* vfe_halt - Trigger halt on VFE module and wait to complete +* @vfe: VFE device +* +* Return 0 on success or a negative error code otherwise +*/ +static int vfe_halt (struct vfe_device *vfe) +{ +/* rely on vfe_disable_output() to stop the VFE */ +return 0 ; +} + +static int vfe_get_output (struct vfe_line *line) +{ +struct vfe_device *vfe =to_vfe (line); +struct vfe_output *output; +unsigned long flags; +int wm_idx; + +spin_lock_irqsave (&vfe->output_lock, flags); + + output = &line->output; +if (output->state !=VFE_OUTPUT_OFF ) { +dev_err (vfe->camss->dev,"Output is running\n" ); + goto error; + } + + output->wm_num =1 ; + + wm_idx =vfe_reserve_wm (vfe, line->id); +if (wm_idx <0 ) { +dev_err (vfe->camss->dev,"Can not reserve wm\n" ); + goto error_get_wm; + } + output->wm_idx[0 ] = wm_idx; + + output->drop_update_idx =0 ; + +spin_unlock_irqrestore (&vfe->output_lock, flags); + +return 0 ; + +error_get_wm: +vfe_release_wm (vfe, output->wm_idx[0 ]); + output->state =VFE_OUTPUT_OFF ; +error: +spin_unlock_irqrestore (&vfe->output_lock, flags); + +return -EINVAL ; +} + +static int vfe_enable_output (struct vfe_line *line) +{ +struct vfe_device *vfe =to_vfe (line); +struct vfe_output *output = &line->output; +const struct vfe_hw_ops *ops = vfe->ops; +unsigned long flags; +unsigned int i; + +spin_lock_irqsave (&vfe->output_lock, flags); + + ops->reg_update_clear (vfe, line->id); + +if (output->state !=VFE_OUTPUT_OFF ) { +dev_err (vfe->camss->dev,"Output is not in reserved state %d\n" , + output->state); +spin_unlock_irqrestore (&vfe->output_lock, flags); +return -EINVAL ; + } + +WARN_ON (output->gen2.active_num); + + output->state =VFE_OUTPUT_ON ; + + output->sequence =0 ; + output->wait_reg_update =0 ; +reinit_completion (&output->reg_update); + +vfe_wm_start (vfe, output->wm_idx[0 ], line); + +for (i =0 ; i <2 ; i++) { + output->buf[i] =vfe_buf_get_pending (output); +if (!output->buf[i]) +break ; + output->gen2.active_num++; +vfe_wm_update (vfe, output->wm_idx[0 ], output->buf[i]->addr[0 ], line); + } + + ops->reg_update (vfe, line->id); + +spin_unlock_irqrestore (&vfe->output_lock, flags); + +return 0 ; +} + +static int vfe_disable_output (struct vfe_line *line) +{ +struct vfe_device *vfe =to_vfe (line); +struct vfe_output *output = &line->output; +unsigned long flags; +unsigned int i; +bool done; +int timeout =0 ; + +do { +spin_lock_irqsave (&vfe->output_lock, flags); + done = !output->gen2.active_num; +spin_unlock_irqrestore (&vfe->output_lock, flags); +usleep_range (10000 ,20000 ); + +if (timeout++ ==100 ) { +dev_err (vfe->camss->dev,"VFE idle timeout - resetting\n" ); +vfe_reset (vfe); + output->gen2.active_num =0 ; +return 0 ; + } + }while (!done); + +spin_lock_irqsave (&vfe->output_lock, flags); +for (i =0 ; i < output->wm_num; i++) +vfe_wm_stop (vfe, output->wm_idx[i]); +spin_unlock_irqrestore (&vfe->output_lock, flags); + +return 0 ; +} + +/* +* vfe_enable - Enable streaming on VFE line +* @line: VFE line +* +* Return 0 on success or a negative error code otherwise +*/ +static int vfe_enable (struct vfe_line *line) +{ +struct vfe_device *vfe =to_vfe (line); +int ret; + +mutex_lock (&vfe->stream_lock); + +if (!vfe->stream_count) +vfe_enable_irq_common (vfe); + + vfe->stream_count++; + +mutex_unlock (&vfe->stream_lock); + + ret =vfe_get_output (line); +if (ret <0 ) + goto error_get_output; + + ret =vfe_enable_output (line); +if (ret <0 ) + goto error_enable_output; + + vfe->was_streaming =1 ; + +return 0 ; + +error_enable_output: +vfe_put_output (line); + +error_get_output: +mutex_lock (&vfe->stream_lock); + + vfe->stream_count--; + +mutex_unlock (&vfe->stream_lock); + +return ret; +} + +/* +* vfe_disable - Disable streaming on VFE line +* @line: VFE line +* +* Return 0 on success or a negative error code otherwise +*/ +static int vfe_disable (struct vfe_line *line) +{ +struct vfe_device *vfe =to_vfe (line); + +vfe_disable_output (line); + +vfe_put_output (line); + +mutex_lock (&vfe->stream_lock); + + vfe->stream_count--; + +mutex_unlock (&vfe->stream_lock); + +return 0 ; +} + +/* +* vfe_isr_reg_update - Process reg update interrupt +* @vfe: VFE Device +* @line_id: VFE line +*/ +static void vfe_isr_reg_update (struct vfe_device *vfe,enum vfe_line_id line_id) +{ +struct vfe_output *output; +unsigned long flags; + +spin_lock_irqsave (&vfe->output_lock, flags); + vfe->ops->reg_update_clear (vfe, line_id); + + output = &vfe->line[line_id].output; + +if (output->wait_reg_update) { + output->wait_reg_update =0 ; +complete (&output->reg_update); + } + +spin_unlock_irqrestore (&vfe->output_lock, flags); +} + +/* +* vfe_isr_wm_done - Process write master done interrupt +* @vfe: VFE Device +* @wm: Write master id +*/ +static void vfe_isr_wm_done (struct vfe_device *vfe,u8 wm) +{ +struct vfe_line *line = &vfe->line[vfe->wm_output_map[wm]]; +struct camss_buffer *ready_buf; +struct vfe_output *output; +unsigned long flags; +u32 index; +u64 ts =ktime_get_ns (); + +spin_lock_irqsave (&vfe->output_lock, flags); + +if (vfe->wm_output_map[wm] ==VFE_LINE_NONE ) { +dev_err_ratelimited (vfe->camss->dev, +"Received wm done for unmapped index\n" ); + goto out_unlock; + } + output = &vfe->line[vfe->wm_output_map[wm]].output; + + ready_buf = output->buf[0 ]; +if (!ready_buf) { +dev_err_ratelimited (vfe->camss->dev, +"Missing ready buf %d!\n" , output->state); + goto out_unlock; + } + + ready_buf->vb.vb2_buf.timestamp = ts; + ready_buf->vb.sequence = output->sequence++; + + index =0 ; + output->buf[0 ] = output->buf[1 ]; +if (output->buf[0 ]) + index =1 ; + + output->buf[index] =vfe_buf_get_pending (output); + +if (output->buf[index]) +vfe_wm_update (vfe, output->wm_idx[0 ], output->buf[index]->addr[0 ], line); +else + output->gen2.active_num--; + +spin_unlock_irqrestore (&vfe->output_lock, flags); + +vb2_buffer_done (&ready_buf->vb.vb2_buf,VB2_BUF_STATE_DONE ); + +return ; + +out_unlock: +spin_unlock_irqrestore (&vfe->output_lock, flags); +} + +/* +* vfe_pm_domain_off - Disable power domains specific to this VFE. +* @vfe: VFE Device +*/ +static void vfe_pm_domain_off (struct vfe_device *vfe) +{ +/* nop */ +} + +/* +* vfe_pm_domain_on - Enable power domains specific to this VFE. +* @vfe: VFE Device +*/ +static int vfe_pm_domain_on (struct vfe_device *vfe) +{ +return 0 ; +} + +/* +* vfe_queue_buffer - Add empty buffer +* @vid: Video device structure +* @buf: Buffer to be enqueued +* +* Add an empty buffer - depending on the current number of buffers it will be +* put in pending buffer queue or directly given to the hardware to be filled. +* +* Return 0 on success or a negative error code otherwise +*/ +static int vfe_queue_buffer (struct camss_video *vid, +struct camss_buffer *buf) +{ + struct vfe_line *line = container_of(vid, struct vfe_line, video_out); +struct vfe_device *vfe =to_vfe (line); +struct vfe_output *output; +unsigned long flags; + + output = &line->output; + +spin_lock_irqsave (&vfe->output_lock, flags); + +if (output->state ==VFE_OUTPUT_ON && output->gen2.active_num <2 ) { + output->buf[output->gen2.active_num++] = buf; +vfe_wm_update (vfe, output->wm_idx[0 ], buf->addr[0 ], line); + }else { +vfe_buf_add_pending (output, buf); + } + +spin_unlock_irqrestore (&vfe->output_lock, flags); + +return 0 ; +} + +static const struct vfe_isr_ops vfe_isr_ops_480 = { + .reset_ack = vfe_isr_reset_ack, + .reg_update = vfe_isr_reg_update, + .comp_done = vfe_isr_comp_done, + .wm_done = vfe_isr_wm_done, +}; + +static const struct camss_video_ops vfe_video_ops_480 = { + .queue_buffer = vfe_queue_buffer, + .flush_buffers = vfe_flush_buffers, +}; + +static void vfe_subdev_init (struct device *dev,struct vfe_device *vfe) +{ + vfe->isr_ops = vfe_isr_ops_480; + vfe->video_ops = vfe_video_ops_480; + vfe->line_num =1 ; +} + +const struct vfe_hw_ops vfe_ops_480 = { + .global_reset = vfe_global_reset, + .hw_version_read = vfe_hw_version_read,
drivers/media/platform/qcom/camss/camss-vfe-480.c line 554
Quoted 10 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +10
-0+ .isr = vfe_isr, + .pm_domain_off = vfe_pm_domain_off, + .pm_domain_on = vfe_pm_domain_on, + .reg_update_clear = vfe_reg_update_clear, + .reg_update = vfe_reg_update, + .subdev_init = vfe_subdev_init, + .vfe_disable = vfe_disable, + .vfe_enable = vfe_enable, + .vfe_halt = vfe_halt, +};
Again there are some functions that could be refactored out to a vfe gen2 parent struct & object. This time I think it's worth refactoring out the common code, especially since we know more platforms based on this architecture are coming.
vfe_queue_buffervfe_pm_domain_onvfe_pm_domain_offvfe_isr_wm_donevfe_isr_reg_updatevfe_get_output (although vfe170 contains a frame_skip chunk that should be removed)vfe_halt
Quoted 14 lines
drivers/media/platform/qcom/camss/camss-vfe.h +1
-1@@ -201,5 +201,6 @@ extern const struct vfe_hw_ops vfe_ops_4_1;extern const struct vfe_hw_ops vfe_ops_4_7;extern const struct vfe_hw_ops vfe_ops_4_8;extern const struct vfe_hw_ops vfe_ops_170; +extern const struct vfe_hw_ops vfe_ops_480;#endif /* QC_MSM_CAMSS_VFE_H */ --2.26.1
Re: [PATCH 14/17] media: camss: Add initial support for VFE hardware version Titan 480
Quoted 114 lines · Robert Foss
Hey Jonathan,
Thanks for sending this out.
There are a few checkpatch --strict warnings/etc. in this patch. I won't cover them individually below.
Quoted 106 lines · Jonathan Marek
Add support for VFE found on SM8250 (Titan 480). This implementation is based on the titan 170 implementation. It supports the normal and lite VFE, and only supports the RDI0 capture path.
Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/Makefile | 1 + .../media/platform/qcom/camss/camss-vfe-480.c | 554 ++++++++++++++++++ drivers/media/platform/qcom/camss/camss-vfe.h | 1 + 3 files changed, 556 insertions(+)create mode 100644
drivers/media/platform/qcom/camss/camss-vfe-480.cdrivers/media/platform/qcom/camss/Makefile +1
-0@@ -15,6 +15,7 @@ qcom-camss-objs += \ camss-vfe-4-7.o \ camss-vfe-4-8.o \ camss-vfe-170.o \ + camss-vfe-480.o \ camss-vfe-gen1.o \ camss-vfe.o \ camss-video.o \drivers/media/platform/qcom/camss/camss-vfe-480.c new file +75
-0@@ -0,0 +1,554 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* camss-vfe-480.c +* +* Qualcomm MSM Camera Subsystem - VFE (Video Front End) Module v480 (SM8250) +* +* Copyright (C) 2020-2021 Linaro Ltd. +* Copyright (C) 2021 Jonathan Marek +*/ + +#include <linux/delay.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/iopoll.h> + +#include "camss.h" +#include "camss-vfe.h" + +/* VFE 2/3 are lite and have a different register layout */ +#define IS_LITE (vfe->id >= 2 ? 1 : 0) + +#define VFE_HW_VERSION (0x00) + +#define VFE_GLOBAL_RESET_CMD (IS_LITE ? 0x0c : 0x1c) +#define GLOBAL_RESET_HW_AND_REG (IS_LITE ? BIT(1) : BIT(0)) + +#define VFE_REG_UPDATE_CMD (IS_LITE ? 0x20 : 0x34) +#define REG_UPDATE_RDI (n) (IS_LITE ? BIT(n) : BIT(1 + (n))) +#define VFE_IRQ_CMD (IS_LITE ? 0x24 : 0x38) +#define IRQ_CMD_GLOBAL_CLEAR BIT(0) + +#define VFE_IRQ_MASK (n) ((IS_LITE ? 0x28 : 0x3c) + (n) * 4) +#define IRQ_MASK_0_RESET_ACK (IS_LITE ? BIT(17) : BIT(0)) +#define IRQ_MASK_0_BUS_TOP_IRQ (IS_LITE ? BIT(4) : BIT(7)) +#define VFE_IRQ_CLEAR (n) ((IS_LITE ? 0x34 : 0x48) + (n) * 4) +#define VFE_IRQ_STATUS (n) ((IS_LITE ? 0x40 : 0x54) + (n) * 4) + +#define BUS_REG_BASE (IS_LITE ? 0x1a00 : 0xaa00) + +#define VFE_BUS_WM_CGC_OVERRIDE (BUS_REG_BASE + 0x08) +#define WM_CGC_OVERRIDE_ALL (0x3FFFFFF) + +#define VFE_BUS_WM_TEST_BUS_CTRL (BUS_REG_BASE + 0xdc) + +#define VFE_BUS_IRQ_MASK (n) (BUS_REG_BASE + 0x18 + (n) * 4) +#define BUS_IRQ_MASK_0_RDI_RUP (n) (IS_LITE ? BIT(n) : BIT(3 + (n))) +#define BUS_IRQ_MASK_0_COMP_DONE (n) (IS_LITE ? BIT(4 + (n)) : BIT(6 + (n))) +#define VFE_BUS_IRQ_CLEAR (n) (BUS_REG_BASE + 0x20 + (n) * 4) +#define VFE_BUS_IRQ_STATUS (n) (BUS_REG_BASE + 0x28 + (n) * 4) +#define VFE_BUS_IRQ_CLEAR_GLOBAL (BUS_REG_BASE + 0x30) + +#define VFE_BUS_WM_CFG (n) (BUS_REG_BASE + 0x200 + (n) * 0x100) +#define WM_CFG_EN (0) +#define WM_CFG_MODE (16) +#define MODE_QCOM_PLAIN (0) +#define MODE_MIPI_RAW (1) +#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_1 (n) (BUS_REG_BASE + 0x210 + (n) * 0x100) +#define VFE_BUS_WM_IMAGE_CFG_2 (n) (BUS_REG_BASE + 0x214 + (n) * 0x100) +#define VFE_BUS_WM_PACKER_CFG (n) (BUS_REG_BASE + 0x218 + (n) * 0x100) +#define VFE_BUS_WM_HEADER_ADDR (n) (BUS_REG_BASE + 0x220 + (n) * 0x100) +#define VFE_BUS_WM_HEADER_INCR (n) (BUS_REG_BASE + 0x224 + (n) * 0x100) +#define VFE_BUS_WM_HEADER_CFG (n) (BUS_REG_BASE + 0x228 + (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_SYSTEM_CACHE_CFG (n) (BUS_REG_BASE + 0x260 + (n) * 0x100) +#define VFE_BUS_WM_BURST_LIMIT (n) (BUS_REG_BASE + 0x264 + (n) * 0x100) +
Quoted 9 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c line 80
Quoted 5 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +5
-0+/* for titan 480, each bus client is hardcoded to a specific path +* and each bus client is part of a hardcoded "comp group" +*/ +#define RDI_WM (n) ((IS_LITE ? 0 : 23) + n) +#define RDI_COMP_GROUP (n) ((IS_LITE ? 0 : 11) + n)The indentation of the different types of defines above differ from vfe170, but I kind of prefer this style. Feel free to change either.
Its not just the indentation, the naming style is a bit different too. Not sure its worth trying to make them fully consistent, but I wouldn't mind just changing the identation for 170.
Quoted 32 lines
Quoted 32 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +32
-0+ +static void vfe_hw_version_read (struct vfe_device *vfe,struct device *dev) +{ +u32 hw_version =readl_relaxed (vfe->base +VFE_HW_VERSION ); + +u32 gen = (hw_version >>28 ) &0xF ; +u32 rev = (hw_version >>16 ) &0xFFF ; +u32 step = hw_version &0xFFFF ; + +dev_dbg (dev,"VFE HW Version = %u.%u.%u\n" , gen, rev, step); +} + +static void vfe_global_reset (struct vfe_device *vfe) +{ +writel_relaxed (IRQ_MASK_0_RESET_ACK , vfe->base +VFE_IRQ_MASK (0 )); +writel_relaxed (GLOBAL_RESET_HW_AND_REG , vfe->base +VFE_GLOBAL_RESET_CMD ); +} + +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; + + wm =RDI_WM (wm);/* map to actual WM used (from wm=RDI index) */ + +/* no clock gating at bus input */ +writel_relaxed (WM_CGC_OVERRIDE_ALL , vfe->base +VFE_BUS_WM_CGC_OVERRIDE ); + +writel_relaxed (0x0 , vfe->base +VFE_BUS_WM_TEST_BUS_CTRL ); + +writel_relaxed (pix->plane_fmt[0 ].bytesperline * pix->height, + vfe->base +VFE_BUS_WM_FRAME_INCR (wm));
Quoted 9 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c line 118
Quoted 6 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +6
-0+writel_relaxed (0xf , vfe->base +VFE_BUS_WM_BURST_LIMIT (wm)); +writel_relaxed (WM_IMAGE_CFG_0_DEFAULT_WIDTH , + vfe->base +VFE_BUS_WM_IMAGE_CFG_0 (wm)); +writel_relaxed (pix->plane_fmt[0 ].bytesperline, + vfe->base +VFE_BUS_WM_IMAGE_CFG_2 (wm)); +writel_relaxed (0 , vfe->base +VFE_BUS_WM_PACKER_CFG (wm));^^^ using more of the 130char line length is probably better for legibility.
More than 100 line length is still a warning from checkpatch, and this is IMO more readable since all the "vfe->base + X" are more or less at the same indentation, and removing the line breaks would make it harder to see which registers are being updated.
Quoted 70 lines
Quoted 70 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +70
-0+ +/* no dropped frames, one irq per frame */ +writel_relaxed (0 , vfe->base +VFE_BUS_WM_FRAMEDROP_PERIOD (wm)); +writel_relaxed (1 , vfe->base +VFE_BUS_WM_FRAMEDROP_PATTERN (wm)); +writel_relaxed (0 , vfe->base +VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD (wm)); +writel_relaxed (1 , vfe->base +VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN (wm)); + +writel_relaxed (1 <<WM_CFG_EN |MODE_MIPI_RAW <<WM_CFG_MODE , + vfe->base +VFE_BUS_WM_CFG (wm)); +} + +static void vfe_wm_stop (struct vfe_device *vfe,u8 wm) +{ + wm =RDI_WM (wm);/* map to actual WM used (from wm=RDI index) */ +writel_relaxed (0 , vfe->base +VFE_BUS_WM_CFG (wm)); +} + +static void vfe_wm_update (struct vfe_device *vfe,u8 wm,u32 addr, +struct vfe_line *line) +{ + wm =RDI_WM (wm);/* map to actual WM used (from wm=RDI index) */ +writel_relaxed (addr, vfe->base +VFE_BUS_WM_IMAGE_ADDR (wm)); +} + +static void vfe_reg_update (struct vfe_device *vfe,enum vfe_line_id line_id) +{ + vfe->reg_update |=REG_UPDATE_RDI (line_id); +writel_relaxed (vfe->reg_update, vfe->base +VFE_REG_UPDATE_CMD ); +} + +static inline void vfe_reg_update_clear (struct vfe_device *vfe, +enum vfe_line_id line_id) +{ + vfe->reg_update &= ~REG_UPDATE_RDI (line_id); +} + +static void vfe_enable_irq_common (struct vfe_device *vfe) +{ +/* enable only the IRQs used: rup and comp_done irqs for RDI0 */ +writel_relaxed (IRQ_MASK_0_RESET_ACK |IRQ_MASK_0_BUS_TOP_IRQ , + vfe->base +VFE_IRQ_MASK (0 )); +writel_relaxed (BUS_IRQ_MASK_0_RDI_RUP (0 ) | +BUS_IRQ_MASK_0_COMP_DONE (RDI_COMP_GROUP (0 )), + vfe->base +VFE_BUS_IRQ_MASK (0 )); +} + +/* +* vfe_isr - VFE module interrupt handler +* @irq: Interrupt line +* @dev: VFE device +* +* Return IRQ_HANDLED on success +*/ +static irqreturn_t vfe_isr (int irq,void *dev) +{ +struct vfe_device *vfe = dev; +u32 status; + + status =readl_relaxed (vfe->base +VFE_IRQ_STATUS (0 )); +writel_relaxed (status, vfe->base +VFE_IRQ_CLEAR (0 )); +writel_relaxed (IRQ_CMD_GLOBAL_CLEAR , vfe->base +VFE_IRQ_CMD ); + +if (status &IRQ_MASK_0_RESET_ACK ) + vfe->isr_ops.reset_ack (vfe); + +if (status &IRQ_MASK_0_BUS_TOP_IRQ ) { +u32 status =readl_relaxed (vfe->base +VFE_BUS_IRQ_STATUS (0 )); +writel_relaxed (status, vfe->base +VFE_BUS_IRQ_CLEAR (0 )); +writel_relaxed (1 , vfe->base +VFE_BUS_IRQ_CLEAR_GLOBAL ); +
Quoted 9 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c line 193
Quoted 5 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +5
-0+if (status &BUS_IRQ_MASK_0_RDI_RUP (0 )) + vfe->isr_ops.reg_update (vfe,0 ); + +if (status &BUS_IRQ_MASK_0_COMP_DONE (RDI_COMP_GROUP (0 ))) + vfe->isr_ops.wm_done (vfe,0 );
COMP_DONEis signalled in the status register, butwm_done()is called.comp_done()seems to never be called.
The current "vfe_isr_comp_done" is not relevant to RDI capture. Titan 480 (unlike 170) now uses comp done IRQs for RDI too.
This is a bit of a hack. wm_done is called with wm=0, but the implementation passes it through RDI_WM(0) to map it to wm=23 (which is the WM for RDI0 - with titan 480 the WM paths are fixed).
...
Quoted 19 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c line 554
Quoted 13 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +13
-0+const struct vfe_hw_ops vfe_ops_480 = { + .global_reset = vfe_global_reset, + .hw_version_read = vfe_hw_version_read, + .isr = vfe_isr, + .pm_domain_off = vfe_pm_domain_off, + .pm_domain_on = vfe_pm_domain_on, + .reg_update_clear = vfe_reg_update_clear, + .reg_update = vfe_reg_update, + .subdev_init = vfe_subdev_init, + .vfe_disable = vfe_disable, + .vfe_enable = vfe_enable, + .vfe_halt = vfe_halt, +};Again there are some functions that could be refactored out to a vfe gen2 parent struct & object. This time I think it's worth refactoring out the common code, especially since we know more platforms based on this architecture are coming.
Quoted 9 lines
vfe_queue_buffervfe_pm_domain_onvfe_pm_domain_offvfe_isr_wm_donevfe_isr_reg_updatevfe_get_output(although vfe170 contains aframe_skipchunk that should be removed)vfe_halt
In the current minimal implementation of both vfe-170 and vfe-480, most of these could be shared, but I think duplicating these few functions for now is fine, it can easily be resolved later. (there might be less shared code than expected if the titan 480 WMs are implementated fully instead of the "hack" I've used to support only RDI cases).
Quoted 14 lines
Quoted 14 lines
drivers/media/platform/qcom/camss/camss-vfe.h +1
-1@@ -201,5 +201,6 @@ extern const struct vfe_hw_ops vfe_ops_4_1;extern const struct vfe_hw_ops vfe_ops_4_7;extern const struct vfe_hw_ops vfe_ops_4_8;extern const struct vfe_hw_ops vfe_ops_170; +extern const struct vfe_hw_ops vfe_ops_480;#endif /* QC_MSM_CAMSS_VFE_H */ --2.26.1
Re: [PATCH 14/17] media: camss: Add initial support for VFE hardware version Titan 480
Quoted 126 lines · Jonathan Marek
Quoted 114 lines · Robert Foss
Hey Jonathan,
Thanks for sending this out.
There are a few checkpatch --strict warnings/etc. in this patch. I won't cover them individually below.
Quoted 106 lines · Jonathan Marek
Add support for VFE found on SM8250 (Titan 480). This implementation is based on the titan 170 implementation. It supports the normal and lite VFE, and only supports the RDI0 capture path.
Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/Makefile | 1 + .../media/platform/qcom/camss/camss-vfe-480.c | 554 ++++++++++++++++++ drivers/media/platform/qcom/camss/camss-vfe.h | 1 + 3 files changed, 556 insertions(+)create mode 100644
drivers/media/platform/qcom/camss/camss-vfe-480.cdrivers/media/platform/qcom/camss/Makefile +1
-0@@ -15,6 +15,7 @@ qcom-camss-objs += \ camss-vfe-4-7.o \ camss-vfe-4-8.o \ camss-vfe-170.o \ + camss-vfe-480.o \ camss-vfe-gen1.o \ camss-vfe.o \ camss-video.o \drivers/media/platform/qcom/camss/camss-vfe-480.c new file +75
-0@@ -0,0 +1,554 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* camss-vfe-480.c +* +* Qualcomm MSM Camera Subsystem - VFE (Video Front End) Module v480 (SM8250) +* +* Copyright (C) 2020-2021 Linaro Ltd. +* Copyright (C) 2021 Jonathan Marek +*/ + +#include <linux/delay.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/iopoll.h> + +#include "camss.h" +#include "camss-vfe.h" + +/* VFE 2/3 are lite and have a different register layout */ +#define IS_LITE (vfe->id >= 2 ? 1 : 0) + +#define VFE_HW_VERSION (0x00) + +#define VFE_GLOBAL_RESET_CMD (IS_LITE ? 0x0c : 0x1c) +#define GLOBAL_RESET_HW_AND_REG (IS_LITE ? BIT(1) : BIT(0)) + +#define VFE_REG_UPDATE_CMD (IS_LITE ? 0x20 : 0x34) +#define REG_UPDATE_RDI (n) (IS_LITE ? BIT(n) : BIT(1 + (n))) +#define VFE_IRQ_CMD (IS_LITE ? 0x24 : 0x38) +#define IRQ_CMD_GLOBAL_CLEAR BIT(0) + +#define VFE_IRQ_MASK (n) ((IS_LITE ? 0x28 : 0x3c) + (n) * 4) +#define IRQ_MASK_0_RESET_ACK (IS_LITE ? BIT(17) : BIT(0)) +#define IRQ_MASK_0_BUS_TOP_IRQ (IS_LITE ? BIT(4) : BIT(7)) +#define VFE_IRQ_CLEAR (n) ((IS_LITE ? 0x34 : 0x48) + (n) * 4) +#define VFE_IRQ_STATUS (n) ((IS_LITE ? 0x40 : 0x54) + (n) * 4) + +#define BUS_REG_BASE (IS_LITE ? 0x1a00 : 0xaa00) + +#define VFE_BUS_WM_CGC_OVERRIDE (BUS_REG_BASE + 0x08) +#define WM_CGC_OVERRIDE_ALL (0x3FFFFFF) + +#define VFE_BUS_WM_TEST_BUS_CTRL (BUS_REG_BASE + 0xdc) + +#define VFE_BUS_IRQ_MASK (n) (BUS_REG_BASE + 0x18 + (n) * 4) +#define BUS_IRQ_MASK_0_RDI_RUP (n) (IS_LITE ? BIT(n) : BIT(3 + (n))) +#define BUS_IRQ_MASK_0_COMP_DONE (n) (IS_LITE ? BIT(4 + (n)) : BIT(6 + (n))) +#define VFE_BUS_IRQ_CLEAR (n) (BUS_REG_BASE + 0x20 + (n) * 4) +#define VFE_BUS_IRQ_STATUS (n) (BUS_REG_BASE + 0x28 + (n) * 4) +#define VFE_BUS_IRQ_CLEAR_GLOBAL (BUS_REG_BASE + 0x30) + +#define VFE_BUS_WM_CFG (n) (BUS_REG_BASE + 0x200 + (n) * 0x100) +#define WM_CFG_EN (0) +#define WM_CFG_MODE (16) +#define MODE_QCOM_PLAIN (0) +#define MODE_MIPI_RAW (1) +#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_1 (n) (BUS_REG_BASE + 0x210 + (n) * 0x100) +#define VFE_BUS_WM_IMAGE_CFG_2 (n) (BUS_REG_BASE + 0x214 + (n) * 0x100) +#define VFE_BUS_WM_PACKER_CFG (n) (BUS_REG_BASE + 0x218 + (n) * 0x100) +#define VFE_BUS_WM_HEADER_ADDR (n) (BUS_REG_BASE + 0x220 + (n) * 0x100) +#define VFE_BUS_WM_HEADER_INCR (n) (BUS_REG_BASE + 0x224 + (n) * 0x100) +#define VFE_BUS_WM_HEADER_CFG (n) (BUS_REG_BASE + 0x228 + (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_SYSTEM_CACHE_CFG (n) (BUS_REG_BASE + 0x260 + (n) * 0x100) +#define VFE_BUS_WM_BURST_LIMIT (n) (BUS_REG_BASE + 0x264 + (n) * 0x100) +Quoted 9 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c line 80
Quoted 5 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +5
-0+/* for titan 480, each bus client is hardcoded to a specific path +* and each bus client is part of a hardcoded "comp group" +*/ +#define RDI_WM (n) ((IS_LITE ? 0 : 23) + n) +#define RDI_COMP_GROUP (n) ((IS_LITE ? 0 : 11) + n)The indentation of the different types of defines above differ from vfe170, but I kind of prefer this style. Feel free to change either.
Quoted 3 lines
Its not just the indentation, the naming style is a bit different too. Not sure its worth trying to make them fully consistent, but I wouldn't mind just changing the identation for 170.
Ack.
Quoted 43 lines
Quoted 33 lines
Quoted 33 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +32
-0+ +static void vfe_hw_version_read (struct vfe_device *vfe,struct device *dev) +{ +u32 hw_version =readl_relaxed (vfe->base +VFE_HW_VERSION ); + +u32 gen = (hw_version >>28 ) &0xF ; +u32 rev = (hw_version >>16 ) &0xFFF ; +u32 step = hw_version &0xFFFF ; + +dev_dbg (dev,"VFE HW Version = %u.%u.%u\n" , gen, rev, step); +} + +static void vfe_global_reset (struct vfe_device *vfe) +{ +writel_relaxed (IRQ_MASK_0_RESET_ACK , vfe->base +VFE_IRQ_MASK (0 )); +writel_relaxed (GLOBAL_RESET_HW_AND_REG , vfe->base +VFE_GLOBAL_RESET_CMD ); +} + +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; + + wm =RDI_WM (wm);/* map to actual WM used (from wm=RDI index) */ + +/* no clock gating at bus input */ +writel_relaxed (WM_CGC_OVERRIDE_ALL , vfe->base +VFE_BUS_WM_CGC_OVERRIDE ); + +writel_relaxed (0x0 , vfe->base +VFE_BUS_WM_TEST_BUS_CTRL ); + +writel_relaxed (pix->plane_fmt[0 ].bytesperline * pix->height, + vfe->base +VFE_BUS_WM_FRAME_INCR (wm));Quoted 9 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c line 118
Quoted 6 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +6
-0+writel_relaxed (0xf , vfe->base +VFE_BUS_WM_BURST_LIMIT (wm)); +writel_relaxed (WM_IMAGE_CFG_0_DEFAULT_WIDTH , + vfe->base +VFE_BUS_WM_IMAGE_CFG_0 (wm)); +writel_relaxed (pix->plane_fmt[0 ].bytesperline, + vfe->base +VFE_BUS_WM_IMAGE_CFG_2 (wm)); +writel_relaxed (0 , vfe->base +VFE_BUS_WM_PACKER_CFG (wm));^^^ using more of the 130char line length is probably better for legibility.
Quoted 4 lines
More than 100 line length is still a warning from checkpatch, and this is IMO more readable since all the "
vfe->base+ X" are more or less at the same indentation, and removing the line breaks would make it harder to see which registers are being updated.
You're right.
Quoted 81 lines
Quoted 71 lines
Quoted 71 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +70
-0+ +/* no dropped frames, one irq per frame */ +writel_relaxed (0 , vfe->base +VFE_BUS_WM_FRAMEDROP_PERIOD (wm)); +writel_relaxed (1 , vfe->base +VFE_BUS_WM_FRAMEDROP_PATTERN (wm)); +writel_relaxed (0 , vfe->base +VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD (wm)); +writel_relaxed (1 , vfe->base +VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN (wm)); + +writel_relaxed (1 <<WM_CFG_EN |MODE_MIPI_RAW <<WM_CFG_MODE , + vfe->base +VFE_BUS_WM_CFG (wm)); +} + +static void vfe_wm_stop (struct vfe_device *vfe,u8 wm) +{ + wm =RDI_WM (wm);/* map to actual WM used (from wm=RDI index) */ +writel_relaxed (0 , vfe->base +VFE_BUS_WM_CFG (wm)); +} + +static void vfe_wm_update (struct vfe_device *vfe,u8 wm,u32 addr, +struct vfe_line *line) +{ + wm =RDI_WM (wm);/* map to actual WM used (from wm=RDI index) */ +writel_relaxed (addr, vfe->base +VFE_BUS_WM_IMAGE_ADDR (wm)); +} + +static void vfe_reg_update (struct vfe_device *vfe,enum vfe_line_id line_id) +{ + vfe->reg_update |=REG_UPDATE_RDI (line_id); +writel_relaxed (vfe->reg_update, vfe->base +VFE_REG_UPDATE_CMD ); +} + +static inline void vfe_reg_update_clear (struct vfe_device *vfe, +enum vfe_line_id line_id) +{ + vfe->reg_update &= ~REG_UPDATE_RDI (line_id); +} + +static void vfe_enable_irq_common (struct vfe_device *vfe) +{ +/* enable only the IRQs used: rup and comp_done irqs for RDI0 */ +writel_relaxed (IRQ_MASK_0_RESET_ACK |IRQ_MASK_0_BUS_TOP_IRQ , + vfe->base +VFE_IRQ_MASK (0 )); +writel_relaxed (BUS_IRQ_MASK_0_RDI_RUP (0 ) | +BUS_IRQ_MASK_0_COMP_DONE (RDI_COMP_GROUP (0 )), + vfe->base +VFE_BUS_IRQ_MASK (0 )); +} + +/* +* vfe_isr - VFE module interrupt handler +* @irq: Interrupt line +* @dev: VFE device +* +* Return IRQ_HANDLED on success +*/ +static irqreturn_t vfe_isr (int irq,void *dev) +{ +struct vfe_device *vfe = dev; +u32 status; + + status =readl_relaxed (vfe->base +VFE_IRQ_STATUS (0 )); +writel_relaxed (status, vfe->base +VFE_IRQ_CLEAR (0 )); +writel_relaxed (IRQ_CMD_GLOBAL_CLEAR , vfe->base +VFE_IRQ_CMD ); + +if (status &IRQ_MASK_0_RESET_ACK ) + vfe->isr_ops.reset_ack (vfe); + +if (status &IRQ_MASK_0_BUS_TOP_IRQ ) { +u32 status =readl_relaxed (vfe->base +VFE_BUS_IRQ_STATUS (0 )); +writel_relaxed (status, vfe->base +VFE_BUS_IRQ_CLEAR (0 )); +writel_relaxed (1 , vfe->base +VFE_BUS_IRQ_CLEAR_GLOBAL ); +Quoted 9 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c line 193
Quoted 5 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +5
-0+if (status &BUS_IRQ_MASK_0_RDI_RUP (0 )) + vfe->isr_ops.reg_update (vfe,0 ); + +if (status &BUS_IRQ_MASK_0_COMP_DONE (RDI_COMP_GROUP (0 ))) + vfe->isr_ops.wm_done (vfe,0 );
COMP_DONEis signalled in the status register, butwm_done()is called.comp_done()seems to never be called.
Quoted 6 lines
The current "
vfe_isr_comp_done" is not relevant to RDI capture. Titan 480 (unlike 170) now uses comp done IRQs for RDI too.This is a bit of a hack.
wm_doneis called with wm=0, but the implementation passes it throughRDI_WM(0)to map it to wm=23 (which is the WM for RDI0 - with titan 480 the WM paths are fixed).
I think that sounds quite reasonable. Maybe a comment explaining the nuances would be nice. For the next guy :p
Quoted 32 lines
...
Quoted 19 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c line 554
Quoted 13 lines
drivers/media/platform/qcom/camss/camss-vfe-480.c new file +13
-0+const struct vfe_hw_ops vfe_ops_480 = { + .global_reset = vfe_global_reset, + .hw_version_read = vfe_hw_version_read, + .isr = vfe_isr, + .pm_domain_off = vfe_pm_domain_off, + .pm_domain_on = vfe_pm_domain_on, + .reg_update_clear = vfe_reg_update_clear, + .reg_update = vfe_reg_update, + .subdev_init = vfe_subdev_init, + .vfe_disable = vfe_disable, + .vfe_enable = vfe_enable, + .vfe_halt = vfe_halt, +};Again there are some functions that could be refactored out to a vfe gen2 parent struct & object. This time I think it's worth refactoring out the common code, especially since we know more platforms based on this architecture are coming.
Quoted 9 lines
vfe_queue_buffervfe_pm_domain_onvfe_pm_domain_offvfe_isr_wm_donevfe_isr_reg_updatevfe_get_output(although vfe170 contains aframe_skipchunk that should be removed)vfe_halt
Quoted 5 lines
In the current minimal implementation of both vfe-170 and vfe-480, most of these could be shared, but I think duplicating these few functions for now is fine, it can easily be resolved later. (there might be less shared code than expected if the titan 480 WMs are implementated fully instead of the "hack" I've used to support only RDI cases).
Fair enough.
Quoted 15 lines
Quoted 15 lines
Quoted 15 lines
drivers/media/platform/qcom/camss/camss-vfe.h +1
-1@@ -201,5 +201,6 @@ extern const struct vfe_hw_ops vfe_ops_4_1;extern const struct vfe_hw_ops vfe_ops_4_7;extern const struct vfe_hw_ops vfe_ops_4_8;extern const struct vfe_hw_ops vfe_ops_170; +extern const struct vfe_hw_ops vfe_ops_480;#endif /* QC_MSM_CAMSS_VFE_H */ --2.26.1
PATCH 15/17 media: camss: add support for V4L2_PIX_FMT_GREY for sdm845 HW
In reply to Jonathan MarekAdd this common format to the various format lists relevant to sdm845.
--- drivers/media/platform/qcom/camss/camss-csid-170.c | 7 +++++++ drivers/media/platform/qcom/camss/camss-csiphy.c | 1 + drivers/media/platform/qcom/camss/camss-vfe.c | 1 + drivers/media/platform/qcom/camss/camss-video.c | 2 ++ 4 files changed, 11 insertions(+)
drivers/media/platform/qcom/camss/camss-csid-170.c +7 -0
@@ -262,6 +262,13 @@ static const struct csid_format csid_formats[] = {10 ,1 , }, + { +MEDIA_BUS_FMT_Y8_1X8 , +DATA_TYPE_RAW_8BIT , +DECODE_FORMAT_UNCOMPRESSED_8_BIT , +10 , +1 , + }, {MEDIA_BUS_FMT_Y10_1X10 , DATA_TYPE_RAW_10BIT,
drivers/media/platform/qcom/camss/camss-csiphy.c +1 -0
@@ -94,6 +94,7 @@ static const struct csiphy_format csiphy_formats_sdm845[] = { { MEDIA_BUS_FMT_SGBRG14_1X14, 14 }, { MEDIA_BUS_FMT_SGRBG14_1X14, 14 }, { MEDIA_BUS_FMT_SRGGB14_1X14, 14 }, + { MEDIA_BUS_FMT_Y8_1X8, 8 }, { MEDIA_BUS_FMT_Y10_1X10, 10 }, };
drivers/media/platform/qcom/camss/camss-vfe.c +1 -0
@@ -118,6 +118,7 @@ static const struct vfe_format formats_rdi_845[] = { { MEDIA_BUS_FMT_SGBRG14_1X14, 14 }, { MEDIA_BUS_FMT_SGRBG14_1X14, 14 }, { MEDIA_BUS_FMT_SRGGB14_1X14, 14 }, + { MEDIA_BUS_FMT_Y8_1X8, 8 }, { MEDIA_BUS_FMT_Y10_1X10, 10 }, { MEDIA_BUS_FMT_Y10_2X8_PADHI_LE, 16 }, };
drivers/media/platform/qcom/camss/camss-video.c +2 -0
@@ -176,6 +176,8 @@ static const struct camss_format_info formats_rdi_845[] = { { { 1, 1 } }, { { 1, 1 } }, { 14 } }, {MEDIA_BUS_FMT_SRGGB14_1X14 ,V4L2_PIX_FMT_SRGGB14P ,1 , { { 1, 1 } }, { { 1, 1 } }, { 14 } }, + { MEDIA_BUS_FMT_Y8_1X8, V4L2_PIX_FMT_GREY, 1, + { { 1, 1 } }, { { 1, 1 } }, { 8 } }, { MEDIA_BUS_FMT_Y10_1X10, V4L2_PIX_FMT_Y10P, 1, { { 1, 1 } }, { { 1, 1 } }, { 10 } }, { MEDIA_BUS_FMT_Y10_2X8_PADHI_LE, V4L2_PIX_FMT_Y10, 1,
Signature · 1 line
2.26.1
Re: [PATCH 15/17] media: camss: add support for V4L2_PIX_FMT_GREY for sdm845 HW
Quoted 58 lines · Jonathan Marek
Add this common format to the various format lists relevant to sdm845.
Signed-off-by Jonathan Marek <[email protected]>--- drivers/media/platform/qcom/camss/camss-csid-170.c | 7 +++++++ drivers/media/platform/qcom/camss/camss-csiphy.c | 1 + drivers/media/platform/qcom/camss/camss-vfe.c | 1 + drivers/media/platform/qcom/camss/camss-video.c | 2 ++ 4 files changed, 11 insertions(+)drivers/media/platform/qcom/camss/camss-csid-170.c +7
-0@@ -262,6 +262,13 @@ static const struct csid_format csid_formats[] = {10 ,1 , }, + { +MEDIA_BUS_FMT_Y8_1X8 , +DATA_TYPE_RAW_8BIT , +DECODE_FORMAT_UNCOMPRESSED_8_BIT , +10 , +1 , + }, {MEDIA_BUS_FMT_Y10_1X10 , DATA_TYPE_RAW_10BIT,drivers/media/platform/qcom/camss/camss-csiphy.c +1
-0@@ -94,6 +94,7 @@ static const struct csiphy_format csiphy_formats_sdm845[] = { { MEDIA_BUS_FMT_SGBRG14_1X14, 14 }, { MEDIA_BUS_FMT_SGRBG14_1X14, 14 }, { MEDIA_BUS_FMT_SRGGB14_1X14, 14 }, + { MEDIA_BUS_FMT_Y8_1X8, 8 }, { MEDIA_BUS_FMT_Y10_1X10, 10 }, };drivers/media/platform/qcom/camss/camss-vfe.c +1
-0@@ -118,6 +118,7 @@ static const struct vfe_format formats_rdi_845[] = { { MEDIA_BUS_FMT_SGBRG14_1X14, 14 }, { MEDIA_BUS_FMT_SGRBG14_1X14, 14 }, { MEDIA_BUS_FMT_SRGGB14_1X14, 14 }, + { MEDIA_BUS_FMT_Y8_1X8, 8 }, { MEDIA_BUS_FMT_Y10_1X10, 10 }, { MEDIA_BUS_FMT_Y10_2X8_PADHI_LE, 16 }, };drivers/media/platform/qcom/camss/camss-video.c +0
-0
drivers/media/platform/qcom/camss/camss-video.c static const struct camss_format_info formats_rdi_845[] = { · line 183
Quoted 8 lines
drivers/media/platform/qcom/camss/camss-video.c +2
-0@@ -176,6 +176,8 @@ static const struct camss_format_info formats_rdi_845[] = { { { 1, 1 } }, { { 1, 1 } }, { 14 } }, {MEDIA_BUS_FMT_SRGGB14_1X14 ,V4L2_PIX_FMT_SRGGB14P ,1 , { { 1, 1 } }, { { 1, 1 } }, { 14 } }, + { MEDIA_BUS_FMT_Y8_1X8, V4L2_PIX_FMT_GREY, 1, + { { 1, 1 } }, { { 1, 1 } }, { 8 } }, { MEDIA_BUS_FMT_Y10_1X10, V4L2_PIX_FMT_Y10P, 1, { { 1, 1 } }, { { 1, 1 } }, { 10 } }, { MEDIA_BUS_FMT_Y10_2X8_PADHI_LE, V4L2_PIX_FMT_Y10, 1,
PATCH 16/17 media: camss: add support for SM8250 camss
In reply to Jonathan MarekThe Titan 480 camss found on SM8250 has 6 CSIPHY and 4 VFE/CSID.
CSID is compatible with the Titan 170 CSID, but the Titan 480 CSID are inside the VFE region (between the "top" and "bus" registers), so a workaround is added to avoid ioremap failure.
--- .../media/platform/qcom/camss/camss-csid.c | 22 +- .../media/platform/qcom/camss/camss-csiphy.c | 8 +- drivers/media/platform/qcom/camss/camss-vfe.c | 9 +- .../media/platform/qcom/camss/camss-video.c | 3 +- drivers/media/platform/qcom/camss/camss.c | 196 ++++++++++++++++-- drivers/media/platform/qcom/camss/camss.h | 1 + 6 files changed, 217 insertions(+), 22 deletions(-)
drivers/media/platform/qcom/camss/camss-csid.c +17 -5
@@ -560,7 +560,8 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid, }else if (camss->version == CAMSS_8x96 || camss->version ==CAMSS_660 ) { csid->ops = &csid_ops_4_7; - }else if (camss->version ==CAMSS_845 ) { + }else if (camss->version ==CAMSS_845 || + camss->version ==CAMSS_8250 ) { csid->ops = &csid_ops_170; }else {return -EINVAL ;
@@ -569,10 +570,21 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid,/* Memory */ - r =platform_get_resource_byname (pdev,IORESOURCE_MEM , res->reg[0 ]); - csid->base =devm_ioremap_resource (dev, r); -if (IS_ERR (csid->base)) -return PTR_ERR (csid->base); +if (camss->version ==CAMSS_8250 ) { +/* for titan 480, CSID registers are inside the VFE region, +* between the VFE "top" and "bus" registers. this requires +* VFE to be initialized before CSID +*/ +if (id >=2 )/* VFE/CSID lite */ + csid->base = camss->vfe[id].base +0x200 ; +else + csid->base = camss->vfe[id].base +0x1200 ; + }else { + r =platform_get_resource_byname (pdev,IORESOURCE_MEM , res->reg[0 ]); + csid->base =devm_ioremap_resource (dev, r); +if (IS_ERR (csid->base)) +return PTR_ERR (csid->base); + }/* Interrupt */
drivers/media/platform/qcom/camss/camss-csiphy.c +6 -2
@@ -582,7 +582,8 @@ int msm_csiphy_subdev_init(struct camss *camss, csiphy->ops = &csiphy_ops_3ph_1_0; csiphy->formats = csiphy_formats_8x96; csiphy->nformats =ARRAY_SIZE (csiphy_formats_8x96); - }else if (camss->version ==CAMSS_845 ) { + }else if (camss->version ==CAMSS_845 || + camss->version ==CAMSS_8250 ) { csiphy->ops = &csiphy_ops_3ph_1_0; csiphy->formats = csiphy_formats_sdm845; csiphy->nformats =ARRAY_SIZE (csiphy_formats_sdm845);
@@ -679,7 +680,10 @@ int msm_csiphy_subdev_init(struct camss *camss,if (!strcmp (clock->name,"csiphy0_timer" ) || !strcmp (clock->name,"csiphy1_timer" ) || - !strcmp (clock->name,"csiphy2_timer" )) + !strcmp (clock->name,"csiphy2_timer" ) || + !strcmp (clock->name,"csiphy3_timer" ) || + !strcmp (clock->name,"csiphy4_timer" ) || + !strcmp (clock->name,"csiphy5_timer" )) csiphy->rate_set[i] = true; if (camss->version == CAMSS_660 &&
drivers/media/platform/qcom/camss/camss-vfe.c +7 -2
@@ -220,7 +220,8 @@ static u32 vfe_src_pad_code(struct vfe_line *line, u32 sink_code, }else if (vfe->camss->version == CAMSS_8x96 || vfe->camss->version ==CAMSS_660 || - vfe->camss->version ==CAMSS_845 ) + vfe->camss->version ==CAMSS_845 || + vfe->camss->version ==CAMSS_8250 )switch (sink_code) {case MEDIA_BUS_FMT_YUYV8_2X8 : {
@@ -1294,6 +1295,9 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,case CAMSS_845 : vfe->ops = &vfe_ops_170;break ; +case CAMSS_8250 : + vfe->ops = &vfe_ops_480; +break ;default :return -EINVAL ; }
@@ -1405,7 +1409,8 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe, l->formats = formats_rdi_8x96; l->nformats =ARRAY_SIZE (formats_rdi_8x96); } - }else if (camss->version ==CAMSS_845 ) { + }else if (camss->version ==CAMSS_845 || + camss->version ==CAMSS_8250 ) { l->formats = formats_rdi_845; l->nformats =ARRAY_SIZE (formats_rdi_845); }else {
drivers/media/platform/qcom/camss/camss-video.c +2 -1
@@ -1011,7 +1011,8 @@ int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev, video->formats = formats_rdi_8x96; video->nformats =ARRAY_SIZE (formats_rdi_8x96); } - }else if (video->camss->version ==CAMSS_845 ) { + }else if (video->camss->version ==CAMSS_845 || + video->camss->version ==CAMSS_8250 ) { video->formats = formats_rdi_845; video->nformats =ARRAY_SIZE (formats_rdi_845); }else {
drivers/media/platform/qcom/camss/camss.c +184 -12
244 lines not on this page · raw message
drivers/media/platform/qcom/camss/camss.h +1 -0
@@ -69,6 +69,7 @@ enum camss_version { CAMSS_8x96,CAMSS_660 ,CAMSS_845 , + CAMSS_8250, }; struct camss {
Signature · 1 line
2.26.1
Re: [PATCH 16/17] media: camss: add support for SM8250 camss
Quoted 22 lines · Jonathan Marek
The Titan 480 camss found on SM8250 has 6 CSIPHY and 4 VFE/CSID.
CSID is compatible with the Titan 170 CSID, but the Titan 480 CSID are inside the VFE region (between the "top" and "bus" registers), so a workaround is added to avoid ioremap failure.
Signed-off-by Jonathan Marek <[email protected]>--- .../media/platform/qcom/camss/camss-csid.c | 22 +- .../media/platform/qcom/camss/camss-csiphy.c | 8 +- drivers/media/platform/qcom/camss/camss-vfe.c | 9 +- .../media/platform/qcom/camss/camss-video.c | 3 +- drivers/media/platform/qcom/camss/camss.c | 196 ++++++++++++++++-- drivers/media/platform/qcom/camss/camss.h | 1 + 6 files changed, 217 insertions(+), 22 deletions(-)drivers/media/platform/qcom/camss/camss-csid.c +0
-0
drivers/media/platform/qcom/camss/camss-csid.c int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid, · line 565
Quoted 7 lines
drivers/media/platform/qcom/camss/camss-csid.c +2
-1@@ -560,7 +560,8 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid, }else if (camss->version == CAMSS_8x96 || camss->version ==CAMSS_660 ) { csid->ops = &csid_ops_4_7; - }else if (camss->version ==CAMSS_845 ) { + }else if (camss->version ==CAMSS_845 || + camss->version ==CAMSS_8250 ) { csid->ops = &csid_ops_170;
csid_ops_170 should probably be renamed csid_ops_gen2 for clarity.
Quoted 9 lines
drivers/media/platform/qcom/camss/camss-csid.c +0
-3}else {return -EINVAL ;@@ -569,10 +570,21 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid,/* Memory */ - r =platform_get_resource_byname (pdev,IORESOURCE_MEM , res->reg[0 ]); - csid->base =devm_ioremap_resource (dev, r); -if (IS_ERR (csid->base))
drivers/media/platform/qcom/camss/camss-csid.c int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid, · line 581
Quoted 10 lines
drivers/media/platform/qcom/camss/camss-csid.c +9
-1-return PTR_ERR (csid->base); +if (camss->version ==CAMSS_8250 ) { +/* for titan 480, CSID registers are inside the VFE region, +* between the VFE "top" and "bus" registers. this requires +* VFE to be initialized before CSID +*/ +if (id >=2 )/* VFE/CSID lite */ + csid->base = camss->vfe[id].base +0x200 ; +else + csid->base = camss->vfe[id].base +0x1200 ;
Could these 0x200 & 0x1200 be replaced with defines?
Quoted 344 lines
drivers/media/platform/qcom/camss/camss-csid.c +6
-0+ }else { + r =platform_get_resource_byname (pdev,IORESOURCE_MEM , res->reg[0 ]); + csid->base =devm_ioremap_resource (dev, r); +if (IS_ERR (csid->base)) +return PTR_ERR (csid->base); + }/* Interrupt */ drivers/media/platform/qcom/camss/camss-csiphy.c +6
-2@@ -582,7 +582,8 @@ int msm_csiphy_subdev_init(struct camss *camss, csiphy->ops = &csiphy_ops_3ph_1_0; csiphy->formats = csiphy_formats_8x96; csiphy->nformats =ARRAY_SIZE (csiphy_formats_8x96); - }else if (camss->version ==CAMSS_845 ) { + }else if (camss->version ==CAMSS_845 || + camss->version ==CAMSS_8250 ) { csiphy->ops = &csiphy_ops_3ph_1_0; csiphy->formats = csiphy_formats_sdm845; csiphy->nformats =ARRAY_SIZE (csiphy_formats_sdm845);@@ -679,7 +680,10 @@ int msm_csiphy_subdev_init(struct camss *camss,if (!strcmp (clock->name,"csiphy0_timer" ) || !strcmp (clock->name,"csiphy1_timer" ) || - !strcmp (clock->name,"csiphy2_timer" )) + !strcmp (clock->name,"csiphy2_timer" ) || + !strcmp (clock->name,"csiphy3_timer" ) || + !strcmp (clock->name,"csiphy4_timer" ) || + !strcmp (clock->name,"csiphy5_timer" )) csiphy->rate_set[i] = true; if (camss->version == CAMSS_660 &&drivers/media/platform/qcom/camss/camss-vfe.c +7
-2@@ -220,7 +220,8 @@ static u32 vfe_src_pad_code(struct vfe_line *line, u32 sink_code, }else if (vfe->camss->version == CAMSS_8x96 || vfe->camss->version ==CAMSS_660 || - vfe->camss->version ==CAMSS_845 ) + vfe->camss->version ==CAMSS_845 || + vfe->camss->version ==CAMSS_8250 )switch (sink_code) {case MEDIA_BUS_FMT_YUYV8_2X8 : {@@ -1294,6 +1295,9 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,case CAMSS_845 : vfe->ops = &vfe_ops_170;break ; +case CAMSS_8250 : + vfe->ops = &vfe_ops_480; +break ;default :return -EINVAL ; }@@ -1405,7 +1409,8 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe, l->formats = formats_rdi_8x96; l->nformats =ARRAY_SIZE (formats_rdi_8x96); } - }else if (camss->version ==CAMSS_845 ) { + }else if (camss->version ==CAMSS_845 || + camss->version ==CAMSS_8250 ) { l->formats = formats_rdi_845; l->nformats =ARRAY_SIZE (formats_rdi_845); }else {drivers/media/platform/qcom/camss/camss-video.c +2
-1@@ -1011,7 +1011,8 @@ int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev, video->formats = formats_rdi_8x96; video->nformats =ARRAY_SIZE (formats_rdi_8x96); } - }else if (video->camss->version ==CAMSS_845 ) { + }else if (video->camss->version ==CAMSS_845 || + video->camss->version ==CAMSS_8250 ) { video->formats = formats_rdi_845; video->nformats =ARRAY_SIZE (formats_rdi_845); }else {drivers/media/platform/qcom/camss/camss.c +184
-12@@ -662,6 +662,162 @@ static const struct resources vfe_res_845[] = { } }; +static const struct resources csiphy_res_8250[] = { +/* CSIPHY0 */ + { + .regulator = {NULL }, + .clock = {"csiphy0" ,"csiphy0_timer" }, + .clock_rate = { {400000000 }, + {300000000 } }, + .reg = {"csiphy0" }, + .interrupt = {"csiphy0" } + }, +/* CSIPHY1 */ + { + .regulator = {NULL }, + .clock = {"csiphy1" ,"csiphy1_timer" }, + .clock_rate = { {400000000 }, + {300000000 } }, + .reg = {"csiphy1" }, + .interrupt = {"csiphy1" } + }, +/* CSIPHY2 */ + { + .regulator = {NULL }, + .clock = {"csiphy2" ,"csiphy2_timer" }, + .clock_rate = { {400000000 }, + {300000000 } }, + .reg = {"csiphy2" }, + .interrupt = {"csiphy2" } + }, +/* CSIPHY3 */ + { + .regulator = {NULL }, + .clock = {"csiphy3" ,"csiphy3_timer" }, + .clock_rate = { {400000000 }, + {300000000 } }, + .reg = {"csiphy3" }, + .interrupt = {"csiphy3" } + }, +/* CSIPHY4 */ + { + .regulator = {NULL }, + .clock = {"csiphy4" ,"csiphy4_timer" }, + .clock_rate = { {400000000 }, + {300000000 } }, + .reg = {"csiphy4" }, + .interrupt = {"csiphy4" } + }, +/* CSIPHY5 */ + { + .regulator = {NULL }, + .clock = {"csiphy5" ,"csiphy5_timer" }, + .clock_rate = { {400000000 }, + {300000000 } }, + .reg = {"csiphy5" }, + .interrupt = {"csiphy5" } + } +}; + +static const struct resources csid_res_8250[] = { +/* CSID0 */ + { + .regulator = {NULL }, + .clock = {"vfe0_csid" ,"vfe0_cphy_rx" ,"vfe0" ,"vfe0_areg" ,"vfe0_ahb" }, + .clock_rate = { {400000000 }, + {400000000 }, + {350000000 ,475000000 ,576000000 ,720000000 }, + {100000000 ,200000000 ,300000000 ,400000000 }, + {0 } }, + .reg = {"csid0" }, + .interrupt = {"csid0" } + }, +/* CSID1 */ + { + .regulator = {NULL }, + .clock = {"vfe1_csid" ,"vfe1_cphy_rx" ,"vfe1" ,"vfe1_areg" ,"vfe1_ahb" }, + .clock_rate = { {400000000 }, + {400000000 }, + {350000000 ,475000000 ,576000000 ,720000000 }, + {100000000 ,200000000 ,300000000 ,400000000 }, + {0 } }, + .reg = {"csid1" }, + .interrupt = {"csid1" } + }, +/* CSID2 */ + { + .regulator = {NULL }, + .clock = {"vfe_lite_csid" ,"vfe_lite_cphy_rx" ,"vfe_lite" ,"vfe_lite_ahb" }, + .clock_rate = { {400000000 }, + {400000000 }, + {400000000 ,480000000 }, + {0 } }, + .reg = {"csid2" }, + .interrupt = {"csid2" } + }, +/* CSID3 */ + { + .regulator = {NULL }, + .clock = {"vfe_lite_csid" ,"vfe_lite_cphy_rx" ,"vfe_lite" ,"vfe_lite_ahb" }, + .clock_rate = { {400000000 }, + {400000000 }, + {400000000 ,480000000 }, + {0 } }, + .reg = {"csid3" }, + .interrupt = {"csid3" } + } +}; + +static const struct resources vfe_res_8250[] = { +/* VFE0 */ + { + .regulator = {NULL }, + .clock = {"vfe0_ahb" ,"vfe0_areg" ,"vfe0" ,"vfe0_axi" ,"cam_hf_axi" }, + .clock_rate = { {0 }, + {100000000 ,200000000 ,300000000 ,400000000 }, + {350000000 ,475000000 ,576000000 ,720000000 }, + {0 }, + {0 } }, + .reg = {"vfe0" }, + .interrupt = {"vfe0" } + }, +/* VFE1 */ + { + .regulator = {NULL }, + .clock = {"vfe1_ahb" ,"vfe1_areg" ,"vfe1" ,"vfe1_axi" ,"cam_hf_axi" }, + .clock_rate = { {0 }, + {100000000 ,200000000 ,300000000 ,400000000 }, + {350000000 ,475000000 ,576000000 ,720000000 }, + {0 }, + {0 } }, + .reg = {"vfe1" }, + .interrupt = {"vfe1" } + }, +/* VFE2 (lite) */ + { + .regulator = {NULL }, + .clock = {"vfe_lite_ahb" ,"vfe_lite_axi" ,"vfe_lite" ,"cam_hf_axi" }, + .clock_rate = { {0 }, + {0 }, + {400000000 ,480000000 }, + {0 } }, + .reg = {"vfe_lite0" }, + .interrupt = {"vfe_lite0" } + }, + +/* VFE3 (lite) */ + { + .regulator = {NULL }, + .clock = {"vfe_lite_ahb" ,"vfe_lite_axi" ,"vfe_lite" ,"cam_hf_axi" }, + .clock_rate = { {0 }, + {0 }, + {400000000 ,480000000 }, + {0 } }, + .reg = {"vfe_lite1" }, + .interrupt = {"vfe_lite1" } + }, +}; +/* * camss_add_clock_margin - Add margin to clock frequency rate * @rate: Clock frequency rate @@ -945,6 +1101,12 @@ static int camss_init_subdevices(struct camss *camss)/* Titan VFEs don't have an ISPIF */ ispif_res =NULL ; vfe_res = vfe_res_845; + }else if (camss->version ==CAMSS_8250 ) { + csiphy_res = csiphy_res_8250; + csid_res = csid_res_8250; +/* Titan VFEs don't have an ISPIF */ + ispif_res =NULL ; + vfe_res = vfe_res_8250; }else {return -EINVAL ; }@@ -960,6 +1122,17 @@ static int camss_init_subdevices(struct camss *camss) } } +/* note: SM8250 requires VFE to be initialized before CSID */ +for (i =0 ; i < camss->vfe_num; i++) { + ret =msm_vfe_subdev_init (camss, &camss->vfe[i], + &vfe_res[i], i); +if (ret <0 ) { +dev_err (camss->dev, +"Fail to init vfe%d sub-device: %d\n" , i, ret); +return ret; + } + } +for (i =0 ; i < camss->csid_num; i++) { ret =msm_csid_subdev_init (camss, &camss->csid[i], &csid_res[i], i);@@ -978,16 +1151,6 @@ static int camss_init_subdevices(struct camss *camss)return ret; } -for (i =0 ; i < camss->vfe_num; i++) { - ret =msm_vfe_subdev_init (camss, &camss->vfe[i], - &vfe_res[i], i); -if (ret <0 ) { -dev_err (camss->dev, -"Fail to init vfe%d sub-device: %d\n" , i, ret); -return ret; - } - } -return 0 ; }@@ -1250,7 +1413,8 @@ static int camss_configure_pd(struct camss *camss)if (camss->version == CAMSS_8x96 || camss->version ==CAMSS_660 ) nbr_pm_domains =PM_DOMAIN_GEN1_COUNT ; -else if (camss->version ==CAMSS_845 ) +else if (camss->version ==CAMSS_845 || + camss->version ==CAMSS_8250 ) nbr_pm_domains =PM_DOMAIN_GEN2_COUNT ;for (i =0 ; i < nbr_pm_domains; i++) {@@ -1326,6 +1490,12 @@ static int camss_probe(struct platform_device *pdev) camss->csiphy_num =4 ; camss->csid_num =3 ; camss->vfe_num =3 ; + }else if (of_device_is_compatible (dev->of_node, +"qcom,sm8250-camss" )) { + camss->version =CAMSS_8250 ; + camss->csiphy_num =6 ; + camss->csid_num =4 ; + camss->vfe_num =4 ; }else { ret = -EINVAL ; goto err_free;@@ -1457,7 +1627,8 @@ void camss_delete(struct camss *camss)if (camss->version == CAMSS_8x96 || camss->version ==CAMSS_660 ) nbr_pm_domains =PM_DOMAIN_GEN1_COUNT ; -else if (camss->version ==CAMSS_845 ) +else if (camss->version ==CAMSS_845 || + camss->version ==CAMSS_8250 ) nbr_pm_domains =PM_DOMAIN_GEN2_COUNT ;for (i =0 ; i < nbr_pm_domains; i++) {@@ -1493,6 +1664,7 @@ static const struct of_device_id camss_dt_match[] = { { .compatible = "qcom,msm8996-camss" }, { .compatible = "qcom,sdm660-camss" }, { .compatible = "qcom,sdm845-camss" }, + { .compatible = "qcom,sm8250-camss" }, { } };drivers/media/platform/qcom/camss/camss.h +0
-0
drivers/media/platform/qcom/camss/camss.h enum camss_version { · line 75
Quoted 7 lines
drivers/media/platform/qcom/camss/camss.h +1
-0@@ -69,6 +69,7 @@ enum camss_version { CAMSS_8x96,CAMSS_660 ,CAMSS_845 , + CAMSS_8250, }; struct camss {
With the above issues fixed, feel free to add my r-b.
PATCH 17/17 media: dt-bindings: media: camss: Add qcom,sm8250-camss binding
In reply to Jonathan MarekAdd bindings for qcom,sm8250-camss in order to support the camera subsystem for SM8250.
--- .../bindings/media/qcom,sm8250-camss.yaml | 398 ++++++++++++++++++ 1 file changed, 398 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml new file +398 -0
@@ -0,0 +1,398 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) + +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/media/qcom,sm8250-camss.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Qualcomm CAMSS ISP + +maintainers: + - Robert Foss <[email protected]> + +description: | + The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms + +properties: + compatible: + const: qcom,sm8250-camss + + clocks: + minItems: 30 + maxItems: 30 + + clock-names: + items: + - const: cam_hf_axi + - const: csiphy0 + - const: csiphy0_timer + - const: csiphy1 + - const: csiphy1_timer + - const: csiphy2 + - const: csiphy2_timer + - const: csiphy3 + - const: csiphy3_timer + - const: csiphy4 + - const: csiphy4_timer + - const: csiphy5 + - const: csiphy5_timer + - const: vfe0_ahb + - const: vfe0_axi + - const: vfe0 + - const: vfe0_cphy_rx + - const: vfe0_csid + - const: vfe0_areg + - const: vfe1_ahb + - const: vfe1_axi + - const: vfe1 + - const: vfe1_cphy_rx + - const: vfe1_csid + - const: vfe1_areg + - const: vfe_lite_ahb + - const: vfe_lite_axi + - const: vfe_lite + - const: vfe_lite_cphy_rx + - const: vfe_lite_csid + + interrupts: + minItems: 14 + maxItems: 14 + + interrupt-names: + items: + - const: csid0 + - const: csid1 + - const: csid2 + - const: csid3 + - const: csiphy0 + - const: csiphy1 + - const: csiphy2 + - const: csiphy3 + - const: csiphy4 + - const: csiphy5 + - const: vfe0 + - const: vfe1 + - const: vfe_lite0 + - const: vfe_lite1 + + iommus: + maxItems: 1 + + power-domains: + items: + - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller. + - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller. + - description: Titan GDSC - Titan ISP Block, Global Distributed Switch Controller. + + ports: + $ref: /schemas/graph.yaml#/properties/ports + + description: + CSI input ports. + + properties: + port@0: + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + description: + Input port for receiving CSI data. + + properties: + endpoint: + $ref: video-interfaces.yaml# + unevaluatedProperties: false + + properties: + clock-lanes: + maxItems: 1 + + data-lanes: + minItems: 1 + maxItems: 4 + + required: + - clock-lanes + - data-lanes + + port@1: + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + description: + Input port for receiving CSI data. + + properties: + endpoint: + $ref: video-interfaces.yaml# + unevaluatedProperties: false + + properties: + clock-lanes: + maxItems: 1 + + data-lanes: + minItems: 1 + maxItems: 4 + + required: + - clock-lanes + - data-lanes + + port@2: + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + description: + Input port for receiving CSI data. + + properties: + endpoint: + $ref: video-interfaces.yaml# + unevaluatedProperties: false + + properties: + clock-lanes: + maxItems: 1 + + data-lanes: + minItems: 1 + maxItems: 4 + + required: + - clock-lanes + - data-lanes + + port@3: + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + description: + Input port for receiving CSI data. + + properties: + endpoint: + $ref: video-interfaces.yaml# + unevaluatedProperties: false + + properties: + clock-lanes: + maxItems: 1 + + data-lanes: + minItems: 1 + maxItems: 4 + + required: + - clock-lanes + - data-lanes + + port@4: + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + description: + Input port for receiving CSI data. + + properties: + endpoint: + $ref: video-interfaces.yaml# + unevaluatedProperties: false + + properties: + clock-lanes: + maxItems: 1 + + data-lanes: + minItems: 1 + maxItems: 4 + + required: + - clock-lanes + - data-lanes + + port@5: + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + description: + Input port for receiving CSI data. + + properties: + endpoint: + $ref: video-interfaces.yaml# + unevaluatedProperties: false + + properties: + clock-lanes: + maxItems: 1 + + data-lanes: + minItems: 1 + maxItems: 4 + + required: + - clock-lanes + - data-lanes + + reg: + minItems: 10 + maxItems: 10 + + reg-names: + items: + - const: csiphy0 + - const: csiphy1 + - const: csiphy2 + - const: csiphy3 + - const: csiphy4 + - const: csiphy5 + - const: vfe0 + - const: vfe1 + - const: vfe_lite0 + - const: vfe_lite1 + +required: + - clock-names + - clocks + - compatible + - interrupt-names + - interrupts + - iommus + - power-domains + - reg + - reg-names + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/qcom,camcc-sm8250.h> + #include <dt-bindings/clock/qcom,gcc-sm8250.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + + camss: camss@ac6a000 { + compatible = "qcom,sm8250-camss"; + + reg = <0 0xac6a000 0 0x2000>, + <0 0xac6c000 0 0x2000>, + <0 0xac6e000 0 0x1000>, + <0 0xac70000 0 0x1000>, + <0 0xac72000 0 0x1000>, + <0 0xac74000 0 0x1000>, + <0 0xacb4000 0 0xd000>, + <0 0xacc3000 0 0xd000>, + <0 0xacd9000 0 0x2200>, + <0 0xacdb200 0 0x2200>; + reg-names = "csiphy0", + "csiphy1", + "csiphy2", + "csiphy3", + "csiphy4", + "csiphy5", + "vfe0", + "vfe1", + "vfe_lite0", + "vfe_lite1"; + + interrupts = <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 448 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 359 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 469 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 360 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "csiphy0", + "csiphy1", + "csiphy2", + "csiphy3", + "csiphy4", + "csiphy5", + "csid0", + "csid1", + "csid2", + "csid3", + "vfe0", + "vfe1", + "vfe_lite0", + "vfe_lite1"; + + power-domains = <&camcc IFE_0_GDSC>, + <&camcc IFE_1_GDSC>, + <&camcc TITAN_TOP_GDSC>; + + clocks = <&gcc GCC_CAMERA_HF_AXI_CLK>, + <&camcc CAM_CC_CSIPHY0_CLK>, + <&camcc CAM_CC_CSI0PHYTIMER_CLK>, + <&camcc CAM_CC_CSIPHY1_CLK>, + <&camcc CAM_CC_CSI1PHYTIMER_CLK>, + <&camcc CAM_CC_CSIPHY2_CLK>, + <&camcc CAM_CC_CSI2PHYTIMER_CLK>, + <&camcc CAM_CC_CSIPHY3_CLK>, + <&camcc CAM_CC_CSI3PHYTIMER_CLK>, + <&camcc CAM_CC_CSIPHY4_CLK>, + <&camcc CAM_CC_CSI4PHYTIMER_CLK>, + <&camcc CAM_CC_CSIPHY5_CLK>, + <&camcc CAM_CC_CSI5PHYTIMER_CLK>, + <&camcc CAM_CC_IFE_0_AHB_CLK>, + <&camcc CAM_CC_IFE_0_AXI_CLK>, + <&camcc CAM_CC_IFE_0_CLK>, + <&camcc CAM_CC_IFE_0_CPHY_RX_CLK>, + <&camcc CAM_CC_IFE_0_CSID_CLK>, + <&camcc CAM_CC_IFE_0_AREG_CLK>, + <&camcc CAM_CC_IFE_1_AHB_CLK>, + <&camcc CAM_CC_IFE_1_AXI_CLK>, + <&camcc CAM_CC_IFE_1_CLK>, + <&camcc CAM_CC_IFE_1_CPHY_RX_CLK>, + <&camcc CAM_CC_IFE_1_CSID_CLK>, + <&camcc CAM_CC_IFE_1_AREG_CLK>, + <&camcc CAM_CC_IFE_LITE_AHB_CLK>, + <&camcc CAM_CC_IFE_LITE_AXI_CLK>, + <&camcc CAM_CC_IFE_LITE_CLK>, + <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>, + <&camcc CAM_CC_IFE_LITE_CSID_CLK>; + + clock-names = "cam_hf_axi", + "csiphy0", + "csiphy0_timer", + "csiphy1", + "csiphy1_timer", + "csiphy2", + "csiphy2_timer", + "csiphy3", + "csiphy3_timer", + "csiphy4", + "csiphy4_timer", + "csiphy5", + "csiphy5_timer", + "vfe0_ahb", + "vfe0_axi", + "vfe0", + "vfe0_cphy_rx", + "vfe0_csid", + "vfe0_areg", + "vfe1_ahb", + "vfe1_axi", + "vfe1", + "vfe1_cphy_rx", + "vfe1_csid", + "vfe1_areg", + "vfe_lite_ahb", + "vfe_lite_axi", + "vfe_lite", + "vfe_lite_cphy_rx", + "vfe_lite_csid"; + + iommus = <&apps_smmu 0x800 0x400>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + }; + }; + };
Signature · 1 line
2.26.1
Rob Herring
maintainerRe: [PATCH 17/17] media: dt-bindings: media: camss: Add qcom,sm8250-camss binding
Quoted 9 lines · Jonathan Marek
Add bindings for qcom,sm8250-camss in order to support the camera subsystem for SM8250.
Signed-off-by Jonathan Marek <[email protected]>--- .../bindings/media/qcom,sm8250-camss.yaml | 398 ++++++++++++++++++ 1 file changed, 398 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:Documentation/devicetree/bindings/media/qcom,sm8250-camss.example.dts:20:18: fatal error: dt-bindings/clock/qcom,camcc-sm8250.h: No such file or directory
20 | #include <dt-bindings/clock/qcom,camcc-sm8250.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/media/qcom,sm8250-camss.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1416: dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1477274
This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
pip3 install dtschema --upgrade
Please check and re-submit.