From 28a1295795d85a25f2e7dd391c43969e95fcb341 Mon Sep 17 00:00:00 2001 From: Sakari Ailus Date: Fri, 19 May 2023 13:44:03 +0200 Subject: [PATCH] media: v4l: async: Allow multiple connections between entities MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit When the v4l2-async framework was introduced, the use case for it was to connect a camera sensor with a parallel receiver. Both tended to be rather simple devices with a single connection between them. The framework has been since improved in multiple ways but there are limitations that have remained, for instance the assumption an async sub-device is connected towards a single notifier and via a single link only. This patch enables connecting a sub-device to one or more notifiers simultaneously, with one or more connections per notifier. The notifier information is moved from the sub-device to the connection and the connections in sub-device are no longer a pointer but a linked list. Signed-off-by: Sakari Ailus Tested-by: Philipp Zabel # imx6qp Tested-by: Niklas Söderlund # rcar + adv746x Tested-by: Aishwarya Kothari # Apalis i.MX6Q with TC358743 Tested-by: Lad Prabhakar # Renesas RZ/G2L SMARC Signed-off-by: Mauro Carvalho Chehab --- drivers/media/v4l2-core/v4l2-async.c | 149 ++++++++++++++++++----------------- include/media/v4l2-async.h | 17 ++-- include/media/v4l2-subdev.h | 7 +- 3 files changed, 90 insertions(+), 83 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index cb962b0..6bd3e17 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -313,29 +313,43 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, struct v4l2_async_connection *asc) { struct v4l2_async_notifier *subdev_notifier; + bool registered = false; int ret; - ret = v4l2_device_register_subdev(v4l2_dev, sd); - if (ret < 0) - return ret; + if (list_empty(&sd->asc_list)) { + ret = v4l2_device_register_subdev(v4l2_dev, sd); + if (ret < 0) + return ret; + registered = true; + } ret = v4l2_async_nf_call_bound(notifier, sd, asc); - if (ret < 0) + if (ret < 0) { + if (asc->match.type == V4L2_ASYNC_MATCH_TYPE_FWNODE) + dev_dbg(notifier_dev(notifier), + "failed binding %pfw (%d)\n", + asc->match.fwnode, ret); goto err_unregister_subdev; + } - /* - * Depending of the function of the entities involved, we may want to - * create links between them (for example between a sensor and its lens - * or between a sensor's source pad and the connected device's sink - * pad). - */ - ret = v4l2_async_create_ancillary_links(notifier, sd); - if (ret) - goto err_call_unbind; - - sd->asd = asc; - sd->notifier = notifier; + if (registered) { + /* + * Depending of the function of the entities involved, we may + * want to create links between them (for example between a + * sensor and its lens or between a sensor's source pad and the + * connected device's sink pad). + */ + ret = v4l2_async_create_ancillary_links(notifier, sd); + if (ret) { + if (asc->match.type == V4L2_ASYNC_MATCH_TYPE_FWNODE) + dev_dbg(notifier_dev(notifier), + "failed creating links for %pfw (%d)\n", + asc->match.fwnode, ret); + goto err_call_unbind; + } + } + list_add(&asc->asc_subdev_entry, &sd->asc_list); asc->sd = sd; /* Move from the waiting list to notifier's done */ @@ -362,9 +376,11 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, err_call_unbind: v4l2_async_nf_call_unbind(notifier, sd, asc); + list_del(&asc->asc_subdev_entry); err_unregister_subdev: - v4l2_device_unregister_subdev(sd); + if (registered) + v4l2_device_unregister_subdev(sd); return ret; } @@ -410,15 +426,16 @@ again: return 0; } -static void v4l2_async_cleanup(struct v4l2_subdev *sd) +static void v4l2_async_unbind_subdev_one(struct v4l2_async_notifier *notifier, + struct v4l2_async_connection *asc) { - v4l2_device_unregister_subdev(sd); - /* - * Subdevice driver will reprobe and put the subdev back - * onto the list - */ - list_del_init(&sd->async_list); - sd->asd = NULL; + list_move_tail(&asc->asc_entry, ¬ifier->waiting_list); + if (list_is_singular(&asc->asc_subdev_entry)) { + v4l2_async_nf_call_unbind(notifier, asc->sd, asc); + v4l2_device_unregister_subdev(asc->sd); + asc->sd = NULL; + } + list_del(&asc->asc_subdev_entry); } /* Unbind all sub-devices in the notifier tree. */ @@ -435,11 +452,7 @@ v4l2_async_nf_unbind_all_subdevs(struct v4l2_async_notifier *notifier) if (subdev_notifier) v4l2_async_nf_unbind_all_subdevs(subdev_notifier); - v4l2_async_nf_call_unbind(notifier, asc->sd, asc); - v4l2_async_cleanup(asc->sd); - list_move_tail(&asc->asc_entry, ¬ifier->waiting_list); - list_move(&asc->sd->async_list, &subdev_list); - asc->sd = NULL; + v4l2_async_unbind_subdev_one(notifier, asc); } notifier->parent = NULL; @@ -456,13 +469,9 @@ v4l2_async_nf_has_async_match_entry(struct v4l2_async_notifier *notifier, if (v4l2_async_match_equal(&asc->match, match)) return true; - list_for_each_entry(asc, ¬ifier->done_list, asc_entry) { - if (WARN_ON(!asc->sd->asd)) - continue; - + list_for_each_entry(asc, ¬ifier->done_list, asc_entry) if (v4l2_async_match_equal(&asc->match, match)) return true; - } return false; } @@ -642,16 +651,12 @@ static void __v4l2_async_nf_cleanup(struct v4l2_async_notifier *notifier) WARN_ON(!list_empty(¬ifier->done_list)); list_for_each_entry_safe(asc, tmp, ¬ifier->waiting_list, asc_entry) { - switch (asc->match.type) { - case V4L2_ASYNC_MATCH_TYPE_FWNODE: - fwnode_handle_put(asc->match.fwnode); - break; - default: - break; - } - list_del(&asc->asc_entry); v4l2_async_nf_call_destroy(notifier, asc); + + if (asc->match.type == V4L2_ASYNC_MATCH_TYPE_FWNODE) + fwnode_handle_put(asc->match.fwnode); + kfree(asc); } } @@ -666,16 +671,14 @@ void v4l2_async_nf_cleanup(struct v4l2_async_notifier *notifier) } EXPORT_SYMBOL_GPL(v4l2_async_nf_cleanup); -static int __v4l2_async_nf_add_connection(struct v4l2_async_notifier *notifier, - struct v4l2_async_connection *asc) +static void __v4l2_async_nf_add_connection(struct v4l2_async_notifier *notifier, + struct v4l2_async_connection *asc) { mutex_lock(&list_lock); list_add_tail(&asc->asc_entry, ¬ifier->waiting_list); mutex_unlock(&list_lock); - - return 0; } struct v4l2_async_connection * @@ -684,21 +687,16 @@ __v4l2_async_nf_add_fwnode(struct v4l2_async_notifier *notifier, unsigned int asc_struct_size) { struct v4l2_async_connection *asc; - int ret; asc = kzalloc(asc_struct_size, GFP_KERNEL); if (!asc) return ERR_PTR(-ENOMEM); + asc->notifier = notifier; asc->match.type = V4L2_ASYNC_MATCH_TYPE_FWNODE; asc->match.fwnode = fwnode_handle_get(fwnode); - ret = __v4l2_async_nf_add_connection(notifier, asc); - if (ret) { - fwnode_handle_put(fwnode); - kfree(asc); - return ERR_PTR(ret); - } + __v4l2_async_nf_add_connection(notifier, asc); return asc; } @@ -731,21 +729,17 @@ __v4l2_async_nf_add_i2c(struct v4l2_async_notifier *notifier, int adapter_id, unsigned short address, unsigned int asc_struct_size) { struct v4l2_async_connection *asc; - int ret; asc = kzalloc(asc_struct_size, GFP_KERNEL); if (!asc) return ERR_PTR(-ENOMEM); + asc->notifier = notifier; asc->match.type = V4L2_ASYNC_MATCH_TYPE_I2C; asc->match.i2c.adapter_id = adapter_id; asc->match.i2c.address = address; - ret = __v4l2_async_nf_add_connection(notifier, asc); - if (ret) { - kfree(asc); - return ERR_PTR(ret); - } + __v4l2_async_nf_add_connection(notifier, asc); return asc; } @@ -754,7 +748,11 @@ EXPORT_SYMBOL_GPL(__v4l2_async_nf_add_i2c); struct v4l2_async_connection * v4l2_async_connection_unique(struct v4l2_subdev *sd) { - return sd->asd; + if (!list_is_singular(&sd->asc_list)) + return NULL; + + return list_first_entry(&sd->asc_list, + struct v4l2_async_connection, asc_subdev_entry); } EXPORT_SYMBOL_GPL(v4l2_async_connection_unique); @@ -762,8 +760,11 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd) { struct v4l2_async_notifier *subdev_notifier; struct v4l2_async_notifier *notifier; + struct v4l2_async_connection *asc; int ret; + INIT_LIST_HEAD(&sd->asc_list); + /* * No reference taken. The reference is held by the device (struct * v4l2_subdev.dev), and async sub-device does not exist independently @@ -786,7 +787,6 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd) list_for_each_entry(notifier, ¬ifier_list, notifier_entry) { struct v4l2_device *v4l2_dev = v4l2_async_nf_find_v4l2_dev(notifier); - struct v4l2_async_connection *asc; if (!v4l2_dev) continue; @@ -823,11 +823,8 @@ err_unbind: if (subdev_notifier) v4l2_async_nf_unbind_all_subdevs(subdev_notifier); - if (sd->asd) { - v4l2_async_nf_call_unbind(notifier, sd, sd->asd); - sd->asd->sd = NULL; - } - v4l2_async_cleanup(sd); + if (asc) + v4l2_async_unbind_subdev_one(notifier, asc); mutex_unlock(&list_lock); @@ -837,6 +834,8 @@ EXPORT_SYMBOL(v4l2_async_register_subdev); void v4l2_async_unregister_subdev(struct v4l2_subdev *sd) { + struct v4l2_async_connection *asc, *asc_tmp; + if (!sd->async_list.next) return; @@ -849,15 +848,19 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd) kfree(sd->subdev_notifier); sd->subdev_notifier = NULL; - if (sd->asd) { - struct v4l2_async_notifier *notifier = sd->notifier; + if (sd->asc_list.next) { + list_for_each_entry_safe(asc, asc_tmp, &sd->asc_list, + asc_subdev_entry) { + list_move(&asc->asc_entry, + &asc->notifier->waiting_list); - list_move(&sd->asd->asc_entry, ¬ifier->waiting_list); - v4l2_async_nf_call_unbind(notifier, sd, sd->asd); - sd->asd->sd = NULL; + v4l2_async_unbind_subdev_one(asc->notifier, asc); + list_del(&asc->asc_subdev_entry); + } } - v4l2_async_cleanup(sd); + list_del(&sd->async_list); + sd->async_list.next = NULL; mutex_unlock(&list_lock); } diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h index 5bc2efe..8670b8e 100644 --- a/include/media/v4l2-async.h +++ b/include/media/v4l2-async.h @@ -62,27 +62,32 @@ struct v4l2_async_match_desc { }; /** - * struct v4l2_async_connection - connection descriptor, as known to a bridge + * struct v4l2_async_connection - sub-device connection descriptor, as known to + * a bridge * * @match: struct of match type and per-bus type matching data sets + * @notifier: the async notifier the connection is related to * @asc_entry: used to add struct v4l2_async_connection objects to the * notifier @waiting_list or @done_list + * @asc_subdev_entry: entry in struct v4l2_async_subdev.asc_list list * @sd: the related sub-device * - * When this struct is used as a member in a driver specific struct, - * the driver specific struct shall contain the &struct - * v4l2_async_connection as its first member. + * When this struct is used as a member in a driver specific struct, the driver + * specific struct shall contain the &struct v4l2_async_connection as its first + * member. */ struct v4l2_async_connection { struct v4l2_async_match_desc match; + struct v4l2_async_notifier *notifier; struct list_head asc_entry; + struct list_head asc_subdev_entry; struct v4l2_subdev *sd; }; /** * struct v4l2_async_notifier_operations - Asynchronous V4L2 notifier operations - * @bound: a subdevice driver has successfully probed one of the subdevices - * @complete: All subdevices have been probed successfully. The complete + * @bound: a sub-device has been bound by the given connection + * @complete: All connections have been bound successfully. The complete * callback is only executed for the root notifier. * @unbind: a subdevice is leaving * @destroy: the asc is about to be freed diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 212d7f1..a8078ae 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1022,10 +1022,10 @@ struct v4l2_subdev_platform_data { * either dev->of_node->fwnode or dev->fwnode (whichever is non-NULL). * @async_list: Links this subdev to a global subdev_list or * @notifier->done_list list. - * @asd: Pointer to respective &struct v4l2_async_connection. - * @notifier: Pointer to the managing notifier. * @subdev_notifier: A sub-device notifier implicitly registered for the sub- * device using v4l2_async_register_subdev_sensor(). + * @asc_list: Async connection list, of &struct + * v4l2_async_connection.subdev_entry. * @pdata: common part of subdevice platform data * @state_lock: A pointer to a lock used for all the subdev's states, set by the * driver. This is optional. If NULL, each state instance will get @@ -1065,9 +1065,8 @@ struct v4l2_subdev { struct device *dev; struct fwnode_handle *fwnode; struct list_head async_list; - struct v4l2_async_connection *asd; - struct v4l2_async_notifier *notifier; struct v4l2_async_notifier *subdev_notifier; + struct list_head asc_list; struct v4l2_subdev_platform_data *pdata; struct mutex *state_lock; -- 2.7.4