Em Wed, 03 Feb 2016 21:03:52 -0700 Shuah Khan shuahkh@osg.samsung.com escreveu:
Add enable_source and disable_source handlers. The enable source handler is called from v4l2-core, dvb-core, and ALSA drivers to check if the shared media source is free. The disable source handler is called to release the shared media source.
Signed-off-by: Shuah Khan shuahkh@osg.samsung.com
drivers/media/usb/au0828/au0828-core.c | 149 +++++++++++++++++++++++++++++++++ drivers/media/usb/au0828/au0828.h | 3 + 2 files changed, 152 insertions(+)
diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c index 4c90f28..fd2265c 100644 --- a/drivers/media/usb/au0828/au0828-core.c +++ b/drivers/media/usb/au0828/au0828-core.c @@ -282,6 +282,7 @@ static int au0828_create_media_graph(struct au0828_dev *dev) return -EINVAL;
if (tuner) {
dev->tuner = tuner;@@ -373,6 +374,150 @@ void au0828_media_graph_notify(struct media_entity *new, void *notify_data) #endif }
+static int au0828_enable_source(struct media_entity *entity,
struct media_pipeline *pipe)+{ +#ifdef CONFIG_MEDIA_CONTROLLER
- struct media_entity *source;
- struct media_entity *sink;
- struct media_link *link, *found_link = NULL;
- int ret = 0;
- struct media_device *mdev = entity->graph_obj.mdev;
- struct au0828_dev *dev;
- if (!mdev)
return -ENODEV;- /* for Audio and Video entities, source is the decoder */
- mutex_lock(&mdev->graph_mutex);
- dev = mdev->source_priv;
- if (!dev->tuner || !dev->decoder) {
ret = -ENODEV;goto end;- }
This is wrong. There are devices without tuner (capture devices) and without analog decoder (pure DVB devices).
In the case of pure DVB devices (e. g. no dev->decoder), it should just enable the DVB path.
In the case of devices without tuner, it should use the same logic needed to handle the S-Video/Composite connector inputs.
Btw, I'm not seeing how this logic would do the right thing if the user selects either S-Video or Composite connectors.
- /*
* For Audio and V4L2 entity, find the link to which decoder* is the sink. Look for an active link between decoder and* tuner, if one exists, nothing to do. If not, look for any* active links between tuner and any other entity. If one* exists, tuner is busy. If tuner is free, setup link and* start pipeline from source (tuner).* For DVB FE entity, the source for the link is the tuner.* Check if tuner is available and setup link and start* pipeline.- */
- if (entity->function != MEDIA_ENT_F_DTV_DEMOD)
sink = dev->decoder;- else
sink = entity;- /* Is an active link between sink and tuner */
- if (dev->active_link) {
if (dev->active_link->sink->entity == sink &&dev->active_link->source->entity == dev->tuner) {ret = 0;goto end;} else {ret = -EBUSY;goto end;}- }
- list_for_each_entry(link, &sink->links, list) {
/* Check sink, and source */if (link->sink->entity == sink &&link->source->entity == dev->tuner) {found_link = link;break;}- }
- if (!found_link) {
ret = -ENODEV;goto end;- }
- /* activate link between source and sink and start pipeline */
- source = found_link->source->entity;
- ret = __media_entity_setup_link(found_link, MEDIA_LNK_FL_ENABLED);
- if (ret) {
pr_err("Activate tuner link %s->%s. Error %d\n",source->name, sink->name, ret);goto end;- }
- ret = __media_entity_pipeline_start(entity, pipe);
- if (ret) {
pr_err("Start Pipeline: %s->%s Error %d\n",source->name, entity->name, ret);ret = __media_entity_setup_link(found_link, 0);pr_err("Deactive link Error %d\n", ret);goto end;- }
Hmm... isn't it to early to activate the pipeline here? My original guess is that, on the analog side, this should happen only at the stream on code. Wouldn't this break apps like mythTV?
- /*
* save active link and active link owner to avoid audio* deactivating video owned link from disable_source and* vice versa- */
- dev->active_link = found_link;
- dev->active_link_owner = entity;
+end:
- mutex_unlock(&mdev->graph_mutex);
- pr_debug("au0828_enable_source() end %s %d %d\n",
entity->name, entity->function, ret);- return ret;
+#endif
- return 0;
+}
+static void au0828_disable_source(struct media_entity *entity) +{ +#ifdef CONFIG_MEDIA_CONTROLLER
- struct media_entity *sink;
- int ret = 0;
- struct media_device *mdev = entity->graph_obj.mdev;
- struct au0828_dev *dev;
- if (!mdev)
return;- mutex_lock(&mdev->graph_mutex);
- dev = mdev->source_priv;
- if (!dev->tuner || !dev->decoder || !dev->active_link) {
ret = -ENODEV;goto end;- }
Same note as before.
- if (entity->function != MEDIA_ENT_F_DTV_DEMOD)
sink = dev->decoder;- else
sink = entity;- /* link is active - stop pipeline from source (tuner) */
- if (dev->active_link && dev->active_link->sink->entity == sink &&
dev->active_link->source->entity == dev->tuner) {/** prevent video from deactivating link when audio* has active pipeline*/if (dev->active_link_owner != entity)goto end;__media_entity_pipeline_stop(entity);ret = __media_entity_setup_link(dev->active_link, 0);if (ret)pr_err("Deactive link Error %d\n", ret);dev->active_link = NULL;dev->active_link_owner = NULL;- }
Most code here looks like the one at au0828_enable_source(). Wouldn't be simpler to merge those code and add a "bool enable" to the function parameters?
+end:
- mutex_unlock(&mdev->graph_mutex);
+#endif +}
static int au0828_media_device_register(struct au0828_dev *dev, struct usb_device *udev) { @@ -403,6 +548,10 @@ static int au0828_media_device_register(struct au0828_dev *dev, ret); return ret; }
- /* set enable_source */
- dev->media_dev->source_priv = (void *) dev;
- dev->media_dev->enable_source = au0828_enable_source;
- dev->media_dev->disable_source = au0828_disable_source;
#endif return 0; } diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h index 54379ec..a7c88a1 100644 --- a/drivers/media/usb/au0828/au0828.h +++ b/drivers/media/usb/au0828/au0828.h @@ -284,6 +284,9 @@ struct au0828_dev { struct media_entity input_ent[AU0828_MAX_INPUT]; struct media_pad input_pad[AU0828_MAX_INPUT]; struct media_entity_notify entity_notify;
- struct media_entity *tuner;
- struct media_link *active_link;
- struct media_entity *active_link_owner;
#endif };