* [PATCH 0/2] media: imx: add capture support for RGB565_2X8 on parallel bus @ 2018-05-03 16:41 Jan Luebbe 2018-05-03 16:41 ` [PATCH 1/2] media: imx: capture: refactor enum_/try_fmt Jan Luebbe ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Jan Luebbe @ 2018-05-03 16:41 UTC (permalink / raw) To: linux-media; +Cc: Jan Luebbe, slongerbeam, p.zabel The IPU can only capture RGB565 with two 8-bit cycles in bayer/generic mode on the parallel bus, compared to a specific mode on MIPI CSI-2. To handle this, we extend imx_media_pixfmt with a cycles per pixel field, which is used for generic formats on the parallel bus. Before actually adding RGB565_2X8 support for the parallel bus, this series simplifies handing of the the different configurations for RGB565 between parallel and MIPI CSI-2 in imx-media-capture. This avoids having to explicitly pass on the format in the second patch. Jan Luebbe (2): media: imx: capture: refactor enum_/try_fmt media: imx: add support for RGB565_2X8 on parallel bus drivers/staging/media/imx/imx-media-capture.c | 38 +++++++-------- drivers/staging/media/imx/imx-media-csi.c | 47 +++++++++++++++++-- drivers/staging/media/imx/imx-media-utils.c | 1 + drivers/staging/media/imx/imx-media.h | 2 + 4 files changed, 63 insertions(+), 25 deletions(-) -- 2.17.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] media: imx: capture: refactor enum_/try_fmt 2018-05-03 16:41 [PATCH 0/2] media: imx: add capture support for RGB565_2X8 on parallel bus Jan Luebbe @ 2018-05-03 16:41 ` Jan Luebbe 2018-05-03 16:41 ` [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus Jan Luebbe 2018-05-05 22:22 ` [PATCH 0/2] media: imx: add capture " Steve Longerbeam 2 siblings, 0 replies; 13+ messages in thread From: Jan Luebbe @ 2018-05-03 16:41 UTC (permalink / raw) To: linux-media; +Cc: Jan Luebbe, slongerbeam, p.zabel By checking and handling the internal IPU formats (ARGB or AYUV) first, we don't need to check whether it's a bayer format, as we can default to passing the input format on in all other cases. This simplifies handling the different configurations for RGB565 between parallel and MIPI CSI-2, as we don't need to check the details of the format anymore. Signed-off-by: Jan Luebbe <jlu@pengutronix.de> --- drivers/staging/media/imx/imx-media-capture.c | 38 +++++++++---------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c index 0ccabe04b0e1..64c23ef92931 100644 --- a/drivers/staging/media/imx/imx-media-capture.c +++ b/drivers/staging/media/imx/imx-media-capture.c @@ -170,23 +170,22 @@ static int capture_enum_fmt_vid_cap(struct file *file, void *fh, } cc_src = imx_media_find_ipu_format(fmt_src.format.code, CS_SEL_ANY); - if (!cc_src) - cc_src = imx_media_find_mbus_format(fmt_src.format.code, - CS_SEL_ANY, true); - if (!cc_src) - return -EINVAL; - - if (cc_src->bayer) { - if (f->index != 0) - return -EINVAL; - fourcc = cc_src->fourcc; - } else { + if (cc_src) { 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); if (ret) return ret; + } else { + cc_src = imx_media_find_mbus_format(fmt_src.format.code, + CS_SEL_ANY, true); + if (WARN_ON(!cc_src)) + return -EINVAL; + + if (f->index != 0) + return -EINVAL; + fourcc = cc_src->fourcc; } f->pixelformat = fourcc; @@ -219,15 +218,7 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh, return ret; cc_src = imx_media_find_ipu_format(fmt_src.format.code, CS_SEL_ANY); - if (!cc_src) - cc_src = imx_media_find_mbus_format(fmt_src.format.code, - CS_SEL_ANY, true); - if (!cc_src) - return -EINVAL; - - if (cc_src->bayer) { - cc = cc_src; - } else { + if (cc_src) { u32 fourcc, cs_sel; cs_sel = (cc_src->cs == IPUV3_COLORSPACE_YUV) ? @@ -239,6 +230,13 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh, imx_media_enum_format(&fourcc, 0, cs_sel); cc = imx_media_find_format(fourcc, cs_sel, false); } + } else { + cc_src = imx_media_find_mbus_format(fmt_src.format.code, + CS_SEL_ANY, true); + if (WARN_ON(!cc_src)) + return -EINVAL; + + cc = cc_src; } imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, &fmt_src.format, cc); -- 2.17.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus 2018-05-03 16:41 [PATCH 0/2] media: imx: add capture support for RGB565_2X8 on parallel bus Jan Luebbe 2018-05-03 16:41 ` [PATCH 1/2] media: imx: capture: refactor enum_/try_fmt Jan Luebbe @ 2018-05-03 16:41 ` Jan Luebbe 2018-05-04 5:07 ` kbuild test robot ` (3 more replies) 2018-05-05 22:22 ` [PATCH 0/2] media: imx: add capture " Steve Longerbeam 2 siblings, 4 replies; 13+ messages in thread From: Jan Luebbe @ 2018-05-03 16:41 UTC (permalink / raw) To: linux-media; +Cc: Jan Luebbe, slongerbeam, p.zabel The IPU can only capture RGB565 with two 8-bit cycles in bayer/generic mode on the parallel bus, compared to a specific mode on MIPI CSI-2. To handle this, we extend imx_media_pixfmt with a cycles per pixel field, which is used for generic formats on the parallel bus. Based on the selected format and bus, we then update the width to account for the multiple cycles per pixel. Signed-off-by: Jan Luebbe <jlu@pengutronix.de> --- drivers/staging/media/imx/imx-media-csi.c | 47 ++++++++++++++++++--- drivers/staging/media/imx/imx-media-utils.c | 1 + drivers/staging/media/imx/imx-media.h | 2 + 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index 08b636084286..aeb6801e5bd2 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -370,6 +370,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv) struct v4l2_mbus_framefmt *infmt; struct ipu_image image; u32 passthrough_bits; + u32 passthrough_cycles; dma_addr_t phys[2]; bool passthrough; u32 burst_size; @@ -395,6 +396,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv) * - raw bayer formats * - the CSI is receiving from a 16-bit parallel bus */ + passthrough_cycles = 1; switch (image.pix.pixelformat) { case V4L2_PIX_FMT_SBGGR8: case V4L2_PIX_FMT_SGBRG8: @@ -431,6 +433,16 @@ static int csi_idmac_setup_channel(struct csi_priv *priv) passthrough = is_parallel_16bit_bus(&priv->upstream_ep); passthrough_bits = 16; break; + case V4L2_PIX_FMT_RGB565: + /* without CSI2 we can only use passthrough mode */ + if (priv->upstream_ep.bus_type != V4L2_MBUS_CSI2) { + burst_size = 16; + passthrough = true; + passthrough_bits = 8; + passthrough_cycles = 2; + break; + }; + /* fallthrough */ default: burst_size = (image.pix.width & 0xf) ? 8 : 16; passthrough = is_parallel_16bit_bus(&priv->upstream_ep); @@ -439,7 +451,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv) } if (passthrough) { - ipu_cpmem_set_resolution(priv->idmac_ch, image.rect.width, + ipu_cpmem_set_resolution(priv->idmac_ch, + image.rect.width * passthrough_cycles, image.rect.height); ipu_cpmem_set_stride(priv->idmac_ch, image.pix.bytesperline); ipu_cpmem_set_buffer(priv->idmac_ch, 0, image.phys0); @@ -632,9 +645,12 @@ static int csi_setup(struct csi_priv *priv) struct v4l2_mbus_framefmt *infmt, *outfmt; struct v4l2_mbus_config mbus_cfg; struct v4l2_mbus_framefmt if_fmt; + struct imx_media_pixfmt *outcc; + struct v4l2_rect crop; infmt = &priv->format_mbus[CSI_SINK_PAD]; outfmt = &priv->format_mbus[priv->active_output_pad]; + outcc = priv->cc[priv->active_output_pad]; /* compose mbus_config from the upstream endpoint */ mbus_cfg.type = priv->upstream_ep.bus_type; @@ -648,8 +664,18 @@ static int csi_setup(struct csi_priv *priv) */ if_fmt = *infmt; if_fmt.field = outfmt->field; + crop = priv->crop; + + /* + * if cycles is set, we need to handle this over multiple cycles as + * generic/bayer data + */ + if ((priv->upstream_ep.bus_type != V4L2_MBUS_CSI2) && outcc->cycles) { + if_fmt.width *= outcc->cycles; + crop.width *= outcc->cycles; + } - ipu_csi_set_window(priv->csi, &priv->crop); + ipu_csi_set_window(priv->csi, &crop); ipu_csi_set_downsize(priv->csi, priv->crop.width == 2 * priv->compose.width, @@ -1029,7 +1055,9 @@ static int csi_link_validate(struct v4l2_subdev *sd, incc = priv->cc[CSI_SINK_PAD]; if (priv->dest != IPU_CSI_DEST_IDMAC && - (incc->bayer || is_parallel_16bit_bus(&upstream_ep))) { + (incc->bayer || + is_parallel_16bit_bus(&upstream_ep) || + (!is_csi2 && incc->cycles))) { v4l2_err(&priv->sd, "bayer/16-bit parallel buses must go to IDMAC pad\n"); ret = -EINVAL; @@ -1131,6 +1159,7 @@ static int csi_enum_mbus_code(struct v4l2_subdev *sd, struct v4l2_subdev_mbus_code_enum *code) { struct csi_priv *priv = v4l2_get_subdevdata(sd); + struct v4l2_fwnode_endpoint upstream_ep; const struct imx_media_pixfmt *incc; struct v4l2_mbus_framefmt *infmt; int ret = 0; @@ -1147,7 +1176,14 @@ static int csi_enum_mbus_code(struct v4l2_subdev *sd, break; case CSI_SRC_PAD_DIRECT: case CSI_SRC_PAD_IDMAC: - if (incc->bayer) { + ret = csi_get_upstream_endpoint(priv, &upstream_ep); + if (ret) { + v4l2_err(&priv->sd, "failed to find upstream endpoint\n"); + goto out; + } + + if (incc->bayer || + (upstream_ep.bus_type != V4L2_MBUS_CSI2 && incc->cycles)) { if (code->index != 0) { ret = -EINVAL; goto out; @@ -1286,7 +1322,8 @@ static void csi_try_fmt(struct csi_priv *priv, sdformat->format.width = compose->width; sdformat->format.height = compose->height; - if (incc->bayer) { + if (incc->bayer || + (upstream_ep->bus_type != V4L2_MBUS_CSI2 && incc->cycles)) { sdformat->format.code = infmt->code; *cc = incc; } else { diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c index 7ec2db84451c..8aa13403b09d 100644 --- a/drivers/staging/media/imx/imx-media-utils.c +++ b/drivers/staging/media/imx/imx-media-utils.c @@ -78,6 +78,7 @@ static const struct imx_media_pixfmt rgb_formats[] = { .codes = {MEDIA_BUS_FMT_RGB565_2X8_LE}, .cs = IPUV3_COLORSPACE_RGB, .bpp = 16, + .cycles = 2, }, { .fourcc = V4L2_PIX_FMT_RGB24, .codes = { diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h index e945e0ed6dd6..57bd094cf765 100644 --- a/drivers/staging/media/imx/imx-media.h +++ b/drivers/staging/media/imx/imx-media.h @@ -62,6 +62,8 @@ struct imx_media_pixfmt { u32 fourcc; u32 codes[4]; int bpp; /* total bpp */ + /* cycles per pixel for generic (bayer) formats for the parallel bus */ + int cycles; enum ipu_color_space cs; bool planar; /* is a planar format */ bool bayer; /* is a raw bayer format */ -- 2.17.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus 2018-05-03 16:41 ` [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus Jan Luebbe @ 2018-05-04 5:07 ` kbuild test robot 2018-05-04 14:44 ` Jan Lübbe 2018-05-04 6:44 ` [PATCH] media: imx: fix semicolon.cocci warnings kbuild test robot ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: kbuild test robot @ 2018-05-04 5:07 UTC (permalink / raw) To: Jan Luebbe; +Cc: kbuild-all, linux-media, Jan Luebbe, slongerbeam, p.zabel [-- Attachment #1: Type: text/plain, Size: 3269 bytes --] Hi Jan, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.17-rc3 next-20180503] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Jan-Luebbe/media-imx-add-capture-support-for-RGB565_2X8-on-parallel-bus/20180504-120003 base: git://linuxtv.org/media_tree.git master config: xtensa-allyesconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=xtensa All warnings (new ones prefixed by >>): drivers/staging/media/imx/imx-media-csi.c: In function 'csi_setup': >> drivers/staging/media/imx/imx-media-csi.c:652:8: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] outcc = priv->cc[priv->active_output_pad]; ^ vim +/const +652 drivers/staging/media/imx/imx-media-csi.c 640 641 /* Update the CSI whole sensor and active windows */ 642 static int csi_setup(struct csi_priv *priv) 643 { 644 struct v4l2_mbus_framefmt *infmt, *outfmt; 645 struct v4l2_mbus_config mbus_cfg; 646 struct v4l2_mbus_framefmt if_fmt; 647 struct imx_media_pixfmt *outcc; 648 struct v4l2_rect crop; 649 650 infmt = &priv->format_mbus[CSI_SINK_PAD]; 651 outfmt = &priv->format_mbus[priv->active_output_pad]; > 652 outcc = priv->cc[priv->active_output_pad]; 653 654 /* compose mbus_config from the upstream endpoint */ 655 mbus_cfg.type = priv->upstream_ep.bus_type; 656 mbus_cfg.flags = (priv->upstream_ep.bus_type == V4L2_MBUS_CSI2) ? 657 priv->upstream_ep.bus.mipi_csi2.flags : 658 priv->upstream_ep.bus.parallel.flags; 659 660 /* 661 * we need to pass input frame to CSI interface, but 662 * with translated field type from output format 663 */ 664 if_fmt = *infmt; 665 if_fmt.field = outfmt->field; 666 crop = priv->crop; 667 668 /* 669 * if cycles is set, we need to handle this over multiple cycles as 670 * generic/bayer data 671 */ 672 if ((priv->upstream_ep.bus_type != V4L2_MBUS_CSI2) && outcc->cycles) { 673 if_fmt.width *= outcc->cycles; 674 crop.width *= outcc->cycles; 675 } 676 677 ipu_csi_set_window(priv->csi, &crop); 678 679 ipu_csi_set_downsize(priv->csi, 680 priv->crop.width == 2 * priv->compose.width, 681 priv->crop.height == 2 * priv->compose.height); 682 683 ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt); 684 685 ipu_csi_set_dest(priv->csi, priv->dest); 686 687 if (priv->dest == IPU_CSI_DEST_IDMAC) 688 ipu_csi_set_skip_smfc(priv->csi, priv->skip->skip_smfc, 689 priv->skip->max_ratio - 1, 0); 690 691 ipu_csi_dump(priv->csi); 692 693 return 0; 694 } 695 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 53302 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus 2018-05-04 5:07 ` kbuild test robot @ 2018-05-04 14:44 ` Jan Lübbe 0 siblings, 0 replies; 13+ messages in thread From: Jan Lübbe @ 2018-05-04 14:44 UTC (permalink / raw) To: kbuild test robot; +Cc: kbuild-all, linux-media, slongerbeam, p.zabel On Fri, 2018-05-04 at 13:07 +0800, kbuild test robot wrote: > drivers/staging/media/imx/imx-media-csi.c: In function > 'csi_setup': > >> drivers/staging/media/imx/imx-media-csi.c:652:8: warning: > assignment discards 'const' qualifier from pointer target type [- > Wdiscarded-qualifiers] > outcc = priv->cc[priv->active_output_pad]; > ^ I've fixed this and the unneeded semicolon for the next round. Regards, Jan -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] media: imx: fix semicolon.cocci warnings 2018-05-03 16:41 ` [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus Jan Luebbe 2018-05-04 5:07 ` kbuild test robot @ 2018-05-04 6:44 ` kbuild test robot 2018-05-04 6:44 ` [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus kbuild test robot 2018-05-04 8:37 ` kbuild test robot 3 siblings, 0 replies; 13+ messages in thread From: kbuild test robot @ 2018-05-04 6:44 UTC (permalink / raw) To: Jan Luebbe; +Cc: kbuild-all, linux-media, Jan Luebbe, slongerbeam, p.zabel From: Fengguang Wu <fengguang.wu@intel.com> drivers/staging/media/imx/imx-media-csi.c:443:3-4: Unneeded semicolon Remove unneeded semicolon. Generated by: scripts/coccinelle/misc/semicolon.cocci Fixes: ad2cc62e77ef ("media: imx: add support for RGB565_2X8 on parallel bus") CC: Jan Luebbe <jlu@pengutronix.de> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> --- imx-media-csi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -440,7 +440,7 @@ static int csi_idmac_setup_channel(struc passthrough_bits = 8; passthrough_cycles = 2; break; - }; + } /* fallthrough */ default: burst_size = (image.pix.width & 0xf) ? 8 : 16; ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus 2018-05-03 16:41 ` [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus Jan Luebbe 2018-05-04 5:07 ` kbuild test robot 2018-05-04 6:44 ` [PATCH] media: imx: fix semicolon.cocci warnings kbuild test robot @ 2018-05-04 6:44 ` kbuild test robot 2018-05-04 8:37 ` kbuild test robot 3 siblings, 0 replies; 13+ messages in thread From: kbuild test robot @ 2018-05-04 6:44 UTC (permalink / raw) To: Jan Luebbe; +Cc: kbuild-all, linux-media, Jan Luebbe, slongerbeam, p.zabel Hi Jan, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.17-rc3 next-20180503] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Jan-Luebbe/media-imx-add-capture-support-for-RGB565_2X8-on-parallel-bus/20180504-120003 base: git://linuxtv.org/media_tree.git master coccinelle warnings: (new ones prefixed by >>) >> drivers/staging/media/imx/imx-media-csi.c:443:3-4: Unneeded semicolon Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus 2018-05-03 16:41 ` [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus Jan Luebbe ` (2 preceding siblings ...) 2018-05-04 6:44 ` [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus kbuild test robot @ 2018-05-04 8:37 ` kbuild test robot 3 siblings, 0 replies; 13+ messages in thread From: kbuild test robot @ 2018-05-04 8:37 UTC (permalink / raw) To: Jan Luebbe; +Cc: kbuild-all, linux-media, Jan Luebbe, slongerbeam, p.zabel Hi Jan, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.17-rc3 next-20180503] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Jan-Luebbe/media-imx-add-capture-support-for-RGB565_2X8-on-parallel-bus/20180504-120003 base: git://linuxtv.org/media_tree.git master reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) vim +652 drivers/staging/media/imx/imx-media-csi.c 640 641 /* Update the CSI whole sensor and active windows */ 642 static int csi_setup(struct csi_priv *priv) 643 { 644 struct v4l2_mbus_framefmt *infmt, *outfmt; 645 struct v4l2_mbus_config mbus_cfg; 646 struct v4l2_mbus_framefmt if_fmt; 647 struct imx_media_pixfmt *outcc; 648 struct v4l2_rect crop; 649 650 infmt = &priv->format_mbus[CSI_SINK_PAD]; 651 outfmt = &priv->format_mbus[priv->active_output_pad]; > 652 outcc = priv->cc[priv->active_output_pad]; 653 654 /* compose mbus_config from the upstream endpoint */ 655 mbus_cfg.type = priv->upstream_ep.bus_type; 656 mbus_cfg.flags = (priv->upstream_ep.bus_type == V4L2_MBUS_CSI2) ? 657 priv->upstream_ep.bus.mipi_csi2.flags : 658 priv->upstream_ep.bus.parallel.flags; 659 660 /* 661 * we need to pass input frame to CSI interface, but 662 * with translated field type from output format 663 */ 664 if_fmt = *infmt; 665 if_fmt.field = outfmt->field; 666 crop = priv->crop; 667 668 /* 669 * if cycles is set, we need to handle this over multiple cycles as 670 * generic/bayer data 671 */ 672 if ((priv->upstream_ep.bus_type != V4L2_MBUS_CSI2) && outcc->cycles) { 673 if_fmt.width *= outcc->cycles; 674 crop.width *= outcc->cycles; 675 } 676 677 ipu_csi_set_window(priv->csi, &crop); 678 679 ipu_csi_set_downsize(priv->csi, 680 priv->crop.width == 2 * priv->compose.width, 681 priv->crop.height == 2 * priv->compose.height); 682 683 ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt); 684 685 ipu_csi_set_dest(priv->csi, priv->dest); 686 687 if (priv->dest == IPU_CSI_DEST_IDMAC) 688 ipu_csi_set_skip_smfc(priv->csi, priv->skip->skip_smfc, 689 priv->skip->max_ratio - 1, 0); 690 691 ipu_csi_dump(priv->csi); 692 693 return 0; 694 } 695 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] media: imx: add capture support for RGB565_2X8 on parallel bus 2018-05-03 16:41 [PATCH 0/2] media: imx: add capture support for RGB565_2X8 on parallel bus Jan Luebbe 2018-05-03 16:41 ` [PATCH 1/2] media: imx: capture: refactor enum_/try_fmt Jan Luebbe 2018-05-03 16:41 ` [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus Jan Luebbe @ 2018-05-05 22:22 ` Steve Longerbeam 2018-05-07 14:23 ` Jan Lübbe 2 siblings, 1 reply; 13+ messages in thread From: Steve Longerbeam @ 2018-05-05 22:22 UTC (permalink / raw) To: Jan Luebbe, linux-media; +Cc: p.zabel Hi Jan, Philipp, I reviewed this patch series, and while I don't have any objections to the code-level changes, but my question is more specifically about how the IPU/CSI deals with receiving RGB565 over a parallel bus. My understanding was that if the CSI receives RGB565 over a parallel 8-bit sensor bus, the CSI_SENS_DATA_FORMAT register field is programmed to RGB565, and the CSI outputs ARGB8888. Then IDMAC component packing can be setup to write pixels to memory as RGB565. Does that not work? Assuming that above does not work (and indeed parallel RGB565 must be handled as pass-through), then I think support for capturing parallel RGB555 as pass-through should be added to this series as well. Also what about RGB565 over a 16-bit parallel sensor bus? The reference manual hints that perhaps this could be treated as non-passthrough ("on the fly processing"), e.g. could be passed on to the IC pre-processor: 16 bit RGB565 This is the only mode that allows on the fly processing of 16 bit data. In this mode the CSI is programmed to receive 16 bit generic data. In this mode the interface is restricted to be in "non-gated mode" and the CSI#_DATA_SOURCE bit has to be set If the external device is 24bit - the user can connect a 16 bit sample of it (RGB565 format). The IPU has to be configured in the same way as the case of CSI#_SENS_DATA_FORMAT=RGB565 Thanks, Steve On 05/03/2018 09:41 AM, Jan Luebbe wrote: > The IPU can only capture RGB565 with two 8-bit cycles in bayer/generic > mode on the parallel bus, compared to a specific mode on MIPI CSI-2. > To handle this, we extend imx_media_pixfmt with a cycles per pixel > field, which is used for generic formats on the parallel bus. > > Before actually adding RGB565_2X8 support for the parallel bus, this > series simplifies handing of the the different configurations for RGB565 > between parallel and MIPI CSI-2 in imx-media-capture. This avoids having > to explicitly pass on the format in the second patch. > > Jan Luebbe (2): > media: imx: capture: refactor enum_/try_fmt > media: imx: add support for RGB565_2X8 on parallel bus > > drivers/staging/media/imx/imx-media-capture.c | 38 +++++++-------- > drivers/staging/media/imx/imx-media-csi.c | 47 +++++++++++++++++-- > drivers/staging/media/imx/imx-media-utils.c | 1 + > drivers/staging/media/imx/imx-media.h | 2 + > 4 files changed, 63 insertions(+), 25 deletions(-) > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] media: imx: add capture support for RGB565_2X8 on parallel bus 2018-05-05 22:22 ` [PATCH 0/2] media: imx: add capture " Steve Longerbeam @ 2018-05-07 14:23 ` Jan Lübbe 2018-05-07 18:21 ` Steve Longerbeam 0 siblings, 1 reply; 13+ messages in thread From: Jan Lübbe @ 2018-05-07 14:23 UTC (permalink / raw) To: Steve Longerbeam, linux-media; +Cc: p.zabel Hi Steve, thanks for reviewing! On Sat, 2018-05-05 at 15:22 -0700, Steve Longerbeam wrote: > I reviewed this patch series, and while I don't have any > objections to the code-level changes, but my question > is more specifically about how the IPU/CSI deals with > receiving RGB565 over a parallel bus. > > My understanding was that if the CSI receives RGB565 > over a parallel 8-bit sensor bus, the CSI_SENS_DATA_FORMAT > register field is programmed to RGB565, and the CSI outputs > ARGB8888. Then IDMAC component packing can be setup to > write pixels to memory as RGB565. Does that not work? This was our first thought too. As far as we can see in our experiments, that mode doesn't actually work for the parallel bus. Philipp's interpretation is that this mode is only intended for use with the MIPI-CSI2 input. > Assuming that above does not work (and indeed parallel RGB565 > must be handled as pass-through), then I think support for capturing > parallel RGB555 as pass-through should be added to this series as > well. I don't have a sensor which produces RGB555, so it wouldn't be able to test it. > Also what about RGB565 over a 16-bit parallel sensor bus? The > reference manual hints that perhaps this could be treated as > non-passthrough ("on the fly processing"), e.g. could be passed > on to the IC pre-processor: > > 16 bit RGB565 > This is the only mode that allows on the fly processing of 16 bit data. In this mode the > CSI is programmed to receive 16 bit generic data. In this mode the interface is > restricted to be in "non-gated mode" and the CSI#_DATA_SOURCE bit has to be set > If the external device is 24bit - the user can connect a 16 bit sample of it (RGB565 > format). The IPU has to be configured in the same way as the case of > CSI#_SENS_DATA_FORMAT=RGB565 I've not looked at this case, as I don't have a sensor with that format either. :/ Thanks, Jan -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] media: imx: add capture support for RGB565_2X8 on parallel bus 2018-05-07 14:23 ` Jan Lübbe @ 2018-05-07 18:21 ` Steve Longerbeam 2018-05-08 14:13 ` Jan Lübbe 0 siblings, 1 reply; 13+ messages in thread From: Steve Longerbeam @ 2018-05-07 18:21 UTC (permalink / raw) To: Jan Lübbe, linux-media; +Cc: p.zabel On 05/07/2018 07:23 AM, Jan Lübbe wrote: > Hi Steve, > > thanks for reviewing! > > On Sat, 2018-05-05 at 15:22 -0700, Steve Longerbeam wrote: >> I reviewed this patch series, and while I don't have any >> objections to the code-level changes, but my question >> is more specifically about how the IPU/CSI deals with >> receiving RGB565 over a parallel bus. >> >> My understanding was that if the CSI receives RGB565 >> over a parallel 8-bit sensor bus, the CSI_SENS_DATA_FORMAT >> register field is programmed to RGB565, and the CSI outputs >> ARGB8888. Then IDMAC component packing can be setup to >> write pixels to memory as RGB565. Does that not work? > This was our first thought too. As far as we can see in our > experiments, that mode doesn't actually work for the parallel bus. > Philipp's interpretation is that this mode is only intended for use > with the MIPI-CSI2 input. Ok, that was my suspicion on reading this as well. And it's likely that the other sensor formats on parallel busses require pass-through, e.g. anything other than UYVY_2x8 and YUYV_2x8 requires pass-through. In other words, if the sensor bus is parallel, only 8-bit bus UYVY_2x8 and YUYV_2x8 can be routed to the IC pad or component packed/unpacked by the IDMAC pad. All other sensor formats on a parallel bus (8 or 16 bit) must be sent to IDMAC pad as pass-through. I think the code can be simplified/made more readable because of this, something like: static inline bool is_parallel_bus(struct v4l2_fwnode_endpoint *ep) { return ep->bus_type != V4L2_MBUS_CSI2; } static inline bool requires_pass_through( struct v4l2_fwnode_endpoint *ep, struct v4l2_mbus_framefmt *infmt, const struct imx_media_pixfmt *incc) { return incc->bayer || (is_parallel_bus(ep) && infmt->code != UYVY_2x8 && infmt->code != YUYV_2x8); } Then requires_pass_through() can be used everywhere we need to determine the pass-though requirement. Also, there's something wrong with the 'switch (image.pix.pixelformat) {...}' block in csi_idmac_setup_channel(). Pass-though, burst size, pass-though bits, should be determined by input media-bus code, not final capture V4L2 pix format. Steve >> Assuming that above does not work (and indeed parallel RGB565 >> must be handled as pass-through), then I think support for capturing >> parallel RGB555 as pass-through should be added to this series as >> well. > I don't have a sensor which produces RGB555, so it wouldn't be able to > test it. Understood, but for code readability and consistency I think the code can be cleaned up as above. >> Also what about RGB565 over a 16-bit parallel sensor bus? The >> reference manual hints that perhaps this could be treated as >> non-passthrough ("on the fly processing"), e.g. could be passed >> on to the IC pre-processor: >> >> 16 bit RGB565 >> This is the only mode that allows on the fly processing of 16 bit data. In this mode the >> CSI is programmed to receive 16 bit generic data. In this mode the interface is >> restricted to be in "non-gated mode" and the CSI#_DATA_SOURCE bit has to be set >> If the external device is 24bit - the user can connect a 16 bit sample of it (RGB565 >> format). The IPU has to be configured in the same way as the case of >> CSI#_SENS_DATA_FORMAT=RGB565 > I've not looked at this case, as I don't have a sensor with that format > either. :/ Ok, I was just curious if you knew anything more about this. Let's ignore it :) Steve ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] media: imx: add capture support for RGB565_2X8 on parallel bus 2018-05-07 18:21 ` Steve Longerbeam @ 2018-05-08 14:13 ` Jan Lübbe 2018-05-08 18:23 ` Steve Longerbeam 0 siblings, 1 reply; 13+ messages in thread From: Jan Lübbe @ 2018-05-08 14:13 UTC (permalink / raw) To: Steve Longerbeam, linux-media; +Cc: p.zabel Hi, On Mon, 2018-05-07 at 11:21 -0700, Steve Longerbeam wrote: > On 05/07/2018 07:23 AM, Jan Lübbe wrote: > > On Sat, 2018-05-05 at 15:22 -0700, Steve Longerbeam wrote: > > > I reviewed this patch series, and while I don't have any > > > objections to the code-level changes, but my question > > > is more specifically about how the IPU/CSI deals with > > > receiving RGB565 over a parallel bus. > > > > > > My understanding was that if the CSI receives RGB565 > > > over a parallel 8-bit sensor bus, the CSI_SENS_DATA_FORMAT > > > register field is programmed to RGB565, and the CSI outputs > > > ARGB8888. Then IDMAC component packing can be setup to > > > write pixels to memory as RGB565. Does that not work? > > > > This was our first thought too. As far as we can see in our > > experiments, that mode doesn't actually work for the parallel bus. > > Philipp's interpretation is that this mode is only intended for use > > with the MIPI-CSI2 input. > > Ok, that was my suspicion on reading this as well. And it's likely > that the other sensor formats on parallel busses require pass-through, > e.g. anything other than UYVY_2x8 and YUYV_2x8 requires > pass-through. OK. > In other words, if the sensor bus is parallel, only 8-bit bus UYVY_2x8 > and YUYV_2x8 can be routed to the IC pad or component packed/unpacked > by the IDMAC pad. All other sensor formats on a parallel bus (8 or 16 > bit) must be sent to IDMAC pad as pass-through. > > I think the code can be simplified/made more readable because of this, > something like: > > static inline bool is_parallel_bus(struct v4l2_fwnode_endpoint *ep) > { > return ep->bus_type != V4L2_MBUS_CSI2; > } > > static inline bool requires_pass_through( > struct v4l2_fwnode_endpoint *ep, > struct v4l2_mbus_framefmt *infmt, > const struct imx_media_pixfmt *incc) { > return incc->bayer || (is_parallel_bus(ep) && infmt->code != > UYVY_2x8 && infmt->code != YUYV_2x8); > } > > > Then requires_pass_through() can be used everywhere we need to > determine the pass-though requirement. OK, i've added these helper functions. In csi_link_validate() we don't have the infmt handy, but as the downstream elements check if they have a native format anyway, this check is redundant and so i've dropped it. > Also, there's something wrong with the 'switch (image.pix.pixelformat) > {...}' block in csi_idmac_setup_channel(). Pass-though, burst size, pass-though > bits, should be determined by input media-bus code, not final capture V4L2 pix > format. I just followed the existing code there, which already configures all of these. > > > Assuming that above does not work (and indeed parallel RGB565 > > > must be handled as pass-through), then I think support for capturing > > > parallel RGB555 as pass-through should be added to this series as > > > well. > > > > I don't have a sensor which produces RGB555, so it wouldn't be able to > > test it. > > Understood, but for code readability and consistency I think the code > can be cleaned up as above. Yes, i've changed that for v2. > > > Also what about RGB565 over a 16-bit parallel sensor bus? The > > > reference manual hints that perhaps this could be treated as > > > non-passthrough ("on the fly processing"), e.g. could be passed > > > on to the IC pre-processor: > > > > > > 16 bit RGB565 > > > This is the only mode that allows on the fly processing of 16 bit data. In this mode the > > > CSI is programmed to receive 16 bit generic data. In this mode the interface is > > > restricted to be in "non-gated mode" and the CSI#_DATA_SOURCE bit has to be set > > > If the external device is 24bit - the user can connect a 16 bit sample of it (RGB565 > > > format). The IPU has to be configured in the same way as the case of > > > CSI#_SENS_DATA_FORMAT=RGB565 > > > > I've not looked at this case, as I don't have a sensor with that format > > either. :/ > > Ok, I was just curious if you knew anything more about this. Let's > ignore it :) OK. :) Regards, Jan -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] media: imx: add capture support for RGB565_2X8 on parallel bus 2018-05-08 14:13 ` Jan Lübbe @ 2018-05-08 18:23 ` Steve Longerbeam 0 siblings, 0 replies; 13+ messages in thread From: Steve Longerbeam @ 2018-05-08 18:23 UTC (permalink / raw) To: Jan Lübbe, linux-media; +Cc: p.zabel Hi Jan, On 05/08/2018 07:13 AM, Jan Lübbe wrote: > Hi, > > On Mon, 2018-05-07 at 11:21 -0700, Steve Longerbeam wrote: >> In other words, if the sensor bus is parallel, only 8-bit bus UYVY_2x8 >> and YUYV_2x8 can be routed to the IC pad or component packed/unpacked >> by the IDMAC pad. All other sensor formats on a parallel bus (8 or 16 >> bit) must be sent to IDMAC pad as pass-through. >> >> I think the code can be simplified/made more readable because of this, >> something like: >> >> static inline bool is_parallel_bus(struct v4l2_fwnode_endpoint *ep) >> { >> return ep->bus_type != V4L2_MBUS_CSI2; >> } >> >> static inline bool requires_pass_through( >> struct v4l2_fwnode_endpoint *ep, >> struct v4l2_mbus_framefmt *infmt, >> const struct imx_media_pixfmt *incc) { >> return incc->bayer || (is_parallel_bus(ep) && infmt->code != >> UYVY_2x8 && infmt->code != YUYV_2x8); >> } >> >> >> Then requires_pass_through() can be used everywhere we need to >> determine the pass-though requirement. > OK, i've added these helper functions. In csi_link_validate() we don't > have the infmt handy, but as the downstream elements check if they have > a native format anyway, this check is redundant and so i've dropped it. Makes sense. > >> Also, there's something wrong with the 'switch (image.pix.pixelformat) >> {...}' block in csi_idmac_setup_channel(). Pass-though, burst size, pass-though >> bits, should be determined by input media-bus code, not final capture V4L2 pix >> format. > I just followed the existing code there, which already configures all > of these. Sorry never mind, I forgot that there is a need to check for planar formats here. >>>> Assuming that above does not work (and indeed parallel RGB565 >>>> must be handled as pass-through), then I think support for capturing >>>> parallel RGB555 as pass-through should be added to this series as >>>> well. >>> I don't have a sensor which produces RGB555, so it wouldn't be able to >>> test it. >> Understood, but for code readability and consistency I think the code >> can be cleaned up as above. > Yes, i've changed that for v2. > > The new macros can be used in more places. I will respond to v2 patch. Steve ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-05-08 18:23 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-03 16:41 [PATCH 0/2] media: imx: add capture support for RGB565_2X8 on parallel bus Jan Luebbe 2018-05-03 16:41 ` [PATCH 1/2] media: imx: capture: refactor enum_/try_fmt Jan Luebbe 2018-05-03 16:41 ` [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus Jan Luebbe 2018-05-04 5:07 ` kbuild test robot 2018-05-04 14:44 ` Jan Lübbe 2018-05-04 6:44 ` [PATCH] media: imx: fix semicolon.cocci warnings kbuild test robot 2018-05-04 6:44 ` [PATCH 2/2] media: imx: add support for RGB565_2X8 on parallel bus kbuild test robot 2018-05-04 8:37 ` kbuild test robot 2018-05-05 22:22 ` [PATCH 0/2] media: imx: add capture " Steve Longerbeam 2018-05-07 14:23 ` Jan Lübbe 2018-05-07 18:21 ` Steve Longerbeam 2018-05-08 14:13 ` Jan Lübbe 2018-05-08 18:23 ` Steve Longerbeam
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.