Skip to contentSkip to conversation

omap3isp: add support for CSI1 bus

RFC In review Pavel Machek 1 participant 3 messages Feb 14, 2017 – Feb 18, 2017 Media (V4L/DVB)

lore
Share & tools
lore

Pavel Machek

author
#0 lore raw

RFC 4/13 omap3isp: add support for CSI1 bus

Obtain the CSI1/CCP2 bus parameters from the OF node.

ISP CSI1 module needs all the bits correctly set to work.

OMAP3430 needs various syscon CONTROL_CSIRXFE bits set in order to operate. Implement the missing functionality.

[FIXME: Laurent has some comments here]

Signed-off-by Sakari Ailus <[email protected]>
Signed-off-by Ivaylo Dimitrov <[email protected]>
Signed-off-by Pavel Machek <[email protected]>
---
 drivers/media/platform/omap3isp/isp.c      | 158 +++++++++++++++++++++--------
 drivers/media/platform/omap3isp/isp.h      |   2 +-
 drivers/media/platform/omap3isp/ispccp2.c  |  42 +++++++-
 drivers/media/platform/omap3isp/ispreg.h   |   4 +
 drivers/media/platform/omap3isp/omap3isp.h |   1 +
 5 files changed, 159 insertions(+), 48 deletions(-)
drivers/media/platform/omap3isp/isp.c +113 -45
index 084ecf4a..61b7359 100644
@@ -2024,21 +2024,92 @@ enum isp_of_phy {
 	ISP_OF_PHY_CSIPHY2,
 };
 
-static int isp_of_parse_node(struct device *dev, struct device_node *node,
-			     struct isp_async_subdev *isd)
+void __isp_of_parse_node_csi1(struct device *dev,
+				   struct isp_ccp2_cfg *buscfg,
+				   struct v4l2_of_endpoint *vep)
+{
+	buscfg->lanecfg.clk.pos = vep->bus.mipi_csi1.clock_lane;
+	buscfg->lanecfg.clk.pol =
+		vep->bus.mipi_csi1.lane_polarity[0];
+	dev_dbg(dev, "clock lane polarity %u, pos %u\n",
+		buscfg->lanecfg.clk.pol,
+		buscfg->lanecfg.clk.pos);
+
+	buscfg->lanecfg.data[0].pos = vep->bus.mipi_csi2.data_lanes[0];
+	buscfg->lanecfg.data[0].pol =
+		vep->bus.mipi_csi2.lane_polarities[1];
+	dev_dbg(dev, "data lane polarity %u, pos %u\n",
+		buscfg->lanecfg.data[0].pol,
+		buscfg->lanecfg.data[0].pos);
+
+	buscfg->strobe_clk_pol = vep->bus.mipi_csi1.clock_inv;
+	buscfg->phy_layer = vep->bus.mipi_csi1.strobe;
+	buscfg->ccp2_mode = vep->bus_type == V4L2_MBUS_CCP2;
+
+	dev_dbg(dev, "clock_inv %u strobe %u ccp2 %u\n",
+		buscfg->strobe_clk_pol,
+		buscfg->phy_layer,
+		buscfg->ccp2_mode);
+	/*
+	 * FIXME: now we assume the CRC is always there.
+	 * Implement a way to obtain this information from the
+	 * sensor. Frame descriptors, perhaps?
+	 */
+	buscfg->crc = 1;
+
+	buscfg->vp_clk_pol = 1;
+}
+
+static void isp_of_parse_node_csi1(struct device *dev,
+				   struct isp_bus_cfg *buscfg,
+				   struct v4l2_of_endpoint *vep)
+{
+	if (vep->base.port == ISP_OF_PHY_CSIPHY1)
+		buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
+	else
+		buscfg->interface = ISP_INTERFACE_CCP2B_PHY2;
+	__isp_of_parse_node_csi1(dev, &buscfg->bus.ccp2, vep);
+}
+
+static void isp_of_parse_node_csi2(struct device *dev,
+				   struct isp_bus_cfg *buscfg,
+				   struct v4l2_of_endpoint *vep)
 {
-	struct isp_bus_cfg *buscfg = &isd->bus;
-	struct v4l2_of_endpoint vep;
 	unsigned int i;
-	int ret;
 
-	ret = v4l2_of_parse_endpoint(node, &vep);
-	if (ret)
-		return ret;
+	if (vep->base.port == ISP_OF_PHY_CSIPHY1)
+		buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
+	else
+		buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
+	buscfg->bus.csi2.lanecfg.clk.pos = vep->bus.mipi_csi2.clock_lane;
+	buscfg->bus.csi2.lanecfg.clk.pol =
+			vep->bus.mipi_csi2.lane_polarities[0];
+	dev_dbg(dev, "clock lane polarity %u, pos %u\n",
+		buscfg->bus.csi2.lanecfg.clk.pol,
+		buscfg->bus.csi2.lanecfg.clk.pos);
+
+	for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
+		buscfg->bus.csi2.lanecfg.data[i].pos =
+			vep->bus.mipi_csi2.data_lanes[i];
+		buscfg->bus.csi2.lanecfg.data[i].pol =
+			vep->bus.mipi_csi2.lane_polarities[i + 1];
+		dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
+			buscfg->bus.csi2.lanecfg.data[i].pol,
+				buscfg->bus.csi2.lanecfg.data[i].pos);
+	}
 
-	dev_dbg(dev, "parsing endpoint %s, interface %u\n", node->full_name,
-		vep.base.port);
+	/*
+	 * FIXME: now we assume the CRC is always there.
+	 * Implement a way to obtain this information from the
+	 * sensor. Frame descriptors, perhaps?
+	 */
+	buscfg->bus.csi2.crc = 1;
+}
 
+static int isp_endpoint_to_buscfg(struct device *dev,
+				  struct v4l2_of_endpoint vep,
+				  struct isp_bus_cfg *buscfg)
+{
 	switch (vep.base.port) {
 	case ISP_OF_PHY_PARALLEL:
 		buscfg->interface = ISP_INTERFACE_PARALLEL;

@@ -2059,45 +2130,42 @@ static int isp_of_parse_node(struct device *dev, struct device_node *node,
 
 	case ISP_OF_PHY_CSIPHY1:
 	case ISP_OF_PHY_CSIPHY2:
-		/* FIXME: always assume CSI-2 for now. */
-		switch (vep.base.port) {
-		case ISP_OF_PHY_CSIPHY1:
-			buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
-			break;
-		case ISP_OF_PHY_CSIPHY2:
-			buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
-			break;
-		}
-		buscfg->bus.csi2.lanecfg.clk.pos = vep.bus.mipi_csi2.clock_lane;
-		buscfg->bus.csi2.lanecfg.clk.pol =
-			vep.bus.mipi_csi2.lane_polarities[0];
-		dev_dbg(dev, "clock lane polarity %u, pos %u\n",
-			buscfg->bus.csi2.lanecfg.clk.pol,
-			buscfg->bus.csi2.lanecfg.clk.pos);
-
-		for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
-			buscfg->bus.csi2.lanecfg.data[i].pos =
-				vep.bus.mipi_csi2.data_lanes[i];
-			buscfg->bus.csi2.lanecfg.data[i].pol =
-				vep.bus.mipi_csi2.lane_polarities[i + 1];
-			dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
-				buscfg->bus.csi2.lanecfg.data[i].pol,
-				buscfg->bus.csi2.lanecfg.data[i].pos);
-		}
-
-		/*
-		 * FIXME: now we assume the CRC is always there.
-		 * Implement a way to obtain this information from the
-		 * sensor. Frame descriptors, perhaps?
-		 */
-		buscfg->bus.csi2.crc = 1;
+		if (vep.bus_type == V4L2_MBUS_CSI2)
+			isp_of_parse_node_csi2(dev, buscfg, &vep);
+		else
+			isp_of_parse_node_csi1(dev, buscfg, &vep);
 		break;
 
 	default:
+		return -1;
+	}
+	return 0;
+}
+
+static int isp_of_parse_node_endpoint(struct device *dev,
+				      struct device_node *node,
+				      struct isp_async_subdev *isd)
+{
+	struct isp_bus_cfg *buscfg;
+	struct v4l2_of_endpoint vep;
+	int ret;
+
+	isd->bus = devm_kzalloc(dev, sizeof(*isd->bus), GFP_KERNEL);
+	if (!isd->bus)
+		return -ENOMEM;
+
+	buscfg = isd->bus;
+
+	ret = v4l2_of_parse_endpoint(node, &vep);
+	if (ret)
+		return ret;
+
+	dev_dbg(dev, "parsing endpoint %s, interface %u\n", node->full_name,
+		vep.base.port);
+
+	if (isp_endpoint_to_buscfg(dev, vep, buscfg))
 		dev_warn(dev, "%s: invalid interface %u\n", node->full_name,
 			 vep.base.port);
-		break;
-	}
 
 	return 0;
 }

@@ -2122,7 +2190,7 @@ static int isp_of_parse_nodes(struct device *dev,
 
 		notifier->subdevs[notifier->num_subdevs] = &isd->asd;
 
-		if (isp_of_parse_node(dev, node, isd))
+		if (isp_of_parse_node_endpoint(dev, node, isd))
 			goto error;
 
 		isd->asd.match.of.node = of_graph_get_remote_port_parent(node);
drivers/media/platform/omap3isp/isp.h +1 -1
index 7e6f663..c0b9d1d 100644
@@ -228,7 +228,7 @@ struct isp_device {
 
 struct isp_async_subdev {
 	struct v4l2_subdev *sd;
-	struct isp_bus_cfg bus;
+	struct isp_bus_cfg *bus;
 	struct v4l2_async_subdev asd;
 };
drivers/media/platform/omap3isp/ispccp2.c +40 -2

78 lines not on this page · raw message

drivers/media/platform/omap3isp/ispreg.h +4 -0
index b5ea8da..d084839 100644
@@ -87,6 +87,8 @@
 #define ISPCCP2_CTRL_PHY_SEL_MASK	0x1
 #define ISPCCP2_CTRL_PHY_SEL_SHIFT	1
 #define ISPCCP2_CTRL_IO_OUT_SEL		(1 << 2)
+#define ISPCCP2_CTRL_IO_OUT_SEL_MASK	0x1
+#define ISPCCP2_CTRL_IO_OUT_SEL_SHIFT	2
 #define ISPCCP2_CTRL_MODE		(1 << 4)
 #define ISPCCP2_CTRL_VP_CLK_FORCE_ON	(1 << 9)
 #define ISPCCP2_CTRL_INV		(1 << 10)

@@ -94,6 +96,8 @@
 #define ISPCCP2_CTRL_INV_SHIFT		10
 #define ISPCCP2_CTRL_VP_ONLY_EN		(1 << 11)
 #define ISPCCP2_CTRL_VP_CLK_POL		(1 << 12)
+#define ISPCCP2_CTRL_VP_CLK_POL_MASK	0x1
+#define ISPCCP2_CTRL_VP_CLK_POL_SHIFT	12
 #define ISPCCP2_CTRL_VPCLK_DIV_SHIFT	15
 #define ISPCCP2_CTRL_VPCLK_DIV_MASK	0x1ffff /* [31:15] */
 #define ISPCCP2_CTRL_VP_OUT_CTRL_SHIFT	8 /* 3430 bits */
drivers/media/platform/omap3isp/omap3isp.h +1 -0
index 443e8f7..f6d1d0d 100644
@@ -108,6 +108,7 @@ struct isp_ccp2_cfg {
 	unsigned int ccp2_mode:1;
 	unsigned int phy_layer:1;
 	unsigned int vpclk_div:2;
+	unsigned int vp_clk_pol:1;
 	struct isp_csiphy_lanes_cfg lanecfg;
 };
Signature · 6 lines
2.1.4


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#1 lore raw

Hi!

Quoted 4 lines

Obtain the CSI1/CCP2 bus parameters from the OF node.

ISP CSI1 module needs all the bits correctly set to work.

Quoted 8 lines

OMAP3430 needs various syscon CONTROL_CSIRXFE bits set in order to operate. Implement the missing functionality.

[FIXME: Laurent has some comments here]

Signed-off-by Sakari Ailus <[email protected]>
Signed-off-by Ivaylo Dimitrov <[email protected]>
Signed-off-by Pavel Machek <[email protected]>

Ok, Sakari wanted patch series, so here it goes.

Patches from this one on are for illustration -- I still need to fix them up. 1-3, I'd like to see merged, so if you have some comments there, go on.

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#2 lore raw
In reply to Pavel Machek

Hi!

I guess I'll need some help here.

Quoted 21 lines
(quoted diff) +17 -0
@@ -160,6 +163,33 @@ static int ccp2_if_enable(struct isp_ccp2_device *ccp2, u8 enable)
 			return ret;
 	}
 
+	if (isp->revision == ISP_REVISION_2_0) {
+		struct media_pad *pad;
+		struct v4l2_subdev *sensor;
+		const struct isp_ccp2_cfg *buscfg;
+		u32 csirxfe;
+
+		pad = media_entity_remote_pad(&ccp2->pads[CCP2_PAD_SINK]);
+		sensor = media_entity_to_v4l2_subdev(pad->entity);
+		/* Struct isp_bus_cfg has union inside */
+		buscfg = &((struct isp_bus_cfg *)sensor->host_priv)->bus.ccp2;
+
+
+		if (enable) {
+			csirxfe = OMAP343X_CONTROL_CSIRXFE_PWRDNZ |
+				  OMAP343X_CONTROL_CSIRXFE_RESET;
+
+			if (buscfg->phy_layer)

Review static int ccp2_if_enable(struct isp_ccp2_device *ccp2, u8 enable) · line 192

Quoted 10 lines
(quoted diff) +10 -0
+				csirxfe |= OMAP343X_CONTROL_CSIRXFE_SELFORM;
+
+			if (buscfg->strobe_clk_pol)
+				csirxfe |= OMAP343X_CONTROL_CSIRXFE_CSIB_INV;
+		} else
+			csirxfe = 0;
+
+		regmap_write(isp->syscon, isp->syscon_offset, csirxfe);
+	}
+

This is ugly. This does not belong here, it is basically duplicate of

. But that function is not called, because ccp2->phy is not initialized for ISP_REVISION_2_0 in ..._ccp2_init():

int omap3isp_ccp2_init(struct isp_device *isp)

{
        if (isp->revision == ISP_REVISION_2_0) {
                ccp2->vdds_csib = devm_regulator_get(isp->dev, "vdds_csib");
                if (IS_ERR(ccp2->vdds_csib)) {
                        if (PTR_ERR(ccp2->vdds_csib) == -EPROBE_DEFER)
                                return -EPROBE_DEFER;
                        dev_dbg(isp->dev,
                                "Could not get regulator vdds_csib\n");
                        ccp2->vdds_csib = NULL;
                }
        } else if (isp->revision == ISP_REVISION_15_0) {
                ccp2->phy = &isp->isp_csiphy1;
        }

...phy is only initialized for REVISION_15 case. (and isp_csiphy1 seems to contain uninitialized data at this point. Below is a fix for that).

If someone has an idea what to do there, please help. I tried ccp2->phy = &isp->isp_csiphy2; but that does not have pipe initialized, so it eventually leads to crash.

Thanks,
Pavel

drivers/media/platform/omap3isp/ispcsiphy.c +9 -7
index 8f73f6d..a2474b6 100644
@@ -362,14 +374,16 @@ int omap3isp_csiphy_init(struct isp_device *isp)
 	phy2->phy_regs = OMAP3_ISP_IOMEM_CSIPHY2;
 	mutex_init(&phy2->mutex);
 
-	if (isp->revision == ISP_REVISION_15_0) {
-		phy1->isp = isp;
-		phy1->csi2 = &isp->isp_csi2c;
-		phy1->num_data_lanes = ISP_CSIPHY1_NUM_DATA_LANES;
-		phy1->cfg_regs = OMAP3_ISP_IOMEM_CSI2C_REGS1;
-		phy1->phy_regs = OMAP3_ISP_IOMEM_CSIPHY1;
-		mutex_init(&phy1->mutex);
+	if (isp->revision != ISP_REVISION_15_0) {
+		memset(phy1, sizeof(*phy1), 0);
+		return 0;
 	}
 
+	phy1->isp = isp;
+	phy1->csi2 = &isp->isp_csi2c;
+	phy1->num_data_lanes = ISP_CSIPHY1_NUM_DATA_LANES;
+	phy1->cfg_regs = OMAP3_ISP_IOMEM_CSI2C_REGS1;
+	phy1->phy_regs = OMAP3_ISP_IOMEM_CSIPHY1;
+	mutex_init(&phy1->mutex);
 	return 0;
 }
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html