| Message ID | 20220714112603.1117335-2-paul.elder@ideasonboard.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: rkisp1: Add support for UYVY output | expand |
Hi Paul, Thank you for the patch. On Thu, Jul 14, 2022 at 08:26:02PM +0900, Paul Elder wrote: > The ISP version in the i.MX8MP has an MI_OUTPUT_ALIGN_FORMAT register > that the rk3399 does not have. This register allows swapping bytes, > which can be used to implement UYVY from YUYV. > > Add a feature flag to signify the presence of this feature, and add it > to the i.MX8MP match data. Also add it as a flag to the format info in > the list of supported formats by the capture v4l2 devices, and update > enum_fmt and s_fmt to take it into account. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > .../platform/rockchip/rkisp1/rkisp1-capture.c | 27 ++++++++++++++----- > .../platform/rockchip/rkisp1/rkisp1-common.h | 1 + > .../platform/rockchip/rkisp1/rkisp1-dev.c | 3 ++- > 3 files changed, 23 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > index c494afbc21b4..85fd85fe208c 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > @@ -47,13 +47,15 @@ enum rkisp1_plane { > * @fourcc: pixel format > * @fmt_type: helper filed for pixel format > * @uv_swap: if cb cr swapped, for yuv > + * @yc_swap: if y and cb/cr swapped, for yuv > * @write_format: defines how YCbCr self picture data is written to memory > * @output_format: defines sp output format > * @mbus: the mbus code on the src resizer pad that matches the pixel format > */ > struct rkisp1_capture_fmt_cfg { > u32 fourcc; > - u8 uv_swap; > + u32 uv_swap : 1; > + u32 yc_swap : 1; > u32 write_format; > u32 output_format; > u32 mbus; > @@ -1150,9 +1152,13 @@ static const struct rkisp1_capture_fmt_cfg * > rkisp1_find_fmt_cfg(const struct rkisp1_capture *cap, const u32 pixelfmt) > { > unsigned int i; > + bool yc_swap_support = > + cap->rkisp1->info->features & RKISP1_FEATURE_MI_OUTPUT_ALIGN; Could you move this before the previous line ? I'm tempted to write a patch at some point that will simplify this with a rkisp1_has_feature() that will... scratch that, https://lore.kernel.org/linux-media/20220719221751.4159-1-laurent.pinchart@ideasonboard.com :-) Could you rebase for v2 ? > > for (i = 0; i < cap->config->fmt_size; i++) { > - if (cap->config->fmts[i].fourcc == pixelfmt) > + const struct rkisp1_capture_fmt_cfg *fmt = &cap->config->fmts[i]; Missing blank line. I thought checkpatch.pl would warn about this. > + if (fmt->fourcc == pixelfmt && > + (!fmt->yc_swap || yc_swap_support)) > return &cap->config->fmts[i]; > } > return NULL; > @@ -1223,22 +1229,29 @@ static int rkisp1_enum_fmt_vid_cap_mplane(struct file *file, void *priv, > struct rkisp1_capture *cap = video_drvdata(file); > const struct rkisp1_capture_fmt_cfg *fmt = NULL; > unsigned int i, n = 0; > + bool yc_swap_support = > + cap->rkisp1->info->features & RKISP1_FEATURE_MI_OUTPUT_ALIGN; > > - if (!f->mbus_code) { > - if (f->index >= cap->config->fmt_size) > - return -EINVAL; > + if (f->index >= cap->config->fmt_size) > + return -EINVAL; > > + if (!f->mbus_code && yc_swap_support) { > fmt = &cap->config->fmts[f->index]; > f->pixelformat = fmt->fourcc; > return 0; > } I'm tempted to drop this optimization, as it makes the code more complex and only brings a small improvement to an ioctl that shouldn't be in a hot path, but that's up to you. > > for (i = 0; i < cap->config->fmt_size; i++) { > - if (cap->config->fmts[i].mbus != f->mbus_code) > + fmt = &cap->config->fmts[i]; > + > + if (!!f->mbus_code && fmt->mbus != f->mbus_code) You can s/!!// Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + continue; > + > + if (!yc_swap_support && fmt->yc_swap) > continue; > > if (n++ == f->index) { > - f->pixelformat = cap->config->fmts[i].fourcc; > + f->pixelformat = fmt->fourcc; > return 0; > } > } > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > index 38be565341d6..b0f9221a1922 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > @@ -119,6 +119,7 @@ enum rkisp1_feature { > RKISP1_FEATURE_MAIN_STRIDE = BIT(3), > RKISP1_FEATURE_DMA_34BIT = BIT(4), > RKISP1_FEATURE_SELF_PATH = BIT(5), > + RKISP1_FEATURE_MI_OUTPUT_ALIGN = BIT(6), > }; > > /* > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > index 3b549c07a9bb..a475933820fd 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > @@ -520,7 +520,8 @@ static const struct rkisp1_info imx8mp_isp_info = { > .isp_ver = IMX8MP_V10, > .features = RKISP1_FEATURE_RSZ_CROP > | RKISP1_FEATURE_MAIN_STRIDE > - | RKISP1_FEATURE_DMA_34BIT, > + | RKISP1_FEATURE_DMA_34BIT > + | RKISP1_FEATURE_MI_OUTPUT_ALIGN, > }; > > static const struct of_device_id rkisp1_of_match[] = {
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c index c494afbc21b4..85fd85fe208c 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c @@ -47,13 +47,15 @@ enum rkisp1_plane { * @fourcc: pixel format * @fmt_type: helper filed for pixel format * @uv_swap: if cb cr swapped, for yuv + * @yc_swap: if y and cb/cr swapped, for yuv * @write_format: defines how YCbCr self picture data is written to memory * @output_format: defines sp output format * @mbus: the mbus code on the src resizer pad that matches the pixel format */ struct rkisp1_capture_fmt_cfg { u32 fourcc; - u8 uv_swap; + u32 uv_swap : 1; + u32 yc_swap : 1; u32 write_format; u32 output_format; u32 mbus; @@ -1150,9 +1152,13 @@ static const struct rkisp1_capture_fmt_cfg * rkisp1_find_fmt_cfg(const struct rkisp1_capture *cap, const u32 pixelfmt) { unsigned int i; + bool yc_swap_support = + cap->rkisp1->info->features & RKISP1_FEATURE_MI_OUTPUT_ALIGN; for (i = 0; i < cap->config->fmt_size; i++) { - if (cap->config->fmts[i].fourcc == pixelfmt) + const struct rkisp1_capture_fmt_cfg *fmt = &cap->config->fmts[i]; + if (fmt->fourcc == pixelfmt && + (!fmt->yc_swap || yc_swap_support)) return &cap->config->fmts[i]; } return NULL; @@ -1223,22 +1229,29 @@ static int rkisp1_enum_fmt_vid_cap_mplane(struct file *file, void *priv, struct rkisp1_capture *cap = video_drvdata(file); const struct rkisp1_capture_fmt_cfg *fmt = NULL; unsigned int i, n = 0; + bool yc_swap_support = + cap->rkisp1->info->features & RKISP1_FEATURE_MI_OUTPUT_ALIGN; - if (!f->mbus_code) { - if (f->index >= cap->config->fmt_size) - return -EINVAL; + if (f->index >= cap->config->fmt_size) + return -EINVAL; + if (!f->mbus_code && yc_swap_support) { fmt = &cap->config->fmts[f->index]; f->pixelformat = fmt->fourcc; return 0; } for (i = 0; i < cap->config->fmt_size; i++) { - if (cap->config->fmts[i].mbus != f->mbus_code) + fmt = &cap->config->fmts[i]; + + if (!!f->mbus_code && fmt->mbus != f->mbus_code) + continue; + + if (!yc_swap_support && fmt->yc_swap) continue; if (n++ == f->index) { - f->pixelformat = cap->config->fmts[i].fourcc; + f->pixelformat = fmt->fourcc; return 0; } } diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h index 38be565341d6..b0f9221a1922 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h @@ -119,6 +119,7 @@ enum rkisp1_feature { RKISP1_FEATURE_MAIN_STRIDE = BIT(3), RKISP1_FEATURE_DMA_34BIT = BIT(4), RKISP1_FEATURE_SELF_PATH = BIT(5), + RKISP1_FEATURE_MI_OUTPUT_ALIGN = BIT(6), }; /* diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c index 3b549c07a9bb..a475933820fd 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c @@ -520,7 +520,8 @@ static const struct rkisp1_info imx8mp_isp_info = { .isp_ver = IMX8MP_V10, .features = RKISP1_FEATURE_RSZ_CROP | RKISP1_FEATURE_MAIN_STRIDE - | RKISP1_FEATURE_DMA_34BIT, + | RKISP1_FEATURE_DMA_34BIT + | RKISP1_FEATURE_MI_OUTPUT_ALIGN, }; static const struct of_device_id rkisp1_of_match[] = {
The ISP version in the i.MX8MP has an MI_OUTPUT_ALIGN_FORMAT register that the rk3399 does not have. This register allows swapping bytes, which can be used to implement UYVY from YUYV. Add a feature flag to signify the presence of this feature, and add it to the i.MX8MP match data. Also add it as a flag to the format info in the list of supported formats by the capture v4l2 devices, and update enum_fmt and s_fmt to take it into account. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- .../platform/rockchip/rkisp1/rkisp1-capture.c | 27 ++++++++++++++----- .../platform/rockchip/rkisp1/rkisp1-common.h | 1 + .../platform/rockchip/rkisp1/rkisp1-dev.c | 3 ++- 3 files changed, 23 insertions(+), 8 deletions(-)