Various improvements of the st-mipid02 bridge such as usage
cci_* helpers, active state, v4l2_get_link_freq ...
Alain Volmat (7):
media: i2c: st-mipid02: add usage of v4l2_get_link_freq
media: i2c: st-mipid02: don't keep track of streaming status
media: i2c: st-mipid02: use cci_* helpers for register access.
media: i2c: st-mipid02: use active state to store pad formats
media: i2c: st-mipid02: use mipi-csi macro for data-type
media: i2c: st-mipid02: removal of unused link_frequency variable
media: i2c: st-mipid02: add Y8 format support
drivers/media/i2c/Kconfig | 1 +
drivers/media/i2c/st-mipid02.c | 481 +++++++++++----------------------
2 files changed, 153 insertions(+), 329 deletions(-)
--
2.25.1
Use cci_read & cci_write functions for accessing registers.
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/media/i2c/Kconfig | 1 +
drivers/media/i2c/st-mipid02.c | 169 +++++++++------------------------
2 files changed, 47 insertions(+), 123 deletions(-)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 59ee0ca2c978..68f84a47f307 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -1432,6 +1432,7 @@ config VIDEO_ST_MIPID02
depends on I2C && VIDEO_DEV
select MEDIA_CONTROLLER
select VIDEO_V4L2_SUBDEV_API
+ select V4L2_CCI_I2C
select V4L2_FWNODE
help
Support for STMicroelectronics MIPID02 CSI-2 to PARALLEL bridge.
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 1efaa54866a8..15f6c9e8b7f1 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -17,24 +17,25 @@
#include <linux/of_graph.h>
#include <linux/regulator/consumer.h>
#include <media/v4l2-async.h>
+#include <media/v4l2-cci.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
#include <media/v4l2-fwnode.h>
#include <media/v4l2-subdev.h>
-#define MIPID02_CLK_LANE_WR_REG1 0x01
-#define MIPID02_CLK_LANE_REG1 0x02
-#define MIPID02_CLK_LANE_REG3 0x04
-#define MIPID02_DATA_LANE0_REG1 0x05
-#define MIPID02_DATA_LANE0_REG2 0x06
-#define MIPID02_DATA_LANE1_REG1 0x09
-#define MIPID02_DATA_LANE1_REG2 0x0a
-#define MIPID02_MODE_REG1 0x14
-#define MIPID02_MODE_REG2 0x15
-#define MIPID02_DATA_ID_RREG 0x17
-#define MIPID02_DATA_SELECTION_CTRL 0x19
-#define MIPID02_PIX_WIDTH_CTRL 0x1e
-#define MIPID02_PIX_WIDTH_CTRL_EMB 0x1f
+#define MIPID02_CLK_LANE_WR_REG1 CCI_REG8(0x01)
+#define MIPID02_CLK_LANE_REG1 CCI_REG8(0x02)
+#define MIPID02_CLK_LANE_REG3 CCI_REG8(0x04)
+#define MIPID02_DATA_LANE0_REG1 CCI_REG8(0x05)
+#define MIPID02_DATA_LANE0_REG2 CCI_REG8(0x06)
+#define MIPID02_DATA_LANE1_REG1 CCI_REG8(0x09)
+#define MIPID02_DATA_LANE1_REG2 CCI_REG8(0x0a)
+#define MIPID02_MODE_REG1 CCI_REG8(0x14)
+#define MIPID02_MODE_REG2 CCI_REG8(0x15)
+#define MIPID02_DATA_ID_RREG CCI_REG8(0x17)
+#define MIPID02_DATA_SELECTION_CTRL CCI_REG8(0x19)
+#define MIPID02_PIX_WIDTH_CTRL CCI_REG8(0x1e)
+#define MIPID02_PIX_WIDTH_CTRL_EMB CCI_REG8(0x1f)
/* Bits definition for MIPID02_CLK_LANE_REG1 */
#define CLK_ENABLE BIT(0)
@@ -88,6 +89,7 @@ struct mipid02_dev {
struct i2c_client *i2c_client;
struct regulator_bulk_data supplies[MIPID02_NUM_SUPPLIES];
struct v4l2_subdev sd;
+ struct regmap *regmap;
struct media_pad pad[MIPID02_PAD_NB];
struct clk *xclk;
struct gpio_desc *reset_gpio;
@@ -237,62 +239,6 @@ static inline struct mipid02_dev *to_mipid02_dev(struct v4l2_subdev *sd)
return container_of(sd, struct mipid02_dev, sd);
}
-static int mipid02_read_reg(struct mipid02_dev *bridge, u16 reg, u8 *val)
-{
- struct i2c_client *client = bridge->i2c_client;
- struct i2c_msg msg[2];
- u8 buf[2];
- int ret;
-
- buf[0] = reg >> 8;
- buf[1] = reg & 0xff;
-
- msg[0].addr = client->addr;
- msg[0].flags = client->flags;
- msg[0].buf = buf;
- msg[0].len = sizeof(buf);
-
- msg[1].addr = client->addr;
- msg[1].flags = client->flags | I2C_M_RD;
- msg[1].buf = val;
- msg[1].len = 1;
-
- ret = i2c_transfer(client->adapter, msg, 2);
- if (ret < 0) {
- dev_dbg(&client->dev, "%s: %x i2c_transfer, reg: %x => %d\n",
- __func__, client->addr, reg, ret);
- return ret;
- }
-
- return 0;
-}
-
-static int mipid02_write_reg(struct mipid02_dev *bridge, u16 reg, u8 val)
-{
- struct i2c_client *client = bridge->i2c_client;
- struct i2c_msg msg;
- u8 buf[3];
- int ret;
-
- buf[0] = reg >> 8;
- buf[1] = reg & 0xff;
- buf[2] = val;
-
- msg.addr = client->addr;
- msg.flags = client->flags;
- msg.buf = buf;
- msg.len = sizeof(buf);
-
- ret = i2c_transfer(client->adapter, &msg, 1);
- if (ret < 0) {
- dev_dbg(&client->dev, "%s: i2c_transfer, reg: %x => %d\n",
- __func__, reg, ret);
- return ret;
- }
-
- return 0;
-}
-
static int mipid02_get_regulators(struct mipid02_dev *bridge)
{
unsigned int i;
@@ -357,13 +303,13 @@ static void mipid02_set_power_off(struct mipid02_dev *bridge)
static int mipid02_detect(struct mipid02_dev *bridge)
{
- u8 reg;
+ u64 reg;
/*
* There is no version registers. Just try to read register
* MIPID02_CLK_LANE_WR_REG1.
*/
- return mipid02_read_reg(bridge, MIPID02_CLK_LANE_WR_REG1, ®);
+ return cci_read(bridge->regmap, MIPID02_CLK_LANE_WR_REG1, ®, NULL);
}
/*
@@ -524,13 +470,9 @@ static int mipid02_stream_disable(struct mipid02_dev *bridge)
goto error;
/* Disable all lanes */
- ret = mipid02_write_reg(bridge, MIPID02_CLK_LANE_REG1, 0);
- if (ret)
- goto error;
- ret = mipid02_write_reg(bridge, MIPID02_DATA_LANE0_REG1, 0);
- if (ret)
- goto error;
- ret = mipid02_write_reg(bridge, MIPID02_DATA_LANE1_REG1, 0);
+ cci_write(bridge->regmap, MIPID02_CLK_LANE_REG1, 0, &ret);
+ cci_write(bridge->regmap, MIPID02_DATA_LANE0_REG1, 0, &ret);
+ cci_write(bridge->regmap, MIPID02_DATA_LANE1_REG1, 0, &ret);
if (ret)
goto error;
error:
@@ -561,51 +503,26 @@ static int mipid02_stream_enable(struct mipid02_dev *bridge)
goto error;
/* write mipi registers */
- ret = mipid02_write_reg(bridge, MIPID02_CLK_LANE_REG1,
- bridge->r.clk_lane_reg1);
- if (ret)
- goto error;
- ret = mipid02_write_reg(bridge, MIPID02_CLK_LANE_REG3, CLK_MIPI_CSI);
- if (ret)
- goto error;
- ret = mipid02_write_reg(bridge, MIPID02_DATA_LANE0_REG1,
- bridge->r.data_lane0_reg1);
- if (ret)
- goto error;
- ret = mipid02_write_reg(bridge, MIPID02_DATA_LANE0_REG2,
- DATA_MIPI_CSI);
- if (ret)
- goto error;
- ret = mipid02_write_reg(bridge, MIPID02_DATA_LANE1_REG1,
- bridge->r.data_lane1_reg1);
- if (ret)
- goto error;
- ret = mipid02_write_reg(bridge, MIPID02_DATA_LANE1_REG2,
- DATA_MIPI_CSI);
- if (ret)
- goto error;
- ret = mipid02_write_reg(bridge, MIPID02_MODE_REG1,
- MODE_NO_BYPASS | bridge->r.mode_reg1);
- if (ret)
- goto error;
- ret = mipid02_write_reg(bridge, MIPID02_MODE_REG2,
- bridge->r.mode_reg2);
- if (ret)
- goto error;
- ret = mipid02_write_reg(bridge, MIPID02_DATA_ID_RREG,
- bridge->r.data_id_rreg);
- if (ret)
- goto error;
- ret = mipid02_write_reg(bridge, MIPID02_DATA_SELECTION_CTRL,
- bridge->r.data_selection_ctrl);
- if (ret)
- goto error;
- ret = mipid02_write_reg(bridge, MIPID02_PIX_WIDTH_CTRL,
- bridge->r.pix_width_ctrl);
- if (ret)
- goto error;
- ret = mipid02_write_reg(bridge, MIPID02_PIX_WIDTH_CTRL_EMB,
- bridge->r.pix_width_ctrl_emb);
+ cci_write(bridge->regmap, MIPID02_CLK_LANE_REG1,
+ bridge->r.clk_lane_reg1, &ret);
+ cci_write(bridge->regmap, MIPID02_CLK_LANE_REG3, CLK_MIPI_CSI, &ret);
+ cci_write(bridge->regmap, MIPID02_DATA_LANE0_REG1,
+ bridge->r.data_lane0_reg1, &ret);
+ cci_write(bridge->regmap, MIPID02_DATA_LANE0_REG2, DATA_MIPI_CSI, &ret);
+ cci_write(bridge->regmap, MIPID02_DATA_LANE1_REG1,
+ bridge->r.data_lane1_reg1, &ret);
+ cci_write(bridge->regmap, MIPID02_DATA_LANE1_REG2, DATA_MIPI_CSI, &ret);
+ cci_write(bridge->regmap, MIPID02_MODE_REG1,
+ MODE_NO_BYPASS | bridge->r.mode_reg1, &ret);
+ cci_write(bridge->regmap, MIPID02_MODE_REG2, bridge->r.mode_reg2, &ret);
+ cci_write(bridge->regmap, MIPID02_DATA_ID_RREG, bridge->r.data_id_rreg,
+ &ret);
+ cci_write(bridge->regmap, MIPID02_DATA_SELECTION_CTRL,
+ bridge->r.data_selection_ctrl, &ret);
+ cci_write(bridge->regmap, MIPID02_PIX_WIDTH_CTRL,
+ bridge->r.pix_width_ctrl, &ret);
+ cci_write(bridge->regmap, MIPID02_PIX_WIDTH_CTRL_EMB,
+ bridge->r.pix_width_ctrl_emb, &ret);
if (ret)
goto error;
@@ -984,6 +901,12 @@ static int mipid02_probe(struct i2c_client *client)
return ret;
}
+ /* Initialise the regmap for further cci access */
+ bridge->regmap = devm_cci_regmap_init_i2c(client, 16);
+ if (IS_ERR(bridge->regmap))
+ return dev_err_probe(dev, PTR_ERR(bridge->regmap),
+ "failed to get cci regmap\n");
+
mutex_init(&bridge->lock);
bridge->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
bridge->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
--
2.25.1
Use the helper v4l2_get_link_freq instead of performing manually
check of the LINK_FREQ or PIXELRATE ctrls.
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/media/i2c/st-mipid02.c | 66 ++++++++++------------------------
1 file changed, 18 insertions(+), 48 deletions(-)
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 914f915749a8..b699f0b4efe7 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -367,64 +367,34 @@ static int mipid02_detect(struct mipid02_dev *bridge)
return mipid02_read_reg(bridge, MIPID02_CLK_LANE_WR_REG1, ®);
}
-static u32 mipid02_get_link_freq_from_cid_link_freq(struct mipid02_dev *bridge,
- struct v4l2_subdev *subdev)
-{
- struct v4l2_querymenu qm = {.id = V4L2_CID_LINK_FREQ, };
- struct v4l2_ctrl *ctrl;
- int ret;
-
- ctrl = v4l2_ctrl_find(subdev->ctrl_handler, V4L2_CID_LINK_FREQ);
- if (!ctrl)
- return 0;
- qm.index = v4l2_ctrl_g_ctrl(ctrl);
-
- ret = v4l2_querymenu(subdev->ctrl_handler, &qm);
- if (ret)
- return 0;
-
- return qm.value;
-}
-
-static u32 mipid02_get_link_freq_from_cid_pixel_rate(struct mipid02_dev *bridge,
- struct v4l2_subdev *subdev)
-{
- struct v4l2_fwnode_endpoint *ep = &bridge->rx;
- struct v4l2_ctrl *ctrl;
- u32 pixel_clock;
- u32 bpp = bpp_from_code(bridge->fmt.code);
-
- ctrl = v4l2_ctrl_find(subdev->ctrl_handler, V4L2_CID_PIXEL_RATE);
- if (!ctrl)
- return 0;
- pixel_clock = v4l2_ctrl_g_ctrl_int64(ctrl);
-
- return pixel_clock * bpp / (2 * ep->bus.mipi_csi2.num_data_lanes);
-}
-
/*
* We need to know link frequency to setup clk_lane_reg1 timings. Link frequency
- * will be computed using connected device V4L2_CID_PIXEL_RATE, bit per pixel
+ * will be retrieve from connected device via v4l2_get_link_freq, bit per pixel
* and number of lanes.
*/
static int mipid02_configure_from_rx_speed(struct mipid02_dev *bridge)
{
struct i2c_client *client = bridge->i2c_client;
struct v4l2_subdev *subdev = bridge->s_subdev;
- u32 link_freq;
-
- link_freq = mipid02_get_link_freq_from_cid_link_freq(bridge, subdev);
- if (!link_freq) {
- link_freq = mipid02_get_link_freq_from_cid_pixel_rate(bridge,
- subdev);
- if (!link_freq) {
- dev_err(&client->dev, "Failed to get link frequency");
- return -EINVAL;
- }
+ struct v4l2_fwnode_endpoint *ep = &bridge->rx;
+ u32 bpp = bpp_from_code(bridge->fmt.code);
+ /*
+ * clk_lane_reg1 requires 4 times the unit interval time, and bitrate
+ * is twice the link frequency, hence ui_4 = 1000000000 * 4 / 2
+ */
+ u64 ui_4 = 2000000000;
+ s64 link_freq;
+
+ link_freq = v4l2_get_link_freq(subdev->ctrl_handler, bpp,
+ 2 * ep->bus.mipi_csi2.num_data_lanes);
+ if (link_freq < 0) {
+ dev_err(&client->dev, "Failed to get link frequency");
+ return -EINVAL;
}
- dev_dbg(&client->dev, "detect link_freq = %d Hz", link_freq);
- bridge->r.clk_lane_reg1 |= (2000000000 / link_freq) << 2;
+ dev_dbg(&client->dev, "detect link_freq = %lld Hz", link_freq);
+ do_div(ui_4, link_freq);
+ bridge->r.clk_lane_reg1 |= ui_4 << 2;
return 0;
}
--
2.25.1
Store formats information within pad allowing to simplify further more
the driver (mutex / format store within the driver structure no more
necessary).
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/media/i2c/st-mipid02.c | 206 ++++++++++++---------------------
1 file changed, 75 insertions(+), 131 deletions(-)
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 15f6c9e8b7f1..7af209905d7b 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -112,9 +112,6 @@ struct mipid02_dev {
u8 pix_width_ctrl;
u8 pix_width_ctrl_emb;
} r;
- /* lock to protect all members below */
- struct mutex lock;
- struct v4l2_mbus_framefmt fmt;
};
static int bpp_from_code(__u32 code)
@@ -192,18 +189,6 @@ static u8 data_type_from_code(__u32 code)
}
}
-static void init_format(struct v4l2_mbus_framefmt *fmt)
-{
- fmt->code = MEDIA_BUS_FMT_SBGGR8_1X8;
- fmt->field = V4L2_FIELD_NONE;
- fmt->colorspace = V4L2_COLORSPACE_SRGB;
- fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SRGB);
- fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
- fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB);
- fmt->width = 640;
- fmt->height = 480;
-}
-
static __u32 get_fmt_code(__u32 code)
{
unsigned int i;
@@ -317,12 +302,13 @@ static int mipid02_detect(struct mipid02_dev *bridge)
* will be retrieve from connected device via v4l2_get_link_freq, bit per pixel
* and number of lanes.
*/
-static int mipid02_configure_from_rx_speed(struct mipid02_dev *bridge)
+static int mipid02_configure_from_rx_speed(struct mipid02_dev *bridge,
+ struct v4l2_mbus_framefmt *fmt)
{
struct i2c_client *client = bridge->i2c_client;
struct v4l2_subdev *subdev = bridge->s_subdev;
struct v4l2_fwnode_endpoint *ep = &bridge->rx;
- u32 bpp = bpp_from_code(bridge->fmt.code);
+ u32 bpp = bpp_from_code(fmt->code);
/*
* clk_lane_reg1 requires 4 times the unit interval time, and bitrate
* is twice the link frequency, hence ui_4 = 1000000000 * 4 / 2
@@ -394,7 +380,8 @@ static int mipid02_configure_data1_lane(struct mipid02_dev *bridge, int nb,
return 0;
}
-static int mipid02_configure_from_rx(struct mipid02_dev *bridge)
+static int mipid02_configure_from_rx(struct mipid02_dev *bridge,
+ struct v4l2_mbus_framefmt *fmt)
{
struct v4l2_fwnode_endpoint *ep = &bridge->rx;
bool are_lanes_swap = ep->bus.mipi_csi2.data_lanes[0] == 2;
@@ -419,7 +406,7 @@ static int mipid02_configure_from_rx(struct mipid02_dev *bridge)
bridge->r.mode_reg1 |= are_lanes_swap ? MODE_DATA_SWAP : 0;
bridge->r.mode_reg1 |= (nb - 1) << 1;
- return mipid02_configure_from_rx_speed(bridge);
+ return mipid02_configure_from_rx_speed(bridge, fmt);
}
static int mipid02_configure_from_tx(struct mipid02_dev *bridge)
@@ -439,16 +426,17 @@ static int mipid02_configure_from_tx(struct mipid02_dev *bridge)
return 0;
}
-static int mipid02_configure_from_code(struct mipid02_dev *bridge)
+static int mipid02_configure_from_code(struct mipid02_dev *bridge,
+ struct v4l2_mbus_framefmt *fmt)
{
u8 data_type;
bridge->r.data_id_rreg = 0;
- if (bridge->fmt.code != MEDIA_BUS_FMT_JPEG_1X8) {
+ if (fmt->code != MEDIA_BUS_FMT_JPEG_1X8) {
bridge->r.data_selection_ctrl |= SELECTION_MANUAL_DATA;
- data_type = data_type_from_code(bridge->fmt.code);
+ data_type = data_type_from_code(fmt->code);
if (!data_type)
return -EINVAL;
bridge->r.data_id_rreg = data_type;
@@ -485,23 +473,31 @@ static int mipid02_stream_disable(struct mipid02_dev *bridge)
static int mipid02_stream_enable(struct mipid02_dev *bridge)
{
struct i2c_client *client = bridge->i2c_client;
+ struct v4l2_subdev_state *state;
+ struct v4l2_mbus_framefmt *fmt;
int ret = -EINVAL;
if (!bridge->s_subdev)
goto error;
memset(&bridge->r, 0, sizeof(bridge->r));
+
+ state = v4l2_subdev_lock_and_get_active_state(&bridge->sd);
+ fmt = v4l2_subdev_state_get_format(state, MIPID02_SINK_0);
+
/* build registers content */
- ret = mipid02_configure_from_rx(bridge);
+ ret = mipid02_configure_from_rx(bridge, fmt);
if (ret)
goto error;
ret = mipid02_configure_from_tx(bridge);
if (ret)
goto error;
- ret = mipid02_configure_from_code(bridge);
+ ret = mipid02_configure_from_code(bridge, fmt);
if (ret)
goto error;
+ v4l2_subdev_unlock_state(state);
+
/* write mipi registers */
cci_write(bridge->regmap, MIPID02_CLK_LANE_REG1,
bridge->r.clk_lane_reg1, &ret);
@@ -556,11 +552,32 @@ static int mipid02_s_stream(struct v4l2_subdev *sd, int enable)
return ret;
}
+static const struct v4l2_mbus_framefmt default_fmt = {
+ .code = MEDIA_BUS_FMT_SBGGR8_1X8,
+ .field = V4L2_FIELD_NONE,
+ .colorspace = V4L2_COLORSPACE_SRGB,
+ .ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT,
+ .quantization = V4L2_QUANTIZATION_FULL_RANGE,
+ .xfer_func = V4L2_XFER_FUNC_DEFAULT,
+ .width = 640,
+ .height = 480,
+};
+
+static int mipid02_init_cfg(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state)
+{
+ *v4l2_subdev_state_get_format(state, MIPID02_SINK_0) = default_fmt;
+ /* MIPID02_SINK_1 isn't supported yet */
+ *v4l2_subdev_state_get_format(state, MIPID02_SOURCE) = default_fmt;
+
+ return 0;
+}
+
static int mipid02_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_mbus_code_enum *code)
{
- struct mipid02_dev *bridge = to_mipid02_dev(sd);
+ struct v4l2_mbus_framefmt *sink_fmt;
int ret = 0;
switch (code->pad) {
@@ -571,10 +588,13 @@ static int mipid02_enum_mbus_code(struct v4l2_subdev *sd,
code->code = mipid02_supported_fmt_codes[code->index];
break;
case MIPID02_SOURCE:
- if (code->index == 0)
- code->code = serial_to_parallel_code(bridge->fmt.code);
- else
+ if (code->index == 0) {
+ sink_fmt = v4l2_subdev_state_get_format(sd_state,
+ MIPID02_SINK_0);
+ code->code = serial_to_parallel_code(sink_fmt->code);
+ } else {
ret = -EINVAL;
+ }
break;
default:
ret = -EINVAL;
@@ -583,113 +603,36 @@ static int mipid02_enum_mbus_code(struct v4l2_subdev *sd,
return ret;
}
-static int mipid02_get_fmt(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *format)
-{
- struct v4l2_mbus_framefmt *mbus_fmt = &format->format;
- struct mipid02_dev *bridge = to_mipid02_dev(sd);
- struct i2c_client *client = bridge->i2c_client;
- struct v4l2_mbus_framefmt *fmt;
-
- dev_dbg(&client->dev, "%s probe %d", __func__, format->pad);
-
- if (format->pad >= MIPID02_PAD_NB)
- return -EINVAL;
- /* second CSI-2 pad not yet supported */
- if (format->pad == MIPID02_SINK_1)
- return -EINVAL;
-
- if (format->which == V4L2_SUBDEV_FORMAT_TRY)
- fmt = v4l2_subdev_state_get_format(sd_state, format->pad);
- else
- fmt = &bridge->fmt;
-
- mutex_lock(&bridge->lock);
-
- *mbus_fmt = *fmt;
- /* code may need to be converted for source */
- if (format->pad == MIPID02_SOURCE)
- mbus_fmt->code = serial_to_parallel_code(mbus_fmt->code);
-
- mutex_unlock(&bridge->lock);
-
- return 0;
-}
-
-static void mipid02_set_fmt_source(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *format)
-{
- struct mipid02_dev *bridge = to_mipid02_dev(sd);
-
- /* source pad mirror sink pad */
- if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
- format->format = bridge->fmt;
- else
- format->format = *v4l2_subdev_state_get_format(sd_state,
- MIPID02_SINK_0);
-
- /* but code may need to be converted */
- format->format.code = serial_to_parallel_code(format->format.code);
-
- /* only apply format for V4L2_SUBDEV_FORMAT_TRY case */
- if (format->which != V4L2_SUBDEV_FORMAT_TRY)
- return;
-
- *v4l2_subdev_state_get_format(sd_state, MIPID02_SOURCE) =
- format->format;
-}
-
-static void mipid02_set_fmt_sink(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *format)
-{
- struct mipid02_dev *bridge = to_mipid02_dev(sd);
- struct v4l2_subdev_format source_fmt;
- struct v4l2_mbus_framefmt *fmt;
-
- format->format.code = get_fmt_code(format->format.code);
-
- if (format->which == V4L2_SUBDEV_FORMAT_TRY)
- fmt = v4l2_subdev_state_get_format(sd_state, format->pad);
- else
- fmt = &bridge->fmt;
-
- *fmt = format->format;
-
- /*
- * Propagate the format change to the source pad, taking
- * care not to update the format pointer given back to user
- */
- source_fmt = *format;
- mipid02_set_fmt_source(sd, sd_state, &source_fmt);
-}
-
static int mipid02_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *format)
+ struct v4l2_subdev_format *fmt)
{
struct mipid02_dev *bridge = to_mipid02_dev(sd);
struct i2c_client *client = bridge->i2c_client;
- int ret = 0;
+ struct v4l2_mbus_framefmt *pad_fmt;
- dev_dbg(&client->dev, "%s for %d", __func__, format->pad);
+ dev_dbg(&client->dev, "%s for %d", __func__, fmt->pad);
- if (format->pad >= MIPID02_PAD_NB)
- return -EINVAL;
/* second CSI-2 pad not yet supported */
- if (format->pad == MIPID02_SINK_1)
+ if (fmt->pad == MIPID02_SINK_1)
return -EINVAL;
- mutex_lock(&bridge->lock);
+ pad_fmt = v4l2_subdev_state_get_format(sd_state, fmt->pad);
+ fmt->format.code = get_fmt_code(fmt->format.code);
- if (format->pad == MIPID02_SOURCE)
- mipid02_set_fmt_source(sd, sd_state, format);
- else
- mipid02_set_fmt_sink(sd, sd_state, format);
+ /* code may need to be converted */
+ if (fmt->pad == MIPID02_SOURCE)
+ fmt->format.code = serial_to_parallel_code(fmt->format.code);
- mutex_unlock(&bridge->lock);
+ *pad_fmt = fmt->format;
+
+ /* Propagate the format to the source pad in case of sink pad update */
+ if (fmt->pad == MIPID02_SINK_0) {
+ pad_fmt = v4l2_subdev_state_get_format(sd_state,
+ MIPID02_SOURCE);
+ *pad_fmt = fmt->format;
+ pad_fmt->code = serial_to_parallel_code(fmt->format.code);
+ }
return 0;
}
@@ -699,8 +642,9 @@ static const struct v4l2_subdev_video_ops mipid02_video_ops = {
};
static const struct v4l2_subdev_pad_ops mipid02_pad_ops = {
+ .init_cfg = mipid02_init_cfg,
.enum_mbus_code = mipid02_enum_mbus_code,
- .get_fmt = mipid02_get_fmt,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = mipid02_set_fmt,
};
@@ -868,8 +812,6 @@ static int mipid02_probe(struct i2c_client *client)
if (!bridge)
return -ENOMEM;
- init_format(&bridge->fmt);
-
bridge->i2c_client = client;
v4l2_i2c_subdev_init(&bridge->sd, client, &mipid02_subdev_ops);
@@ -907,7 +849,6 @@ static int mipid02_probe(struct i2c_client *client)
return dev_err_probe(dev, PTR_ERR(bridge->regmap),
"failed to get cci regmap\n");
- mutex_init(&bridge->lock);
bridge->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
bridge->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
bridge->sd.entity.ops = &mipid02_subdev_entity_ops;
@@ -918,7 +859,13 @@ static int mipid02_probe(struct i2c_client *client)
bridge->pad);
if (ret) {
dev_err(&client->dev, "pads init failed %d", ret);
- goto mutex_cleanup;
+ return ret;
+ }
+
+ ret = v4l2_subdev_init_finalize(&bridge->sd);
+ if (ret < 0) {
+ dev_err(dev, "subdev init error: %d\n", ret);
+ goto entity_cleanup;
}
/* enable clock, power and reset device if available */
@@ -962,8 +909,6 @@ static int mipid02_probe(struct i2c_client *client)
mipid02_set_power_off(bridge);
entity_cleanup:
media_entity_cleanup(&bridge->sd.entity);
-mutex_cleanup:
- mutex_destroy(&bridge->lock);
return ret;
}
@@ -978,7 +923,6 @@ static void mipid02_remove(struct i2c_client *client)
v4l2_async_unregister_subdev(&bridge->sd);
mipid02_set_power_off(bridge);
media_entity_cleanup(&bridge->sd.entity);
- mutex_destroy(&bridge->lock);
}
static const struct of_device_id mipid02_dt_ids[] = {
--
2.25.1
Use MIPI data-type macros.
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/media/i2c/st-mipid02.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 7af209905d7b..783c2848c584 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -16,6 +16,7 @@
#include <linux/module.h>
#include <linux/of_graph.h>
#include <linux/regulator/consumer.h>
+#include <media/mipi-csi2.h>
#include <media/v4l2-async.h>
#include <media/v4l2-cci.h>
#include <media/v4l2-ctrls.h>
@@ -158,17 +159,17 @@ static u8 data_type_from_code(__u32 code)
case MEDIA_BUS_FMT_SGBRG8_1X8:
case MEDIA_BUS_FMT_SGRBG8_1X8:
case MEDIA_BUS_FMT_SRGGB8_1X8:
- return 0x2a;
+ return MIPI_CSI2_DT_RAW8;
case MEDIA_BUS_FMT_SBGGR10_1X10:
case MEDIA_BUS_FMT_SGBRG10_1X10:
case MEDIA_BUS_FMT_SGRBG10_1X10:
case MEDIA_BUS_FMT_SRGGB10_1X10:
- return 0x2b;
+ return MIPI_CSI2_DT_RAW10;
case MEDIA_BUS_FMT_SBGGR12_1X12:
case MEDIA_BUS_FMT_SGBRG12_1X12:
case MEDIA_BUS_FMT_SGRBG12_1X12:
case MEDIA_BUS_FMT_SRGGB12_1X12:
- return 0x2c;
+ return MIPI_CSI2_DT_RAW12;
case MEDIA_BUS_FMT_YUYV8_1X16:
case MEDIA_BUS_FMT_YVYU8_1X16:
case MEDIA_BUS_FMT_UYVY8_1X16:
@@ -177,13 +178,13 @@ static u8 data_type_from_code(__u32 code)
case MEDIA_BUS_FMT_YVYU8_2X8:
case MEDIA_BUS_FMT_UYVY8_2X8:
case MEDIA_BUS_FMT_VYUY8_2X8:
- return 0x1e;
+ return MIPI_CSI2_DT_YUV422_8B;
case MEDIA_BUS_FMT_BGR888_1X24:
- return 0x24;
+ return MIPI_CSI2_DT_RGB888;
case MEDIA_BUS_FMT_RGB565_1X16:
case MEDIA_BUS_FMT_RGB565_2X8_LE:
case MEDIA_BUS_FMT_RGB565_2X8_BE:
- return 0x22;
+ return MIPI_CSI2_DT_RGB565;
default:
return 0;
}
--
2.25.1
link_frequency variable within struct mipid02_dev seems to have never
been used hence remove it.
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/media/i2c/st-mipid02.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 783c2848c584..ef0d7f8ba17d 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -96,7 +96,6 @@ struct mipid02_dev {
struct gpio_desc *reset_gpio;
/* endpoints info */
struct v4l2_fwnode_endpoint rx;
- u64 link_frequency;
struct v4l2_fwnode_endpoint tx;
/* remote source */
struct v4l2_async_notifier notifier;
--
2.25.1
Add support of MEDIA_BUS_FMT_Y8_1X8.
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/media/i2c/st-mipid02.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index ef0d7f8ba17d..366e840c8858 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -70,7 +70,7 @@ static const u32 mipid02_supported_fmt_codes[] = {
MEDIA_BUS_FMT_RGB565_2X8_LE, MEDIA_BUS_FMT_RGB565_2X8_BE,
MEDIA_BUS_FMT_YUYV8_2X8, MEDIA_BUS_FMT_YVYU8_2X8,
MEDIA_BUS_FMT_UYVY8_2X8, MEDIA_BUS_FMT_VYUY8_2X8,
- MEDIA_BUS_FMT_JPEG_1X8
+ MEDIA_BUS_FMT_Y8_1X8, MEDIA_BUS_FMT_JPEG_1X8
};
/* regulator supplies */
@@ -121,6 +121,7 @@ static int bpp_from_code(__u32 code)
case MEDIA_BUS_FMT_SGBRG8_1X8:
case MEDIA_BUS_FMT_SGRBG8_1X8:
case MEDIA_BUS_FMT_SRGGB8_1X8:
+ case MEDIA_BUS_FMT_Y8_1X8:
return 8;
case MEDIA_BUS_FMT_SBGGR10_1X10:
case MEDIA_BUS_FMT_SGBRG10_1X10:
@@ -158,6 +159,7 @@ static u8 data_type_from_code(__u32 code)
case MEDIA_BUS_FMT_SGBRG8_1X8:
case MEDIA_BUS_FMT_SGRBG8_1X8:
case MEDIA_BUS_FMT_SRGGB8_1X8:
+ case MEDIA_BUS_FMT_Y8_1X8:
return MIPI_CSI2_DT_RAW8;
case MEDIA_BUS_FMT_SBGGR10_1X10:
case MEDIA_BUS_FMT_SGBRG10_1X10:
--
2.25.1
On Sat, Nov 25, 2023 at 07:20:52PM +0100, Alain Volmat wrote:
> Store formats information within pad allowing to simplify further more
> the driver (mutex / format store within the driver structure no more
> necessary).
>
> Signed-off-by: Alain Volmat <[email protected]>
Applied with the following diff:
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 7af209905d7b..2cc07b3ed0da 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -563,8 +563,8 @@ static const struct v4l2_mbus_framefmt default_fmt = {
.height = 480,
};
-static int mipid02_init_cfg(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *state)
+static int mipid02_init_state(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state)
{
*v4l2_subdev_state_get_format(state, MIPID02_SINK_0) = default_fmt;
/* MIPID02_SINK_1 isn't supported yet */
@@ -642,7 +642,6 @@ static const struct v4l2_subdev_video_ops mipid02_video_ops = {
};
static const struct v4l2_subdev_pad_ops mipid02_pad_ops = {
- .init_cfg = mipid02_init_cfg,
.enum_mbus_code = mipid02_enum_mbus_code,
.get_fmt = v4l2_subdev_get_fmt,
.set_fmt = mipid02_set_fmt,
@@ -653,6 +652,10 @@ static const struct v4l2_subdev_ops mipid02_subdev_ops = {
.pad = &mipid02_pad_ops,
};
+static const struct v4l2_subdev_internal_ops mipid02_subdev_internal_ops = {
+ .init_state = mipid02_init_state,
+};
+
static const struct media_entity_operations mipid02_subdev_entity_ops = {
.link_validate = v4l2_subdev_link_validate,
};
@@ -851,6 +854,7 @@ static int mipid02_probe(struct i2c_client *client)
bridge->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
bridge->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+ bridge->sd.internal_ops = &mipid02_subdev_internal_ops;
bridge->sd.entity.ops = &mipid02_subdev_entity_ops;
bridge->pad[0].flags = MEDIA_PAD_FL_SINK;
bridge->pad[1].flags = MEDIA_PAD_FL_SINK;
--
Sakari Ailus
Hi Sakari,
On Tue, Nov 28, 2023 at 06:40:02AM +0000, Sakari Ailus wrote:
> On Sat, Nov 25, 2023 at 07:20:52PM +0100, Alain Volmat wrote:
> > Store formats information within pad allowing to simplify further more
> > the driver (mutex / format store within the driver structure no more
> > necessary).
> >
> > Signed-off-by: Alain Volmat <[email protected]>
>
> Applied with the following diff:
>
> diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
> index 7af209905d7b..2cc07b3ed0da 100644
> --- a/drivers/media/i2c/st-mipid02.c
> +++ b/drivers/media/i2c/st-mipid02.c
> @@ -563,8 +563,8 @@ static const struct v4l2_mbus_framefmt default_fmt = {
> .height = 480,
> };
>
> -static int mipid02_init_cfg(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *state)
> +static int mipid02_init_state(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state)
> {
> *v4l2_subdev_state_get_format(state, MIPID02_SINK_0) = default_fmt;
> /* MIPID02_SINK_1 isn't supported yet */
> @@ -642,7 +642,6 @@ static const struct v4l2_subdev_video_ops mipid02_video_ops = {
> };
>
> static const struct v4l2_subdev_pad_ops mipid02_pad_ops = {
> - .init_cfg = mipid02_init_cfg,
> .enum_mbus_code = mipid02_enum_mbus_code,
> .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = mipid02_set_fmt,
> @@ -653,6 +652,10 @@ static const struct v4l2_subdev_ops mipid02_subdev_ops = {
> .pad = &mipid02_pad_ops,
> };
>
> +static const struct v4l2_subdev_internal_ops mipid02_subdev_internal_ops = {
> + .init_state = mipid02_init_state,
> +};
> +
> static const struct media_entity_operations mipid02_subdev_entity_ops = {
> .link_validate = v4l2_subdev_link_validate,
> };
> @@ -851,6 +854,7 @@ static int mipid02_probe(struct i2c_client *client)
>
> bridge->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> bridge->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> + bridge->sd.internal_ops = &mipid02_subdev_internal_ops;
> bridge->sd.entity.ops = &mipid02_subdev_entity_ops;
> bridge->pad[0].flags = MEDIA_PAD_FL_SINK;
> bridge->pad[1].flags = MEDIA_PAD_FL_SINK;
>
Thanks a lot Sakari for taking care of that !!
Regards,
Alain
> --
> Sakari Ailus