From: Sakari Ailus <sakari.ailus@linux.intel.com> To: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> Cc: Helen Koike <helen.koike@collabora.com>, linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org, eddie.cai.linux@gmail.com, mchehab@kernel.org, heiko@sntech.de, jacob2.chen@rock-chips.com, jeffy.chen@rock-chips.com, zyc@rock-chips.com, linux-kernel@vger.kernel.org, tfiga@chromium.org, hans.verkuil@cisco.com, laurent.pinchart@ideasonboard.com, kernel@collabora.com, ezequiel@collabora.com, linux-media@vger.kernel.org, linux-arm-kernel@lists.infradead.org, zhengsq@rock-chips.com, Jacob Chen <cc@rock-chips.com>, Allon Huang <allon.huang@rock-chips.com>, Dafna Hirschfeld <dafna3@gmail.com> Subject: Re: [PATCH v8 05/14] media: rkisp1: add Rockchip ISP1 subdev driver Date: Wed, 12 Feb 2020 23:13:17 +0200 [thread overview] Message-ID: <20200212211317.GD3087@kekkonen.localdomain> (raw) In-Reply-To: <78856358-1afd-31a7-86dd-22f7d6d7fb05@collabora.com> Hi Dafna, Apologies for the late reply. I learned the mail had got lost due to mail server issues. On Fri, Jan 31, 2020 at 08:38:34PM +0100, Dafna Hirschfeld wrote: > Hi, > I (Dafna Hirschfeld) will work in following months with Helen Koike to fix the issues > in the TODO file of this driver: drivers/staging/media/rkisp1/TODO > > On 15.08.19 15:17, Sakari Ailus wrote: > > On Thu, Aug 15, 2019 at 11:24:22AM +0300, Sakari Ailus wrote: > > > Hi Helen, > > > > > > On Wed, Aug 14, 2019 at 09:58:05PM -0300, Helen Koike wrote: > > > > > > ... > > > > > > > > > +static int rkisp1_isp_sd_set_fmt(struct v4l2_subdev *sd, > > > > > > + struct v4l2_subdev_pad_config *cfg, > > > > > > + struct v4l2_subdev_format *fmt) > > > > > > +{ > > > > > > + struct rkisp1_device *isp_dev = sd_to_isp_dev(sd); > > > > > > + struct rkisp1_isp_subdev *isp_sd = &isp_dev->isp_sdev; > > > > > > + struct v4l2_mbus_framefmt *mf = &fmt->format; > > > > > > + > > > > > > > > > > Note that for sub-device nodes, the driver is itself responsible for > > > > > serialising the access to its data structures. > > > > > > > > But looking at subdev_do_ioctl_lock(), it seems that it serializes the > > > > ioctl calls for subdevs, no? Or I'm misunderstanding something (which is > > > > most probably) ? > > > > > > Good question. I had missed this change --- subdev_do_ioctl_lock() is > > > relatively new. But setting that lock is still not possible as the struct > > 'the struct' - do you mean the 'vdev' struct allocated in > 'v4l2_device_register_subdev_nodes' ? Yes. > > > > is allocated in the framework and the device is registered before the > > > > driver gets hold of it. It's a good idea to provide the same serialisation > > > for subdevs as well. > > > > > > I'll get back to this later. > > > > The main reason is actually that these ops are also called through the > > sub-device kAPI, not only through the uAPI, and the locks are only taken > > through the calls via uAPI. > > actually it seems that although 'subdev_do_ioctl_lock' exit, I wonder if > any subdevice uses that vdev->lock in subdev_do_ioctl_lock. > It is not initialized in v4l2_device_register_subdev_nodes where the vdev is allocated > and I wonder if any subdevice actually initialize it somewhere else. For example it is null in this > driver and in vimc. It needs to be set before the video device is registered, so indeed, it seems no driver can make use it. > > > > > So adding the locks to uAPI calls alone would not address the issue. > > What I can do is add a mutex to every struct of a subdevice and lock it > at the beginning of each subdevice operation. > Is this an acceptable solution? Please do. That's what other drivers do at the moment as well. -- Kind regards, Sakari Ailus
WARNING: multiple messages have this Message-ID (diff)
From: Sakari Ailus <sakari.ailus@linux.intel.com> To: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> Cc: devicetree@vger.kernel.org, eddie.cai.linux@gmail.com, kernel@collabora.com, heiko@sntech.de, jacob2.chen@rock-chips.com, Dafna Hirschfeld <dafna3@gmail.com>, jeffy.chen@rock-chips.com, zyc@rock-chips.com, linux-kernel@vger.kernel.org, tfiga@chromium.org, Allon Huang <allon.huang@rock-chips.com>, linux-rockchip@lists.infradead.org, Helen Koike <helen.koike@collabora.com>, Jacob Chen <cc@rock-chips.com>, hans.verkuil@cisco.com, laurent.pinchart@ideasonboard.com, zhengsq@rock-chips.com, mchehab@kernel.org, ezequiel@collabora.com, linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Subject: Re: [PATCH v8 05/14] media: rkisp1: add Rockchip ISP1 subdev driver Date: Wed, 12 Feb 2020 23:13:17 +0200 [thread overview] Message-ID: <20200212211317.GD3087@kekkonen.localdomain> (raw) In-Reply-To: <78856358-1afd-31a7-86dd-22f7d6d7fb05@collabora.com> Hi Dafna, Apologies for the late reply. I learned the mail had got lost due to mail server issues. On Fri, Jan 31, 2020 at 08:38:34PM +0100, Dafna Hirschfeld wrote: > Hi, > I (Dafna Hirschfeld) will work in following months with Helen Koike to fix the issues > in the TODO file of this driver: drivers/staging/media/rkisp1/TODO > > On 15.08.19 15:17, Sakari Ailus wrote: > > On Thu, Aug 15, 2019 at 11:24:22AM +0300, Sakari Ailus wrote: > > > Hi Helen, > > > > > > On Wed, Aug 14, 2019 at 09:58:05PM -0300, Helen Koike wrote: > > > > > > ... > > > > > > > > > +static int rkisp1_isp_sd_set_fmt(struct v4l2_subdev *sd, > > > > > > + struct v4l2_subdev_pad_config *cfg, > > > > > > + struct v4l2_subdev_format *fmt) > > > > > > +{ > > > > > > + struct rkisp1_device *isp_dev = sd_to_isp_dev(sd); > > > > > > + struct rkisp1_isp_subdev *isp_sd = &isp_dev->isp_sdev; > > > > > > + struct v4l2_mbus_framefmt *mf = &fmt->format; > > > > > > + > > > > > > > > > > Note that for sub-device nodes, the driver is itself responsible for > > > > > serialising the access to its data structures. > > > > > > > > But looking at subdev_do_ioctl_lock(), it seems that it serializes the > > > > ioctl calls for subdevs, no? Or I'm misunderstanding something (which is > > > > most probably) ? > > > > > > Good question. I had missed this change --- subdev_do_ioctl_lock() is > > > relatively new. But setting that lock is still not possible as the struct > > 'the struct' - do you mean the 'vdev' struct allocated in > 'v4l2_device_register_subdev_nodes' ? Yes. > > > > is allocated in the framework and the device is registered before the > > > > driver gets hold of it. It's a good idea to provide the same serialisation > > > for subdevs as well. > > > > > > I'll get back to this later. > > > > The main reason is actually that these ops are also called through the > > sub-device kAPI, not only through the uAPI, and the locks are only taken > > through the calls via uAPI. > > actually it seems that although 'subdev_do_ioctl_lock' exit, I wonder if > any subdevice uses that vdev->lock in subdev_do_ioctl_lock. > It is not initialized in v4l2_device_register_subdev_nodes where the vdev is allocated > and I wonder if any subdevice actually initialize it somewhere else. For example it is null in this > driver and in vimc. It needs to be set before the video device is registered, so indeed, it seems no driver can make use it. > > > > > So adding the locks to uAPI calls alone would not address the issue. > > What I can do is add a mutex to every struct of a subdevice and lock it > at the beginning of each subdevice operation. > Is this an acceptable solution? Please do. That's what other drivers do at the moment as well. -- Kind regards, Sakari Ailus _______________________________________________ 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:[~2020-02-12 21:13 UTC|newest] Thread overview: 173+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-30 18:42 [PATCH v8 00/14] Rockchip ISP1 Driver Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-07-30 18:42 ` [PATCH v8 01/14] media: videodev2.h, v4l2-ioctl: add rkisp1 meta buffer format Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-08-15 13:30 ` Laurent Pinchart 2019-08-15 13:30 ` Laurent Pinchart 2019-07-30 18:42 ` [PATCH v8 02/14] media: doc: add document for " Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-08-07 13:09 ` Sakari Ailus 2019-08-07 13:09 ` Sakari Ailus 2019-08-15 13:51 ` Laurent Pinchart 2019-08-15 13:51 ` Laurent Pinchart 2019-07-30 18:42 ` [PATCH v8 03/14] media: rkisp1: Add user space ABI definitions Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-08-15 18:46 ` Laurent Pinchart 2019-08-15 18:46 ` Laurent Pinchart 2020-07-10 12:59 ` Dafna Hirschfeld 2020-07-10 12:59 ` Dafna Hirschfeld 2020-07-10 12:59 ` Dafna Hirschfeld 2020-07-10 13:36 ` Laurent Pinchart 2020-07-10 13:36 ` Laurent Pinchart 2020-07-10 13:36 ` Laurent Pinchart 2020-07-10 14:30 ` Dafna Hirschfeld 2020-07-10 14:30 ` Dafna Hirschfeld 2019-07-30 18:42 ` [PATCH v8 04/14] media: rkisp1: add Rockchip MIPI Synopsys DPHY driver Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-08-07 13:05 ` Sakari Ailus 2019-08-07 13:05 ` Sakari Ailus 2019-08-07 13:37 ` Helen Koike 2019-08-07 13:37 ` Helen Koike 2019-08-15 17:54 ` Laurent Pinchart 2019-08-15 17:54 ` Laurent Pinchart 2019-08-15 18:26 ` Heiko Stübner 2019-08-15 18:26 ` Heiko Stübner 2019-08-21 21:46 ` Helen Koike 2019-08-21 21:46 ` Helen Koike 2019-08-22 2:32 ` Laurent Pinchart 2019-08-22 2:32 ` Laurent Pinchart 2019-08-22 2:32 ` Laurent Pinchart 2019-07-30 18:42 ` [PATCH v8 05/14] media: rkisp1: add Rockchip ISP1 subdev driver Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-08-06 18:51 ` Helen Koike 2019-08-06 18:51 ` Helen Koike 2019-08-07 10:39 ` Hans Verkuil 2019-08-07 10:39 ` Hans Verkuil 2019-08-15 19:35 ` Laurent Pinchart 2019-08-15 19:35 ` Laurent Pinchart 2020-03-25 6:34 ` Dafna Hirschfeld 2020-03-25 6:34 ` Dafna Hirschfeld 2020-03-25 7:11 ` Laurent Pinchart 2020-03-25 7:11 ` Laurent Pinchart 2020-03-25 8:51 ` Dafna Hirschfeld 2020-03-25 8:51 ` Dafna Hirschfeld 2020-03-25 9:11 ` Laurent Pinchart 2020-03-25 9:11 ` Laurent Pinchart 2019-08-08 9:14 ` Sakari Ailus 2019-08-08 9:14 ` Sakari Ailus 2019-08-15 0:58 ` Helen Koike 2019-08-15 0:58 ` Helen Koike 2019-08-15 0:58 ` Helen Koike 2019-08-15 8:24 ` Sakari Ailus 2019-08-15 8:24 ` Sakari Ailus 2019-08-15 10:29 ` Tomasz Figa 2019-08-15 10:29 ` Tomasz Figa 2019-08-15 10:29 ` Tomasz Figa 2019-08-15 10:45 ` Sakari Ailus 2019-08-15 10:45 ` Sakari Ailus 2019-08-15 10:45 ` Sakari Ailus 2019-08-15 13:17 ` Sakari Ailus 2019-08-15 13:17 ` Sakari Ailus 2020-01-31 19:38 ` Dafna Hirschfeld 2020-01-31 19:38 ` Dafna Hirschfeld 2020-02-12 21:13 ` Sakari Ailus [this message] 2020-02-12 21:13 ` Sakari Ailus 2020-02-13 12:50 ` Dafna Hirschfeld 2020-02-13 12:50 ` Dafna Hirschfeld 2019-08-16 0:13 ` Laurent Pinchart 2019-08-16 0:13 ` Laurent Pinchart 2020-07-11 11:04 ` Dafna Hirschfeld 2020-07-11 11:04 ` Dafna Hirschfeld 2020-07-17 7:46 ` Dafna Hirschfeld 2020-07-17 7:46 ` Dafna Hirschfeld 2020-07-22 16:01 ` Tomasz Figa 2020-07-22 16:01 ` Tomasz Figa 2020-07-22 15:24 ` Tomasz Figa 2020-07-22 15:24 ` Tomasz Figa 2020-07-22 15:24 ` Tomasz Figa 2020-07-22 16:30 ` Laurent Pinchart 2020-07-22 16:30 ` Laurent Pinchart 2020-07-22 16:30 ` Laurent Pinchart 2020-07-22 17:12 ` Tomasz Figa 2020-07-22 17:12 ` Tomasz Figa 2020-07-22 17:12 ` Tomasz Figa 2020-07-22 17:50 ` Laurent Pinchart 2020-07-22 17:50 ` Laurent Pinchart 2020-07-22 17:50 ` Laurent Pinchart 2020-08-05 21:10 ` Dafna Hirschfeld 2020-08-05 21:10 ` Dafna Hirschfeld 2020-08-06 9:21 ` Dafna Hirschfeld 2020-08-06 9:21 ` Dafna Hirschfeld 2020-08-06 12:22 ` Tomasz Figa 2020-08-06 12:22 ` Tomasz Figa 2020-08-06 12:22 ` Tomasz Figa 2020-08-07 16:08 ` Dafna Hirschfeld 2020-08-07 16:08 ` Dafna Hirschfeld 2020-08-07 16:08 ` Dafna Hirschfeld 2020-08-07 16:08 ` Dafna Hirschfeld 2020-08-13 6:17 ` Dafna Hirschfeld 2020-08-13 6:17 ` Dafna Hirschfeld 2020-08-13 6:17 ` Dafna Hirschfeld 2020-08-13 6:17 ` Dafna Hirschfeld 2020-08-06 12:08 ` Tomasz Figa 2020-08-06 12:08 ` Tomasz Figa 2020-08-06 12:08 ` Tomasz Figa 2020-08-07 16:02 ` Dafna Hirschfeld 2020-08-07 16:02 ` Dafna Hirschfeld 2020-08-07 16:02 ` Dafna Hirschfeld 2020-08-07 16:02 ` Dafna Hirschfeld 2019-07-30 18:42 ` [PATCH v8 06/14] media: rkisp1: add ISP1 statistics driver Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-08-08 9:37 ` Sakari Ailus 2019-08-08 9:37 ` Sakari Ailus 2019-07-30 18:42 ` [PATCH v8 07/14] media: rkisp1: add ISP1 params driver Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-07-30 18:42 ` [PATCH v8 08/14] media: rkisp1: add capture device driver Helen Koike 2019-07-30 18:42 ` [PATCH v8 09/14] media: rkisp1: add rockchip isp1 core driver Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-08-07 15:27 ` Sakari Ailus 2019-08-07 15:27 ` Sakari Ailus 2019-08-08 21:59 ` Helen Koike 2019-08-08 21:59 ` Helen Koike 2019-08-09 12:05 ` Sakari Ailus 2019-08-09 12:05 ` Sakari Ailus 2019-08-07 15:36 ` Sakari Ailus 2019-08-07 15:36 ` Sakari Ailus 2019-07-30 18:42 ` [PATCH v8 10/14] dt-bindings: Document the Rockchip ISP1 bindings Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-08-16 0:21 ` Laurent Pinchart 2019-08-16 0:21 ` Laurent Pinchart 2019-08-16 0:21 ` Laurent Pinchart 2019-07-30 18:42 ` [PATCH v8 11/14] dt-bindings: Document the Rockchip MIPI RX D-PHY bindings Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-08-15 18:14 ` Laurent Pinchart 2019-08-15 18:14 ` Laurent Pinchart 2019-07-30 18:42 ` [PATCH v8 12/14] arm64: dts: rockchip: add isp0 node for rk3399 Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-07-30 18:42 ` [PATCH v8 13/14] arm64: dts: rockchip: add rx0 mipi-phy " Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-07-30 18:42 ` [PATCH v8 14/14] MAINTAINERS: add entry for Rockchip ISP1 driver Helen Koike 2019-07-30 18:42 ` Helen Koike 2019-08-15 13:56 ` Laurent Pinchart 2019-08-15 13:56 ` Laurent Pinchart 2019-07-30 20:15 ` [PATCH v8 00/14] Rockchip ISP1 Driver Hans Verkuil 2019-07-30 20:15 ` Hans Verkuil 2019-07-30 20:50 ` Helen Koike 2019-07-30 20:50 ` Helen Koike 2019-07-31 0:08 ` Helen Koike 2019-07-31 0:08 ` Helen Koike 2019-07-31 4:29 ` Hans Verkuil 2019-07-31 4:29 ` Hans Verkuil 2019-07-31 4:33 ` Hans Verkuil 2019-07-31 4:33 ` Hans Verkuil 2019-07-31 4:55 ` Hans Verkuil 2019-07-31 4:55 ` Hans Verkuil 2019-07-31 4:55 ` Hans Verkuil 2019-07-31 14:42 ` Helen Koike 2019-07-31 14:42 ` Helen Koike 2019-08-07 15:37 ` Sakari Ailus 2019-08-07 15:37 ` Sakari Ailus 2019-08-07 17:57 ` Helen Koike 2019-08-07 17:57 ` Helen Koike 2019-08-09 18:45 ` Manivannan Sadhasivam 2019-08-09 18:45 ` Manivannan Sadhasivam
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=20200212211317.GD3087@kekkonen.localdomain \ --to=sakari.ailus@linux.intel.com \ --cc=allon.huang@rock-chips.com \ --cc=cc@rock-chips.com \ --cc=dafna.hirschfeld@collabora.com \ --cc=dafna3@gmail.com \ --cc=devicetree@vger.kernel.org \ --cc=eddie.cai.linux@gmail.com \ --cc=ezequiel@collabora.com \ --cc=hans.verkuil@cisco.com \ --cc=heiko@sntech.de \ --cc=helen.koike@collabora.com \ --cc=jacob2.chen@rock-chips.com \ --cc=jeffy.chen@rock-chips.com \ --cc=kernel@collabora.com \ --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=linux-rockchip@lists.infradead.org \ --cc=mchehab@kernel.org \ --cc=tfiga@chromium.org \ --cc=zhengsq@rock-chips.com \ --cc=zyc@rock-chips.com \ /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.