From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Cc: linux-media@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, linux-staging@lists.linux.dev, Mauro Carvalho Chehab <mchehab@kernel.org>, Chen-Yu Tsai <wens@csie.org>, Jernej Skrabec <jernej.skrabec@gmail.com>, Samuel Holland <samuel@sholland.org>, Adam Pigg <adam@piggz.co.uk>, Thomas Petazzoni <thomas.petazzoni@bootlin.com> Subject: Re: [PATCH 8/9] media: sun6i-isp: capture: Implement MC I/O with extended enum_fmt Date: Sat, 25 Mar 2023 23:42:12 +0200 [thread overview] Message-ID: <20230325214212.GF22214@pendragon.ideasonboard.com> (raw) In-Reply-To: <20230324151228.2778112-9-paul.kocialkowski@bootlin.com> Hi Paul, Thank you for the patch. On Fri, Mar 24, 2023 at 04:12:27PM +0100, Paul Kocialkowski wrote: > This driver needs the media-controller API to operate and should not be > considered as a video-device-centric implementation. > > Properly report the IO_MC device cap and extend the enum_fmt > implementation to support enumeration with an explicit mbus_code. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > .../staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c > index 1595a9607775..5160b93b69ff 100644 > --- a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c > +++ b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c > @@ -439,6 +439,12 @@ static int sun6i_isp_capture_enum_fmt(struct file *file, void *private, > struct v4l2_fmtdesc *fmtdesc) > { > u32 index = fmtdesc->index; > + u32 mbus_code = fmtdesc->mbus_code; > + > + if (mbus_code && !sun6i_isp_proc_format_find(mbus_code)) > + return -EINVAL; > + This doesn't look right. As far as I understand, sun6i_isp_proc_format_find() looks up media bus codes for the ISP sink pad. Here you enuemrate pixel formats of the ISP output, so the media bus code given by userspace corresponds to the ISP source pad. I've had a look at sun6i_isp_proc_set_fmt() to see what media bus codes are used on the ISP output, and couldn't figure it out as it seems incorrectly implemented :-) The function doesn't check format->pad. > + /* Capture format is independent from proc format. */ > > if (index >= ARRAY_SIZE(sun6i_isp_capture_formats)) > return -EINVAL; > @@ -685,7 +691,8 @@ int sun6i_isp_capture_setup(struct sun6i_isp_device *isp_dev) > > strscpy(video_dev->name, SUN6I_ISP_CAPTURE_NAME, > sizeof(video_dev->name)); > - video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING; > + video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING | > + V4L2_CAP_IO_MC; > video_dev->vfl_dir = VFL_DIR_RX; > video_dev->release = video_device_release_empty; > video_dev->fops = &sun6i_isp_capture_fops; -- Regards, Laurent Pinchart
WARNING: multiple messages have this Message-ID (diff)
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Cc: linux-media@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, linux-staging@lists.linux.dev, Mauro Carvalho Chehab <mchehab@kernel.org>, Chen-Yu Tsai <wens@csie.org>, Jernej Skrabec <jernej.skrabec@gmail.com>, Samuel Holland <samuel@sholland.org>, Adam Pigg <adam@piggz.co.uk>, Thomas Petazzoni <thomas.petazzoni@bootlin.com> Subject: Re: [PATCH 8/9] media: sun6i-isp: capture: Implement MC I/O with extended enum_fmt Date: Sat, 25 Mar 2023 23:42:12 +0200 [thread overview] Message-ID: <20230325214212.GF22214@pendragon.ideasonboard.com> (raw) In-Reply-To: <20230324151228.2778112-9-paul.kocialkowski@bootlin.com> Hi Paul, Thank you for the patch. On Fri, Mar 24, 2023 at 04:12:27PM +0100, Paul Kocialkowski wrote: > This driver needs the media-controller API to operate and should not be > considered as a video-device-centric implementation. > > Properly report the IO_MC device cap and extend the enum_fmt > implementation to support enumeration with an explicit mbus_code. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > .../staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c > index 1595a9607775..5160b93b69ff 100644 > --- a/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c > +++ b/drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_capture.c > @@ -439,6 +439,12 @@ static int sun6i_isp_capture_enum_fmt(struct file *file, void *private, > struct v4l2_fmtdesc *fmtdesc) > { > u32 index = fmtdesc->index; > + u32 mbus_code = fmtdesc->mbus_code; > + > + if (mbus_code && !sun6i_isp_proc_format_find(mbus_code)) > + return -EINVAL; > + This doesn't look right. As far as I understand, sun6i_isp_proc_format_find() looks up media bus codes for the ISP sink pad. Here you enuemrate pixel formats of the ISP output, so the media bus code given by userspace corresponds to the ISP source pad. I've had a look at sun6i_isp_proc_set_fmt() to see what media bus codes are used on the ISP output, and couldn't figure it out as it seems incorrectly implemented :-) The function doesn't check format->pad. > + /* Capture format is independent from proc format. */ > > if (index >= ARRAY_SIZE(sun6i_isp_capture_formats)) > return -EINVAL; > @@ -685,7 +691,8 @@ int sun6i_isp_capture_setup(struct sun6i_isp_device *isp_dev) > > strscpy(video_dev->name, SUN6I_ISP_CAPTURE_NAME, > sizeof(video_dev->name)); > - video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING; > + video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING | > + V4L2_CAP_IO_MC; > video_dev->vfl_dir = VFL_DIR_RX; > video_dev->release = video_device_release_empty; > video_dev->fops = &sun6i_isp_capture_fops; -- Regards, Laurent Pinchart _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-03-25 21:42 UTC|newest] Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-03-24 15:12 [PATCH 0/9] media: sun6i-csi/isp: Implement MC I/O support Paul Kocialkowski 2023-03-24 15:12 ` Paul Kocialkowski 2023-03-24 15:12 ` [PATCH 1/9] media: v4l2: Add RGB565X pixel format to v4l2 format info Paul Kocialkowski 2023-03-24 15:12 ` Paul Kocialkowski 2023-03-25 7:06 ` Jernej Škrabec 2023-03-25 7:06 ` Jernej Škrabec 2023-03-25 20:59 ` Laurent Pinchart 2023-03-25 20:59 ` Laurent Pinchart 2023-03-24 15:12 ` [PATCH 2/9] media: v4l2: Add NV12_16L16 " Paul Kocialkowski 2023-03-24 15:12 ` Paul Kocialkowski 2023-03-25 7:06 ` Jernej Škrabec 2023-03-25 7:06 ` Jernej Škrabec 2023-03-25 21:01 ` Laurent Pinchart 2023-03-25 21:01 ` Laurent Pinchart 2023-03-31 18:54 ` Nicolas Dufresne 2023-03-31 18:54 ` Nicolas Dufresne 2023-04-05 4:21 ` Laurent Pinchart 2023-04-05 4:21 ` Laurent Pinchart 2023-04-11 13:03 ` Nicolas Dufresne 2023-04-11 13:03 ` Nicolas Dufresne 2023-04-11 15:30 ` Nicolas Dufresne 2023-04-11 15:30 ` Nicolas Dufresne 2023-03-24 15:12 ` [PATCH 3/9] media: v4l2: Introduce compressed pixel encoding definition and helper Paul Kocialkowski 2023-03-24 15:12 ` Paul Kocialkowski 2023-03-25 7:07 ` Jernej Škrabec 2023-03-25 7:07 ` Jernej Škrabec 2023-03-24 15:12 ` [PATCH 4/9] media: v4l2: Add JPEG pixel format to v4l2 format info Paul Kocialkowski 2023-03-24 15:12 ` Paul Kocialkowski 2023-03-25 7:08 ` Jernej Škrabec 2023-03-25 7:08 ` Jernej Škrabec 2023-03-31 19:07 ` Nicolas Dufresne 2023-03-31 19:07 ` Nicolas Dufresne 2023-03-24 15:12 ` [PATCH 5/9] media: sun6i-csi: capture: Rework and separate format validation Paul Kocialkowski 2023-03-24 15:12 ` Paul Kocialkowski 2023-03-25 7:09 ` Jernej Škrabec 2023-03-25 7:09 ` Jernej Škrabec 2023-03-25 21:24 ` Laurent Pinchart 2023-03-25 21:24 ` Laurent Pinchart 2023-03-24 15:12 ` [PATCH 6/9] media: sun6i-csi: capture: Implement MC I/O with extended enum_fmt Paul Kocialkowski 2023-03-24 15:12 ` Paul Kocialkowski 2023-03-25 7:13 ` Jernej Škrabec 2023-03-25 7:13 ` Jernej Škrabec 2023-03-25 21:29 ` Laurent Pinchart 2023-03-25 21:29 ` Laurent Pinchart 2023-03-24 15:12 ` [PATCH 7/9] media: sun6i-csi: capture: Implement enum_framesizes Paul Kocialkowski 2023-03-24 15:12 ` Paul Kocialkowski 2023-03-25 7:13 ` Jernej Škrabec 2023-03-25 7:13 ` Jernej Škrabec 2023-03-25 21:33 ` Laurent Pinchart 2023-03-25 21:33 ` Laurent Pinchart 2023-03-24 15:12 ` [PATCH 8/9] media: sun6i-isp: capture: Implement MC I/O with extended enum_fmt Paul Kocialkowski 2023-03-24 15:12 ` Paul Kocialkowski 2023-03-25 7:14 ` Jernej Škrabec 2023-03-25 7:14 ` Jernej Škrabec 2023-03-25 21:42 ` Laurent Pinchart [this message] 2023-03-25 21:42 ` Laurent Pinchart 2023-03-24 15:12 ` [PATCH 9/9] media: sun6i-isp: capture: Implement enum_framesizes Paul Kocialkowski 2023-03-24 15:12 ` Paul Kocialkowski 2023-03-25 7:14 ` Jernej Škrabec 2023-03-25 7:14 ` Jernej Škrabec
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=20230325214212.GF22214@pendragon.ideasonboard.com \ --to=laurent.pinchart@ideasonboard.com \ --cc=adam@piggz.co.uk \ --cc=jernej.skrabec@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=linux-staging@lists.linux.dev \ --cc=linux-sunxi@lists.linux.dev \ --cc=mchehab@kernel.org \ --cc=paul.kocialkowski@bootlin.com \ --cc=samuel@sholland.org \ --cc=thomas.petazzoni@bootlin.com \ --cc=wens@csie.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.