Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/5] Improvements to IMX290 CMOS driver
@ 2019-11-29 19:05 Manivannan Sadhasivam
  2019-11-29 19:05 ` [PATCH 1/5] media: i2c: imx290: Add support for 2 data lanes Manivannan Sadhasivam
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Manivannan Sadhasivam @ 2019-11-29 19:05 UTC (permalink / raw)
  To: mchehab, sakari.ailus
  Cc: linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, peter.griffin, Manivannan Sadhasivam

Hello,

This patchset adds improvements to the existing IMX290 CMOS driver from
Sony. The major changes are adding 2 lane support, test pattern generation,
RAW12 mode support and configurable link frequency & pixel rate.

The link frequency & pixel rate combinations depend on various factors like
lane count, resolution and image format as per the datasheet.

Thanks,
Mani

Manivannan Sadhasivam (5):
  media: i2c: imx290: Add support for 2 data lanes
  media: i2c: imx290: Add support for test pattern generation
  media: i2c: imx290: Add RAW12 mode support
  media: i2c: imx290: Add support to enumerate all frame sizes
  media: i2c: imx290: Add configurable link frequency and pixel rate

 drivers/media/i2c/imx290.c | 328 +++++++++++++++++++++++++++++++------
 1 file changed, 277 insertions(+), 51 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/5] media: i2c: imx290: Add support for 2 data lanes
  2019-11-29 19:05 [PATCH 0/5] Improvements to IMX290 CMOS driver Manivannan Sadhasivam
@ 2019-11-29 19:05 ` Manivannan Sadhasivam
  2019-12-03  8:45   ` Sakari Ailus
  2019-11-29 19:05 ` [PATCH 2/5] media: i2c: imx290: Add support for test pattern generation Manivannan Sadhasivam
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Manivannan Sadhasivam @ 2019-11-29 19:05 UTC (permalink / raw)
  To: mchehab, sakari.ailus
  Cc: linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, peter.griffin, Manivannan Sadhasivam

The IMX290 sensor can output frames with 2/4 CSI2 data lanes. This commit
adds support for 2 lane mode in addition to the 4 lane and also
configuring the data lane settings in the driver based on system
configuration.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/media/i2c/imx290.c | 130 ++++++++++++++++++++++++++++++++++---
 1 file changed, 121 insertions(+), 9 deletions(-)

diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index f7678e5a5d87..1d49910937fb 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -25,7 +25,18 @@
 #define IMX290_STANDBY 0x3000
 #define IMX290_REGHOLD 0x3001
 #define IMX290_XMSTA 0x3002
+#define IMX290_FR_FDG_SEL 0x3009
 #define IMX290_GAIN 0x3014
+#define IMX290_HMAX_LOW 0x301c
+#define IMX290_HMAX_HIGH 0x301d
+#define IMX290_PHY_LANE_NUM 0x3407
+#define IMX290_CSI_LANE_MODE 0x3443
+
+/* HMAX fields */
+#define IMX290_HMAX_2_1920 0x1130
+#define IMX290_HMAX_4_1920 0x0898
+#define IMX290_HMAX_2_720 0x19C8
+#define IMX290_HMAX_4_720 0x0CE4
 
 #define IMX290_DEFAULT_LINK_FREQ 445500000
 
@@ -56,6 +67,7 @@ struct imx290 {
 	struct device *dev;
 	struct clk *xclk;
 	struct regmap *regmap;
+	int nlanes;
 
 	struct v4l2_subdev sd;
 	struct v4l2_fwnode_endpoint ep;
@@ -89,14 +101,11 @@ static const struct regmap_config imx290_regmap_config = {
 
 static const struct imx290_regval imx290_global_init_settings[] = {
 	{ 0x3007, 0x00 },
-	{ 0x3009, 0x00 },
 	{ 0x3018, 0x65 },
 	{ 0x3019, 0x04 },
 	{ 0x301a, 0x00 },
-	{ 0x3443, 0x03 },
 	{ 0x3444, 0x20 },
 	{ 0x3445, 0x25 },
-	{ 0x3407, 0x03 },
 	{ 0x303a, 0x0c },
 	{ 0x3040, 0x00 },
 	{ 0x3041, 0x00 },
@@ -169,7 +178,6 @@ static const struct imx290_regval imx290_1080p_settings[] = {
 	{ 0x3164, 0x1a },
 	{ 0x3480, 0x49 },
 	/* data rate settings */
-	{ 0x3009, 0x01 },
 	{ 0x3405, 0x10 },
 	{ 0x3446, 0x57 },
 	{ 0x3447, 0x00 },
@@ -187,8 +195,6 @@ static const struct imx290_regval imx290_1080p_settings[] = {
 	{ 0x3453, 0x00 },
 	{ 0x3454, 0x17 },
 	{ 0x3455, 0x00 },
-	{ 0x301c, 0x98 },
-	{ 0x301d, 0x08 },
 };
 
 static const struct imx290_regval imx290_720p_settings[] = {
@@ -210,7 +216,6 @@ static const struct imx290_regval imx290_720p_settings[] = {
 	{ 0x3164, 0x1a },
 	{ 0x3480, 0x49 },
 	/* data rate settings */
-	{ 0x3009, 0x01 },
 	{ 0x3405, 0x10 },
 	{ 0x3446, 0x4f },
 	{ 0x3447, 0x00 },
@@ -228,8 +233,6 @@ static const struct imx290_regval imx290_720p_settings[] = {
 	{ 0x3453, 0x00 },
 	{ 0x3454, 0x17 },
 	{ 0x3455, 0x00 },
-	{ 0x301c, 0xe4 },
-	{ 0x301d, 0x0c },
 };
 
 static const struct imx290_regval imx290_10bit_settings[] = {
@@ -522,6 +525,25 @@ static int imx290_write_current_format(struct imx290 *imx290,
 	return 0;
 }
 
+static int imx290_set_hmax(struct imx290 *imx290, u32 val)
+{
+	int ret;
+
+	ret = imx290_write_reg(imx290, IMX290_HMAX_LOW, (val & 0xff));
+	if (ret) {
+		dev_err(imx290->dev, "Error setting HMAX register\n");
+		return ret;
+	}
+
+	ret = imx290_write_reg(imx290, IMX290_HMAX_HIGH, ((val >> 8) & 0xff));
+	if (ret) {
+		dev_err(imx290->dev, "Error setting HMAX register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 /* Start streaming */
 static int imx290_start_streaming(struct imx290 *imx290)
 {
@@ -551,6 +573,40 @@ static int imx290_start_streaming(struct imx290 *imx290)
 		return ret;
 	}
 
+	switch (imx290->nlanes) {
+	case 2:
+		if (imx290->current_mode->width == 1920) {
+			ret = imx290_set_hmax(imx290, IMX290_HMAX_2_1920);
+			if (ret < 0)
+				return ret;
+		} else {
+			ret = imx290_set_hmax(imx290, IMX290_HMAX_2_720);
+			if (ret < 0)
+				return ret;
+		}
+
+		break;
+	case 4:
+		if (imx290->current_mode->width == 1920) {
+			ret = imx290_set_hmax(imx290, IMX290_HMAX_4_1920);
+			if (ret < 0)
+				return ret;
+		} else {
+			ret = imx290_set_hmax(imx290, IMX290_HMAX_4_720);
+			if (ret < 0)
+				return ret;
+		}
+
+		break;
+	default:
+		/*
+		 * We should never hit this since the data lane count is
+		 * validated in probe itself
+		 */
+		dev_err(imx290->dev, "Lane configuration not supported\n");
+		return -EINVAL;
+	}
+
 	/* Apply customized values from user */
 	ret = v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler);
 	if (ret) {
@@ -607,6 +663,49 @@ static int imx290_get_regulators(struct device *dev, struct imx290 *imx290)
 				       imx290->supplies);
 }
 
+static int imx290_set_data_lanes(struct imx290 *imx290)
+{
+	int ret = 0, laneval, frsel;
+
+	switch (imx290->nlanes) {
+	case 2:
+		laneval = 0x01;
+		frsel = 0x02;
+		break;
+	case 4:
+		laneval = 0x03;
+		frsel = 0x01;
+		break;
+	default:
+		/*
+		 * We should never hit this since the data lane count is
+		 * validated in probe itself
+		 */
+		dev_err(imx290->dev, "Lane configuration not supported\n");
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	ret = imx290_write_reg(imx290, IMX290_PHY_LANE_NUM, laneval);
+	if (ret) {
+		dev_err(imx290->dev, "Error setting Physical Lane number register\n");
+		goto exit;
+	}
+
+	ret = imx290_write_reg(imx290, IMX290_CSI_LANE_MODE, laneval);
+	if (ret) {
+		dev_err(imx290->dev, "Error setting CSI Lane mode register\n");
+		goto exit;
+	}
+
+	ret = imx290_write_reg(imx290, IMX290_FR_FDG_SEL, frsel);
+	if (ret)
+		dev_err(imx290->dev, "Error setting FR/FDG SEL register\n");
+
+exit:
+	return ret;
+}
+
 static int imx290_power_on(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
@@ -703,6 +802,16 @@ static int imx290_probe(struct i2c_client *client)
 		goto free_err;
 	}
 
+	/* Get number of data lanes */
+	imx290->nlanes = imx290->ep.bus.mipi_csi2.num_data_lanes;
+	if (imx290->nlanes != 2 && imx290->nlanes != 4) {
+		dev_err(dev, "Invalid data lanes: %d\n", imx290->nlanes);
+		ret = -EINVAL;
+		goto free_err;
+	}
+
+	dev_dbg(dev, "Using %u data lanes\n", imx290->nlanes);
+
 	if (!imx290->ep.nr_of_link_frequencies) {
 		dev_err(dev, "link-frequency property not found in DT\n");
 		ret = -EINVAL;
@@ -822,6 +931,9 @@ static int imx290_probe(struct i2c_client *client)
 		goto free_entity;
 	}
 
+	/* Set data lane count */
+	imx290_set_data_lanes(imx290);
+
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 	pm_runtime_idle(dev);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/5] media: i2c: imx290: Add support for test pattern generation
  2019-11-29 19:05 [PATCH 0/5] Improvements to IMX290 CMOS driver Manivannan Sadhasivam
  2019-11-29 19:05 ` [PATCH 1/5] media: i2c: imx290: Add support for 2 data lanes Manivannan Sadhasivam
@ 2019-11-29 19:05 ` Manivannan Sadhasivam
  2019-12-03  8:48   ` Sakari Ailus
  2019-11-29 19:05 ` [PATCH 3/5] media: i2c: imx290: Add RAW12 mode support Manivannan Sadhasivam
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Manivannan Sadhasivam @ 2019-11-29 19:05 UTC (permalink / raw)
  To: mchehab, sakari.ailus
  Cc: linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, peter.griffin, Manivannan Sadhasivam

Add support for generating following test patterns by IMX290:

* Sequence Pattern 1
* Horizontal Color-bar Chart
* Vertical Color-bar Chart
* Sequence Pattern 2
* Gradation Pattern 1
* Gradation Pattern 2
* 000/555h Toggle Pattern

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/media/i2c/imx290.c | 41 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index 1d49910937fb..e218c959a729 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -26,12 +26,19 @@
 #define IMX290_REGHOLD 0x3001
 #define IMX290_XMSTA 0x3002
 #define IMX290_FR_FDG_SEL 0x3009
+#define IMX290_BLKLEVEL_LOW 0x300a
+#define IMX290_BLKLEVEL_HIGH 0x300b
 #define IMX290_GAIN 0x3014
 #define IMX290_HMAX_LOW 0x301c
 #define IMX290_HMAX_HIGH 0x301d
+#define IMX290_PGCTRL 0x308c
 #define IMX290_PHY_LANE_NUM 0x3407
 #define IMX290_CSI_LANE_MODE 0x3443
 
+#define IMX290_PGCTRL_REGEN BIT(0)
+#define IMX290_PGCTRL_THRU BIT(1)
+#define IMX290_PGCTRL_MODE(n) ((n) << 4)
+
 /* HMAX fields */
 #define IMX290_HMAX_2_1920 0x1130
 #define IMX290_HMAX_4_1920 0x0898
@@ -99,6 +106,17 @@ static const struct regmap_config imx290_regmap_config = {
 	.cache_type = REGCACHE_RBTREE,
 };
 
+static const char * const imx290_test_pattern_menu[] = {
+	"Disabled",
+	"Sequence Pattern 1",
+	"Horizontal Color-bar Chart",
+	"Vertical Color-bar Chart",
+	"Sequence Pattern 2",
+	"Gradation Pattern 1",
+	"Gradation Pattern 2",
+	"000/555h Toggle Pattern",
+};
+
 static const struct imx290_regval imx290_global_init_settings[] = {
 	{ 0x3007, 0x00 },
 	{ 0x3018, 0x65 },
@@ -394,6 +412,22 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_GAIN:
 		ret = imx290_set_gain(imx290, ctrl->val);
 		break;
+	case V4L2_CID_TEST_PATTERN:
+		if (ctrl->val) {
+			imx290_write_reg(imx290, IMX290_BLKLEVEL_LOW, 0x00);
+			imx290_write_reg(imx290, IMX290_BLKLEVEL_HIGH, 0x00);
+			mdelay(10);
+			imx290_write_reg(imx290, IMX290_PGCTRL,
+					 (u8)(IMX290_PGCTRL_REGEN |
+					 IMX290_PGCTRL_THRU |
+					 IMX290_PGCTRL_MODE(ctrl->val)));
+		} else {
+			imx290_write_reg(imx290, IMX290_PGCTRL, 0x00);
+			mdelay(10);
+			imx290_write_reg(imx290, IMX290_BLKLEVEL_LOW, 0x3c);
+			imx290_write_reg(imx290, IMX290_BLKLEVEL_HIGH, 0x00);
+		}
+		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -878,7 +912,7 @@ static int imx290_probe(struct i2c_client *client)
 
 	mutex_init(&imx290->lock);
 
-	v4l2_ctrl_handler_init(&imx290->ctrls, 3);
+	v4l2_ctrl_handler_init(&imx290->ctrls, 4);
 
 	v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
 			  V4L2_CID_GAIN, 0, 72, 1, 0);
@@ -896,6 +930,11 @@ static int imx290_probe(struct i2c_client *client)
 					       INT_MAX, 1,
 					       imx290_modes[0].pixel_rate);
 
+	v4l2_ctrl_new_std_menu_items(&imx290->ctrls, &imx290_ctrl_ops,
+				     V4L2_CID_TEST_PATTERN,
+				     ARRAY_SIZE(imx290_test_pattern_menu) - 1,
+				     0, 0, imx290_test_pattern_menu);
+
 	imx290->sd.ctrl_handler = &imx290->ctrls;
 
 	if (imx290->ctrls.error) {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/5] media: i2c: imx290: Add RAW12 mode support
  2019-11-29 19:05 [PATCH 0/5] Improvements to IMX290 CMOS driver Manivannan Sadhasivam
  2019-11-29 19:05 ` [PATCH 1/5] media: i2c: imx290: Add support for 2 data lanes Manivannan Sadhasivam
  2019-11-29 19:05 ` [PATCH 2/5] media: i2c: imx290: Add support for test pattern generation Manivannan Sadhasivam
@ 2019-11-29 19:05 ` Manivannan Sadhasivam
  2019-11-29 19:49   ` Fabio Estevam
  2019-12-03  8:54   ` Sakari Ailus
  2019-11-29 19:05 ` [PATCH 4/5] media: i2c: imx290: Add support to enumerate all frame sizes Manivannan Sadhasivam
  2019-11-29 19:05 ` [PATCH 5/5] media: i2c: imx290: Add configurable link frequency and pixel rate Manivannan Sadhasivam
  4 siblings, 2 replies; 12+ messages in thread
From: Manivannan Sadhasivam @ 2019-11-29 19:05 UTC (permalink / raw)
  To: mchehab, sakari.ailus
  Cc: linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, peter.griffin, Manivannan Sadhasivam

IMX290 is capable of outputting frames in both Raw Bayer (packed) 10 and
12 bit formats. Since the driver already supports RAW10 mode, let's add
the missing RAW12 mode as well.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/media/i2c/imx290.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index e218c959a729..d5bb3a59ac46 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -75,6 +75,7 @@ struct imx290 {
 	struct clk *xclk;
 	struct regmap *regmap;
 	int nlanes;
+	u8 bpp;
 
 	struct v4l2_subdev sd;
 	struct v4l2_fwnode_endpoint ep;
@@ -98,6 +99,7 @@ struct imx290_pixfmt {
 
 static const struct imx290_pixfmt imx290_formats[] = {
 	{ MEDIA_BUS_FMT_SRGGB10_1X10 },
+	{ MEDIA_BUS_FMT_SRGGB12_1X12 },
 };
 
 static const struct regmap_config imx290_regmap_config = {
@@ -265,6 +267,18 @@ static const struct imx290_regval imx290_10bit_settings[] = {
 	{ 0x300b, 0x00},
 };
 
+static const struct imx290_regval imx290_12bit_settings[] = {
+	{ 0x3005, 0x01 },
+	{ 0x3046, 0x01 },
+	{ 0x3129, 0x00 },
+	{ 0x317c, 0x00 },
+	{ 0x31ec, 0x0e },
+	{ 0x3441, 0x0c },
+	{ 0x3442, 0x0c },
+	{ 0x300a, 0xf0 },
+	{ 0x300b, 0x00 },
+};
+
 /* supported link frequencies */
 static const s64 imx290_link_freq[] = {
 	IMX290_DEFAULT_LINK_FREQ,
@@ -550,6 +564,21 @@ static int imx290_write_current_format(struct imx290 *imx290,
 			dev_err(imx290->dev, "Could not set format registers\n");
 			return ret;
 		}
+
+		imx290->bpp = 10;
+
+		break;
+	case MEDIA_BUS_FMT_SRGGB12_1X12:
+		ret = imx290_set_register_array(imx290, imx290_12bit_settings,
+						ARRAY_SIZE(
+							imx290_12bit_settings));
+		if (ret < 0) {
+			dev_err(imx290->dev, "Could not set format registers\n");
+			return ret;
+		}
+
+		imx290->bpp = 12;
+
 		break;
 	default:
 		dev_err(imx290->dev, "Unknown pixel format\n");
@@ -910,6 +939,9 @@ static int imx290_probe(struct i2c_client *client)
 		goto free_err;
 	}
 
+	/* Default bits per pixel value */
+	imx290->bpp = 10;
+
 	mutex_init(&imx290->lock);
 
 	v4l2_ctrl_handler_init(&imx290->ctrls, 4);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 4/5] media: i2c: imx290: Add support to enumerate all frame sizes
  2019-11-29 19:05 [PATCH 0/5] Improvements to IMX290 CMOS driver Manivannan Sadhasivam
                   ` (2 preceding siblings ...)
  2019-11-29 19:05 ` [PATCH 3/5] media: i2c: imx290: Add RAW12 mode support Manivannan Sadhasivam
@ 2019-11-29 19:05 ` Manivannan Sadhasivam
  2019-12-03  8:56   ` Sakari Ailus
  2019-11-29 19:05 ` [PATCH 5/5] media: i2c: imx290: Add configurable link frequency and pixel rate Manivannan Sadhasivam
  4 siblings, 1 reply; 12+ messages in thread
From: Manivannan Sadhasivam @ 2019-11-29 19:05 UTC (permalink / raw)
  To: mchehab, sakari.ailus
  Cc: linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, peter.griffin, Manivannan Sadhasivam

Add support to enumerate all frame sizes supported by IMX290. This is
required for using with userspace tools such as libcamera.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/media/i2c/imx290.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index d5bb3a59ac46..f26c4a0ee0a0 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -468,6 +468,25 @@ static int imx290_enum_mbus_code(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int imx290_enum_frame_size(struct v4l2_subdev *subdev,
+				  struct v4l2_subdev_pad_config *cfg,
+				  struct v4l2_subdev_frame_size_enum *fse)
+{
+	if ((fse->code != imx290_formats[0].code) &&
+	    (fse->code != imx290_formats[1].code))
+		return -EINVAL;
+
+	if (fse->index >= ARRAY_SIZE(imx290_modes))
+		return -EINVAL;
+
+	fse->min_width = imx290_modes[fse->index].width;
+	fse->max_width = imx290_modes[fse->index].width;
+	fse->min_height = imx290_modes[fse->index].height;
+	fse->max_height = imx290_modes[fse->index].height;
+
+	return 0;
+}
+
 static int imx290_get_fmt(struct v4l2_subdev *sd,
 			  struct v4l2_subdev_pad_config *cfg,
 			  struct v4l2_subdev_format *fmt)
@@ -820,6 +839,7 @@ static const struct v4l2_subdev_video_ops imx290_video_ops = {
 static const struct v4l2_subdev_pad_ops imx290_pad_ops = {
 	.init_cfg = imx290_entity_init_cfg,
 	.enum_mbus_code = imx290_enum_mbus_code,
+	.enum_frame_size = imx290_enum_frame_size,
 	.get_fmt = imx290_get_fmt,
 	.set_fmt = imx290_set_fmt,
 };
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 5/5] media: i2c: imx290: Add configurable link frequency and pixel rate
  2019-11-29 19:05 [PATCH 0/5] Improvements to IMX290 CMOS driver Manivannan Sadhasivam
                   ` (3 preceding siblings ...)
  2019-11-29 19:05 ` [PATCH 4/5] media: i2c: imx290: Add support to enumerate all frame sizes Manivannan Sadhasivam
@ 2019-11-29 19:05 ` Manivannan Sadhasivam
  4 siblings, 0 replies; 12+ messages in thread
From: Manivannan Sadhasivam @ 2019-11-29 19:05 UTC (permalink / raw)
  To: mchehab, sakari.ailus
  Cc: linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, peter.griffin, Manivannan Sadhasivam

IMX290 operates with multiple link frequency and pixel rate combinations.
The initial driver used a single setting for both but since we now have
the lane count support in place, let's add configurable link frequency
and pixel rate.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/media/i2c/imx290.c | 155 +++++++++++++++++++++----------------
 1 file changed, 89 insertions(+), 66 deletions(-)

diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index f26c4a0ee0a0..d794ade26609 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -45,8 +45,6 @@
 #define IMX290_HMAX_2_720 0x19C8
 #define IMX290_HMAX_4_720 0x0CE4
 
-#define IMX290_DEFAULT_LINK_FREQ 445500000
-
 static const char * const imx290_supply_name[] = {
 	"vdda",
 	"vddd",
@@ -63,8 +61,6 @@ struct imx290_regval {
 struct imx290_mode {
 	u32 width;
 	u32 height;
-	u32 pixel_rate;
-	u32 link_freq_index;
 
 	const struct imx290_regval *data;
 	u32 data_size;
@@ -281,7 +277,10 @@ static const struct imx290_regval imx290_12bit_settings[] = {
 
 /* supported link frequencies */
 static const s64 imx290_link_freq[] = {
-	IMX290_DEFAULT_LINK_FREQ,
+	891000000, /* 1920x1080 -  2 lane */
+	445500000, /* 1920x1080 -  4 lane */
+	594000000, /* 1280x720  -  2 lane */
+	297000000, /* 1280x720  -  4 lane */
 };
 
 /* Mode configs */
@@ -291,16 +290,12 @@ static const struct imx290_mode imx290_modes[] = {
 		.height = 1080,
 		.data = imx290_1080p_settings,
 		.data_size = ARRAY_SIZE(imx290_1080p_settings),
-		.pixel_rate = 178200000,
-		.link_freq_index = 0,
 	},
 	{
 		.width = 1280,
 		.height = 720,
 		.data = imx290_720p_settings,
 		.data_size = ARRAY_SIZE(imx290_720p_settings),
-		.pixel_rate = 178200000,
-		.link_freq_index = 0,
 	},
 };
 
@@ -509,6 +504,73 @@ static int imx290_get_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static s64 imx290_get_link_freq_index(struct imx290 *imx290)
+{
+	const struct imx290_mode *cur_mode = imx290->current_mode;
+	u8 index;
+
+	if (cur_mode->width == 1920)
+		index = imx290->nlanes / 4;
+	else
+		index = (imx290->nlanes / 4) + 2;
+
+	return index;
+}
+
+static s64 imx290_get_link_freq(struct imx290 *imx290)
+{
+	u8 index = imx290_get_link_freq_index(imx290);
+
+	return imx290_link_freq[index];
+}
+
+static u64 imx290_calc_pixel_rate(struct imx290 *imx290)
+{
+	s64 link_freq = imx290_get_link_freq(imx290);
+	u8 nlanes = imx290->nlanes;
+
+	/* pixel rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
+	return (link_freq * 2 * nlanes / imx290->bpp);
+}
+
+static int imx290_write_current_format(struct imx290 *imx290,
+				       struct v4l2_mbus_framefmt *format)
+{
+	int ret;
+
+	switch (format->code) {
+	case MEDIA_BUS_FMT_SRGGB10_1X10:
+		ret = imx290_set_register_array(imx290, imx290_10bit_settings,
+						ARRAY_SIZE(
+							imx290_10bit_settings));
+		if (ret < 0) {
+			dev_err(imx290->dev, "Could not set format registers\n");
+			return ret;
+		}
+
+		imx290->bpp = 10;
+
+		break;
+	case MEDIA_BUS_FMT_SRGGB12_1X12:
+		ret = imx290_set_register_array(imx290, imx290_12bit_settings,
+						ARRAY_SIZE(
+							imx290_12bit_settings));
+		if (ret < 0) {
+			dev_err(imx290->dev, "Could not set format registers\n");
+			return ret;
+		}
+
+		imx290->bpp = 12;
+
+		break;
+	default:
+		dev_err(imx290->dev, "Unknown pixel format\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int imx290_set_fmt(struct v4l2_subdev *sd,
 			  struct v4l2_subdev_pad_config *cfg,
 		      struct v4l2_subdev_format *fmt)
@@ -517,6 +579,7 @@ static int imx290_set_fmt(struct v4l2_subdev *sd,
 	const struct imx290_mode *mode;
 	struct v4l2_mbus_framefmt *format;
 	unsigned int i;
+	int ret = 0;
 
 	mutex_lock(&imx290->lock);
 
@@ -542,17 +605,27 @@ static int imx290_set_fmt(struct v4l2_subdev *sd,
 		format = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
 	} else {
 		format = &imx290->current_format;
-		__v4l2_ctrl_s_ctrl(imx290->link_freq, mode->link_freq_index);
-		__v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate, mode->pixel_rate);
-
 		imx290->current_mode = mode;
+
+		/* Set current frame format */
+		ret = imx290_write_current_format(imx290, &fmt->format);
+		if (ret < 0) {
+			dev_err(imx290->dev, "Could not set frame format\n");
+			goto err_out;
+		}
+
+		__v4l2_ctrl_s_ctrl(imx290->link_freq,
+				   imx290_get_link_freq_index(imx290));
+		__v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate,
+					 imx290_calc_pixel_rate(imx290));
 	}
 
 	*format = fmt->format;
 
+err_out:
 	mutex_unlock(&imx290->lock);
 
-	return 0;
+	return ret;
 }
 
 static int imx290_entity_init_cfg(struct v4l2_subdev *subdev,
@@ -569,44 +642,6 @@ static int imx290_entity_init_cfg(struct v4l2_subdev *subdev,
 	return 0;
 }
 
-static int imx290_write_current_format(struct imx290 *imx290,
-				       struct v4l2_mbus_framefmt *format)
-{
-	int ret;
-
-	switch (format->code) {
-	case MEDIA_BUS_FMT_SRGGB10_1X10:
-		ret = imx290_set_register_array(imx290, imx290_10bit_settings,
-						ARRAY_SIZE(
-							imx290_10bit_settings));
-		if (ret < 0) {
-			dev_err(imx290->dev, "Could not set format registers\n");
-			return ret;
-		}
-
-		imx290->bpp = 10;
-
-		break;
-	case MEDIA_BUS_FMT_SRGGB12_1X12:
-		ret = imx290_set_register_array(imx290, imx290_12bit_settings,
-						ARRAY_SIZE(
-							imx290_12bit_settings));
-		if (ret < 0) {
-			dev_err(imx290->dev, "Could not set format registers\n");
-			return ret;
-		}
-
-		imx290->bpp = 12;
-
-		break;
-	default:
-		dev_err(imx290->dev, "Unknown pixel format\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static int imx290_set_hmax(struct imx290 *imx290, u32 val)
 {
 	int ret;
@@ -640,13 +675,6 @@ static int imx290_start_streaming(struct imx290 *imx290)
 		return ret;
 	}
 
-	/* Set current frame format */
-	ret = imx290_write_current_format(imx290, &imx290->current_format);
-	if (ret < 0) {
-		dev_err(imx290->dev, "Could not set frame format\n");
-		return ret;
-	}
-
 	/* Apply default values of current mode */
 	ret = imx290_set_register_array(imx290, imx290->current_mode->data,
 					imx290->current_mode->data_size);
@@ -901,12 +929,6 @@ static int imx290_probe(struct i2c_client *client)
 		goto free_err;
 	}
 
-	if (imx290->ep.link_frequencies[0] != IMX290_DEFAULT_LINK_FREQ) {
-		dev_err(dev, "Unsupported link frequency\n");
-		ret = -EINVAL;
-		goto free_err;
-	}
-
 	/* Only CSI2 is supported for now */
 	if (imx290->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {
 		dev_err(dev, "Unsupported bus type, should be CSI2\n");
@@ -973,14 +995,15 @@ static int imx290_probe(struct i2c_client *client)
 				       &imx290_ctrl_ops,
 				       V4L2_CID_LINK_FREQ,
 				       ARRAY_SIZE(imx290_link_freq) - 1,
-				       0, imx290_link_freq);
+				       (imx290->nlanes / 4),
+				       imx290_link_freq);
 	if (imx290->link_freq)
 		imx290->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 
 	imx290->pixel_rate = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
 					       V4L2_CID_PIXEL_RATE, 1,
 					       INT_MAX, 1,
-					       imx290_modes[0].pixel_rate);
+					       imx290_calc_pixel_rate(imx290));
 
 	v4l2_ctrl_new_std_menu_items(&imx290->ctrls, &imx290_ctrl_ops,
 				     V4L2_CID_TEST_PATTERN,
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] media: i2c: imx290: Add RAW12 mode support
  2019-11-29 19:05 ` [PATCH 3/5] media: i2c: imx290: Add RAW12 mode support Manivannan Sadhasivam
@ 2019-11-29 19:49   ` Fabio Estevam
  2019-11-30 14:09     ` Manivannan Sadhasivam
  2019-12-03  8:54   ` Sakari Ailus
  1 sibling, 1 reply; 12+ messages in thread
From: Fabio Estevam @ 2019-11-29 19:49 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Mauro Carvalho Chehab, Sakari Ailus,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	c.barrett, linux-kernel, a.brela, Peter Griffin,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-media

Hi Manivannan,

On Fri, Nov 29, 2019 at 4:07 PM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
               }
> +
> +               imx290->bpp = 10;
> +
> +               break;
> +       case MEDIA_BUS_FMT_SRGGB12_1X12:
> +               ret = imx290_set_register_array(imx290, imx290_12bit_settings,
> +                                               ARRAY_SIZE(
> +                                                       imx290_12bit_settings));

Could you please write the ARRAY_SIZE and its parameter in the same line?

It would improve readability.

Thanks

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] media: i2c: imx290: Add RAW12 mode support
  2019-11-29 19:49   ` Fabio Estevam
@ 2019-11-30 14:09     ` Manivannan Sadhasivam
  0 siblings, 0 replies; 12+ messages in thread
From: Manivannan Sadhasivam @ 2019-11-30 14:09 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Mauro Carvalho Chehab, Sakari Ailus,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	c.barrett, linux-kernel, a.brela, Peter Griffin,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-media

Hi Fabio,

On Fri, Nov 29, 2019 at 04:49:25PM -0300, Fabio Estevam wrote:
> Hi Manivannan,
> 
> On Fri, Nov 29, 2019 at 4:07 PM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
>                }
> > +
> > +               imx290->bpp = 10;
> > +
> > +               break;
> > +       case MEDIA_BUS_FMT_SRGGB12_1X12:
> > +               ret = imx290_set_register_array(imx290, imx290_12bit_settings,
> > +                                               ARRAY_SIZE(
> > +                                                       imx290_12bit_settings));
> 
> Could you please write the ARRAY_SIZE and its parameter in the same line?
> 
> It would improve readability.
> 

I don't favor this change but Sakari did this to supress the checkpatch
warning while applying my initial patch, so now I did this here itself
to maintain the uniformity.

Thanks,
Mani

> Thanks

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/5] media: i2c: imx290: Add support for 2 data lanes
  2019-11-29 19:05 ` [PATCH 1/5] media: i2c: imx290: Add support for 2 data lanes Manivannan Sadhasivam
@ 2019-12-03  8:45   ` Sakari Ailus
  0 siblings, 0 replies; 12+ messages in thread
From: Sakari Ailus @ 2019-12-03  8:45 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: mchehab, linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, peter.griffin

Hi Manivannan,

Thanks for the patchset.

On Sat, Nov 30, 2019 at 12:35:37AM +0530, Manivannan Sadhasivam wrote:
> The IMX290 sensor can output frames with 2/4 CSI2 data lanes. This commit
> adds support for 2 lane mode in addition to the 4 lane and also
> configuring the data lane settings in the driver based on system
> configuration.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/media/i2c/imx290.c | 130 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 121 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index f7678e5a5d87..1d49910937fb 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -25,7 +25,18 @@
>  #define IMX290_STANDBY 0x3000
>  #define IMX290_REGHOLD 0x3001
>  #define IMX290_XMSTA 0x3002
> +#define IMX290_FR_FDG_SEL 0x3009
>  #define IMX290_GAIN 0x3014
> +#define IMX290_HMAX_LOW 0x301c
> +#define IMX290_HMAX_HIGH 0x301d
> +#define IMX290_PHY_LANE_NUM 0x3407
> +#define IMX290_CSI_LANE_MODE 0x3443
> +
> +/* HMAX fields */
> +#define IMX290_HMAX_2_1920 0x1130
> +#define IMX290_HMAX_4_1920 0x0898
> +#define IMX290_HMAX_2_720 0x19C8
> +#define IMX290_HMAX_4_720 0x0CE4
>  
>  #define IMX290_DEFAULT_LINK_FREQ 445500000
>  
> @@ -56,6 +67,7 @@ struct imx290 {
>  	struct device *dev;
>  	struct clk *xclk;
>  	struct regmap *regmap;
> +	int nlanes;

You're using u8 for another small (unsigned) integer later. How about u8
here?

>  
>  	struct v4l2_subdev sd;
>  	struct v4l2_fwnode_endpoint ep;
> @@ -89,14 +101,11 @@ static const struct regmap_config imx290_regmap_config = {
>  
>  static const struct imx290_regval imx290_global_init_settings[] = {
>  	{ 0x3007, 0x00 },
> -	{ 0x3009, 0x00 },
>  	{ 0x3018, 0x65 },
>  	{ 0x3019, 0x04 },
>  	{ 0x301a, 0x00 },
> -	{ 0x3443, 0x03 },
>  	{ 0x3444, 0x20 },
>  	{ 0x3445, 0x25 },
> -	{ 0x3407, 0x03 },
>  	{ 0x303a, 0x0c },
>  	{ 0x3040, 0x00 },
>  	{ 0x3041, 0x00 },
> @@ -169,7 +178,6 @@ static const struct imx290_regval imx290_1080p_settings[] = {
>  	{ 0x3164, 0x1a },
>  	{ 0x3480, 0x49 },
>  	/* data rate settings */
> -	{ 0x3009, 0x01 },
>  	{ 0x3405, 0x10 },
>  	{ 0x3446, 0x57 },
>  	{ 0x3447, 0x00 },
> @@ -187,8 +195,6 @@ static const struct imx290_regval imx290_1080p_settings[] = {
>  	{ 0x3453, 0x00 },
>  	{ 0x3454, 0x17 },
>  	{ 0x3455, 0x00 },
> -	{ 0x301c, 0x98 },
> -	{ 0x301d, 0x08 },
>  };
>  
>  static const struct imx290_regval imx290_720p_settings[] = {
> @@ -210,7 +216,6 @@ static const struct imx290_regval imx290_720p_settings[] = {
>  	{ 0x3164, 0x1a },
>  	{ 0x3480, 0x49 },
>  	/* data rate settings */
> -	{ 0x3009, 0x01 },
>  	{ 0x3405, 0x10 },
>  	{ 0x3446, 0x4f },
>  	{ 0x3447, 0x00 },
> @@ -228,8 +233,6 @@ static const struct imx290_regval imx290_720p_settings[] = {
>  	{ 0x3453, 0x00 },
>  	{ 0x3454, 0x17 },
>  	{ 0x3455, 0x00 },
> -	{ 0x301c, 0xe4 },
> -	{ 0x301d, 0x0c },
>  };
>  
>  static const struct imx290_regval imx290_10bit_settings[] = {
> @@ -522,6 +525,25 @@ static int imx290_write_current_format(struct imx290 *imx290,
>  	return 0;
>  }
>  
> +static int imx290_set_hmax(struct imx290 *imx290, u32 val)
> +{
> +	int ret;
> +
> +	ret = imx290_write_reg(imx290, IMX290_HMAX_LOW, (val & 0xff));
> +	if (ret) {
> +		dev_err(imx290->dev, "Error setting HMAX register\n");
> +		return ret;
> +	}
> +
> +	ret = imx290_write_reg(imx290, IMX290_HMAX_HIGH, ((val >> 8) & 0xff));
> +	if (ret) {
> +		dev_err(imx290->dev, "Error setting HMAX register\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  /* Start streaming */
>  static int imx290_start_streaming(struct imx290 *imx290)
>  {
> @@ -551,6 +573,40 @@ static int imx290_start_streaming(struct imx290 *imx290)
>  		return ret;
>  	}
>  
> +	switch (imx290->nlanes) {
> +	case 2:
> +		if (imx290->current_mode->width == 1920) {
> +			ret = imx290_set_hmax(imx290, IMX290_HMAX_2_1920);
> +			if (ret < 0)
> +				return ret;
> +		} else {
> +			ret = imx290_set_hmax(imx290, IMX290_HMAX_2_720);
> +			if (ret < 0)
> +				return ret;
> +		}
> +
> +		break;
> +	case 4:
> +		if (imx290->current_mode->width == 1920) {
> +			ret = imx290_set_hmax(imx290, IMX290_HMAX_4_1920);
> +			if (ret < 0)
> +				return ret;
> +		} else {
> +			ret = imx290_set_hmax(imx290, IMX290_HMAX_4_720);
> +			if (ret < 0)
> +				return ret;
> +		}
> +
> +		break;
> +	default:
> +		/*
> +		 * We should never hit this since the data lane count is
> +		 * validated in probe itself
> +		 */
> +		dev_err(imx290->dev, "Lane configuration not supported\n");
> +		return -EINVAL;
> +	}
> +
>  	/* Apply customized values from user */
>  	ret = v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler);
>  	if (ret) {
> @@ -607,6 +663,49 @@ static int imx290_get_regulators(struct device *dev, struct imx290 *imx290)
>  				       imx290->supplies);
>  }
>  
> +static int imx290_set_data_lanes(struct imx290 *imx290)
> +{
> +	int ret = 0, laneval, frsel;
> +
> +	switch (imx290->nlanes) {
> +	case 2:
> +		laneval = 0x01;
> +		frsel = 0x02;
> +		break;
> +	case 4:
> +		laneval = 0x03;
> +		frsel = 0x01;
> +		break;
> +	default:
> +		/*
> +		 * We should never hit this since the data lane count is
> +		 * validated in probe itself
> +		 */
> +		dev_err(imx290->dev, "Lane configuration not supported\n");
> +		ret = -EINVAL;
> +		goto exit;
> +	}
> +
> +	ret = imx290_write_reg(imx290, IMX290_PHY_LANE_NUM, laneval);
> +	if (ret) {
> +		dev_err(imx290->dev, "Error setting Physical Lane number register\n");
> +		goto exit;
> +	}
> +
> +	ret = imx290_write_reg(imx290, IMX290_CSI_LANE_MODE, laneval);
> +	if (ret) {
> +		dev_err(imx290->dev, "Error setting CSI Lane mode register\n");
> +		goto exit;
> +	}
> +
> +	ret = imx290_write_reg(imx290, IMX290_FR_FDG_SEL, frsel);
> +	if (ret)
> +		dev_err(imx290->dev, "Error setting FR/FDG SEL register\n");
> +
> +exit:
> +	return ret;
> +}
> +
>  static int imx290_power_on(struct device *dev)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
> @@ -703,6 +802,16 @@ static int imx290_probe(struct i2c_client *client)
>  		goto free_err;
>  	}
>  
> +	/* Get number of data lanes */
> +	imx290->nlanes = imx290->ep.bus.mipi_csi2.num_data_lanes;
> +	if (imx290->nlanes != 2 && imx290->nlanes != 4) {
> +		dev_err(dev, "Invalid data lanes: %d\n", imx290->nlanes);
> +		ret = -EINVAL;
> +		goto free_err;
> +	}
> +
> +	dev_dbg(dev, "Using %u data lanes\n", imx290->nlanes);
> +
>  	if (!imx290->ep.nr_of_link_frequencies) {
>  		dev_err(dev, "link-frequency property not found in DT\n");
>  		ret = -EINVAL;
> @@ -822,6 +931,9 @@ static int imx290_probe(struct i2c_client *client)
>  		goto free_entity;
>  	}
>  
> +	/* Set data lane count */
> +	imx290_set_data_lanes(imx290);

The sensor is likely (but not necessarily) about to be powered off here.
Wouldn't this also belong to be put to the power on sequence?

> +
>  	pm_runtime_set_active(dev);
>  	pm_runtime_enable(dev);
>  	pm_runtime_idle(dev);

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/5] media: i2c: imx290: Add support for test pattern generation
  2019-11-29 19:05 ` [PATCH 2/5] media: i2c: imx290: Add support for test pattern generation Manivannan Sadhasivam
@ 2019-12-03  8:48   ` Sakari Ailus
  0 siblings, 0 replies; 12+ messages in thread
From: Sakari Ailus @ 2019-12-03  8:48 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: mchehab, linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, peter.griffin

Hi Manivannan,

On Sat, Nov 30, 2019 at 12:35:38AM +0530, Manivannan Sadhasivam wrote:
> Add support for generating following test patterns by IMX290:
> 
> * Sequence Pattern 1
> * Horizontal Color-bar Chart
> * Vertical Color-bar Chart
> * Sequence Pattern 2
> * Gradation Pattern 1
> * Gradation Pattern 2
> * 000/555h Toggle Pattern
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/media/i2c/imx290.c | 41 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index 1d49910937fb..e218c959a729 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -26,12 +26,19 @@
>  #define IMX290_REGHOLD 0x3001
>  #define IMX290_XMSTA 0x3002
>  #define IMX290_FR_FDG_SEL 0x3009
> +#define IMX290_BLKLEVEL_LOW 0x300a
> +#define IMX290_BLKLEVEL_HIGH 0x300b
>  #define IMX290_GAIN 0x3014
>  #define IMX290_HMAX_LOW 0x301c
>  #define IMX290_HMAX_HIGH 0x301d
> +#define IMX290_PGCTRL 0x308c
>  #define IMX290_PHY_LANE_NUM 0x3407
>  #define IMX290_CSI_LANE_MODE 0x3443
>  
> +#define IMX290_PGCTRL_REGEN BIT(0)
> +#define IMX290_PGCTRL_THRU BIT(1)
> +#define IMX290_PGCTRL_MODE(n) ((n) << 4)
> +
>  /* HMAX fields */
>  #define IMX290_HMAX_2_1920 0x1130
>  #define IMX290_HMAX_4_1920 0x0898
> @@ -99,6 +106,17 @@ static const struct regmap_config imx290_regmap_config = {
>  	.cache_type = REGCACHE_RBTREE,
>  };
>  
> +static const char * const imx290_test_pattern_menu[] = {
> +	"Disabled",
> +	"Sequence Pattern 1",
> +	"Horizontal Color-bar Chart",
> +	"Vertical Color-bar Chart",
> +	"Sequence Pattern 2",
> +	"Gradation Pattern 1",
> +	"Gradation Pattern 2",
> +	"000/555h Toggle Pattern",
> +};
> +
>  static const struct imx290_regval imx290_global_init_settings[] = {
>  	{ 0x3007, 0x00 },
>  	{ 0x3018, 0x65 },
> @@ -394,6 +412,22 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
>  	case V4L2_CID_GAIN:
>  		ret = imx290_set_gain(imx290, ctrl->val);
>  		break;
> +	case V4L2_CID_TEST_PATTERN:
> +		if (ctrl->val) {
> +			imx290_write_reg(imx290, IMX290_BLKLEVEL_LOW, 0x00);
> +			imx290_write_reg(imx290, IMX290_BLKLEVEL_HIGH, 0x00);
> +			mdelay(10);

Any particular reason for a busy loop instead of sleeping? Same below.

> +			imx290_write_reg(imx290, IMX290_PGCTRL,
> +					 (u8)(IMX290_PGCTRL_REGEN |
> +					 IMX290_PGCTRL_THRU |
> +					 IMX290_PGCTRL_MODE(ctrl->val)));
> +		} else {
> +			imx290_write_reg(imx290, IMX290_PGCTRL, 0x00);
> +			mdelay(10);
> +			imx290_write_reg(imx290, IMX290_BLKLEVEL_LOW, 0x3c);
> +			imx290_write_reg(imx290, IMX290_BLKLEVEL_HIGH, 0x00);
> +		}
> +		break;
>  	default:
>  		ret = -EINVAL;
>  		break;
> @@ -878,7 +912,7 @@ static int imx290_probe(struct i2c_client *client)
>  
>  	mutex_init(&imx290->lock);
>  
> -	v4l2_ctrl_handler_init(&imx290->ctrls, 3);
> +	v4l2_ctrl_handler_init(&imx290->ctrls, 4);
>  
>  	v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
>  			  V4L2_CID_GAIN, 0, 72, 1, 0);
> @@ -896,6 +930,11 @@ static int imx290_probe(struct i2c_client *client)
>  					       INT_MAX, 1,
>  					       imx290_modes[0].pixel_rate);
>  
> +	v4l2_ctrl_new_std_menu_items(&imx290->ctrls, &imx290_ctrl_ops,
> +				     V4L2_CID_TEST_PATTERN,
> +				     ARRAY_SIZE(imx290_test_pattern_menu) - 1,
> +				     0, 0, imx290_test_pattern_menu);
> +
>  	imx290->sd.ctrl_handler = &imx290->ctrls;
>  
>  	if (imx290->ctrls.error) {

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] media: i2c: imx290: Add RAW12 mode support
  2019-11-29 19:05 ` [PATCH 3/5] media: i2c: imx290: Add RAW12 mode support Manivannan Sadhasivam
  2019-11-29 19:49   ` Fabio Estevam
@ 2019-12-03  8:54   ` Sakari Ailus
  1 sibling, 0 replies; 12+ messages in thread
From: Sakari Ailus @ 2019-12-03  8:54 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: mchehab, linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, peter.griffin

Hi Manivannan,

On Sat, Nov 30, 2019 at 12:35:39AM +0530, Manivannan Sadhasivam wrote:
> IMX290 is capable of outputting frames in both Raw Bayer (packed) 10 and
> 12 bit formats. Since the driver already supports RAW10 mode, let's add
> the missing RAW12 mode as well.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/media/i2c/imx290.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index e218c959a729..d5bb3a59ac46 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -75,6 +75,7 @@ struct imx290 {
>  	struct clk *xclk;
>  	struct regmap *regmap;
>  	int nlanes;
> +	u8 bpp;
>  
>  	struct v4l2_subdev sd;
>  	struct v4l2_fwnode_endpoint ep;
> @@ -98,6 +99,7 @@ struct imx290_pixfmt {
>  
>  static const struct imx290_pixfmt imx290_formats[] = {
>  	{ MEDIA_BUS_FMT_SRGGB10_1X10 },
> +	{ MEDIA_BUS_FMT_SRGGB12_1X12 },
>  };
>  
>  static const struct regmap_config imx290_regmap_config = {
> @@ -265,6 +267,18 @@ static const struct imx290_regval imx290_10bit_settings[] = {
>  	{ 0x300b, 0x00},
>  };
>  
> +static const struct imx290_regval imx290_12bit_settings[] = {
> +	{ 0x3005, 0x01 },
> +	{ 0x3046, 0x01 },
> +	{ 0x3129, 0x00 },
> +	{ 0x317c, 0x00 },
> +	{ 0x31ec, 0x0e },
> +	{ 0x3441, 0x0c },
> +	{ 0x3442, 0x0c },
> +	{ 0x300a, 0xf0 },
> +	{ 0x300b, 0x00 },
> +};
> +
>  /* supported link frequencies */
>  static const s64 imx290_link_freq[] = {
>  	IMX290_DEFAULT_LINK_FREQ,
> @@ -550,6 +564,21 @@ static int imx290_write_current_format(struct imx290 *imx290,
>  			dev_err(imx290->dev, "Could not set format registers\n");
>  			return ret;
>  		}
> +
> +		imx290->bpp = 10;
> +
> +		break;
> +	case MEDIA_BUS_FMT_SRGGB12_1X12:
> +		ret = imx290_set_register_array(imx290, imx290_12bit_settings,
> +						ARRAY_SIZE(
> +							imx290_12bit_settings));
> +		if (ret < 0) {
> +			dev_err(imx290->dev, "Could not set format registers\n");
> +			return ret;
> +		}
> +
> +		imx290->bpp = 12;
> +
>  		break;
>  	default:
>  		dev_err(imx290->dev, "Unknown pixel format\n");
> @@ -910,6 +939,9 @@ static int imx290_probe(struct i2c_client *client)
>  		goto free_err;
>  	}
>  
> +	/* Default bits per pixel value */
> +	imx290->bpp = 10;

Where is the format being initialised at the moment? Nowhere?

If that is the case, I think it should be fixed before this patch.

> +
>  	mutex_init(&imx290->lock);
>  
>  	v4l2_ctrl_handler_init(&imx290->ctrls, 4);

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] media: i2c: imx290: Add support to enumerate all frame sizes
  2019-11-29 19:05 ` [PATCH 4/5] media: i2c: imx290: Add support to enumerate all frame sizes Manivannan Sadhasivam
@ 2019-12-03  8:56   ` Sakari Ailus
  0 siblings, 0 replies; 12+ messages in thread
From: Sakari Ailus @ 2019-12-03  8:56 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: mchehab, linux-media, linux-kernel, devicetree, linux-arm-kernel,
	c.barrett, a.brela, peter.griffin

On Sat, Nov 30, 2019 at 12:35:40AM +0530, Manivannan Sadhasivam wrote:
> Add support to enumerate all frame sizes supported by IMX290. This is
> required for using with userspace tools such as libcamera.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/media/i2c/imx290.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index d5bb3a59ac46..f26c4a0ee0a0 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -468,6 +468,25 @@ static int imx290_enum_mbus_code(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int imx290_enum_frame_size(struct v4l2_subdev *subdev,
> +				  struct v4l2_subdev_pad_config *cfg,
> +				  struct v4l2_subdev_frame_size_enum *fse)
> +{
> +	if ((fse->code != imx290_formats[0].code) &&
> +	    (fse->code != imx290_formats[1].code))

Please use a loop over imx290_formats instead.

> +		return -EINVAL;
> +
> +	if (fse->index >= ARRAY_SIZE(imx290_modes))
> +		return -EINVAL;
> +
> +	fse->min_width = imx290_modes[fse->index].width;
> +	fse->max_width = imx290_modes[fse->index].width;
> +	fse->min_height = imx290_modes[fse->index].height;
> +	fse->max_height = imx290_modes[fse->index].height;
> +
> +	return 0;
> +}
> +
>  static int imx290_get_fmt(struct v4l2_subdev *sd,
>  			  struct v4l2_subdev_pad_config *cfg,
>  			  struct v4l2_subdev_format *fmt)
> @@ -820,6 +839,7 @@ static const struct v4l2_subdev_video_ops imx290_video_ops = {
>  static const struct v4l2_subdev_pad_ops imx290_pad_ops = {
>  	.init_cfg = imx290_entity_init_cfg,
>  	.enum_mbus_code = imx290_enum_mbus_code,
> +	.enum_frame_size = imx290_enum_frame_size,
>  	.get_fmt = imx290_get_fmt,
>  	.set_fmt = imx290_set_fmt,
>  };

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-29 19:05 [PATCH 0/5] Improvements to IMX290 CMOS driver Manivannan Sadhasivam
2019-11-29 19:05 ` [PATCH 1/5] media: i2c: imx290: Add support for 2 data lanes Manivannan Sadhasivam
2019-12-03  8:45   ` Sakari Ailus
2019-11-29 19:05 ` [PATCH 2/5] media: i2c: imx290: Add support for test pattern generation Manivannan Sadhasivam
2019-12-03  8:48   ` Sakari Ailus
2019-11-29 19:05 ` [PATCH 3/5] media: i2c: imx290: Add RAW12 mode support Manivannan Sadhasivam
2019-11-29 19:49   ` Fabio Estevam
2019-11-30 14:09     ` Manivannan Sadhasivam
2019-12-03  8:54   ` Sakari Ailus
2019-11-29 19:05 ` [PATCH 4/5] media: i2c: imx290: Add support to enumerate all frame sizes Manivannan Sadhasivam
2019-12-03  8:56   ` Sakari Ailus
2019-11-29 19:05 ` [PATCH 5/5] media: i2c: imx290: Add configurable link frequency and pixel rate Manivannan Sadhasivam

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git