| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Ytcu5UqMfOPfkHEu@pendragon.ideasonboard.com>
Date: Wed, 20 Jul 2022 01:23:33 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Paul Elder <paul.elder@...asonboard.com>
Cc: linux-media@...r.kernel.org, Dafna Hirschfeld <dafna@...tmail.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Heiko Stuebner <heiko@...ech.de>,
linux-rockchip@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] media: rkisp1: Add YC swap capability
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@...asonboard.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@...asonboard.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[] = {
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists