Hi Laurent, On Wed, Aug 23, 2023 at 12:14:46PM +0300, Laurent Pinchart wrote: > Hi Sakari, > > On Wed, Aug 23, 2023 at 09:01:56AM +0000, Sakari Ailus wrote: > > On Tue, Aug 22, 2023 at 06:58:47PM +0100, Dave Stevenson wrote: > > > On Mon, 21 Aug 2023 at 23:30, Laurent Pinchart wrote: > > > > > > > > There is no need to stop streaming at system suspend time, or restart it > > > > when the system is resumed, as the host-side driver is responsible for > > > > stopping and restarting the pipeline in a controlled way by calling the > > > > .s_stream() operation. Drop the system suspend and resume handlers, and > > > > simplify the .s_stream() handler as a result. > > > > > > I'll believe you, but the docs for power management in camera sensor > > > drivers [1] state > > > "Please see examples in e.g. drivers/media/i2c/ov8856.c and > > > drivers/media/i2c/ccs/ccs-core.c. The two drivers work in both ACPI > > > and DT based systems." > > > > > > Looking at CCS we find the suspend hook stopping streaming [2], and > > > resume hook starting it [3]. Same in ov8856 [4]. > > > > > > Could you reference the documentation that states that the host-side > > > driver is responsible for starting and stopping? Is this an ACPI vs DT > > > difference? > > > > There's no difference between DT and ACPI, no. > > The comment about drivers working with both ACPI- and DT-based systems > was related, as far as I understand, to usage of runtime PM, I don't > think it was related to system PM at all. > > > Starting streaming on resume from system suspend is haphazard as there's no > > guarantee on the timing of resuming devices (if the suspend and resume > > independently), or similarly if the receiver driver explicitly starts > > streaming, there's no guarantee the sub-device driver has resumed. > > > > So I think we'd need more than what currently exists on the framework side > > to do this reliably --- at least when it comes to CSI-2. > > Device links help here. See for instance mxc_isi_async_notifier_bound() > in drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c. Ideally this > shouldn't be handled by drivers but by the V4L2 core though. In any > case, the sensors don't need to deal with it, enforcing correct ordering > is an issue for the connected receiver. Nice idea. Perhaps this could be integrated somehow to the V4L2 async framework? And it depends on streaming control --- if you don't have streaming ongoing, then you don't need that dependency. Also errors in resuming should only be related to the devices that failed to resume. It's going to get complicated. > > Sakari, you didn't explicitly answer whether you think that system > suspend/resume handlers are needed or not for camera sensor drivers. > What is your opinion on this ? Streaming needs to be explicitly started and stopped by the downstream pipeline, so camera sensors shouldn't need suspend or resume handlers (at least for this reason). -- Regards, Sakari Ailus
