diff mbox series

[v6,08/11] media: imx: utils: Split find|enum_format into fourcc and mbus functions

Message ID 20200404224130.23118-9-slongerbeam@gmail.com (mailing list archive)
State New, archived
Headers show
Series media: imx: Miscellaneous format-related cleanups | expand

Commit Message

Steve Longerbeam April 4, 2020, 10:41 p.m. UTC 
To make the code easier to follow, split up find_format() into separate
search functions for pixel formats and media-bus codes, and inline
find_format() into the exported functions imx_media_find_format()
and imx_media_find_mbus_format().

Do the equivalent for enum_format().

Also add comment blocks for the exported find|enum functions.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx-media-utils.c | 148 +++++++++++++-------
 1 file changed, 100 insertions(+), 48 deletions(-)

Comments

Laurent Pinchart April 5, 2020, 5:16 p.m. UTC | #1 
Hi Steve,

Thank you for the patch.

On Sat, Apr 04, 2020 at 03:41:27PM -0700, Steve Longerbeam wrote:
> To make the code easier to follow, split up find_format() into separate
> search functions for pixel formats and media-bus codes, and inline
> find_format() into the exported functions imx_media_find_format()
> and imx_media_find_mbus_format().
> 
> Do the equivalent for enum_format().
> 
> Also add comment blocks for the exported find|enum functions.
> 
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
>  drivers/staging/media/imx/imx-media-utils.c | 148 +++++++++++++-------
>  1 file changed, 100 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
> index bd8ebbf0b26d..a18d826996d1 100644
> --- a/drivers/staging/media/imx/imx-media-utils.c
> +++ b/drivers/staging/media/imx/imx-media-utils.c
> @@ -199,10 +199,15 @@ static const struct imx_media_pixfmt pixel_formats[] = {
>  	},
>  };
>  
> -static const struct imx_media_pixfmt *find_format(u32 fourcc,
> -						  u32 code,
> -						  enum imx_pixfmt_sel fmt_sel,
> -						  bool allow_non_mbus)
> +/*
> + * Search for and return an entry in the pixel_formats[] array that matches
> + * the requested selection criteria.

To differentiate this from the next function, I would write

 * Search in the pixel_formats[] array for an entry with the given fourcc that
 * matches the requested selection criteria and return it.

> + *
> + * @fourcc: Search for an entry with the given fourcc pixel format.
> + * @fmt_sel: Allow entries only with the given selection criteria.
> + */
> +const struct imx_media_pixfmt *
> +imx_media_find_format(u32 fourcc, enum imx_pixfmt_sel fmt_sel)
>  {
>  	bool sel_ipu = fmt_sel & PIXFMT_SEL_IPU;
>  	unsigned int i;
> @@ -212,7 +217,6 @@ static const struct imx_media_pixfmt *find_format(u32 fourcc,
>  	for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) {
>  		const struct imx_media_pixfmt *fmt = &pixel_formats[i];
>  		enum imx_pixfmt_sel sel;
> -		unsigned int j;
>  
>  		if (sel_ipu != fmt->ipufmt)
>  			continue;
> @@ -221,14 +225,42 @@ static const struct imx_media_pixfmt *find_format(u32 fourcc,
>  			((fmt->cs == IPUV3_COLORSPACE_YUV) ?
>  			 PIXFMT_SEL_YUV : PIXFMT_SEL_RGB);
>  
> -		if (!(fmt_sel & sel) ||
> -		    (!allow_non_mbus && !fmt->codes))
> +		if ((fmt_sel & sel) && fmt->fourcc == fourcc)
> +			return fmt;
> +	}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(imx_media_find_format);
> +
> +/*
> + * Search for and return an entry in the pixel_formats[] array that matches
> + * the requested selection criteria.

And here

 * Search in the pixel_formats[] array for an entry with the given media bus
 * code that matches the requested selection criteria and return it.

> + *
> + * @code: Search for an entry with the given media-bus code.
> + * @fmt_sel: Allow entries only with the given selection criteria.
> + */
> +const struct imx_media_pixfmt *
> +imx_media_find_mbus_format(u32 code, enum imx_pixfmt_sel fmt_sel)
> +{
> +	bool sel_ipu = fmt_sel & PIXFMT_SEL_IPU;
> +	unsigned int i;
> +
> +	fmt_sel &= ~PIXFMT_SEL_IPU;
> +
> +	for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) {
> +		const struct imx_media_pixfmt *fmt = &pixel_formats[i];
> +		enum imx_pixfmt_sel sel;
> +		unsigned int j;
> +
> +		if (sel_ipu != fmt->ipufmt)
>  			continue;
>  
> -		if (fourcc && fmt->fourcc == fourcc)
> -			return fmt;
> +		sel = fmt->bayer ? PIXFMT_SEL_BAYER :
> +			((fmt->cs == IPUV3_COLORSPACE_YUV) ?
> +			 PIXFMT_SEL_YUV : PIXFMT_SEL_RGB);
>  
> -		if (!code || !fmt->codes)
> +		if (!(fmt_sel & sel) || !fmt->codes)
>  			continue;
>  
>  		for (j = 0; fmt->codes[j]; j++) {
> @@ -239,10 +271,21 @@ static const struct imx_media_pixfmt *find_format(u32 fourcc,
>  
>  	return NULL;
>  }
> +EXPORT_SYMBOL_GPL(imx_media_find_mbus_format);
>  
> -static int enum_format(u32 *fourcc, u32 *code, u32 index,
> -		       enum imx_pixfmt_sel fmt_sel,
> -		       bool allow_non_mbus)
> +/*
> + * Enumerate entries in the pixel_formats[] array that match the
> + * requested selection criteria. Returns the fourcc that matches the

s/Returns/Return/

> + * selection criteria at the requested match index.
> + *
> + * @fourcc: The returned fourcc that matches the search criteria at
> + *          the requested match index.
> + * @index: The requested match index.
> + * @fmt_sel: Include in the enumeration entries with the given selection
> + *           criteria.
> + */
> +int imx_media_enum_format(u32 *fourcc, u32 index,
> +			  enum imx_pixfmt_sel fmt_sel)
>  {
>  	bool sel_ipu = fmt_sel & PIXFMT_SEL_IPU;
>  	unsigned int i;
> @@ -252,7 +295,6 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
>  	for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) {
>  		const struct imx_media_pixfmt *fmt = &pixel_formats[i];
>  		enum imx_pixfmt_sel sel;
> -		unsigned int j;
>  
>  		if (sel_ipu != fmt->ipufmt)
>  			continue;
> @@ -261,19 +303,54 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
>  			((fmt->cs == IPUV3_COLORSPACE_YUV) ?
>  			 PIXFMT_SEL_YUV : PIXFMT_SEL_RGB);
>  
> -		if (!(fmt_sel & sel) ||
> -		    (!allow_non_mbus && !fmt->codes))
> +		if (!(fmt_sel & sel))
>  			continue;
>  
> -		if (fourcc && index == 0) {
> +		if (index == 0) {
>  			*fourcc = fmt->fourcc;
>  			return 0;
>  		}
>  
> -		if (!code) {
> -			index--;
> +		index--;
> +	}
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(imx_media_enum_format);
> +
> +/*
> + * Enumerate entries in the pixel_formats[] array that match the
> + * requested search criteria. Returns the media-bus code that matches

s/Returns/Return/

> + * the search criteria at the requested match index.
> + *
> + * @code: The returned media-bus code that matches the search criteria at
> + *        the requested match index.
> + * @index: The requested match index.
> + * @fmt_sel: Include in the enumeration entries with the given selection
> + *           criteria.
> + */
> +int imx_media_enum_mbus_format(u32 *code, u32 index,
> +			       enum imx_pixfmt_sel fmt_sel)
> +{
> +	bool sel_ipu = fmt_sel & PIXFMT_SEL_IPU;
> +	unsigned int i;
> +
> +	fmt_sel &= ~PIXFMT_SEL_IPU;
> +
> +	for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) {
> +		const struct imx_media_pixfmt *fmt = &pixel_formats[i];
> +		enum imx_pixfmt_sel sel;
> +		unsigned int j;
> +
> +		if (sel_ipu != fmt->ipufmt)
> +			continue;
> +
> +		sel = fmt->bayer ? PIXFMT_SEL_BAYER :
> +			((fmt->cs == IPUV3_COLORSPACE_YUV) ?
> +			 PIXFMT_SEL_YUV : PIXFMT_SEL_RGB);
> +
> +		if (!(fmt_sel & sel) || !fmt->codes)
>  			continue;
> -		}
>  
>  		for (j = 0; fmt->codes[j]; j++) {
>  			if (index == 0) {
> @@ -287,45 +364,20 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
>  
>  	return -EINVAL;
>  }
> -
> -const struct imx_media_pixfmt *
> -imx_media_find_format(u32 fourcc, enum imx_pixfmt_sel fmt_sel)
> -{
> -	return find_format(fourcc, 0, fmt_sel, true);
> -}
> -EXPORT_SYMBOL_GPL(imx_media_find_format);
> -
> -int imx_media_enum_format(u32 *fourcc, u32 index, enum imx_pixfmt_sel fmt_sel)
> -{
> -	return enum_format(fourcc, NULL, index, fmt_sel, true);
> -}
> -EXPORT_SYMBOL_GPL(imx_media_enum_format);
> -
> -const struct imx_media_pixfmt *
> -imx_media_find_mbus_format(u32 code, enum imx_pixfmt_sel fmt_sel)
> -{
> -	return find_format(0, code, fmt_sel, false);
> -}
> -EXPORT_SYMBOL_GPL(imx_media_find_mbus_format);
> -
> -int imx_media_enum_mbus_format(u32 *code, u32 index,
> -			       enum imx_pixfmt_sel fmt_sel)
> -{
> -	return enum_format(NULL, code, index, fmt_sel, false);
> -}
>  EXPORT_SYMBOL_GPL(imx_media_enum_mbus_format);
>  
>  const struct imx_media_pixfmt *
>  imx_media_find_ipu_format(u32 code, enum imx_pixfmt_sel fmt_sel)
>  {
> -	return find_format(0, code, fmt_sel | PIXFMT_SEL_IPU, false);
> +	return imx_media_find_mbus_format(code, fmt_sel | PIXFMT_SEL_IPU);
>  }
>  EXPORT_SYMBOL_GPL(imx_media_find_ipu_format);
>  
>  int imx_media_enum_ipu_format(u32 *code, u32 index,
>  			      enum imx_pixfmt_sel fmt_sel)
>  {
> -	return enum_format(NULL, code, index, fmt_sel | PIXFMT_SEL_IPU, false);
> +	return imx_media_enum_mbus_format(code, index,
> +					  fmt_sel | PIXFMT_SEL_IPU);
>  }
>  EXPORT_SYMBOL_GPL(imx_media_enum_ipu_format);

You could turn those two functions into static inlines in imx-media.h.
Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
diff mbox series

Patch

diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index bd8ebbf0b26d..a18d826996d1 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -199,10 +199,15 @@  static const struct imx_media_pixfmt pixel_formats[] = {
 	},
 };
 
-static const struct imx_media_pixfmt *find_format(u32 fourcc,
-						  u32 code,
-						  enum imx_pixfmt_sel fmt_sel,
-						  bool allow_non_mbus)
+/*
+ * Search for and return an entry in the pixel_formats[] array that matches
+ * the requested selection criteria.
+ *
+ * @fourcc: Search for an entry with the given fourcc pixel format.
+ * @fmt_sel: Allow entries only with the given selection criteria.
+ */
+const struct imx_media_pixfmt *
+imx_media_find_format(u32 fourcc, enum imx_pixfmt_sel fmt_sel)
 {
 	bool sel_ipu = fmt_sel & PIXFMT_SEL_IPU;
 	unsigned int i;
@@ -212,7 +217,6 @@  static const struct imx_media_pixfmt *find_format(u32 fourcc,
 	for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) {
 		const struct imx_media_pixfmt *fmt = &pixel_formats[i];
 		enum imx_pixfmt_sel sel;
-		unsigned int j;
 
 		if (sel_ipu != fmt->ipufmt)
 			continue;
@@ -221,14 +225,42 @@  static const struct imx_media_pixfmt *find_format(u32 fourcc,
 			((fmt->cs == IPUV3_COLORSPACE_YUV) ?
 			 PIXFMT_SEL_YUV : PIXFMT_SEL_RGB);
 
-		if (!(fmt_sel & sel) ||
-		    (!allow_non_mbus && !fmt->codes))
+		if ((fmt_sel & sel) && fmt->fourcc == fourcc)
+			return fmt;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(imx_media_find_format);
+
+/*
+ * Search for and return an entry in the pixel_formats[] array that matches
+ * the requested selection criteria.
+ *
+ * @code: Search for an entry with the given media-bus code.
+ * @fmt_sel: Allow entries only with the given selection criteria.
+ */
+const struct imx_media_pixfmt *
+imx_media_find_mbus_format(u32 code, enum imx_pixfmt_sel fmt_sel)
+{
+	bool sel_ipu = fmt_sel & PIXFMT_SEL_IPU;
+	unsigned int i;
+
+	fmt_sel &= ~PIXFMT_SEL_IPU;
+
+	for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) {
+		const struct imx_media_pixfmt *fmt = &pixel_formats[i];
+		enum imx_pixfmt_sel sel;
+		unsigned int j;
+
+		if (sel_ipu != fmt->ipufmt)
 			continue;
 
-		if (fourcc && fmt->fourcc == fourcc)
-			return fmt;
+		sel = fmt->bayer ? PIXFMT_SEL_BAYER :
+			((fmt->cs == IPUV3_COLORSPACE_YUV) ?
+			 PIXFMT_SEL_YUV : PIXFMT_SEL_RGB);
 
-		if (!code || !fmt->codes)
+		if (!(fmt_sel & sel) || !fmt->codes)
 			continue;
 
 		for (j = 0; fmt->codes[j]; j++) {
@@ -239,10 +271,21 @@  static const struct imx_media_pixfmt *find_format(u32 fourcc,
 
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(imx_media_find_mbus_format);
 
-static int enum_format(u32 *fourcc, u32 *code, u32 index,
-		       enum imx_pixfmt_sel fmt_sel,
-		       bool allow_non_mbus)
+/*
+ * Enumerate entries in the pixel_formats[] array that match the
+ * requested selection criteria. Returns the fourcc that matches the
+ * selection criteria at the requested match index.
+ *
+ * @fourcc: The returned fourcc that matches the search criteria at
+ *          the requested match index.
+ * @index: The requested match index.
+ * @fmt_sel: Include in the enumeration entries with the given selection
+ *           criteria.
+ */
+int imx_media_enum_format(u32 *fourcc, u32 index,
+			  enum imx_pixfmt_sel fmt_sel)
 {
 	bool sel_ipu = fmt_sel & PIXFMT_SEL_IPU;
 	unsigned int i;
@@ -252,7 +295,6 @@  static int enum_format(u32 *fourcc, u32 *code, u32 index,
 	for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) {
 		const struct imx_media_pixfmt *fmt = &pixel_formats[i];
 		enum imx_pixfmt_sel sel;
-		unsigned int j;
 
 		if (sel_ipu != fmt->ipufmt)
 			continue;
@@ -261,19 +303,54 @@  static int enum_format(u32 *fourcc, u32 *code, u32 index,
 			((fmt->cs == IPUV3_COLORSPACE_YUV) ?
 			 PIXFMT_SEL_YUV : PIXFMT_SEL_RGB);
 
-		if (!(fmt_sel & sel) ||
-		    (!allow_non_mbus && !fmt->codes))
+		if (!(fmt_sel & sel))
 			continue;
 
-		if (fourcc && index == 0) {
+		if (index == 0) {
 			*fourcc = fmt->fourcc;
 			return 0;
 		}
 
-		if (!code) {
-			index--;
+		index--;
+	}
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(imx_media_enum_format);
+
+/*
+ * Enumerate entries in the pixel_formats[] array that match the
+ * requested search criteria. Returns the media-bus code that matches
+ * the search criteria at the requested match index.
+ *
+ * @code: The returned media-bus code that matches the search criteria at
+ *        the requested match index.
+ * @index: The requested match index.
+ * @fmt_sel: Include in the enumeration entries with the given selection
+ *           criteria.
+ */
+int imx_media_enum_mbus_format(u32 *code, u32 index,
+			       enum imx_pixfmt_sel fmt_sel)
+{
+	bool sel_ipu = fmt_sel & PIXFMT_SEL_IPU;
+	unsigned int i;
+
+	fmt_sel &= ~PIXFMT_SEL_IPU;
+
+	for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) {
+		const struct imx_media_pixfmt *fmt = &pixel_formats[i];
+		enum imx_pixfmt_sel sel;
+		unsigned int j;
+
+		if (sel_ipu != fmt->ipufmt)
+			continue;
+
+		sel = fmt->bayer ? PIXFMT_SEL_BAYER :
+			((fmt->cs == IPUV3_COLORSPACE_YUV) ?
+			 PIXFMT_SEL_YUV : PIXFMT_SEL_RGB);
+
+		if (!(fmt_sel & sel) || !fmt->codes)
 			continue;
-		}
 
 		for (j = 0; fmt->codes[j]; j++) {
 			if (index == 0) {
@@ -287,45 +364,20 @@  static int enum_format(u32 *fourcc, u32 *code, u32 index,
 
 	return -EINVAL;
 }
-
-const struct imx_media_pixfmt *
-imx_media_find_format(u32 fourcc, enum imx_pixfmt_sel fmt_sel)
-{
-	return find_format(fourcc, 0, fmt_sel, true);
-}
-EXPORT_SYMBOL_GPL(imx_media_find_format);
-
-int imx_media_enum_format(u32 *fourcc, u32 index, enum imx_pixfmt_sel fmt_sel)
-{
-	return enum_format(fourcc, NULL, index, fmt_sel, true);
-}
-EXPORT_SYMBOL_GPL(imx_media_enum_format);
-
-const struct imx_media_pixfmt *
-imx_media_find_mbus_format(u32 code, enum imx_pixfmt_sel fmt_sel)
-{
-	return find_format(0, code, fmt_sel, false);
-}
-EXPORT_SYMBOL_GPL(imx_media_find_mbus_format);
-
-int imx_media_enum_mbus_format(u32 *code, u32 index,
-			       enum imx_pixfmt_sel fmt_sel)
-{
-	return enum_format(NULL, code, index, fmt_sel, false);
-}
 EXPORT_SYMBOL_GPL(imx_media_enum_mbus_format);
 
 const struct imx_media_pixfmt *
 imx_media_find_ipu_format(u32 code, enum imx_pixfmt_sel fmt_sel)
 {
-	return find_format(0, code, fmt_sel | PIXFMT_SEL_IPU, false);
+	return imx_media_find_mbus_format(code, fmt_sel | PIXFMT_SEL_IPU);
 }
 EXPORT_SYMBOL_GPL(imx_media_find_ipu_format);
 
 int imx_media_enum_ipu_format(u32 *code, u32 index,
 			      enum imx_pixfmt_sel fmt_sel)
 {
-	return enum_format(NULL, code, index, fmt_sel | PIXFMT_SEL_IPU, false);
+	return imx_media_enum_mbus_format(code, index,
+					  fmt_sel | PIXFMT_SEL_IPU);
 }
 EXPORT_SYMBOL_GPL(imx_media_enum_ipu_format);