[PATCH 0/6] Add support for SDM630/660 Camera Subsystem

[PATCH 0/6] Add support for SDM630/660 Camera Subsystem

[kholk11]

From: AngeloGioacchino Del Regno <kholk11@gmail.com>

This patch series implements support for the entire camera subsystem
found in SDM630/636/660 and SDA variants, including CSIPHY 3-Phase,
CSID v5.0, ISPIF 3.0 (though it didn't need any adaptation) and
VFE 4.8.

One small note about VFE4.8, even if I wrote it in the commit that
adds support for it: I know, the VFE support here is split in
multiple files having the name of the actual VFE version that it is
targeting... but it didn't feel right to commonize the VFE 4.7 file
and make another one only for VFE4.8, when it's just about something
like 3 small differences.
That VFE 4.8 seems to be just a minor revision of VFE 4.7.

While at it, also fix a small issue when using two VFEs: only one
of them was being resetted (always VFE0) so, after the first usage
of VFE1, in case we leave it in a bad state, it would not properly
start again. Now... it's fine :)))

P.S.: SDM630/660's camss seems to be *very* similar to MSM8998, so
      likely 90% of this series should be reusable on that one, too!

Tested on:
 - Sony Xperia XA2 (IMX300 on CSI0/PHY0/VFE0, IMX219 on CSI2,PHY2,VFE1)
   * VFE0/1 RDI only, as the VIDEO one does not work with SRGGB Bayer
     formats yet. As far as I can see, that color format hasn't been
     implemented yet in the video interface.

AngeloGioacchino Del Regno (6):
  media: camss: csiphy-3ph: Add support for SDM630/660
  media: camss: ispif: Correctly reset based on the VFE ID
  media: camss: vfe: Add support for VFE 4.8
  media: camss: Add support for SDM630/636/660 camera subsystem
  media: dt-bindings: media: qcom,camss: Add bindings for SDM660 camss
  media: camss: csiphy: Set rate on csiX_phy clock on SDM630/660

 .../devicetree/bindings/media/qcom,camss.txt  |   7 +
 .../media/platform/qcom/camss/camss-csid.c    |   9 +-
 .../qcom/camss/camss-csiphy-3ph-1-0.c         |   7 +-
 .../media/platform/qcom/camss/camss-csiphy.c  |  25 ++-
 .../media/platform/qcom/camss/camss-csiphy.h  |   1 +
 .../media/platform/qcom/camss/camss-ispif.c   | 100 ++++++---
 .../media/platform/qcom/camss/camss-ispif.h   |   2 +-
 .../media/platform/qcom/camss/camss-vfe-4-7.c | 129 ++++++++++-
 drivers/media/platform/qcom/camss/camss-vfe.c |  19 +-
 drivers/media/platform/qcom/camss/camss-vfe.h |   1 +
 .../media/platform/qcom/camss/camss-video.c   |   3 +-
 drivers/media/platform/qcom/camss/camss.c     | 206 +++++++++++++++++-
 drivers/media/platform/qcom/camss/camss.h     |   1 +
 13 files changed, 448 insertions(+), 62 deletions(-)

-- 
2.28.0

[PATCH 1/6] media: camss: csiphy-3ph: Add support for SDM630/660

[kholk11]

From: AngeloGioacchino Del Regno <kholk11@gmail.com>

The CSIPHY on SDM630/660 needs a slightly longer T_HS_CLK_MISS
configuration on lanes CFG4.

Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
 drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
index 2e65caf1ecae..97cb9de85031 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
@@ -8,6 +8,7 @@
  * Copyright (C) 2016-2018 Linaro Ltd.
  */
 
+#include "camss.h"
 #include "camss-csiphy.h"
 
 #include <linux/delay.h>
@@ -21,6 +22,7 @@
 #define CSIPHY_3PH_LNn_CFG3(n)			(0x008 + 0x100 * (n))
 #define CSIPHY_3PH_LNn_CFG4(n)			(0x00c + 0x100 * (n))
 #define CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS	0xa4
+#define CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS_660	0xa5
 #define CSIPHY_3PH_LNn_CFG5(n)			(0x010 + 0x100 * (n))
 #define CSIPHY_3PH_LNn_CFG5_T_HS_DTERM		0x02
 #define CSIPHY_3PH_LNn_CFG5_HS_REC_EQ_FQ_INT	0x50
@@ -198,7 +200,10 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
 	val = CSIPHY_3PH_LNn_CFG1_SWI_REC_DLY_PRG;
 	writel_relaxed(val, csiphy->base + CSIPHY_3PH_LNn_CFG1(l));
 
-	val = CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS;
+	if (csiphy->camss->version == CAMSS_660)
+		val = CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS_660;
+	else
+		val = CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS;
 	writel_relaxed(val, csiphy->base + CSIPHY_3PH_LNn_CFG4(l));
 
 	val = CSIPHY_3PH_LNn_MISC1_IS_CLKLANE;
-- 
2.28.0

[PATCH 2/6] media: camss: ispif: Correctly reset based on the VFE ID

[kholk11]

From: AngeloGioacchino Del Regno <kholk11@gmail.com>

Resetting the ISPIF VFE0 context is wrong if we are using the VFE1
for dual-camera or simply because a secondary camera is connected
to it: in this case the reset will always happen on the VFE0 ctx
of the ISPIF, which is .. useless.

Fix this usecase by adding the ISPIF_RST_CMD_1 address and choose
where to do the (or what to) reset based on the VFE line id.

Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
 .../media/platform/qcom/camss/camss-ispif.c   | 87 ++++++++++++-------
 .../media/platform/qcom/camss/camss-ispif.h   |  2 +-
 2 files changed, 57 insertions(+), 32 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-ispif.c b/drivers/media/platform/qcom/camss/camss-ispif.c
index db94cfd6c508..252db6b33dab 100644
--- a/drivers/media/platform/qcom/camss/camss-ispif.c
+++ b/drivers/media/platform/qcom/camss/camss-ispif.c
@@ -26,6 +26,7 @@
 #define MSM_ISPIF_NAME "msm_ispif"
 
 #define ISPIF_RST_CMD_0			0x008
+#define ISPIF_RST_CMD_1			0x00c
 #define ISPIF_RST_CMD_0_STROBED_RST_EN		(1 << 0)
 #define ISPIF_RST_CMD_0_MISC_LOGIC_RST		(1 << 1)
 #define ISPIF_RST_CMD_0_SW_REG_RST		(1 << 2)
@@ -179,7 +180,10 @@ static irqreturn_t ispif_isr_8x96(int irq, void *dev)
 	writel(0x1, ispif->base + ISPIF_IRQ_GLOBAL_CLEAR_CMD);
 
 	if ((value0 >> 27) & 0x1)
-		complete(&ispif->reset_complete);
+		complete(&ispif->reset_complete[0]);
+
+	if ((value3 >> 27) & 0x1)
+		complete(&ispif->reset_complete[1]);
 
 	if (unlikely(value0 & ISPIF_VFE_m_IRQ_STATUS_0_PIX0_OVERFLOW))
 		dev_err_ratelimited(to_device(ispif), "VFE0 pix0 overflow\n");
@@ -237,7 +241,7 @@ static irqreturn_t ispif_isr_8x16(int irq, void *dev)
 	writel(0x1, ispif->base + ISPIF_IRQ_GLOBAL_CLEAR_CMD);
 
 	if ((value0 >> 27) & 0x1)
-		complete(&ispif->reset_complete);
+		complete(&ispif->reset_complete[0]);
 
 	if (unlikely(value0 & ISPIF_VFE_m_IRQ_STATUS_0_PIX0_OVERFLOW))
 		dev_err_ratelimited(to_device(ispif), "VFE0 pix0 overflow\n");
@@ -257,33 +261,17 @@ static irqreturn_t ispif_isr_8x16(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
-/*
- * ispif_reset - Trigger reset on ISPIF module and wait to complete
- * @ispif: ISPIF device
- *
- * Return 0 on success or a negative error code otherwise
- */
-static int ispif_reset(struct ispif_device *ispif)
+static int ispif_vfe_reset(struct ispif_device *ispif, u8 vfe_id)
 {
-	unsigned long time;
 	u32 val;
-	int ret;
-
-	ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE0);
-	if (ret < 0)
-		return ret;
 
-	ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE1);
-	if (ret < 0)
-		return ret;
-
-	ret = camss_enable_clocks(ispif->nclocks_for_reset,
-				  ispif->clock_for_reset,
-				  to_device(ispif));
-	if (ret < 0)
-		return ret;
+	if (vfe_id > (to_camss(ispif)->vfe_num - 1)) {
+		dev_err(to_device(ispif),
+			"Error: asked reset for invalid VFE%d\n", vfe_id);
+		return -ENOENT;
+	}
 
-	reinit_completion(&ispif->reset_complete);
+	reinit_completion(&ispif->reset_complete[vfe_id]);
 
 	val = ISPIF_RST_CMD_0_STROBED_RST_EN |
 		ISPIF_RST_CMD_0_MISC_LOGIC_RST |
@@ -303,15 +291,51 @@ static int ispif_reset(struct ispif_device *ispif)
 		ISPIF_RST_CMD_0_RDI_OUTPUT_1_MISR_RST |
 		ISPIF_RST_CMD_0_RDI_OUTPUT_2_MISR_RST;
 
-	writel_relaxed(val, ispif->base + ISPIF_RST_CMD_0);
+	if (vfe_id == 1)
+		writel_relaxed(val, ispif->base + ISPIF_RST_CMD_1);
+	else
+		writel_relaxed(val, ispif->base + ISPIF_RST_CMD_0);
 
-	time = wait_for_completion_timeout(&ispif->reset_complete,
+	time = wait_for_completion_timeout(&ispif->reset_complete[vfe_id],
 		msecs_to_jiffies(ISPIF_RESET_TIMEOUT_MS));
 	if (!time) {
-		dev_err(to_device(ispif), "ISPIF reset timeout\n");
-		ret = -EIO;
+		dev_err(to_device(ispif),
+			"ISPIF for VFE%d reset timeout\n", vfe_id);
+		return -EIO;
 	}
 
+	return 0;
+}
+
+/*
+ * ispif_reset - Trigger reset on ISPIF module and wait to complete
+ * @ispif: ISPIF device
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int ispif_reset(struct ispif_device *ispif, u8 vfe_id)
+{
+	unsigned long time;
+	int ret;
+
+	ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE0);
+	if (ret < 0)
+		return ret;
+
+	ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE1);
+	if (ret < 0)
+		return ret;
+
+	ret = camss_enable_clocks(ispif->nclocks_for_reset,
+				  ispif->clock_for_reset,
+				  to_device(ispif));
+	if (ret < 0)
+		return ret;
+
+	ret = ispif_vfe_reset(ispif, vfe_id);
+	if (ret)
+		dev_dbg(to_device(ispif), "ISPIF Reset failed\n");
+
 	camss_disable_clocks(ispif->nclocks_for_reset, ispif->clock_for_reset);
 
 	camss_pm_domain_off(to_camss(ispif), PM_DOMAIN_VFE0);
@@ -355,7 +379,7 @@ static int ispif_set_power(struct v4l2_subdev *sd, int on)
 			goto exit;
 		}
 
-		ret = ispif_reset(ispif);
+		ret = ispif_reset(ispif, line->vfe_id);
 		if (ret < 0) {
 			pm_runtime_put_sync(dev);
 			camss_disable_clocks(ispif->nclocks, ispif->clock);
@@ -1192,7 +1216,8 @@ int msm_ispif_subdev_init(struct ispif_device *ispif,
 
 	mutex_init(&ispif->config_lock);
 
-	init_completion(&ispif->reset_complete);
+	for (i = 0; i < MSM_ISPIF_VFE_NUM; i++)
+		init_completion(&ispif->reset_complete[i]);
 
 	return 0;
 }
diff --git a/drivers/media/platform/qcom/camss/camss-ispif.h b/drivers/media/platform/qcom/camss/camss-ispif.h
index 1a5ba2425a42..4132174f7ea1 100644
--- a/drivers/media/platform/qcom/camss/camss-ispif.h
+++ b/drivers/media/platform/qcom/camss/camss-ispif.h
@@ -56,7 +56,7 @@ struct ispif_device {
 	int nclocks;
 	struct camss_clock  *clock_for_reset;
 	int nclocks_for_reset;
-	struct completion reset_complete;
+	struct completion reset_complete[MSM_ISPIF_VFE_NUM];
 	int power_count;
 	struct mutex power_lock;
 	struct ispif_intf_cmd_reg intf_cmd[MSM_ISPIF_VFE_NUM];
-- 
2.28.0

[PATCH 3/6] media: camss: vfe: Add support for VFE 4.8

[kholk11]

From: AngeloGioacchino Del Regno <kholk11@gmail.com>

Add the support for VFE 4.8 in the camss-vfe-4-7 driver, as this one
really is a minor revision, requiring the very same management and
basically having the same register layout as VFE 4.7, but needing
a different QoS and DS configuration, using a different register to
enable the wm and habing the same UB size for both instances (instead
of a different size between instance 0 and 1).

Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
 .../media/platform/qcom/camss/camss-vfe-4-7.c | 129 ++++++++++++++++--
 drivers/media/platform/qcom/camss/camss-vfe.h |   1 +
 2 files changed, 122 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
index 0dca8bf9281e..e48d58a4a9d1 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
@@ -133,6 +133,11 @@
 #define VFE_0_BUS_BDG_QOS_CFG_7		0x420
 #define VFE_0_BUS_BDG_QOS_CFG_7_CFG	0x0001aaa9
 
+#define VFE48_0_BUS_BDG_QOS_CFG_0_CFG	0xaaa5aaa5
+#define VFE48_0_BUS_BDG_QOS_CFG_3_CFG	0xaa55aaa5
+#define VFE48_0_BUS_BDG_QOS_CFG_4_CFG	0xaa55aa55
+#define VFE48_0_BUS_BDG_QOS_CFG_7_CFG	0x0005aa55
+
 #define VFE_0_BUS_BDG_DS_CFG_0		0x424
 #define VFE_0_BUS_BDG_DS_CFG_0_CFG	0xcccc0011
 #define VFE_0_BUS_BDG_DS_CFG_1		0x428
@@ -153,6 +158,9 @@
 #define VFE_0_BUS_BDG_DS_CFG_16		0x464
 #define VFE_0_BUS_BDG_DS_CFG_16_CFG	0x40000103
 
+#define VFE48_0_BUS_BDG_DS_CFG_0_CFG	0xcccc1111
+#define VFE48_0_BUS_BDG_DS_CFG_16_CFG	0x00000110
+
 #define VFE_0_RDI_CFG_x(x)		(0x46c + (0x4 * (x)))
 #define VFE_0_RDI_CFG_x_RDI_STREAM_SEL_SHIFT	28
 #define VFE_0_RDI_CFG_x_RDI_STREAM_SEL_MASK	(0xf << 28)
@@ -231,6 +239,9 @@
 #define VFE_0_REALIGN_BUF_CFG_CR_ODD_PIXEL     BIT(3)
 #define VFE_0_REALIGN_BUF_CFG_HSUB_ENABLE      BIT(4)
 
+#define VFE48_0_BUS_IMAGE_MASTER_CMD		0xcec
+#define VFE48_0_BUS_IMAGE_MASTER_n_SHIFT(x)	(2 * (x))
+
 #define CAMIF_TIMEOUT_SLEEP_US 1000
 #define CAMIF_TIMEOUT_ALL_US 1000000
 
@@ -246,7 +257,7 @@ static void vfe_hw_version_read(struct vfe_device *vfe, struct device *dev)
 	dev_err(dev, "VFE HW Version = 0x%08x\n", hw_version);
 }
 
-static u16 vfe_get_ub_size(u8 vfe_id)
+static u16 vfe47_get_ub_size(u8 vfe_id)
 {
 	if (vfe_id == 0)
 		return MSM_VFE_VFE0_UB_SIZE_RDI;
@@ -299,7 +310,7 @@ static void vfe_halt_clear(struct vfe_device *vfe)
 	writel_relaxed(0x0, vfe->base + VFE_0_BUS_BDG_CMD);
 }
 
-static void vfe_wm_enable(struct vfe_device *vfe, u8 wm, u8 enable)
+static void vfe47_wm_enable(struct vfe_device *vfe, u8 wm, u8 enable)
 {
 	if (enable)
 		vfe_reg_set(vfe, VFE_0_BUS_IMAGE_MASTER_n_WR_CFG(wm),
@@ -883,7 +894,7 @@ static void vfe_set_clamp_cfg(struct vfe_device *vfe)
 	writel_relaxed(val, vfe->base + VFE_0_CLAMP_ENC_MIN_CFG);
 }
 
-static void vfe_set_qos(struct vfe_device *vfe)
+static void vfe47_set_qos(struct vfe_device *vfe)
 {
 	u32 val = VFE_0_BUS_BDG_QOS_CFG_0_CFG;
 	u32 val7 = VFE_0_BUS_BDG_QOS_CFG_7_CFG;
@@ -898,7 +909,7 @@ static void vfe_set_qos(struct vfe_device *vfe)
 	writel_relaxed(val7, vfe->base + VFE_0_BUS_BDG_QOS_CFG_7);
 }
 
-static void vfe_set_ds(struct vfe_device *vfe)
+static void vfe47_set_ds(struct vfe_device *vfe)
 {
 	u32 val = VFE_0_BUS_BDG_DS_CFG_0_CFG;
 	u32 val16 = VFE_0_BUS_BDG_DS_CFG_16_CFG;
@@ -1098,11 +1109,113 @@ static irqreturn_t vfe_isr(int irq, void *dev)
 
 const struct vfe_hw_ops vfe_ops_4_7 = {
 	.hw_version_read = vfe_hw_version_read,
-	.get_ub_size = vfe_get_ub_size,
+	.get_ub_size = vfe47_get_ub_size,
+	.global_reset = vfe_global_reset,
+	.halt_request = vfe_halt_request,
+	.halt_clear = vfe_halt_clear,
+	.wm_enable = vfe47_wm_enable,
+	.wm_frame_based = vfe_wm_frame_based,
+	.wm_line_based = vfe_wm_line_based,
+	.wm_set_framedrop_period = vfe_wm_set_framedrop_period,
+	.wm_set_framedrop_pattern = vfe_wm_set_framedrop_pattern,
+	.wm_set_ub_cfg = vfe_wm_set_ub_cfg,
+	.bus_reload_wm = vfe_bus_reload_wm,
+	.wm_set_ping_addr = vfe_wm_set_ping_addr,
+	.wm_set_pong_addr = vfe_wm_set_pong_addr,
+	.wm_get_ping_pong_status = vfe_wm_get_ping_pong_status,
+	.bus_enable_wr_if = vfe_bus_enable_wr_if,
+	.bus_connect_wm_to_rdi = vfe_bus_connect_wm_to_rdi,
+	.wm_set_subsample = vfe_wm_set_subsample,
+	.bus_disconnect_wm_from_rdi = vfe_bus_disconnect_wm_from_rdi,
+	.set_xbar_cfg = vfe_set_xbar_cfg,
+	.set_realign_cfg = vfe_set_realign_cfg,
+	.set_rdi_cid = vfe_set_rdi_cid,
+	.reg_update = vfe_reg_update,
+	.reg_update_clear = vfe_reg_update_clear,
+	.enable_irq_wm_line = vfe_enable_irq_wm_line,
+	.enable_irq_pix_line = vfe_enable_irq_pix_line,
+	.enable_irq_common = vfe_enable_irq_common,
+	.set_demux_cfg = vfe_set_demux_cfg,
+	.set_scale_cfg = vfe_set_scale_cfg,
+	.set_crop_cfg = vfe_set_crop_cfg,
+	.set_clamp_cfg = vfe_set_clamp_cfg,
+	.set_qos = vfe47_set_qos,
+	.set_ds = vfe47_set_ds,
+	.set_cgc_override = vfe_set_cgc_override,
+	.set_camif_cfg = vfe_set_camif_cfg,
+	.set_camif_cmd = vfe_set_camif_cmd,
+	.set_module_cfg = vfe_set_module_cfg,
+	.camif_wait_for_stop = vfe_camif_wait_for_stop,
+	.isr_read = vfe_isr_read,
+	.violation_read = vfe_violation_read,
+	.isr = vfe_isr,
+};
+
+static u16 vfe48_get_ub_size(u8 vfe_id)
+{
+	/* On VFE4.8 the ub-size is the same on both instances */
+	return MSM_VFE_VFE0_UB_SIZE_RDI;
+}
+
+static void vfe48_wm_enable(struct vfe_device *vfe, u8 wm, u8 enable)
+{
+	if (enable)
+		writel_relaxed(2 << VFE48_0_BUS_IMAGE_MASTER_n_SHIFT(wm),
+			       vfe->base + VFE48_0_BUS_IMAGE_MASTER_CMD);
+	else
+		writel_relaxed(1 << VFE48_0_BUS_IMAGE_MASTER_n_SHIFT(wm),
+			       vfe->base + VFE48_0_BUS_IMAGE_MASTER_CMD);
+	wmb();
+}
+
+static void vfe48_set_qos(struct vfe_device *vfe)
+{
+	u32 val = VFE48_0_BUS_BDG_QOS_CFG_0_CFG;
+	u32 val3 = VFE48_0_BUS_BDG_QOS_CFG_3_CFG;
+	u32 val4 = VFE48_0_BUS_BDG_QOS_CFG_4_CFG;
+	u32 val7 = VFE48_0_BUS_BDG_QOS_CFG_7_CFG;
+
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_QOS_CFG_0);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_QOS_CFG_1);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_QOS_CFG_2);
+	writel_relaxed(val3, vfe->base + VFE_0_BUS_BDG_QOS_CFG_3);
+	writel_relaxed(val4, vfe->base + VFE_0_BUS_BDG_QOS_CFG_4);
+	writel_relaxed(val4, vfe->base + VFE_0_BUS_BDG_QOS_CFG_5);
+	writel_relaxed(val4, vfe->base + VFE_0_BUS_BDG_QOS_CFG_6);
+	writel_relaxed(val7, vfe->base + VFE_0_BUS_BDG_QOS_CFG_7);
+}
+
+static void vfe48_set_ds(struct vfe_device *vfe)
+{
+	u32 val = VFE48_0_BUS_BDG_DS_CFG_0_CFG;
+	u32 val16 = VFE48_0_BUS_BDG_DS_CFG_16_CFG;
+
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_0);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_1);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_2);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_3);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_4);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_5);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_6);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_7);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_8);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_9);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_10);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_11);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_12);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_13);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_14);
+	writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_15);
+	writel_relaxed(val16, vfe->base + VFE_0_BUS_BDG_DS_CFG_16);
+}
+
+const struct vfe_hw_ops vfe_ops_4_8 = {
+	.hw_version_read = vfe_hw_version_read,
+	.get_ub_size = vfe48_get_ub_size,
 	.global_reset = vfe_global_reset,
 	.halt_request = vfe_halt_request,
 	.halt_clear = vfe_halt_clear,
-	.wm_enable = vfe_wm_enable,
+	.wm_enable = vfe48_wm_enable,
 	.wm_frame_based = vfe_wm_frame_based,
 	.wm_line_based = vfe_wm_line_based,
 	.wm_set_framedrop_period = vfe_wm_set_framedrop_period,
@@ -1128,8 +1241,8 @@ const struct vfe_hw_ops vfe_ops_4_7 = {
 	.set_scale_cfg = vfe_set_scale_cfg,
 	.set_crop_cfg = vfe_set_crop_cfg,
 	.set_clamp_cfg = vfe_set_clamp_cfg,
-	.set_qos = vfe_set_qos,
-	.set_ds = vfe_set_ds,
+	.set_qos = vfe48_set_qos,
+	.set_ds = vfe48_set_ds,
 	.set_cgc_override = vfe_set_cgc_override,
 	.set_camif_cfg = vfe_set_camif_cfg,
 	.set_camif_cmd = vfe_set_camif_cmd,
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h
index a90b0d2cc6de..5bce6736e4bb 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.h
+++ b/drivers/media/platform/qcom/camss/camss-vfe.h
@@ -180,5 +180,6 @@ void msm_vfe_get_vfe_line_id(struct media_entity *entity, enum vfe_line_id *id);
 
 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;
 
 #endif /* QC_MSM_CAMSS_VFE_H */
-- 
2.28.0

[PATCH 4/6] media: camss: Add support for SDM630/636/660 camera subsystem

[kholk11]

From: AngeloGioacchino Del Regno <kholk11@gmail.com>

Add support for the Qualcomm SDM630/636/660 and SDA variants' camera
subsystem. These SoCs are equipped with:
- 3x CSI PHY 3-Phase v1.0 (downstream csiphy-v3.5)
- 4x CSID v5.0
- 2x ISPIF v3.0
- 2x VFE 4.8
As a note, this camera subsystem is very similar to the one that
is found in the MSM8998/APQ8098 SoCs.

Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
 .../media/platform/qcom/camss/camss-csid.c    |   9 +-
 .../media/platform/qcom/camss/camss-csiphy.c  |   3 +-
 .../media/platform/qcom/camss/camss-ispif.c   |  17 +-
 drivers/media/platform/qcom/camss/camss-vfe.c |  19 +-
 .../media/platform/qcom/camss/camss-video.c   |   3 +-
 drivers/media/platform/qcom/camss/camss.c     | 206 +++++++++++++++++-
 drivers/media/platform/qcom/camss/camss.h     |   1 +
 7 files changed, 238 insertions(+), 20 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index 2ffcda06706b..be3fe76f3dc3 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -383,7 +383,8 @@ static u32 csid_src_pad_code(struct csid_device *csid, u32 sink_code,
 			return 0;
 
 		return sink_code;
-	} else if (csid->camss->version == CAMSS_8x96) {
+	} else if (csid->camss->version == CAMSS_8x96 ||
+		   csid->camss->version == CAMSS_660) {
 		switch (sink_code) {
 		case MEDIA_BUS_FMT_SBGGR10_1X10:
 		{
@@ -718,7 +719,8 @@ static int csid_set_stream(struct v4l2_subdev *sd, int enable)
 		val |= df << CAMSS_CSID_CID_n_CFG_DECODE_FORMAT_SHIFT;
 		val |= CAMSS_CSID_CID_n_CFG_RDI_MODE_RAW_DUMP;
 
-		if (csid->camss->version == CAMSS_8x96) {
+		if (csid->camss->version == CAMSS_8x96 ||
+		    csid->camss->version == CAMSS_660) {
 			u32 sink_code = csid->fmt[MSM_CSID_PAD_SINK].code;
 			u32 src_code = csid->fmt[MSM_CSID_PAD_SRC].code;
 
@@ -1098,7 +1100,8 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid,
 		csid->formats = csid_formats_8x16;
 		csid->nformats =
 				ARRAY_SIZE(csid_formats_8x16);
-	} else if (camss->version == CAMSS_8x96) {
+	} else if (camss->version == CAMSS_8x96 ||
+		   camss->version == CAMSS_660) {
 		csid->formats = csid_formats_8x96;
 		csid->nformats =
 				ARRAY_SIZE(csid_formats_8x96);
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index 03ef9c5f4774..c00f25aac21b 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -552,7 +552,8 @@ int msm_csiphy_subdev_init(struct camss *camss,
 		csiphy->ops = &csiphy_ops_2ph_1_0;
 		csiphy->formats = csiphy_formats_8x16;
 		csiphy->nformats = ARRAY_SIZE(csiphy_formats_8x16);
-	} else if (camss->version == CAMSS_8x96) {
+	} else if (camss->version == CAMSS_8x96 ||
+		   camss->version == CAMSS_660) {
 		csiphy->ops = &csiphy_ops_3ph_1_0;
 		csiphy->formats = csiphy_formats_8x96;
 		csiphy->nformats = ARRAY_SIZE(csiphy_formats_8x96);
diff --git a/drivers/media/platform/qcom/camss/camss-ispif.c b/drivers/media/platform/qcom/camss/camss-ispif.c
index 252db6b33dab..adeb92808998 100644
--- a/drivers/media/platform/qcom/camss/camss-ispif.c
+++ b/drivers/media/platform/qcom/camss/camss-ispif.c
@@ -263,6 +263,7 @@ static irqreturn_t ispif_isr_8x16(int irq, void *dev)
 
 static int ispif_vfe_reset(struct ispif_device *ispif, u8 vfe_id)
 {
+	unsigned long time;
 	u32 val;
 
 	if (vfe_id > (to_camss(ispif)->vfe_num - 1)) {
@@ -315,7 +316,6 @@ static int ispif_vfe_reset(struct ispif_device *ispif, u8 vfe_id)
  */
 static int ispif_reset(struct ispif_device *ispif, u8 vfe_id)
 {
-	unsigned long time;
 	int ret;
 
 	ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE0);
@@ -825,7 +825,8 @@ static int ispif_set_stream(struct v4l2_subdev *sd, int enable)
 		ispif_select_csid(ispif, intf, csid, vfe, 1);
 		ispif_select_cid(ispif, intf, cid, vfe, 1);
 		ispif_config_irq(ispif, intf, vfe, 1);
-		if (to_camss(ispif)->version == CAMSS_8x96)
+		if (to_camss(ispif)->version == CAMSS_8x96 ||
+		    to_camss(ispif)->version == CAMSS_660)
 			ispif_config_pack(ispif,
 					  line->fmt[MSM_ISPIF_PAD_SINK].code,
 					  intf, cid, vfe, 1);
@@ -842,7 +843,8 @@ static int ispif_set_stream(struct v4l2_subdev *sd, int enable)
 			return ret;
 
 		mutex_lock(&ispif->config_lock);
-		if (to_camss(ispif)->version == CAMSS_8x96)
+		if (to_camss(ispif)->version == CAMSS_8x96 ||
+		    to_camss(ispif)->version == CAMSS_660)
 			ispif_config_pack(ispif,
 					  line->fmt[MSM_ISPIF_PAD_SINK].code,
 					  intf, cid, vfe, 0);
@@ -1098,7 +1100,8 @@ int msm_ispif_subdev_init(struct ispif_device *ispif,
 	/* Number of ISPIF lines - same as number of CSID hardware modules */
 	if (to_camss(ispif)->version == CAMSS_8x16)
 		ispif->line_num = 2;
-	else if (to_camss(ispif)->version == CAMSS_8x96)
+	else if (to_camss(ispif)->version == CAMSS_8x96 ||
+		 to_camss(ispif)->version == CAMSS_660)
 		ispif->line_num = 4;
 	else
 		return -EINVAL;
@@ -1116,7 +1119,8 @@ int msm_ispif_subdev_init(struct ispif_device *ispif,
 			ispif->line[i].formats = ispif_formats_8x16;
 			ispif->line[i].nformats =
 					ARRAY_SIZE(ispif_formats_8x16);
-		} else if (to_camss(ispif)->version == CAMSS_8x96) {
+		} else if (to_camss(ispif)->version == CAMSS_8x96 ||
+			   to_camss(ispif)->version == CAMSS_660) {
 			ispif->line[i].formats = ispif_formats_8x96;
 			ispif->line[i].nformats =
 					ARRAY_SIZE(ispif_formats_8x96);
@@ -1156,7 +1160,8 @@ int msm_ispif_subdev_init(struct ispif_device *ispif,
 	if (to_camss(ispif)->version == CAMSS_8x16)
 		ret = devm_request_irq(dev, ispif->irq, ispif_isr_8x16,
 			       IRQF_TRIGGER_RISING, ispif->irq_name, ispif);
-	else if (to_camss(ispif)->version == CAMSS_8x96)
+	else if (to_camss(ispif)->version == CAMSS_8x96 ||
+		 to_camss(ispif)->version == CAMSS_660)
 		ret = devm_request_irq(dev, ispif->irq, ispif_isr_8x96,
 			       IRQF_TRIGGER_RISING, ispif->irq_name, ispif);
 	else
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index b7d2293a5004..fae2b513b2f9 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -205,7 +205,8 @@ static u32 vfe_src_pad_code(struct vfe_line *line, u32 sink_code,
 
 			return sink_code;
 		}
-	else if (vfe->camss->version == CAMSS_8x96)
+	else if (vfe->camss->version == CAMSS_8x96 ||
+		 vfe->camss->version == CAMSS_660)
 		switch (sink_code) {
 		case MEDIA_BUS_FMT_YUYV8_2X8:
 		{
@@ -1991,12 +1992,19 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,
 	vfe->isr_ops.comp_done = vfe_isr_comp_done;
 	vfe->isr_ops.wm_done = vfe_isr_wm_done;
 
-	if (camss->version == CAMSS_8x16)
+	switch (camss->version) {
+	case CAMSS_8x16:
 		vfe->ops = &vfe_ops_4_1;
-	else if (camss->version == CAMSS_8x96)
+		break;
+	case CAMSS_8x96:
 		vfe->ops = &vfe_ops_4_7;
-	else
+		break;
+	case CAMSS_660:
+		vfe->ops = &vfe_ops_4_8;
+		break;
+	default:
 		return -EINVAL;
+	}
 
 	/* Memory */
 
@@ -2095,7 +2103,8 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,
 				l->formats = formats_rdi_8x16;
 				l->nformats = ARRAY_SIZE(formats_rdi_8x16);
 			}
-		} else if (camss->version == CAMSS_8x96) {
+		} else if (camss->version == CAMSS_8x96 ||
+			   camss->version == CAMSS_660) {
 			if (i == VFE_LINE_PIX) {
 				l->formats = formats_pix_8x96;
 				l->nformats = ARRAY_SIZE(formats_pix_8x96);
diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 114c3ae4a4ab..fbba9e45d732 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -970,7 +970,8 @@ int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev,
 			video->formats = formats_rdi_8x16;
 			video->nformats = ARRAY_SIZE(formats_rdi_8x16);
 		}
-	} else if (video->camss->version == CAMSS_8x96) {
+	} else if (video->camss->version == CAMSS_8x96 ||
+		   video->camss->version == CAMSS_660) {
 		if (is_pix) {
 			video->formats = formats_pix_8x96;
 			video->nformats = ARRAY_SIZE(formats_pix_8x96);
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 9186881afc98..8fefce57bc49 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -283,6 +283,188 @@ static const struct resources vfe_res_8x96[] = {
 	}
 };
 
+static const struct resources csiphy_res_660[] = {
+	/* CSIPHY0 */
+	{
+		.regulator = { NULL },
+		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy0_timer",
+			   "csi0_phy", "csiphy_ahb2crif" },
+		.clock_rate = { { 0 },
+				{ 0 },
+				{ 0 },
+				{ 100000000, 200000000, 269333333 },
+				{ 0 } },
+		.reg = { "csiphy0", "csiphy0_clk_mux" },
+		.interrupt = { "csiphy0" }
+	},
+
+	/* CSIPHY1 */
+	{
+		.regulator = { NULL },
+		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy1_timer",
+			   "csi1_phy", "csiphy_ahb2crif" },
+		.clock_rate = { { 0 },
+				{ 0 },
+				{ 0 },
+				{ 100000000, 200000000, 269333333 },
+				{ 0 } },
+		.reg = { "csiphy1", "csiphy1_clk_mux" },
+		.interrupt = { "csiphy1" }
+	},
+
+	/* CSIPHY2 */
+	{
+		.regulator = { NULL },
+		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy2_timer",
+			   "csi2_phy", "csiphy_ahb2crif" },
+		.clock_rate = { { 0 },
+				{ 0 },
+				{ 0 },
+				{ 100000000, 200000000, 269333333 },
+				{ 0 } },
+		.reg = { "csiphy2", "csiphy2_clk_mux" },
+		.interrupt = { "csiphy2" }
+	}
+};
+
+static const struct resources csid_res_660[] = {
+	/* CSID0 */
+	{
+		.regulator = { "vdda", "vdd_sec" },
+		.clock = { "top_ahb", "ispif_ahb", "csi0_ahb", "ahb",
+			   "csi0", "csi0_phy", "csi0_pix", "csi0_rdi",
+			   "cphy_csid0" },
+		.clock_rate = { { 0 },
+				{ 0 },
+				{ 0 },
+				{ 0 },
+				{ 100000000, 200000000, 310000000,
+				  404000000, 465000000 },
+				{ 0 },
+				{ 0 },
+				{ 0 },
+				{ 0 } },
+		.reg = { "csid0" },
+		.interrupt = { "csid0" }
+	},
+
+	/* CSID1 */
+	{
+		.regulator = { "vdda", "vdd_sec" },
+		.clock = { "top_ahb", "ispif_ahb", "csi1_ahb", "ahb",
+			   "csi1", "csi1_phy", "csi1_pix", "csi1_rdi",
+			   "cphy_csid1" },
+		.clock_rate = { { 0 },
+				{ 0 },
+				{ 0 },
+				{ 0 },
+				{ 100000000, 200000000, 310000000,
+				  404000000, 465000000 },
+				{ 0 },
+				{ 0 },
+				{ 0 },
+				{ 0 } },
+		.reg = { "csid1" },
+		.interrupt = { "csid1" }
+	},
+
+	/* CSID2 */
+	{
+		.regulator = { "vdda", "vdd_sec" },
+		.clock = { "top_ahb", "ispif_ahb", "csi2_ahb", "ahb",
+			   "csi2", "csi2_phy", "csi2_pix", "csi2_rdi",
+			   "cphy_csid2" },
+		.clock_rate = { { 0 },
+				{ 0 },
+				{ 0 },
+				{ 0 },
+				{ 100000000, 200000000, 310000000,
+				  404000000, 465000000 },
+				{ 0 },
+				{ 0 },
+				{ 0 },
+				{ 0 } },
+		.reg = { "csid2" },
+		.interrupt = { "csid2" }
+	},
+
+	/* CSID3 */
+	{
+		.regulator = { "vdda", "vdd_sec" },
+		.clock = { "top_ahb", "ispif_ahb", "csi3_ahb", "ahb",
+			   "csi3", "csi3_phy", "csi3_pix", "csi3_rdi",
+			   "cphy_csid3" },
+		.clock_rate = { { 0 },
+				{ 0 },
+				{ 0 },
+				{ 0 },
+				{ 100000000, 200000000, 310000000,
+				  404000000, 465000000 },
+				{ 0 },
+				{ 0 },
+				{ 0 },
+				{ 0 } },
+		.reg = { "csid3" },
+		.interrupt = { "csid3" }
+	}
+};
+
+static const struct resources_ispif ispif_res_660 = {
+	/* ISPIF */
+	.clock = { "top_ahb", "ahb", "ispif_ahb",
+		   "csi0", "csi0_pix", "csi0_rdi",
+		   "csi1", "csi1_pix", "csi1_rdi",
+		   "csi2", "csi2_pix", "csi2_rdi",
+		   "csi3", "csi3_pix", "csi3_rdi" },
+	.clock_for_reset = { "vfe0", "csi_vfe0", "vfe1", "csi_vfe1" },
+	.reg = { "ispif", "csi_clk_mux" },
+	.interrupt = "ispif"
+};
+
+static const struct resources vfe_res_660[] = {
+	/* VFE0 */
+	{
+		.regulator = { NULL },
+		.clock = { "throttle_axi", "top_ahb", "ahb", "vfe0",
+			   "csi_vfe0", "vfe_ahb", "vfe0_ahb", "vfe_axi",
+			   "vfe0_stream"},
+		.clock_rate = { { 0 },
+				{ 0 },
+				{ 0 },
+				{ 120000000, 200000000, 256000000,
+				  300000000, 404000000, 480000000,
+				  540000000, 576000000 },
+				{ 0 },
+				{ 0 },
+				{ 0 },
+				{ 0 },
+				{ 0 } },
+		.reg = { "vfe0" },
+		.interrupt = { "vfe0" }
+	},
+
+	/* VFE1 */
+	{
+		.regulator = { NULL },
+		.clock = { "throttle_axi", "top_ahb", "ahb", "vfe1",
+			   "csi_vfe1", "vfe_ahb", "vfe1_ahb", "vfe_axi",
+			   "vfe1_stream"},
+		.clock_rate = { { 0 },
+				{ 0 },
+				{ 0 },
+				{ 120000000, 200000000, 256000000,
+				  300000000, 404000000, 480000000,
+				  540000000, 576000000 },
+				{ 0 },
+				{ 0 },
+				{ 0 },
+				{ 0 },
+				{ 0 } },
+		.reg = { "vfe1" },
+		.interrupt = { "vfe1" }
+	}
+};
+
 /*
  * camss_add_clock_margin - Add margin to clock frequency rate
  * @rate: Clock frequency rate
@@ -397,7 +579,8 @@ int camss_get_pixel_clock(struct media_entity *entity, u32 *pixel_clock)
 
 int camss_pm_domain_on(struct camss *camss, int id)
 {
-	if (camss->version == CAMSS_8x96) {
+	if (camss->version == CAMSS_8x96 ||
+	    camss->version == CAMSS_660) {
 		camss->genpd_link[id] = device_link_add(camss->dev,
 				camss->genpd[id], DL_FLAG_STATELESS |
 				DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE);
@@ -411,7 +594,8 @@ int camss_pm_domain_on(struct camss *camss, int id)
 
 void camss_pm_domain_off(struct camss *camss, int id)
 {
-	if (camss->version == CAMSS_8x96)
+	if (camss->version == CAMSS_8x96 ||
+	    camss->version == CAMSS_660)
 		device_link_del(camss->genpd_link[id]);
 }
 
@@ -533,6 +717,11 @@ static int camss_init_subdevices(struct camss *camss)
 		csid_res = csid_res_8x96;
 		ispif_res = &ispif_res_8x96;
 		vfe_res = vfe_res_8x96;
+	} else if (camss->version == CAMSS_660) {
+		csiphy_res = csiphy_res_660;
+		csid_res = csid_res_660;
+		ispif_res = &ispif_res_660;
+		vfe_res = vfe_res_660;
 	} else {
 		return -EINVAL;
 	}
@@ -833,6 +1022,12 @@ static int camss_probe(struct platform_device *pdev)
 		camss->csiphy_num = 3;
 		camss->csid_num = 4;
 		camss->vfe_num = 2;
+	} else if (of_device_is_compatible(dev->of_node,
+					   "qcom,sdm660-camss")) {
+		camss->version = CAMSS_660;
+		camss->csiphy_num = 3;
+		camss->csid_num = 4;
+		camss->vfe_num = 2;
 	} else {
 		ret = -EINVAL;
 		goto err_free;
@@ -919,7 +1114,8 @@ static int camss_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (camss->version == CAMSS_8x96) {
+	if (camss->version == CAMSS_8x96 ||
+	    camss->version == CAMSS_660) {
 		camss->genpd[PM_DOMAIN_VFE0] = dev_pm_domain_attach_by_id(
 						camss->dev, PM_DOMAIN_VFE0);
 		if (IS_ERR(camss->genpd[PM_DOMAIN_VFE0]))
@@ -958,7 +1154,8 @@ void camss_delete(struct camss *camss)
 
 	pm_runtime_disable(camss->dev);
 
-	if (camss->version == CAMSS_8x96) {
+	if (camss->version == CAMSS_8x96 ||
+	    camss->version == CAMSS_660) {
 		dev_pm_domain_detach(camss->genpd[PM_DOMAIN_VFE0], true);
 		dev_pm_domain_detach(camss->genpd[PM_DOMAIN_VFE1], true);
 	}
@@ -989,6 +1186,7 @@ static int camss_remove(struct platform_device *pdev)
 static const struct of_device_id camss_dt_match[] = {
 	{ .compatible = "qcom,msm8916-camss" },
 	{ .compatible = "qcom,msm8996-camss" },
+	{ .compatible = "qcom,sdm660-camss" },
 	{ }
 };
 
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index 1376b07889bf..3a0484683cd6 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -65,6 +65,7 @@ enum pm_domain {
 enum camss_version {
 	CAMSS_8x16,
 	CAMSS_8x96,
+	CAMSS_660,
 };
 
 struct camss {
-- 
2.28.0

[PATCH 5/6] media: dt-bindings: media: qcom,camss: Add bindings for SDM660 camss

[kholk11]

From: AngeloGioacchino Del Regno <kholk11@gmail.com>

Add bindings for qcom,sdm660-camss in order to support the camera
subsystem on SDM630/660 and SDA variants.

Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
 Documentation/devicetree/bindings/media/qcom,camss.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/qcom,camss.txt b/Documentation/devicetree/bindings/media/qcom,camss.txt
index 09eb6ed99114..498234629e21 100644
--- a/Documentation/devicetree/bindings/media/qcom,camss.txt
+++ b/Documentation/devicetree/bindings/media/qcom,camss.txt
@@ -8,6 +8,7 @@ Qualcomm Camera Subsystem
 	Definition: Should contain one of:
 		- "qcom,msm8916-camss"
 		- "qcom,msm8996-camss"
+		- "qcom,sdm660-camss"
 - reg:
 	Usage: required
 	Value type: <prop-encoded-array>
@@ -64,30 +65,36 @@ Qualcomm Camera Subsystem
 	Value type: <stringlist>
 	Definition: Should contain the following entries:
 		- "top_ahb"
+		- "throttle_axi"	(660 only)
 		- "ispif_ahb"
 		- "csiphy0_timer"
 		- "csiphy1_timer"
 		- "csiphy2_timer"	(8996 only)
+		- "csiphy_ahb2crif"	(660 only)
 		- "csi0_ahb"
 		- "csi0"
 		- "csi0_phy"
 		- "csi0_pix"
 		- "csi0_rdi"
+		- "cphy_csid0"		(660 only)
 		- "csi1_ahb"
 		- "csi1"
 		- "csi1_phy"
 		- "csi1_pix"
 		- "csi1_rdi"
+		- "cphy_csid1"		(660 only)
 		- "csi2_ahb"		(8996 only)
 		- "csi2"		(8996 only)
 		- "csi2_phy"		(8996 only)
 		- "csi2_pix"		(8996 only)
 		- "csi2_rdi"		(8996 only)
+		- "cphy_csid2"		(660 only)
 		- "csi3_ahb"		(8996 only)
 		- "csi3"		(8996 only)
 		- "csi3_phy"		(8996 only)
 		- "csi3_pix"		(8996 only)
 		- "csi3_rdi"		(8996 only)
+		- "cphy_csid3"		(660 only)
 		- "ahb"
 		- "vfe0"
 		- "csi_vfe0"
-- 
2.28.0

[PATCH 6/6] media: camss: csiphy: Set rate on csiX_phy clock on SDM630/660

[kholk11]

From: AngeloGioacchino Del Regno <kholk11@gmail.com>

The SDM630/660 SoCs (and variants) have another clock source
for the PHY, which must be set to a rate that's equal or
greater than the CSI PHY timer clock: failing to do this
will produce PHY overflows when trying to get a stream from
a very high bandwidth camera sensor and outputting no frame
or a partial one.

Since I haven't found any usecase in which the csiX_phy
clock needs to be higher than the csiXphy_timer, let's just
set the same rate on both, which seems to work just perfect.

Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
 .../media/platform/qcom/camss/camss-csiphy.c  | 22 ++++++++++++++++---
 .../media/platform/qcom/camss/camss-csiphy.h  |  1 +
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index c00f25aac21b..a5d717d022a5 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -113,9 +113,7 @@ static int csiphy_set_clock_rates(struct csiphy_device *csiphy)
 	for (i = 0; i < csiphy->nclocks; i++) {
 		struct camss_clock *clock = &csiphy->clock[i];
 
-		if (!strcmp(clock->name, "csiphy0_timer") ||
-		    !strcmp(clock->name, "csiphy1_timer") ||
-		    !strcmp(clock->name, "csiphy2_timer")) {
+		if (csiphy->rate_set[i]) {
 			u8 bpp = csiphy_get_bpp(csiphy->formats,
 					csiphy->nformats,
 					csiphy->fmt[MSM_CSIPHY_PAD_SINK].code);
@@ -611,6 +609,13 @@ int msm_csiphy_subdev_init(struct camss *camss,
 	if (!csiphy->clock)
 		return -ENOMEM;
 
+	csiphy->rate_set = devm_kcalloc(dev,
+					csiphy->nclocks,
+					sizeof(*csiphy->rate_set),
+					GFP_KERNEL);
+	if (!csiphy->rate_set)
+		return -ENOMEM;
+
 	for (i = 0; i < csiphy->nclocks; i++) {
 		struct camss_clock *clock = &csiphy->clock[i];
 
@@ -638,6 +643,17 @@ int msm_csiphy_subdev_init(struct camss *camss,
 
 		for (j = 0; j < clock->nfreqs; j++)
 			clock->freq[j] = res->clock_rate[i][j];
+
+		if (!strcmp(clock->name, "csiphy0_timer") ||
+		    !strcmp(clock->name, "csiphy1_timer") ||
+		    !strcmp(clock->name, "csiphy2_timer"))
+			csiphy->rate_set[i] = true;
+
+		if (camss->version == CAMSS_660 &&
+		    (!strcmp(clock->name, "csi0_phy") ||
+		     !strcmp(clock->name, "csi1_phy") ||
+		     !strcmp(clock->name, "csi2_phy")))
+			csiphy->rate_set[i] = true;
 	}
 
 	return 0;
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
index 376f865ad383..f7967ef836dc 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.h
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
@@ -66,6 +66,7 @@ struct csiphy_device {
 	u32 irq;
 	char irq_name[30];
 	struct camss_clock *clock;
+	bool *rate_set;
 	int nclocks;
 	u32 timer_clk_rate;
 	struct csiphy_config cfg;
-- 
2.28.0

Re: [PATCH 1/6] media: camss: csiphy-3ph: Add support for SDM630/660

[Robert Foss]

Hi Angelo,

On Sun, 18 Oct 2020 at 14:52, <kholk11@gmail.com> wrote:
>
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> The CSIPHY on SDM630/660 needs a slightly longer T_HS_CLK_MISS
> configuration on lanes CFG4.
>
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
>  drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> index 2e65caf1ecae..97cb9de85031 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> @@ -8,6 +8,7 @@
>   * Copyright (C) 2016-2018 Linaro Ltd.
>   */
>
> +#include "camss.h"
>  #include "camss-csiphy.h"
>
>  #include <linux/delay.h>
> @@ -21,6 +22,7 @@
>  #define CSIPHY_3PH_LNn_CFG3(n)                 (0x008 + 0x100 * (n))
>  #define CSIPHY_3PH_LNn_CFG4(n)                 (0x00c + 0x100 * (n))
>  #define CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS      0xa4
> +#define CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS_660  0xa5
>  #define CSIPHY_3PH_LNn_CFG5(n)                 (0x010 + 0x100 * (n))
>  #define CSIPHY_3PH_LNn_CFG5_T_HS_DTERM         0x02
>  #define CSIPHY_3PH_LNn_CFG5_HS_REC_EQ_FQ_INT   0x50
> @@ -198,7 +200,10 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
>         val = CSIPHY_3PH_LNn_CFG1_SWI_REC_DLY_PRG;
>         writel_relaxed(val, csiphy->base + CSIPHY_3PH_LNn_CFG1(l));
>
> -       val = CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS;
> +       if (csiphy->camss->version == CAMSS_660)

The CAMSS_660 enum is not defined until patch #4, so building fails
here. I expect to see this issue in a few other places, but I'll
refrain from pointing them all out.

> +               val = CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS_660;
> +       else
> +               val = CSIPHY_3PH_LNn_CFG4_T_HS_CLK_MISS;
>         writel_relaxed(val, csiphy->base + CSIPHY_3PH_LNn_CFG4(l));
>
>         val = CSIPHY_3PH_LNn_MISC1_IS_CLKLANE;
> --
> 2.28.0
>

Re: [PATCH 2/6] media: camss: ispif: Correctly reset based on the VFE ID

[Robert Foss]

Nice catch! This patch looks good to me.

Signed-off-by: Robert Foss <robert.foss@linaro.org>

On Sun, 18 Oct 2020 at 14:54, <kholk11@gmail.com> wrote:
>
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> Resetting the ISPIF VFE0 context is wrong if we are using the VFE1
> for dual-camera or simply because a secondary camera is connected
> to it: in this case the reset will always happen on the VFE0 ctx
> of the ISPIF, which is .. useless.
>
> Fix this usecase by adding the ISPIF_RST_CMD_1 address and choose
> where to do the (or what to) reset based on the VFE line id.
>
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
>  .../media/platform/qcom/camss/camss-ispif.c   | 87 ++++++++++++-------
>  .../media/platform/qcom/camss/camss-ispif.h   |  2 +-
>  2 files changed, 57 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-ispif.c b/drivers/media/platform/qcom/camss/camss-ispif.c
> index db94cfd6c508..252db6b33dab 100644
> --- a/drivers/media/platform/qcom/camss/camss-ispif.c
> +++ b/drivers/media/platform/qcom/camss/camss-ispif.c
> @@ -26,6 +26,7 @@
>  #define MSM_ISPIF_NAME "msm_ispif"
>
>  #define ISPIF_RST_CMD_0                        0x008
> +#define ISPIF_RST_CMD_1                        0x00c
>  #define ISPIF_RST_CMD_0_STROBED_RST_EN         (1 << 0)
>  #define ISPIF_RST_CMD_0_MISC_LOGIC_RST         (1 << 1)
>  #define ISPIF_RST_CMD_0_SW_REG_RST             (1 << 2)
> @@ -179,7 +180,10 @@ static irqreturn_t ispif_isr_8x96(int irq, void *dev)
>         writel(0x1, ispif->base + ISPIF_IRQ_GLOBAL_CLEAR_CMD);
>
>         if ((value0 >> 27) & 0x1)
> -               complete(&ispif->reset_complete);
> +               complete(&ispif->reset_complete[0]);
> +
> +       if ((value3 >> 27) & 0x1)
> +               complete(&ispif->reset_complete[1]);
>
>         if (unlikely(value0 & ISPIF_VFE_m_IRQ_STATUS_0_PIX0_OVERFLOW))
>                 dev_err_ratelimited(to_device(ispif), "VFE0 pix0 overflow\n");
> @@ -237,7 +241,7 @@ static irqreturn_t ispif_isr_8x16(int irq, void *dev)
>         writel(0x1, ispif->base + ISPIF_IRQ_GLOBAL_CLEAR_CMD);
>
>         if ((value0 >> 27) & 0x1)
> -               complete(&ispif->reset_complete);
> +               complete(&ispif->reset_complete[0]);
>
>         if (unlikely(value0 & ISPIF_VFE_m_IRQ_STATUS_0_PIX0_OVERFLOW))
>                 dev_err_ratelimited(to_device(ispif), "VFE0 pix0 overflow\n");
> @@ -257,33 +261,17 @@ static irqreturn_t ispif_isr_8x16(int irq, void *dev)
>         return IRQ_HANDLED;
>  }
>
> -/*
> - * ispif_reset - Trigger reset on ISPIF module and wait to complete
> - * @ispif: ISPIF device
> - *
> - * Return 0 on success or a negative error code otherwise
> - */
> -static int ispif_reset(struct ispif_device *ispif)
> +static int ispif_vfe_reset(struct ispif_device *ispif, u8 vfe_id)
>  {
> -       unsigned long time;
>         u32 val;
> -       int ret;
> -
> -       ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE0);
> -       if (ret < 0)
> -               return ret;
>
> -       ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE1);
> -       if (ret < 0)
> -               return ret;
> -
> -       ret = camss_enable_clocks(ispif->nclocks_for_reset,
> -                                 ispif->clock_for_reset,
> -                                 to_device(ispif));
> -       if (ret < 0)
> -               return ret;
> +       if (vfe_id > (to_camss(ispif)->vfe_num - 1)) {
> +               dev_err(to_device(ispif),
> +                       "Error: asked reset for invalid VFE%d\n", vfe_id);
> +               return -ENOENT;
> +       }
>
> -       reinit_completion(&ispif->reset_complete);
> +       reinit_completion(&ispif->reset_complete[vfe_id]);
>
>         val = ISPIF_RST_CMD_0_STROBED_RST_EN |
>                 ISPIF_RST_CMD_0_MISC_LOGIC_RST |
> @@ -303,15 +291,51 @@ static int ispif_reset(struct ispif_device *ispif)
>                 ISPIF_RST_CMD_0_RDI_OUTPUT_1_MISR_RST |
>                 ISPIF_RST_CMD_0_RDI_OUTPUT_2_MISR_RST;
>
> -       writel_relaxed(val, ispif->base + ISPIF_RST_CMD_0);
> +       if (vfe_id == 1)
> +               writel_relaxed(val, ispif->base + ISPIF_RST_CMD_1);
> +       else
> +               writel_relaxed(val, ispif->base + ISPIF_RST_CMD_0);
>
> -       time = wait_for_completion_timeout(&ispif->reset_complete,
> +       time = wait_for_completion_timeout(&ispif->reset_complete[vfe_id],
>                 msecs_to_jiffies(ISPIF_RESET_TIMEOUT_MS));
>         if (!time) {
> -               dev_err(to_device(ispif), "ISPIF reset timeout\n");
> -               ret = -EIO;
> +               dev_err(to_device(ispif),
> +                       "ISPIF for VFE%d reset timeout\n", vfe_id);
> +               return -EIO;
>         }
>
> +       return 0;
> +}
> +
> +/*
> + * ispif_reset - Trigger reset on ISPIF module and wait to complete
> + * @ispif: ISPIF device
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int ispif_reset(struct ispif_device *ispif, u8 vfe_id)
> +{
> +       unsigned long time;
> +       int ret;
> +
> +       ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE0);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE1);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = camss_enable_clocks(ispif->nclocks_for_reset,
> +                                 ispif->clock_for_reset,
> +                                 to_device(ispif));
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = ispif_vfe_reset(ispif, vfe_id);
> +       if (ret)
> +               dev_dbg(to_device(ispif), "ISPIF Reset failed\n");
> +
>         camss_disable_clocks(ispif->nclocks_for_reset, ispif->clock_for_reset);
>
>         camss_pm_domain_off(to_camss(ispif), PM_DOMAIN_VFE0);
> @@ -355,7 +379,7 @@ static int ispif_set_power(struct v4l2_subdev *sd, int on)
>                         goto exit;
>                 }
>
> -               ret = ispif_reset(ispif);
> +               ret = ispif_reset(ispif, line->vfe_id);
>                 if (ret < 0) {
>                         pm_runtime_put_sync(dev);
>                         camss_disable_clocks(ispif->nclocks, ispif->clock);
> @@ -1192,7 +1216,8 @@ int msm_ispif_subdev_init(struct ispif_device *ispif,
>
>         mutex_init(&ispif->config_lock);
>
> -       init_completion(&ispif->reset_complete);
> +       for (i = 0; i < MSM_ISPIF_VFE_NUM; i++)
> +               init_completion(&ispif->reset_complete[i]);
>
>         return 0;
>  }
> diff --git a/drivers/media/platform/qcom/camss/camss-ispif.h b/drivers/media/platform/qcom/camss/camss-ispif.h
> index 1a5ba2425a42..4132174f7ea1 100644
> --- a/drivers/media/platform/qcom/camss/camss-ispif.h
> +++ b/drivers/media/platform/qcom/camss/camss-ispif.h
> @@ -56,7 +56,7 @@ struct ispif_device {
>         int nclocks;
>         struct camss_clock  *clock_for_reset;
>         int nclocks_for_reset;
> -       struct completion reset_complete;
> +       struct completion reset_complete[MSM_ISPIF_VFE_NUM];
>         int power_count;
>         struct mutex power_lock;
>         struct ispif_intf_cmd_reg intf_cmd[MSM_ISPIF_VFE_NUM];
> --
> 2.28.0
>

Re: [PATCH 2/6] media: camss: ispif: Correctly reset based on the VFE ID

[Robert Foss]

I found a build issue in this commit.


On Tue, 20 Oct 2020 at 10:59, Robert Foss <robert.foss@linaro.org> wrote:
>
> Nice catch! This patch looks good to me.
>
> Signed-off-by: Robert Foss <robert.foss@linaro.org>
>
> On Sun, 18 Oct 2020 at 14:54, <kholk11@gmail.com> wrote:
> >
> > From: AngeloGioacchino Del Regno <kholk11@gmail.com>
> >
> > Resetting the ISPIF VFE0 context is wrong if we are using the VFE1
> > for dual-camera or simply because a secondary camera is connected
> > to it: in this case the reset will always happen on the VFE0 ctx
> > of the ISPIF, which is .. useless.
> >
> > Fix this usecase by adding the ISPIF_RST_CMD_1 address and choose
> > where to do the (or what to) reset based on the VFE line id.
> >
> > Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> > ---
> >  .../media/platform/qcom/camss/camss-ispif.c   | 87 ++++++++++++-------
> >  .../media/platform/qcom/camss/camss-ispif.h   |  2 +-
> >  2 files changed, 57 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/media/platform/qcom/camss/camss-ispif.c b/drivers/media/platform/qcom/camss/camss-ispif.c
> > index db94cfd6c508..252db6b33dab 100644
> > --- a/drivers/media/platform/qcom/camss/camss-ispif.c
> > +++ b/drivers/media/platform/qcom/camss/camss-ispif.c
> > @@ -26,6 +26,7 @@
> >  #define MSM_ISPIF_NAME "msm_ispif"
> >
> >  #define ISPIF_RST_CMD_0                        0x008
> > +#define ISPIF_RST_CMD_1                        0x00c
> >  #define ISPIF_RST_CMD_0_STROBED_RST_EN         (1 << 0)
> >  #define ISPIF_RST_CMD_0_MISC_LOGIC_RST         (1 << 1)
> >  #define ISPIF_RST_CMD_0_SW_REG_RST             (1 << 2)
> > @@ -179,7 +180,10 @@ static irqreturn_t ispif_isr_8x96(int irq, void *dev)
> >         writel(0x1, ispif->base + ISPIF_IRQ_GLOBAL_CLEAR_CMD);
> >
> >         if ((value0 >> 27) & 0x1)
> > -               complete(&ispif->reset_complete);
> > +               complete(&ispif->reset_complete[0]);
> > +
> > +       if ((value3 >> 27) & 0x1)
> > +               complete(&ispif->reset_complete[1]);
> >
> >         if (unlikely(value0 & ISPIF_VFE_m_IRQ_STATUS_0_PIX0_OVERFLOW))
> >                 dev_err_ratelimited(to_device(ispif), "VFE0 pix0 overflow\n");
> > @@ -237,7 +241,7 @@ static irqreturn_t ispif_isr_8x16(int irq, void *dev)
> >         writel(0x1, ispif->base + ISPIF_IRQ_GLOBAL_CLEAR_CMD);
> >
> >         if ((value0 >> 27) & 0x1)
> > -               complete(&ispif->reset_complete);
> > +               complete(&ispif->reset_complete[0]);
> >
> >         if (unlikely(value0 & ISPIF_VFE_m_IRQ_STATUS_0_PIX0_OVERFLOW))
> >                 dev_err_ratelimited(to_device(ispif), "VFE0 pix0 overflow\n");
> > @@ -257,33 +261,17 @@ static irqreturn_t ispif_isr_8x16(int irq, void *dev)
> >         return IRQ_HANDLED;
> >  }
> >
> > -/*
> > - * ispif_reset - Trigger reset on ISPIF module and wait to complete
> > - * @ispif: ISPIF device
> > - *
> > - * Return 0 on success or a negative error code otherwise
> > - */
> > -static int ispif_reset(struct ispif_device *ispif)
> > +static int ispif_vfe_reset(struct ispif_device *ispif, u8 vfe_id)
> >  {
> > -       unsigned long time;
> >         u32 val;
> > -       int ret;
> > -
> > -       ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE0);
> > -       if (ret < 0)
> > -               return ret;
> >
> > -       ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE1);
> > -       if (ret < 0)
> > -               return ret;
> > -
> > -       ret = camss_enable_clocks(ispif->nclocks_for_reset,
> > -                                 ispif->clock_for_reset,
> > -                                 to_device(ispif));
> > -       if (ret < 0)
> > -               return ret;
> > +       if (vfe_id > (to_camss(ispif)->vfe_num - 1)) {
> > +               dev_err(to_device(ispif),
> > +                       "Error: asked reset for invalid VFE%d\n", vfe_id);
> > +               return -ENOENT;
> > +       }
> >
> > -       reinit_completion(&ispif->reset_complete);
> > +       reinit_completion(&ispif->reset_complete[vfe_id]);
> >
> >         val = ISPIF_RST_CMD_0_STROBED_RST_EN |
> >                 ISPIF_RST_CMD_0_MISC_LOGIC_RST |
> > @@ -303,15 +291,51 @@ static int ispif_reset(struct ispif_device *ispif)
> >                 ISPIF_RST_CMD_0_RDI_OUTPUT_1_MISR_RST |
> >                 ISPIF_RST_CMD_0_RDI_OUTPUT_2_MISR_RST;
> >
> > -       writel_relaxed(val, ispif->base + ISPIF_RST_CMD_0);
> > +       if (vfe_id == 1)
> > +               writel_relaxed(val, ispif->base + ISPIF_RST_CMD_1);
> > +       else
> > +               writel_relaxed(val, ispif->base + ISPIF_RST_CMD_0);
> >
> > -       time = wait_for_completion_timeout(&ispif->reset_complete,
> > +       time = wait_for_completion_timeout(&ispif->reset_complete[vfe_id],

'time' is not a variable that exists in this scope, so the build fails.

> >                 msecs_to_jiffies(ISPIF_RESET_TIMEOUT_MS));
> >         if (!time) {
> > -               dev_err(to_device(ispif), "ISPIF reset timeout\n");
> > -               ret = -EIO;
> > +               dev_err(to_device(ispif),
> > +                       "ISPIF for VFE%d reset timeout\n", vfe_id);
> > +               return -EIO;
> >         }
> >
> > +       return 0;
> > +}
> > +
> > +/*
> > + * ispif_reset - Trigger reset on ISPIF module and wait to complete
> > + * @ispif: ISPIF device
> > + *
> > + * Return 0 on success or a negative error code otherwise
> > + */
> > +static int ispif_reset(struct ispif_device *ispif, u8 vfe_id)
> > +{
> > +       unsigned long time;
> > +       int ret;
> > +
> > +       ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE0);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE1);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       ret = camss_enable_clocks(ispif->nclocks_for_reset,
> > +                                 ispif->clock_for_reset,
> > +                                 to_device(ispif));
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       ret = ispif_vfe_reset(ispif, vfe_id);
> > +       if (ret)
> > +               dev_dbg(to_device(ispif), "ISPIF Reset failed\n");
> > +
> >         camss_disable_clocks(ispif->nclocks_for_reset, ispif->clock_for_reset);
> >
> >         camss_pm_domain_off(to_camss(ispif), PM_DOMAIN_VFE0);
> > @@ -355,7 +379,7 @@ static int ispif_set_power(struct v4l2_subdev *sd, int on)
> >                         goto exit;
> >                 }
> >
> > -               ret = ispif_reset(ispif);
> > +               ret = ispif_reset(ispif, line->vfe_id);
> >                 if (ret < 0) {
> >                         pm_runtime_put_sync(dev);
> >                         camss_disable_clocks(ispif->nclocks, ispif->clock);
> > @@ -1192,7 +1216,8 @@ int msm_ispif_subdev_init(struct ispif_device *ispif,
> >
> >         mutex_init(&ispif->config_lock);
> >
> > -       init_completion(&ispif->reset_complete);
> > +       for (i = 0; i < MSM_ISPIF_VFE_NUM; i++)
> > +               init_completion(&ispif->reset_complete[i]);
> >
> >         return 0;
> >  }
> > diff --git a/drivers/media/platform/qcom/camss/camss-ispif.h b/drivers/media/platform/qcom/camss/camss-ispif.h
> > index 1a5ba2425a42..4132174f7ea1 100644
> > --- a/drivers/media/platform/qcom/camss/camss-ispif.h
> > +++ b/drivers/media/platform/qcom/camss/camss-ispif.h
> > @@ -56,7 +56,7 @@ struct ispif_device {
> >         int nclocks;
> >         struct camss_clock  *clock_for_reset;
> >         int nclocks_for_reset;
> > -       struct completion reset_complete;
> > +       struct completion reset_complete[MSM_ISPIF_VFE_NUM];
> >         int power_count;
> >         struct mutex power_lock;
> >         struct ispif_intf_cmd_reg intf_cmd[MSM_ISPIF_VFE_NUM];
> > --
> > 2.28.0
> >

Re: [PATCH 3/6] media: camss: vfe: Add support for VFE 4.8

[Robert Foss]

Hey Angelo,

The VFE abstraction layers are something I've been looking at too, and
there's no neat solution that both eliminates code duplication,
separates revisions into individual files, and still avoids creating
even more abstraction layers. So while combining vfe47 and vfe48 looks
and feels a bit cluttered, I think it is a necessary evil.

I've got some minor nits below.

On Sun, 18 Oct 2020 at 14:54, <kholk11@gmail.com> wrote:
>
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> Add the support for VFE 4.8 in the camss-vfe-4-7 driver, as this one
> really is a minor revision, requiring the very same management and
> basically having the same register layout as VFE 4.7, but needing
> a different QoS and DS configuration, using a different register to
> enable the wm and habing the same UB size for both instances (instead
> of a different size between instance 0 and 1).
>
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
>  .../media/platform/qcom/camss/camss-vfe-4-7.c | 129 ++++++++++++++++--
>  drivers/media/platform/qcom/camss/camss-vfe.h |   1 +
>  2 files changed, 122 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
> index 0dca8bf9281e..e48d58a4a9d1 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
> @@ -133,6 +133,11 @@
>  #define VFE_0_BUS_BDG_QOS_CFG_7                0x420
>  #define VFE_0_BUS_BDG_QOS_CFG_7_CFG    0x0001aaa9
>
> +#define VFE48_0_BUS_BDG_QOS_CFG_0_CFG  0xaaa5aaa5
> +#define VFE48_0_BUS_BDG_QOS_CFG_3_CFG  0xaa55aaa5
> +#define VFE48_0_BUS_BDG_QOS_CFG_4_CFG  0xaa55aa55
> +#define VFE48_0_BUS_BDG_QOS_CFG_7_CFG  0x0005aa55
> +
>  #define VFE_0_BUS_BDG_DS_CFG_0         0x424
>  #define VFE_0_BUS_BDG_DS_CFG_0_CFG     0xcccc0011
>  #define VFE_0_BUS_BDG_DS_CFG_1         0x428
> @@ -153,6 +158,9 @@
>  #define VFE_0_BUS_BDG_DS_CFG_16                0x464
>  #define VFE_0_BUS_BDG_DS_CFG_16_CFG    0x40000103
>
> +#define VFE48_0_BUS_BDG_DS_CFG_0_CFG   0xcccc1111
> +#define VFE48_0_BUS_BDG_DS_CFG_16_CFG  0x00000110
> +
>  #define VFE_0_RDI_CFG_x(x)             (0x46c + (0x4 * (x)))
>  #define VFE_0_RDI_CFG_x_RDI_STREAM_SEL_SHIFT   28
>  #define VFE_0_RDI_CFG_x_RDI_STREAM_SEL_MASK    (0xf << 28)
> @@ -231,6 +239,9 @@
>  #define VFE_0_REALIGN_BUF_CFG_CR_ODD_PIXEL     BIT(3)
>  #define VFE_0_REALIGN_BUF_CFG_HSUB_ENABLE      BIT(4)
>
> +#define VFE48_0_BUS_IMAGE_MASTER_CMD           0xcec
> +#define VFE48_0_BUS_IMAGE_MASTER_n_SHIFT(x)    (2 * (x))
> +
>  #define CAMIF_TIMEOUT_SLEEP_US 1000
>  #define CAMIF_TIMEOUT_ALL_US 1000000
>
> @@ -246,7 +257,7 @@ static void vfe_hw_version_read(struct vfe_device *vfe, struct device *dev)
>         dev_err(dev, "VFE HW Version = 0x%08x\n", hw_version);
>  }
>
> -static u16 vfe_get_ub_size(u8 vfe_id)
> +static u16 vfe47_get_ub_size(u8 vfe_id)
>  {
>         if (vfe_id == 0)
>                 return MSM_VFE_VFE0_UB_SIZE_RDI;
> @@ -299,7 +310,7 @@ static void vfe_halt_clear(struct vfe_device *vfe)
>         writel_relaxed(0x0, vfe->base + VFE_0_BUS_BDG_CMD);
>  }
>
> -static void vfe_wm_enable(struct vfe_device *vfe, u8 wm, u8 enable)
> +static void vfe47_wm_enable(struct vfe_device *vfe, u8 wm, u8 enable)
>  {
>         if (enable)
>                 vfe_reg_set(vfe, VFE_0_BUS_IMAGE_MASTER_n_WR_CFG(wm),
> @@ -883,7 +894,7 @@ static void vfe_set_clamp_cfg(struct vfe_device *vfe)
>         writel_relaxed(val, vfe->base + VFE_0_CLAMP_ENC_MIN_CFG);
>  }
>
> -static void vfe_set_qos(struct vfe_device *vfe)
> +static void vfe47_set_qos(struct vfe_device *vfe)
>  {
>         u32 val = VFE_0_BUS_BDG_QOS_CFG_0_CFG;
>         u32 val7 = VFE_0_BUS_BDG_QOS_CFG_7_CFG;
> @@ -898,7 +909,7 @@ static void vfe_set_qos(struct vfe_device *vfe)
>         writel_relaxed(val7, vfe->base + VFE_0_BUS_BDG_QOS_CFG_7);
>  }
>
> -static void vfe_set_ds(struct vfe_device *vfe)
> +static void vfe47_set_ds(struct vfe_device *vfe)
>  {
>         u32 val = VFE_0_BUS_BDG_DS_CFG_0_CFG;
>         u32 val16 = VFE_0_BUS_BDG_DS_CFG_16_CFG;
> @@ -1098,11 +1109,113 @@ static irqreturn_t vfe_isr(int irq, void *dev)
>
>  const struct vfe_hw_ops vfe_ops_4_7 = {
>         .hw_version_read = vfe_hw_version_read,
> -       .get_ub_size = vfe_get_ub_size,
> +       .get_ub_size = vfe47_get_ub_size,
> +       .global_reset = vfe_global_reset,
> +       .halt_request = vfe_halt_request,
> +       .halt_clear = vfe_halt_clear,
> +       .wm_enable = vfe47_wm_enable,
> +       .wm_frame_based = vfe_wm_frame_based,
> +       .wm_line_based = vfe_wm_line_based,
> +       .wm_set_framedrop_period = vfe_wm_set_framedrop_period,
> +       .wm_set_framedrop_pattern = vfe_wm_set_framedrop_pattern,
> +       .wm_set_ub_cfg = vfe_wm_set_ub_cfg,
> +       .bus_reload_wm = vfe_bus_reload_wm,
> +       .wm_set_ping_addr = vfe_wm_set_ping_addr,
> +       .wm_set_pong_addr = vfe_wm_set_pong_addr,
> +       .wm_get_ping_pong_status = vfe_wm_get_ping_pong_status,
> +       .bus_enable_wr_if = vfe_bus_enable_wr_if,
> +       .bus_connect_wm_to_rdi = vfe_bus_connect_wm_to_rdi,
> +       .wm_set_subsample = vfe_wm_set_subsample,
> +       .bus_disconnect_wm_from_rdi = vfe_bus_disconnect_wm_from_rdi,
> +       .set_xbar_cfg = vfe_set_xbar_cfg,
> +       .set_realign_cfg = vfe_set_realign_cfg,
> +       .set_rdi_cid = vfe_set_rdi_cid,
> +       .reg_update = vfe_reg_update,
> +       .reg_update_clear = vfe_reg_update_clear,
> +       .enable_irq_wm_line = vfe_enable_irq_wm_line,
> +       .enable_irq_pix_line = vfe_enable_irq_pix_line,
> +       .enable_irq_common = vfe_enable_irq_common,
> +       .set_demux_cfg = vfe_set_demux_cfg,
> +       .set_scale_cfg = vfe_set_scale_cfg,
> +       .set_crop_cfg = vfe_set_crop_cfg,
> +       .set_clamp_cfg = vfe_set_clamp_cfg,
> +       .set_qos = vfe47_set_qos,
> +       .set_ds = vfe47_set_ds,
> +       .set_cgc_override = vfe_set_cgc_override,
> +       .set_camif_cfg = vfe_set_camif_cfg,
> +       .set_camif_cmd = vfe_set_camif_cmd,
> +       .set_module_cfg = vfe_set_module_cfg,
> +       .camif_wait_for_stop = vfe_camif_wait_for_stop,
> +       .isr_read = vfe_isr_read,
> +       .violation_read = vfe_violation_read,
> +       .isr = vfe_isr,
> +};

I would consider splitting the assignments that aren't related to
vfe48 support out into another commit. The ones that are vfe48 related
can stay in this commit.

> +
> +static u16 vfe48_get_ub_size(u8 vfe_id)
> +{
> +       /* On VFE4.8 the ub-size is the same on both instances */
> +       return MSM_VFE_VFE0_UB_SIZE_RDI;
> +}
> +
> +static void vfe48_wm_enable(struct vfe_device *vfe, u8 wm, u8 enable)
> +{
> +       if (enable)
> +               writel_relaxed(2 << VFE48_0_BUS_IMAGE_MASTER_n_SHIFT(wm),
> +                              vfe->base + VFE48_0_BUS_IMAGE_MASTER_CMD);
> +       else
> +               writel_relaxed(1 << VFE48_0_BUS_IMAGE_MASTER_n_SHIFT(wm),
> +                              vfe->base + VFE48_0_BUS_IMAGE_MASTER_CMD);
> +       wmb();

checkpatch rightly complains about this wmb() call being uncommented.
None or nearly none of the other wmb() calls are documented, but I
think it would be good practice to start documenting their purpose in
order to increase long-term maintainability.

> +}
> +
> +static void vfe48_set_qos(struct vfe_device *vfe)
> +{
> +       u32 val = VFE48_0_BUS_BDG_QOS_CFG_0_CFG;
> +       u32 val3 = VFE48_0_BUS_BDG_QOS_CFG_3_CFG;
> +       u32 val4 = VFE48_0_BUS_BDG_QOS_CFG_4_CFG;
> +       u32 val7 = VFE48_0_BUS_BDG_QOS_CFG_7_CFG;
> +
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_QOS_CFG_0);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_QOS_CFG_1);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_QOS_CFG_2);
> +       writel_relaxed(val3, vfe->base + VFE_0_BUS_BDG_QOS_CFG_3);
> +       writel_relaxed(val4, vfe->base + VFE_0_BUS_BDG_QOS_CFG_4);
> +       writel_relaxed(val4, vfe->base + VFE_0_BUS_BDG_QOS_CFG_5);
> +       writel_relaxed(val4, vfe->base + VFE_0_BUS_BDG_QOS_CFG_6);
> +       writel_relaxed(val7, vfe->base + VFE_0_BUS_BDG_QOS_CFG_7);
> +}
> +
> +static void vfe48_set_ds(struct vfe_device *vfe)
> +{
> +       u32 val = VFE48_0_BUS_BDG_DS_CFG_0_CFG;
> +       u32 val16 = VFE48_0_BUS_BDG_DS_CFG_16_CFG;
> +
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_0);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_1);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_2);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_3);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_4);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_5);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_6);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_7);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_8);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_9);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_10);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_11);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_12);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_13);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_14);
> +       writel_relaxed(val, vfe->base + VFE_0_BUS_BDG_DS_CFG_15);
> +       writel_relaxed(val16, vfe->base + VFE_0_BUS_BDG_DS_CFG_16);
> +}
> +
> +const struct vfe_hw_ops vfe_ops_4_8 = {
> +       .hw_version_read = vfe_hw_version_read,
> +       .get_ub_size = vfe48_get_ub_size,
>         .global_reset = vfe_global_reset,
>         .halt_request = vfe_halt_request,
>         .halt_clear = vfe_halt_clear,
> -       .wm_enable = vfe_wm_enable,
> +       .wm_enable = vfe48_wm_enable,
>         .wm_frame_based = vfe_wm_frame_based,
>         .wm_line_based = vfe_wm_line_based,
>         .wm_set_framedrop_period = vfe_wm_set_framedrop_period,
> @@ -1128,8 +1241,8 @@ const struct vfe_hw_ops vfe_ops_4_7 = {
>         .set_scale_cfg = vfe_set_scale_cfg,
>         .set_crop_cfg = vfe_set_crop_cfg,
>         .set_clamp_cfg = vfe_set_clamp_cfg,
> -       .set_qos = vfe_set_qos,
> -       .set_ds = vfe_set_ds,
> +       .set_qos = vfe48_set_qos,
> +       .set_ds = vfe48_set_ds,
>         .set_cgc_override = vfe_set_cgc_override,
>         .set_camif_cfg = vfe_set_camif_cfg,
>         .set_camif_cmd = vfe_set_camif_cmd,
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h
> index a90b0d2cc6de..5bce6736e4bb 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.h
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.h
> @@ -180,5 +180,6 @@ void msm_vfe_get_vfe_line_id(struct media_entity *entity, enum vfe_line_id *id);
>
>  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;
>
>  #endif /* QC_MSM_CAMSS_VFE_H */
> --
> 2.28.0
>

Re: [PATCH 4/6] media: camss: Add support for SDM630/636/660 camera subsystem

[Robert Foss]

Looks good to me.

Signed-off-by: Robert Foss <robert.foss@linaro.org>

On Sun, 18 Oct 2020 at 14:53, <kholk11@gmail.com> wrote:
>
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> Add support for the Qualcomm SDM630/636/660 and SDA variants' camera
> subsystem. These SoCs are equipped with:
> - 3x CSI PHY 3-Phase v1.0 (downstream csiphy-v3.5)
> - 4x CSID v5.0
> - 2x ISPIF v3.0
> - 2x VFE 4.8
> As a note, this camera subsystem is very similar to the one that
> is found in the MSM8998/APQ8098 SoCs.
>
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
>  .../media/platform/qcom/camss/camss-csid.c    |   9 +-
>  .../media/platform/qcom/camss/camss-csiphy.c  |   3 +-
>  .../media/platform/qcom/camss/camss-ispif.c   |  17 +-
>  drivers/media/platform/qcom/camss/camss-vfe.c |  19 +-
>  .../media/platform/qcom/camss/camss-video.c   |   3 +-
>  drivers/media/platform/qcom/camss/camss.c     | 206 +++++++++++++++++-
>  drivers/media/platform/qcom/camss/camss.h     |   1 +
>  7 files changed, 238 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
> index 2ffcda06706b..be3fe76f3dc3 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid.c
> @@ -383,7 +383,8 @@ static u32 csid_src_pad_code(struct csid_device *csid, u32 sink_code,
>                         return 0;
>
>                 return sink_code;
> -       } else if (csid->camss->version == CAMSS_8x96) {
> +       } else if (csid->camss->version == CAMSS_8x96 ||
> +                  csid->camss->version == CAMSS_660) {
>                 switch (sink_code) {
>                 case MEDIA_BUS_FMT_SBGGR10_1X10:
>                 {
> @@ -718,7 +719,8 @@ static int csid_set_stream(struct v4l2_subdev *sd, int enable)
>                 val |= df << CAMSS_CSID_CID_n_CFG_DECODE_FORMAT_SHIFT;
>                 val |= CAMSS_CSID_CID_n_CFG_RDI_MODE_RAW_DUMP;
>
> -               if (csid->camss->version == CAMSS_8x96) {
> +               if (csid->camss->version == CAMSS_8x96 ||
> +                   csid->camss->version == CAMSS_660) {
>                         u32 sink_code = csid->fmt[MSM_CSID_PAD_SINK].code;
>                         u32 src_code = csid->fmt[MSM_CSID_PAD_SRC].code;
>
> @@ -1098,7 +1100,8 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid,
>                 csid->formats = csid_formats_8x16;
>                 csid->nformats =
>                                 ARRAY_SIZE(csid_formats_8x16);
> -       } else if (camss->version == CAMSS_8x96) {
> +       } else if (camss->version == CAMSS_8x96 ||
> +                  camss->version == CAMSS_660) {
>                 csid->formats = csid_formats_8x96;
>                 csid->nformats =
>                                 ARRAY_SIZE(csid_formats_8x96);
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
> index 03ef9c5f4774..c00f25aac21b 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
> @@ -552,7 +552,8 @@ int msm_csiphy_subdev_init(struct camss *camss,
>                 csiphy->ops = &csiphy_ops_2ph_1_0;
>                 csiphy->formats = csiphy_formats_8x16;
>                 csiphy->nformats = ARRAY_SIZE(csiphy_formats_8x16);
> -       } else if (camss->version == CAMSS_8x96) {
> +       } else if (camss->version == CAMSS_8x96 ||
> +                  camss->version == CAMSS_660) {
>                 csiphy->ops = &csiphy_ops_3ph_1_0;
>                 csiphy->formats = csiphy_formats_8x96;
>                 csiphy->nformats = ARRAY_SIZE(csiphy_formats_8x96);
> diff --git a/drivers/media/platform/qcom/camss/camss-ispif.c b/drivers/media/platform/qcom/camss/camss-ispif.c
> index 252db6b33dab..adeb92808998 100644
> --- a/drivers/media/platform/qcom/camss/camss-ispif.c
> +++ b/drivers/media/platform/qcom/camss/camss-ispif.c
> @@ -263,6 +263,7 @@ static irqreturn_t ispif_isr_8x16(int irq, void *dev)
>
>  static int ispif_vfe_reset(struct ispif_device *ispif, u8 vfe_id)
>  {
> +       unsigned long time;
>         u32 val;
>
>         if (vfe_id > (to_camss(ispif)->vfe_num - 1)) {
> @@ -315,7 +316,6 @@ static int ispif_vfe_reset(struct ispif_device *ispif, u8 vfe_id)
>   */
>  static int ispif_reset(struct ispif_device *ispif, u8 vfe_id)
>  {
> -       unsigned long time;
>         int ret;
>
>         ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE0);
> @@ -825,7 +825,8 @@ static int ispif_set_stream(struct v4l2_subdev *sd, int enable)
>                 ispif_select_csid(ispif, intf, csid, vfe, 1);
>                 ispif_select_cid(ispif, intf, cid, vfe, 1);
>                 ispif_config_irq(ispif, intf, vfe, 1);
> -               if (to_camss(ispif)->version == CAMSS_8x96)
> +               if (to_camss(ispif)->version == CAMSS_8x96 ||
> +                   to_camss(ispif)->version == CAMSS_660)
>                         ispif_config_pack(ispif,
>                                           line->fmt[MSM_ISPIF_PAD_SINK].code,
>                                           intf, cid, vfe, 1);
> @@ -842,7 +843,8 @@ static int ispif_set_stream(struct v4l2_subdev *sd, int enable)
>                         return ret;
>
>                 mutex_lock(&ispif->config_lock);
> -               if (to_camss(ispif)->version == CAMSS_8x96)
> +               if (to_camss(ispif)->version == CAMSS_8x96 ||
> +                   to_camss(ispif)->version == CAMSS_660)
>                         ispif_config_pack(ispif,
>                                           line->fmt[MSM_ISPIF_PAD_SINK].code,
>                                           intf, cid, vfe, 0);
> @@ -1098,7 +1100,8 @@ int msm_ispif_subdev_init(struct ispif_device *ispif,
>         /* Number of ISPIF lines - same as number of CSID hardware modules */
>         if (to_camss(ispif)->version == CAMSS_8x16)
>                 ispif->line_num = 2;
> -       else if (to_camss(ispif)->version == CAMSS_8x96)
> +       else if (to_camss(ispif)->version == CAMSS_8x96 ||
> +                to_camss(ispif)->version == CAMSS_660)
>                 ispif->line_num = 4;
>         else
>                 return -EINVAL;
> @@ -1116,7 +1119,8 @@ int msm_ispif_subdev_init(struct ispif_device *ispif,
>                         ispif->line[i].formats = ispif_formats_8x16;
>                         ispif->line[i].nformats =
>                                         ARRAY_SIZE(ispif_formats_8x16);
> -               } else if (to_camss(ispif)->version == CAMSS_8x96) {
> +               } else if (to_camss(ispif)->version == CAMSS_8x96 ||
> +                          to_camss(ispif)->version == CAMSS_660) {
>                         ispif->line[i].formats = ispif_formats_8x96;
>                         ispif->line[i].nformats =
>                                         ARRAY_SIZE(ispif_formats_8x96);
> @@ -1156,7 +1160,8 @@ int msm_ispif_subdev_init(struct ispif_device *ispif,
>         if (to_camss(ispif)->version == CAMSS_8x16)
>                 ret = devm_request_irq(dev, ispif->irq, ispif_isr_8x16,
>                                IRQF_TRIGGER_RISING, ispif->irq_name, ispif);
> -       else if (to_camss(ispif)->version == CAMSS_8x96)
> +       else if (to_camss(ispif)->version == CAMSS_8x96 ||
> +                to_camss(ispif)->version == CAMSS_660)
>                 ret = devm_request_irq(dev, ispif->irq, ispif_isr_8x96,
>                                IRQF_TRIGGER_RISING, ispif->irq_name, ispif);
>         else
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index b7d2293a5004..fae2b513b2f9 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -205,7 +205,8 @@ static u32 vfe_src_pad_code(struct vfe_line *line, u32 sink_code,
>
>                         return sink_code;
>                 }
> -       else if (vfe->camss->version == CAMSS_8x96)
> +       else if (vfe->camss->version == CAMSS_8x96 ||
> +                vfe->camss->version == CAMSS_660)
>                 switch (sink_code) {
>                 case MEDIA_BUS_FMT_YUYV8_2X8:
>                 {
> @@ -1991,12 +1992,19 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,
>         vfe->isr_ops.comp_done = vfe_isr_comp_done;
>         vfe->isr_ops.wm_done = vfe_isr_wm_done;
>
> -       if (camss->version == CAMSS_8x16)
> +       switch (camss->version) {
> +       case CAMSS_8x16:
>                 vfe->ops = &vfe_ops_4_1;
> -       else if (camss->version == CAMSS_8x96)
> +               break;
> +       case CAMSS_8x96:
>                 vfe->ops = &vfe_ops_4_7;
> -       else
> +               break;
> +       case CAMSS_660:
> +               vfe->ops = &vfe_ops_4_8;
> +               break;
> +       default:
>                 return -EINVAL;
> +       }
>
>         /* Memory */
>
> @@ -2095,7 +2103,8 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,
>                                 l->formats = formats_rdi_8x16;
>                                 l->nformats = ARRAY_SIZE(formats_rdi_8x16);
>                         }
> -               } else if (camss->version == CAMSS_8x96) {
> +               } else if (camss->version == CAMSS_8x96 ||
> +                          camss->version == CAMSS_660) {
>                         if (i == VFE_LINE_PIX) {
>                                 l->formats = formats_pix_8x96;
>                                 l->nformats = ARRAY_SIZE(formats_pix_8x96);
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index 114c3ae4a4ab..fbba9e45d732 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -970,7 +970,8 @@ int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev,
>                         video->formats = formats_rdi_8x16;
>                         video->nformats = ARRAY_SIZE(formats_rdi_8x16);
>                 }
> -       } else if (video->camss->version == CAMSS_8x96) {
> +       } else if (video->camss->version == CAMSS_8x96 ||
> +                  video->camss->version == CAMSS_660) {
>                 if (is_pix) {
>                         video->formats = formats_pix_8x96;
>                         video->nformats = ARRAY_SIZE(formats_pix_8x96);
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 9186881afc98..8fefce57bc49 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -283,6 +283,188 @@ static const struct resources vfe_res_8x96[] = {
>         }
>  };
>
> +static const struct resources csiphy_res_660[] = {
> +       /* CSIPHY0 */
> +       {
> +               .regulator = { NULL },
> +               .clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy0_timer",
> +                          "csi0_phy", "csiphy_ahb2crif" },
> +               .clock_rate = { { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 100000000, 200000000, 269333333 },
> +                               { 0 } },
> +               .reg = { "csiphy0", "csiphy0_clk_mux" },
> +               .interrupt = { "csiphy0" }
> +       },
> +
> +       /* CSIPHY1 */
> +       {
> +               .regulator = { NULL },
> +               .clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy1_timer",
> +                          "csi1_phy", "csiphy_ahb2crif" },
> +               .clock_rate = { { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 100000000, 200000000, 269333333 },
> +                               { 0 } },
> +               .reg = { "csiphy1", "csiphy1_clk_mux" },
> +               .interrupt = { "csiphy1" }
> +       },
> +
> +       /* CSIPHY2 */
> +       {
> +               .regulator = { NULL },
> +               .clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy2_timer",
> +                          "csi2_phy", "csiphy_ahb2crif" },
> +               .clock_rate = { { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 100000000, 200000000, 269333333 },
> +                               { 0 } },
> +               .reg = { "csiphy2", "csiphy2_clk_mux" },
> +               .interrupt = { "csiphy2" }
> +       }
> +};
> +
> +static const struct resources csid_res_660[] = {
> +       /* CSID0 */
> +       {
> +               .regulator = { "vdda", "vdd_sec" },
> +               .clock = { "top_ahb", "ispif_ahb", "csi0_ahb", "ahb",
> +                          "csi0", "csi0_phy", "csi0_pix", "csi0_rdi",
> +                          "cphy_csid0" },
> +               .clock_rate = { { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 100000000, 200000000, 310000000,
> +                                 404000000, 465000000 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 } },
> +               .reg = { "csid0" },
> +               .interrupt = { "csid0" }
> +       },
> +
> +       /* CSID1 */
> +       {
> +               .regulator = { "vdda", "vdd_sec" },
> +               .clock = { "top_ahb", "ispif_ahb", "csi1_ahb", "ahb",
> +                          "csi1", "csi1_phy", "csi1_pix", "csi1_rdi",
> +                          "cphy_csid1" },
> +               .clock_rate = { { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 100000000, 200000000, 310000000,
> +                                 404000000, 465000000 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 } },
> +               .reg = { "csid1" },
> +               .interrupt = { "csid1" }
> +       },
> +
> +       /* CSID2 */
> +       {
> +               .regulator = { "vdda", "vdd_sec" },
> +               .clock = { "top_ahb", "ispif_ahb", "csi2_ahb", "ahb",
> +                          "csi2", "csi2_phy", "csi2_pix", "csi2_rdi",
> +                          "cphy_csid2" },
> +               .clock_rate = { { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 100000000, 200000000, 310000000,
> +                                 404000000, 465000000 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 } },
> +               .reg = { "csid2" },
> +               .interrupt = { "csid2" }
> +       },
> +
> +       /* CSID3 */
> +       {
> +               .regulator = { "vdda", "vdd_sec" },
> +               .clock = { "top_ahb", "ispif_ahb", "csi3_ahb", "ahb",
> +                          "csi3", "csi3_phy", "csi3_pix", "csi3_rdi",
> +                          "cphy_csid3" },
> +               .clock_rate = { { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 100000000, 200000000, 310000000,
> +                                 404000000, 465000000 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 } },
> +               .reg = { "csid3" },
> +               .interrupt = { "csid3" }
> +       }
> +};
> +
> +static const struct resources_ispif ispif_res_660 = {
> +       /* ISPIF */
> +       .clock = { "top_ahb", "ahb", "ispif_ahb",
> +                  "csi0", "csi0_pix", "csi0_rdi",
> +                  "csi1", "csi1_pix", "csi1_rdi",
> +                  "csi2", "csi2_pix", "csi2_rdi",
> +                  "csi3", "csi3_pix", "csi3_rdi" },
> +       .clock_for_reset = { "vfe0", "csi_vfe0", "vfe1", "csi_vfe1" },
> +       .reg = { "ispif", "csi_clk_mux" },
> +       .interrupt = "ispif"
> +};
> +
> +static const struct resources vfe_res_660[] = {
> +       /* VFE0 */
> +       {
> +               .regulator = { NULL },
> +               .clock = { "throttle_axi", "top_ahb", "ahb", "vfe0",
> +                          "csi_vfe0", "vfe_ahb", "vfe0_ahb", "vfe_axi",
> +                          "vfe0_stream"},
> +               .clock_rate = { { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 120000000, 200000000, 256000000,
> +                                 300000000, 404000000, 480000000,
> +                                 540000000, 576000000 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 } },
> +               .reg = { "vfe0" },
> +               .interrupt = { "vfe0" }
> +       },
> +
> +       /* VFE1 */
> +       {
> +               .regulator = { NULL },
> +               .clock = { "throttle_axi", "top_ahb", "ahb", "vfe1",
> +                          "csi_vfe1", "vfe_ahb", "vfe1_ahb", "vfe_axi",
> +                          "vfe1_stream"},
> +               .clock_rate = { { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 120000000, 200000000, 256000000,
> +                                 300000000, 404000000, 480000000,
> +                                 540000000, 576000000 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 },
> +                               { 0 } },
> +               .reg = { "vfe1" },
> +               .interrupt = { "vfe1" }
> +       }
> +};
> +
>  /*
>   * camss_add_clock_margin - Add margin to clock frequency rate
>   * @rate: Clock frequency rate
> @@ -397,7 +579,8 @@ int camss_get_pixel_clock(struct media_entity *entity, u32 *pixel_clock)
>
>  int camss_pm_domain_on(struct camss *camss, int id)
>  {
> -       if (camss->version == CAMSS_8x96) {
> +       if (camss->version == CAMSS_8x96 ||
> +           camss->version == CAMSS_660) {
>                 camss->genpd_link[id] = device_link_add(camss->dev,
>                                 camss->genpd[id], DL_FLAG_STATELESS |
>                                 DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE);
> @@ -411,7 +594,8 @@ int camss_pm_domain_on(struct camss *camss, int id)
>
>  void camss_pm_domain_off(struct camss *camss, int id)
>  {
> -       if (camss->version == CAMSS_8x96)
> +       if (camss->version == CAMSS_8x96 ||
> +           camss->version == CAMSS_660)
>                 device_link_del(camss->genpd_link[id]);
>  }
>
> @@ -533,6 +717,11 @@ static int camss_init_subdevices(struct camss *camss)
>                 csid_res = csid_res_8x96;
>                 ispif_res = &ispif_res_8x96;
>                 vfe_res = vfe_res_8x96;
> +       } else if (camss->version == CAMSS_660) {
> +               csiphy_res = csiphy_res_660;
> +               csid_res = csid_res_660;
> +               ispif_res = &ispif_res_660;
> +               vfe_res = vfe_res_660;
>         } else {
>                 return -EINVAL;
>         }
> @@ -833,6 +1022,12 @@ static int camss_probe(struct platform_device *pdev)
>                 camss->csiphy_num = 3;
>                 camss->csid_num = 4;
>                 camss->vfe_num = 2;
> +       } else if (of_device_is_compatible(dev->of_node,
> +                                          "qcom,sdm660-camss")) {
> +               camss->version = CAMSS_660;
> +               camss->csiphy_num = 3;
> +               camss->csid_num = 4;
> +               camss->vfe_num = 2;
>         } else {
>                 ret = -EINVAL;
>                 goto err_free;
> @@ -919,7 +1114,8 @@ static int camss_probe(struct platform_device *pdev)
>                 }
>         }
>
> -       if (camss->version == CAMSS_8x96) {
> +       if (camss->version == CAMSS_8x96 ||
> +           camss->version == CAMSS_660) {
>                 camss->genpd[PM_DOMAIN_VFE0] = dev_pm_domain_attach_by_id(
>                                                 camss->dev, PM_DOMAIN_VFE0);
>                 if (IS_ERR(camss->genpd[PM_DOMAIN_VFE0]))
> @@ -958,7 +1154,8 @@ void camss_delete(struct camss *camss)
>
>         pm_runtime_disable(camss->dev);
>
> -       if (camss->version == CAMSS_8x96) {
> +       if (camss->version == CAMSS_8x96 ||
> +           camss->version == CAMSS_660) {
>                 dev_pm_domain_detach(camss->genpd[PM_DOMAIN_VFE0], true);
>                 dev_pm_domain_detach(camss->genpd[PM_DOMAIN_VFE1], true);
>         }
> @@ -989,6 +1186,7 @@ static int camss_remove(struct platform_device *pdev)
>  static const struct of_device_id camss_dt_match[] = {
>         { .compatible = "qcom,msm8916-camss" },
>         { .compatible = "qcom,msm8996-camss" },
> +       { .compatible = "qcom,sdm660-camss" },
>         { }
>  };
>
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index 1376b07889bf..3a0484683cd6 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -65,6 +65,7 @@ enum pm_domain {
>  enum camss_version {
>         CAMSS_8x16,
>         CAMSS_8x96,
> +       CAMSS_660,
>  };
>
>  struct camss {
> --
> 2.28.0
>

Re: [PATCH 5/6] media: dt-bindings: media: qcom,camss: Add bindings for SDM660 camss

[Robert Foss]

Looks good to me.

Signed-off-by: Robert Foss <robert.foss@linaro.org>

On Sun, 18 Oct 2020 at 14:53, <kholk11@gmail.com> wrote:
>
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> Add bindings for qcom,sdm660-camss in order to support the camera
> subsystem on SDM630/660 and SDA variants.
>
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
>  Documentation/devicetree/bindings/media/qcom,camss.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,camss.txt b/Documentation/devicetree/bindings/media/qcom,camss.txt
> index 09eb6ed99114..498234629e21 100644
> --- a/Documentation/devicetree/bindings/media/qcom,camss.txt
> +++ b/Documentation/devicetree/bindings/media/qcom,camss.txt
> @@ -8,6 +8,7 @@ Qualcomm Camera Subsystem
>         Definition: Should contain one of:
>                 - "qcom,msm8916-camss"
>                 - "qcom,msm8996-camss"
> +               - "qcom,sdm660-camss"
>  - reg:
>         Usage: required
>         Value type: <prop-encoded-array>
> @@ -64,30 +65,36 @@ Qualcomm Camera Subsystem
>         Value type: <stringlist>
>         Definition: Should contain the following entries:
>                 - "top_ahb"
> +               - "throttle_axi"        (660 only)
>                 - "ispif_ahb"
>                 - "csiphy0_timer"
>                 - "csiphy1_timer"
>                 - "csiphy2_timer"       (8996 only)
> +               - "csiphy_ahb2crif"     (660 only)
>                 - "csi0_ahb"
>                 - "csi0"
>                 - "csi0_phy"
>                 - "csi0_pix"
>                 - "csi0_rdi"
> +               - "cphy_csid0"          (660 only)
>                 - "csi1_ahb"
>                 - "csi1"
>                 - "csi1_phy"
>                 - "csi1_pix"
>                 - "csi1_rdi"
> +               - "cphy_csid1"          (660 only)
>                 - "csi2_ahb"            (8996 only)
>                 - "csi2"                (8996 only)
>                 - "csi2_phy"            (8996 only)
>                 - "csi2_pix"            (8996 only)
>                 - "csi2_rdi"            (8996 only)
> +               - "cphy_csid2"          (660 only)
>                 - "csi3_ahb"            (8996 only)
>                 - "csi3"                (8996 only)
>                 - "csi3_phy"            (8996 only)
>                 - "csi3_pix"            (8996 only)
>                 - "csi3_rdi"            (8996 only)
> +               - "cphy_csid3"          (660 only)
>                 - "ahb"
>                 - "vfe0"
>                 - "csi_vfe0"
> --
> 2.28.0
>

Re: [PATCH 6/6] media: camss: csiphy: Set rate on csiX_phy clock on SDM630/660

[Robert Foss]

Looks good to me.

Signed-off-by: Robert Foss <robert.foss@linaro.org>

On Sun, 18 Oct 2020 at 14:53, <kholk11@gmail.com> wrote:
>
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> The SDM630/660 SoCs (and variants) have another clock source
> for the PHY, which must be set to a rate that's equal or
> greater than the CSI PHY timer clock: failing to do this
> will produce PHY overflows when trying to get a stream from
> a very high bandwidth camera sensor and outputting no frame
> or a partial one.
>
> Since I haven't found any usecase in which the csiX_phy
> clock needs to be higher than the csiXphy_timer, let's just
> set the same rate on both, which seems to work just perfect.
>
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
>  .../media/platform/qcom/camss/camss-csiphy.c  | 22 ++++++++++++++++---
>  .../media/platform/qcom/camss/camss-csiphy.h  |  1 +
>  2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
> index c00f25aac21b..a5d717d022a5 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
> @@ -113,9 +113,7 @@ static int csiphy_set_clock_rates(struct csiphy_device *csiphy)
>         for (i = 0; i < csiphy->nclocks; i++) {
>                 struct camss_clock *clock = &csiphy->clock[i];
>
> -               if (!strcmp(clock->name, "csiphy0_timer") ||
> -                   !strcmp(clock->name, "csiphy1_timer") ||
> -                   !strcmp(clock->name, "csiphy2_timer")) {
> +               if (csiphy->rate_set[i]) {
>                         u8 bpp = csiphy_get_bpp(csiphy->formats,
>                                         csiphy->nformats,
>                                         csiphy->fmt[MSM_CSIPHY_PAD_SINK].code);
> @@ -611,6 +609,13 @@ int msm_csiphy_subdev_init(struct camss *camss,
>         if (!csiphy->clock)
>                 return -ENOMEM;
>
> +       csiphy->rate_set = devm_kcalloc(dev,
> +                                       csiphy->nclocks,
> +                                       sizeof(*csiphy->rate_set),
> +                                       GFP_KERNEL);
> +       if (!csiphy->rate_set)
> +               return -ENOMEM;
> +
>         for (i = 0; i < csiphy->nclocks; i++) {
>                 struct camss_clock *clock = &csiphy->clock[i];
>
> @@ -638,6 +643,17 @@ int msm_csiphy_subdev_init(struct camss *camss,
>
>                 for (j = 0; j < clock->nfreqs; j++)
>                         clock->freq[j] = res->clock_rate[i][j];
> +
> +               if (!strcmp(clock->name, "csiphy0_timer") ||
> +                   !strcmp(clock->name, "csiphy1_timer") ||
> +                   !strcmp(clock->name, "csiphy2_timer"))
> +                       csiphy->rate_set[i] = true;
> +
> +               if (camss->version == CAMSS_660 &&
> +                   (!strcmp(clock->name, "csi0_phy") ||
> +                    !strcmp(clock->name, "csi1_phy") ||
> +                    !strcmp(clock->name, "csi2_phy")))
> +                       csiphy->rate_set[i] = true;
>         }
>
>         return 0;
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
> index 376f865ad383..f7967ef836dc 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.h
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
> @@ -66,6 +66,7 @@ struct csiphy_device {
>         u32 irq;
>         char irq_name[30];
>         struct camss_clock *clock;
> +       bool *rate_set;
>         int nclocks;
>         u32 timer_clk_rate;
>         struct csiphy_config cfg;
> --
> 2.28.0
>

Re: [PATCH 0/6] Add support for SDM630/660 Camera Subsystem

[Robert Foss]

Hey Angelo,

Thanks for submitting this, and fixing other issues as you went.

I sent out some SoBs which should have been Reviewed-bys, so if you
add them to your commit please replace the SoBs with RBs.


Rob.

On Sun, 18 Oct 2020 at 14:52, <kholk11@gmail.com> wrote:
>
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> This patch series implements support for the entire camera subsystem
> found in SDM630/636/660 and SDA variants, including CSIPHY 3-Phase,
> CSID v5.0, ISPIF 3.0 (though it didn't need any adaptation) and
> VFE 4.8.
>
> One small note about VFE4.8, even if I wrote it in the commit that
> adds support for it: I know, the VFE support here is split in
> multiple files having the name of the actual VFE version that it is
> targeting... but it didn't feel right to commonize the VFE 4.7 file
> and make another one only for VFE4.8, when it's just about something
> like 3 small differences.
> That VFE 4.8 seems to be just a minor revision of VFE 4.7.
>
> While at it, also fix a small issue when using two VFEs: only one
> of them was being resetted (always VFE0) so, after the first usage
> of VFE1, in case we leave it in a bad state, it would not properly
> start again. Now... it's fine :)))
>
> P.S.: SDM630/660's camss seems to be *very* similar to MSM8998, so
>       likely 90% of this series should be reusable on that one, too!
>
> Tested on:
>  - Sony Xperia XA2 (IMX300 on CSI0/PHY0/VFE0, IMX219 on CSI2,PHY2,VFE1)
>    * VFE0/1 RDI only, as the VIDEO one does not work with SRGGB Bayer
>      formats yet. As far as I can see, that color format hasn't been
>      implemented yet in the video interface.
>
> AngeloGioacchino Del Regno (6):
>   media: camss: csiphy-3ph: Add support for SDM630/660
>   media: camss: ispif: Correctly reset based on the VFE ID
>   media: camss: vfe: Add support for VFE 4.8
>   media: camss: Add support for SDM630/636/660 camera subsystem
>   media: dt-bindings: media: qcom,camss: Add bindings for SDM660 camss
>   media: camss: csiphy: Set rate on csiX_phy clock on SDM630/660
>
>  .../devicetree/bindings/media/qcom,camss.txt  |   7 +
>  .../media/platform/qcom/camss/camss-csid.c    |   9 +-
>  .../qcom/camss/camss-csiphy-3ph-1-0.c         |   7 +-
>  .../media/platform/qcom/camss/camss-csiphy.c  |  25 ++-
>  .../media/platform/qcom/camss/camss-csiphy.h  |   1 +
>  .../media/platform/qcom/camss/camss-ispif.c   | 100 ++++++---
>  .../media/platform/qcom/camss/camss-ispif.h   |   2 +-
>  .../media/platform/qcom/camss/camss-vfe-4-7.c | 129 ++++++++++-
>  drivers/media/platform/qcom/camss/camss-vfe.c |  19 +-
>  drivers/media/platform/qcom/camss/camss-vfe.h |   1 +
>  .../media/platform/qcom/camss/camss-video.c   |   3 +-
>  drivers/media/platform/qcom/camss/camss.c     | 206 +++++++++++++++++-
>  drivers/media/platform/qcom/camss/camss.h     |   1 +
>  13 files changed, 448 insertions(+), 62 deletions(-)
>
> --
> 2.28.0
>

Re: [PATCH 2/6] media: camss: ispif: Correctly reset based on the VFE ID

[AngeloGioacchino Del Regno]

Il giorno mar 20 ott 2020 alle ore 11:03 Robert Foss
<robert.foss@linaro.org> ha scritto:
>
> I found a build issue in this commit.
>
>
> On Tue, 20 Oct 2020 at 10:59, Robert Foss <robert.foss@linaro.org> wrote:
> >
> > Nice catch! This patch looks good to me.
> >
Thank you! :)))

> > Signed-off-by: Robert Foss <robert.foss@linaro.org>
> >
> > On Sun, 18 Oct 2020 at 14:54, <kholk11@gmail.com> wrote:
> > >
> > > From: AngeloGioacchino Del Regno <kholk11@gmail.com>
> > >
> > > Resetting the ISPIF VFE0 context is wrong if we are using the VFE1
> > > for dual-camera or simply because a secondary camera is connected
> > > to it: in this case the reset will always happen on the VFE0 ctx
> > > of the ISPIF, which is .. useless.
> > >
> > > Fix this usecase by adding the ISPIF_RST_CMD_1 address and choose
> > > where to do the (or what to) reset based on the VFE line id.
> > >
> > > Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> > > ---
> > >  .../media/platform/qcom/camss/camss-ispif.c   | 87 ++++++++++++-------
> > >  .../media/platform/qcom/camss/camss-ispif.h   |  2 +-
> > >  2 files changed, 57 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/qcom/camss/camss-ispif.c b/drivers/media/platform/qcom/camss/camss-ispif.c
> > > index db94cfd6c508..252db6b33dab 100644
> > > --- a/drivers/media/platform/qcom/camss/camss-ispif.c
> > > +++ b/drivers/media/platform/qcom/camss/camss-ispif.c
> > > @@ -26,6 +26,7 @@
> > >  #define MSM_ISPIF_NAME "msm_ispif"
> > >
> > >  #define ISPIF_RST_CMD_0                        0x008
> > > +#define ISPIF_RST_CMD_1                        0x00c
> > >  #define ISPIF_RST_CMD_0_STROBED_RST_EN         (1 << 0)
> > >  #define ISPIF_RST_CMD_0_MISC_LOGIC_RST         (1 << 1)
> > >  #define ISPIF_RST_CMD_0_SW_REG_RST             (1 << 2)
> > > @@ -179,7 +180,10 @@ static irqreturn_t ispif_isr_8x96(int irq, void *dev)
> > >         writel(0x1, ispif->base + ISPIF_IRQ_GLOBAL_CLEAR_CMD);
> > >
> > >         if ((value0 >> 27) & 0x1)
> > > -               complete(&ispif->reset_complete);
> > > +               complete(&ispif->reset_complete[0]);
> > > +
> > > +       if ((value3 >> 27) & 0x1)
> > > +               complete(&ispif->reset_complete[1]);
> > >
> > >         if (unlikely(value0 & ISPIF_VFE_m_IRQ_STATUS_0_PIX0_OVERFLOW))
> > >                 dev_err_ratelimited(to_device(ispif), "VFE0 pix0 overflow\n");
> > > @@ -237,7 +241,7 @@ static irqreturn_t ispif_isr_8x16(int irq, void *dev)
> > >         writel(0x1, ispif->base + ISPIF_IRQ_GLOBAL_CLEAR_CMD);
> > >
> > >         if ((value0 >> 27) & 0x1)
> > > -               complete(&ispif->reset_complete);
> > > +               complete(&ispif->reset_complete[0]);
> > >
> > >         if (unlikely(value0 & ISPIF_VFE_m_IRQ_STATUS_0_PIX0_OVERFLOW))
> > >                 dev_err_ratelimited(to_device(ispif), "VFE0 pix0 overflow\n");
> > > @@ -257,33 +261,17 @@ static irqreturn_t ispif_isr_8x16(int irq, void *dev)
> > >         return IRQ_HANDLED;
> > >  }
> > >
> > > -/*
> > > - * ispif_reset - Trigger reset on ISPIF module and wait to complete
> > > - * @ispif: ISPIF device
> > > - *
> > > - * Return 0 on success or a negative error code otherwise
> > > - */
> > > -static int ispif_reset(struct ispif_device *ispif)
> > > +static int ispif_vfe_reset(struct ispif_device *ispif, u8 vfe_id)
> > >  {
> > > -       unsigned long time;
> > >         u32 val;
> > > -       int ret;
> > > -
> > > -       ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE0);
> > > -       if (ret < 0)
> > > -               return ret;
> > >
> > > -       ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE1);
> > > -       if (ret < 0)
> > > -               return ret;
> > > -
> > > -       ret = camss_enable_clocks(ispif->nclocks_for_reset,
> > > -                                 ispif->clock_for_reset,
> > > -                                 to_device(ispif));
> > > -       if (ret < 0)
> > > -               return ret;
> > > +       if (vfe_id > (to_camss(ispif)->vfe_num - 1)) {
> > > +               dev_err(to_device(ispif),
> > > +                       "Error: asked reset for invalid VFE%d\n", vfe_id);
> > > +               return -ENOENT;
> > > +       }
> > >
> > > -       reinit_completion(&ispif->reset_complete);
> > > +       reinit_completion(&ispif->reset_complete[vfe_id]);
> > >
> > >         val = ISPIF_RST_CMD_0_STROBED_RST_EN |
> > >                 ISPIF_RST_CMD_0_MISC_LOGIC_RST |
> > > @@ -303,15 +291,51 @@ static int ispif_reset(struct ispif_device *ispif)
> > >                 ISPIF_RST_CMD_0_RDI_OUTPUT_1_MISR_RST |
> > >                 ISPIF_RST_CMD_0_RDI_OUTPUT_2_MISR_RST;
> > >
> > > -       writel_relaxed(val, ispif->base + ISPIF_RST_CMD_0);
> > > +       if (vfe_id == 1)
> > > +               writel_relaxed(val, ispif->base + ISPIF_RST_CMD_1);
> > > +       else
> > > +               writel_relaxed(val, ispif->base + ISPIF_RST_CMD_0);
> > >
> > > -       time = wait_for_completion_timeout(&ispif->reset_complete,
> > > +       time = wait_for_completion_timeout(&ispif->reset_complete[vfe_id],
>
> 'time' is not a variable that exists in this scope, so the build fails.
>
Sorry about that, it looks like I've messed up a fix for that and it
went into another
unrelated commit instead of this one.
In V2 it's definitely going to be moved here.

> > >                 msecs_to_jiffies(ISPIF_RESET_TIMEOUT_MS));
> > >         if (!time) {
> > > -               dev_err(to_device(ispif), "ISPIF reset timeout\n");
> > > -               ret = -EIO;
> > > +               dev_err(to_device(ispif),
> > > +                       "ISPIF for VFE%d reset timeout\n", vfe_id);
> > > +               return -EIO;
> > >         }
> > >
> > > +       return 0;
> > > +}
> > > +
> > > +/*
> > > + * ispif_reset - Trigger reset on ISPIF module and wait to complete
> > > + * @ispif: ISPIF device
> > > + *
> > > + * Return 0 on success or a negative error code otherwise
> > > + */
> > > +static int ispif_reset(struct ispif_device *ispif, u8 vfe_id)
> > > +{
> > > +       unsigned long time;
> > > +       int ret;
> > > +
> > > +       ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE0);
> > > +       if (ret < 0)
> > > +               return ret;
> > > +
> > > +       ret = camss_pm_domain_on(to_camss(ispif), PM_DOMAIN_VFE1);
> > > +       if (ret < 0)
> > > +               return ret;
> > > +
> > > +       ret = camss_enable_clocks(ispif->nclocks_for_reset,
> > > +                                 ispif->clock_for_reset,
> > > +                                 to_device(ispif));
> > > +       if (ret < 0)
> > > +               return ret;
> > > +
> > > +       ret = ispif_vfe_reset(ispif, vfe_id);
> > > +       if (ret)
> > > +               dev_dbg(to_device(ispif), "ISPIF Reset failed\n");
> > > +
> > >         camss_disable_clocks(ispif->nclocks_for_reset, ispif->clock_for_reset);
> > >
> > >         camss_pm_domain_off(to_camss(ispif), PM_DOMAIN_VFE0);
> > > @@ -355,7 +379,7 @@ static int ispif_set_power(struct v4l2_subdev *sd, int on)
> > >                         goto exit;
> > >                 }
> > >
> > > -               ret = ispif_reset(ispif);
> > > +               ret = ispif_reset(ispif, line->vfe_id);
> > >                 if (ret < 0) {
> > >                         pm_runtime_put_sync(dev);
> > >                         camss_disable_clocks(ispif->nclocks, ispif->clock);
> > > @@ -1192,7 +1216,8 @@ int msm_ispif_subdev_init(struct ispif_device *ispif,
> > >
> > >         mutex_init(&ispif->config_lock);
> > >
> > > -       init_completion(&ispif->reset_complete);
> > > +       for (i = 0; i < MSM_ISPIF_VFE_NUM; i++)
> > > +               init_completion(&ispif->reset_complete[i]);
> > >
> > >         return 0;
> > >  }
> > > diff --git a/drivers/media/platform/qcom/camss/camss-ispif.h b/drivers/media/platform/qcom/camss/camss-ispif.h
> > > index 1a5ba2425a42..4132174f7ea1 100644
> > > --- a/drivers/media/platform/qcom/camss/camss-ispif.h
> > > +++ b/drivers/media/platform/qcom/camss/camss-ispif.h
> > > @@ -56,7 +56,7 @@ struct ispif_device {
> > >         int nclocks;
> > >         struct camss_clock  *clock_for_reset;
> > >         int nclocks_for_reset;
> > > -       struct completion reset_complete;
> > > +       struct completion reset_complete[MSM_ISPIF_VFE_NUM];
> > >         int power_count;
> > >         struct mutex power_lock;
> > >         struct ispif_intf_cmd_reg intf_cmd[MSM_ISPIF_VFE_NUM];
> > > --
> > > 2.28.0
> > >

Regards,
Angelo