From: <yuji2.ishikawa@toshiba.co.jp> To: <laurent.pinchart@ideasonboard.com> Cc: <hverkuil@xs4all.nl>, <mchehab@kernel.org>, <nobuhiro1.iwamatsu@toshiba.co.jp>, <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>, <rafael.j.wysocki@intel.com>, <broonie@kernel.org>, <linux-media@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org> Subject: RE: [PATCH v5 3/6] media: platform: visconti: Add Toshiba Visconti Video Input Interface driver user interace Date: Thu, 2 Feb 2023 04:52:56 +0000 [thread overview] Message-ID: <TYAPR01MB6201A88B85E74C3EA3481BD092D69@TYAPR01MB6201.jpnprd01.prod.outlook.com> (raw) In-Reply-To: <Y9LnWfZb+v17+h+X@pendragon.ideasonboard.com> Hello Laurent, > -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Friday, January 27, 2023 5:49 AM > To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開) > <yuji2.ishikawa@toshiba.co.jp> > Cc: hverkuil@xs4all.nl; mchehab@kernel.org; iwamatsu nobuhiro(岩松 信洋 □ > SWC◯ACT) <nobuhiro1.iwamatsu@toshiba.co.jp>; robh+dt@kernel.org; > krzysztof.kozlowski+dt@linaro.org; rafael.j.wysocki@intel.com; > broonie@kernel.org; linux-media@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org > Subject: Re: [PATCH v5 3/6] media: platform: visconti: Add Toshiba Visconti > Video Input Interface driver user interace > > Hello Ishikawa-san, > > On Wed, Jan 25, 2023 at 10:20:27AM +0000, yuji2.ishikawa@toshiba.co.jp wrote: > > On Wednesday, January 18, 2023 10:04 AM, Laurent Pinchart wrote: > > > On Tue, Jan 17, 2023 at 12:47:10PM +0100, Hans Verkuil wrote: > > > > On 11/01/2023 03:24, Yuji Ishikawa wrote: > > > > > Add support to Video Input Interface on Toshiba Visconti ARM SoCs. > > > > > The interface device includes CSI2 Receiver, frame grabber, > > > > > video DMAC and image signal processor. > > > > > This patch provides the user interface layer. > > > > > > > > > > A driver instance provides three /dev/videoX device files; one > > > > > for RGB image capture, another one for optional RGB capture with > > > > > different parameters and the last one for RAW capture. > > > > > > > > > > Through the device files, the driver provides streaming (DMA-BUF) > interface. > > > > > A userland application should feed DMA-BUF instances for capture > buffers. > > > > > > > > > > The driver is based on media controller framework. > > > > > Its operations are roughly mapped to two subdrivers; one for ISP > > > > > and > > > > > CSI2 receiver (yields 1 instance), the other for capture (yields > > > > > 3 instances for each capture mode). > > > > > > > > > > Signed-off-by: Yuji Ishikawa <yuji2.ishikawa@toshiba.co.jp> > > > > > --- > > > > > Changelog v2: > > > > > - Resend v1 because a patch exceeds size limit. > > > > > > > > > > Changelog v3: > > > > > - Adapted to media control framework > > > > > - Introduced ISP subdevice, capture device > > > > > - Remove private IOCTLs and add vendor specific V4L2 controls > > > > > - Change function name avoiding camelcase and uppercase letters > > > > > > > > > > Changelog v4: > > > > > - Split patches because the v3 patch exceeds size limit > > > > > - Stop using ID number to identify driver instance: > > > > > - Use dynamically allocated structure to hold HW specific context, > > > > > instead of static one. > > > > > - Call HW layer functions with the context structure instead > > > > > of ID number > > > > > - Use pm_runtime to trigger initialization of HW > > > > > along with open/close of device files. > > > > > > > > > > Changelog v5: > > > > > - Fix coding style problems in viif.c > > > > > --- > > > > > drivers/media/platform/visconti/Makefile | 1 + > > > > > drivers/media/platform/visconti/viif.c | 545 ++++++++ > > > > > drivers/media/platform/visconti/viif.h | 203 +++ > > > > > .../media/platform/visconti/viif_capture.c | 1201 > +++++++++++++++++ > > > > > drivers/media/platform/visconti/viif_isp.c | 846 > ++++++++++++ > > > > > 5 files changed, 2796 insertions(+) create mode 100644 > > > > > drivers/media/platform/visconti/viif.c > > > > > create mode 100644 drivers/media/platform/visconti/viif.h > > > > > create mode 100644 > > > > > drivers/media/platform/visconti/viif_capture.c > > > > > create mode 100644 drivers/media/platform/visconti/viif_isp.c > > > > > > [snip] > > > > > > > > +static int viif_s_edid(struct file *file, void *fh, struct > > > > > +v4l2_edid *edid) { > > > > > + struct viif_device *viif_dev = > video_drvdata_to_capdev(file)->viif_dev; > > > > > + struct viif_subdev *viif_sd = viif_dev->sd; > > > > > + > > > > > + return v4l2_subdev_call(viif_sd->v4l2_sd, pad, set_edid, > > > > > +edid); } > > > > > > > > Has this driver been tested with an HDMI receiver? If not, then I > > > > would recommend dropping support for it until you actually can > > > > test with such hardware. > > > > > > > > The DV_TIMINGS API is for HDMI/DVI/DisplayPort etc. interfaces, > > > > it's not meant for CSI and similar interfaces. > > > > > > More than that, for MC-based drivers, the video node should *never* > > > forward ioctls to a connected subdev. The *only* valid calls to > > > v4l2_subdev_call() in this file are > > > > > > - to video.s_stream() in the start and stop streaming handler > > > > > > - to pad.g_fmt() when starting streaming to validate that the connected > > > subdev outputs a format compatible with the format set on the video > > > capture device > > > > > > That's it, nothing else, all other calls to v4l2_subdev_call() must > > > be dropped from the implementation of the video_device. > > > > Thank you for your comment. I understand the restriction. > > I'll remove following functions corresponding to ioctls. > > > > * viif_enum_input > > * viif_g_selection > > * viif_s_selection > > * viif_dv_timings_cap > > * viif_enum_dv_timings > > * viif_g_dv_timings > > * viif_s_dv_timings > > * viif_query_dv_timings > > * viif_g_edid > > * viif_s_edid > > * viif_g_parm > > * viif_s_parm > > * viif_enum_framesizes > > This one should stay, it should report the minimum and maximum sizes > supported by the video nodes, regardless of the configuration of the connected > subdev. I'll keep it. > > * viif_enum_frameintervals > > > > I can call subdevices directly if I need. Is it a correct understanding? > > what do you mean exactly by calling subdevices directly ? I meant userland can configure subdevices with /dev/v4l-subdev. > > As for viif_try_fmt_vid_cap and viif_s_fmt_vid_cap, I'll remove > > pad.g_fmt() call which is for checking pixel format. > > The check will be moved to viif_capture_link_validate() validation > > routine triggered by a start streaming event. > > > > > [snip] > > -- > Regards, > > Laurent Pinchart Regards, Yuji Ishikawa
WARNING: multiple messages have this Message-ID (diff)
From: <yuji2.ishikawa@toshiba.co.jp> To: <laurent.pinchart@ideasonboard.com> Cc: <hverkuil@xs4all.nl>, <mchehab@kernel.org>, <nobuhiro1.iwamatsu@toshiba.co.jp>, <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>, <rafael.j.wysocki@intel.com>, <broonie@kernel.org>, <linux-media@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org> Subject: RE: [PATCH v5 3/6] media: platform: visconti: Add Toshiba Visconti Video Input Interface driver user interace Date: Thu, 2 Feb 2023 04:52:56 +0000 [thread overview] Message-ID: <TYAPR01MB6201A88B85E74C3EA3481BD092D69@TYAPR01MB6201.jpnprd01.prod.outlook.com> (raw) In-Reply-To: <Y9LnWfZb+v17+h+X@pendragon.ideasonboard.com> Hello Laurent, > -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Friday, January 27, 2023 5:49 AM > To: ishikawa yuji(石川 悠司 ○RDC□AITC○EA開) > <yuji2.ishikawa@toshiba.co.jp> > Cc: hverkuil@xs4all.nl; mchehab@kernel.org; iwamatsu nobuhiro(岩松 信洋 □ > SWC◯ACT) <nobuhiro1.iwamatsu@toshiba.co.jp>; robh+dt@kernel.org; > krzysztof.kozlowski+dt@linaro.org; rafael.j.wysocki@intel.com; > broonie@kernel.org; linux-media@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org > Subject: Re: [PATCH v5 3/6] media: platform: visconti: Add Toshiba Visconti > Video Input Interface driver user interace > > Hello Ishikawa-san, > > On Wed, Jan 25, 2023 at 10:20:27AM +0000, yuji2.ishikawa@toshiba.co.jp wrote: > > On Wednesday, January 18, 2023 10:04 AM, Laurent Pinchart wrote: > > > On Tue, Jan 17, 2023 at 12:47:10PM +0100, Hans Verkuil wrote: > > > > On 11/01/2023 03:24, Yuji Ishikawa wrote: > > > > > Add support to Video Input Interface on Toshiba Visconti ARM SoCs. > > > > > The interface device includes CSI2 Receiver, frame grabber, > > > > > video DMAC and image signal processor. > > > > > This patch provides the user interface layer. > > > > > > > > > > A driver instance provides three /dev/videoX device files; one > > > > > for RGB image capture, another one for optional RGB capture with > > > > > different parameters and the last one for RAW capture. > > > > > > > > > > Through the device files, the driver provides streaming (DMA-BUF) > interface. > > > > > A userland application should feed DMA-BUF instances for capture > buffers. > > > > > > > > > > The driver is based on media controller framework. > > > > > Its operations are roughly mapped to two subdrivers; one for ISP > > > > > and > > > > > CSI2 receiver (yields 1 instance), the other for capture (yields > > > > > 3 instances for each capture mode). > > > > > > > > > > Signed-off-by: Yuji Ishikawa <yuji2.ishikawa@toshiba.co.jp> > > > > > --- > > > > > Changelog v2: > > > > > - Resend v1 because a patch exceeds size limit. > > > > > > > > > > Changelog v3: > > > > > - Adapted to media control framework > > > > > - Introduced ISP subdevice, capture device > > > > > - Remove private IOCTLs and add vendor specific V4L2 controls > > > > > - Change function name avoiding camelcase and uppercase letters > > > > > > > > > > Changelog v4: > > > > > - Split patches because the v3 patch exceeds size limit > > > > > - Stop using ID number to identify driver instance: > > > > > - Use dynamically allocated structure to hold HW specific context, > > > > > instead of static one. > > > > > - Call HW layer functions with the context structure instead > > > > > of ID number > > > > > - Use pm_runtime to trigger initialization of HW > > > > > along with open/close of device files. > > > > > > > > > > Changelog v5: > > > > > - Fix coding style problems in viif.c > > > > > --- > > > > > drivers/media/platform/visconti/Makefile | 1 + > > > > > drivers/media/platform/visconti/viif.c | 545 ++++++++ > > > > > drivers/media/platform/visconti/viif.h | 203 +++ > > > > > .../media/platform/visconti/viif_capture.c | 1201 > +++++++++++++++++ > > > > > drivers/media/platform/visconti/viif_isp.c | 846 > ++++++++++++ > > > > > 5 files changed, 2796 insertions(+) create mode 100644 > > > > > drivers/media/platform/visconti/viif.c > > > > > create mode 100644 drivers/media/platform/visconti/viif.h > > > > > create mode 100644 > > > > > drivers/media/platform/visconti/viif_capture.c > > > > > create mode 100644 drivers/media/platform/visconti/viif_isp.c > > > > > > [snip] > > > > > > > > +static int viif_s_edid(struct file *file, void *fh, struct > > > > > +v4l2_edid *edid) { > > > > > + struct viif_device *viif_dev = > video_drvdata_to_capdev(file)->viif_dev; > > > > > + struct viif_subdev *viif_sd = viif_dev->sd; > > > > > + > > > > > + return v4l2_subdev_call(viif_sd->v4l2_sd, pad, set_edid, > > > > > +edid); } > > > > > > > > Has this driver been tested with an HDMI receiver? If not, then I > > > > would recommend dropping support for it until you actually can > > > > test with such hardware. > > > > > > > > The DV_TIMINGS API is for HDMI/DVI/DisplayPort etc. interfaces, > > > > it's not meant for CSI and similar interfaces. > > > > > > More than that, for MC-based drivers, the video node should *never* > > > forward ioctls to a connected subdev. The *only* valid calls to > > > v4l2_subdev_call() in this file are > > > > > > - to video.s_stream() in the start and stop streaming handler > > > > > > - to pad.g_fmt() when starting streaming to validate that the connected > > > subdev outputs a format compatible with the format set on the video > > > capture device > > > > > > That's it, nothing else, all other calls to v4l2_subdev_call() must > > > be dropped from the implementation of the video_device. > > > > Thank you for your comment. I understand the restriction. > > I'll remove following functions corresponding to ioctls. > > > > * viif_enum_input > > * viif_g_selection > > * viif_s_selection > > * viif_dv_timings_cap > > * viif_enum_dv_timings > > * viif_g_dv_timings > > * viif_s_dv_timings > > * viif_query_dv_timings > > * viif_g_edid > > * viif_s_edid > > * viif_g_parm > > * viif_s_parm > > * viif_enum_framesizes > > This one should stay, it should report the minimum and maximum sizes > supported by the video nodes, regardless of the configuration of the connected > subdev. I'll keep it. > > * viif_enum_frameintervals > > > > I can call subdevices directly if I need. Is it a correct understanding? > > what do you mean exactly by calling subdevices directly ? I meant userland can configure subdevices with /dev/v4l-subdev. > > As for viif_try_fmt_vid_cap and viif_s_fmt_vid_cap, I'll remove > > pad.g_fmt() call which is for checking pixel format. > > The check will be moved to viif_capture_link_validate() validation > > routine triggered by a start streaming event. > > > > > [snip] > > -- > Regards, > > Laurent Pinchart Regards, Yuji Ishikawa _______________________________________________ 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-02-02 4:58 UTC|newest] Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-01-11 2:24 [PATCH v5 0/6] Add Toshiba Visconti Video Input Interface driver Yuji Ishikawa 2023-01-11 2:24 ` Yuji Ishikawa 2023-01-11 2:24 ` [PATCH v5 1/6] dt-bindings: media: platform: visconti: Add Toshiba Visconti Video Input Interface bindings Yuji Ishikawa 2023-01-11 2:24 ` Yuji Ishikawa 2023-01-11 9:19 ` Krzysztof Kozlowski 2023-01-11 9:19 ` Krzysztof Kozlowski 2023-01-11 12:48 ` yuji2.ishikawa 2023-01-11 12:48 ` yuji2.ishikawa 2023-01-17 15:26 ` Laurent Pinchart 2023-01-17 15:26 ` Laurent Pinchart 2023-01-17 15:42 ` Krzysztof Kozlowski 2023-01-17 15:42 ` Krzysztof Kozlowski 2023-01-17 15:58 ` Laurent Pinchart 2023-01-17 15:58 ` Laurent Pinchart 2023-01-17 17:01 ` Krzysztof Kozlowski 2023-01-17 17:01 ` Krzysztof Kozlowski 2023-01-22 19:25 ` Laurent Pinchart 2023-01-22 19:25 ` Laurent Pinchart 2023-01-30 9:06 ` yuji2.ishikawa 2023-01-30 9:06 ` yuji2.ishikawa 2023-02-01 9:45 ` Laurent Pinchart 2023-02-01 9:45 ` Laurent Pinchart 2023-02-01 11:24 ` yuji2.ishikawa 2023-02-01 11:24 ` yuji2.ishikawa 2023-01-11 2:24 ` [PATCH v5 2/6] media: platform: visconti: Add Toshiba Visconti Video Input Interface driver Yuji Ishikawa 2023-01-11 15:30 ` kernel test robot 2023-01-11 22:55 ` kernel test robot 2023-01-17 10:01 ` Hans Verkuil 2023-01-17 10:01 ` Hans Verkuil 2023-01-25 12:12 ` yuji2.ishikawa 2023-01-17 22:39 ` Sakari Ailus 2023-02-01 2:02 ` yuji2.ishikawa 2023-02-01 9:41 ` Laurent Pinchart 2023-02-01 9:41 ` Laurent Pinchart 2023-02-01 11:22 ` yuji2.ishikawa 2023-02-01 11:22 ` yuji2.ishikawa 2023-01-18 0:52 ` Laurent Pinchart 2023-01-18 0:52 ` Laurent Pinchart 2023-02-02 4:37 ` yuji2.ishikawa 2023-01-11 2:24 ` [PATCH v5 3/6] media: platform: visconti: Add Toshiba Visconti Video Input Interface driver user interace Yuji Ishikawa 2023-01-11 2:24 ` Yuji Ishikawa 2023-01-17 11:47 ` Hans Verkuil 2023-01-17 11:47 ` Hans Verkuil 2023-01-18 1:04 ` Laurent Pinchart 2023-01-18 1:04 ` Laurent Pinchart 2023-01-25 10:20 ` yuji2.ishikawa 2023-01-25 10:20 ` yuji2.ishikawa 2023-01-26 20:49 ` Laurent Pinchart 2023-01-26 20:49 ` Laurent Pinchart 2023-02-02 4:52 ` yuji2.ishikawa [this message] 2023-02-02 4:52 ` yuji2.ishikawa 2023-02-02 7:58 ` Laurent Pinchart 2023-02-02 7:58 ` Laurent Pinchart 2023-01-26 1:25 ` yuji2.ishikawa 2023-01-26 8:01 ` Hans Verkuil 2023-01-26 8:01 ` Hans Verkuil 2023-01-27 12:47 ` yuji2.ishikawa 2023-01-27 12:47 ` yuji2.ishikawa 2023-01-11 2:24 ` [PATCH v5 4/6] media: platform: visconti: Add Toshiba Visconti Video Input Interface driver v4l2 controls handler Yuji Ishikawa 2023-01-17 11:19 ` Hans Verkuil 2023-01-26 0:38 ` yuji2.ishikawa 2023-01-26 8:39 ` Hans Verkuil 2023-01-26 8:39 ` Hans Verkuil 2023-01-26 11:35 ` Laurent Pinchart 2023-01-26 11:35 ` Laurent Pinchart 2023-02-03 1:35 ` yuji2.ishikawa 2023-02-03 1:35 ` yuji2.ishikawa 2023-02-02 12:42 ` yuji2.ishikawa 2023-02-02 12:42 ` yuji2.ishikawa 2023-01-11 2:24 ` [PATCH v5 5/6] documentation: media: add documentation for Toshiba Visconti Video Input Interface driver Yuji Ishikawa 2023-01-11 2:24 ` Yuji Ishikawa 2023-01-11 2:24 ` [PATCH v5 6/6] MAINTAINERS: Add entries for Toshiba Visconti Video Input Interface Yuji Ishikawa 2023-01-11 2:24 ` Yuji Ishikawa
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=TYAPR01MB6201A88B85E74C3EA3481BD092D69@TYAPR01MB6201.jpnprd01.prod.outlook.com \ --to=yuji2.ishikawa@toshiba.co.jp \ --cc=broonie@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=hverkuil@xs4all.nl \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=mchehab@kernel.org \ --cc=nobuhiro1.iwamatsu@toshiba.co.jp \ --cc=rafael.j.wysocki@intel.com \ --cc=robh+dt@kernel.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.