Message ID | 20190912160122.5545-2-p.zabel@pengutronix.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/3] media: imx: enable V4L2_PIX_FMT_XBGR32, _BGRX32, and _RGBX32 | expand |
On 9/12/19 6:01 PM, Philipp Zabel wrote: > Merge yuv_formats and rgb_formats into a single array. Always loop over > all entries, skipping those that are incompatible with the requested > enumeration. This simplifies the code, lets us get rid of the manual > counting of array entries, and stops accidentally ignoring some non-mbus > RGB formats. > > Before: > > $ v4l2-ctl -d /dev/video14 --list-formats-out > ioctl: VIDIOC_ENUM_FMT > Type: Video Output > > [0]: 'UYVY' (UYVY 4:2:2) > [1]: 'YUYV' (YUYV 4:2:2) > [2]: 'YU12' (Planar YUV 4:2:0) > [3]: 'YV12' (Planar YVU 4:2:0) > [4]: '422P' (Planar YUV 4:2:2) > [5]: 'NV12' (Y/CbCr 4:2:0) > [6]: 'NV16' (Y/CbCr 4:2:2) > [7]: 'RGBP' (16-bit RGB 5-6-5) > [8]: 'RGB3' (24-bit RGB 8-8-8) > [9]: 'BX24' (32-bit XRGB 8-8-8-8) > > After: > > $ v4l2-ctl -d /dev/video14 --list-formats-out > ioctl: VIDIOC_ENUM_FMT > Type: Video Output > > [0]: 'UYVY' (UYVY 4:2:2) > [1]: 'YUYV' (YUYV 4:2:2) > [2]: 'YU12' (Planar YUV 4:2:0) > [3]: 'YV12' (Planar YVU 4:2:0) > [4]: '422P' (Planar YUV 4:2:2) > [5]: 'NV12' (Y/CbCr 4:2:0) > [6]: 'NV16' (Y/CbCr 4:2:2) > [7]: 'RGBP' (16-bit RGB 5-6-5) > [8]: 'RGB3' (24-bit RGB 8-8-8) > [9]: 'BGR3' (24-bit BGR 8-8-8) > [10]: 'BX24' (32-bit XRGB 8-8-8-8) > [11]: 'XR24' (32-bit BGRX 8-8-8-8) > [12]: 'RX24' (32-bit XBGR 8-8-8-8) > [13]: 'XB24' (32-bit RGBX 8-8-8-8) > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > --- > drivers/staging/media/imx/imx-media-utils.c | 170 ++++++-------------- > 1 file changed, 45 insertions(+), 125 deletions(-) > > diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c > index 0788a1874557..d61a8f4533dc 100644 > --- a/drivers/staging/media/imx/imx-media-utils.c > +++ b/drivers/staging/media/imx/imx-media-utils.c > @@ -9,12 +9,9 @@ > > /* > * List of supported pixel formats for the subdevs. > - * > - * In all of these tables, the non-mbus formats (with no > - * mbus codes) must all fall at the end of the table. > */ > - > -static const struct imx_media_pixfmt yuv_formats[] = { > +static const struct imx_media_pixfmt pixel_formats[] = { > + /*** YUV formats start here ***/ > { > .fourcc = V4L2_PIX_FMT_UYVY, > .codes = { > @@ -31,12 +28,7 @@ static const struct imx_media_pixfmt yuv_formats[] = { > }, > .cs = IPUV3_COLORSPACE_YUV, > .bpp = 16, > - }, > - /*** > - * non-mbus YUV formats start here. NOTE! when adding non-mbus > - * formats, NUM_NON_MBUS_YUV_FORMATS must be updated below. > - ***/ > - { > + }, { > .fourcc = V4L2_PIX_FMT_YUV420, > .cs = IPUV3_COLORSPACE_YUV, > .bpp = 12, > @@ -62,13 +54,7 @@ static const struct imx_media_pixfmt yuv_formats[] = { > .bpp = 16, > .planar = true, > }, > -}; > - > -#define NUM_NON_MBUS_YUV_FORMATS 5 > -#define NUM_YUV_FORMATS ARRAY_SIZE(yuv_formats) > -#define NUM_MBUS_YUV_FORMATS (NUM_YUV_FORMATS - NUM_NON_MBUS_YUV_FORMATS) > - > -static const struct imx_media_pixfmt rgb_formats[] = { > + /*** RGB formats start here ***/ > { > .fourcc = V4L2_PIX_FMT_RGB565, > .codes = {MEDIA_BUS_FMT_RGB565_2X8_LE}, > @@ -83,12 +69,28 @@ static const struct imx_media_pixfmt rgb_formats[] = { > }, > .cs = IPUV3_COLORSPACE_RGB, > .bpp = 24, > + }, { > + .fourcc = V4L2_PIX_FMT_BGR24, > + .cs = IPUV3_COLORSPACE_RGB, > + .bpp = 24, > }, { > .fourcc = V4L2_PIX_FMT_XRGB32, > .codes = {MEDIA_BUS_FMT_ARGB8888_1X32}, > .cs = IPUV3_COLORSPACE_RGB, > .bpp = 32, > .ipufmt = true, > + }, { > + .fourcc = V4L2_PIX_FMT_XBGR32, > + .cs = IPUV3_COLORSPACE_RGB, > + .bpp = 32, > + }, { > + .fourcc = V4L2_PIX_FMT_BGRX32, > + .cs = IPUV3_COLORSPACE_RGB, > + .bpp = 32, > + }, { > + .fourcc = V4L2_PIX_FMT_RGBX32, > + .cs = IPUV3_COLORSPACE_RGB, > + .bpp = 32, > }, > /*** raw bayer and grayscale formats start here ***/ > { > @@ -175,33 +177,8 @@ static const struct imx_media_pixfmt rgb_formats[] = { > .bpp = 16, > .bayer = true, > }, > - /*** > - * non-mbus RGB formats start here. NOTE! when adding non-mbus > - * formats, NUM_NON_MBUS_RGB_FORMATS must be updated below. > - ***/ > - { > - .fourcc = V4L2_PIX_FMT_BGR24, > - .cs = IPUV3_COLORSPACE_RGB, > - .bpp = 24, > - }, { > - .fourcc = V4L2_PIX_FMT_XBGR32, > - .cs = IPUV3_COLORSPACE_RGB, > - .bpp = 32, > - }, { > - .fourcc = V4L2_PIX_FMT_BGRX32, > - .cs = IPUV3_COLORSPACE_RGB, > - .bpp = 32, > - }, { > - .fourcc = V4L2_PIX_FMT_RGBX32, > - .cs = IPUV3_COLORSPACE_RGB, > - .bpp = 32, > - }, > }; > > -#define NUM_NON_MBUS_RGB_FORMATS 2 > -#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, > @@ -240,20 +217,20 @@ static void init_mbus_colorimetry(struct v4l2_mbus_framefmt *mbus, > } > > 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) > +struct imx_media_pixfmt *find_format(u32 fourcc, > + u32 code, > + enum codespace_sel cs_sel, > + bool allow_non_mbus, > + bool allow_bayer) > { > const struct imx_media_pixfmt *fmt; > int i, j; > > - for (i = 0; i < array_size; i++) { > - fmt = &array[i]; > + for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) { > + fmt = &pixel_formats[i]; > > - if ((!allow_non_mbus && !fmt->codes[0]) || > + if ((cs_sel != CS_SEL_ANY && fmt->cs != cs_sel) || I'm getting this compiler warnings: drivers/staging/media/imx/imx-media-utils.c: In function ‘find_format’: drivers/staging/media/imx/imx-media-utils.c:232:40: warning: comparison between ‘const enum ipu_color_space’ and ‘enum codespace_sel’ [-Wenum-compare] 232 | if ((cs_sel != CS_SEL_ANY && fmt->cs != cs_sel) || | ^~ > + (!allow_non_mbus && !fmt->codes[0]) || > (!allow_bayer && fmt->bayer)) > continue; > > @@ -263,7 +240,7 @@ struct imx_media_pixfmt *__find_format(u32 fourcc, > if (!code) > continue; > > - for (j = 0; fmt->codes[j]; j++) { > + for (j = 0; j < ARRAY_SIZE(fmt->codes) && fmt->codes[j]; j++) { > if (code == fmt->codes[j]) > return fmt; > } > @@ -271,86 +248,29 @@ struct imx_media_pixfmt *__find_format(u32 fourcc, > return NULL; > } > > -static const struct imx_media_pixfmt *find_format(u32 fourcc, > - u32 code, > - enum codespace_sel cs_sel, > - bool allow_non_mbus, > - bool allow_bayer) > -{ > - const struct imx_media_pixfmt *ret; > - > - switch (cs_sel) { > - case CS_SEL_YUV: > - return __find_format(fourcc, code, allow_non_mbus, allow_bayer, > - yuv_formats, NUM_YUV_FORMATS); > - case CS_SEL_RGB: > - return __find_format(fourcc, code, allow_non_mbus, allow_bayer, > - rgb_formats, NUM_RGB_FORMATS); > - case CS_SEL_ANY: > - ret = __find_format(fourcc, code, allow_non_mbus, allow_bayer, > - yuv_formats, NUM_YUV_FORMATS); > - if (ret) > - return ret; > - return __find_format(fourcc, code, allow_non_mbus, allow_bayer, > - rgb_formats, NUM_RGB_FORMATS); > - default: > - return NULL; > - } > -} > - > static int enum_format(u32 *fourcc, u32 *code, u32 index, > enum codespace_sel cs_sel, > bool allow_non_mbus, > bool allow_bayer) > { > const struct imx_media_pixfmt *fmt; > - u32 mbus_yuv_sz = NUM_MBUS_YUV_FORMATS; > - u32 mbus_rgb_sz = NUM_MBUS_RGB_FORMATS; > - u32 yuv_sz = NUM_YUV_FORMATS; > - u32 rgb_sz = NUM_RGB_FORMATS; > + unsigned int i, j = 0; > > - switch (cs_sel) { > - case CS_SEL_YUV: > - if (index >= yuv_sz || > - (!allow_non_mbus && 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)) > - return -EINVAL; > - fmt = &rgb_formats[index]; > - if (!allow_bayer && fmt->bayer) > - return -EINVAL; > - break; > - case CS_SEL_ANY: > - if (!allow_non_mbus) { > - if (index >= mbus_yuv_sz) { > - index -= mbus_yuv_sz; > - if (index >= mbus_rgb_sz) > - return -EINVAL; > - fmt = &rgb_formats[index]; > - if (!allow_bayer && fmt->bayer) > - return -EINVAL; > - } else { > - fmt = &yuv_formats[index]; > - } > - } else { > - if (index >= yuv_sz + rgb_sz) > - return -EINVAL; > - if (index >= yuv_sz) { > - fmt = &rgb_formats[index - yuv_sz]; > - if (!allow_bayer && fmt->bayer) > - return -EINVAL; > - } else { > - fmt = &yuv_formats[index]; > - } > - } > - break; > - default: > - return -EINVAL; > + for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) { > + fmt = &pixel_formats[i]; > + > + if ((cs_sel != CS_SEL_ANY && fmt->cs != cs_sel) || Same warning here. I'm dropping this patch, I'll only merge the first patch. Regards, Hans > + (!allow_non_mbus && !fmt->codes[0]) || > + (!allow_bayer && fmt->bayer)) > + continue; > + > + if (index == j) > + break; > + > + j++; > } > + if (i == ARRAY_SIZE(pixel_formats)) > + return -EINVAL; > > if (fourcc) > *fourcc = fmt->fourcc; >
Hi Philipp, Thanks for this patch and the next. I agree it simplifies the code. Besides the compile warning caught by Hans and the kbuild test robot, I have only one other suggestion. Can you change the name of the index j (which becomes k in the next patch), to something like "match_index", which makes it more clear that this is a count of the pixel_formats[] entries that match the requested search criteria. For testing I made this change in my github fork of the media-tree: git@github.com:slongerbeam/mediatree.git, branch imx/philipp-pixfmts-cleanup In that branch I also fixed the compile warnings. Finally, I added a patch that also moves the IPU-internal formats (YUV4 and BX24) into pixel_formats[] that results in similar simplifications, with an additional search parameter to find_format() and enum_format() to search only for IPU-internal formats or only non-IPU-internal formats. Let me know if you agree with those changes, and if so please include them in next version of this series. I tested on i.MX6Q SabreSD and all is good. Steve On 9/12/19 9:01 AM, Philipp Zabel wrote: > Merge yuv_formats and rgb_formats into a single array. Always loop over > all entries, skipping those that are incompatible with the requested > enumeration. This simplifies the code, lets us get rid of the manual > counting of array entries, and stops accidentally ignoring some non-mbus > RGB formats. > > Before: > > $ v4l2-ctl -d /dev/video14 --list-formats-out > ioctl: VIDIOC_ENUM_FMT > Type: Video Output > > [0]: 'UYVY' (UYVY 4:2:2) > [1]: 'YUYV' (YUYV 4:2:2) > [2]: 'YU12' (Planar YUV 4:2:0) > [3]: 'YV12' (Planar YVU 4:2:0) > [4]: '422P' (Planar YUV 4:2:2) > [5]: 'NV12' (Y/CbCr 4:2:0) > [6]: 'NV16' (Y/CbCr 4:2:2) > [7]: 'RGBP' (16-bit RGB 5-6-5) > [8]: 'RGB3' (24-bit RGB 8-8-8) > [9]: 'BX24' (32-bit XRGB 8-8-8-8) > > After: > > $ v4l2-ctl -d /dev/video14 --list-formats-out > ioctl: VIDIOC_ENUM_FMT > Type: Video Output > > [0]: 'UYVY' (UYVY 4:2:2) > [1]: 'YUYV' (YUYV 4:2:2) > [2]: 'YU12' (Planar YUV 4:2:0) > [3]: 'YV12' (Planar YVU 4:2:0) > [4]: '422P' (Planar YUV 4:2:2) > [5]: 'NV12' (Y/CbCr 4:2:0) > [6]: 'NV16' (Y/CbCr 4:2:2) > [7]: 'RGBP' (16-bit RGB 5-6-5) > [8]: 'RGB3' (24-bit RGB 8-8-8) > [9]: 'BGR3' (24-bit BGR 8-8-8) > [10]: 'BX24' (32-bit XRGB 8-8-8-8) > [11]: 'XR24' (32-bit BGRX 8-8-8-8) > [12]: 'RX24' (32-bit XBGR 8-8-8-8) > [13]: 'XB24' (32-bit RGBX 8-8-8-8) > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > --- > drivers/staging/media/imx/imx-media-utils.c | 170 ++++++-------------- > 1 file changed, 45 insertions(+), 125 deletions(-) > > diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c > index 0788a1874557..d61a8f4533dc 100644 > --- a/drivers/staging/media/imx/imx-media-utils.c > +++ b/drivers/staging/media/imx/imx-media-utils.c > @@ -9,12 +9,9 @@ > > /* > * List of supported pixel formats for the subdevs. > - * > - * In all of these tables, the non-mbus formats (with no > - * mbus codes) must all fall at the end of the table. > */ > - > -static const struct imx_media_pixfmt yuv_formats[] = { > +static const struct imx_media_pixfmt pixel_formats[] = { > + /*** YUV formats start here ***/ > { > .fourcc = V4L2_PIX_FMT_UYVY, > .codes = { > @@ -31,12 +28,7 @@ static const struct imx_media_pixfmt yuv_formats[] = { > }, > .cs = IPUV3_COLORSPACE_YUV, > .bpp = 16, > - }, > - /*** > - * non-mbus YUV formats start here. NOTE! when adding non-mbus > - * formats, NUM_NON_MBUS_YUV_FORMATS must be updated below. > - ***/ > - { > + }, { > .fourcc = V4L2_PIX_FMT_YUV420, > .cs = IPUV3_COLORSPACE_YUV, > .bpp = 12, > @@ -62,13 +54,7 @@ static const struct imx_media_pixfmt yuv_formats[] = { > .bpp = 16, > .planar = true, > }, > -}; > - > -#define NUM_NON_MBUS_YUV_FORMATS 5 > -#define NUM_YUV_FORMATS ARRAY_SIZE(yuv_formats) > -#define NUM_MBUS_YUV_FORMATS (NUM_YUV_FORMATS - NUM_NON_MBUS_YUV_FORMATS) > - > -static const struct imx_media_pixfmt rgb_formats[] = { > + /*** RGB formats start here ***/ > { > .fourcc = V4L2_PIX_FMT_RGB565, > .codes = {MEDIA_BUS_FMT_RGB565_2X8_LE}, > @@ -83,12 +69,28 @@ static const struct imx_media_pixfmt rgb_formats[] = { > }, > .cs = IPUV3_COLORSPACE_RGB, > .bpp = 24, > + }, { > + .fourcc = V4L2_PIX_FMT_BGR24, > + .cs = IPUV3_COLORSPACE_RGB, > + .bpp = 24, > }, { > .fourcc = V4L2_PIX_FMT_XRGB32, > .codes = {MEDIA_BUS_FMT_ARGB8888_1X32}, > .cs = IPUV3_COLORSPACE_RGB, > .bpp = 32, > .ipufmt = true, > + }, { > + .fourcc = V4L2_PIX_FMT_XBGR32, > + .cs = IPUV3_COLORSPACE_RGB, > + .bpp = 32, > + }, { > + .fourcc = V4L2_PIX_FMT_BGRX32, > + .cs = IPUV3_COLORSPACE_RGB, > + .bpp = 32, > + }, { > + .fourcc = V4L2_PIX_FMT_RGBX32, > + .cs = IPUV3_COLORSPACE_RGB, > + .bpp = 32, > }, > /*** raw bayer and grayscale formats start here ***/ > { > @@ -175,33 +177,8 @@ static const struct imx_media_pixfmt rgb_formats[] = { > .bpp = 16, > .bayer = true, > }, > - /*** > - * non-mbus RGB formats start here. NOTE! when adding non-mbus > - * formats, NUM_NON_MBUS_RGB_FORMATS must be updated below. > - ***/ > - { > - .fourcc = V4L2_PIX_FMT_BGR24, > - .cs = IPUV3_COLORSPACE_RGB, > - .bpp = 24, > - }, { > - .fourcc = V4L2_PIX_FMT_XBGR32, > - .cs = IPUV3_COLORSPACE_RGB, > - .bpp = 32, > - }, { > - .fourcc = V4L2_PIX_FMT_BGRX32, > - .cs = IPUV3_COLORSPACE_RGB, > - .bpp = 32, > - }, { > - .fourcc = V4L2_PIX_FMT_RGBX32, > - .cs = IPUV3_COLORSPACE_RGB, > - .bpp = 32, > - }, > }; > > -#define NUM_NON_MBUS_RGB_FORMATS 2 > -#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, > @@ -240,20 +217,20 @@ static void init_mbus_colorimetry(struct v4l2_mbus_framefmt *mbus, > } > > 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) > +struct imx_media_pixfmt *find_format(u32 fourcc, > + u32 code, > + enum codespace_sel cs_sel, > + bool allow_non_mbus, > + bool allow_bayer) > { > const struct imx_media_pixfmt *fmt; > int i, j; > > - for (i = 0; i < array_size; i++) { > - fmt = &array[i]; > + for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) { > + fmt = &pixel_formats[i]; > > - if ((!allow_non_mbus && !fmt->codes[0]) || > + if ((cs_sel != CS_SEL_ANY && fmt->cs != cs_sel) || > + (!allow_non_mbus && !fmt->codes[0]) || > (!allow_bayer && fmt->bayer)) > continue; > > @@ -263,7 +240,7 @@ struct imx_media_pixfmt *__find_format(u32 fourcc, > if (!code) > continue; > > - for (j = 0; fmt->codes[j]; j++) { > + for (j = 0; j < ARRAY_SIZE(fmt->codes) && fmt->codes[j]; j++) { > if (code == fmt->codes[j]) > return fmt; > } > @@ -271,86 +248,29 @@ struct imx_media_pixfmt *__find_format(u32 fourcc, > return NULL; > } > > -static const struct imx_media_pixfmt *find_format(u32 fourcc, > - u32 code, > - enum codespace_sel cs_sel, > - bool allow_non_mbus, > - bool allow_bayer) > -{ > - const struct imx_media_pixfmt *ret; > - > - switch (cs_sel) { > - case CS_SEL_YUV: > - return __find_format(fourcc, code, allow_non_mbus, allow_bayer, > - yuv_formats, NUM_YUV_FORMATS); > - case CS_SEL_RGB: > - return __find_format(fourcc, code, allow_non_mbus, allow_bayer, > - rgb_formats, NUM_RGB_FORMATS); > - case CS_SEL_ANY: > - ret = __find_format(fourcc, code, allow_non_mbus, allow_bayer, > - yuv_formats, NUM_YUV_FORMATS); > - if (ret) > - return ret; > - return __find_format(fourcc, code, allow_non_mbus, allow_bayer, > - rgb_formats, NUM_RGB_FORMATS); > - default: > - return NULL; > - } > -} > - > static int enum_format(u32 *fourcc, u32 *code, u32 index, > enum codespace_sel cs_sel, > bool allow_non_mbus, > bool allow_bayer) > { > const struct imx_media_pixfmt *fmt; > - u32 mbus_yuv_sz = NUM_MBUS_YUV_FORMATS; > - u32 mbus_rgb_sz = NUM_MBUS_RGB_FORMATS; > - u32 yuv_sz = NUM_YUV_FORMATS; > - u32 rgb_sz = NUM_RGB_FORMATS; > + unsigned int i, j = 0; > > - switch (cs_sel) { > - case CS_SEL_YUV: > - if (index >= yuv_sz || > - (!allow_non_mbus && 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)) > - return -EINVAL; > - fmt = &rgb_formats[index]; > - if (!allow_bayer && fmt->bayer) > - return -EINVAL; > - break; > - case CS_SEL_ANY: > - if (!allow_non_mbus) { > - if (index >= mbus_yuv_sz) { > - index -= mbus_yuv_sz; > - if (index >= mbus_rgb_sz) > - return -EINVAL; > - fmt = &rgb_formats[index]; > - if (!allow_bayer && fmt->bayer) > - return -EINVAL; > - } else { > - fmt = &yuv_formats[index]; > - } > - } else { > - if (index >= yuv_sz + rgb_sz) > - return -EINVAL; > - if (index >= yuv_sz) { > - fmt = &rgb_formats[index - yuv_sz]; > - if (!allow_bayer && fmt->bayer) > - return -EINVAL; > - } else { > - fmt = &yuv_formats[index]; > - } > - } > - break; > - default: > - return -EINVAL; > + for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) { > + fmt = &pixel_formats[i]; > + > + if ((cs_sel != CS_SEL_ANY && fmt->cs != cs_sel) || > + (!allow_non_mbus && !fmt->codes[0]) || > + (!allow_bayer && fmt->bayer)) > + continue; > + > + if (index == j) > + break; > + > + j++; > } > + if (i == ARRAY_SIZE(pixel_formats)) > + return -EINVAL; > > if (fourcc) > *fourcc = fmt->fourcc;
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c index 0788a1874557..d61a8f4533dc 100644 --- a/drivers/staging/media/imx/imx-media-utils.c +++ b/drivers/staging/media/imx/imx-media-utils.c @@ -9,12 +9,9 @@ /* * List of supported pixel formats for the subdevs. - * - * In all of these tables, the non-mbus formats (with no - * mbus codes) must all fall at the end of the table. */ - -static const struct imx_media_pixfmt yuv_formats[] = { +static const struct imx_media_pixfmt pixel_formats[] = { + /*** YUV formats start here ***/ { .fourcc = V4L2_PIX_FMT_UYVY, .codes = { @@ -31,12 +28,7 @@ static const struct imx_media_pixfmt yuv_formats[] = { }, .cs = IPUV3_COLORSPACE_YUV, .bpp = 16, - }, - /*** - * non-mbus YUV formats start here. NOTE! when adding non-mbus - * formats, NUM_NON_MBUS_YUV_FORMATS must be updated below. - ***/ - { + }, { .fourcc = V4L2_PIX_FMT_YUV420, .cs = IPUV3_COLORSPACE_YUV, .bpp = 12, @@ -62,13 +54,7 @@ static const struct imx_media_pixfmt yuv_formats[] = { .bpp = 16, .planar = true, }, -}; - -#define NUM_NON_MBUS_YUV_FORMATS 5 -#define NUM_YUV_FORMATS ARRAY_SIZE(yuv_formats) -#define NUM_MBUS_YUV_FORMATS (NUM_YUV_FORMATS - NUM_NON_MBUS_YUV_FORMATS) - -static const struct imx_media_pixfmt rgb_formats[] = { + /*** RGB formats start here ***/ { .fourcc = V4L2_PIX_FMT_RGB565, .codes = {MEDIA_BUS_FMT_RGB565_2X8_LE}, @@ -83,12 +69,28 @@ static const struct imx_media_pixfmt rgb_formats[] = { }, .cs = IPUV3_COLORSPACE_RGB, .bpp = 24, + }, { + .fourcc = V4L2_PIX_FMT_BGR24, + .cs = IPUV3_COLORSPACE_RGB, + .bpp = 24, }, { .fourcc = V4L2_PIX_FMT_XRGB32, .codes = {MEDIA_BUS_FMT_ARGB8888_1X32}, .cs = IPUV3_COLORSPACE_RGB, .bpp = 32, .ipufmt = true, + }, { + .fourcc = V4L2_PIX_FMT_XBGR32, + .cs = IPUV3_COLORSPACE_RGB, + .bpp = 32, + }, { + .fourcc = V4L2_PIX_FMT_BGRX32, + .cs = IPUV3_COLORSPACE_RGB, + .bpp = 32, + }, { + .fourcc = V4L2_PIX_FMT_RGBX32, + .cs = IPUV3_COLORSPACE_RGB, + .bpp = 32, }, /*** raw bayer and grayscale formats start here ***/ { @@ -175,33 +177,8 @@ static const struct imx_media_pixfmt rgb_formats[] = { .bpp = 16, .bayer = true, }, - /*** - * non-mbus RGB formats start here. NOTE! when adding non-mbus - * formats, NUM_NON_MBUS_RGB_FORMATS must be updated below. - ***/ - { - .fourcc = V4L2_PIX_FMT_BGR24, - .cs = IPUV3_COLORSPACE_RGB, - .bpp = 24, - }, { - .fourcc = V4L2_PIX_FMT_XBGR32, - .cs = IPUV3_COLORSPACE_RGB, - .bpp = 32, - }, { - .fourcc = V4L2_PIX_FMT_BGRX32, - .cs = IPUV3_COLORSPACE_RGB, - .bpp = 32, - }, { - .fourcc = V4L2_PIX_FMT_RGBX32, - .cs = IPUV3_COLORSPACE_RGB, - .bpp = 32, - }, }; -#define NUM_NON_MBUS_RGB_FORMATS 2 -#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, @@ -240,20 +217,20 @@ static void init_mbus_colorimetry(struct v4l2_mbus_framefmt *mbus, } 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) +struct imx_media_pixfmt *find_format(u32 fourcc, + u32 code, + enum codespace_sel cs_sel, + bool allow_non_mbus, + bool allow_bayer) { const struct imx_media_pixfmt *fmt; int i, j; - for (i = 0; i < array_size; i++) { - fmt = &array[i]; + for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) { + fmt = &pixel_formats[i]; - if ((!allow_non_mbus && !fmt->codes[0]) || + if ((cs_sel != CS_SEL_ANY && fmt->cs != cs_sel) || + (!allow_non_mbus && !fmt->codes[0]) || (!allow_bayer && fmt->bayer)) continue; @@ -263,7 +240,7 @@ struct imx_media_pixfmt *__find_format(u32 fourcc, if (!code) continue; - for (j = 0; fmt->codes[j]; j++) { + for (j = 0; j < ARRAY_SIZE(fmt->codes) && fmt->codes[j]; j++) { if (code == fmt->codes[j]) return fmt; } @@ -271,86 +248,29 @@ struct imx_media_pixfmt *__find_format(u32 fourcc, return NULL; } -static const struct imx_media_pixfmt *find_format(u32 fourcc, - u32 code, - enum codespace_sel cs_sel, - bool allow_non_mbus, - bool allow_bayer) -{ - const struct imx_media_pixfmt *ret; - - switch (cs_sel) { - case CS_SEL_YUV: - return __find_format(fourcc, code, allow_non_mbus, allow_bayer, - yuv_formats, NUM_YUV_FORMATS); - case CS_SEL_RGB: - return __find_format(fourcc, code, allow_non_mbus, allow_bayer, - rgb_formats, NUM_RGB_FORMATS); - case CS_SEL_ANY: - ret = __find_format(fourcc, code, allow_non_mbus, allow_bayer, - yuv_formats, NUM_YUV_FORMATS); - if (ret) - return ret; - return __find_format(fourcc, code, allow_non_mbus, allow_bayer, - rgb_formats, NUM_RGB_FORMATS); - default: - return NULL; - } -} - static int enum_format(u32 *fourcc, u32 *code, u32 index, enum codespace_sel cs_sel, bool allow_non_mbus, bool allow_bayer) { const struct imx_media_pixfmt *fmt; - u32 mbus_yuv_sz = NUM_MBUS_YUV_FORMATS; - u32 mbus_rgb_sz = NUM_MBUS_RGB_FORMATS; - u32 yuv_sz = NUM_YUV_FORMATS; - u32 rgb_sz = NUM_RGB_FORMATS; + unsigned int i, j = 0; - switch (cs_sel) { - case CS_SEL_YUV: - if (index >= yuv_sz || - (!allow_non_mbus && 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)) - return -EINVAL; - fmt = &rgb_formats[index]; - if (!allow_bayer && fmt->bayer) - return -EINVAL; - break; - case CS_SEL_ANY: - if (!allow_non_mbus) { - if (index >= mbus_yuv_sz) { - index -= mbus_yuv_sz; - if (index >= mbus_rgb_sz) - return -EINVAL; - fmt = &rgb_formats[index]; - if (!allow_bayer && fmt->bayer) - return -EINVAL; - } else { - fmt = &yuv_formats[index]; - } - } else { - if (index >= yuv_sz + rgb_sz) - return -EINVAL; - if (index >= yuv_sz) { - fmt = &rgb_formats[index - yuv_sz]; - if (!allow_bayer && fmt->bayer) - return -EINVAL; - } else { - fmt = &yuv_formats[index]; - } - } - break; - default: - return -EINVAL; + for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) { + fmt = &pixel_formats[i]; + + if ((cs_sel != CS_SEL_ANY && fmt->cs != cs_sel) || + (!allow_non_mbus && !fmt->codes[0]) || + (!allow_bayer && fmt->bayer)) + continue; + + if (index == j) + break; + + j++; } + if (i == ARRAY_SIZE(pixel_formats)) + return -EINVAL; if (fourcc) *fourcc = fmt->fourcc;
Merge yuv_formats and rgb_formats into a single array. Always loop over all entries, skipping those that are incompatible with the requested enumeration. This simplifies the code, lets us get rid of the manual counting of array entries, and stops accidentally ignoring some non-mbus RGB formats. Before: $ v4l2-ctl -d /dev/video14 --list-formats-out ioctl: VIDIOC_ENUM_FMT Type: Video Output [0]: 'UYVY' (UYVY 4:2:2) [1]: 'YUYV' (YUYV 4:2:2) [2]: 'YU12' (Planar YUV 4:2:0) [3]: 'YV12' (Planar YVU 4:2:0) [4]: '422P' (Planar YUV 4:2:2) [5]: 'NV12' (Y/CbCr 4:2:0) [6]: 'NV16' (Y/CbCr 4:2:2) [7]: 'RGBP' (16-bit RGB 5-6-5) [8]: 'RGB3' (24-bit RGB 8-8-8) [9]: 'BX24' (32-bit XRGB 8-8-8-8) After: $ v4l2-ctl -d /dev/video14 --list-formats-out ioctl: VIDIOC_ENUM_FMT Type: Video Output [0]: 'UYVY' (UYVY 4:2:2) [1]: 'YUYV' (YUYV 4:2:2) [2]: 'YU12' (Planar YUV 4:2:0) [3]: 'YV12' (Planar YVU 4:2:0) [4]: '422P' (Planar YUV 4:2:2) [5]: 'NV12' (Y/CbCr 4:2:0) [6]: 'NV16' (Y/CbCr 4:2:2) [7]: 'RGBP' (16-bit RGB 5-6-5) [8]: 'RGB3' (24-bit RGB 8-8-8) [9]: 'BGR3' (24-bit BGR 8-8-8) [10]: 'BX24' (32-bit XRGB 8-8-8-8) [11]: 'XR24' (32-bit BGRX 8-8-8-8) [12]: 'RX24' (32-bit XBGR 8-8-8-8) [13]: 'XB24' (32-bit RGBX 8-8-8-8) Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> --- drivers/staging/media/imx/imx-media-utils.c | 170 ++++++-------------- 1 file changed, 45 insertions(+), 125 deletions(-)