Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
Hi Sylwester,
Should I take the series in my tree, or would you like to push it yourself to
avoid conflicts with other Exynos patches ?
On Monday 10 June 2013 01:10:30 Sakari Ailus wrote:
Hi Sylwester,
Thanks for the update.
Sylwester Nawrocki wrote:
...
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c index 1d7dbd5..1a2d25c 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -792,9 +792,9 @@ int omap3isp_pipeline_pm_use(struct media_entity
*entity, int use)
/*
* isp_pipeline_link_notify - Link management notification callback
- * @source: Pad at the start of the link
- * @sink: Pad at the end of the link
+ * @link: The link
* @flags: New link flags that will be applied
+ * @notification: The link's state change notification type
(MEDIA_DEV_NOTIFY_*)
*
* 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
@@ -804,29 +804,38 @@ int omap3isp_pipeline_pm_use(struct media_entity
*entity, int use)
* off is assumed to never fail. This function will not fail for
disconnection * events.
*/
-static int isp_pipeline_link_notify(struct media_pad *source,
- struct media_pad *sink, u32 flags)
+static int isp_pipeline_link_notify(struct media_link *link, u32 flags,
+ unsigned int notification)
{
- int source_use = isp_pipeline_pm_use_count(source-entity);
- int sink_use = isp_pipeline_pm_use_count(sink-entity);
+ struct media_entity *source = link-source-entity;
+ struct media_entity *sink = link-sink-entity;
+ int source_use = isp_pipeline_pm_use_count(source);
+ int sink_use = isp_pipeline_pm_use_count(sink);
int ret;
- if (!(flags MEDIA_LNK_FL_ENABLED)) {
+ if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH
+ !(link-flags MEDIA_LNK_FL_ENABLED)) {
/* Powering off entities is assumed to never fail. */
- isp_pipeline_pm_power(source-entity, -sink_use);
- isp_pipeline_pm_power(sink-entity, -source_use);
+ isp_pipeline_pm_power(source, -sink_use);
+ isp_pipeline_pm_power(sink, -source_use);
return 0;
}
- ret = isp_pipeline_pm_power(source-entity, sink_use);
- if (ret 0)
- return ret;
+ if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH
+ (flags MEDIA_LNK_FL_ENABLED)) {
You could return zero here if the opposite was true, and unindent the
rest. Up to you --- the patch is fine.
Acked-by: Sakari Ailus sakari.ai...@iki.fi
- ret = isp_pipeline_pm_power(sink-entity, source_use);
- if (ret 0)
- isp_pipeline_pm_power(source-entity, -sink_use);
+ ret = isp_pipeline_pm_power(source, sink_use);
+ if (ret 0)
+ return ret;
- return ret;
+ ret = isp_pipeline_pm_power(sink, source_use);
+ if (ret 0)
+ isp_pipeline_pm_power(source, -sink_use);
+
+ return ret;
+ }
+
+ return 0;
}
/*
---
--
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
Hi Laurent, On 06/10/2013 11:46 AM, Laurent Pinchart wrote: Hi Sylwester, Should I take the series in my tree, or would you like to push it yourself to avoid conflicts with other Exynos patches ? My plan was to handle this series together with the other Exynos patches it depends on. I would send a pull request today. I guess there would be the least conflicts this way. Sorry about embedding this core patch in my series. Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
On Monday 10 June 2013 12:20:15 Sylwester Nawrocki wrote: On 06/10/2013 11:46 AM, Laurent Pinchart wrote: Hi Sylwester, Should I take the series in my tree, or would you like to push it yourself to avoid conflicts with other Exynos patches ? My plan was to handle this series together with the other Exynos patches it depends on. I would send a pull request today. I guess there would be the least conflicts this way. Sorry about embedding this core patch in my series. No worries, I'm fine with that. That's one less pull request for me to handle :-) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
Hi Sakari,
On 06/10/2013 12:10 AM, Sakari Ailus wrote:
Sylwester Nawrocki wrote:
...
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index 1d7dbd5..1a2d25c 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -792,9 +792,9 @@ int omap3isp_pipeline_pm_use(struct media_entity
*entity, int use)
/*
* isp_pipeline_link_notify - Link management notification callback
- * @source: Pad at the start of the link
- * @sink: Pad at the end of the link
+ * @link: The link
* @flags: New link flags that will be applied
+ * @notification: The link's state change notification type
(MEDIA_DEV_NOTIFY_*)
*
* 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
@@ -804,29 +804,38 @@ int omap3isp_pipeline_pm_use(struct media_entity
*entity, int use)
* off is assumed to never fail. This function will not fail for
disconnection
* events.
*/
-static int isp_pipeline_link_notify(struct media_pad *source,
-struct media_pad *sink, u32 flags)
+static int isp_pipeline_link_notify(struct media_link *link, u32 flags,
+unsigned int notification)
{
-int source_use = isp_pipeline_pm_use_count(source-entity);
-int sink_use = isp_pipeline_pm_use_count(sink-entity);
+struct media_entity *source = link-source-entity;
+struct media_entity *sink = link-sink-entity;
+int source_use = isp_pipeline_pm_use_count(source);
+int sink_use = isp_pipeline_pm_use_count(sink);
int ret;
-if (!(flags MEDIA_LNK_FL_ENABLED)) {
+if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH
+!(link-flags MEDIA_LNK_FL_ENABLED)) {
/* Powering off entities is assumed to never fail. */
-isp_pipeline_pm_power(source-entity, -sink_use);
-isp_pipeline_pm_power(sink-entity, -source_use);
+isp_pipeline_pm_power(source, -sink_use);
+isp_pipeline_pm_power(sink, -source_use);
return 0;
}
-ret = isp_pipeline_pm_power(source-entity, sink_use);
-if (ret 0)
-return ret;
+if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH
+(flags MEDIA_LNK_FL_ENABLED)) {
You could return zero here if the opposite was true, and unindent the
rest. Up to you --- the patch is fine.
All right, thanks for the Ack. An updated patch to follow.
Acked-by: Sakari Ailus sakari.ai...@iki.fi
-ret = isp_pipeline_pm_power(sink-entity, source_use);
-if (ret 0)
-isp_pipeline_pm_power(source-entity, -sink_use);
+ret = isp_pipeline_pm_power(source, sink_use);
+if (ret 0)
+return ret;
-return ret;
+ret = isp_pipeline_pm_power(sink, source_use);
+if (ret 0)
+isp_pipeline_pm_power(source, -sink_use);
+
+return ret;
+}
+
+return 0;
}
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
On Monday 10 June 2013 12:47:02 Sylwester Nawrocki wrote:
On 06/10/2013 12:10 AM, Sakari Ailus wrote:
Sylwester Nawrocki wrote:
...
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c index 1d7dbd5..1a2d25c 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -792,9 +792,9 @@ int omap3isp_pipeline_pm_use(struct media_entity
*entity, int use)
/*
* isp_pipeline_link_notify - Link management notification callback
- * @source: Pad at the start of the link
- * @sink: Pad at the end of the link
+ * @link: The link
* @flags: New link flags that will be applied
+ * @notification: The link's state change notification type
(MEDIA_DEV_NOTIFY_*)
*
* 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
@@ -804,29 +804,38 @@ int omap3isp_pipeline_pm_use(struct media_entity
*entity, int use)
* off is assumed to never fail. This function will not fail for
disconnection * events.
*/
-static int isp_pipeline_link_notify(struct media_pad *source,
- struct media_pad *sink, u32 flags)
+static int isp_pipeline_link_notify(struct media_link *link, u32 flags,
+ unsigned int notification)
{
- int source_use = isp_pipeline_pm_use_count(source-entity);
- int sink_use = isp_pipeline_pm_use_count(sink-entity);
+ struct media_entity *source = link-source-entity;
+ struct media_entity *sink = link-sink-entity;
+ int source_use = isp_pipeline_pm_use_count(source);
+ int sink_use = isp_pipeline_pm_use_count(sink);
int ret;
- if (!(flags MEDIA_LNK_FL_ENABLED)) {
+ if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH
+ !(link-flags MEDIA_LNK_FL_ENABLED)) {
/* Powering off entities is assumed to never fail. */
- isp_pipeline_pm_power(source-entity, -sink_use);
- isp_pipeline_pm_power(sink-entity, -source_use);
+ isp_pipeline_pm_power(source, -sink_use);
+ isp_pipeline_pm_power(sink, -source_use);
return 0;
}
- ret = isp_pipeline_pm_power(source-entity, sink_use);
- if (ret 0)
- return ret;
+ if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH
+ (flags MEDIA_LNK_FL_ENABLED)) {
You could return zero here if the opposite was true, and unindent the
rest. Up to you --- the patch is fine.
I would personally keep the code as-is, to keep the symmetry, but I'm fine
with both :-)
All right, thanks for the Ack. An updated patch to follow.
Acked-by: Sakari Ailus sakari.ai...@iki.fi
- ret = isp_pipeline_pm_power(sink-entity, source_use);
- if (ret 0)
- isp_pipeline_pm_power(source-entity, -sink_use);
+ ret = isp_pipeline_pm_power(source, sink_use);
+ if (ret 0)
+ return ret;
- return ret;
+ ret = isp_pipeline_pm_power(sink, source_use);
+ if (ret 0)
+ isp_pipeline_pm_power(source, -sink_use);
+
+ return ret;
+ }
+
+ return 0;
}
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
On 06/10/2013 12:49 PM, Laurent Pinchart wrote:
-static int isp_pipeline_link_notify(struct media_pad *source,
- struct media_pad *sink, u32 flags)
+static int isp_pipeline_link_notify(struct media_link *link, u32
flags,
+ unsigned int notification)
{
- int source_use = isp_pipeline_pm_use_count(source-entity);
- int sink_use = isp_pipeline_pm_use_count(sink-entity);
+ struct media_entity *source = link-source-entity;
+ struct media_entity *sink = link-sink-entity;
+ int source_use = isp_pipeline_pm_use_count(source);
+ int sink_use = isp_pipeline_pm_use_count(sink);
int ret;
- if (!(flags MEDIA_LNK_FL_ENABLED)) {
+ if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH
+ !(link-flags MEDIA_LNK_FL_ENABLED)) {
/* Powering off entities is assumed to never fail. */
- isp_pipeline_pm_power(source-entity, -sink_use);
- isp_pipeline_pm_power(sink-entity, -source_use);
+ isp_pipeline_pm_power(source, -sink_use);
+ isp_pipeline_pm_power(sink, -source_use);
return 0;
}
- ret = isp_pipeline_pm_power(source-entity, sink_use);
- if (ret 0)
- return ret;
+ if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH
+ (flags MEDIA_LNK_FL_ENABLED)) {
You could return zero here if the opposite was true, and unindent the
rest. Up to you --- the patch is fine.
I would personally keep the code as-is, to keep the symmetry, but I'm fine
with both :-)
I had also an impression that it looks more symmetric as-is. I would leave it
unchanged then. ;)
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
Hi Sylwester,
Thanks for the patch.
On Sunday 09 June 2013 22:14:37 Sylwester Nawrocki wrote:
Currently the media device link_notify callback is invoked before the
actual change of state of a link when the link is being enabled, and
after the actual change of state when the link is being disabled.
This doesn't allow a media device driver to perform any operations
on a full graph before a link is disabled, as well as performing
any tasks on a modified graph right after a link's state is changed.
This patch modifies signature of the link_notify callback. This
callback is now called always before and after a link's state change.
To distinguish the notifications a 'notification' argument is added
to the link_notify callback: MEDIA_DEV_NOTIFY_PRE_LINK_CH indicates
notification before link's state change and
MEDIA_DEV_NOTIFY_POST_LINK_CH corresponds to a notification after
link flags change.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
This looks good to me. For the media controller core and omap3isp driver,
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
I'd like to have Sakari's ack as well.
---
Changes since v1:
- link_notify callback 'flags' argument's type changed to u32,
- in the omap3isp driver check link-flags instead of the passed flags
argument of the link_notify handler to see if pipeline should be
powered off,
- use link-flags to check link's state in the fimc_md_link_notify()
handler instead of link_notify 'flags' argument.
---
drivers/media/media-entity.c | 18 +++
drivers/media/platform/exynos4-is/media-dev.c | 18 ++-
drivers/media/platform/omap3isp/isp.c | 41 --
include/media/media-device.h |9 -
4 files changed, 47 insertions(+), 39 deletions(-)
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index e1cd132..7004cb0 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -496,25 +496,17 @@ int __media_entity_setup_link(struct media_link *link,
u32 flags)
mdev = source-parent;
- if ((flags MEDIA_LNK_FL_ENABLED) mdev-link_notify) {
- ret = mdev-link_notify(link-source, link-sink,
- MEDIA_LNK_FL_ENABLED);
+ if (mdev-link_notify) {
+ ret = mdev-link_notify(link, flags,
+ MEDIA_DEV_NOTIFY_PRE_LINK_CH);
if (ret 0)
return ret;
}
ret = __media_entity_setup_link_notify(link, flags);
- if (ret 0)
- goto err;
- if (!(flags MEDIA_LNK_FL_ENABLED) mdev-link_notify)
- mdev-link_notify(link-source, link-sink, 0);
-
- return 0;
-
-err:
- if ((flags MEDIA_LNK_FL_ENABLED) mdev-link_notify)
- mdev-link_notify(link-source, link-sink, 0);
+ if (mdev-link_notify)
+ mdev-link_notify(link, flags, MEDIA_DEV_NOTIFY_POST_LINK_CH);
return ret;
}
diff --git a/drivers/media/platform/exynos4-is/media-dev.c
b/drivers/media/platform/exynos4-is/media-dev.c index 424ff92..045a6ae
100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1287,34 +1287,36 @@ int fimc_md_set_camclk(struct v4l2_subdev *sd, bool
on) return __fimc_md_set_camclk(fmd, si, on);
}
-static int fimc_md_link_notify(struct media_pad *source,
-struct media_pad *sink, u32 flags)
+static int fimc_md_link_notify(struct media_link *link, u32 flags,
+ unsigned int notification)
{
+ struct media_entity *sink = link-sink-entity;
struct exynos_video_entity *ve;
struct video_device *vdev;
struct fimc_pipeline *pipeline;
int i, ret = 0;
- if (media_entity_type(sink-entity) != MEDIA_ENT_T_DEVNODE_V4L)
+ if (media_entity_type(sink) != MEDIA_ENT_T_DEVNODE_V4L ||
+ notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH)
return 0;
- vdev = media_entity_to_video_device(sink-entity);
+ vdev = media_entity_to_video_device(sink);
ve = vdev_to_exynos_video_entity(vdev);
pipeline = to_fimc_pipeline(ve-pipe);
- if (!(flags MEDIA_LNK_FL_ENABLED) pipeline-subdevs[IDX_SENSOR]) {
- if (sink-entity-use_count 0)
+ if (!(link-flags MEDIA_LNK_FL_ENABLED)
pipeline-subdevs[IDX_SENSOR]) { +if (sink-use_count 0)
ret = __fimc_pipeline_close(ve-pipe);
for (i = 0; i IDX_MAX; i++)
pipeline-subdevs[i] = NULL;
- } else if (sink-entity-use_count 0) {
+ } else if (sink-use_count 0) {
/*
* Link activation. Enable power of pipeline elements only if
* the pipeline
Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
Hi Sylwester,
Thanks for the update.
Sylwester Nawrocki wrote:
...
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index 1d7dbd5..1a2d25c 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -792,9 +792,9 @@ int omap3isp_pipeline_pm_use(struct media_entity *entity,
int use)
/*
* isp_pipeline_link_notify - Link management notification callback
- * @source: Pad at the start of the link
- * @sink: Pad at the end of the link
+ * @link: The link
* @flags: New link flags that will be applied
+ * @notification: The link's state change notification type
(MEDIA_DEV_NOTIFY_*)
*
* 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
@@ -804,29 +804,38 @@ int omap3isp_pipeline_pm_use(struct media_entity *entity,
int use)
* off is assumed to never fail. This function will not fail for disconnection
* events.
*/
-static int isp_pipeline_link_notify(struct media_pad *source,
- struct media_pad *sink, u32 flags)
+static int isp_pipeline_link_notify(struct media_link *link, u32 flags,
+ unsigned int notification)
{
- int source_use = isp_pipeline_pm_use_count(source-entity);
- int sink_use = isp_pipeline_pm_use_count(sink-entity);
+ struct media_entity *source = link-source-entity;
+ struct media_entity *sink = link-sink-entity;
+ int source_use = isp_pipeline_pm_use_count(source);
+ int sink_use = isp_pipeline_pm_use_count(sink);
int ret;
- if (!(flags MEDIA_LNK_FL_ENABLED)) {
+ if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH
+ !(link-flags MEDIA_LNK_FL_ENABLED)) {
/* Powering off entities is assumed to never fail. */
- isp_pipeline_pm_power(source-entity, -sink_use);
- isp_pipeline_pm_power(sink-entity, -source_use);
+ isp_pipeline_pm_power(source, -sink_use);
+ isp_pipeline_pm_power(sink, -source_use);
return 0;
}
- ret = isp_pipeline_pm_power(source-entity, sink_use);
- if (ret 0)
- return ret;
+ if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH
+ (flags MEDIA_LNK_FL_ENABLED)) {
You could return zero here if the opposite was true, and unindent the
rest. Up to you --- the patch is fine.
Acked-by: Sakari Ailus sakari.ai...@iki.fi
- ret = isp_pipeline_pm_power(sink-entity, source_use);
- if (ret 0)
- isp_pipeline_pm_power(source-entity, -sink_use);
+ ret = isp_pipeline_pm_power(source, sink_use);
+ if (ret 0)
+ return ret;
- return ret;
+ ret = isp_pipeline_pm_power(sink, source_use);
+ if (ret 0)
+ isp_pipeline_pm_power(source, -sink_use);
+
+ return ret;
+ }
+
+ return 0;
}
/*
-
--
Kind regards,
Sakari Ailus
sakari.ai...@iki.fi
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
