diff mbox series

[12/19] media: max9286: Implement .get_frame_desc()

Message ID 20240430103956.60190-13-jacopo.mondi@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series [01/19] media: adv748x: Add support for active state | expand

Commit Message

Jacopo Mondi April 30, 2024, 10:39 a.m. UTC 
Implement the .get_frame_desc() pad operation to allow the receiver
to retrieve information on the multiplexed source pad.

Record in the max9286_format_info structure the MIPI CSI-2
data type and use it to populate the frame_desc_entry.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 drivers/media/i2c/max9286.c | 120 ++++++++++++++++++++++++++++--------
 1 file changed, 95 insertions(+), 25 deletions(-)

Comments

Laurent Pinchart May 2, 2024, 6:32 p.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Tue, Apr 30, 2024 at 12:39:48PM +0200, Jacopo Mondi wrote:
> Implement the .get_frame_desc() pad operation to allow the receiver
> to retrieve information on the multiplexed source pad.
> 
> Record in the max9286_format_info structure the MIPI CSI-2
> data type and use it to populate the frame_desc_entry.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  drivers/media/i2c/max9286.c | 120 ++++++++++++++++++++++++++++--------
>  1 file changed, 95 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> index f203e4527257..4b4f4c03c10a 100644
> --- a/drivers/media/i2c/max9286.c
> +++ b/drivers/media/i2c/max9286.c
> @@ -23,6 +23,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
>  
> +#include <media/mipi-csi2.h>
>  #include <media/v4l2-async.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
> @@ -145,7 +146,12 @@
>  
>  struct max9286_format_info {
>  	u32 code;
> -	u8 datatype;
> +	/* The gmsl data format configuration. */
> +	u8 gmsl_dt;
> +	/* The format bpp, used for stride calculation. */
> +	u8 bpp;
> +	/* The Data Type identifier as defined by the MIPI CSI-2 standard. */
> +	u8 mipi_dt;
>  };
>  
>  struct max9286_i2c_speed {
> @@ -235,28 +241,44 @@ static inline struct max9286_priv *sd_to_max9286(struct v4l2_subdev *sd)
>  static const struct max9286_format_info max9286_formats[] = {
>  	{
>  		.code = MEDIA_BUS_FMT_UYVY8_1X16,
> -		.datatype = MAX9286_DATATYPE_YUV422_8BIT,
> +		.gmsl_dt = MAX9286_DATATYPE_YUV422_8BIT,
> +		.bpp = 16,
> +		.mipi_dt = MIPI_CSI2_DT_YUV422_8B,
>  	}, {
>  		.code = MEDIA_BUS_FMT_VYUY8_1X16,
> -		.datatype = MAX9286_DATATYPE_YUV422_8BIT,
> +		.gmsl_dt = MAX9286_DATATYPE_YUV422_8BIT,
> +		.bpp = 16,
> +		.mipi_dt = MIPI_CSI2_DT_YUV422_8B,
>  	}, {
>  		.code = MEDIA_BUS_FMT_YUYV8_1X16,
> -		.datatype = MAX9286_DATATYPE_YUV422_8BIT,
> +		.gmsl_dt = MAX9286_DATATYPE_YUV422_8BIT,
> +		.bpp = 16,
> +		.mipi_dt = MIPI_CSI2_DT_YUV422_8B,
>  	}, {
>  		.code = MEDIA_BUS_FMT_YVYU8_1X16,
> -		.datatype = MAX9286_DATATYPE_YUV422_8BIT,
> +		.gmsl_dt = MAX9286_DATATYPE_YUV422_8BIT,
> +		.bpp = 16,
> +		.mipi_dt = MIPI_CSI2_DT_YUV422_8B,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SBGGR12_1X12,
> -		.datatype = MAX9286_DATATYPE_RAW12,
> +		.gmsl_dt = MAX9286_DATATYPE_RAW12,
> +		.bpp = 12,
> +		.mipi_dt = MIPI_CSI2_DT_RAW12,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SGBRG12_1X12,
> -		.datatype = MAX9286_DATATYPE_RAW12,
> +		.gmsl_dt = MAX9286_DATATYPE_RAW12,
> +		.bpp = 12,
> +		.mipi_dt = MIPI_CSI2_DT_RAW12,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SGRBG12_1X12,
> -		.datatype = MAX9286_DATATYPE_RAW12,
> +		.gmsl_dt = MAX9286_DATATYPE_RAW12,
> +		.bpp = 12,
> +		.mipi_dt = MIPI_CSI2_DT_RAW12,
>  	}, {
>  		.code = MEDIA_BUS_FMT_SRGGB12_1X12,
> -		.datatype = MAX9286_DATATYPE_RAW12,
> +		.gmsl_dt = MAX9286_DATATYPE_RAW12,
> +		.bpp = 12,
> +		.mipi_dt = MIPI_CSI2_DT_RAW12,
>  	},
>  };
>  
> @@ -532,19 +554,23 @@ static int max9286_check_config_link(struct max9286_priv *priv,
>  	return 0;
>  }
>  
> +static const struct max9286_format_info *
> +max9286_get_format_info(unsigned int code)
> +{
> +	for (unsigned int i = 0; i < ARRAY_SIZE(max9286_formats); ++i) {
> +		if (max9286_formats[i].code == code)
> +			return &max9286_formats[i];
> +	}
> +
> +	return NULL;
> +}
> +
>  static void max9286_set_video_format(struct max9286_priv *priv,
>  				     const struct v4l2_mbus_framefmt *format)
>  {
>  	const struct max9286_format_info *info = NULL;
> -	unsigned int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(max9286_formats); ++i) {
> -		if (max9286_formats[i].code == format->code) {
> -			info = &max9286_formats[i];
> -			break;
> -		}
> -	}
>  
> +	info = max9286_get_format_info(format->code);
>  	if (WARN_ON(!info))
>  		return;
>  
> @@ -559,7 +585,7 @@ static void max9286_set_video_format(struct max9286_priv *priv,
>  	/* Enable CSI-2 Lane D0-D3 only, DBL mode. */
>  	max9286_write(priv, 0x12, MAX9286_CSIDBL | MAX9286_DBL |
>  		      MAX9286_CSILANECNT(priv->csi2_data_lanes) |
> -		      info->datatype);
> +		      info->gmsl_dt);
>  
>  	/*
>  	 * Enable HS/VS encoding, use HS as line valid source, use D14/15 for
> @@ -900,7 +926,7 @@ static int max9286_set_fmt(struct v4l2_subdev *sd,
>  			   struct v4l2_subdev_state *state,
>  			   struct v4l2_subdev_format *format)
>  {
> -	unsigned int i;
> +	const struct max9286_format_info *info;
>  
>  	/*
>  	 * Disable setting format on the source pad: format is propagated
> @@ -910,12 +936,8 @@ static int max9286_set_fmt(struct v4l2_subdev *sd,
>  		return -EINVAL;
>  
>  	/* Validate the format. */
> -	for (i = 0; i < ARRAY_SIZE(max9286_formats); ++i) {
> -		if (max9286_formats[i].code == format->format.code)
> -			break;
> -	}
> -
> -	if (i == ARRAY_SIZE(max9286_formats))
> +	info = max9286_get_format_info(format->format.code);
> +	if (!info)
>  		format->format.code = max9286_formats[0].code;
>  
>  	*v4l2_subdev_state_get_format(state, format->pad, 0) = format->format;
> @@ -930,6 +952,53 @@ static int max9286_set_fmt(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int max9286_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +				  struct v4l2_mbus_frame_desc *fd)
> +{
> +	struct v4l2_subdev_route *route;
> +	struct v4l2_subdev_state *state;
> +	unsigned int num_routes = 0;
> +	int ret = 0;
> +
> +	if (pad != MAX9286_SRC_PAD)
> +		return -EINVAL;
> +
> +	state = v4l2_subdev_lock_and_get_active_state(sd);

Add a blank line.

> +	for_each_active_route(&state->routing, route) {
> +		struct v4l2_mbus_frame_desc_entry *entry;
> +		const struct max9286_format_info *info;
> +		struct v4l2_mbus_framefmt *fmt;

const

> +
> +		fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
> +						   route->sink_stream);
> +		info = max9286_get_format_info(fmt->code);
> +		if (WARN_ON(!info)) {

I don't think this can happen. You can drop the check and the err_unlock
label.

> +			ret = -EINVAL;
> +			goto err_unlock;
> +		}
> +
> +		entry = &fd->entry[num_routes];
> +		entry->stream = num_routes;
> +		entry->flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
> +		entry->length = fmt->width * fmt->height * info->bpp / 8;

As explained in the review of 07/19, you can drop flags and length (and
thus the bpp field from max9286_format_info).

> +		entry->pixelcode = fmt->code;
> +
> +		/* VC is set according to link ordering, see register 0x15. */
> +		entry->bus.csi2.vc = route->sink_pad;
> +		entry->bus.csi2.dt = info->mipi_dt;
> +
> +		num_routes++;
> +	}
> +
> +	fd->num_entries = num_routes;
> +	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> +
> +err_unlock:
> +	v4l2_subdev_unlock_state(state);
> +
> +	return ret;
> +}
> +
>  static const struct v4l2_subdev_video_ops max9286_video_ops = {
>  	.s_stream	= max9286_s_stream,
>  };
> @@ -940,6 +1009,7 @@ static const struct v4l2_subdev_pad_ops max9286_pad_ops = {
>  	.set_fmt	= max9286_set_fmt,
>  	.get_frame_interval = v4l2_subdev_get_frame_interval,
>  	.set_frame_interval = max9286_set_frame_interval,
> +	.get_frame_desc	= max9286_get_frame_desc,
>  };
>  
>  static const struct v4l2_subdev_ops max9286_subdev_ops = {
diff mbox series

Patch

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index f203e4527257..4b4f4c03c10a 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -23,6 +23,7 @@ 
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
+#include <media/mipi-csi2.h>
 #include <media/v4l2-async.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
@@ -145,7 +146,12 @@ 
 
 struct max9286_format_info {
 	u32 code;
-	u8 datatype;
+	/* The gmsl data format configuration. */
+	u8 gmsl_dt;
+	/* The format bpp, used for stride calculation. */
+	u8 bpp;
+	/* The Data Type identifier as defined by the MIPI CSI-2 standard. */
+	u8 mipi_dt;
 };
 
 struct max9286_i2c_speed {
@@ -235,28 +241,44 @@  static inline struct max9286_priv *sd_to_max9286(struct v4l2_subdev *sd)
 static const struct max9286_format_info max9286_formats[] = {
 	{
 		.code = MEDIA_BUS_FMT_UYVY8_1X16,
-		.datatype = MAX9286_DATATYPE_YUV422_8BIT,
+		.gmsl_dt = MAX9286_DATATYPE_YUV422_8BIT,
+		.bpp = 16,
+		.mipi_dt = MIPI_CSI2_DT_YUV422_8B,
 	}, {
 		.code = MEDIA_BUS_FMT_VYUY8_1X16,
-		.datatype = MAX9286_DATATYPE_YUV422_8BIT,
+		.gmsl_dt = MAX9286_DATATYPE_YUV422_8BIT,
+		.bpp = 16,
+		.mipi_dt = MIPI_CSI2_DT_YUV422_8B,
 	}, {
 		.code = MEDIA_BUS_FMT_YUYV8_1X16,
-		.datatype = MAX9286_DATATYPE_YUV422_8BIT,
+		.gmsl_dt = MAX9286_DATATYPE_YUV422_8BIT,
+		.bpp = 16,
+		.mipi_dt = MIPI_CSI2_DT_YUV422_8B,
 	}, {
 		.code = MEDIA_BUS_FMT_YVYU8_1X16,
-		.datatype = MAX9286_DATATYPE_YUV422_8BIT,
+		.gmsl_dt = MAX9286_DATATYPE_YUV422_8BIT,
+		.bpp = 16,
+		.mipi_dt = MIPI_CSI2_DT_YUV422_8B,
 	}, {
 		.code = MEDIA_BUS_FMT_SBGGR12_1X12,
-		.datatype = MAX9286_DATATYPE_RAW12,
+		.gmsl_dt = MAX9286_DATATYPE_RAW12,
+		.bpp = 12,
+		.mipi_dt = MIPI_CSI2_DT_RAW12,
 	}, {
 		.code = MEDIA_BUS_FMT_SGBRG12_1X12,
-		.datatype = MAX9286_DATATYPE_RAW12,
+		.gmsl_dt = MAX9286_DATATYPE_RAW12,
+		.bpp = 12,
+		.mipi_dt = MIPI_CSI2_DT_RAW12,
 	}, {
 		.code = MEDIA_BUS_FMT_SGRBG12_1X12,
-		.datatype = MAX9286_DATATYPE_RAW12,
+		.gmsl_dt = MAX9286_DATATYPE_RAW12,
+		.bpp = 12,
+		.mipi_dt = MIPI_CSI2_DT_RAW12,
 	}, {
 		.code = MEDIA_BUS_FMT_SRGGB12_1X12,
-		.datatype = MAX9286_DATATYPE_RAW12,
+		.gmsl_dt = MAX9286_DATATYPE_RAW12,
+		.bpp = 12,
+		.mipi_dt = MIPI_CSI2_DT_RAW12,
 	},
 };
 
@@ -532,19 +554,23 @@  static int max9286_check_config_link(struct max9286_priv *priv,
 	return 0;
 }
 
+static const struct max9286_format_info *
+max9286_get_format_info(unsigned int code)
+{
+	for (unsigned int i = 0; i < ARRAY_SIZE(max9286_formats); ++i) {
+		if (max9286_formats[i].code == code)
+			return &max9286_formats[i];
+	}
+
+	return NULL;
+}
+
 static void max9286_set_video_format(struct max9286_priv *priv,
 				     const struct v4l2_mbus_framefmt *format)
 {
 	const struct max9286_format_info *info = NULL;
-	unsigned int i;
-
-	for (i = 0; i < ARRAY_SIZE(max9286_formats); ++i) {
-		if (max9286_formats[i].code == format->code) {
-			info = &max9286_formats[i];
-			break;
-		}
-	}
 
+	info = max9286_get_format_info(format->code);
 	if (WARN_ON(!info))
 		return;
 
@@ -559,7 +585,7 @@  static void max9286_set_video_format(struct max9286_priv *priv,
 	/* Enable CSI-2 Lane D0-D3 only, DBL mode. */
 	max9286_write(priv, 0x12, MAX9286_CSIDBL | MAX9286_DBL |
 		      MAX9286_CSILANECNT(priv->csi2_data_lanes) |
-		      info->datatype);
+		      info->gmsl_dt);
 
 	/*
 	 * Enable HS/VS encoding, use HS as line valid source, use D14/15 for
@@ -900,7 +926,7 @@  static int max9286_set_fmt(struct v4l2_subdev *sd,
 			   struct v4l2_subdev_state *state,
 			   struct v4l2_subdev_format *format)
 {
-	unsigned int i;
+	const struct max9286_format_info *info;
 
 	/*
 	 * Disable setting format on the source pad: format is propagated
@@ -910,12 +936,8 @@  static int max9286_set_fmt(struct v4l2_subdev *sd,
 		return -EINVAL;
 
 	/* Validate the format. */
-	for (i = 0; i < ARRAY_SIZE(max9286_formats); ++i) {
-		if (max9286_formats[i].code == format->format.code)
-			break;
-	}
-
-	if (i == ARRAY_SIZE(max9286_formats))
+	info = max9286_get_format_info(format->format.code);
+	if (!info)
 		format->format.code = max9286_formats[0].code;
 
 	*v4l2_subdev_state_get_format(state, format->pad, 0) = format->format;
@@ -930,6 +952,53 @@  static int max9286_set_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int max9286_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+				  struct v4l2_mbus_frame_desc *fd)
+{
+	struct v4l2_subdev_route *route;
+	struct v4l2_subdev_state *state;
+	unsigned int num_routes = 0;
+	int ret = 0;
+
+	if (pad != MAX9286_SRC_PAD)
+		return -EINVAL;
+
+	state = v4l2_subdev_lock_and_get_active_state(sd);
+	for_each_active_route(&state->routing, route) {
+		struct v4l2_mbus_frame_desc_entry *entry;
+		const struct max9286_format_info *info;
+		struct v4l2_mbus_framefmt *fmt;
+
+		fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
+						   route->sink_stream);
+		info = max9286_get_format_info(fmt->code);
+		if (WARN_ON(!info)) {
+			ret = -EINVAL;
+			goto err_unlock;
+		}
+
+		entry = &fd->entry[num_routes];
+		entry->stream = num_routes;
+		entry->flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
+		entry->length = fmt->width * fmt->height * info->bpp / 8;
+		entry->pixelcode = fmt->code;
+
+		/* VC is set according to link ordering, see register 0x15. */
+		entry->bus.csi2.vc = route->sink_pad;
+		entry->bus.csi2.dt = info->mipi_dt;
+
+		num_routes++;
+	}
+
+	fd->num_entries = num_routes;
+	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+
+err_unlock:
+	v4l2_subdev_unlock_state(state);
+
+	return ret;
+}
+
 static const struct v4l2_subdev_video_ops max9286_video_ops = {
 	.s_stream	= max9286_s_stream,
 };
@@ -940,6 +1009,7 @@  static const struct v4l2_subdev_pad_ops max9286_pad_ops = {
 	.set_fmt	= max9286_set_fmt,
 	.get_frame_interval = v4l2_subdev_get_frame_interval,
 	.set_frame_interval = max9286_set_frame_interval,
+	.get_frame_desc	= max9286_get_frame_desc,
 };
 
 static const struct v4l2_subdev_ops max9286_subdev_ops = {