Hi Andrey On Wed, Sep 13, 2023 at 11:48:13PM +0300, Andrey Skvortsov wrote: > Hi Sakari, > > On 23-09-13 15:27, Sakari Ailus wrote: > > Hi Andrey, > > > > 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 ? Laurent's patch for imx219 (and his forthcoming patch to remove the same pattern from all sensor drivers in mainline) might be opening th door for unexpected bugs as long as we don't enforce a resume/suspend ordering ? > > 1. https://salsa.debian.org/Mobian-team/devices/kernels/sunxi64-linux/-/blob/mobian-6.1/debian/patches/camera/0076-media-gc2145-implement-system-suspend.patch > 2. https://elixir.bootlin.com/linux/latest/source/drivers/media/i2c/imx219.c#L1159 Can we get a link to the receiver driver you're using in your kernel ? Thanks j > > -- > Best regards, > Andrey Skvortsov
