Hi,
I'm repeating myself, but this seems the proper place to post this.
This patch causes a regression with the logitech c930e.
Please see comments below:
2013/11/10 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>:
> Hi Pawel,
>
> Thank you for the patch.
>
> On Friday 30 August 2013 11:17:02 Pawel Osciak wrote:
>> This adds support for pipelines that fork into branches consisting of more
>> than one entity, creating a chain for each fork and putting common entities
>> on all chains that share them.
>>
>> This requires us to share the ctrl_mutex across forked chains. Whenever we
>> discover a shared part of a chain, we assign the pointer to an existing
>> mutex to the sharing chain instead of creating a new one.
>>
>> The "forward scan" is not needed anymore, as after scanning back from OTs,
>> we go over all entities which are not on a path from an OT and accept
>> single-XU branches, adding them to the existing chains.
>>
>> Also extract control locking into __uvc_ctrl_{lock,unlock} functions.
>
> This is one core piece of the UVC 1.5 rework, and I have mixed feelings about
> it.
>
> Adding entities to multiple overlapping chains somehow doesn't feel right.
> What would you think about using a pipeline object instead, which would store
> all entities in the pipeline ?
>
> The driver currently iterates over all entities in a chain for several
> purposes:
>
> - registering streaming terminals as video nodes (uvc_driver.c)
> - registering MC entities (uvc_entity.c)
> - enumerating inputs (uvc_v4l2.c)
> - finding controls and extension units (uvc_ctrl.c)
>
> The first two uses could easily be replaced by iterations over the whole
> pipeline. Input enumeration would probably require a bit of custom code
> anyway, so we would be left with controls.
>
> At first sight it would make sense to expose on a video node only the controls
> that can be reached from that video node moving up the pipeline (relatively to
> the video flow). However, this might break existing applications, as the
> driver currently also includes controls reacheable by forward scans of side
> branches. We thus need to carefully think about what controls to include, and
> we need to take into account output video nodes corresponding to input
> streaming terminals.
>
>> Signed-off-by: Pawel Osciak <posciak@xxxxxxxxxxxx>
>> ---
>> drivers/media/usb/uvc/uvc_ctrl.c | 70 ++++++++-----
>> drivers/media/usb/uvc/uvc_driver.c | 210 +++++++++++++++++++--------------
>> drivers/media/usb/uvc/uvc_entity.c | 15 ++-
>> drivers/media/usb/uvc/uvc_v4l2.c | 9 +-
>> drivers/media/usb/uvc/uvcvideo.h | 20 +++-
>> 5 files changed, 199 insertions(+), 125 deletions(-)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c
>> b/drivers/media/usb/uvc/uvc_ctrl.c index a2f4501..ba159a4 100644
>> --- a/drivers/media/usb/uvc/uvc_ctrl.c
>> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
>> @@ -841,6 +841,7 @@ static struct uvc_control *uvc_find_control(struct
>> uvc_video_chain *chain, {
>> struct uvc_control *ctrl = NULL;
>> struct uvc_entity *entity;
>> + struct uvc_chain_entry *entry;
>> int next = v4l2_id & V4L2_CTRL_FLAG_NEXT_CTRL;
>>
>> *mapping = NULL;
>> @@ -849,7 +850,8 @@ static struct uvc_control *uvc_find_control(struct
>> uvc_video_chain *chain, v4l2_id &= V4L2_CTRL_ID_MASK;
>>
>> /* Find the control. */
>> - list_for_each_entry(entity, &chain->entities, chain) {
>> + list_for_each_entry(entry, &chain->entities, chain_entry) {
>> + entity = entry->entity;
>> __uvc_find_control(entity, v4l2_id, mapping, &ctrl, next);
>> if (ctrl && !next)
>> return ctrl;
>> @@ -1048,6 +1050,16 @@ static int __uvc_query_v4l2_ctrl(struct
>> uvc_video_chain *chain, return 0;
>> }
>>
>> +int __uvc_ctrl_lock(struct uvc_video_chain *chain)
>> +{
>> + return mutex_lock_interruptible(&chain->pipeline->ctrl_mutex) ?
>> + -ERESTARTSYS : 0;
>> +}
>> +void __uvc_ctrl_unlock(struct uvc_video_chain *chain)
>> +{
>> + mutex_unlock(&chain->pipeline->ctrl_mutex);
>> +}
>> +
>> int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
>> struct v4l2_queryctrl *v4l2_ctrl)
>> {
>> @@ -1055,9 +1067,9 @@ int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
>> struct uvc_control_mapping *mapping;
>> int ret;
>>
>> - ret = mutex_lock_interruptible(&chain->ctrl_mutex);
>> + ret = __uvc_ctrl_lock(chain);
>> if (ret < 0)
>> - return -ERESTARTSYS;
>> + return ret;
>>
>> ctrl = uvc_find_control(chain, v4l2_ctrl->id, &mapping);
>> if (ctrl == NULL) {
>> @@ -1067,7 +1079,7 @@ int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
>>
>> ret = __uvc_query_v4l2_ctrl(chain, ctrl, mapping, v4l2_ctrl);
>> done:
>> - mutex_unlock(&chain->ctrl_mutex);
>> + __uvc_ctrl_unlock(chain);
>> return ret;
>> }
>>
>> @@ -1094,9 +1106,9 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
>> query_menu->id = id;
>> query_menu->index = index;
>>
>> - ret = mutex_lock_interruptible(&chain->ctrl_mutex);
>> + ret = __uvc_ctrl_lock(chain);
>> if (ret < 0)
>> - return -ERESTARTSYS;
>> + return ret;
>>
>> ctrl = uvc_find_control(chain, query_menu->id, &mapping);
>> if (ctrl == NULL || mapping->v4l2_type != V4L2_CTRL_TYPE_MENU) {
>> @@ -1132,7 +1144,7 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
>> strlcpy(query_menu->name, menu_info->name, sizeof query_menu->name);
>>
>> done:
>> - mutex_unlock(&chain->ctrl_mutex);
>> + __uvc_ctrl_unlock(chain);
>> return ret;
>> }
>>
>> @@ -1257,9 +1269,9 @@ static int uvc_ctrl_add_event(struct
>> v4l2_subscribed_event *sev, unsigned elems) struct uvc_control *ctrl;
>> int ret;
>>
>> - ret = mutex_lock_interruptible(&handle->chain->ctrl_mutex);
>> + ret = __uvc_ctrl_lock(handle->chain);
>> if (ret < 0)
>> - return -ERESTARTSYS;
>> + return ret;
>>
>> ctrl = uvc_find_control(handle->chain, sev->id, &mapping);
>> if (ctrl == NULL) {
>> @@ -1285,7 +1297,7 @@ static int uvc_ctrl_add_event(struct
>> v4l2_subscribed_event *sev, unsigned elems) }
>>
>> done:
>> - mutex_unlock(&handle->chain->ctrl_mutex);
>> + __uvc_ctrl_unlock(handle->chain);
>> return ret;
>> }
>>
>> @@ -1293,9 +1305,9 @@ static void uvc_ctrl_del_event(struct
>> v4l2_subscribed_event *sev) {
>> struct uvc_fh *handle = container_of(sev->fh, struct uvc_fh, vfh);
>>
>> - mutex_lock(&handle->chain->ctrl_mutex);
>> + __uvc_ctrl_lock(handle->chain);
>> list_del(&sev->node);
>> - mutex_unlock(&handle->chain->ctrl_mutex);
>> + __uvc_ctrl_unlock(handle->chain);
>> }
>>
>> const struct v4l2_subscribed_event_ops uvc_ctrl_sub_ev_ops = {
>> @@ -1331,7 +1343,7 @@ const struct v4l2_subscribed_event_ops
>> uvc_ctrl_sub_ev_ops = { */
>> int uvc_ctrl_begin(struct uvc_video_chain *chain)
>> {
>> - return mutex_lock_interruptible(&chain->ctrl_mutex) ? -ERESTARTSYS : 0;
>> + return __uvc_ctrl_lock(chain);
>> }
>>
>> static int uvc_ctrl_commit_entity(struct uvc_device *dev,
>> @@ -1390,10 +1402,12 @@ int __uvc_ctrl_commit(struct uvc_fh *handle, int
>> rollback, {
>> struct uvc_video_chain *chain = handle->chain;
>> struct uvc_entity *entity;
>> + struct uvc_chain_entry *entry;
>> int ret = 0;
>>
>> /* Find the control. */
>> - list_for_each_entry(entity, &chain->entities, chain) {
>> + list_for_each_entry(entry, &chain->entities, chain_entry) {
>> + entity = entry->entity;
>> ret = uvc_ctrl_commit_entity(chain->dev, entity, rollback);
>> if (ret < 0)
>> goto done;
>> @@ -1402,7 +1416,7 @@ int __uvc_ctrl_commit(struct uvc_fh *handle, int
>> rollback, if (!rollback)
>> uvc_ctrl_send_events(handle, xctrls, xctrls_count);
>> done:
>> - mutex_unlock(&chain->ctrl_mutex);
>> + __uvc_ctrl_unlock(chain);
>> return ret;
>> }
>>
>> @@ -1667,7 +1681,8 @@ static int uvc_ctrl_init_xu_ctrl(struct uvc_device
>> *dev, int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
>> struct uvc_xu_control_query *xqry)
>> {
>> - struct uvc_entity *entity;
>> + struct uvc_entity *entity = NULL;
>> + struct uvc_chain_entry *entry;
>> struct uvc_control *ctrl;
>> unsigned int i, found = 0;
>> __u32 reqflags;
>> @@ -1676,13 +1691,14 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
>> int ret;
>>
>> /* Find the extension unit. */
>> - list_for_each_entry(entity, &chain->entities, chain) {
>> + list_for_each_entry(entry, &chain->entities, chain_entry) {
>> + entity = entry->entity;
>> if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT &&
>> entity->id == xqry->unit)
>> break;
>> }
>>
>> - if (entity->id != xqry->unit) {
>> + if (!entity || entity->id != xqry->unit) {
>> uvc_trace(UVC_TRACE_CONTROL, "Extension unit %u not found.\n",
>> xqry->unit);
>> return -ENOENT;
>> @@ -1703,8 +1719,9 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
>> return -ENOENT;
>> }
>>
>> - if (mutex_lock_interruptible(&chain->ctrl_mutex))
>> - return -ERESTARTSYS;
>> + ret = __uvc_ctrl_lock(chain);
>> + if (ret < 0)
>> + return ret;
>>
>> ret = uvc_ctrl_init_xu_ctrl(chain->dev, ctrl);
>> if (ret < 0) {
>> @@ -1778,7 +1795,7 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
>> ret = -EFAULT;
>> done:
>> kfree(data);
>> - mutex_unlock(&chain->ctrl_mutex);
>> + __uvc_ctrl_unlock(chain);
>> return ret;
>> }
>>
>> @@ -1904,6 +1921,7 @@ int uvc_ctrl_add_mapping(struct uvc_video_chain
>> *chain, const struct uvc_control_mapping *mapping)
>> {
>> struct uvc_device *dev = chain->dev;
>> + struct uvc_chain_entry *entry;
>> struct uvc_control_mapping *map;
>> struct uvc_entity *entity;
>> struct uvc_control *ctrl;
>> @@ -1918,8 +1936,9 @@ int uvc_ctrl_add_mapping(struct uvc_video_chain
>> *chain, }
>>
>> /* Search for the matching (GUID/CS) control on the current chain */
>> - list_for_each_entry(entity, &chain->entities, chain) {
>> + list_for_each_entry(entry, &chain->entities, chain_entry) {
>> unsigned int i;
>> + entity = entry->entity;
>>
>> if (UVC_ENTITY_TYPE(entity) != UVC_VC_EXTENSION_UNIT ||
>> !uvc_entity_match_guid(entity, mapping->entity))
>> @@ -1939,8 +1958,9 @@ int uvc_ctrl_add_mapping(struct uvc_video_chain
>> *chain, if (!found)
>> return -ENOENT;
>>
>> - if (mutex_lock_interruptible(&chain->ctrl_mutex))
>> - return -ERESTARTSYS;
>> + ret = __uvc_ctrl_lock(chain);
>> + if (ret < 0)
>> + return ret;
>>
>> /* Perform delayed initialization of XU controls */
>> ret = uvc_ctrl_init_xu_ctrl(dev, ctrl);
>> @@ -1974,7 +1994,7 @@ int uvc_ctrl_add_mapping(struct uvc_video_chain
>> *chain, atomic_dec(&dev->nmappings);
>>
>> done:
>> - mutex_unlock(&chain->ctrl_mutex);
>> + __uvc_ctrl_unlock(chain);
>> return ret;
>> }
>>
>> diff --git a/drivers/media/usb/uvc/uvc_driver.c
>> b/drivers/media/usb/uvc/uvc_driver.c index 81695d4..d7ff707 100644
>> --- a/drivers/media/usb/uvc/uvc_driver.c
>> +++ b/drivers/media/usb/uvc/uvc_driver.c
>> @@ -1215,6 +1215,36 @@ next_descriptor:
>> * UVC device scan
>> */
>>
>> +static int uvc_add_chain_entry(struct uvc_video_chain *chain,
>> + struct uvc_entity *entity)
>> +{
>> + struct uvc_chain_entry *chain_entry;
>> +
>> + chain_entry = kzalloc(sizeof(struct uvc_chain_entry), GFP_KERNEL);
>> + if (!chain_entry)
>> + return -ENOMEM;
>> +
>> + chain_entry->entity = entity;
>> + list_add_tail(&chain_entry->chain_entry, &chain->entities);
>> + if (!entity->chain)
>> + entity->chain = chain;
>> +
>> + return 0;
>> +}
>> +
>> +static void uvc_delete_chain(struct uvc_video_chain *chain)
>> +{
>> + struct list_head *p, *n;
>> + struct uvc_chain_entry *entry;
>> +
>> + list_for_each_safe(p, n, &chain->entities) {
>> + entry = list_entry(p, struct uvc_chain_entry, chain_entry);
>> + kfree(entry);
>> + }
>> +
>> + kfree(chain);
>> +}
>> +
>> /*
>> * Scan the UVC descriptors to locate a chain starting at an Output
>> Terminal * and containing the following units:
>> @@ -1320,72 +1350,7 @@ static int uvc_scan_chain_entity(struct
>> uvc_video_chain *chain, return -1;
>> }
>>
>> - list_add_tail(&entity->chain, &chain->entities);
>> - return 0;
>> -}
>> -
>> -static int uvc_scan_chain_forward(struct uvc_video_chain *chain,
>> - struct uvc_entity *entity, struct uvc_entity *prev)
>> -{
>> - struct uvc_entity *forward;
>> - int found;
>> -
>> - /* Forward scan */
>> - forward = NULL;
>> - found = 0;
>> -
>> - while (1) {
>> - forward = uvc_entity_by_reference(chain->dev, entity->id,
>> - forward);
>> - if (forward == NULL)
>> - break;
>> - if (forward == prev)
>> - continue;
>> -
>> - switch (UVC_ENTITY_TYPE(forward)) {
>> - case UVC_VC_EXTENSION_UNIT:
>> - if (forward->bNrInPins != 1) {
>> - uvc_trace(UVC_TRACE_DESCR, "Extension unit %d "
>> - "has more than 1 input pin.\n",
>> - entity->id);
>> - return -EINVAL;
>> - }
>> -
>> - list_add_tail(&forward->chain, &chain->entities);
>> - if (uvc_trace_param & UVC_TRACE_PROBE) {
>> - if (!found)
>> - printk(" (->");
>> -
>> - printk(" XU %d", forward->id);
>> - found = 1;
>> - }
>> - break;
>> -
>> - case UVC_OTT_VENDOR_SPECIFIC:
>> - case UVC_OTT_DISPLAY:
>> - case UVC_OTT_MEDIA_TRANSPORT_OUTPUT:
>> - case UVC_TT_STREAMING:
>> - if (UVC_ENTITY_IS_ITERM(forward)) {
>> - uvc_trace(UVC_TRACE_DESCR, "Unsupported input "
>> - "terminal %u.\n", forward->id);
>> - return -EINVAL;
>> - }
>> -
>> - list_add_tail(&forward->chain, &chain->entities);
>> - if (uvc_trace_param & UVC_TRACE_PROBE) {
>> - if (!found)
>> - printk(" (->");
>> -
>> - printk(" OT %d", forward->id);
>> - found = 1;
>> - }
>> - break;
>> - }
>> - }
>> - if (found)
>> - printk(")");
>> -
>> - return 0;
>> + return uvc_add_chain_entry(chain, entity);
>> }
>>
>> static int uvc_scan_chain_backward(struct uvc_video_chain *chain,
>> @@ -1394,6 +1359,7 @@ static int uvc_scan_chain_backward(struct
>> uvc_video_chain *chain, struct uvc_entity *entity = *_entity;
>> struct uvc_entity *term;
>> int id = -EINVAL, i;
>> + int ret;
>>
>> switch (UVC_ENTITY_TYPE(entity)) {
>> case UVC_VC_EXTENSION_UNIT:
>> @@ -1425,8 +1391,9 @@ static int uvc_scan_chain_backward(struct
>> uvc_video_chain *chain, if (uvc_trace_param & UVC_TRACE_PROBE)
>> printk(" %d", term->id);
>>
>> - list_add_tail(&term->chain, &chain->entities);
>> - uvc_scan_chain_forward(chain, term, entity);
>> + ret = uvc_add_chain_entry(chain, term);
>> + if (ret)
>> + return ret;
>> }
>>
>> if (uvc_trace_param & UVC_TRACE_PROBE)
>> @@ -1473,23 +1440,23 @@ static int uvc_scan_chain(struct uvc_video_chain
>> *chain, prev = NULL;
>>
>> while (entity != NULL) {
>> - /* Entity must not be part of an existing chain */
>> - if (entity->chain.next || entity->chain.prev) {
>> - uvc_trace(UVC_TRACE_DESCR, "Found reference to "
>> - "entity %d already in chain.\n", entity->id);
>> + if (entity->chain == chain) {
>> + uvc_trace(UVC_TRACE_DESCR, "Found a cycle in the "
>> + "chain");
>> return -EINVAL;
>> }
>>
>> + /* If this entity is a part of an existing chain, the
>> + * current chain belongs to the same pipeline.
>> + */
>> + if (entity->chain)
>> + chain->pipeline = entity->chain->pipeline;
>> +
>> /* Process entity */
>> if (uvc_scan_chain_entity(chain, entity) < 0)
>> return -EINVAL;
>>
>> - /* Forward scan */
>> - if (uvc_scan_chain_forward(chain, entity, prev) < 0)
>> - return -EINVAL;
>> -
>> /* Backward scan */
>> - prev = entity;
>> if (uvc_scan_chain_backward(chain, &entity) < 0)
>> return -EINVAL;
>> }
>> @@ -1501,10 +1468,12 @@ static unsigned int uvc_print_terms(struct list_head
>> *terms, u16 dir, char *buffer)
>> {
>> struct uvc_entity *term;
>> + struct uvc_chain_entry *entry;
>> unsigned int nterms = 0;
>> char *p = buffer;
>>
>> - list_for_each_entry(term, terms, chain) {
>> + list_for_each_entry(entry, terms, chain_entry) {
>> + term = entry->entity;
>> if (!UVC_ENTITY_IS_TERM(term) ||
>> UVC_TERM_DIRECTION(term) != dir)
>> continue;
>> @@ -1541,39 +1510,58 @@ static const char *uvc_print_chain(struct
>> uvc_video_chain *chain) static int uvc_scan_device(struct uvc_device *dev)
>> {
>> struct uvc_video_chain *chain;
>> - struct uvc_entity *term;
>> + struct uvc_entity *entity, *source;
>> + int ret;
>>
>> - list_for_each_entry(term, &dev->entities, list) {
>> - if (!UVC_ENTITY_IS_OTERM(term))
>> + list_for_each_entry(entity, &dev->entities, list) {
>> + if (!UVC_ENTITY_IS_OTERM(entity))
>> continue;
>>
>> - /* If the terminal is already included in a chain, skip it.
>> - * This can happen for chains that have multiple output
>> - * terminals, where all output terminals beside the first one
>> - * will be inserted in the chain in forward scans.
>> + /* Allow single-unit branches of Output Terminals to reside
>> + * on the existing chains.
>> */
>> - if (term->chain.next || term->chain.prev)
>> - continue;
>> + source = uvc_entity_by_id(dev, entity->baSourceID[0]);
>> + if (entity == NULL) {
>> + uvc_trace(UVC_TRACE_DESCR, "Found reference to "
>> + "unknown entity %d.\n", entity->baSourceID[0]);
>> + return -EINVAL;
>> + }
>>
>> chain = kzalloc(sizeof(*chain), GFP_KERNEL);
>> if (chain == NULL)
>> return -ENOMEM;
>>
>> INIT_LIST_HEAD(&chain->entities);
>> - mutex_init(&chain->ctrl_mutex);
>> chain->dev = dev;
>> v4l2_prio_init(&chain->prio);
>>
>> - term->flags |= UVC_ENTITY_FLAG_DEFAULT;
>> + entity->flags |= UVC_ENTITY_FLAG_DEFAULT;
>>
>> - if (uvc_scan_chain(chain, term) < 0) {
>> - kfree(chain);
>> + if (uvc_scan_chain(chain, entity) < 0) {
>> + uvc_delete_chain(chain);
>> continue;
>> }
>>
>> uvc_trace(UVC_TRACE_PROBE, "Found a valid video chain (%s).\n",
>> uvc_print_chain(chain));
>>
>> + /*
>> + * If none of the entities are shared, allocate a new pipeline.
>> + * Otherwise, the shared pipeline is already set up.
>> + */
>> + if (!chain->pipeline) {
>> + chain->pipeline = kzalloc(sizeof(*chain->pipeline),
>> + GFP_KERNEL);
>> + if (!chain->pipeline) {
>> + uvc_delete_chain(chain);
>> + return -ENOMEM;
>> + }
>> + mutex_init(&chain->pipeline->ctrl_mutex);
>> + atomic_set(&chain->pipeline->num_chains, 1);
>> + } else {
>> + atomic_inc(&chain->pipeline->num_chains);
>> + }
>> +
>> list_add_tail(&chain->list, &dev->chains);
>> }
>>
>> @@ -1582,6 +1570,38 @@ static int uvc_scan_device(struct uvc_device *dev)
>> return -1;
>> }
>>
>> + /* Find branches with no OTERMs (if any) by looking for entities not
>> + * on any chain. Accept only branches with a single Extension Unit.
>> + */
>> + list_for_each_entry(entity, &dev->entities, list) {
>> + if (entity->chain)
>> + continue;
>> +
>> + if (UVC_ENTITY_TYPE(entity) != UVC_VC_EXTENSION_UNIT
>> + || entity->bNrInPins != 1
>> + || uvc_entity_by_reference(dev, entity->id, NULL)) {
>> + uvc_printk(KERN_INFO, "Found an invalid branch "
>> + "starting at entity id %d.\n", entity->id);
>> + return -1;
>> + }
This fails for the c930e, to be more exact 'uvc_entity_by_reference'
fails for unit 12 (the h264 extension unit).
This prevents the driver from creating the video node, making the
camera unusable.
As a simple test I commented this line ( /* ||
uvc_entity_by_reference(dev, entity->id, NULL) */ ), the video node is
created and the camera works fine.
>> +
>> + /* Single-unit XU branch. */
>> + source = uvc_entity_by_id(dev, entity->baSourceID[0]);
>> + if (source == NULL) {
>> + uvc_trace(UVC_TRACE_DESCR, "Found reference to "
>> + "unknown entity %d.\n", entity->baSourceID[0]);
>> + return -EINVAL;
>> + }
>> + if (!source->chain)
>> + continue;
>> +
>> + ret = uvc_add_chain_entry(source->chain, entity);
>> + if (ret)
>> + return ret;
>> + uvc_trace(UVC_TRACE_DESCR, "XU %d <- (%d)\n",
>> + entity->id, source->id);
>> + }
>> +
>> return 0;
>> }
>>
>> @@ -1619,7 +1639,9 @@ static void uvc_delete(struct uvc_device *dev)
>> list_for_each_safe(p, n, &dev->chains) {
>> struct uvc_video_chain *chain;
>> chain = list_entry(p, struct uvc_video_chain, list);
>> - kfree(chain);
>> + if (atomic_dec_and_test(&chain->pipeline->num_chains))
>> + kfree(chain->pipeline);
>> + uvc_delete_chain(chain);
>> }
>>
>> list_for_each_safe(p, n, &dev->entities) {
>> @@ -1763,9 +1785,11 @@ static int uvc_register_terms(struct uvc_device *dev,
>> {
>> struct uvc_streaming *stream;
>> struct uvc_entity *term;
>> + struct uvc_chain_entry *entry;
>> int ret;
>>
>> - list_for_each_entry(term, &chain->entities, chain) {
>> + list_for_each_entry(entry, &chain->entities, chain_entry) {
>> + term = entry->entity;
>> if (UVC_ENTITY_TYPE(term) != UVC_TT_STREAMING)
>> continue;
>>
>> diff --git a/drivers/media/usb/uvc/uvc_entity.c
>> b/drivers/media/usb/uvc/uvc_entity.c index dc56a59..657f49a 100644
>> --- a/drivers/media/usb/uvc/uvc_entity.c
>> +++ b/drivers/media/usb/uvc/uvc_entity.c
>> @@ -104,9 +104,14 @@ static int uvc_mc_init_entity(struct uvc_entity
>> *entity) int uvc_mc_register_entities(struct uvc_video_chain *chain)
>> {
>> struct uvc_entity *entity;
>> + struct uvc_chain_entry *entry;
>> int ret;
>>
>> - list_for_each_entry(entity, &chain->entities, chain) {
>> + list_for_each_entry(entry, &chain->entities, chain_entry) {
>> + entity = entry->entity;
>> + if (entity->registered)
>> + continue;
>> +
>> ret = uvc_mc_init_entity(entity);
>> if (ret < 0) {
>> uvc_printk(KERN_INFO, "Failed to initialize entity for "
>> @@ -115,13 +120,19 @@ int uvc_mc_register_entities(struct uvc_video_chain
>> *chain) }
>> }
>>
>> - list_for_each_entry(entity, &chain->entities, chain) {
>> + list_for_each_entry(entry, &chain->entities, chain_entry) {
>> + entity = entry->entity;
>> + if (entity->registered)
>> + continue;
>> +
>> ret = uvc_mc_register_entity(chain, entity);
>> if (ret < 0) {
>> uvc_printk(KERN_INFO, "Failed to register entity for "
>> "entity %u\n", entity->id);
>> return ret;
>> }
>> +
>> + entity->registered = true;
>> }
>>
>> return 0;
>> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
>> b/drivers/media/usb/uvc/uvc_v4l2.c index 3afff92..a899159 100644
>> --- a/drivers/media/usb/uvc/uvc_v4l2.c
>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
>> @@ -713,6 +713,7 @@ static long uvc_v4l2_do_ioctl(struct file *file,
>> unsigned int cmd, void *arg) const struct uvc_entity *selector =
>> chain->selector;
>> struct v4l2_input *input = arg;
>> struct uvc_entity *iterm = NULL;
>> + struct uvc_chain_entry *entry;
>> u32 index = input->index;
>> int pin = 0;
>>
>> @@ -720,14 +721,18 @@ static long uvc_v4l2_do_ioctl(struct file *file,
>> unsigned int cmd, void *arg) (chain->dev->quirks &
>> UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
>> if (index != 0)
>> return -EINVAL;
>> - list_for_each_entry(iterm, &chain->entities, chain) {
>> + list_for_each_entry(entry, &chain->entities,
>> + chain_entry) {
>> + iterm = entry->entity;
>> if (UVC_ENTITY_IS_ITERM(iterm))
>> break;
>> }
>> pin = iterm->id;
>> } else if (index < selector->bNrInPins) {
>> pin = selector->baSourceID[index];
>> - list_for_each_entry(iterm, &chain->entities, chain) {
>> + list_for_each_entry(entry, &chain->entities,
>> + chain_entry) {
>> + iterm = entry->entity;
>> if (!UVC_ENTITY_IS_ITERM(iterm))
>> continue;
>> if (iterm->id == pin)
>> diff --git a/drivers/media/usb/uvc/uvcvideo.h
>> b/drivers/media/usb/uvc/uvcvideo.h index 75e0153..731b378 100644
>> --- a/drivers/media/usb/uvc/uvcvideo.h
>> +++ b/drivers/media/usb/uvc/uvcvideo.h
>> @@ -229,8 +229,8 @@ struct uvc_format_desc {
>>
>> struct uvc_entity {
>> struct list_head list; /* Entity as part of a UVC device. */
>> - struct list_head chain; /* Entity as part of a video device
>> - * chain. */
>> + struct uvc_video_chain *chain; /* Entity as a part of a video device
>> + chain. */
>> unsigned int flags;
>>
>> __u8 id;
>> @@ -243,6 +243,7 @@ struct uvc_entity {
>> unsigned int num_pads;
>> unsigned int num_links;
>> struct media_pad *pads;
>> + bool registered; /* True if already registered with MC */
>>
>> union {
>> struct {
>> @@ -289,6 +290,12 @@ struct uvc_entity {
>> struct uvc_control *controls;
>> };
>>
>> +struct uvc_chain_entry {
>> + struct list_head chain_entry;
>> + struct uvc_entity *entity;
>> + struct uvc_video_chain *chain;
>> +};
>> +
>> struct uvc_frame {
>> __u8 bFrameIndex;
>> __u8 bmCapabilities;
>> @@ -366,6 +373,12 @@ struct uvc_video_queue {
>> struct list_head irqqueue;
>> };
>>
>> +struct uvc_video_pipeline {
>> + struct mutex ctrl_mutex; /* Protects controls in all
>> + chains of this pipeline */
>> + atomic_t num_chains;
>> +};
>> +
>> struct uvc_video_chain {
>> struct uvc_device *dev;
>> struct list_head list;
>> @@ -374,7 +387,8 @@ struct uvc_video_chain {
>> struct uvc_entity *processing; /* Processing unit */
>> struct uvc_entity *selector; /* Selector unit */
>>
>> - struct mutex ctrl_mutex; /* Protects ctrl.info */
>> + struct uvc_video_pipeline *pipeline; /* Pipeline this chain
>> + belongs to */
>>
>> struct v4l2_prio_state prio; /* V4L2 priority state */
>> u32 caps; /* V4L2 chain-wide caps */
> --
> Regards,
>
> Laurent Pinchart
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Regards,
Paulo
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
