| Message ID | 1370808878-11379-2-git-send-email-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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)
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; > } > > /* -----------------------------------------------------------------------------
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; > > > > } > > > > /* > > ----------------------------------------------------------------------- > > ------
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 :-)
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 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; > >> > >> }
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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)