Hi Laurent, On Thu, Sep 14, 2023 at 12:02:41PM +0300, Laurent Pinchart wrote: > On Thu, Sep 14, 2023 at 10:54:40AM +0200, Jacopo Mondi wrote: > > On Wed, Sep 13, 2023 at 11:48:13PM +0300, Andrey Skvortsov wrote: > > > On 23-09-13 15:27, Sakari Ailus wrote: > > > > On Fri, Aug 18, 2023 at 08:34:16PM +0300, Andrey Skvortsov wrote: > > > > > If system was suspended while camera sensor was used, data and > > > > > interrupts were still coming from sensor and that caused unstable > > > > > system. Sometimes system hanged during a resume. Use > > > > > pm_runtime_force_* helpers in order to support system suspend. > > > > > > > > > > Signed-off-by: Andrey Skvortsov <andrej.skvortzov@xxxxxxxxx> > > > > > > > > Thanks for the patch. > > > > > > > > It's not been documented really how system suspend and resume should > > > > work for complex cameras. But I don't think it can be done by drivers > > > > separately as the CSI-2 bus initialisation requires actions from both > > > > sender and receiver drivers, at particular points of time. > > > > > > Thanks for the review. > > > > > > I've tested this on PinePhone A64. It uses DVP, maybe because of that > > > system suspend/resume worked good in my case. > > > Originally I've implemented system suspend/resume similar to this [1] > > > or [2] as I've seen this approach in other mainlined drivers. But some > > > drivers reuse pm_runtime_force_* helpers, so I've went with this. > > > > > > Do you think it would be better to use something like [2] until there > > > is better well defined way for system suspend/resume for complex cameras? > > > > > > > please don't :) > > https://patchwork.linuxtv.org/project/linux-media/patch/20230913135638.26277-16-laurent.pinchart@xxxxxxxxxxxxxxxx/ > > > > However... > > > > > > So I think we'll need to initiate this from the driver handling DMA, just > > > > as starting and stopping streaming. Even then, there needs to be a > > > > certainty that the sensor device has resumed before streaming is started. I > > > > recall Laurent suggested device links for that purpose, but I don't think > > > > any work has been done to implement it that way. > > > > .. as Sakari suggested, the driver handling the DMA should be in > > charge of calling s_stream() on the sensor subdev in its > > suspend/resume handlers. There's the risk the receiver resumes while > > the sensor is still suspended, and at this time there's no solution in > > mainline to handle this correctly. > > > > Laurent/Sakari, how should this be handled for the time being ? > > device_link() should handle this. See mxc_isi_async_notifier_bound() in > drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c. > > It would be nice if this could be done in the V4L2 core. I haven't > checked if that's possible. I can't see why it wouldn't be, and it seems easy, too. We're still early in the cycle so if someone writes the patches, I can't see why we couldn't get them for 6.7. -- Regards, Sakari Ailus
