diff mbox series

media: v4l2-mc: Mark v4l2_pipeline_link_notify() as deprecated

Message ID 20240822214125.3161-1-laurent.pinchart+renesas@ideasonboard.com (mailing list archive)
State Accepted
Headers
Series media: v4l2-mc: Mark v4l2_pipeline_link_notify() as deprecated |

Commit Message

Laurent Pinchart Aug. 22, 2024, 9:41 p.m. UTC 
  Commit b97213a41140 ("media: v4l2-mc: Make v4l2_pipeline_pm_{get,put}
deprecated") marked the v4l2_pipeline_pm_get() and
v4l2_pipeline_pm_put() functions as deprecated, but forgot to address
the related v4l2_pipeline_link_notify() function similarly. Fix it.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 include/media/v4l2-mc.h | 3 +++
 1 file changed, 3 insertions(+)


base-commit: a043ea54bbb975ca9239c69fd17f430488d33522

Comments

Sakari Ailus Aug. 23, 2024, 7:16 a.m. UTC | #1 
Hi Laurent,

Thanks for the patch.

On Fri, Aug 23, 2024 at 12:41:25AM +0300, Laurent Pinchart wrote:
> Commit b97213a41140 ("media: v4l2-mc: Make v4l2_pipeline_pm_{get,put}
> deprecated") marked the v4l2_pipeline_pm_get() and
> v4l2_pipeline_pm_put() functions as deprecated, but forgot to address
> the related v4l2_pipeline_link_notify() function similarly. Fix it.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

How about adding a warning for the use of these functions? Possibly on
debug level if pr_warn_once() is considered too drastic?

> ---
>  include/media/v4l2-mc.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
> index ed0a44b6eada..1837c9fd78cf 100644
> --- a/include/media/v4l2-mc.h
> +++ b/include/media/v4l2-mc.h
> @@ -178,6 +178,9 @@ void v4l2_pipeline_pm_put(struct media_entity *entity);
>   * @flags: New link flags that will be applied
>   * @notification: The link's state change notification type (MEDIA_DEV_NOTIFY_*)
>   *
> + * THIS FUNCTION IS DEPRECATED. DO NOT USE IN NEW DRIVERS. USE RUNTIME PM
> + * ON SUB-DEVICE DRIVERS INSTEAD.
> + *
>   * React to link management on powered pipelines by updating the use count of
>   * all entities in the source and sink sides of the link. Entities are powered
>   * on or off accordingly. The use of this function should be paired
> 
> base-commit: a043ea54bbb975ca9239c69fd17f430488d33522
Laurent Pinchart Aug. 23, 2024, 1:57 p.m. UTC | #2 
On Fri, Aug 23, 2024 at 07:16:42AM +0000, Sakari Ailus wrote:
> On Fri, Aug 23, 2024 at 12:41:25AM +0300, Laurent Pinchart wrote:
> > Commit b97213a41140 ("media: v4l2-mc: Make v4l2_pipeline_pm_{get,put}
> > deprecated") marked the v4l2_pipeline_pm_get() and
> > v4l2_pipeline_pm_put() functions as deprecated, but forgot to address
> > the related v4l2_pipeline_link_notify() function similarly. Fix it.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> How about adding a warning for the use of these functions? Possibly on
> debug level if pr_warn_once() is considered too drastic?

I think we need to do a bit of homework first, as there's a large number
of drivers using these, directly or indirectly. We should at least
convert the sensor drivers still using .s_power() to runtime PM, to make
it possible to convert the other drivers.

> > ---
> >  include/media/v4l2-mc.h | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
> > index ed0a44b6eada..1837c9fd78cf 100644
> > --- a/include/media/v4l2-mc.h
> > +++ b/include/media/v4l2-mc.h
> > @@ -178,6 +178,9 @@ void v4l2_pipeline_pm_put(struct media_entity *entity);
> >   * @flags: New link flags that will be applied
> >   * @notification: The link's state change notification type (MEDIA_DEV_NOTIFY_*)
> >   *
> > + * THIS FUNCTION IS DEPRECATED. DO NOT USE IN NEW DRIVERS. USE RUNTIME PM
> > + * ON SUB-DEVICE DRIVERS INSTEAD.
> > + *
> >   * React to link management on powered pipelines by updating the use count of
> >   * all entities in the source and sink sides of the link. Entities are powered
> >   * on or off accordingly. The use of this function should be paired
> > 
> > base-commit: a043ea54bbb975ca9239c69fd17f430488d33522
Sakari Ailus Aug. 24, 2024, 2:28 p.m. UTC | #3 
Hi Laurent,

On Fri, Aug 23, 2024 at 04:57:28PM +0300, Laurent Pinchart wrote:
> On Fri, Aug 23, 2024 at 07:16:42AM +0000, Sakari Ailus wrote:
> > On Fri, Aug 23, 2024 at 12:41:25AM +0300, Laurent Pinchart wrote:
> > > Commit b97213a41140 ("media: v4l2-mc: Make v4l2_pipeline_pm_{get,put}
> > > deprecated") marked the v4l2_pipeline_pm_get() and
> > > v4l2_pipeline_pm_put() functions as deprecated, but forgot to address
> > > the related v4l2_pipeline_link_notify() function similarly. Fix it.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > 
> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > 
> > How about adding a warning for the use of these functions? Possibly on
> > debug level if pr_warn_once() is considered too drastic?
> 
> I think we need to do a bit of homework first, as there's a large number
> of drivers using these, directly or indirectly. We should at least
> convert the sensor drivers still using .s_power() to runtime PM, to make
> it possible to convert the other drivers.

With that, we could just drop the implementation of the pipeline PM stuff
and replace it with a warning.

I think a pr_debug_once() would still be appropriate, at least. People
generally won't read the documentation unless something is broken.

> 
> > > ---
> > >  include/media/v4l2-mc.h | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
> > > index ed0a44b6eada..1837c9fd78cf 100644
> > > --- a/include/media/v4l2-mc.h
> > > +++ b/include/media/v4l2-mc.h
> > > @@ -178,6 +178,9 @@ void v4l2_pipeline_pm_put(struct media_entity *entity);
> > >   * @flags: New link flags that will be applied
> > >   * @notification: The link's state change notification type (MEDIA_DEV_NOTIFY_*)
> > >   *
> > > + * THIS FUNCTION IS DEPRECATED. DO NOT USE IN NEW DRIVERS. USE RUNTIME PM
> > > + * ON SUB-DEVICE DRIVERS INSTEAD.
> > > + *
> > >   * React to link management on powered pipelines by updating the use count of
> > >   * all entities in the source and sink sides of the link. Entities are powered
> > >   * on or off accordingly. The use of this function should be paired
> > > 
> > > base-commit: a043ea54bbb975ca9239c69fd17f430488d33522
Laurent Pinchart Aug. 24, 2024, 3:50 p.m. UTC | #4 
On Sat, Aug 24, 2024 at 02:28:22PM +0000, Sakari Ailus wrote:
> On Fri, Aug 23, 2024 at 04:57:28PM +0300, Laurent Pinchart wrote:
> > On Fri, Aug 23, 2024 at 07:16:42AM +0000, Sakari Ailus wrote:
> > > On Fri, Aug 23, 2024 at 12:41:25AM +0300, Laurent Pinchart wrote:
> > > > Commit b97213a41140 ("media: v4l2-mc: Make v4l2_pipeline_pm_{get,put}
> > > > deprecated") marked the v4l2_pipeline_pm_get() and
> > > > v4l2_pipeline_pm_put() functions as deprecated, but forgot to address
> > > > the related v4l2_pipeline_link_notify() function similarly. Fix it.
> > > > 
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > 
> > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > 
> > > How about adding a warning for the use of these functions? Possibly on
> > > debug level if pr_warn_once() is considered too drastic?
> > 
> > I think we need to do a bit of homework first, as there's a large number
> > of drivers using these, directly or indirectly. We should at least
> > convert the sensor drivers still using .s_power() to runtime PM, to make
> > it possible to convert the other drivers.
> 
> With that, we could just drop the implementation of the pipeline PM stuff
> and replace it with a warning.
> 
> I think a pr_debug_once() would still be appropriate, at least. People
> generally won't read the documentation unless something is broken.

Generally I agree, but my concern here is that someone hitting the
warning would need to first convert all the remaining sensor drivers to
runtime PM before they could safely address the issue on the host driver
side. That's quite a bit of yak shaving.

> > > > ---
> > > >  include/media/v4l2-mc.h | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
> > > > index ed0a44b6eada..1837c9fd78cf 100644
> > > > --- a/include/media/v4l2-mc.h
> > > > +++ b/include/media/v4l2-mc.h
> > > > @@ -178,6 +178,9 @@ void v4l2_pipeline_pm_put(struct media_entity *entity);
> > > >   * @flags: New link flags that will be applied
> > > >   * @notification: The link's state change notification type (MEDIA_DEV_NOTIFY_*)
> > > >   *
> > > > + * THIS FUNCTION IS DEPRECATED. DO NOT USE IN NEW DRIVERS. USE RUNTIME PM
> > > > + * ON SUB-DEVICE DRIVERS INSTEAD.
> > > > + *
> > > >   * React to link management on powered pipelines by updating the use count of
> > > >   * all entities in the source and sink sides of the link. Entities are powered
> > > >   * on or off accordingly. The use of this function should be paired
> > > > 
> > > > base-commit: a043ea54bbb975ca9239c69fd17f430488d33522
Sakari Ailus Aug. 24, 2024, 4:28 p.m. UTC | #5 
Hi Laurent,

On Sat, Aug 24, 2024 at 06:50:09PM +0300, Laurent Pinchart wrote:
> On Sat, Aug 24, 2024 at 02:28:22PM +0000, Sakari Ailus wrote:
> > On Fri, Aug 23, 2024 at 04:57:28PM +0300, Laurent Pinchart wrote:
> > > On Fri, Aug 23, 2024 at 07:16:42AM +0000, Sakari Ailus wrote:
> > > > On Fri, Aug 23, 2024 at 12:41:25AM +0300, Laurent Pinchart wrote:
> > > > > Commit b97213a41140 ("media: v4l2-mc: Make v4l2_pipeline_pm_{get,put}
> > > > > deprecated") marked the v4l2_pipeline_pm_get() and
> > > > > v4l2_pipeline_pm_put() functions as deprecated, but forgot to address
> > > > > the related v4l2_pipeline_link_notify() function similarly. Fix it.
> > > > > 
> > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > > 
> > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > 
> > > > How about adding a warning for the use of these functions? Possibly on
> > > > debug level if pr_warn_once() is considered too drastic?
> > > 
> > > I think we need to do a bit of homework first, as there's a large number
> > > of drivers using these, directly or indirectly. We should at least
> > > convert the sensor drivers still using .s_power() to runtime PM, to make
> > > it possible to convert the other drivers.
> > 
> > With that, we could just drop the implementation of the pipeline PM stuff
> > and replace it with a warning.
> > 
> > I think a pr_debug_once() would still be appropriate, at least. People
> > generally won't read the documentation unless something is broken.
> 
> Generally I agree, but my concern here is that someone hitting the
> warning would need to first convert all the remaining sensor drivers to
> runtime PM before they could safely address the issue on the host driver
> side. That's quite a bit of yak shaving.

That's a fair point.

How about then adding such a warning to sub-device drivers implementing
s_power? That'd be much easier to fix for anyone encountering it.

That being said, there could be drivers that need s_power, due to missing
other APIs, for TV tuners on PCI cards for instance.

Cc Hans.

> 
> > > > > ---
> > > > >  include/media/v4l2-mc.h | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > > 
> > > > > diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
> > > > > index ed0a44b6eada..1837c9fd78cf 100644
> > > > > --- a/include/media/v4l2-mc.h
> > > > > +++ b/include/media/v4l2-mc.h
> > > > > @@ -178,6 +178,9 @@ void v4l2_pipeline_pm_put(struct media_entity *entity);
> > > > >   * @flags: New link flags that will be applied
> > > > >   * @notification: The link's state change notification type (MEDIA_DEV_NOTIFY_*)
> > > > >   *
> > > > > + * THIS FUNCTION IS DEPRECATED. DO NOT USE IN NEW DRIVERS. USE RUNTIME PM
> > > > > + * ON SUB-DEVICE DRIVERS INSTEAD.
> > > > > + *
> > > > >   * React to link management on powered pipelines by updating the use count of
> > > > >   * all entities in the source and sink sides of the link. Entities are powered
> > > > >   * on or off accordingly. The use of this function should be paired
> > > > > 
> > > > > base-commit: a043ea54bbb975ca9239c69fd17f430488d33522
>
Laurent Pinchart Aug. 24, 2024, 11:24 p.m. UTC | #6 
On Sat, Aug 24, 2024 at 04:28:08PM +0000, Sakari Ailus wrote:
> On Sat, Aug 24, 2024 at 06:50:09PM +0300, Laurent Pinchart wrote:
> > On Sat, Aug 24, 2024 at 02:28:22PM +0000, Sakari Ailus wrote:
> > > On Fri, Aug 23, 2024 at 04:57:28PM +0300, Laurent Pinchart wrote:
> > > > On Fri, Aug 23, 2024 at 07:16:42AM +0000, Sakari Ailus wrote:
> > > > > On Fri, Aug 23, 2024 at 12:41:25AM +0300, Laurent Pinchart wrote:
> > > > > > Commit b97213a41140 ("media: v4l2-mc: Make v4l2_pipeline_pm_{get,put}
> > > > > > deprecated") marked the v4l2_pipeline_pm_get() and
> > > > > > v4l2_pipeline_pm_put() functions as deprecated, but forgot to address
> > > > > > the related v4l2_pipeline_link_notify() function similarly. Fix it.
> > > > > > 
> > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > > > 
> > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > > 
> > > > > How about adding a warning for the use of these functions? Possibly on
> > > > > debug level if pr_warn_once() is considered too drastic?
> > > > 
> > > > I think we need to do a bit of homework first, as there's a large number
> > > > of drivers using these, directly or indirectly. We should at least
> > > > convert the sensor drivers still using .s_power() to runtime PM, to make
> > > > it possible to convert the other drivers.
> > > 
> > > With that, we could just drop the implementation of the pipeline PM stuff
> > > and replace it with a warning.
> > > 
> > > I think a pr_debug_once() would still be appropriate, at least. People
> > > generally won't read the documentation unless something is broken.
> > 
> > Generally I agree, but my concern here is that someone hitting the
> > warning would need to first convert all the remaining sensor drivers to
> > runtime PM before they could safely address the issue on the host driver
> > side. That's quite a bit of yak shaving.
> 
> That's a fair point.
> 
> How about then adding such a warning to sub-device drivers implementing
> s_power? That'd be much easier to fix for anyone encountering it.
> 
> That being said, there could be drivers that need s_power, due to missing
> other APIs, for TV tuners on PCI cards for instance.

I was thinking about the same. We can only warn if there are
alternatives for all use cases.

> Cc Hans.
> 
> > > > > > ---
> > > > > >  include/media/v4l2-mc.h | 3 +++
> > > > > >  1 file changed, 3 insertions(+)
> > > > > > 
> > > > > > diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
> > > > > > index ed0a44b6eada..1837c9fd78cf 100644
> > > > > > --- a/include/media/v4l2-mc.h
> > > > > > +++ b/include/media/v4l2-mc.h
> > > > > > @@ -178,6 +178,9 @@ void v4l2_pipeline_pm_put(struct media_entity *entity);
> > > > > >   * @flags: New link flags that will be applied
> > > > > >   * @notification: The link's state change notification type (MEDIA_DEV_NOTIFY_*)
> > > > > >   *
> > > > > > + * THIS FUNCTION IS DEPRECATED. DO NOT USE IN NEW DRIVERS. USE RUNTIME PM
> > > > > > + * ON SUB-DEVICE DRIVERS INSTEAD.
> > > > > > + *
> > > > > >   * React to link management on powered pipelines by updating the use count of
> > > > > >   * all entities in the source and sink sides of the link. Entities are powered
> > > > > >   * on or off accordingly. The use of this function should be paired
> > > > > > 
> > > > > > base-commit: a043ea54bbb975ca9239c69fd17f430488d33522
diff mbox series

Patch

diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
index ed0a44b6eada..1837c9fd78cf 100644
--- a/include/media/v4l2-mc.h
+++ b/include/media/v4l2-mc.h
@@ -178,6 +178,9 @@  void v4l2_pipeline_pm_put(struct media_entity *entity);
  * @flags: New link flags that will be applied
  * @notification: The link's state change notification type (MEDIA_DEV_NOTIFY_*)
  *
+ * THIS FUNCTION IS DEPRECATED. DO NOT USE IN NEW DRIVERS. USE RUNTIME PM
+ * ON SUB-DEVICE DRIVERS INSTEAD.
+ *
  * React to link management on powered pipelines by updating the use count of
  * all entities in the source and sink sides of the link. Entities are powered
  * on or off accordingly. The use of this function should be paired