Re: [PATCH v8 35/38] media: ov2740: Add support for embedded data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sakari,

On 3/13/24 08:25, Sakari Ailus wrote:
Add support for embedded data. This introduces two internal pads for pixel
and embedded data streams. As the driver supports a single mode only,
there's no need for backward compatibility in mode selection.

The embedded data is configured to be placed before the image data whereas
after the image data is the default.

Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
---
  drivers/media/i2c/ov2740.c | 150 +++++++++++++++++++++++++++++++++----
  1 file changed, 137 insertions(+), 13 deletions(-)

diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index df57f0096e98..7488b2535071 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -11,6 +11,7 @@
  #include <linux/pm_runtime.h>
  #include <linux/nvmem-provider.h>
  #include <linux/regmap.h>
+#include <media/mipi-csi2.h>
  #include <media/v4l2-ctrls.h>
  #include <media/v4l2-device.h>
  #include <media/v4l2-fwnode.h>
@@ -71,11 +72,31 @@
  #define OV2740_REG_ISP_CTRL00		0x5000
  /* ISP CTRL01 */
  #define OV2740_REG_ISP_CTRL01		0x5001
+
+/* Embedded data line location control */
+#define OV2740_REG_EMBEDDED_FLAG	0x5a08
+#define OV2740_EMBEDDED_FLAG_FOOTER	BIT(2) /* otherwise it's in header */
+#define OV2740_EMBEDDED_FLAG_MYSTERY	BIT(1)
  /* Customer Addresses: 0x7010 - 0x710F */
  #define CUSTOMER_USE_OTP_SIZE		0x100
  /* OTP registers from sensor */
  #define OV2740_REG_OTP_CUSTOMER		0x7010
+enum {
+	OV2740_PAD_SOURCE,
+	OV2740_PAD_PIXEL,
+	OV2740_PAD_META,
+	OV2740_NUM_PADS,
+};
+
+enum {
+	OV2740_STREAM_PIXEL,
+	OV2740_STREAM_META,
+};
+
+#define OV2740_META_WIDTH		100U /* 97 bytes of actual data */
+#define OV2740_META_HEIGHT		1U
+
  struct nvm_data {
  	struct nvmem_device *nvmem;
  	struct regmap *regmap;
@@ -513,7 +534,7 @@ static const struct ov2740_mode supported_modes_180mhz[] = {
struct ov2740 {
  	struct v4l2_subdev sd;
-	struct media_pad pad;
+	struct media_pad pads[OV2740_NUM_PADS];
  	struct v4l2_ctrl_handler ctrl_handler;
/* V4L2 Controls */
@@ -976,6 +997,11 @@ static int ov2740_enable_streams(struct v4l2_subdev *sd,
  	if (ret)
  		goto out_pm_put;
+ ret = ov2740_write_reg(ov2740, OV2740_REG_EMBEDDED_FLAG, 1,
+			       OV2740_EMBEDDED_FLAG_MYSTERY);
+	if (ret)
+		return ret;
+
  	ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
  			       OV2740_MODE_STREAMING);
  	if (ret) {
@@ -1013,23 +1039,49 @@ static int ov2740_disable_streams(struct v4l2_subdev *sd,
  	return ret;
  }
-static int ov2740_set_format(struct v4l2_subdev *sd,
-			     struct v4l2_subdev_state *sd_state,
-			     struct v4l2_subdev_format *fmt)
+static int __ov2740_set_format(struct v4l2_subdev *sd,
+			       struct v4l2_subdev_state *sd_state,
+			       struct v4l2_mbus_framefmt *format,
+			       enum v4l2_subdev_format_whence which,
+			       unsigned int pad, unsigned int stream)
  {
+	struct v4l2_mbus_framefmt *src_pix_fmt, *src_meta_fmt, *pix_fmt,
+		*meta_fmt;
  	struct ov2740 *ov2740 = to_ov2740(sd);
  	const struct ov2740_mode *mode;
  	s32 vblank_def, h_blank;
+ /*
+	 * Allow setting format on internal pixel pad as well as the source
+	 * pad's pixel stream (for compatibility).
+	 */
+	if (pad == OV2740_PAD_SOURCE || pad == OV2740_PAD_META ||
+	    stream == OV2740_STREAM_META) {
This is equivalent to
if (pad != OV2740_PAD_PIXEL)
Correct me if I'm wrong but this code doesn't allow to set the format on the source pad.

Should it be:
if ((pad == OV2740_PAD_SOURCE && stream == OV2740_STREAM_META) ||
	pad == OV2740_PAD_META) {


+		*format = *v4l2_subdev_state_get_format(sd_state, pad, stream);
+		return 0;
+	}
+
+	pix_fmt = v4l2_subdev_state_get_format(sd_state, OV2740_PAD_PIXEL, 0);
+	meta_fmt = v4l2_subdev_state_get_format(sd_state, OV2740_PAD_META, 0);
+	src_pix_fmt = v4l2_subdev_state_get_format(sd_state, OV2740_PAD_SOURCE,
+						   OV2740_STREAM_PIXEL);
+	src_meta_fmt = v4l2_subdev_state_get_format(sd_state, OV2740_PAD_SOURCE,
+						    OV2740_STREAM_META);
+
  	mode = v4l2_find_nearest_size(ov2740->supported_modes,
  				      ov2740->supported_modes_count,
  				      width, height,
-				      fmt->format.width, fmt->format.height);
+				      format->width, format->height);
+	ov2740_update_pad_format(mode, pix_fmt);
+	*format = *src_pix_fmt = *pix_fmt;
- ov2740_update_pad_format(mode, &fmt->format);
-	*v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
+	meta_fmt->code = MEDIA_BUS_FMT_OV2740_EMBEDDED;
+	meta_fmt->width = OV2740_META_WIDTH;
+	meta_fmt->height = OV2740_META_HEIGHT;
+	*src_meta_fmt = *meta_fmt;
+	src_meta_fmt->code = MEDIA_BUS_FMT_META_10;
- if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
+	if (which == V4L2_SUBDEV_FORMAT_TRY)
  		return 0;
ov2740->cur_mode = mode;
Regards,
--
Julien Massot
Senior Software Engineer
Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux