Re: [PATCH v2 37/66] media: sun6i-csi: Move power management to runtime pm in capture

From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org,
	linux-phy@lists.infradead.org, linux-clk@vger.kernel.org,
	linux-staging@lists.linux.dev, Yong Deng <yong.deng@magewell.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Maxime Ripard <mripard@kernel.org>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Chen-Yu Tsai <wens@csie.org>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Helen Koike <helen.koike@collabora.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH v2 37/66] media: sun6i-csi: Move power management to runtime pm in capture
Date: Tue, 15 Feb 2022 11:21:17 +0100	[thread overview]
Message-ID: <Ygt+nZJrZMNXV4Cl@aptenodytes> (raw)
In-Reply-To: <Ygt6vwydxg9/WuDH@pendragon.ideasonboard.com>

[-- Attachment #1: Type: text/plain, Size: 2028 bytes --]

Hi Laurent,

On Tue 15 Feb 22, 12:04, Laurent Pinchart wrote:
> Hi Paul,
> 
> On Tue, Feb 15, 2022 at 10:56:22AM +0100, Paul Kocialkowski wrote:
> > On Mon 14 Feb 22, 20:30, Sakari Ailus wrote:
> > > On Sat, Feb 05, 2022 at 07:54:00PM +0100, Paul Kocialkowski wrote:
> > > > Let's just enable the module when we start using it (at stream on)
> > > > and benefit from runtime pm instead of enabling it at first open.
> > > > 
> > > > Also reorder the call to v4l2_pipeline_pm_get.
> > > > 
> > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > 
> > > Nice patch!
> > 
> > Thanks!
> > 
> > > Do you still need v4l2_pipeline_pm_put()? Removing it would be a separate
> > > patch of course.
> > 
> > My understanding is that this is still useful if there are drivers in the
> > pipeline that rely on s_power instead of rpm (a typical case could be an
> > old sensor driver). So that's why this is kept around, but all other components
> > of the pipeline (isp/csi/mipi csi-2) are using rpm now.
> 
> If that's not the case on your test platforms, I think it would be
> better to drop support for this old API, and convert drivers that still
> use .s_power() if someone needs to use one on an Allwinner platform.

I agree this is the path to follow but it feels like we're not quite there
yet and a bunch of driver were not converted at this point, including some
popular ones like ov5640, which I know for sure is used with Allwinner devices.

Honestly I'd be happy to get rid of these legacy functions as soon as the
transition is done, but doing it now would mean breaking a significant number
of use cases (which I'm trying to avoid here despite all the changes).

I definitely wouldn't be confident making that transition here and it
probably wouldn't be a good idea to make that a requirement to merge this
(already quite big) series.

What do you think?

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]