Re: [PATCH v2 7/7] [media] tvp5150: add s_power callback

[Mauro Carvalho Chehab <mchehab+samsung@kernel.org>]

Em Fri, 14 Sep 2018 20:20:46 +0200
Marco Felsch <m.felsch@pengutronix.de> escreveu:

> Hi Sakari,
> 
> On 18-09-14 16:23, Sakari Ailus wrote:
> > Hi Marco,
> > 
> > On Mon, Aug 13, 2018 at 11:25:08AM +0200, Marco Felsch wrote:  
> > > Don't en-/disable the interrupts during s_stream because someone can
> > > disable the stream but wants to get informed if the stream is locked
> > > again. So keep the interrupts enabled the whole time the pipeline is
> > > opened.
> > > 
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > ---
> > >  drivers/media/i2c/tvp5150.c | 23 +++++++++++++++++------
> > >  1 file changed, 17 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > > index e736f609fecd..e296f5bfae21 100644
> > > --- a/drivers/media/i2c/tvp5150.c
> > > +++ b/drivers/media/i2c/tvp5150.c
> > > @@ -1389,11 +1389,26 @@ static const struct media_entity_operations tvp5150_sd_media_ops = {
> > >  /****************************************************************************
> > >  			I2C Command
> > >   ****************************************************************************/
> > > +static int tvp5150_s_power(struct  v4l2_subdev *sd, int on)
> > > +{
> > > +	struct tvp5150 *decoder = to_tvp5150(sd);
> > > +	unsigned int val = 0;
> > > +
> > > +	if (on)
> > > +		val = TVP5150_INT_A_LOCK;
> > > +
> > > +	if (decoder->irq)
> > > +		/* Enable / Disable lock interrupt */
> > > +		regmap_update_bits(decoder->regmap, TVP5150_INT_ENABLE_REG_A,
> > > +				   TVP5150_INT_A_LOCK, val);  
> > 
> > Could you use runtime PM instead?  
> 
> I will test it next monday. What's the different between s_power and
> runtime PM?
> 
> > 
> > For an example, the dw9714 driver does this: drivers/media/i2c/dw9714.c .  
> 
> Hopefully I got you right, should I use the
> v4l2_subdev_internal_ops.open/close and call the pm_runtime_put/get
> there or did you mean the driver.pm callbacks? I'm not that familiar
> with the pm ops at the moment, sorry.

I guess the main issue here is: will this work if the bridge
driver is em28xx?

Whatever change we do, tvp5150 should still fully work with em28xx,
as several devices use this demod there.

Changing em28xx to cope with runtime PM would be *very* complex,
as there are lots of other drivers that can work with it, and
touching those will affect lots of other drivers. At the end, it
will very likely affect all PCI/PCIe V4L2 drivers, and several
USB ones.

If it can be done without affecting PM with em28xx, let's do it.
Otherwise, let's stick with s_power on this series, and let 
the mass PM rework on non-platform drivers to happen on some
separate patchset.

Thanks,
Mauro