[PATCH 0/8] media: imx: Miscalleanous format-related cleanups

[PATCH 0/8] media: imx: Miscalleanous format-related cleanups

[Laurent Pinchart]

Hello,

This patch series started as an attempt to fix the format get and set
subdev operations on the i.MX7 CSI-2 receiver subdev, which it does in
patch 1/8. Patch 2/8 further cleans up the format-related code in that
subdev.

Patches 3/8 to 8/8 pushes the cleanups further as I was attempting to
fix the format enumeration on the video node at the end of the pipeline.
I realized as part of that effort that there's more work than I
anticipated, and I'm currently evaluating the possible options.
Nonetheless, I think the cleanups make sense even without what I wanted
to build on top of them, so I'm sending them out already.

Laurent Pinchart (8):
  media: imx: imx7-mipi-csis: Cleanup and fix subdev pad format handling
  media: imx: imx7-mipi-csis: Centralize initialization of pad formats
  media: imx: utils: Inline init_mbus_colorimetry() in its caller
  media: imx: utils: Handle Bayer format lookup through a selection flag
  media: imx: utils: Simplify IPU format lookup and enumeration
  media: imx: utils: Make imx_media_pixfmt handle variable number of
    codes
  media: imx: utils: Remove unneeded argument to (find|enum)_format()
  media: imx: utils: Rename format lookup and enumeration functions

 drivers/staging/media/imx/imx-ic-prp.c        |   8 +-
 drivers/staging/media/imx/imx-ic-prpencvf.c   |   6 +-
 drivers/staging/media/imx/imx-media-capture.c |  22 +-
 .../staging/media/imx/imx-media-csc-scaler.c  |   2 +-
 drivers/staging/media/imx/imx-media-csi.c     |  26 +-
 drivers/staging/media/imx/imx-media-utils.c   | 313 ++++++++----------
 drivers/staging/media/imx/imx-media-vdic.c    |   6 +-
 drivers/staging/media/imx/imx-media.h         |  24 +-
 drivers/staging/media/imx/imx7-media-csi.c    |  15 +-
 drivers/staging/media/imx/imx7-mipi-csis.c    | 138 ++++----
 10 files changed, 271 insertions(+), 289 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 1/8] media: imx: imx7-mipi-csis: Cleanup and fix subdev pad format handling

[Laurent Pinchart]

The subdev set pad format operation currently misbehaves in multiple ways:

- mipi_csis_try_format() unconditionally stores the format in the device
  state, even for V4L2_SUBDEV_FORMAT_TRY.

- The format is never stored in the pad cfg, but the pad cfg format
  always overwrites the format requested by the user.

- The sink format is not propagated to the source.

Fix all this by reworking the set format operation as follows:

1. For the source pad, turn set() into get() as the source format is not
   modifiable.
2. Validate the requested format and updated the stored format
   accordingly.
3. Return the format actually set.
4. Propagate the format from sink to source.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 82 ++++++++++------------
 1 file changed, 37 insertions(+), 45 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index de17a1d22873..25017ab78095 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -669,28 +669,6 @@ static int mipi_csis_init_cfg(struct v4l2_subdev *mipi_sd,
 	return 0;
 }
 
-static struct csis_pix_format const *
-mipi_csis_try_format(struct v4l2_subdev *mipi_sd, struct v4l2_mbus_framefmt *mf)
-{
-	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
-	struct csis_pix_format const *csis_fmt;
-
-	csis_fmt = find_csis_format(mf->code);
-	if (!csis_fmt)
-		csis_fmt = &mipi_csis_formats[0];
-
-	v4l_bound_align_image(&mf->width, 1, CSIS_MAX_PIX_WIDTH,
-			      csis_fmt->pix_width_alignment,
-			      &mf->height, 1, CSIS_MAX_PIX_HEIGHT, 1,
-			      0);
-
-	state->format_mbus.code = csis_fmt->code;
-	state->format_mbus.width = mf->width;
-	state->format_mbus.height = mf->height;
-
-	return csis_fmt;
-}
-
 static struct v4l2_mbus_framefmt *
 mipi_csis_get_format(struct csi_state *state,
 		     struct v4l2_subdev_pad_config *cfg,
@@ -703,53 +681,67 @@ mipi_csis_get_format(struct csi_state *state,
 	return &state->format_mbus;
 }
 
-static int mipi_csis_set_fmt(struct v4l2_subdev *mipi_sd,
+static int mipi_csis_get_fmt(struct v4l2_subdev *mipi_sd,
 			     struct v4l2_subdev_pad_config *cfg,
 			     struct v4l2_subdev_format *sdformat)
 {
 	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
-	struct csis_pix_format const *csis_fmt;
 	struct v4l2_mbus_framefmt *fmt;
 
-	if (sdformat->pad >= CSIS_PADS_NUM)
-		return -EINVAL;
-
-	fmt = mipi_csis_get_format(state, cfg, sdformat->which, sdformat->pad);
-
 	mutex_lock(&state->lock);
-	if (sdformat->pad == CSIS_PAD_SOURCE) {
-		sdformat->format = *fmt;
-		goto unlock;
-	}
-
-	csis_fmt = mipi_csis_try_format(mipi_sd, &sdformat->format);
-
+	fmt = mipi_csis_get_format(state, cfg, sdformat->which, sdformat->pad);
 	sdformat->format = *fmt;
-
-	if (csis_fmt && sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
-		state->csis_fmt = csis_fmt;
-	else
-		cfg->try_fmt = sdformat->format;
-
-unlock:
 	mutex_unlock(&state->lock);
 
 	return 0;
 }
 
-static int mipi_csis_get_fmt(struct v4l2_subdev *mipi_sd,
+static int mipi_csis_set_fmt(struct v4l2_subdev *mipi_sd,
 			     struct v4l2_subdev_pad_config *cfg,
 			     struct v4l2_subdev_format *sdformat)
 {
 	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct csis_pix_format const *csis_fmt;
 	struct v4l2_mbus_framefmt *fmt;
 
-	mutex_lock(&state->lock);
+	/*
+	 * The CSIS can't transcode in any way, the source format can't be
+	 * modified.
+	 */
+	if (sdformat->pad == CSIS_PAD_SOURCE)
+		return mipi_csis_get_fmt(mipi_sd, cfg, sdformat);
+
+	if (sdformat->pad != CSIS_PAD_SINK)
+		return -EINVAL;
 
 	fmt = mipi_csis_get_format(state, cfg, sdformat->which, sdformat->pad);
 
+	mutex_lock(&state->lock);
+
+	/* Validate the media bus code and clamp the size. */
+	csis_fmt = find_csis_format(sdformat->format.code);
+	if (!csis_fmt)
+		csis_fmt = &mipi_csis_formats[0];
+
+	fmt->code = csis_fmt->code;
+	fmt->width = sdformat->format.width;
+	fmt->height = sdformat->format.height;
+
+	v4l_bound_align_image(&fmt->width, 1, CSIS_MAX_PIX_WIDTH,
+			      csis_fmt->pix_width_alignment,
+			      &fmt->height, 1, CSIS_MAX_PIX_HEIGHT, 1, 0);
+
 	sdformat->format = *fmt;
 
+	/* Propagate the format from sink to source. */
+	fmt = mipi_csis_get_format(state, cfg, sdformat->which,
+				   CSIS_PAD_SOURCE);
+	*fmt = sdformat->format;
+
+	/* Store the CSIS format descriptor for active formats. */
+	if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+		state->csis_fmt = csis_fmt;
+
 	mutex_unlock(&state->lock);
 
 	return 0;
-- 
Regards,

Laurent Pinchart

[PATCH 2/8] media: imx: imx7-mipi-csis: Centralize initialization of pad formats

[Laurent Pinchart]

Pad formats for the active configuration are manually initialized in
mipi_csis_subdev_init(), while pad formats for the TRY configurations
are initialized by the subdev .init_cfg() operation. This creates a risk
of the two configurations not being synchronized. Fix it by initializing
formats in the .init_cfg() operation only, and calling it from
mipi_csis_subdev_init().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 56 ++++++++++++----------
 1 file changed, 32 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 25017ab78095..52d59047ee36 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -649,26 +649,6 @@ static int mipi_csis_link_setup(struct media_entity *entity,
 	return ret;
 }
 
-static int mipi_csis_init_cfg(struct v4l2_subdev *mipi_sd,
-			      struct v4l2_subdev_pad_config *cfg)
-{
-	struct v4l2_mbus_framefmt *mf;
-	unsigned int i;
-	int ret;
-
-	for (i = 0; i < CSIS_PADS_NUM; i++) {
-		mf = v4l2_subdev_get_try_format(mipi_sd, cfg, i);
-
-		ret = imx_media_init_mbus_fmt(mf, MIPI_CSIS_DEF_PIX_HEIGHT,
-					      MIPI_CSIS_DEF_PIX_WIDTH, 0,
-					      V4L2_FIELD_NONE, NULL);
-		if (ret < 0)
-			return ret;
-	}
-
-	return 0;
-}
-
 static struct v4l2_mbus_framefmt *
 mipi_csis_get_format(struct csi_state *state,
 		     struct v4l2_subdev_pad_config *cfg,
@@ -681,6 +661,37 @@ mipi_csis_get_format(struct csi_state *state,
 	return &state->format_mbus;
 }
 
+static int mipi_csis_init_cfg(struct v4l2_subdev *mipi_sd,
+			      struct v4l2_subdev_pad_config *cfg)
+{
+	struct csi_state *state = mipi_sd_to_csis_state(mipi_sd);
+	struct v4l2_mbus_framefmt *fmt_sink;
+	struct v4l2_mbus_framefmt *fmt_source;
+	enum v4l2_subdev_format_whence which;
+	int ret;
+
+	which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
+	fmt_sink = mipi_csis_get_format(state, cfg, which, CSIS_PAD_SINK);
+	ret = imx_media_init_mbus_fmt(fmt_sink, MIPI_CSIS_DEF_PIX_WIDTH,
+				      MIPI_CSIS_DEF_PIX_HEIGHT, 0,
+				      V4L2_FIELD_NONE, NULL);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * When called from mipi_csis_subdev_init() to initialize the active
+	 * configuration, cfg is NULL, which indicates there's no source pad
+	 * configuration to set.
+	 */
+	if (!cfg)
+		return 0;
+
+	fmt_source = mipi_csis_get_format(state, cfg, which, CSIS_PAD_SOURCE);
+	*fmt_source = *fmt_sink;
+
+	return 0;
+}
+
 static int mipi_csis_get_fmt(struct v4l2_subdev *mipi_sd,
 			     struct v4l2_subdev_pad_config *cfg,
 			     struct v4l2_subdev_format *sdformat)
@@ -875,10 +886,7 @@ static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
 	mipi_sd->dev = &pdev->dev;
 
 	state->csis_fmt = &mipi_csis_formats[0];
-	state->format_mbus.code = mipi_csis_formats[0].code;
-	state->format_mbus.width = MIPI_CSIS_DEF_PIX_WIDTH;
-	state->format_mbus.height = MIPI_CSIS_DEF_PIX_HEIGHT;
-	state->format_mbus.field = V4L2_FIELD_NONE;
+	mipi_csis_init_cfg(mipi_sd, NULL);
 
 	v4l2_set_subdevdata(mipi_sd, &pdev->dev);
 
-- 
Regards,

Laurent Pinchart

[PATCH 3/8] media: imx: utils: Inline init_mbus_colorimetry() in its caller

[Laurent Pinchart]

The init_mbus_colorimetry() function is small and used in a single
place. The code becomes easier to follow if it gets inline in its
caller. Do so.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/staging/media/imx/imx-media-utils.c | 24 +++++++++------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index fae981698c49..75c4097a9d16 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -233,19 +233,6 @@ static const struct imx_media_pixfmt ipu_rgb_formats[] = {
 
 #define NUM_IPU_RGB_FORMATS ARRAY_SIZE(ipu_rgb_formats)
 
-static void init_mbus_colorimetry(struct v4l2_mbus_framefmt *mbus,
-				  const struct imx_media_pixfmt *fmt)
-{
-	mbus->colorspace = (fmt->cs == IPUV3_COLORSPACE_RGB) ?
-		V4L2_COLORSPACE_SRGB : V4L2_COLORSPACE_SMPTE170M;
-	mbus->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(mbus->colorspace);
-	mbus->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(mbus->colorspace);
-	mbus->quantization =
-		V4L2_MAP_QUANTIZATION_DEFAULT(fmt->cs == IPUV3_COLORSPACE_RGB,
-					      mbus->colorspace,
-					      mbus->ycbcr_enc);
-}
-
 static const
 struct imx_media_pixfmt *__find_format(u32 fourcc,
 				       u32 code,
@@ -488,7 +475,16 @@ int imx_media_init_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
 	}
 
 	mbus->code = code;
-	init_mbus_colorimetry(mbus, lcc);
+
+	mbus->colorspace = (lcc->cs == IPUV3_COLORSPACE_RGB)
+			? V4L2_COLORSPACE_SRGB : V4L2_COLORSPACE_SMPTE170M;
+	mbus->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(mbus->colorspace);
+	mbus->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(mbus->colorspace);
+	mbus->quantization =
+		V4L2_MAP_QUANTIZATION_DEFAULT(lcc->cs == IPUV3_COLORSPACE_RGB,
+					      mbus->colorspace,
+					      mbus->ycbcr_enc);
+
 	if (cc)
 		*cc = lcc;
 
-- 
Regards,

Laurent Pinchart

[PATCH 4/8] media: imx: utils: Handle Bayer format lookup through a selection flag

[Laurent Pinchart]

The format lookup (and enumeration) functions take a boolean flag to
tell if Bayer formats should be considered. This leads to hard to read
lines such as

	return enum_format(fourcc, NULL, index, cs_sel, true, false);

where the boolean parameters can easily be mixed. To make the code
clearer, add a CS_SEL_BAYER flag that can be passed through the
codespace_sel parameter of the lookup functions to replace the bool
parameter.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/staging/media/imx/imx-media-capture.c | 16 ++---
 drivers/staging/media/imx/imx-media-csi.c     | 15 +++--
 drivers/staging/media/imx/imx-media-utils.c   | 63 ++++++++++---------
 drivers/staging/media/imx/imx-media.h         | 15 +++--
 drivers/staging/media/imx/imx7-media-csi.c    | 13 ++--
 5 files changed, 62 insertions(+), 60 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 7712e7be8625..6685b149db99 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -91,7 +91,8 @@ static int capture_enum_framesizes(struct file *file, void *fh,
 	};
 	int ret;
 
-	cc = imx_media_find_format(fsize->pixel_format, CS_SEL_ANY, true);
+	cc = imx_media_find_format(fsize->pixel_format,
+				   CS_SEL_ANY | CS_SEL_BAYER);
 	if (!cc)
 		return -EINVAL;
 
@@ -133,7 +134,8 @@ static int capture_enum_frameintervals(struct file *file, void *fh,
 	};
 	int ret;
 
-	cc = imx_media_find_format(fival->pixel_format, CS_SEL_ANY, true);
+	cc = imx_media_find_format(fival->pixel_format,
+				   CS_SEL_ANY | CS_SEL_BAYER);
 	if (!cc)
 		return -EINVAL;
 
@@ -177,7 +179,7 @@ static int capture_enum_fmt_vid_cap(struct file *file, void *fh,
 			return ret;
 	} else {
 		cc_src = imx_media_find_mbus_format(fmt_src.format.code,
-						    CS_SEL_ANY, true);
+						    CS_SEL_ANY | CS_SEL_BAYER);
 		if (WARN_ON(!cc_src))
 			return -EINVAL;
 
@@ -217,14 +219,14 @@ static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
 			CS_SEL_YUV : CS_SEL_RGB;
 		fourcc = f->fmt.pix.pixelformat;
 
-		cc = imx_media_find_format(fourcc, cs_sel, false);
+		cc = imx_media_find_format(fourcc, cs_sel);
 		if (!cc) {
 			imx_media_enum_format(&fourcc, 0, cs_sel);
-			cc = imx_media_find_format(fourcc, cs_sel, false);
+			cc = imx_media_find_format(fourcc, cs_sel);
 		}
 	} else {
 		cc_src = imx_media_find_mbus_format(fmt_src->format.code,
-						    CS_SEL_ANY, true);
+						    CS_SEL_ANY | CS_SEL_BAYER);
 		if (WARN_ON(!cc_src))
 			return -EINVAL;
 
@@ -790,7 +792,7 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
 	vdev->compose.width = fmt_src.format.width;
 	vdev->compose.height = fmt_src.format.height;
 	vdev->cc = imx_media_find_format(vdev->fmt.fmt.pix.pixelformat,
-					 CS_SEL_ANY, false);
+					 CS_SEL_ANY);
 
 	v4l2_info(sd, "Registered %s as /dev/%s\n", vfd->name,
 		  video_device_node_name(vfd));
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index f4abac93c0e4..912490d21225 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1234,12 +1234,13 @@ static int csi_enum_mbus_code(struct v4l2_subdev *sd,
 	mutex_lock(&priv->lock);
 
 	infmt = __csi_get_fmt(priv, cfg, CSI_SINK_PAD, code->which);
-	incc = imx_media_find_mbus_format(infmt->code, CS_SEL_ANY, true);
+	incc = imx_media_find_mbus_format(infmt->code,
+					  CS_SEL_ANY | CS_SEL_BAYER);
 
 	switch (code->pad) {
 	case CSI_SINK_PAD:
 		ret = imx_media_enum_mbus_format(&code->code, code->index,
-						 CS_SEL_ANY, true);
+						 CS_SEL_ANY | CS_SEL_BAYER);
 		break;
 	case CSI_SRC_PAD_DIRECT:
 	case CSI_SRC_PAD_IDMAC:
@@ -1434,7 +1435,7 @@ static void csi_try_fmt(struct csi_priv *priv,
 	case CSI_SRC_PAD_DIRECT:
 	case CSI_SRC_PAD_IDMAC:
 		incc = imx_media_find_mbus_format(infmt->code,
-						  CS_SEL_ANY, true);
+						  CS_SEL_ANY | CS_SEL_BAYER);
 
 		sdformat->format.width = compose->width;
 		sdformat->format.height = compose->height;
@@ -1468,12 +1469,10 @@ static void csi_try_fmt(struct csi_priv *priv,
 				      MIN_H, MAX_H, H_ALIGN, S_ALIGN);
 
 		*cc = imx_media_find_mbus_format(sdformat->format.code,
-						 CS_SEL_ANY, true);
+						 CS_SEL_ANY | CS_SEL_BAYER);
 		if (!*cc) {
-			imx_media_enum_mbus_format(&code, 0,
-						   CS_SEL_ANY, false);
-			*cc = imx_media_find_mbus_format(code,
-							 CS_SEL_ANY, false);
+			imx_media_enum_mbus_format(&code, 0, CS_SEL_ANY);
+			*cc = imx_media_find_mbus_format(code, CS_SEL_ANY);
 			sdformat->format.code = (*cc)->codes[0];
 		}
 
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 75c4097a9d16..6cce2e39a0e7 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -268,24 +268,27 @@ struct imx_media_pixfmt *__find_format(u32 fourcc,
 static const struct imx_media_pixfmt *find_format(u32 fourcc,
 						  u32 code,
 						  enum codespace_sel cs_sel,
-						  bool allow_non_mbus,
-						  bool allow_bayer)
+						  bool allow_non_mbus)
 {
 	const struct imx_media_pixfmt *ret;
 
-	switch (cs_sel) {
+	switch (cs_sel & (CS_SEL_YUV | CS_SEL_RGB)) {
 	case CS_SEL_YUV:
-		return __find_format(fourcc, code, allow_non_mbus, allow_bayer,
+		return __find_format(fourcc, code, allow_non_mbus,
+				     cs_sel & CS_SEL_BAYER,
 				     yuv_formats, NUM_YUV_FORMATS);
 	case CS_SEL_RGB:
-		return __find_format(fourcc, code, allow_non_mbus, allow_bayer,
+		return __find_format(fourcc, code, allow_non_mbus,
+				    cs_sel & CS_SEL_BAYER,
 				     rgb_formats, NUM_RGB_FORMATS);
 	case CS_SEL_ANY:
-		ret = __find_format(fourcc, code, allow_non_mbus, allow_bayer,
+		ret = __find_format(fourcc, code, allow_non_mbus,
+				    cs_sel & CS_SEL_BAYER,
 				    yuv_formats, NUM_YUV_FORMATS);
 		if (ret)
 			return ret;
-		return __find_format(fourcc, code, allow_non_mbus, allow_bayer,
+		return __find_format(fourcc, code, allow_non_mbus,
+				     cs_sel & CS_SEL_BAYER,
 				     rgb_formats, NUM_RGB_FORMATS);
 	default:
 		return NULL;
@@ -294,8 +297,7 @@ static const struct imx_media_pixfmt *find_format(u32 fourcc,
 
 static int enum_format(u32 *fourcc, u32 *code, u32 index,
 		       enum codespace_sel cs_sel,
-		       bool allow_non_mbus,
-		       bool allow_bayer)
+		       bool allow_non_mbus)
 {
 	const struct imx_media_pixfmt *fmt;
 	u32 mbus_yuv_sz = NUM_MBUS_YUV_FORMATS;
@@ -303,7 +305,7 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
 	u32 yuv_sz = NUM_YUV_FORMATS;
 	u32 rgb_sz = NUM_RGB_FORMATS;
 
-	switch (cs_sel) {
+	switch (cs_sel & (CS_SEL_YUV | CS_SEL_RGB)) {
 	case CS_SEL_YUV:
 		if (index >= yuv_sz ||
 		    (!allow_non_mbus && index >= mbus_yuv_sz))
@@ -315,7 +317,7 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
 		    (!allow_non_mbus && index >= mbus_rgb_sz))
 			return -EINVAL;
 		fmt = &rgb_formats[index];
-		if (!allow_bayer && fmt->bayer)
+		if (!(cs_sel & CS_SEL_BAYER) && fmt->bayer)
 			return -EINVAL;
 		break;
 	case CS_SEL_ANY:
@@ -325,7 +327,7 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
 				if (index >= mbus_rgb_sz)
 					return -EINVAL;
 				fmt = &rgb_formats[index];
-				if (!allow_bayer && fmt->bayer)
+				if (!(cs_sel & CS_SEL_BAYER) && fmt->bayer)
 					return -EINVAL;
 			} else {
 				fmt = &yuv_formats[index];
@@ -335,7 +337,7 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
 				return -EINVAL;
 			if (index >= yuv_sz) {
 				fmt = &rgb_formats[index - yuv_sz];
-				if (!allow_bayer && fmt->bayer)
+				if (!(cs_sel & CS_SEL_BAYER) && fmt->bayer)
 					return -EINVAL;
 			} else {
 				fmt = &yuv_formats[index];
@@ -355,30 +357,28 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
 }
 
 const struct imx_media_pixfmt *
-imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel, bool allow_bayer)
+imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel)
 {
-	return find_format(fourcc, 0, cs_sel, true, allow_bayer);
+	return find_format(fourcc, 0, cs_sel, true);
 }
 EXPORT_SYMBOL_GPL(imx_media_find_format);
 
 int imx_media_enum_format(u32 *fourcc, u32 index, enum codespace_sel cs_sel)
 {
-	return enum_format(fourcc, NULL, index, cs_sel, true, false);
+	return enum_format(fourcc, NULL, index, cs_sel, true);
 }
 EXPORT_SYMBOL_GPL(imx_media_enum_format);
 
 const struct imx_media_pixfmt *
-imx_media_find_mbus_format(u32 code, enum codespace_sel cs_sel,
-			   bool allow_bayer)
+imx_media_find_mbus_format(u32 code, enum codespace_sel cs_sel)
 {
-	return find_format(0, code, cs_sel, false, allow_bayer);
+	return find_format(0, code, cs_sel, false);
 }
 EXPORT_SYMBOL_GPL(imx_media_find_mbus_format);
 
-int imx_media_enum_mbus_format(u32 *code, u32 index, enum codespace_sel cs_sel,
-			       bool allow_bayer)
+int imx_media_enum_mbus_format(u32 *code, u32 index, enum codespace_sel cs_sel)
 {
-	return enum_format(NULL, code, index, cs_sel, false, allow_bayer);
+	return enum_format(NULL, code, index, cs_sel, false);
 }
 EXPORT_SYMBOL_GPL(imx_media_enum_mbus_format);
 
@@ -466,8 +466,8 @@ int imx_media_init_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
 	mbus->height = height;
 	mbus->field = field;
 	if (code == 0)
-		imx_media_enum_mbus_format(&code, 0, CS_SEL_YUV, false);
-	lcc = imx_media_find_mbus_format(code, CS_SEL_ANY, false);
+		imx_media_enum_mbus_format(&code, 0, CS_SEL_YUV);
+	lcc = imx_media_find_mbus_format(code, CS_SEL_ANY);
 	if (!lcc) {
 		lcc = imx_media_find_ipu_format(code, CS_SEL_ANY);
 		if (!lcc)
@@ -538,7 +538,8 @@ void imx_media_try_colorimetry(struct v4l2_mbus_framefmt *tryfmt,
 	const struct imx_media_pixfmt *cc;
 	bool is_rgb = false;
 
-	cc = imx_media_find_mbus_format(tryfmt->code, CS_SEL_ANY, true);
+	cc = imx_media_find_mbus_format(tryfmt->code,
+					CS_SEL_ANY | CS_SEL_BAYER);
 	if (!cc)
 		cc = imx_media_find_ipu_format(tryfmt->code, CS_SEL_ANY);
 	if (cc && cc->cs == IPUV3_COLORSPACE_RGB)
@@ -592,8 +593,9 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
 	if (!cc) {
 		cc = imx_media_find_ipu_format(mbus->code, CS_SEL_ANY);
 		if (!cc)
-			cc = imx_media_find_mbus_format(mbus->code, CS_SEL_ANY,
-							true);
+			cc = imx_media_find_mbus_format(mbus->code,
+							CS_SEL_ANY |
+							CS_SEL_BAYER);
 		if (!cc)
 			return -EINVAL;
 	}
@@ -605,8 +607,8 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
 	if (cc->ipufmt && cc->cs == IPUV3_COLORSPACE_YUV) {
 		u32 code;
 
-		imx_media_enum_mbus_format(&code, 0, CS_SEL_YUV, false);
-		cc = imx_media_find_mbus_format(code, CS_SEL_YUV, false);
+		imx_media_enum_mbus_format(&code, 0, CS_SEL_YUV);
+		cc = imx_media_find_mbus_format(code, CS_SEL_YUV);
 	}
 
 	/* Round up width for minimum burst size */
@@ -657,7 +659,8 @@ int imx_media_ipu_image_to_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
 {
 	const struct imx_media_pixfmt *fmt;
 
-	fmt = imx_media_find_format(image->pix.pixelformat, CS_SEL_ANY, true);
+	fmt = imx_media_find_format(image->pix.pixelformat,
+				    CS_SEL_ANY | CS_SEL_BAYER);
 	if (!fmt)
 		return -EINVAL;
 
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 11861191324a..dc6f8fe7c66c 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -150,20 +150,19 @@ struct imx_media_dev {
 };
 
 enum codespace_sel {
-	CS_SEL_YUV = 0,
-	CS_SEL_RGB,
-	CS_SEL_ANY,
+	CS_SEL_YUV = BIT(0),
+	CS_SEL_RGB = BIT(1),
+	CS_SEL_ANY = CS_SEL_YUV | CS_SEL_RGB,
+	CS_SEL_BAYER = BIT(2),
 };
 
 /* imx-media-utils.c */
 const struct imx_media_pixfmt *
-imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel, bool allow_bayer);
+imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel);
 int imx_media_enum_format(u32 *fourcc, u32 index, enum codespace_sel cs_sel);
 const struct imx_media_pixfmt *
-imx_media_find_mbus_format(u32 code, enum codespace_sel cs_sel,
-			   bool allow_bayer);
-int imx_media_enum_mbus_format(u32 *code, u32 index, enum codespace_sel cs_sel,
-			       bool allow_bayer);
+imx_media_find_mbus_format(u32 code, enum codespace_sel cs_sel);
+int imx_media_enum_mbus_format(u32 *code, u32 index, enum codespace_sel cs_sel);
 const struct imx_media_pixfmt *
 imx_media_find_ipu_format(u32 code, enum codespace_sel cs_sel);
 int imx_media_enum_ipu_format(u32 *code, u32 index, enum codespace_sel cs_sel);
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 3225082ce58d..4f1f9e7dff95 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -959,7 +959,7 @@ static int imx7_csi_enum_mbus_code(struct v4l2_subdev *sd,
 	switch (code->pad) {
 	case IMX7_CSI_PAD_SINK:
 		ret = imx_media_enum_mbus_format(&code->code, code->index,
-						 CS_SEL_ANY, true);
+						 CS_SEL_ANY | CS_SEL_BAYER);
 		break;
 	case IMX7_CSI_PAD_SRC:
 		if (code->index != 0) {
@@ -1019,8 +1019,8 @@ static int imx7_csi_try_fmt(struct imx7_csi *csi,
 
 	switch (sdformat->pad) {
 	case IMX7_CSI_PAD_SRC:
-		in_cc = imx_media_find_mbus_format(in_fmt->code, CS_SEL_ANY,
-						   true);
+		in_cc = imx_media_find_mbus_format(in_fmt->code,
+						   CS_SEL_ANY | CS_SEL_BAYER);
 
 		sdformat->format.width = in_fmt->width;
 		sdformat->format.height = in_fmt->height;
@@ -1033,11 +1033,10 @@ static int imx7_csi_try_fmt(struct imx7_csi *csi,
 		break;
 	case IMX7_CSI_PAD_SINK:
 		*cc = imx_media_find_mbus_format(sdformat->format.code,
-						 CS_SEL_ANY, true);
+						 CS_SEL_ANY | CS_SEL_BAYER);
 		if (!*cc) {
-			imx_media_enum_mbus_format(&code, 0, CS_SEL_ANY, false);
-			*cc = imx_media_find_mbus_format(code, CS_SEL_ANY,
-							 false);
+			imx_media_enum_mbus_format(&code, 0, CS_SEL_ANY);
+			*cc = imx_media_find_mbus_format(code, CS_SEL_ANY);
 			sdformat->format.code = (*cc)->codes[0];
 		}
 
-- 
Regards,

Laurent Pinchart

[PATCH 5/8] media: imx: utils: Simplify IPU format lookup and enumeration

[Laurent Pinchart]

The IPU formats are stored in two separate tables, one for YUV and one
for RGB formats. This complicates the lookup and enumeration function
without really increasing efficiency, as both tables contain a single
element. Merge the two tables and simplify the functions, and move the
resulting table next to the functions that use it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/staging/media/imx/imx-media-utils.c | 128 ++++++++------------
 1 file changed, 50 insertions(+), 78 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 6cce2e39a0e7..5036a856be95 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -209,30 +209,6 @@ static const struct imx_media_pixfmt rgb_formats[] = {
 #define NUM_RGB_FORMATS ARRAY_SIZE(rgb_formats)
 #define NUM_MBUS_RGB_FORMATS (NUM_RGB_FORMATS - NUM_NON_MBUS_RGB_FORMATS)
 
-static const struct imx_media_pixfmt ipu_yuv_formats[] = {
-	{
-		.fourcc = V4L2_PIX_FMT_YUV32,
-		.codes  = {MEDIA_BUS_FMT_AYUV8_1X32},
-		.cs     = IPUV3_COLORSPACE_YUV,
-		.bpp    = 32,
-		.ipufmt = true,
-	},
-};
-
-#define NUM_IPU_YUV_FORMATS ARRAY_SIZE(ipu_yuv_formats)
-
-static const struct imx_media_pixfmt ipu_rgb_formats[] = {
-	{
-		.fourcc	= V4L2_PIX_FMT_XRGB32,
-		.codes  = {MEDIA_BUS_FMT_ARGB8888_1X32},
-		.cs     = IPUV3_COLORSPACE_RGB,
-		.bpp    = 32,
-		.ipufmt = true,
-	},
-};
-
-#define NUM_IPU_RGB_FORMATS ARRAY_SIZE(ipu_rgb_formats)
-
 static const
 struct imx_media_pixfmt *__find_format(u32 fourcc,
 				       u32 code,
@@ -382,77 +358,73 @@ int imx_media_enum_mbus_format(u32 *code, u32 index, enum codespace_sel cs_sel)
 }
 EXPORT_SYMBOL_GPL(imx_media_enum_mbus_format);
 
+/* -----------------------------------------------------------------------------
+ * IPU Formats Lookup and Enumeration
+ */
+
+static const struct imx_media_pixfmt ipu_formats[] = {
+	{
+		.fourcc = V4L2_PIX_FMT_YUV32,
+		.codes  = {MEDIA_BUS_FMT_AYUV8_1X32},
+		.cs     = IPUV3_COLORSPACE_YUV,
+		.bpp    = 32,
+		.ipufmt = true,
+	}, {
+		.fourcc	= V4L2_PIX_FMT_XRGB32,
+		.codes  = {MEDIA_BUS_FMT_ARGB8888_1X32},
+		.cs     = IPUV3_COLORSPACE_RGB,
+		.bpp    = 32,
+		.ipufmt = true,
+	},
+};
+
 const struct imx_media_pixfmt *
 imx_media_find_ipu_format(u32 code, enum codespace_sel cs_sel)
 {
-	const struct imx_media_pixfmt *array, *fmt, *ret = NULL;
-	u32 array_size;
-	int i, j;
+	bool accept_yuv = cs_sel & CS_SEL_YUV;
+	bool accept_rgb = cs_sel & CS_SEL_RGB;
+	unsigned int i, j;
 
-	switch (cs_sel) {
-	case CS_SEL_YUV:
-		array_size = NUM_IPU_YUV_FORMATS;
-		array = ipu_yuv_formats;
-		break;
-	case CS_SEL_RGB:
-		array_size = NUM_IPU_RGB_FORMATS;
-		array = ipu_rgb_formats;
-		break;
-	case CS_SEL_ANY:
-		array_size = NUM_IPU_YUV_FORMATS + NUM_IPU_RGB_FORMATS;
-		array = ipu_yuv_formats;
-		break;
-	default:
+	if (!code)
 		return NULL;
-	}
 
-	for (i = 0; i < array_size; i++) {
-		if (cs_sel == CS_SEL_ANY && i >= NUM_IPU_YUV_FORMATS)
-			fmt = &ipu_rgb_formats[i - NUM_IPU_YUV_FORMATS];
-		else
-			fmt = &array[i];
-
-		for (j = 0; code && fmt->codes[j]; j++) {
-			if (code == fmt->codes[j]) {
-				ret = fmt;
-				goto out;
-			}
+	for (i = 0; i < ARRAY_SIZE(ipu_formats); i++) {
+		const struct imx_media_pixfmt *fmt = &ipu_formats[i];
+
+		if ((!accept_yuv && fmt->cs == IPUV3_COLORSPACE_YUV) ||
+		    (!accept_rgb && fmt->cs == IPUV3_COLORSPACE_RGB))
+			continue;
+
+		for (j = 0; fmt->codes[j]; j++) {
+			if (code == fmt->codes[j])
+				return fmt;
 		}
 	}
 
-out:
-	return ret;
+	return NULL;
 }
 EXPORT_SYMBOL_GPL(imx_media_find_ipu_format);
 
 int imx_media_enum_ipu_format(u32 *code, u32 index, enum codespace_sel cs_sel)
 {
-	switch (cs_sel) {
-	case CS_SEL_YUV:
-		if (index >= NUM_IPU_YUV_FORMATS)
-			return -EINVAL;
-		*code = ipu_yuv_formats[index].codes[0];
-		break;
-	case CS_SEL_RGB:
-		if (index >= NUM_IPU_RGB_FORMATS)
-			return -EINVAL;
-		*code = ipu_rgb_formats[index].codes[0];
-		break;
-	case CS_SEL_ANY:
-		if (index >= NUM_IPU_YUV_FORMATS + NUM_IPU_RGB_FORMATS)
-			return -EINVAL;
-		if (index >= NUM_IPU_YUV_FORMATS) {
-			index -= NUM_IPU_YUV_FORMATS;
-			*code = ipu_rgb_formats[index].codes[0];
-		} else {
-			*code = ipu_yuv_formats[index].codes[0];
+	bool accept_yuv = cs_sel & CS_SEL_YUV;
+	bool accept_rgb = cs_sel & CS_SEL_RGB;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(ipu_formats); i++) {
+		const struct imx_media_pixfmt *fmt = &ipu_formats[i];
+
+		if ((!accept_yuv && fmt->cs == IPUV3_COLORSPACE_YUV) ||
+		    (!accept_rgb && fmt->cs == IPUV3_COLORSPACE_RGB))
+			continue;
+
+		if (index-- == 0) {
+			*code = fmt->codes[0];
+			return 0;
 		}
-		break;
-	default:
-		return -EINVAL;
 	}
 
-	return 0;
+	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(imx_media_enum_ipu_format);
 
-- 
Regards,

Laurent Pinchart

[PATCH 6/8] media: imx: utils: Make imx_media_pixfmt handle variable number of codes

[Laurent Pinchart]

The imx_media_pixfmt structures includes a codes member that stores
media bus codes as a fixed array of 4 integers. The functions dealing
with the imx_media_pixfmt structures assume that the array of codes is
terminated by a 0 elements. This mechanism is fragile, as demonstrated
by several instances of the structure contained 4 non-zero codes.

Fix this by turning the array into a pointer, and providing an
IMX_BUS_FMTS macro to initialize the codes member with a guaranteed 0
element at the end.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/staging/media/imx/imx-media-utils.c | 78 ++++++++++++---------
 drivers/staging/media/imx/imx-media.h       |  2 +-
 2 files changed, 46 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 5036a856be95..291d0b781278 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -14,21 +14,27 @@
  * mbus codes) must all fall at the end of the table.
  */
 
+#define IMX_BUS_FMTS(fmts...)	\
+	(const u32[]) {		\
+		fmts,		\
+		0		\
+	}
+
 static const struct imx_media_pixfmt yuv_formats[] = {
 	{
 		.fourcc	= V4L2_PIX_FMT_UYVY,
-		.codes  = {
+		.codes  = IMX_BUS_FMTS(
 			MEDIA_BUS_FMT_UYVY8_2X8,
 			MEDIA_BUS_FMT_UYVY8_1X16
-		},
+		),
 		.cs     = IPUV3_COLORSPACE_YUV,
 		.bpp    = 16,
 	}, {
 		.fourcc	= V4L2_PIX_FMT_YUYV,
-		.codes  = {
+		.codes  = IMX_BUS_FMTS(
 			MEDIA_BUS_FMT_YUYV8_2X8,
 			MEDIA_BUS_FMT_YUYV8_1X16
-		},
+		),
 		.cs     = IPUV3_COLORSPACE_YUV,
 		.bpp    = 16,
 	},
@@ -71,21 +77,21 @@ static const struct imx_media_pixfmt yuv_formats[] = {
 static const struct imx_media_pixfmt rgb_formats[] = {
 	{
 		.fourcc	= V4L2_PIX_FMT_RGB565,
-		.codes  = {MEDIA_BUS_FMT_RGB565_2X8_LE},
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_RGB565_2X8_LE),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 16,
 		.cycles = 2,
 	}, {
 		.fourcc	= V4L2_PIX_FMT_RGB24,
-		.codes  = {
+		.codes  = IMX_BUS_FMTS(
 			MEDIA_BUS_FMT_RGB888_1X24,
 			MEDIA_BUS_FMT_RGB888_2X12_LE
-		},
+		),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 24,
 	}, {
 		.fourcc	= V4L2_PIX_FMT_XRGB32,
-		.codes  = {MEDIA_BUS_FMT_ARGB8888_1X32},
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_ARGB8888_1X32),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 32,
 		.ipufmt = true,
@@ -93,91 +99,91 @@ static const struct imx_media_pixfmt rgb_formats[] = {
 	/*** raw bayer and grayscale formats start here ***/
 	{
 		.fourcc = V4L2_PIX_FMT_SBGGR8,
-		.codes  = {MEDIA_BUS_FMT_SBGGR8_1X8},
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SBGGR8_1X8),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 8,
 		.bayer  = true,
 	}, {
 		.fourcc = V4L2_PIX_FMT_SGBRG8,
-		.codes  = {MEDIA_BUS_FMT_SGBRG8_1X8},
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SGBRG8_1X8),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 8,
 		.bayer  = true,
 	}, {
 		.fourcc = V4L2_PIX_FMT_SGRBG8,
-		.codes  = {MEDIA_BUS_FMT_SGRBG8_1X8},
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SGRBG8_1X8),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 8,
 		.bayer  = true,
 	}, {
 		.fourcc = V4L2_PIX_FMT_SRGGB8,
-		.codes  = {MEDIA_BUS_FMT_SRGGB8_1X8},
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_SRGGB8_1X8),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 8,
 		.bayer  = true,
 	}, {
 		.fourcc = V4L2_PIX_FMT_SBGGR16,
-		.codes  = {
+		.codes  = IMX_BUS_FMTS(
 			MEDIA_BUS_FMT_SBGGR10_1X10,
 			MEDIA_BUS_FMT_SBGGR12_1X12,
 			MEDIA_BUS_FMT_SBGGR14_1X14,
 			MEDIA_BUS_FMT_SBGGR16_1X16
-		},
+		),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 16,
 		.bayer  = true,
 	}, {
 		.fourcc = V4L2_PIX_FMT_SGBRG16,
-		.codes  = {
+		.codes  = IMX_BUS_FMTS(
 			MEDIA_BUS_FMT_SGBRG10_1X10,
 			MEDIA_BUS_FMT_SGBRG12_1X12,
 			MEDIA_BUS_FMT_SGBRG14_1X14,
-			MEDIA_BUS_FMT_SGBRG16_1X16,
-		},
+			MEDIA_BUS_FMT_SGBRG16_1X16
+		),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 16,
 		.bayer  = true,
 	}, {
 		.fourcc = V4L2_PIX_FMT_SGRBG16,
-		.codes  = {
+		.codes  = IMX_BUS_FMTS(
 			MEDIA_BUS_FMT_SGRBG10_1X10,
 			MEDIA_BUS_FMT_SGRBG12_1X12,
 			MEDIA_BUS_FMT_SGRBG14_1X14,
-			MEDIA_BUS_FMT_SGRBG16_1X16,
-		},
+			MEDIA_BUS_FMT_SGRBG16_1X16
+		),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 16,
 		.bayer  = true,
 	}, {
 		.fourcc = V4L2_PIX_FMT_SRGGB16,
-		.codes  = {
+		.codes  = IMX_BUS_FMTS(
 			MEDIA_BUS_FMT_SRGGB10_1X10,
 			MEDIA_BUS_FMT_SRGGB12_1X12,
 			MEDIA_BUS_FMT_SRGGB14_1X14,
-			MEDIA_BUS_FMT_SRGGB16_1X16,
-		},
+			MEDIA_BUS_FMT_SRGGB16_1X16
+		),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 16,
 		.bayer  = true,
 	}, {
 		.fourcc = V4L2_PIX_FMT_GREY,
-		.codes = {
+		.codes = IMX_BUS_FMTS(
 			MEDIA_BUS_FMT_Y8_1X8,
 			MEDIA_BUS_FMT_Y10_1X10,
-			MEDIA_BUS_FMT_Y12_1X12,
-		},
+			MEDIA_BUS_FMT_Y12_1X12
+		),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 8,
 		.bayer  = true,
 	}, {
 		.fourcc = V4L2_PIX_FMT_Y10,
-		.codes = {MEDIA_BUS_FMT_Y10_1X10},
+		.codes = IMX_BUS_FMTS(MEDIA_BUS_FMT_Y10_1X10),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 16,
 		.bayer  = true,
 	}, {
 		.fourcc = V4L2_PIX_FMT_Y12,
-		.codes = {MEDIA_BUS_FMT_Y12_1X12},
+		.codes = IMX_BUS_FMTS(MEDIA_BUS_FMT_Y12_1X12),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 16,
 		.bayer  = true,
@@ -223,14 +229,14 @@ struct imx_media_pixfmt *__find_format(u32 fourcc,
 	for (i = 0; i < array_size; i++) {
 		fmt = &array[i];
 
-		if ((!allow_non_mbus && !fmt->codes[0]) ||
+		if ((!allow_non_mbus && !fmt->codes) ||
 		    (!allow_bayer && fmt->bayer))
 			continue;
 
 		if (fourcc && fmt->fourcc == fourcc)
 			return fmt;
 
-		if (!code)
+		if (!code || !fmt->codes)
 			continue;
 
 		for (j = 0; fmt->codes[j]; j++) {
@@ -327,7 +333,7 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
 	if (fourcc)
 		*fourcc = fmt->fourcc;
 	if (code)
-		*code = fmt->codes[0];
+		*code = fmt->codes ? fmt->codes[0] : 0;
 
 	return 0;
 }
@@ -365,13 +371,13 @@ EXPORT_SYMBOL_GPL(imx_media_enum_mbus_format);
 static const struct imx_media_pixfmt ipu_formats[] = {
 	{
 		.fourcc = V4L2_PIX_FMT_YUV32,
-		.codes  = {MEDIA_BUS_FMT_AYUV8_1X32},
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_AYUV8_1X32),
 		.cs     = IPUV3_COLORSPACE_YUV,
 		.bpp    = 32,
 		.ipufmt = true,
 	}, {
 		.fourcc	= V4L2_PIX_FMT_XRGB32,
-		.codes  = {MEDIA_BUS_FMT_ARGB8888_1X32},
+		.codes  = IMX_BUS_FMTS(MEDIA_BUS_FMT_ARGB8888_1X32),
 		.cs     = IPUV3_COLORSPACE_RGB,
 		.bpp    = 32,
 		.ipufmt = true,
@@ -395,6 +401,9 @@ imx_media_find_ipu_format(u32 code, enum codespace_sel cs_sel)
 		    (!accept_rgb && fmt->cs == IPUV3_COLORSPACE_RGB))
 			continue;
 
+		if (!fmt->codes)
+			continue;
+
 		for (j = 0; fmt->codes[j]; j++) {
 			if (code == fmt->codes[j])
 				return fmt;
@@ -418,6 +427,9 @@ int imx_media_enum_ipu_format(u32 *code, u32 index, enum codespace_sel cs_sel)
 		    (!accept_rgb && fmt->cs == IPUV3_COLORSPACE_RGB))
 			continue;
 
+		if (!fmt->codes)
+			continue;
+
 		if (index-- == 0) {
 			*code = fmt->codes[0];
 			return 0;
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index dc6f8fe7c66c..9cedc27a6b4a 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -69,7 +69,7 @@ enum {
 
 struct imx_media_pixfmt {
 	u32     fourcc;
-	u32     codes[4];
+	const u32 *codes;
 	int     bpp;     /* total bpp */
 	/* cycles per pixel for generic (bayer) formats for the parallel bus */
 	int	cycles;
-- 
Regards,

Laurent Pinchart

[PATCH 7/8] media: imx: utils: Remove unneeded argument to (find|enum)_format()

[Laurent Pinchart]

The find_format() and enum_format() functions take an argument that
tells whether to take into account formats that don't have associated
media bus codes. The same information can be deduced from the fourcc
argument passed to these functions. Remove the allow_non_mbus argument
and use fourcc instead internally.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/staging/media/imx/imx-media-utils.c | 31 ++++++++++-----------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 291d0b781278..c437c0e09108 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -218,7 +218,6 @@ static const struct imx_media_pixfmt rgb_formats[] = {
 static const
 struct imx_media_pixfmt *__find_format(u32 fourcc,
 				       u32 code,
-				       bool allow_non_mbus,
 				       bool allow_bayer,
 				       const struct imx_media_pixfmt *array,
 				       u32 array_size)
@@ -229,7 +228,7 @@ struct imx_media_pixfmt *__find_format(u32 fourcc,
 	for (i = 0; i < array_size; i++) {
 		fmt = &array[i];
 
-		if ((!allow_non_mbus && !fmt->codes) ||
+		if ((!fourcc && !fmt->codes) ||
 		    (!allow_bayer && fmt->bayer))
 			continue;
 
@@ -249,27 +248,26 @@ struct imx_media_pixfmt *__find_format(u32 fourcc,
 
 static const struct imx_media_pixfmt *find_format(u32 fourcc,
 						  u32 code,
-						  enum codespace_sel cs_sel,
-						  bool allow_non_mbus)
+						  enum codespace_sel cs_sel)
 {
 	const struct imx_media_pixfmt *ret;
 
 	switch (cs_sel & (CS_SEL_YUV | CS_SEL_RGB)) {
 	case CS_SEL_YUV:
-		return __find_format(fourcc, code, allow_non_mbus,
+		return __find_format(fourcc, code,
 				     cs_sel & CS_SEL_BAYER,
 				     yuv_formats, NUM_YUV_FORMATS);
 	case CS_SEL_RGB:
-		return __find_format(fourcc, code, allow_non_mbus,
+		return __find_format(fourcc, code,
 				    cs_sel & CS_SEL_BAYER,
 				     rgb_formats, NUM_RGB_FORMATS);
 	case CS_SEL_ANY:
-		ret = __find_format(fourcc, code, allow_non_mbus,
+		ret = __find_format(fourcc, code,
 				    cs_sel & CS_SEL_BAYER,
 				    yuv_formats, NUM_YUV_FORMATS);
 		if (ret)
 			return ret;
-		return __find_format(fourcc, code, allow_non_mbus,
+		return __find_format(fourcc, code,
 				     cs_sel & CS_SEL_BAYER,
 				     rgb_formats, NUM_RGB_FORMATS);
 	default:
@@ -278,8 +276,7 @@ static const struct imx_media_pixfmt *find_format(u32 fourcc,
 }
 
 static int enum_format(u32 *fourcc, u32 *code, u32 index,
-		       enum codespace_sel cs_sel,
-		       bool allow_non_mbus)
+		       enum codespace_sel cs_sel)
 {
 	const struct imx_media_pixfmt *fmt;
 	u32 mbus_yuv_sz = NUM_MBUS_YUV_FORMATS;
@@ -290,20 +287,20 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
 	switch (cs_sel & (CS_SEL_YUV | CS_SEL_RGB)) {
 	case CS_SEL_YUV:
 		if (index >= yuv_sz ||
-		    (!allow_non_mbus && index >= mbus_yuv_sz))
+		    (!fourcc && index >= mbus_yuv_sz))
 			return -EINVAL;
 		fmt = &yuv_formats[index];
 		break;
 	case CS_SEL_RGB:
 		if (index >= rgb_sz ||
-		    (!allow_non_mbus && index >= mbus_rgb_sz))
+		    (!fourcc && index >= mbus_rgb_sz))
 			return -EINVAL;
 		fmt = &rgb_formats[index];
 		if (!(cs_sel & CS_SEL_BAYER) && fmt->bayer)
 			return -EINVAL;
 		break;
 	case CS_SEL_ANY:
-		if (!allow_non_mbus) {
+		if (!fourcc) {
 			if (index >= mbus_yuv_sz) {
 				index -= mbus_yuv_sz;
 				if (index >= mbus_rgb_sz)
@@ -341,26 +338,26 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
 const struct imx_media_pixfmt *
 imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel)
 {
-	return find_format(fourcc, 0, cs_sel, true);
+	return find_format(fourcc, 0, cs_sel);
 }
 EXPORT_SYMBOL_GPL(imx_media_find_format);
 
 int imx_media_enum_format(u32 *fourcc, u32 index, enum codespace_sel cs_sel)
 {
-	return enum_format(fourcc, NULL, index, cs_sel, true);
+	return enum_format(fourcc, NULL, index, cs_sel);
 }
 EXPORT_SYMBOL_GPL(imx_media_enum_format);
 
 const struct imx_media_pixfmt *
 imx_media_find_mbus_format(u32 code, enum codespace_sel cs_sel)
 {
-	return find_format(0, code, cs_sel, false);
+	return find_format(0, code, cs_sel);
 }
 EXPORT_SYMBOL_GPL(imx_media_find_mbus_format);
 
 int imx_media_enum_mbus_format(u32 *code, u32 index, enum codespace_sel cs_sel)
 {
-	return enum_format(NULL, code, index, cs_sel, false);
+	return enum_format(NULL, code, index, cs_sel);
 }
 EXPORT_SYMBOL_GPL(imx_media_enum_mbus_format);
 
-- 
Regards,

Laurent Pinchart

[PATCH 8/8] media: imx: utils: Rename format lookup and enumeration functions

[Laurent Pinchart]

Rename the format lookup and enumeration functions according to their
usage:

- Rename imx_media_(find|enum)_format() to *_pixel_format() to
  explicitly state on what formats the functions operate. This aligns
  the naming scheme with the media bus and IPU format functions that
  already end with *_mbus_format() and *_ipu_formats().

- Rename all enumeration functions to pluralize 'formats' at the end, as
  they enumerate multiple formats.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/staging/media/imx/imx-ic-prp.c        |  8 ++---
 drivers/staging/media/imx/imx-ic-prpencvf.c   |  6 ++--
 drivers/staging/media/imx/imx-media-capture.c | 20 +++++------
 .../staging/media/imx/imx-media-csc-scaler.c  |  2 +-
 drivers/staging/media/imx/imx-media-csi.c     | 15 ++++-----
 drivers/staging/media/imx/imx-media-utils.c   | 33 ++++++++++---------
 drivers/staging/media/imx/imx-media-vdic.c    |  6 ++--
 drivers/staging/media/imx/imx-media.h         | 11 ++++---
 drivers/staging/media/imx/imx7-media-csi.c    |  6 ++--
 9 files changed, 55 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-prp.c b/drivers/staging/media/imx/imx-ic-prp.c
index 2a4f77e83ed3..ce10004ca01b 100644
--- a/drivers/staging/media/imx/imx-ic-prp.c
+++ b/drivers/staging/media/imx/imx-ic-prp.c
@@ -106,8 +106,8 @@ static int prp_enum_mbus_code(struct v4l2_subdev *sd,
 
 	switch (code->pad) {
 	case PRP_SINK_PAD:
-		ret = imx_media_enum_ipu_format(&code->code, code->index,
-						CS_SEL_ANY);
+		ret = imx_media_enum_ipu_formats(&code->code, code->index,
+						 CS_SEL_ANY);
 		break;
 	case PRP_SRC_PAD_PRPENC:
 	case PRP_SRC_PAD_PRPVF:
@@ -182,7 +182,7 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
 		cc = imx_media_find_ipu_format(sdformat->format.code,
 					       CS_SEL_ANY);
 		if (!cc) {
-			imx_media_enum_ipu_format(&code, 0, CS_SEL_ANY);
+			imx_media_enum_ipu_formats(&code, 0, CS_SEL_ANY);
 			cc = imx_media_find_ipu_format(code, CS_SEL_ANY);
 			sdformat->format.code = cc->codes[0];
 		}
@@ -438,7 +438,7 @@ static int prp_registered(struct v4l2_subdev *sd)
 	priv->frame_interval.denominator = 30;
 
 	/* set a default mbus format  */
-	imx_media_enum_ipu_format(&code, 0, CS_SEL_YUV);
+	imx_media_enum_ipu_formats(&code, 0, CS_SEL_YUV);
 	return imx_media_init_mbus_fmt(&priv->format_mbus, 640, 480, code,
 				       V4L2_FIELD_NONE, NULL);
 }
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 09c4e3f33807..ec814acc3485 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -850,7 +850,7 @@ static int prp_enum_mbus_code(struct v4l2_subdev *sd,
 	if (code->pad >= PRPENCVF_NUM_PADS)
 		return -EINVAL;
 
-	return imx_media_enum_ipu_format(&code->code, code->index, CS_SEL_ANY);
+	return imx_media_enum_ipu_formats(&code->code, code->index, CS_SEL_ANY);
 }
 
 static int prp_get_fmt(struct v4l2_subdev *sd,
@@ -889,7 +889,7 @@ static void prp_try_fmt(struct prp_priv *priv,
 	if (!*cc) {
 		u32 code;
 
-		imx_media_enum_ipu_format(&code, 0, CS_SEL_ANY);
+		imx_media_enum_ipu_formats(&code, 0, CS_SEL_ANY);
 		*cc = imx_media_find_ipu_format(code, CS_SEL_ANY);
 		sdformat->format.code = (*cc)->codes[0];
 	}
@@ -1248,7 +1248,7 @@ static int prp_registered(struct v4l2_subdev *sd)
 	u32 code;
 
 	/* set a default mbus format  */
-	imx_media_enum_ipu_format(&code, 0, CS_SEL_YUV);
+	imx_media_enum_ipu_formats(&code, 0, CS_SEL_YUV);
 	for (i = 0; i < PRPENCVF_NUM_PADS; i++) {
 		ret = imx_media_init_mbus_fmt(&priv->format_mbus[i],
 					      640, 480, code, V4L2_FIELD_NONE,
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 6685b149db99..ba45478d2062 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -91,8 +91,8 @@ static int capture_enum_framesizes(struct file *file, void *fh,
 	};
 	int ret;
 
-	cc = imx_media_find_format(fsize->pixel_format,
-				   CS_SEL_ANY | CS_SEL_BAYER);
+	cc = imx_media_find_pixel_format(fsize->pixel_format,
+					 CS_SEL_ANY | CS_SEL_BAYER);
 	if (!cc)
 		return -EINVAL;
 
@@ -134,8 +134,8 @@ static int capture_enum_frameintervals(struct file *file, void *fh,
 	};
 	int ret;
 
-	cc = imx_media_find_format(fival->pixel_format,
-				   CS_SEL_ANY | CS_SEL_BAYER);
+	cc = imx_media_find_pixel_format(fival->pixel_format,
+					 CS_SEL_ANY | CS_SEL_BAYER);
 	if (!cc)
 		return -EINVAL;
 
@@ -174,7 +174,7 @@ static int capture_enum_fmt_vid_cap(struct file *file, void *fh,
 		u32 cs_sel = (cc_src->cs == IPUV3_COLORSPACE_YUV) ?
 			CS_SEL_YUV : CS_SEL_RGB;
 
-		ret = imx_media_enum_format(&fourcc, f->index, cs_sel);
+		ret = imx_media_enum_pixel_formats(&fourcc, f->index, cs_sel);
 		if (ret)
 			return ret;
 	} else {
@@ -219,10 +219,10 @@ static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
 			CS_SEL_YUV : CS_SEL_RGB;
 		fourcc = f->fmt.pix.pixelformat;
 
-		cc = imx_media_find_format(fourcc, cs_sel);
+		cc = imx_media_find_pixel_format(fourcc, cs_sel);
 		if (!cc) {
-			imx_media_enum_format(&fourcc, 0, cs_sel);
-			cc = imx_media_find_format(fourcc, cs_sel);
+			imx_media_enum_pixel_formats(&fourcc, 0, cs_sel);
+			cc = imx_media_find_pixel_format(fourcc, cs_sel);
 		}
 	} else {
 		cc_src = imx_media_find_mbus_format(fmt_src->format.code,
@@ -791,8 +791,8 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
 				      &fmt_src.format, NULL);
 	vdev->compose.width = fmt_src.format.width;
 	vdev->compose.height = fmt_src.format.height;
-	vdev->cc = imx_media_find_format(vdev->fmt.fmt.pix.pixelformat,
-					 CS_SEL_ANY);
+	vdev->cc = imx_media_find_pixel_format(vdev->fmt.fmt.pix.pixelformat,
+					       CS_SEL_ANY);
 
 	v4l2_info(sd, "Registered %s as /dev/%s\n", vfd->name,
 		  video_device_node_name(vfd));
diff --git a/drivers/staging/media/imx/imx-media-csc-scaler.c b/drivers/staging/media/imx/imx-media-csc-scaler.c
index 2b635ebf62d6..3435f7b2a2a1 100644
--- a/drivers/staging/media/imx/imx-media-csc-scaler.c
+++ b/drivers/staging/media/imx/imx-media-csc-scaler.c
@@ -164,7 +164,7 @@ static int ipu_csc_scaler_enum_fmt(struct file *file, void *fh,
 	u32 fourcc;
 	int ret;
 
-	ret = imx_media_enum_format(&fourcc, f->index, CS_SEL_ANY);
+	ret = imx_media_enum_pixel_formats(&fourcc, f->index, CS_SEL_ANY);
 	if (ret)
 		return ret;
 
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 912490d21225..45166367f5a0 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1239,8 +1239,8 @@ static int csi_enum_mbus_code(struct v4l2_subdev *sd,
 
 	switch (code->pad) {
 	case CSI_SINK_PAD:
-		ret = imx_media_enum_mbus_format(&code->code, code->index,
-						 CS_SEL_ANY | CS_SEL_BAYER);
+		ret = imx_media_enum_mbus_formats(&code->code, code->index,
+						  CS_SEL_ANY | CS_SEL_BAYER);
 		break;
 	case CSI_SRC_PAD_DIRECT:
 	case CSI_SRC_PAD_IDMAC:
@@ -1259,9 +1259,8 @@ static int csi_enum_mbus_code(struct v4l2_subdev *sd,
 		} else {
 			u32 cs_sel = (incc->cs == IPUV3_COLORSPACE_YUV) ?
 				CS_SEL_YUV : CS_SEL_RGB;
-			ret = imx_media_enum_ipu_format(&code->code,
-							code->index,
-							cs_sel);
+			ret = imx_media_enum_ipu_formats(&code->code,
+							 code->index, cs_sel);
 		}
 		break;
 	default:
@@ -1450,7 +1449,7 @@ static void csi_try_fmt(struct csi_priv *priv,
 			*cc = imx_media_find_ipu_format(sdformat->format.code,
 							cs_sel);
 			if (!*cc) {
-				imx_media_enum_ipu_format(&code, 0, cs_sel);
+				imx_media_enum_ipu_formats(&code, 0, cs_sel);
 				*cc = imx_media_find_ipu_format(code, cs_sel);
 				sdformat->format.code = (*cc)->codes[0];
 			}
@@ -1471,7 +1470,7 @@ static void csi_try_fmt(struct csi_priv *priv,
 		*cc = imx_media_find_mbus_format(sdformat->format.code,
 						 CS_SEL_ANY | CS_SEL_BAYER);
 		if (!*cc) {
-			imx_media_enum_mbus_format(&code, 0, CS_SEL_ANY);
+			imx_media_enum_mbus_formats(&code, 0, CS_SEL_ANY);
 			*cc = imx_media_find_mbus_format(code, CS_SEL_ANY);
 			sdformat->format.code = (*cc)->codes[0];
 		}
@@ -1758,7 +1757,7 @@ static int csi_registered(struct v4l2_subdev *sd)
 	for (i = 0; i < CSI_NUM_PADS; i++) {
 		code = 0;
 		if (i != CSI_SINK_PAD)
-			imx_media_enum_ipu_format(&code, 0, CS_SEL_YUV);
+			imx_media_enum_ipu_formats(&code, 0, CS_SEL_YUV);
 
 		/* set a default mbus format  */
 		ret = imx_media_init_mbus_fmt(&priv->format_mbus[i],
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index c437c0e09108..06415c8f707e 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -275,8 +275,8 @@ static const struct imx_media_pixfmt *find_format(u32 fourcc,
 	}
 }
 
-static int enum_format(u32 *fourcc, u32 *code, u32 index,
-		       enum codespace_sel cs_sel)
+static int enum_formats(u32 *fourcc, u32 *code, u32 index,
+			enum codespace_sel cs_sel)
 {
 	const struct imx_media_pixfmt *fmt;
 	u32 mbus_yuv_sz = NUM_MBUS_YUV_FORMATS;
@@ -336,17 +336,18 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
 }
 
 const struct imx_media_pixfmt *
-imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel)
+imx_media_find_pixel_format(u32 fourcc, enum codespace_sel cs_sel)
 {
 	return find_format(fourcc, 0, cs_sel);
 }
-EXPORT_SYMBOL_GPL(imx_media_find_format);
+EXPORT_SYMBOL_GPL(imx_media_find_pixel_format);
 
-int imx_media_enum_format(u32 *fourcc, u32 index, enum codespace_sel cs_sel)
+int imx_media_enum_pixel_formats(u32 *fourcc, u32 index,
+				 enum codespace_sel cs_sel)
 {
-	return enum_format(fourcc, NULL, index, cs_sel);
+	return enum_formats(fourcc, NULL, index, cs_sel);
 }
-EXPORT_SYMBOL_GPL(imx_media_enum_format);
+EXPORT_SYMBOL_GPL(imx_media_enum_pixel_formats);
 
 const struct imx_media_pixfmt *
 imx_media_find_mbus_format(u32 code, enum codespace_sel cs_sel)
@@ -355,11 +356,11 @@ imx_media_find_mbus_format(u32 code, enum codespace_sel cs_sel)
 }
 EXPORT_SYMBOL_GPL(imx_media_find_mbus_format);
 
-int imx_media_enum_mbus_format(u32 *code, u32 index, enum codespace_sel cs_sel)
+int imx_media_enum_mbus_formats(u32 *code, u32 index, enum codespace_sel cs_sel)
 {
-	return enum_format(NULL, code, index, cs_sel);
+	return enum_formats(NULL, code, index, cs_sel);
 }
-EXPORT_SYMBOL_GPL(imx_media_enum_mbus_format);
+EXPORT_SYMBOL_GPL(imx_media_enum_mbus_formats);
 
 /* -----------------------------------------------------------------------------
  * IPU Formats Lookup and Enumeration
@@ -411,7 +412,7 @@ imx_media_find_ipu_format(u32 code, enum codespace_sel cs_sel)
 }
 EXPORT_SYMBOL_GPL(imx_media_find_ipu_format);
 
-int imx_media_enum_ipu_format(u32 *code, u32 index, enum codespace_sel cs_sel)
+int imx_media_enum_ipu_formats(u32 *code, u32 index, enum codespace_sel cs_sel)
 {
 	bool accept_yuv = cs_sel & CS_SEL_YUV;
 	bool accept_rgb = cs_sel & CS_SEL_RGB;
@@ -435,7 +436,7 @@ int imx_media_enum_ipu_format(u32 *code, u32 index, enum codespace_sel cs_sel)
 
 	return -EINVAL;
 }
-EXPORT_SYMBOL_GPL(imx_media_enum_ipu_format);
+EXPORT_SYMBOL_GPL(imx_media_enum_ipu_formats);
 
 int imx_media_init_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
 			    u32 width, u32 height, u32 code, u32 field,
@@ -447,7 +448,7 @@ int imx_media_init_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
 	mbus->height = height;
 	mbus->field = field;
 	if (code == 0)
-		imx_media_enum_mbus_format(&code, 0, CS_SEL_YUV);
+		imx_media_enum_mbus_formats(&code, 0, CS_SEL_YUV);
 	lcc = imx_media_find_mbus_format(code, CS_SEL_ANY);
 	if (!lcc) {
 		lcc = imx_media_find_ipu_format(code, CS_SEL_ANY);
@@ -588,7 +589,7 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
 	if (cc->ipufmt && cc->cs == IPUV3_COLORSPACE_YUV) {
 		u32 code;
 
-		imx_media_enum_mbus_format(&code, 0, CS_SEL_YUV);
+		imx_media_enum_mbus_formats(&code, 0, CS_SEL_YUV);
 		cc = imx_media_find_mbus_format(code, CS_SEL_YUV);
 	}
 
@@ -640,8 +641,8 @@ int imx_media_ipu_image_to_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
 {
 	const struct imx_media_pixfmt *fmt;
 
-	fmt = imx_media_find_format(image->pix.pixelformat,
-				    CS_SEL_ANY | CS_SEL_BAYER);
+	fmt = imx_media_find_pixel_format(image->pix.pixelformat,
+					  CS_SEL_ANY | CS_SEL_BAYER);
 	if (!fmt)
 		return -EINVAL;
 
diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
index 0d83c2c41606..9dbf63796806 100644
--- a/drivers/staging/media/imx/imx-media-vdic.c
+++ b/drivers/staging/media/imx/imx-media-vdic.c
@@ -548,7 +548,7 @@ static int vdic_enum_mbus_code(struct v4l2_subdev *sd,
 	if (code->pad >= VDIC_NUM_PADS)
 		return -EINVAL;
 
-	return imx_media_enum_ipu_format(&code->code, code->index, CS_SEL_YUV);
+	return imx_media_enum_ipu_formats(&code->code, code->index, CS_SEL_YUV);
 }
 
 static int vdic_get_fmt(struct v4l2_subdev *sd,
@@ -587,7 +587,7 @@ static void vdic_try_fmt(struct vdic_priv *priv,
 	if (!*cc) {
 		u32 code;
 
-		imx_media_enum_ipu_format(&code, 0, CS_SEL_YUV);
+		imx_media_enum_ipu_formats(&code, 0, CS_SEL_YUV);
 		*cc = imx_media_find_ipu_format(code, CS_SEL_YUV);
 		sdformat->format.code = (*cc)->codes[0];
 	}
@@ -850,7 +850,7 @@ static int vdic_registered(struct v4l2_subdev *sd)
 	for (i = 0; i < VDIC_NUM_PADS; i++) {
 		code = 0;
 		if (i != VDIC_SINK_PAD_IDMAC)
-			imx_media_enum_ipu_format(&code, 0, CS_SEL_YUV);
+			imx_media_enum_ipu_formats(&code, 0, CS_SEL_YUV);
 
 		/* set a default mbus format  */
 		ret = imx_media_init_mbus_fmt(&priv->format_mbus[i],
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 9cedc27a6b4a..cd80f1c59a5c 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -158,14 +158,17 @@ enum codespace_sel {
 
 /* imx-media-utils.c */
 const struct imx_media_pixfmt *
-imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel);
-int imx_media_enum_format(u32 *fourcc, u32 index, enum codespace_sel cs_sel);
+imx_media_find_pixel_format(u32 fourcc, enum codespace_sel cs_sel);
+int imx_media_enum_pixel_formats(u32 *fourcc, u32 index,
+				 enum codespace_sel cs_sel);
 const struct imx_media_pixfmt *
 imx_media_find_mbus_format(u32 code, enum codespace_sel cs_sel);
-int imx_media_enum_mbus_format(u32 *code, u32 index, enum codespace_sel cs_sel);
+int imx_media_enum_mbus_formats(u32 *code, u32 index,
+				enum codespace_sel cs_sel);
 const struct imx_media_pixfmt *
 imx_media_find_ipu_format(u32 code, enum codespace_sel cs_sel);
-int imx_media_enum_ipu_format(u32 *code, u32 index, enum codespace_sel cs_sel);
+int imx_media_enum_ipu_formats(u32 *code, u32 index, enum codespace_sel cs_sel);
+
 int imx_media_init_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
 			    u32 width, u32 height, u32 code, u32 field,
 			    const struct imx_media_pixfmt **cc);
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 4f1f9e7dff95..f54f32fb78c7 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -958,8 +958,8 @@ static int imx7_csi_enum_mbus_code(struct v4l2_subdev *sd,
 
 	switch (code->pad) {
 	case IMX7_CSI_PAD_SINK:
-		ret = imx_media_enum_mbus_format(&code->code, code->index,
-						 CS_SEL_ANY | CS_SEL_BAYER);
+		ret = imx_media_enum_mbus_formats(&code->code, code->index,
+						  CS_SEL_ANY | CS_SEL_BAYER);
 		break;
 	case IMX7_CSI_PAD_SRC:
 		if (code->index != 0) {
@@ -1035,7 +1035,7 @@ static int imx7_csi_try_fmt(struct imx7_csi *csi,
 		*cc = imx_media_find_mbus_format(sdformat->format.code,
 						 CS_SEL_ANY | CS_SEL_BAYER);
 		if (!*cc) {
-			imx_media_enum_mbus_format(&code, 0, CS_SEL_ANY);
+			imx_media_enum_mbus_formats(&code, 0, CS_SEL_ANY);
 			*cc = imx_media_find_mbus_format(code, CS_SEL_ANY);
 			sdformat->format.code = (*cc)->codes[0];
 		}
-- 
Regards,

Laurent Pinchart

[PATCH 9/8] media: imx: utils: Constify mbus argument to imx_media_mbus_fmt_to_pix_fmt

[Laurent Pinchart]

The imx_media_mbus_fmt_to_pix_fmt() function doesn't need to modify its
mbus argument. Make it const.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/staging/media/imx/imx-media-utils.c | 2 +-
 drivers/staging/media/imx/imx-media.h       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 06415c8f707e..6cb2122d5f3f 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -566,7 +566,7 @@ void imx_media_try_colorimetry(struct v4l2_mbus_framefmt *tryfmt,
 EXPORT_SYMBOL_GPL(imx_media_try_colorimetry);
 
 int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
-				  struct v4l2_mbus_framefmt *mbus,
+				  const struct v4l2_mbus_framefmt *mbus,
 				  const struct imx_media_pixfmt *cc)
 {
 	u32 width;
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index cd80f1c59a5c..bb73a76eea84 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -177,7 +177,7 @@ int imx_media_init_cfg(struct v4l2_subdev *sd,
 void imx_media_try_colorimetry(struct v4l2_mbus_framefmt *tryfmt,
 			       bool ic_route);
 int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
-				  struct v4l2_mbus_framefmt *mbus,
+				  const struct v4l2_mbus_framefmt *mbus,
 				  const struct imx_media_pixfmt *cc);
 int imx_media_mbus_fmt_to_ipu_image(struct ipu_image *image,
 				    struct v4l2_mbus_framefmt *mbus);
-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/8] media: imx: Miscalleanous format-related cleanups

[Rui Miguel Silva]

Hi Laurent,
Thanks for the cleanups on this.
On Tue, Mar 10, 2020 at 06:18:37PM +0200, Laurent Pinchart wrote:
> Hello,
> 
> This patch series started as an attempt to fix the format get and set
> subdev operations on the i.MX7 CSI-2 receiver subdev, which it does in
> patch 1/8. Patch 2/8 further cleans up the format-related code in that
> subdev.

for the imx7 part:

Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>

------
Cheers,
     Rui
> 
> Patches 3/8 to 8/8 pushes the cleanups further as I was attempting to
> fix the format enumeration on the video node at the end of the pipeline.
> I realized as part of that effort that there's more work than I
> anticipated, and I'm currently evaluating the possible options.
> Nonetheless, I think the cleanups make sense even without what I wanted
> to build on top of them, so I'm sending them out already.
> 
> Laurent Pinchart (8):
>   media: imx: imx7-mipi-csis: Cleanup and fix subdev pad format handling
>   media: imx: imx7-mipi-csis: Centralize initialization of pad formats
>   media: imx: utils: Inline init_mbus_colorimetry() in its caller
>   media: imx: utils: Handle Bayer format lookup through a selection flag
>   media: imx: utils: Simplify IPU format lookup and enumeration
>   media: imx: utils: Make imx_media_pixfmt handle variable number of
>     codes
>   media: imx: utils: Remove unneeded argument to (find|enum)_format()
>   media: imx: utils: Rename format lookup and enumeration functions
> 
>  drivers/staging/media/imx/imx-ic-prp.c        |   8 +-
>  drivers/staging/media/imx/imx-ic-prpencvf.c   |   6 +-
>  drivers/staging/media/imx/imx-media-capture.c |  22 +-
>  .../staging/media/imx/imx-media-csc-scaler.c  |   2 +-
>  drivers/staging/media/imx/imx-media-csi.c     |  26 +-
>  drivers/staging/media/imx/imx-media-utils.c   | 313 ++++++++----------
>  drivers/staging/media/imx/imx-media-vdic.c    |   6 +-
>  drivers/staging/media/imx/imx-media.h         |  24 +-
>  drivers/staging/media/imx/imx7-media-csi.c    |  15 +-
>  drivers/staging/media/imx/imx7-mipi-csis.c    | 138 ++++----
>  10 files changed, 271 insertions(+), 289 deletions(-)
> 
> -- 
> Regards,
> 
> Laurent Pinchart
>
> 

Re: [PATCH 0/8] media: imx: Miscalleanous format-related cleanups

[Steve Longerbeam]

Hi Laurent,

I agree that the find/enum format code in imx-utils needs cleanup, it's 
too confusing. I will be ready to give my ack to the imx-utils patches 
once I do some smoke testing on a sabre target when I return from vacation.

Note that Phillip also submitted a fixup patch to the find/enum format 
code, IIRC it did more consolidating of the imx_media_pixfmt tables. I 
can't find it and it has gotten lost, but I tested and provided a 
reviewed-by at the time.

Steve


On 3/10/20 9:18 AM, Laurent Pinchart wrote:
> Hello,
>
> This patch series started as an attempt to fix the format get and set
> subdev operations on the i.MX7 CSI-2 receiver subdev, which it does in
> patch 1/8. Patch 2/8 further cleans up the format-related code in that
> subdev.
>
> Patches 3/8 to 8/8 pushes the cleanups further as I was attempting to
> fix the format enumeration on the video node at the end of the pipeline.
> I realized as part of that effort that there's more work than I
> anticipated, and I'm currently evaluating the possible options.
> Nonetheless, I think the cleanups make sense even without what I wanted
> to build on top of them, so I'm sending them out already.
>
> Laurent Pinchart (8):
>    media: imx: imx7-mipi-csis: Cleanup and fix subdev pad format handling
>    media: imx: imx7-mipi-csis: Centralize initialization of pad formats
>    media: imx: utils: Inline init_mbus_colorimetry() in its caller
>    media: imx: utils: Handle Bayer format lookup through a selection flag
>    media: imx: utils: Simplify IPU format lookup and enumeration
>    media: imx: utils: Make imx_media_pixfmt handle variable number of
>      codes
>    media: imx: utils: Remove unneeded argument to (find|enum)_format()
>    media: imx: utils: Rename format lookup and enumeration functions
>
>   drivers/staging/media/imx/imx-ic-prp.c        |   8 +-
>   drivers/staging/media/imx/imx-ic-prpencvf.c   |   6 +-
>   drivers/staging/media/imx/imx-media-capture.c |  22 +-
>   .../staging/media/imx/imx-media-csc-scaler.c  |   2 +-
>   drivers/staging/media/imx/imx-media-csi.c     |  26 +-
>   drivers/staging/media/imx/imx-media-utils.c   | 313 ++++++++----------
>   drivers/staging/media/imx/imx-media-vdic.c    |   6 +-
>   drivers/staging/media/imx/imx-media.h         |  24 +-
>   drivers/staging/media/imx/imx7-media-csi.c    |  15 +-
>   drivers/staging/media/imx/imx7-mipi-csis.c    | 138 ++++----
>   10 files changed, 271 insertions(+), 289 deletions(-)
>

Re: [PATCH 0/8] media: imx: Miscalleanous format-related cleanups

[Laurent Pinchart]

Hi Steve,

On Wed, Mar 11, 2020 at 05:16:49PM -0700, Steve Longerbeam wrote:
> Hi Laurent,
> 
> I agree that the find/enum format code in imx-utils needs cleanup, it's 
> too confusing. I will be ready to give my ack to the imx-utils patches 
> once I do some smoke testing on a sabre target when I return from vacation.
> 
> Note that Phillip also submitted a fixup patch to the find/enum format 
> code, IIRC it did more consolidating of the imx_media_pixfmt tables. I 
> can't find it and it has gotten lost, but I tested and provided a 
> reviewed-by at the time.

I've found them in the mail archive. There were 3 patches, Hans said he
would take the first two, but only the first one got merged. I'll take
the two others and build on top of them, fixing the issues pointed out
by the kbuild robot and addressing Hans concerns.

> On 3/10/20 9:18 AM, Laurent Pinchart wrote:
> > Hello,
> >
> > This patch series started as an attempt to fix the format get and set
> > subdev operations on the i.MX7 CSI-2 receiver subdev, which it does in
> > patch 1/8. Patch 2/8 further cleans up the format-related code in that
> > subdev.
> >
> > Patches 3/8 to 8/8 pushes the cleanups further as I was attempting to
> > fix the format enumeration on the video node at the end of the pipeline.
> > I realized as part of that effort that there's more work than I
> > anticipated, and I'm currently evaluating the possible options.
> > Nonetheless, I think the cleanups make sense even without what I wanted
> > to build on top of them, so I'm sending them out already.
> >
> > Laurent Pinchart (8):
> >    media: imx: imx7-mipi-csis: Cleanup and fix subdev pad format handling
> >    media: imx: imx7-mipi-csis: Centralize initialization of pad formats
> >    media: imx: utils: Inline init_mbus_colorimetry() in its caller
> >    media: imx: utils: Handle Bayer format lookup through a selection flag
> >    media: imx: utils: Simplify IPU format lookup and enumeration
> >    media: imx: utils: Make imx_media_pixfmt handle variable number of
> >      codes
> >    media: imx: utils: Remove unneeded argument to (find|enum)_format()
> >    media: imx: utils: Rename format lookup and enumeration functions
> >
> >   drivers/staging/media/imx/imx-ic-prp.c        |   8 +-
> >   drivers/staging/media/imx/imx-ic-prpencvf.c   |   6 +-
> >   drivers/staging/media/imx/imx-media-capture.c |  22 +-
> >   .../staging/media/imx/imx-media-csc-scaler.c  |   2 +-
> >   drivers/staging/media/imx/imx-media-csi.c     |  26 +-
> >   drivers/staging/media/imx/imx-media-utils.c   | 313 ++++++++----------
> >   drivers/staging/media/imx/imx-media-vdic.c    |   6 +-
> >   drivers/staging/media/imx/imx-media.h         |  24 +-
> >   drivers/staging/media/imx/imx7-media-csi.c    |  15 +-
> >   drivers/staging/media/imx/imx7-mipi-csis.c    | 138 ++++----
> >   10 files changed, 271 insertions(+), 289 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/8] media: imx: Miscalleanous format-related cleanups

[Steve Longerbeam]

On 3/11/20 5:47 PM, Laurent Pinchart wrote:
> Hi Steve,
>
> On Wed, Mar 11, 2020 at 05:16:49PM -0700, Steve Longerbeam wrote:
>> Hi Laurent,
>>
>> I agree that the find/enum format code in imx-utils needs cleanup, it's
>> too confusing. I will be ready to give my ack to the imx-utils patches
>> once I do some smoke testing on a sabre target when I return from vacation.
>>
>> Note that Phillip also submitted a fixup patch to the find/enum format
>> code, IIRC it did more consolidating of the imx_media_pixfmt tables. I
>> can't find it and it has gotten lost, but I tested and provided a
>> reviewed-by at the time.
> I've found them in the mail archive. There were 3 patches, Hans said he
> would take the first two, but only the first one got merged. I'll take
> the two others and build on top of them, fixing the issues pointed out
> by the kbuild robot and addressing Hans concerns.

Thanks!

Steve

>
>> On 3/10/20 9:18 AM, Laurent Pinchart wrote:
>>> Hello,
>>>
>>> This patch series started as an attempt to fix the format get and set
>>> subdev operations on the i.MX7 CSI-2 receiver subdev, which it does in
>>> patch 1/8. Patch 2/8 further cleans up the format-related code in that
>>> subdev.
>>>
>>> Patches 3/8 to 8/8 pushes the cleanups further as I was attempting to
>>> fix the format enumeration on the video node at the end of the pipeline.
>>> I realized as part of that effort that there's more work than I
>>> anticipated, and I'm currently evaluating the possible options.
>>> Nonetheless, I think the cleanups make sense even without what I wanted
>>> to build on top of them, so I'm sending them out already.
>>>
>>> Laurent Pinchart (8):
>>>     media: imx: imx7-mipi-csis: Cleanup and fix subdev pad format handling
>>>     media: imx: imx7-mipi-csis: Centralize initialization of pad formats
>>>     media: imx: utils: Inline init_mbus_colorimetry() in its caller
>>>     media: imx: utils: Handle Bayer format lookup through a selection flag
>>>     media: imx: utils: Simplify IPU format lookup and enumeration
>>>     media: imx: utils: Make imx_media_pixfmt handle variable number of
>>>       codes
>>>     media: imx: utils: Remove unneeded argument to (find|enum)_format()
>>>     media: imx: utils: Rename format lookup and enumeration functions
>>>
>>>    drivers/staging/media/imx/imx-ic-prp.c        |   8 +-
>>>    drivers/staging/media/imx/imx-ic-prpencvf.c   |   6 +-
>>>    drivers/staging/media/imx/imx-media-capture.c |  22 +-
>>>    .../staging/media/imx/imx-media-csc-scaler.c  |   2 +-
>>>    drivers/staging/media/imx/imx-media-csi.c     |  26 +-
>>>    drivers/staging/media/imx/imx-media-utils.c   | 313 ++++++++----------
>>>    drivers/staging/media/imx/imx-media-vdic.c    |   6 +-
>>>    drivers/staging/media/imx/imx-media.h         |  24 +-
>>>    drivers/staging/media/imx/imx7-media-csi.c    |  15 +-
>>>    drivers/staging/media/imx/imx7-mipi-csis.c    | 138 ++++----
>>>    10 files changed, 271 insertions(+), 289 deletions(-)

Re: [PATCH 0/8] media: imx: Miscalleanous format-related cleanups

[Steve Longerbeam]

Hi Laurent,

On 3/11/20 5:47 PM, Laurent Pinchart wrote:
> Hi Steve,
>
> On Wed, Mar 11, 2020 at 05:16:49PM -0700, Steve Longerbeam wrote:
>> Hi Laurent,
>>
>> I agree that the find/enum format code in imx-utils needs cleanup, it's
>> too confusing. I will be ready to give my ack to the imx-utils patches
>> once I do some smoke testing on a sabre target when I return from vacation.
>>
>> Note that Phillip also submitted a fixup patch to the find/enum format
>> code, IIRC it did more consolidating of the imx_media_pixfmt tables. I
>> can't find it and it has gotten lost, but I tested and provided a
>> reviewed-by at the time.
> I've found them in the mail archive. There were 3 patches, Hans said he
> would take the first two, but only the first one got merged. I'll take
> the two others and build on top of them, fixing the issues pointed out
> by the kbuild robot and addressing Hans concerns.

I found the thread finally. In fact I did some work on this set already, 
I fixed the kbuild warnings as well as added some function headers 
describing all the format search criteria arguments. Also changed a 
local var in enum_formats() to better document the enum algorithm.

Unless you've started on this work already, I will be returning from 
vaca tomorrow and can pick this up again, merging in your patches, as 
well as splitting up find|enum_formats() into find|enum_formats() and 
find|enum_codes(), which is the remaining suggestion from Hans.


Steve

>
>> On 3/10/20 9:18 AM, Laurent Pinchart wrote:
>>> Hello,
>>>
>>> This patch series started as an attempt to fix the format get and set
>>> subdev operations on the i.MX7 CSI-2 receiver subdev, which it does in
>>> patch 1/8. Patch 2/8 further cleans up the format-related code in that
>>> subdev.
>>>
>>> Patches 3/8 to 8/8 pushes the cleanups further as I was attempting to
>>> fix the format enumeration on the video node at the end of the pipeline.
>>> I realized as part of that effort that there's more work than I
>>> anticipated, and I'm currently evaluating the possible options.
>>> Nonetheless, I think the cleanups make sense even without what I wanted
>>> to build on top of them, so I'm sending them out already.
>>>
>>> Laurent Pinchart (8):
>>>     media: imx: imx7-mipi-csis: Cleanup and fix subdev pad format handling
>>>     media: imx: imx7-mipi-csis: Centralize initialization of pad formats
>>>     media: imx: utils: Inline init_mbus_colorimetry() in its caller
>>>     media: imx: utils: Handle Bayer format lookup through a selection flag
>>>     media: imx: utils: Simplify IPU format lookup and enumeration
>>>     media: imx: utils: Make imx_media_pixfmt handle variable number of
>>>       codes
>>>     media: imx: utils: Remove unneeded argument to (find|enum)_format()
>>>     media: imx: utils: Rename format lookup and enumeration functions
>>>
>>>    drivers/staging/media/imx/imx-ic-prp.c        |   8 +-
>>>    drivers/staging/media/imx/imx-ic-prpencvf.c   |   6 +-
>>>    drivers/staging/media/imx/imx-media-capture.c |  22 +-
>>>    .../staging/media/imx/imx-media-csc-scaler.c  |   2 +-
>>>    drivers/staging/media/imx/imx-media-csi.c     |  26 +-
>>>    drivers/staging/media/imx/imx-media-utils.c   | 313 ++++++++----------
>>>    drivers/staging/media/imx/imx-media-vdic.c    |   6 +-
>>>    drivers/staging/media/imx/imx-media.h         |  24 +-
>>>    drivers/staging/media/imx/imx7-media-csi.c    |  15 +-
>>>    drivers/staging/media/imx/imx7-mipi-csis.c    | 138 ++++----
>>>    10 files changed, 271 insertions(+), 289 deletions(-)

Re: [PATCH 0/8] media: imx: Miscalleanous format-related cleanups

[Laurent Pinchart]

Hi Steve,

On Sat, Mar 14, 2020 at 03:30:53PM -0700, Steve Longerbeam wrote:
> On 3/11/20 5:47 PM, Laurent Pinchart wrote:
> > On Wed, Mar 11, 2020 at 05:16:49PM -0700, Steve Longerbeam wrote:
> >> Hi Laurent,
> >>
> >> I agree that the find/enum format code in imx-utils needs cleanup, it's
> >> too confusing. I will be ready to give my ack to the imx-utils patches
> >> once I do some smoke testing on a sabre target when I return from vacation.
> >>
> >> Note that Phillip also submitted a fixup patch to the find/enum format
> >> code, IIRC it did more consolidating of the imx_media_pixfmt tables. I
> >> can't find it and it has gotten lost, but I tested and provided a
> >> reviewed-by at the time.
> >
> > I've found them in the mail archive. There were 3 patches, Hans said he
> > would take the first two, but only the first one got merged. I'll take
> > the two others and build on top of them, fixing the issues pointed out
> > by the kbuild robot and addressing Hans concerns.
> 
> I found the thread finally. In fact I did some work on this set already, 
> I fixed the kbuild warnings as well as added some function headers 
> describing all the format search criteria arguments. Also changed a 
> local var in enum_formats() to better document the enum algorithm.
> 
> Unless you've started on this work already, I will be returning from 
> vaca tomorrow and can pick this up again, merging in your patches, as 
> well as splitting up find|enum_formats() into find|enum_formats() and 
> find|enum_codes(), which is the remaining suggestion from Hans.

I've done this already, I'll try to send the patches on Monday.

> >> On 3/10/20 9:18 AM, Laurent Pinchart wrote:
> >>> Hello,
> >>>
> >>> This patch series started as an attempt to fix the format get and set
> >>> subdev operations on the i.MX7 CSI-2 receiver subdev, which it does in
> >>> patch 1/8. Patch 2/8 further cleans up the format-related code in that
> >>> subdev.
> >>>
> >>> Patches 3/8 to 8/8 pushes the cleanups further as I was attempting to
> >>> fix the format enumeration on the video node at the end of the pipeline.
> >>> I realized as part of that effort that there's more work than I
> >>> anticipated, and I'm currently evaluating the possible options.
> >>> Nonetheless, I think the cleanups make sense even without what I wanted
> >>> to build on top of them, so I'm sending them out already.
> >>>
> >>> Laurent Pinchart (8):
> >>>     media: imx: imx7-mipi-csis: Cleanup and fix subdev pad format handling
> >>>     media: imx: imx7-mipi-csis: Centralize initialization of pad formats
> >>>     media: imx: utils: Inline init_mbus_colorimetry() in its caller
> >>>     media: imx: utils: Handle Bayer format lookup through a selection flag
> >>>     media: imx: utils: Simplify IPU format lookup and enumeration
> >>>     media: imx: utils: Make imx_media_pixfmt handle variable number of
> >>>       codes
> >>>     media: imx: utils: Remove unneeded argument to (find|enum)_format()
> >>>     media: imx: utils: Rename format lookup and enumeration functions
> >>>
> >>>    drivers/staging/media/imx/imx-ic-prp.c        |   8 +-
> >>>    drivers/staging/media/imx/imx-ic-prpencvf.c   |   6 +-
> >>>    drivers/staging/media/imx/imx-media-capture.c |  22 +-
> >>>    .../staging/media/imx/imx-media-csc-scaler.c  |   2 +-
> >>>    drivers/staging/media/imx/imx-media-csi.c     |  26 +-
> >>>    drivers/staging/media/imx/imx-media-utils.c   | 313 ++++++++----------
> >>>    drivers/staging/media/imx/imx-media-vdic.c    |   6 +-
> >>>    drivers/staging/media/imx/imx-media.h         |  24 +-
> >>>    drivers/staging/media/imx/imx7-media-csi.c    |  15 +-
> >>>    drivers/staging/media/imx/imx7-mipi-csis.c    | 138 ++++----
> >>>    10 files changed, 271 insertions(+), 289 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/8] media: imx: Miscalleanous format-related cleanups

[Steve Longerbeam]

On 3/14/20 3:32 PM, Laurent Pinchart wrote:
> Hi Steve,
>
> On Sat, Mar 14, 2020 at 03:30:53PM -0700, Steve Longerbeam wrote:
>> On 3/11/20 5:47 PM, Laurent Pinchart wrote:
>>> On Wed, Mar 11, 2020 at 05:16:49PM -0700, Steve Longerbeam wrote:
>>>> Hi Laurent,
>>>>
>>>> I agree that the find/enum format code in imx-utils needs cleanup, it's
>>>> too confusing. I will be ready to give my ack to the imx-utils patches
>>>> once I do some smoke testing on a sabre target when I return from vacation.
>>>>
>>>> Note that Phillip also submitted a fixup patch to the find/enum format
>>>> code, IIRC it did more consolidating of the imx_media_pixfmt tables. I
>>>> can't find it and it has gotten lost, but I tested and provided a
>>>> reviewed-by at the time.
>>> I've found them in the mail archive. There were 3 patches, Hans said he
>>> would take the first two, but only the first one got merged. I'll take
>>> the two others and build on top of them, fixing the issues pointed out
>>> by the kbuild robot and addressing Hans concerns.
>> I found the thread finally. In fact I did some work on this set already,
>> I fixed the kbuild warnings as well as added some function headers
>> describing all the format search criteria arguments. Also changed a
>> local var in enum_formats() to better document the enum algorithm.
>>
>> Unless you've started on this work already, I will be returning from
>> vaca tomorrow and can pick this up again, merging in your patches, as
>> well as splitting up find|enum_formats() into find|enum_formats() and
>> find|enum_codes(), which is the remaining suggestion from Hans.
> I've done this already, I'll try to send the patches on Monday.

Ok great. Thanks for the work.

Steve

>>>> On 3/10/20 9:18 AM, Laurent Pinchart wrote:
>>>>> Hello,
>>>>>
>>>>> This patch series started as an attempt to fix the format get and set
>>>>> subdev operations on the i.MX7 CSI-2 receiver subdev, which it does in
>>>>> patch 1/8. Patch 2/8 further cleans up the format-related code in that
>>>>> subdev.
>>>>>
>>>>> Patches 3/8 to 8/8 pushes the cleanups further as I was attempting to
>>>>> fix the format enumeration on the video node at the end of the pipeline.
>>>>> I realized as part of that effort that there's more work than I
>>>>> anticipated, and I'm currently evaluating the possible options.
>>>>> Nonetheless, I think the cleanups make sense even without what I wanted
>>>>> to build on top of them, so I'm sending them out already.
>>>>>
>>>>> Laurent Pinchart (8):
>>>>>      media: imx: imx7-mipi-csis: Cleanup and fix subdev pad format handling
>>>>>      media: imx: imx7-mipi-csis: Centralize initialization of pad formats
>>>>>      media: imx: utils: Inline init_mbus_colorimetry() in its caller
>>>>>      media: imx: utils: Handle Bayer format lookup through a selection flag
>>>>>      media: imx: utils: Simplify IPU format lookup and enumeration
>>>>>      media: imx: utils: Make imx_media_pixfmt handle variable number of
>>>>>        codes
>>>>>      media: imx: utils: Remove unneeded argument to (find|enum)_format()
>>>>>      media: imx: utils: Rename format lookup and enumeration functions
>>>>>
>>>>>     drivers/staging/media/imx/imx-ic-prp.c        |   8 +-
>>>>>     drivers/staging/media/imx/imx-ic-prpencvf.c   |   6 +-
>>>>>     drivers/staging/media/imx/imx-media-capture.c |  22 +-
>>>>>     .../staging/media/imx/imx-media-csc-scaler.c  |   2 +-
>>>>>     drivers/staging/media/imx/imx-media-csi.c     |  26 +-
>>>>>     drivers/staging/media/imx/imx-media-utils.c   | 313 ++++++++----------
>>>>>     drivers/staging/media/imx/imx-media-vdic.c    |   6 +-
>>>>>     drivers/staging/media/imx/imx-media.h         |  24 +-
>>>>>     drivers/staging/media/imx/imx7-media-csi.c    |  15 +-
>>>>>     drivers/staging/media/imx/imx7-mipi-csis.c    | 138 ++++----
>>>>>     10 files changed, 271 insertions(+), 289 deletions(-)

Re: [PATCH 0/8] media: imx: Miscalleanous format-related cleanups

[Steve Longerbeam]

Hi Laurent,

On 3/14/20 3:32 PM, Laurent Pinchart wrote:
> Hi Steve,
>
> On Sat, Mar 14, 2020 at 03:30:53PM -0700, Steve Longerbeam wrote:
>> On 3/11/20 5:47 PM, Laurent Pinchart wrote:
>>> On Wed, Mar 11, 2020 at 05:16:49PM -0700, Steve Longerbeam wrote:
>>>> Hi Laurent,
>>>>
>>>> I agree that the find/enum format code in imx-utils needs cleanup, it's
>>>> too confusing. I will be ready to give my ack to the imx-utils patches
>>>> once I do some smoke testing on a sabre target when I return from vacation.
>>>>
>>>> Note that Phillip also submitted a fixup patch to the find/enum format
>>>> code, IIRC it did more consolidating of the imx_media_pixfmt tables. I
>>>> can't find it and it has gotten lost, but I tested and provided a
>>>> reviewed-by at the time.
>>> I've found them in the mail archive. There were 3 patches, Hans said he
>>> would take the first two, but only the first one got merged. I'll take
>>> the two others and build on top of them, fixing the issues pointed out
>>> by the kbuild robot and addressing Hans concerns.
>> I found the thread finally. In fact I did some work on this set already,
>> I fixed the kbuild warnings as well as added some function headers
>> describing all the format search criteria arguments. Also changed a
>> local var in enum_formats() to better document the enum algorithm.
>>
>> Unless you've started on this work already, I will be returning from
>> vaca tomorrow and can pick this up again, merging in your patches, as
>> well as splitting up find|enum_formats() into find|enum_formats() and
>> find|enum_codes(), which is the remaining suggestion from Hans.
> I've done this already, I'll try to send the patches on Monday.

If you don't mind, I've done this work and tested on an imx6 target. So 
I will post a series.

Steve

>
>>>> On 3/10/20 9:18 AM, Laurent Pinchart wrote:
>>>>> Hello,
>>>>>
>>>>> This patch series started as an attempt to fix the format get and set
>>>>> subdev operations on the i.MX7 CSI-2 receiver subdev, which it does in
>>>>> patch 1/8. Patch 2/8 further cleans up the format-related code in that
>>>>> subdev.
>>>>>
>>>>> Patches 3/8 to 8/8 pushes the cleanups further as I was attempting to
>>>>> fix the format enumeration on the video node at the end of the pipeline.
>>>>> I realized as part of that effort that there's more work than I
>>>>> anticipated, and I'm currently evaluating the possible options.
>>>>> Nonetheless, I think the cleanups make sense even without what I wanted
>>>>> to build on top of them, so I'm sending them out already.
>>>>>
>>>>> Laurent Pinchart (8):
>>>>>      media: imx: imx7-mipi-csis: Cleanup and fix subdev pad format handling
>>>>>      media: imx: imx7-mipi-csis: Centralize initialization of pad formats
>>>>>      media: imx: utils: Inline init_mbus_colorimetry() in its caller
>>>>>      media: imx: utils: Handle Bayer format lookup through a selection flag
>>>>>      media: imx: utils: Simplify IPU format lookup and enumeration
>>>>>      media: imx: utils: Make imx_media_pixfmt handle variable number of
>>>>>        codes
>>>>>      media: imx: utils: Remove unneeded argument to (find|enum)_format()
>>>>>      media: imx: utils: Rename format lookup and enumeration functions
>>>>>
>>>>>     drivers/staging/media/imx/imx-ic-prp.c        |   8 +-
>>>>>     drivers/staging/media/imx/imx-ic-prpencvf.c   |   6 +-
>>>>>     drivers/staging/media/imx/imx-media-capture.c |  22 +-
>>>>>     .../staging/media/imx/imx-media-csc-scaler.c  |   2 +-
>>>>>     drivers/staging/media/imx/imx-media-csi.c     |  26 +-
>>>>>     drivers/staging/media/imx/imx-media-utils.c   | 313 ++++++++----------
>>>>>     drivers/staging/media/imx/imx-media-vdic.c    |   6 +-
>>>>>     drivers/staging/media/imx/imx-media.h         |  24 +-
>>>>>     drivers/staging/media/imx/imx7-media-csi.c    |  15 +-
>>>>>     drivers/staging/media/imx/imx7-mipi-csis.c    | 138 ++++----
>>>>>     10 files changed, 271 insertions(+), 289 deletions(-)

Re: [PATCH 0/8] media: imx: Miscalleanous format-related cleanups

[Laurent Pinchart]

Hi Steve,

On Thu, Mar 26, 2020 at 12:05:38PM -0700, Steve Longerbeam wrote:
> On 3/14/20 3:32 PM, Laurent Pinchart wrote:
> > On Sat, Mar 14, 2020 at 03:30:53PM -0700, Steve Longerbeam wrote:
> >> On 3/11/20 5:47 PM, Laurent Pinchart wrote:
> >>> On Wed, Mar 11, 2020 at 05:16:49PM -0700, Steve Longerbeam wrote:
> >>>> Hi Laurent,
> >>>>
> >>>> I agree that the find/enum format code in imx-utils needs cleanup, it's
> >>>> too confusing. I will be ready to give my ack to the imx-utils patches
> >>>> once I do some smoke testing on a sabre target when I return from vacation.
> >>>>
> >>>> Note that Phillip also submitted a fixup patch to the find/enum format
> >>>> code, IIRC it did more consolidating of the imx_media_pixfmt tables. I
> >>>> can't find it and it has gotten lost, but I tested and provided a
> >>>> reviewed-by at the time.
> >>>
> >>> I've found them in the mail archive. There were 3 patches, Hans said he
> >>> would take the first two, but only the first one got merged. I'll take
> >>> the two others and build on top of them, fixing the issues pointed out
> >>> by the kbuild robot and addressing Hans concerns.
> >>
> >> I found the thread finally. In fact I did some work on this set already,
> >> I fixed the kbuild warnings as well as added some function headers
> >> describing all the format search criteria arguments. Also changed a
> >> local var in enum_formats() to better document the enum algorithm.
> >>
> >> Unless you've started on this work already, I will be returning from
> >> vaca tomorrow and can pick this up again, merging in your patches, as
> >> well as splitting up find|enum_formats() into find|enum_formats() and
> >> find|enum_codes(), which is the remaining suggestion from Hans.
> > I've done this already, I'll try to send the patches on Monday.
> 
> If you don't mind, I've done this work and tested on an imx6 target. So 
> I will post a series.

No worries. I've been a bit delayed indeed. I'll still post my cleanups,
in case they contain anything useful in addition to yours.

> >>>> On 3/10/20 9:18 AM, Laurent Pinchart wrote:
> >>>>> Hello,
> >>>>>
> >>>>> This patch series started as an attempt to fix the format get and set
> >>>>> subdev operations on the i.MX7 CSI-2 receiver subdev, which it does in
> >>>>> patch 1/8. Patch 2/8 further cleans up the format-related code in that
> >>>>> subdev.
> >>>>>
> >>>>> Patches 3/8 to 8/8 pushes the cleanups further as I was attempting to
> >>>>> fix the format enumeration on the video node at the end of the pipeline.
> >>>>> I realized as part of that effort that there's more work than I
> >>>>> anticipated, and I'm currently evaluating the possible options.
> >>>>> Nonetheless, I think the cleanups make sense even without what I wanted
> >>>>> to build on top of them, so I'm sending them out already.
> >>>>>
> >>>>> Laurent Pinchart (8):
> >>>>>      media: imx: imx7-mipi-csis: Cleanup and fix subdev pad format handling
> >>>>>      media: imx: imx7-mipi-csis: Centralize initialization of pad formats
> >>>>>      media: imx: utils: Inline init_mbus_colorimetry() in its caller
> >>>>>      media: imx: utils: Handle Bayer format lookup through a selection flag
> >>>>>      media: imx: utils: Simplify IPU format lookup and enumeration
> >>>>>      media: imx: utils: Make imx_media_pixfmt handle variable number of
> >>>>>        codes
> >>>>>      media: imx: utils: Remove unneeded argument to (find|enum)_format()
> >>>>>      media: imx: utils: Rename format lookup and enumeration functions
> >>>>>
> >>>>>     drivers/staging/media/imx/imx-ic-prp.c        |   8 +-
> >>>>>     drivers/staging/media/imx/imx-ic-prpencvf.c   |   6 +-
> >>>>>     drivers/staging/media/imx/imx-media-capture.c |  22 +-
> >>>>>     .../staging/media/imx/imx-media-csc-scaler.c  |   2 +-
> >>>>>     drivers/staging/media/imx/imx-media-csi.c     |  26 +-
> >>>>>     drivers/staging/media/imx/imx-media-utils.c   | 313 ++++++++----------
> >>>>>     drivers/staging/media/imx/imx-media-vdic.c    |   6 +-
> >>>>>     drivers/staging/media/imx/imx-media.h         |  24 +-
> >>>>>     drivers/staging/media/imx/imx7-media-csi.c    |  15 +-
> >>>>>     drivers/staging/media/imx/imx7-mipi-csis.c    | 138 ++++----
> >>>>>     10 files changed, 271 insertions(+), 289 deletions(-)

-- 
Regards,

Laurent Pinchart