| Message ID | 20240822214125.3161-1-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: v4l2-mc: Mark v4l2_pipeline_link_notify() as deprecated | expand |
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
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
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
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
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 >
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 --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
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