From: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
To: Sebastian Fricke <sebastian.fricke.linux@gmail.com>,
linux-media@vger.kernel.org
Cc: helen.koike@collabora.com, Hans Verkuil <hverkuil@xs4all.nl>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Tomasz Figa <tfiga@chromium.org>
Subject: Re: [PATCH v3] media: rkisp1: isp: Add the enum_frame_size ioctl
Date: Mon, 14 Dec 2020 08:30:32 +0100 [thread overview]
Message-ID: <4f93fbf8-94c1-0fbd-732e-ae27e990ffb2@collabora.com> (raw)
In-Reply-To: <20201212185306.19135-1-sebastian.fricke.linux@gmail.com>
Am 12.12.20 um 19:53 schrieb Sebastian Fricke:
> Implement the VIDIOC_SUBDEV_ENUM_FRAME_SIZE ioctl for the isp entity,
> check if the mbus code is valid for the given pad.
> This call is not available for the parameter or metadata pads of the RkISP1.
>
> Signed-off-by: Sebastian Fricke <sebastian.fricke.linux@gmail.com>
> ---
>
> v1: https://patchwork.kernel.org/project/linux-media/patch/20201206053935.24028-1-sebastian.fricke.linux@gmail.com/
> v2: https://patchwork.kernel.org/project/linux-media/patch/20201207184358.3793-1-sebastian.fricke.linux@gmail.com/
>
> Changes since v2:
> - Change the return code for the parameter and metadata pads to -ENOTTY instead of -EINVAL
> as this reports that the pad doesn't support the ioctl
> - Highlight more clearly that this patch is for the isp entity of the rkisp1, within the commit description
> as well as in the commit title
> - Add the correct v4l2-complience output to the patch mail
>
> Changes since v1:
> - Replace the custom bus code check with the `rkisp1_isp_mbus_info_get`
> function
> - Add a missing line break above the function
>
> I have tested this patch with the following script:
> http://paste.debian.net/1176614/
>
> The results on my NanoPC-T4 (Linux nanopct4 5.10.0-rc6-rockchip64):
>
> pad 0 = RKISP1_ISP_PAD_SINK_VIDEO
> pad 1 = RKISP1_ISP_PAD_SINK_PARAMS
> pad 2 = RKISP1_ISP_PAD_SOURCE_VIDEO
> pad 3 = RKISP1_ISP_PAD_SOURCE_STATS
>
> basti@nanopct4:~$ python3 rkisp1_enum_frame_size_test.py
> TEST 0: pad 0 - code 0x300f - size 32x32 - 4032x3024
> TEST 1: pad 0 - code 0x3007 - size 32x32 - 4032x3024
> TEST 2: pad 0 - code 0x300e - size 32x32 - 4032x3024
> TEST 3: pad 0 - code 0x300a - size 32x32 - 4032x3024
> TEST 4: pad 0 - code 0x3012 - size 32x32 - 4032x3024
> TEST 5: pad 0 - code 0x3008 - size 32x32 - 4032x3024
> TEST 6: pad 0 - code 0x3010 - size 32x32 - 4032x3024
> TEST 7: pad 0 - code 0x3011 - size 32x32 - 4032x3024
> TEST 8: pad 0 - code 0x3014 - size 32x32 - 4032x3024
> TEST 9: pad 0 - code 0x3001 - size 32x32 - 4032x3024
> TEST 10: pad 0 - code 0x3013 - size 32x32 - 4032x3024
> TEST 11: pad 0 - code 0x3002 - size 32x32 - 4032x3024
> TEST 12: pad 0 - code 0x2011 - size 32x32 - 4032x3024
> TEST 13: pad 0 - code 0x2012 - size 32x32 - 4032x3024
> TEST 14: pad 0 - code 0x200f - size 32x32 - 4032x3024
> TEST 15: pad 0 - code 0x2010 - size 32x32 - 4032x3024
> TEST 16: pad 1 - code 0x7001 - size /
> TEST 17: pad 2 - code 0x2008 - size 32x32 - 4032x3024
> TEST 18: pad 2 - code 0x300f - size 32x32 - 4032x3024
> TEST 19: pad 2 - code 0x3007 - size 32x32 - 4032x3024
> TEST 20: pad 2 - code 0x300e - size 32x32 - 4032x3024
> TEST 21: pad 2 - code 0x300a - size 32x32 - 4032x3024
> TEST 22: pad 2 - code 0x3012 - size 32x32 - 4032x3024
> TEST 23: pad 2 - code 0x3008 - size 32x32 - 4032x3024
> TEST 24: pad 2 - code 0x3010 - size 32x32 - 4032x3024
> TEST 25: pad 2 - code 0x3011 - size 32x32 - 4032x3024
> TEST 26: pad 2 - code 0x3014 - size 32x32 - 4032x3024
> TEST 27: pad 2 - code 0x3001 - size 32x32 - 4032x3024
> TEST 28: pad 2 - code 0x3013 - size 32x32 - 4032x3024
> TEST 29: pad 2 - code 0x3002 - size 32x32 - 4032x3024
> TEST 30: pad 3 - code 0x7001 - size /
> TEST 31: pad 0 - code 0xdead - size / (test with an invalid media bus code)
> TEST 32: pad 6 - code 0x300f - size / (test with an invalid pad)
> TEST 33: pad 0 - code 0x2008 - size / (test with a format that is not supported by the pad)
> TEST 34: pad 2 - code 0x2010 - size / (test with a format that is not supported by the pad)
>
> And here is the output from `v4l2-compliance --device /dev/v4l-subdev0`:
> Media Driver Info:
> Driver name : rkisp1
> ...
> Name : rkisp1_isp
>
> ...
>
> Sub-Device ioctls (Sink Pad 0):
> test Try VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
> test Active VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
> ...
>
> Sub-Device ioctls (Sink Pad 1):
> test Try VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
> fail: v4l2-test-subdevs.cpp(303): fmt.width == 0 || fmt.width > 65536
> fail: v4l2-test-subdevs.cpp(348): checkMBusFrameFmt(node, fmt.format)
> test Active VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
> fail: v4l2-test-subdevs.cpp(303): fmt.width == 0 || fmt.width > 65536
> fail: v4l2-test-subdevs.cpp(348): checkMBusFrameFmt(node, fmt.format)
> ...
>
> Sub-Device ioctls (Source Pad 2):
> test Try VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
> test Active VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
> ...
>
> Sub-Device ioctls (Source Pad 3):
> test Try VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
> fail: v4l2-test-subdevs.cpp(303): fmt.width == 0 || fmt.width > 65536
> fail: v4l2-test-subdevs.cpp(348): checkMBusFrameFmt(node, fmt.format)
> test Active VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
> fail: v4l2-test-subdevs.cpp(303): fmt.width == 0 || fmt.width > 65536
> fail: v4l2-test-subdevs.cpp(348): checkMBusFrameFmt(node, fmt.format)
> ...
> ...
> As reported by Dafna Hirschfeld from v2 the errors on pad 1 & 3 are caused by a
> problem within v4l2-utils.
>
> ---
> .../platform/rockchip/rkisp1/rkisp1-isp.c | 34 +++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index 889982d8ca41..2e5b57e3aedc 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -600,6 +600,39 @@ static int rkisp1_isp_enum_mbus_code(struct v4l2_subdev *sd,
> return -EINVAL;
> }
>
> +static int rkisp1_isp_enum_frame_size(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_frame_size_enum *fse)
> +{
> + const struct rkisp1_isp_mbus_info *mbus_info;
> +
> + if (fse->pad == RKISP1_ISP_PAD_SINK_PARAMS ||
> + fse->pad == RKISP1_ISP_PAD_SOURCE_STATS)
> + return -ENOTTY;
> +
> + if (fse->index > 0)
> + return -EINVAL;
> +
> + mbus_info = rkisp1_isp_mbus_info_get(fse->code);
> + if (!mbus_info)
> + return -EINVAL;
> +
> + if (!(mbus_info->direction & RKISP1_ISP_SD_SINK) &&
> + fse->pad == RKISP1_ISP_PAD_SINK_VIDEO)
> + return -EINVAL;
> +
> + if (!(mbus_info->direction & RKISP1_ISP_SD_SRC) &&
> + fse->pad == RKISP1_ISP_PAD_SOURCE_VIDEO)
> + return -EINVAL;
> +
> + fse->min_width = RKISP1_ISP_MIN_WIDTH;
> + fse->max_width = RKISP1_ISP_MAX_WIDTH;
> + fse->min_height = RKISP1_ISP_MIN_HEIGHT;
> + fse->max_height = RKISP1_ISP_MAX_HEIGHT;
> +
> + return 0;
> +}
> +
> static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
> struct v4l2_subdev_pad_config *cfg)
> {
> @@ -880,6 +913,7 @@ static int rkisp1_subdev_link_validate(struct media_link *link)
>
> static const struct v4l2_subdev_pad_ops rkisp1_isp_pad_ops = {
> .enum_mbus_code = rkisp1_isp_enum_mbus_code,
> + .enum_frame_size = rkisp1_isp_enum_frame_size,
> .get_selection = rkisp1_isp_get_selection,
> .set_selection = rkisp1_isp_set_selection,
> .init_cfg = rkisp1_isp_init_config,
>
Acked-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
next prev parent reply other threads:[~2020-12-14 7:31 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-12 18:53 [PATCH v3] media: rkisp1: isp: Add the enum_frame_size ioctl Sebastian Fricke
2020-12-14 7:30 ` Dafna Hirschfeld [this message]
2020-12-14 11:52 ` Helen Koike
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4f93fbf8-94c1-0fbd-732e-ae27e990ffb2@collabora.com \
--to=dafna.hirschfeld@collabora.com \
--cc=helen.koike@collabora.com \
--cc=hverkuil@xs4all.nl \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=sebastian.fricke.linux@gmail.com \
--cc=tfiga@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox