media: ov5640: set correct default format for CSI-2 mode

Message ID 20221212040526.3549644-1-guoniu.zhou@oss.nxp.com (mailing list archive)
State Accepted
Delegated to: Sakari Ailus
Headers
Series media: ov5640: set correct default format for CSI-2 mode |

Commit Message

G.N. Zhou (OSS) Dec. 12, 2022, 4:05 a.m. UTC
  From: "Guoniu.zhou" <guoniu.zhou@nxp.com>

In commit a89f14bbcfa5 ("media: ov5640: Split DVP and CSI-2 formats"),
it splits format list for DVP and CSI-2 mode, but the default format
defined in commit 90b0f355c5a3 ("media: ov5640: Implement init_cfg")
is only supported by DVP mode, so define a new default format for
CSI-2 mode.

Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
---
 drivers/media/i2c/ov5640.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
  

Comments

Jai Luthra Dec. 28, 2022, 8:05 a.m. UTC | #1
Hi,

Thanks for the patch.

On 12/12/22 09:35, G.N. Zhou (OSS) wrote:
> From: "Guoniu.zhou" <guoniu.zhou@nxp.com>
> 
> In commit a89f14bbcfa5 ("media: ov5640: Split DVP and CSI-2 formats"),
> it splits format list for DVP and CSI-2 mode, but the default format
> defined in commit 90b0f355c5a3 ("media: ov5640: Implement init_cfg")
> is only supported by DVP mode, so define a new default format for
> CSI-2 mode.
> 
> Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>

Reviewed-by: Jai Luthra <j-luthra@ti.com>

> ---
>   drivers/media/i2c/ov5640.c | 21 ++++++++++++++++++---
>   1 file changed, 18 insertions(+), 3 deletions(-)
> ...snip...
  
Jacopo Mondi Jan. 2, 2023, 8:52 a.m. UTC | #2
Hello,
   sorry for the delay

On Mon, Dec 12, 2022 at 12:05:26PM +0800, G.N. Zhou (OSS) wrote:
> From: "Guoniu.zhou" <guoniu.zhou@nxp.com>
>
> In commit a89f14bbcfa5 ("media: ov5640: Split DVP and CSI-2 formats"),
> it splits format list for DVP and CSI-2 mode, but the default format
> defined in commit 90b0f355c5a3 ("media: ov5640: Implement init_cfg")
> is only supported by DVP mode, so define a new default format for
> CSI-2 mode.
>
> Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>

Thanks for fixing, this seems correct to me
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
   j

> ---
>  drivers/media/i2c/ov5640.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index e0f908af581b..2c37ed7b75d3 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -520,7 +520,18 @@ static u32 ov5640_code_to_bpp(struct ov5640_dev *sensor, u32 code)
>   */
>  /* YUV422 UYVY VGA@30fps */
>
> -static const struct v4l2_mbus_framefmt ov5640_default_fmt = {
> +static const struct v4l2_mbus_framefmt ov5640_csi2_default_fmt = {
> +	.code = MEDIA_BUS_FMT_UYVY8_1X16,
> +	.width = 640,
> +	.height = 480,
> +	.colorspace = V4L2_COLORSPACE_SRGB,
> +	.ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SRGB),
> +	.quantization = V4L2_QUANTIZATION_FULL_RANGE,
> +	.xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB),
> +	.field = V4L2_FIELD_NONE,
> +};
> +
> +static const struct v4l2_mbus_framefmt ov5640_dvp_default_fmt = {
>  	.code = MEDIA_BUS_FMT_UYVY8_2X8,
>  	.width = 640,
>  	.height = 480,
> @@ -3719,11 +3730,13 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int enable)
>  static int ov5640_init_cfg(struct v4l2_subdev *sd,
>  			   struct v4l2_subdev_state *state)
>  {
> +	struct ov5640_dev *sensor = to_ov5640_dev(sd);
>  	struct v4l2_mbus_framefmt *fmt =
>  				v4l2_subdev_get_try_format(sd, state, 0);
>  	struct v4l2_rect *crop = v4l2_subdev_get_try_crop(sd, state, 0);
>
> -	*fmt = ov5640_default_fmt;
> +	*fmt = ov5640_is_csi2(sensor) ? ov5640_csi2_default_fmt :
> +					ov5640_dvp_default_fmt;
>
>  	crop->left = OV5640_PIXEL_ARRAY_LEFT;
>  	crop->top = OV5640_PIXEL_ARRAY_TOP;
> @@ -3812,7 +3825,6 @@ static int ov5640_probe(struct i2c_client *client)
>  	 * default init sequence initialize sensor to
>  	 * YUV422 UYVY VGA@30fps
>  	 */
> -	sensor->fmt = ov5640_default_fmt;
>  	sensor->frame_interval.numerator = 1;
>  	sensor->frame_interval.denominator = ov5640_framerates[OV5640_30_FPS];
>  	sensor->current_fr = OV5640_30_FPS;
> @@ -3845,6 +3857,9 @@ static int ov5640_probe(struct i2c_client *client)
>  		return -EINVAL;
>  	}
>
> +	sensor->fmt = ov5640_is_csi2(sensor) ? ov5640_csi2_default_fmt :
> +					       ov5640_dvp_default_fmt;
> +
>  	/* get system clock (xclk) */
>  	sensor->xclk = devm_clk_get(dev, "xclk");
>  	if (IS_ERR(sensor->xclk)) {
> --
> 2.37.1
>
  

Patch

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index e0f908af581b..2c37ed7b75d3 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -520,7 +520,18 @@  static u32 ov5640_code_to_bpp(struct ov5640_dev *sensor, u32 code)
  */
 /* YUV422 UYVY VGA@30fps */
 
-static const struct v4l2_mbus_framefmt ov5640_default_fmt = {
+static const struct v4l2_mbus_framefmt ov5640_csi2_default_fmt = {
+	.code = MEDIA_BUS_FMT_UYVY8_1X16,
+	.width = 640,
+	.height = 480,
+	.colorspace = V4L2_COLORSPACE_SRGB,
+	.ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SRGB),
+	.quantization = V4L2_QUANTIZATION_FULL_RANGE,
+	.xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB),
+	.field = V4L2_FIELD_NONE,
+};
+
+static const struct v4l2_mbus_framefmt ov5640_dvp_default_fmt = {
 	.code = MEDIA_BUS_FMT_UYVY8_2X8,
 	.width = 640,
 	.height = 480,
@@ -3719,11 +3730,13 @@  static int ov5640_s_stream(struct v4l2_subdev *sd, int enable)
 static int ov5640_init_cfg(struct v4l2_subdev *sd,
 			   struct v4l2_subdev_state *state)
 {
+	struct ov5640_dev *sensor = to_ov5640_dev(sd);
 	struct v4l2_mbus_framefmt *fmt =
 				v4l2_subdev_get_try_format(sd, state, 0);
 	struct v4l2_rect *crop = v4l2_subdev_get_try_crop(sd, state, 0);
 
-	*fmt = ov5640_default_fmt;
+	*fmt = ov5640_is_csi2(sensor) ? ov5640_csi2_default_fmt :
+					ov5640_dvp_default_fmt;
 
 	crop->left = OV5640_PIXEL_ARRAY_LEFT;
 	crop->top = OV5640_PIXEL_ARRAY_TOP;
@@ -3812,7 +3825,6 @@  static int ov5640_probe(struct i2c_client *client)
 	 * default init sequence initialize sensor to
 	 * YUV422 UYVY VGA@30fps
 	 */
-	sensor->fmt = ov5640_default_fmt;
 	sensor->frame_interval.numerator = 1;
 	sensor->frame_interval.denominator = ov5640_framerates[OV5640_30_FPS];
 	sensor->current_fr = OV5640_30_FPS;
@@ -3845,6 +3857,9 @@  static int ov5640_probe(struct i2c_client *client)
 		return -EINVAL;
 	}
 
+	sensor->fmt = ov5640_is_csi2(sensor) ? ov5640_csi2_default_fmt :
+					       ov5640_dvp_default_fmt;
+
 	/* get system clock (xclk) */
 	sensor->xclk = devm_clk_get(dev, "xclk");
 	if (IS_ERR(sensor->xclk)) {