Hi,
On 18-09-14 15:57, Mauro Carvalho Chehab wrote:
> Em Fri, 14 Sep 2018 20:20:46 +0200
> Marco Felsch <m.fel...@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.fel...@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 did some tests with the PM runtime ops. I used the suspend() callback
to disbale the IRQ and the resume() callback to enable it. But this
won't work with my IRQ-driven setup because if the signal is not locked
(maybe lost due to no input stream) the tvp went into suspend. In this
mode the IRQ is disabled and I have no chance to enable the chip again.
As Mauro pointed out below, there maybe some other issues I can't test
at the moment. I'm with Mauro to keep the .s_power() callback for this
series. There should be another patchset which covers the PM rework for
all devices using the TVP5150.
>
> 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
>
Kind regards,
Marco
