From: Steve Longerbeam <slongerbeam@gmail.com>
To: Jacopo Mondi <jacopo@jmondi.org>
Cc: linux-media@vger.kernel.org
Subject: Re: [PATCH v2 05/23] media: entity: Add functions to convert fwnode endpoints to media links
Date: Sat, 14 Dec 2019 13:31:37 -0800 [thread overview]
Message-ID: <a685789a-cfe5-73eb-99d6-034043734a68@gmail.com> (raw)
In-Reply-To: <20191128134657.mlxzawiywyjlqzst@uno.localdomain>
Hi Jacopo,
On 11/28/19 5:46 AM, Jacopo Mondi wrote:
> Hi Steve,
>
> On Sun, Nov 24, 2019 at 11:06:45AM -0800, Steve Longerbeam wrote:
>> Adds two functions:
>>
>> media_create_fwnode_pad_links(), which converts fwnode endpoints that
>> connect a local pad to all pads on a remote entity into media links.
>>
>> media_create_fwnode_links(), which converts fwnode endpoints that
>> connect all pads from a local entity to all pads on a remote entity into
>> media links.
>>
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> ---
>> Changes in v2:
>> - fixed/improved the prototype descriptions.
>> ---
>> drivers/media/mc/mc-entity.c | 178 +++++++++++++++++++++++++++++++++++
>> include/media/media-entity.h | 71 ++++++++++++++
>> 2 files changed, 249 insertions(+)
>>
>> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
>> index e9e090244fd4..45bbd6c91019 100644
>> --- a/drivers/media/mc/mc-entity.c
>> +++ b/drivers/media/mc/mc-entity.c
>> @@ -787,6 +787,184 @@ int media_create_pad_links(const struct media_device *mdev,
>> }
>> EXPORT_SYMBOL_GPL(media_create_pad_links);
>>
>> +static int __media_create_fwnode_pad_link(struct media_pad *local_pad,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_ep,
>> + const u32 flags)
>> +{
>> + struct media_entity *local = local_pad->entity;
>> + unsigned long local_dir = local_pad->flags;
>> + unsigned long remote_dir = (local_dir & MEDIA_PAD_FL_SOURCE) ?
>> + MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
>> + struct media_entity *src, *sink;
>> + int src_pad, sink_pad;
>> + int remote_pad;
>> + int ret;
>> +
>> + remote_pad = media_entity_get_fwnode_pad(remote, remote_ep,
>> + remote_dir);
>> + if (remote_pad < 0)
>> + return 0;
>> +
>> + if (local_dir & MEDIA_PAD_FL_SOURCE) {
>> + src = local;
>> + src_pad = local_pad->index;
>> + sink = remote;
>> + sink_pad = remote_pad;
>> + } else {
>> + src = remote;
>> + src_pad = remote_pad;
>> + sink = local;
>> + sink_pad = local_pad->index;
>> + }
>> +
>> + /* make sure link doesn't already exist */
>> + if (media_entity_find_link(&src->pads[src_pad],
>> + &sink->pads[sink_pad]))
>> + return 0;
>> +
>> + ret = media_create_pad_link(src, src_pad, sink, sink_pad, flags);
>> + if (ret) {
>> + dev_dbg(sink->graph_obj.mdev->dev,
>> + "%s:%d -> %s:%d failed with %d\n",
>> + src->name, src_pad, sink->name, sink_pad,
>> + ret);
>> + return ret;
>> + }
>> +
>> + dev_dbg(sink->graph_obj.mdev->dev, "%s:%d -> %s:%d\n",
>> + src->name, src_pad, sink->name, sink_pad);
>> +
>> + return 0;
>> +}
>> +
>> +int __media_create_fwnode_pad_links(struct media_pad *local_pad,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags)
>> +{
>> + struct fwnode_handle *endpoint;
>> +
>> + fwnode_graph_for_each_endpoint(remote_fwnode, endpoint) {
>> + struct fwnode_link link;
>> + int ret;
>> +
>> + ret = fwnode_graph_parse_link(endpoint, &link);
>> + if (ret)
>> + continue;
>> +
>> + /*
>> + * if this endpoint does not link to the local fwnode
>> + * device, ignore it and continue.
>> + */
>> + if (link.remote_port_parent != local_fwnode) {
>> + fwnode_graph_put_link(&link);
>> + continue;
>> + }
>> +
>> + ret = __media_create_fwnode_pad_link(local_pad,
>> + remote, endpoint,
>> + link_flags);
>> +
>> + fwnode_graph_put_link(&link);
>> +
>> + if (ret) {
>> + fwnode_handle_put(endpoint);
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(__media_create_fwnode_pad_links);
>> +
>> +int media_create_fwnode_pad_links(struct media_pad *local_pad,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags)
>> +{
>> + struct media_device *mdev = local_pad->entity->graph_obj.mdev;
>> + int ret;
>> +
>> + mutex_lock(&mdev->graph_mutex);
>> + ret = __media_create_fwnode_pad_links(local_pad, local_fwnode,
>> + remote, remote_fwnode,
>> + link_flags);
>> + mutex_unlock(&mdev->graph_mutex);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(media_create_fwnode_pad_links);
>> +
>> +int __media_create_fwnode_links(struct media_entity *local,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags)
>> +{
>> + struct fwnode_handle *endpoint;
>> +
>> + fwnode_graph_for_each_endpoint(local_fwnode, endpoint) {
>> + struct fwnode_link link;
>> + int local_pad;
>> + int ret;
>> +
>> + local_pad = media_entity_get_fwnode_pad(local, endpoint,
>> + MEDIA_PAD_FL_SINK |
>> + MEDIA_PAD_FL_SOURCE);
> I wonder.. I feel like we could have saved a lot of churn if we record
> the local endpoint on which we register an async device, likely in
> struct v4l2_async_subdev.
>
> At bound() time we would receive back the local endpoint on which the
> just bound subdev was originally registered, we could get the remote
> endpoint by parsing the fwnode_graph_link and from there we could
> provide utilities like the ones you have here, by saving testing all
> endpoints until we don't find one that matches the subdev which got
> bound.
>
> This would probably just work for V4L2_ASYNC_MATCH_FWNODE though, but
> have you considered this solution ? It would avoid trying all the
> local endpoints blindly here and it would encourage drivers to provide
> a function to do the endpoint->pad_index translation (which ideally
> they should, to avoid workarounds like the ones we have in
> media_entity_get_fwnode_pad()
It sounds like a promising idea, I'll start looking into it.
Steve
> Thanks
> j
>
>> + if (local_pad < 0)
>> + continue;
>> +
>> + ret = fwnode_graph_parse_link(endpoint, &link);
>> + if (ret)
>> + continue;
>> +
>> + /*
>> + * if this endpoint does not link to the remote fwnode
>> + * device, ignore it and continue.
>> + */
>> + if (link.remote_port_parent != remote_fwnode) {
>> + fwnode_graph_put_link(&link);
>> + continue;
>> + }
>> +
>> + ret = __media_create_fwnode_pad_link(&local->pads[local_pad],
>> + remote,
>> + link.remote.local_fwnode,
>> + link_flags);
>> +
>> + fwnode_graph_put_link(&link);
>> +
>> + if (ret) {
>> + fwnode_handle_put(endpoint);
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(__media_create_fwnode_links);
>> +
>> +int media_create_fwnode_links(struct media_entity *local,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags)
>> +{
>> + struct media_device *mdev = local->graph_obj.mdev;
>> + int ret;
>> +
>> + mutex_lock(&mdev->graph_mutex);
>> + ret = __media_create_fwnode_links(local, local_fwnode,
>> + remote, remote_fwnode, link_flags);
>> + mutex_unlock(&mdev->graph_mutex);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(media_create_fwnode_links);
>> +
>> void __media_entity_remove_links(struct media_entity *entity)
>> {
>> struct media_link *link, *tmp;
>> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
>> index de7fc3676b5a..100673ad83c4 100644
>> --- a/include/media/media-entity.h
>> +++ b/include/media/media-entity.h
>> @@ -772,6 +772,77 @@ int media_create_pad_links(const struct media_device *mdev,
>> u32 flags,
>> const bool allow_both_undefined);
>>
>> +/**
>> + * media_create_fwnode_pad_links() - create links between a single local pad
>> + * and a remote entity, using the fwnode endpoints
>> + * between them.
>> + *
>> + * @local_pad: Pointer to &media_pad of the local media pad.
>> + * @local_fwnode: Pointer to the local device's firmware node.
>> + * @remote: Pointer to &media_entity of the remote device.
>> + * @remote_fwnode: Pointer to the remote device's firmware node.
>> + * @link_flags: Link flags, as defined in include/uapi/linux/media.h.
>> + *
>> + * .. note::
>> + *
>> + * Before calling this function, media_entity_pads_init() and
>> + * media_device_register_entity() should be called previously for
>> + * both entities to be linked.
>> + *
>> + * Locked (via the mdev graph_mutex) and unlocked versions of this
>> + * function are provided. If this function is called from v4l2-async
>> + * notifier bound handlers, the locked version should be used to
>> + * prevent races with other subdevices loading and binding to their
>> + * notifiers in parallel. The unlocked version can for example be
>> + * called from v4l2-async notifier complete handlers, after all
>> + * subdevices have loaded and bound.
>> + */
>> +int __media_create_fwnode_pad_links(struct media_pad *local_pad,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags);
>> +int media_create_fwnode_pad_links(struct media_pad *local_pad,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags);
>> +
>> +/**
>> + * media_create_fwnode_links() - create links between two entities, using
>> + * the fwnode endpoints between them.
>> + *
>> + * @local: Pointer to &media_entity of the local device.
>> + * @local_fwnode: Pointer to the local device's firmware node.
>> + * @remote: Pointer to &media_entity of the remote device.
>> + * @remote_fwnode: Pointer to the remote device's firmware node.
>> + * @link_flags: Link flags, as defined in include/uapi/linux/media.h.
>> + *
>> + * .. note::
>> + *
>> + * Before calling this function, media_entity_pads_init() and
>> + * media_device_register_entity() should be called previously for
>> + * both entities to be linked.
>> + *
>> + * Locked (via the mdev graph_mutex) and unlocked versions of this
>> + * function are provided. If this function is called from v4l2-async
>> + * notifier bound handlers, the locked version should be used to
>> + * prevent races with other subdevices loading and binding to their
>> + * notifiers in parallel. The unlocked version can for example be
>> + * called from v4l2-async notifier complete handlers, after all
>> + * subdevices have loaded and bound.
>> + */
>> +int __media_create_fwnode_links(struct media_entity *local,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags);
>> +int media_create_fwnode_links(struct media_entity *local,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags);
>> +
>> void __media_entity_remove_links(struct media_entity *entity);
>>
>> /**
>> --
>> 2.17.1
>>
next prev parent reply other threads:[~2019-12-14 21:31 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 01/23] media: entity: Pass entity to get_fwnode_pad operation Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 02/23] media: entity: Modify default behavior of media_entity_get_fwnode_pad Steve Longerbeam
2019-11-28 12:04 ` Jacopo Mondi
2019-12-14 21:29 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 03/23] media: entity: Convert media_entity_get_fwnode_pad() args to const Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 04/23] media: Move v4l2_fwnode_parse_link from v4l2 to driver base Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 05/23] media: entity: Add functions to convert fwnode endpoints to media links Steve Longerbeam
2019-11-28 13:46 ` Jacopo Mondi
2019-12-14 21:31 ` Steve Longerbeam [this message]
2019-11-24 19:06 ` [PATCH v2 06/23] media: adv748x: csi2: Implement get_fwnode_pad Steve Longerbeam
2019-11-28 11:53 ` Jacopo Mondi
2019-12-14 20:43 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 07/23] media: rcar-csi2: Fix fwnode media link creation Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 08/23] media: cadence: csi2rx: " Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 09/23] media: sun6i: " Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 10/23] media: st-mipid02: " Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 11/23] media: stm32-dcmi: " Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 12/23] media: sunxi: " Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 13/23] media: v4l2-fwnode: Pass notifier to v4l2_async_register_fwnode_subdev() Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 14/23] media: video-mux: Create media links in bound notifier Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 15/23] media: imx: mipi csi-2: " Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 16/23] media: imx7-mipi-csis: " Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 17/23] media: imx7-media-csi: " Steve Longerbeam
2019-11-26 22:49 ` Rui Miguel Silva
2019-11-24 19:06 ` [PATCH v2 18/23] media: imx: csi: Implement get_fwnode_pad Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 19/23] media: imx: csi: Embed notifier in struct csi_priv Steve Longerbeam
2019-11-24 19:07 ` [PATCH v2 20/23] media: imx: csi: Create media links in bound notifier Steve Longerbeam
2019-11-24 19:07 ` [PATCH v2 21/23] media: imx: csi: Lookup upstream endpoint with imx_media_get_pad_fwnode Steve Longerbeam
2019-11-24 19:07 ` [PATCH v2 22/23] media: imx: Create missing links from CSI-2 receiver Steve Longerbeam
2019-11-24 19:07 ` [PATCH v2 23/23] media: imx: TODO: Remove media link creation todos Steve Longerbeam
2019-11-27 2:10 ` [PATCH v2 00/23] media: imx: Create media links in bound notifiers Adam Ford
2019-11-27 2:23 ` Steve Longerbeam
2019-11-27 2:31 ` Adam Ford
2019-12-16 17:20 ` Adam Ford
2020-02-02 19:30 ` Steve Longerbeam
2020-02-04 10:53 ` Adam Ford
2020-02-05 0:44 ` Steve Longerbeam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a685789a-cfe5-73eb-99d6-034043734a68@gmail.com \
--to=slongerbeam@gmail.com \
--cc=jacopo@jmondi.org \
--cc=linux-media@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).