* [REVIEW PATCH v3 0/2] Media link_notify behaviour change and exynos4-is updates
@ 2013-06-09 20:14 Sylwester Nawrocki
2013-06-09 20:14 ` [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour Sylwester Nawrocki
2013-06-09 20:14 ` [REVIEW PATCH v3 2/2] exynos4-is: Extend link_notify handler to support fimc-is/lite pipelines Sylwester Nawrocki
0 siblings, 2 replies; 11+ messages in thread
From: Sylwester Nawrocki @ 2013-06-09 20:14 UTC (permalink / raw)
To: linux-media
Cc: laurent.pinchart, sakari.ailus, kyungmin.park, sw0312.kim,
a.hajda, hj210.choi, shaik.ameer, arun.kk, s.nawrocki
Hi All,
This is an updated version of the patch set [1], I've dropped patches
01/11...09/11, which I've already queued for 3.11. This series sits on
top of my for-next branch [2].
This iteration attempts to address issues pointed out by Sakari,
thanks! The changes since v2 were:
- link_notify callback 'flags' argument's type changed to u32,
- in the omap3isp driver link->flags checked instead of the passed
flags argument of the link_notify handler to see if the pipeline
should be powered off,
- in the exynos4-is driver link->flags checked instead of the flags
argument of the fimc_md_link_notify() handler to see if the pipelines
should be powered on.
Thanks,
Sylwester
[1] http://www.spinics.net/lists/linux-media/msg64258.html
[2] http://git.linuxtv.org/snawrocki/samsung.git/for-next
Sylwester Nawrocki (2):
media: Change media device link_notify behaviour
exynos4-is: Extend link_notify handler to support fimc-is/lite
pipelines
drivers/media/media-entity.c | 18 +---
drivers/media/platform/exynos4-is/media-dev.c | 101 ++++++++++++++++++++-----
drivers/media/platform/omap3isp/isp.c | 41 ++++++----
include/media/media-device.h | 9 ++-
4 files changed, 118 insertions(+), 51 deletions(-)
--
1.7.4.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
2013-06-09 20:14 [REVIEW PATCH v3 0/2] Media link_notify behaviour change and exynos4-is updates Sylwester Nawrocki
@ 2013-06-09 20:14 ` Sylwester Nawrocki
2013-06-09 20:33 ` Laurent Pinchart
2013-06-09 22:10 ` Sakari Ailus
2013-06-09 20:14 ` [REVIEW PATCH v3 2/2] exynos4-is: Extend link_notify handler to support fimc-is/lite pipelines Sylwester Nawrocki
1 sibling, 2 replies; 11+ messages in thread
From: Sylwester Nawrocki @ 2013-06-09 20:14 UTC (permalink / raw)
To: linux-media
Cc: laurent.pinchart, sakari.ailus, kyungmin.park, sw0312.kim,
a.hajda, hj210.choi, shaik.ameer, arun.kk, s.nawrocki
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.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
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 is already in use, i.e. its video node is open.
* Recreate the controls destroyed during the link deactivation.
*/
- ret = __fimc_pipeline_open(ve->pipe, sink->entity, true);
+ ret = __fimc_pipeline_open(ve->pipe, sink, true);
}
return ret ? -EPIPE : ret;
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)) {
- 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;
}
/* -----------------------------------------------------------------------------
diff --git a/include/media/media-device.h b/include/media/media-device.h
index eaade98..12155a9 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -45,6 +45,7 @@ struct device;
* @entities: List of registered entities
* @lock: Entities list lock
* @graph_mutex: Entities graph operation lock
+ * @link_notify: Link state change notification callback
*
* This structure represents an abstract high-level media device. It allows easy
* access to entities and provides basic media device-level support. The
@@ -75,10 +76,14 @@ struct media_device {
/* Serializes graph operations. */
struct mutex graph_mutex;
- int (*link_notify)(struct media_pad *source,
- struct media_pad *sink, u32 flags);
+ int (*link_notify)(struct media_link *link, u32 flags,
+ unsigned int notification);
};
+/* Supported link_notify @notification values. */
+#define MEDIA_DEV_NOTIFY_PRE_LINK_CH 0
+#define MEDIA_DEV_NOTIFY_POST_LINK_CH 1
+
/* media_devnode to media_device */
#define to_media_device(node) container_of(node, struct media_device, devnode)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [REVIEW PATCH v3 2/2] exynos4-is: Extend link_notify handler to support fimc-is/lite pipelines
2013-06-09 20:14 [REVIEW PATCH v3 0/2] Media link_notify behaviour change and exynos4-is updates Sylwester Nawrocki
2013-06-09 20:14 ` [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour Sylwester Nawrocki
@ 2013-06-09 20:14 ` Sylwester Nawrocki
1 sibling, 0 replies; 11+ messages in thread
From: Sylwester Nawrocki @ 2013-06-09 20:14 UTC (permalink / raw)
To: linux-media
Cc: laurent.pinchart, sakari.ailus, kyungmin.park, sw0312.kim,
a.hajda, hj210.choi, shaik.ameer, arun.kk, s.nawrocki
This patch corrects the link_notify handler to support more complex
pipelines, including fimc-lite and fimc-is entities.
After the FIMC-IS driver addition the assumptions made in the link_notify
callback are no longer valid, e.g. the link between fimc-lite subdev and
its video node is not immutable any more and there is more subdevs than
just sensor, MIPI-CSIS and FIMC(-LITE).
The graph is now walked and for each video node found a media pipeline
which ends at this node is disabled/enabled (the subdevs are powered
on/off).
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes since v1:
- check link->flags instead of the flags argument of link_notify handler
to see if the pipelines' should be powered back on.
---
drivers/media/platform/exynos4-is/media-dev.c | 103 +++++++++++++++++++-----
1 files changed, 81 insertions(+), 22 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 045a6ae..ec79ebb 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1287,39 +1287,98 @@ 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_link *link, u32 flags,
- unsigned int notification)
+static int __fimc_md_modify_pipeline(struct media_entity *entity, bool enable)
{
- struct media_entity *sink = link->sink->entity;
struct exynos_video_entity *ve;
+ struct fimc_pipeline *p;
struct video_device *vdev;
- struct fimc_pipeline *pipeline;
- int i, ret = 0;
+ int ret;
- if (media_entity_type(sink) != MEDIA_ENT_T_DEVNODE_V4L ||
- notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH)
+ vdev = media_entity_to_video_device(entity);
+ if (vdev->entity.use_count == 0)
return 0;
- vdev = media_entity_to_video_device(sink);
ve = vdev_to_exynos_video_entity(vdev);
- pipeline = to_fimc_pipeline(ve->pipe);
+ p = to_fimc_pipeline(ve->pipe);
+ /*
+ * Nothing to do if we are disabling the pipeline, some link
+ * has been disconnected and p->subdevs array is cleared now.
+ */
+ if (!enable && p->subdevs[IDX_SENSOR] == NULL)
+ return 0;
- if (!(link->flags & MEDIA_LNK_FL_ENABLED) && pipeline->subdevs[IDX_SENSOR]) {
- if (sink->use_count > 0)
- ret = __fimc_pipeline_close(ve->pipe);
+ if (enable)
+ ret = __fimc_pipeline_open(ve->pipe, entity, true);
+ else
+ ret = __fimc_pipeline_close(ve->pipe);
- for (i = 0; i < IDX_MAX; i++)
- pipeline->subdevs[i] = NULL;
- } else if (sink->use_count > 0) {
- /*
- * Link activation. Enable power of pipeline elements only if
- * the pipeline is already in use, i.e. its video node is open.
- * Recreate the controls destroyed during the link deactivation.
- */
- ret = __fimc_pipeline_open(ve->pipe, sink, true);
+ if (ret == 0 && !enable)
+ memset(p->subdevs, 0, sizeof(p->subdevs));
+
+ return ret;
+}
+
+/* Locking: called with entity->parent->graph_mutex mutex held. */
+static int __fimc_md_modify_pipelines(struct media_entity *entity, bool enable)
+{
+ struct media_entity *entity_err = entity;
+ struct media_entity_graph graph;
+ int ret;
+
+ /*
+ * Walk current graph and call the pipeline open/close routine for each
+ * opened video node that belongs to the graph of entities connected
+ * through active links. This is needed as we cannot power on/off the
+ * subdevs in random order.
+ */
+ media_entity_graph_walk_start(&graph, entity);
+
+ while ((entity = media_entity_graph_walk_next(&graph))) {
+ if (media_entity_type(entity) != MEDIA_ENT_T_DEVNODE)
+ continue;
+
+ ret = __fimc_md_modify_pipeline(entity, enable);
+
+ if (ret < 0)
+ goto err;
+ }
+
+ return 0;
+ err:
+ media_entity_graph_walk_start(&graph, entity_err);
+
+ while ((entity_err = media_entity_graph_walk_next(&graph))) {
+ if (media_entity_type(entity_err) != MEDIA_ENT_T_DEVNODE)
+ continue;
+
+ __fimc_md_modify_pipeline(entity_err, !enable);
+
+ if (entity_err == entity)
+ break;
+ }
+
+ return ret;
+}
+
+static int fimc_md_link_notify(struct media_link *link, unsigned int flags,
+ unsigned int notification)
+{
+ struct media_entity *sink = link->sink->entity;
+ int ret = 0;
+
+ /* Before link disconnection */
+ if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH) {
+ if (!(flags & MEDIA_LNK_FL_ENABLED))
+ ret = __fimc_md_modify_pipelines(sink, false);
+ else
+ ; /* TODO: Link state change validation */
+ /* After link activation */
+ } else if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH &&
+ (link->flags & MEDIA_LNK_FL_ENABLED)) {
+ ret = __fimc_md_modify_pipelines(sink, true);
}
- return ret ? -EPIPE : ret;
+ return ret ? -EPIPE : 0;
}
static ssize_t fimc_md_sysfs_show(struct device *dev,
--
1.7.4.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
2013-06-09 20:14 ` [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour Sylwester Nawrocki
@ 2013-06-09 20:33 ` Laurent Pinchart
2013-06-09 22:10 ` Sakari Ailus
1 sibling, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2013-06-09 20:33 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: linux-media, sakari.ailus, kyungmin.park, sw0312.kim, a.hajda,
hj210.choi, shaik.ameer, arun.kk, s.nawrocki
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.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
This looks good to me. For the media controller core and omap3isp driver,
Acked-by: Laurent Pinchart <laurent.pinchart@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 is already in use, i.e. its video node is open.
> * Recreate the controls destroyed during the link deactivation.
> */
> - ret = __fimc_pipeline_open(ve->pipe, sink->entity, true);
> + ret = __fimc_pipeline_open(ve->pipe, sink, true);
> }
>
> return ret ? -EPIPE : ret;
> 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)) {
>
> - 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;
> }
>
> /*
> ---------------------------------------------------------------------------
> -- diff --git a/include/media/media-device.h b/include/media/media-device.h
> index eaade98..12155a9 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -45,6 +45,7 @@ struct device;
> * @entities: List of registered entities
> * @lock: Entities list lock
> * @graph_mutex: Entities graph operation lock
> + * @link_notify: Link state change notification callback
> *
> * This structure represents an abstract high-level media device. It allows
> easy * access to entities and provides basic media device-level support.
> The @@ -75,10 +76,14 @@ struct media_device {
> /* Serializes graph operations. */
> struct mutex graph_mutex;
>
> - int (*link_notify)(struct media_pad *source,
> - struct media_pad *sink, u32 flags);
> + int (*link_notify)(struct media_link *link, u32 flags,
> + unsigned int notification);
> };
>
> +/* Supported link_notify @notification values. */
> +#define MEDIA_DEV_NOTIFY_PRE_LINK_CH 0
> +#define MEDIA_DEV_NOTIFY_POST_LINK_CH 1
> +
> /* media_devnode to media_device */
> #define to_media_device(node) container_of(node, struct media_device,
> devnode)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
2013-06-09 20:14 ` [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour Sylwester Nawrocki
2013-06-09 20:33 ` Laurent Pinchart
@ 2013-06-09 22:10 ` Sakari Ailus
2013-06-10 9:46 ` Laurent Pinchart
2013-06-10 10:47 ` Sylwester Nawrocki
1 sibling, 2 replies; 11+ messages in thread
From: Sakari Ailus @ 2013-06-09 22:10 UTC (permalink / raw)
To: Sylwester Nawrocki, linux-media
Cc: laurent.pinchart, kyungmin.park, sw0312.kim, a.hajda, hj210.choi,
shaik.ameer, arun.kk, s.nawrocki
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.ailus@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.ailus@iki.fi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
2013-06-09 22:10 ` Sakari Ailus
@ 2013-06-10 9:46 ` Laurent Pinchart
2013-06-10 10:20 ` Sylwester Nawrocki
2013-06-10 10:47 ` Sylwester Nawrocki
1 sibling, 1 reply; 11+ messages in thread
From: Laurent Pinchart @ 2013-06-10 9:46 UTC (permalink / raw)
To: Sakari Ailus
Cc: Sylwester Nawrocki, linux-media, kyungmin.park, sw0312.kim,
a.hajda, hj210.choi, shaik.ameer, arun.kk, s.nawrocki
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.ailus@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
2013-06-10 9:46 ` Laurent Pinchart
@ 2013-06-10 10:20 ` Sylwester Nawrocki
2013-06-10 10:21 ` Laurent Pinchart
0 siblings, 1 reply; 11+ messages in thread
From: Sylwester Nawrocki @ 2013-06-10 10:20 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sakari Ailus, linux-media, kyungmin.park, sw0312.kim, a.hajda,
hj210.choi, shaik.ameer, arun.kk
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
2013-06-10 10:20 ` Sylwester Nawrocki
@ 2013-06-10 10:21 ` Laurent Pinchart
0 siblings, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2013-06-10 10:21 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Sakari Ailus, linux-media, kyungmin.park, sw0312.kim, a.hajda,
hj210.choi, shaik.ameer, arun.kk
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
2013-06-09 22:10 ` Sakari Ailus
2013-06-10 9:46 ` Laurent Pinchart
@ 2013-06-10 10:47 ` Sylwester Nawrocki
2013-06-10 10:49 ` Laurent Pinchart
1 sibling, 1 reply; 11+ messages in thread
From: Sylwester Nawrocki @ 2013-06-10 10:47 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, laurent.pinchart, kyungmin.park, sw0312.kim,
a.hajda, hj210.choi, shaik.ameer, arun.kk, s.nawrocki
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.ailus@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
2013-06-10 10:47 ` Sylwester Nawrocki
@ 2013-06-10 10:49 ` Laurent Pinchart
2013-06-10 10:59 ` Sylwester Nawrocki
0 siblings, 1 reply; 11+ messages in thread
From: Laurent Pinchart @ 2013-06-10 10:49 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Sakari Ailus, linux-media, kyungmin.park, sw0312.kim, a.hajda,
hj210.choi, shaik.ameer, arun.kk
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.ailus@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour
2013-06-10 10:49 ` Laurent Pinchart
@ 2013-06-10 10:59 ` Sylwester Nawrocki
0 siblings, 0 replies; 11+ messages in thread
From: Sylwester Nawrocki @ 2013-06-10 10:59 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sakari Ailus, linux-media, kyungmin.park, sw0312.kim, a.hajda,
hj210.choi, shaik.ameer, arun.kk
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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-06-10 10:59 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-09 20:14 [REVIEW PATCH v3 0/2] Media link_notify behaviour change and exynos4-is updates Sylwester Nawrocki
2013-06-09 20:14 ` [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour Sylwester Nawrocki
2013-06-09 20:33 ` Laurent Pinchart
2013-06-09 22:10 ` Sakari Ailus
2013-06-10 9:46 ` Laurent Pinchart
2013-06-10 10:20 ` Sylwester Nawrocki
2013-06-10 10:21 ` Laurent Pinchart
2013-06-10 10:47 ` Sylwester Nawrocki
2013-06-10 10:49 ` Laurent Pinchart
2013-06-10 10:59 ` Sylwester Nawrocki
2013-06-09 20:14 ` [REVIEW PATCH v3 2/2] exynos4-is: Extend link_notify handler to support fimc-is/lite pipelines Sylwester Nawrocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).