This patch series updates ALSA driver, and au0828 core
driver to use Managed Media controller API and Media
Controller API to share media resource (tuner).
This Patch v2 series is based on linux_media master.
This work addresses Mauro and Takashi's comments.
Composite or S-Video connector is the input case
is still under discussion and isn't addressed in
this series.
Patches 3 and 4 need documentation updates and will
be sent later this week.
Shuah Khan (22):
uapi/media.h: Declare interface types for ALSA
media: Add ALSA Media Controller function entities
media: Media Controller register/unregister entity_notify API
media: Media Controller enable/disable source handler API
media: Media Controller export non locking __media_entity_setup_link()
media: Media Controller non-locking
__media_entity_pipeline_start/stop()
media: v4l-core add enable/disable source common interfaces
media: Move au8522_media_pads enum to au8522.h from au8522_priv.h
media: au8522 change to create MC pad for ALSA Audio Out
media: Change v4l-core to check if source is free
media: dvb-frontend invoke enable/disable_source handlers
media: au0828 video remove au0828_enable_analog_tuner()
media: au0828 video change to use v4l_enable_media_source()
media: au0828 change to use Managed Media Controller API
media: au0828 handle media_init and media_register window
media: au0828 create tuner to decoder link in disabled state
media: au0828 disable tuner to demod link
media: au0828 Use au8522_media_pads enum for pad defines
media: au0828-core register entity_notify hook
media: au0828 add enable, disable source handlers
sound/usb: Use Media Controller API to share media resources
media: Ensure media device unregister is done only once
drivers/media/dvb-core/dvb_frontend.c | 139 ++----------
drivers/media/dvb-core/dvb_frontend.h | 3 +
drivers/media/dvb-frontends/au8522.h | 8 +
drivers/media/dvb-frontends/au8522_decoder.c | 1 +
drivers/media/dvb-frontends/au8522_priv.h | 8 -
drivers/media/media-device.c | 73 +++++-
drivers/media/media-devnode.c | 15 +-
drivers/media/media-entity.c | 51 ++++-
drivers/media/usb/au0828/au0828-core.c | 272 +++++++++++++++++++++--
drivers/media/usb/au0828/au0828-video.c | 75 +------
drivers/media/usb/au0828/au0828.h | 4 +
drivers/media/v4l2-core/Makefile | 2 +-
drivers/media/v4l2-core/v4l2-fh.c | 2 +
drivers/media/v4l2-core/v4l2-ioctl.c | 30 +++
drivers/media/v4l2-core/v4l2-mc.c | 60 +++++
drivers/media/v4l2-core/videobuf2-core.c | 4 +
include/media/media-device.h | 44 ++++
include/media/media-devnode.h | 17 ++
include/media/media-entity.h | 12 +
include/media/v4l2-dev.h | 1 +
include/media/v4l2-mc.h | 52 +++++
include/uapi/linux/media.h | 33 +++
sound/usb/Kconfig | 4 +
sound/usb/Makefile | 2 +
sound/usb/card.c | 14 ++
sound/usb/card.h | 1 +
sound/usb/media.c | 319 +++++++++++++++++++++++++++
sound/usb/media.h | 72 ++++++
sound/usb/mixer.h | 1 +
sound/usb/pcm.c | 28 ++-
sound/usb/quirks-table.h | 1 +
sound/usb/stream.c | 2 +
sound/usb/usbaudio.h | 3 +
33 files changed, 1117 insertions(+), 236 deletions(-)
create mode 100644 drivers/media/v4l2-core/v4l2-mc.c
create mode 100644 sound/usb/media.c
create mode 100644 sound/usb/media.h
--
2.5.0
Declare the interface types to be used on alsa for
the new G_TOPOLOGY ioctl.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/media-entity.c | 16 ++++++++++++++++
include/uapi/linux/media.h | 22 ++++++++++++++++++++++
2 files changed, 38 insertions(+)
Add a new interfaces to be used by v4l-core to invoke enable
source and disable_source handlers in the media_device. The
enable_source helper function invokes the enable_source handler
to find media source entity connected to the entity and check
is it is available or busy. If source is available, link is
activated and pipeline is started. The disable_source helper
function invokes the disable_source handler to deactivate and
stop the pipeline.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/v4l2-core/Makefile | 2 +-
drivers/media/v4l2-core/v4l2-mc.c | 60 +++++++++++++++++++++++++++++++++++++++
include/media/v4l2-dev.h | 1 +
include/media/v4l2-mc.h | 52 +++++++++++++++++++++++++++++++++
4 files changed, 114 insertions(+), 1 deletion(-)
create mode 100644 drivers/media/v4l2-core/v4l2-mc.c
Add non-locking __media_entity_pipeline_start/stop() interfaces
to be called from code paths that hold the graph_mutex. For this
change, the media_entity_pipeline_start() routine is renamed to
__media_entity_pipeline_start() minus the graph_mutex lock and
unlock. media_entity_pipeline_start() now calls the non-locking
__media_entity_pipeline_start() holding the graph_lock. The stop
interface, media_entity_pipeline_stop() routine is renamed to
__media_entity_pipeline_stop() minus the graph_mutex lock and
unlock. media_entity_pipeline_stop() now calls the non-locking
__media_entity_pipeline_stop() holding the graph_lock.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/media-entity.c | 34 ++++++++++++++++++++++++----------
include/media/media-entity.h | 12 ++++++++++++
2 files changed, 36 insertions(+), 10 deletions(-)
Export __media_entity_setup_link() to be used from code paths
that hold the graph_mutex.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/media-entity.c | 1 +
1 file changed, 1 insertion(+)
Change au0828 to use Managed Media Controller API to
share media device and coordinate creating/deleting
the shared media device with the snd-usb-audio driver.
The shared media device is created as device resource
of the parent usb device of the two drivers.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/usb/au0828/au0828-core.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
Media device initialization and registration
steps are split. There is a window between
media device init and media device register
during usb probe.
au0828 bridge driver and snd-usb-audio could
try to initialize the media device, if they
simply checked, whether the device has been
registered. They also need to check whether
the device has been initialized.
Change the au0828-core to check if media device
is already initialized during initialization step
and check if media device is already registered
during the registration step.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/usb/au0828/au0828-core.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
@@ -227,7 +227,8 @@ static int au0828_media_device_init(struct au0828_dev *dev,if(!mdev)return-ENOMEM;-if(!media_devnode_is_registered(&mdev->devnode)){+/* check if media device is already initialized */+if(!mdev->dev){mdev->dev=&udev->dev;if(udev->product)
@@ -325,6 +326,27 @@ static int au0828_create_media_graph(struct au0828_dev *dev)return0;}+staticintau0828_media_device_register(structau0828_dev*dev,+structusb_device*udev)+{+#ifdef CONFIG_MEDIA_CONTROLLER+intret;++if(dev->media_dev&&+!media_devnode_is_registered(&dev->media_dev->devnode)){++/* register media device */+ret=media_device_register(dev->media_dev);+if(ret){+dev_err(&udev->dev,+"Media Device Register Error: %d\n",ret);+returnret;+}+}+#endif+return0;+}+staticintau0828_usb_probe(structusb_interface*interface,conststructusb_device_id*id){
@@ -452,7 +474,7 @@ static int au0828_usb_probe(struct usb_interface *interface,}#ifdef CONFIG_MEDIA_CONTROLLER-retval=media_device_register(dev->media_dev);+retval=au0828_media_device_register(dev,usbdev);#endifdone:
Create tuner to demod pad link in disabled state to
avoid disable step when tuner resource is requested
by dvb.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/usb/au0828/au0828-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
@@ -278,9 +278,9 @@ static int au0828_create_media_graph(struct au0828_dev *dev)return-EINVAL;if(tuner){+/* create tuner to decoder link in deactivated state */ret=media_create_pad_link(tuner,TUNER_PAD_OUTPUT,-decoder,0,-MEDIA_LNK_FL_ENABLED);+decoder,0,0);if(ret)returnret;}
media_device_unregister() checks if the media device
is registered or not as the first step. However, the
MEDIA_FLAG_REGISTERED bit doesn't get cleared until
the end leaving a large window for two drivers to
attempt media device unregister.
The above leads to general protection faults when
device is removed.
Fix the problem with two phase media device unregister.
Add a new interface media_devnode_start_unregister()
to clear the MEDIA_FLAG_REGISTERED bit. Change
media_device_unregister() call this interface to mark
the start of unregister. This will ensure that media
device unregister is done only once.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/media-device.c | 12 ++++++------
drivers/media/media-devnode.c | 15 ++++++++++-----
include/media/media-devnode.h | 17 +++++++++++++++++
3 files changed, 33 insertions(+), 11 deletions(-)
@@ -747,17 +747,17 @@ void media_device_unregister(struct media_device *mdev)structmedia_entity*next;structmedia_interface*intf,*tmp_intf;structmedia_entity_notify*notify,*nextp;+intret;if(mdev==NULL)return;-spin_lock(&mdev->lock);--/* Check if mdev was ever registered at all */-if(!media_devnode_is_registered(&mdev->devnode)){-spin_unlock(&mdev->lock);+/* Start unregister - continue if necessary */+ret=media_devnode_start_unregister(&mdev->devnode);+if(ret)return;-}++spin_lock(&mdev->lock);/* Remove all entities from the media device */list_for_each_entry_safe(entity,next,&mdev->entities,graph_obj.list)
@@ -272,15 +272,20 @@ error:returnret;}-voidmedia_devnode_unregister(structmedia_devnode*mdev)+int__must_checkmedia_devnode_start_unregister(structmedia_devnode*mdev){-/* Check if mdev was ever registered at all */-if(!media_devnode_is_registered(mdev))-return;-mutex_lock(&media_devnode_lock);+if(!media_devnode_is_registered(mdev)){+mutex_unlock(&media_devnode_lock);+return-EINVAL;+}clear_bit(MEDIA_FLAG_REGISTERED,&mdev->flags);mutex_unlock(&media_devnode_lock);+return0;+}++voidmedia_devnode_unregister(structmedia_devnode*mdev)+{device_unregister(&mdev->dev);}
Change ALSA driver to use Media Controller API to
share media resources with DVB and V4L2 drivers
on a AU0828 media device. Media Controller specific
initialization is done after sound card is registered.
ALSA creates Media interface and entity function graph
nodes for Control, Mixer, PCM Playback, and PCM Capture
devices.
snd_usb_hw_params() will call Media Controller enable
source handler interface to request the media resource.
If resource request is granted, it will release it from
snd_usb_hw_free(). If resource is busy, -EBUSY is returned.
Media specific cleanup is done in usb_audio_disconnect().
Signed-off-by: Shuah Khan <redacted>
---
sound/usb/Kconfig | 4 +
sound/usb/Makefile | 2 +
sound/usb/card.c | 14 +++
sound/usb/card.h | 1 +
sound/usb/media.c | 319 +++++++++++++++++++++++++++++++++++++++++++++++
sound/usb/media.h | 72 +++++++++++
sound/usb/mixer.h | 1 +
sound/usb/pcm.c | 28 ++++-
sound/usb/quirks-table.h | 1 +
sound/usb/stream.c | 2 +
sound/usb/usbaudio.h | 3 +
11 files changed, 442 insertions(+), 5 deletions(-)
create mode 100644 sound/usb/media.c
create mode 100644 sound/usb/media.h
@@ -0,0 +1,319 @@+/*+*media.c-MediaControllerspecificALSAdrivercode+*+*Copyright(c)2015ShuahKhan<shuahkh@osg.samsung.com>+*Copyright(c)2015SamsungElectronicsCo.,Ltd.+*+*ThisfileisreleasedundertheGPLv2.+*/++/*+*ThisfileaddsMediaControllersupporttoALSAdriver+*tousetheMediaControllerAPItosharetunerwithDVB+*andV4L2driversthatcontrolmediadevice.Mediadevice+*iscreatedbasedonexistingquirksframework.Usingthis+*approach,themediacontrollerAPIusagecanbeaddedfor+*aspecificdevice.+*/++#include<linux/init.h>+#include<linux/list.h>+#include<linux/mutex.h>+#include<linux/slab.h>+#include<linux/usb.h>++#include<sound/pcm.h>+#include<sound/core.h>++#include"usbaudio.h"+#include"card.h"+#include"mixer.h"+#include"media.h"++staticintmedia_enable_source(structmedia_ctl*mctl)+{+if(mctl&&mctl->media_dev->enable_source)+returnmctl->media_dev->enable_source(&mctl->media_entity,+&mctl->media_pipe);+return0;+}++staticvoidmedia_disable_source(structmedia_ctl*mctl)+{+if(mctl&&mctl->media_dev->disable_source)+mctl->media_dev->disable_source(&mctl->media_entity);+}++intmedia_stream_init(structsnd_usb_substream*subs,structsnd_pcm*pcm,+intstream)+{+structmedia_device*mdev;+structmedia_ctl*mctl;+structdevice*pcm_dev=&pcm->streams[stream].dev;+u32intf_type;+intret=0;+u16mixer_pad;+structmedia_entity*entity;++mdev=subs->stream->chip->media_dev;+if(!mdev)+return-ENODEV;++if(subs->media_ctl)+return0;++/* allocate media_ctl */+mctl=kzalloc(sizeof(*mctl),GFP_KERNEL);+if(!mctl)+return-ENOMEM;++mctl->media_dev=mdev;+if(stream==SNDRV_PCM_STREAM_PLAYBACK){+intf_type=MEDIA_INTF_T_ALSA_PCM_PLAYBACK;+mctl->media_entity.function=MEDIA_ENT_F_AUDIO_PLAYBACK;+mctl->media_pad.flags=MEDIA_PAD_FL_SOURCE;+mixer_pad=1;+}else{+intf_type=MEDIA_INTF_T_ALSA_PCM_CAPTURE;+mctl->media_entity.function=MEDIA_ENT_F_AUDIO_CAPTURE;+mctl->media_pad.flags=MEDIA_PAD_FL_SINK;+mixer_pad=2;+}+mctl->media_entity.name=pcm->name;+media_entity_pads_init(&mctl->media_entity,1,&mctl->media_pad);+ret=media_device_register_entity(mctl->media_dev,+&mctl->media_entity);+if(ret)+gotoerr1;++mctl->intf_devnode=media_devnode_create(mdev,intf_type,0,+MAJOR(pcm_dev->devt),+MINOR(pcm_dev->devt));+if(!mctl->intf_devnode){+ret=-ENOMEM;+gotoerr2;+}+mctl->intf_link=media_create_intf_link(&mctl->media_entity,+&mctl->intf_devnode->intf,+MEDIA_LNK_FL_ENABLED);+if(!mctl->intf_link){+ret=-ENOMEM;+gotoerr3;+}++/* create link between mixer and audio */+media_device_for_each_entity(entity,mdev){+switch(entity->function){+caseMEDIA_ENT_F_AUDIO_MIXER:+ret=media_create_pad_link(entity,mixer_pad,+&mctl->media_entity,0,+MEDIA_LNK_FL_ENABLED);+if(ret)+gotoerr4;+break;+}+}++subs->media_ctl=mctl;+return0;++err4:+media_remove_intf_link(mctl->intf_link);+err3:+media_devnode_remove(mctl->intf_devnode);+err2:+media_device_unregister_entity(&mctl->media_entity);+err1:+kfree(mctl);+returnret;+}++voidmedia_stream_delete(structsnd_usb_substream*subs)+{+structmedia_ctl*mctl=subs->media_ctl;++if(mctl&&mctl->media_dev){+structmedia_device*mdev;++mdev=subs->stream->chip->media_dev;+if(mdev&&media_devnode_is_registered(&mdev->devnode)){+media_devnode_remove(mctl->intf_devnode);+media_device_unregister_entity(&mctl->media_entity);+media_entity_cleanup(&mctl->media_entity);+}+kfree(mctl);+subs->media_ctl=NULL;+}+}++intmedia_start_pipeline(structsnd_usb_substream*subs)+{+structmedia_ctl*mctl=subs->media_ctl;++if(mctl)+returnmedia_enable_source(mctl);+return0;+}++voidmedia_stop_pipeline(structsnd_usb_substream*subs)+{+structmedia_ctl*mctl=subs->media_ctl;++if(mctl)+media_disable_source(mctl);+}++intmedia_mixer_init(structsnd_usb_audio*chip)+{+structdevice*ctl_dev=&chip->card->ctl_dev;+structmedia_intf_devnode*ctl_intf;+structusb_mixer_interface*mixer;+structmedia_device*mdev=chip->media_dev;+structmedia_mixer_ctl*mctl;+u32intf_type=MEDIA_INTF_T_ALSA_CONTROL;+intret;++if(!mdev)+return-ENODEV;++ctl_intf=chip->ctl_intf_media_devnode;+if(!ctl_intf){+ctl_intf=(void*)media_devnode_create(mdev,+intf_type,0,+MAJOR(ctl_dev->devt),+MINOR(ctl_dev->devt));+if(!ctl_intf)+return-ENOMEM;+chip->ctl_intf_media_devnode=ctl_intf;+}++list_for_each_entry(mixer,&chip->mixer_list,list){++if(mixer->media_mixer_ctl)+continue;++/* allocate media_mixer_ctl */+mctl=kzalloc(sizeof(*mctl),GFP_KERNEL);+if(!mctl)+return-ENOMEM;++mctl->media_dev=mdev;+mctl->media_entity.function=MEDIA_ENT_F_AUDIO_MIXER;+mctl->media_entity.name=chip->card->mixername;+mctl->media_pad[0].flags=MEDIA_PAD_FL_SINK;+mctl->media_pad[1].flags=MEDIA_PAD_FL_SOURCE;+mctl->media_pad[2].flags=MEDIA_PAD_FL_SOURCE;+media_entity_pads_init(&mctl->media_entity,MEDIA_MIXER_PAD_MAX,+mctl->media_pad);+ret=media_device_register_entity(mctl->media_dev,+&mctl->media_entity);+if(ret){+kfree(mctl);+returnret;+}++mctl->intf_link=media_create_intf_link(&mctl->media_entity,+&ctl_intf->intf,+MEDIA_LNK_FL_ENABLED);+if(!mctl->intf_link){+media_device_unregister_entity(&mctl->media_entity);+media_entity_cleanup(&mctl->media_entity);+kfree(mctl);+return-ENOMEM;+}+mctl->intf_devnode=ctl_intf;+mixer->media_mixer_ctl=mctl;+}+return0;+}++staticvoidmedia_mixer_delete(structsnd_usb_audio*chip)+{+structusb_mixer_interface*mixer;+structmedia_device*mdev=chip->media_dev;++if(!mdev)+return;++list_for_each_entry(mixer,&chip->mixer_list,list){+structmedia_mixer_ctl*mctl;++mctl=mixer->media_mixer_ctl;+if(!mixer->media_mixer_ctl)+continue;++if(media_devnode_is_registered(&mdev->devnode)){+media_device_unregister_entity(&mctl->media_entity);+media_entity_cleanup(&mctl->media_entity);+}+kfree(mctl);+mixer->media_mixer_ctl=NULL;+}+if(media_devnode_is_registered(&mdev->devnode))+media_devnode_remove(chip->ctl_intf_media_devnode);+chip->ctl_intf_media_devnode=NULL;+}++intmedia_device_create(structsnd_usb_audio*chip,+structusb_interface*iface)+{+structmedia_device*mdev;+structusb_device*usbdev=interface_to_usbdev(iface);+intret;++mdev=media_device_get_devres(&usbdev->dev);+if(!mdev)+return-ENOMEM;+if(!mdev->dev){+/* register media device */+mdev->dev=&usbdev->dev;+if(usbdev->product)+strlcpy(mdev->model,usbdev->product,+sizeof(mdev->model));+if(usbdev->serial)+strlcpy(mdev->serial,usbdev->serial,+sizeof(mdev->serial));+strcpy(mdev->bus_info,usbdev->devpath);+mdev->hw_revision=le16_to_cpu(usbdev->descriptor.bcdDevice);+media_device_init(mdev);+}+if(!media_devnode_is_registered(&mdev->devnode)){+ret=media_device_register(mdev);+if(ret){+dev_err(&usbdev->dev,+"Couldn't register media device. Error: %d\n",+ret);+returnret;+}+}++/* save media device - avoid lookups */+chip->media_dev=mdev;++/* Create media entities for mixer and control dev */+ret=media_mixer_init(chip);+if(ret){+dev_err(&usbdev->dev,+"Couldn't create media mixer entities. Error: %d\n",+ret);++/* clear saved media_dev */+chip->media_dev=NULL;++returnret;+}+return0;+}++voidmedia_device_delete(structsnd_usb_audio*chip)+{+structmedia_device*mdev=chip->media_dev;++media_mixer_delete(chip);++if(mdev){+if(media_devnode_is_registered(&mdev->devnode))+media_device_unregister(mdev);+chip->media_dev=NULL;+}+}
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 <redacted>
---
drivers/media/usb/au0828/au0828-core.c | 149 +++++++++++++++++++++++++++++++++
drivers/media/usb/au0828/au0828.h | 3 +
2 files changed, 152 insertions(+)
@@ -282,6 +282,7 @@ static int au0828_create_media_graph(struct au0828_dev *dev)return-EINVAL;if(tuner){+dev->tuner=tuner;/* create tuner to decoder link in deactivated state */ret=media_create_pad_link(tuner,TUNER_PAD_OUTPUT,decoder,0,0);
@@ -373,6 +374,150 @@ void au0828_media_graph_notify(struct media_entity *new, void *notify_data)#endif}+staticintau0828_enable_source(structmedia_entity*entity,+structmedia_pipeline*pipe)+{+#ifdef CONFIG_MEDIA_CONTROLLER+structmedia_entity*source;+structmedia_entity*sink;+structmedia_link*link,*found_link=NULL;+intret=0;+structmedia_device*mdev=entity->graph_obj.mdev;+structau0828_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;+gotoend;+}++/*+*ForAudioandV4L2entity,findthelinktowhichdecoder+*isthesink.Lookforanactivelinkbetweendecoderand+*tuner,ifoneexists,nothingtodo.Ifnot,lookforany+*activelinksbetweentunerandanyotherentity.Ifone+*exists,tunerisbusy.Iftunerisfree,setuplinkand+*startpipelinefromsource(tuner).+*ForDVBFEentity,thesourceforthelinkisthetuner.+*Checkiftunerisavailableandsetuplinkandstart+*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;+gotoend;+}else{+ret=-EBUSY;+gotoend;+}+}++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;+gotoend;+}++/* 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);+gotoend;+}++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);+gotoend;+}+/*+*saveactivelinkandactivelinkownertoavoidaudio+*deactivatingvideoownedlinkfromdisable_sourceand+*viceversa+*/+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);+returnret;+#endif+return0;+}++staticvoidau0828_disable_source(structmedia_entity*entity)+{+#ifdef CONFIG_MEDIA_CONTROLLER+structmedia_entity*sink;+intret=0;+structmedia_device*mdev=entity->graph_obj.mdev;+structau0828_dev*dev;++if(!mdev)+return;++mutex_lock(&mdev->graph_mutex);+dev=mdev->source_priv;+if(!dev->tuner||!dev->decoder||!dev->active_link){+ret=-ENODEV;+gotoend;+}++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){+/*+*preventvideofromdeactivatinglinkwhenaudio+*hasactivepipeline+*/+if(dev->active_link_owner!=entity)+gotoend;+__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;+}++end:+mutex_unlock(&mdev->graph_mutex);+#endif+}+staticintau0828_media_device_register(structau0828_dev*dev,structusb_device*udev){
@@ -403,6 +548,10 @@ static int au0828_media_device_register(struct au0828_dev *dev,ret);returnret;}+/* 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;#endifreturn0;}
@@ -347,14 +347,42 @@ static int au0828_create_media_graph(struct au0828_dev *dev)return0;}+voidau0828_media_graph_notify(structmedia_entity*new,void*notify_data)+{+#ifdef CONFIG_MEDIA_CONTROLLER+structau0828_dev*dev=(structau0828_dev*)notify_data;+intret;++if(!dev->decoder)+return;++switch(new->function){+caseMEDIA_ENT_F_AUDIO_MIXER:+ret=media_create_pad_link(dev->decoder,+AU8522_PAD_AUDIO_OUT,+new,0,+MEDIA_LNK_FL_ENABLED);+if(ret)+dev_err(&dev->usbdev->dev,+"Mixer Pad Link Create Error: %d\n",+ret);+break;+default:+break;+}+#endif+}+staticintau0828_media_device_register(structau0828_dev*dev,structusb_device*udev){#ifdef CONFIG_MEDIA_CONTROLLERintret;-if(dev->media_dev&&-!media_devnode_is_registered(&dev->media_dev->devnode)){+if(!dev->media_dev)+return0;++if(!media_devnode_is_registered(&dev->media_dev->devnode)){/* register media device */ret=media_device_register(dev->media_dev);
Change au0828_create_media_graph() to find and disable
tuner and demod link. This helps avoid an additional
disable step when tuner is requested by video or audio.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/usb/au0828/au0828-core.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
Change au0828 to check if tuner is free or not
before changing tuner configuration.
vidioc_g_tuner(), and au0828_v4l2_close() now call
v4l-core interface v4l_enable_media_source() before
changing tuner configuration.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/usb/au0828/au0828-video.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
@@ -1006,8 +1006,12 @@ static int au0828_v4l2_close(struct file *filp)gotoend;if(dev->users==1){-/* Save some power by putting tuner to sleep */-v4l2_device_call_all(&dev->v4l2_dev,0,core,s_power,0);+/* Save some power by putting tuner to sleep, if it is free */+/* What happens when radio is using tuner?? */+ret=v4l_enable_media_source(vdev);+if(ret==0)+v4l2_device_call_all(&dev->v4l2_dev,0,core,+s_power,0);dev->std_set_in_tuner_core=0;/* When close the device, set the usb intf0 into alt0 to free
@@ -1408,10 +1412,16 @@ static int vidioc_s_audio(struct file *file, void *priv, const struct v4l2_audiostaticintvidioc_g_tuner(structfile*file,void*priv,structv4l2_tuner*t){structau0828_dev*dev=video_drvdata(file);+structvideo_device*vfd=video_devdata(file);+intret;if(t->index!=0)return-EINVAL;+ret=v4l_enable_media_source(vfd);+if(ret)+returnret;+dprintk(1,"%s called std_set %d dev_state %d\n",__func__,dev->std_set_in_tuner_core,dev->dev_state);
Remove au0828_enable_analog_tuner() as it is
no longer needed because v4l2-core implements
common interfaces to check for media source
availability.
In addition, queue_setup() no longer needs the
tuner availability check since v4l2-core does it.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/usb/au0828/au0828-video.c | 61 ---------------------------------
1 file changed, 61 deletions(-)
Change dvb frontend to check if tuner is free when
device opened in RW mode. Call to enable_source
handler either returns with an active pipeline to
tuner or error if tuner is busy. Tuner is released
when frontend is released calling the disable_source
handler.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/dvb-core/dvb_frontend.c | 139 +++++-----------------------------
drivers/media/dvb-core/dvb_frontend.h | 3 +
2 files changed, 24 insertions(+), 118 deletions(-)
@@ -596,104 +591,12 @@ static void dvb_frontend_wakeup(struct dvb_frontend *fe)wake_up_interruptible(&fepriv->wait_queue);}-/**-*dvb_enable_media_tuner()-triestoenabletheDVBtuner-*-*@fe:structdvb_frontendpointer-*-*Thisfunctionensuresthatjustonemediatunerisenabledforagiven-*frontend.Ithastwodifferentbehaviors:-*-Fortrivialdeviceswithjustonetuner:-*itjustenablestheexistingtuner->felink-*-Fordeviceswithmorethanonetuner:-*Itisuptothedrivertoimplementthelogicthatwillenableonetuner-*anddisabletheotherones.However,ifmorethanonetunerisenabledfor-*thesamefrontend,itwillprintanerrormessageandreturn-EINVAL.-*-*Atreturn,itwillreturntheerrorcodereturnedbymedia_entity_setup_link,-*or0ifeverythingisOK,ifnotunerislinkedtothefrontendorifthe-*mdevisNULL.-*/-#ifdef CONFIG_MEDIA_CONTROLLER_DVB-staticintdvb_enable_media_tuner(structdvb_frontend*fe)-{-structdvb_frontend_private*fepriv=fe->frontend_priv;-structdvb_adapter*adapter=fe->dvb;-structmedia_device*mdev=adapter->mdev;-structmedia_entity*entity,*source;-structmedia_link*link,*found_link=NULL;-intret,n_links=0,active_links=0;--fepriv->pipe_start_entity=NULL;--if(!mdev)-return0;--entity=fepriv->dvbdev->entity;-fepriv->pipe_start_entity=entity;--list_for_each_entry(link,&entity->links,list){-if(link->sink->entity==entity){-found_link=link;-n_links++;-if(link->flags&MEDIA_LNK_FL_ENABLED)-active_links++;-}-}--if(!n_links||active_links==1||!found_link)-return0;--/*-*Ifafrontendhasmorethanonetunerlinked,itisuptothedriver-*toselectwithonewillbetheactiveone,asthefrontendcorecan't-*guess.Ifthedriverdoesn'tdothat,itisabug.-*/-if(n_links>1&&active_links!=1){-dev_err(fe->dvb->device,-"WARNING: there are %d active links among %d tuners. This is a driver's bug!\n",-active_links,n_links);-return-EINVAL;-}--source=found_link->source->entity;-fepriv->pipe_start_entity=source;-list_for_each_entry(link,&source->links,list){-structmedia_entity*sink;-intflags=0;--sink=link->sink->entity;-if(sink==entity)-flags=MEDIA_LNK_FL_ENABLED;--ret=media_entity_setup_link(link,flags);-if(ret){-dev_err(fe->dvb->device,-"Couldn't change link %s->%s to %s. Error %d\n",-source->name,sink->name,-flags?"enabled":"disabled",-ret);-returnret;-}else-dev_dbg(fe->dvb->device,-"link %s->%s was %s\n",-source->name,sink->name,-flags?"ENABLED":"disabled");-}-return0;-}-#endif-staticintdvb_frontend_thread(void*data){structdvb_frontend*fe=data;structdvb_frontend_private*fepriv=fe->frontend_priv;enumfe_statuss;enumdvbfe_algoalgo;-#ifdef CONFIG_MEDIA_CONTROLLER_DVB-intret;-#endif-boolre_tune=false;boolsemheld=false;
@@ -706,20 +609,6 @@ static int dvb_frontend_thread(void *data)fepriv->wakeup=0;fepriv->reinitialise=0;-#ifdef CONFIG_MEDIA_CONTROLLER_DVB-ret=dvb_enable_media_tuner(fe);-if(ret){-/* FIXME: return an error if it fails */-dev_info(fe->dvb->device,-"proceeding with FE task\n");-}elseif(fepriv->pipe_start_entity){-ret=media_entity_pipeline_start(fepriv->pipe_start_entity,-&fepriv->pipe);-if(ret)-returnret;-}-#endif-dvb_frontend_init(fe);set_freezable();
Change s_input, s_fmt, s_tuner, s_frequency, querystd,
s_hw_freq_seek, and vb2_core_streamon interfaces that
alter the tuner configuration to check if it is free,
by calling v4l_enable_media_source(). If source isn't
free, return -EBUSY. v4l_disable_media_source() is
called from v4l2_fh_exit() to release tuner (source).
vb2_core_streamon() uses v4l_vb2q_enable_media_source().
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/v4l2-core/v4l2-fh.c | 2 ++
drivers/media/v4l2-core/v4l2-ioctl.c | 30 ++++++++++++++++++++++++++++++
drivers/media/v4l2-core/videobuf2-core.c | 4 ++++
3 files changed, 36 insertions(+)
@@ -1041,6 +1042,12 @@ static int v4l_querycap(const struct v4l2_ioctl_ops *ops,staticintv4l_s_input(conststructv4l2_ioctl_ops*ops,structfile*file,void*fh,void*arg){+structvideo_device*vfd=video_devdata(file);+intret;++ret=v4l_enable_media_source(vfd);+if(ret)+returnret;returnops->vidioc_s_input(file,fh,*(unsignedint*)arg);}
@@ -1448,6 +1455,9 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,boolis_tx=vfd->vfl_dir!=VFL_DIR_RX;intret;+ret=v4l_enable_media_source(vfd);+if(ret)+returnret;v4l_sanitize_format(p);switch(p->type){
@@ -1637,7 +1647,11 @@ static int v4l_s_tuner(const struct v4l2_ioctl_ops *ops,{structvideo_device*vfd=video_devdata(file);structv4l2_tuner*p=arg;+intret;+ret=v4l_enable_media_source(vfd);+if(ret)+returnret;p->type=(vfd->vfl_type==VFL_TYPE_RADIO)?V4L2_TUNER_RADIO:V4L2_TUNER_ANALOG_TV;returnops->vidioc_s_tuner(file,fh,p);
@@ -1691,7 +1705,11 @@ static int v4l_s_frequency(const struct v4l2_ioctl_ops *ops,structvideo_device*vfd=video_devdata(file);conststructv4l2_frequency*p=arg;enumv4l2_tuner_typetype;+intret;+ret=v4l_enable_media_source(vfd);+if(ret)+returnret;if(vfd->vfl_type==VFL_TYPE_SDR){if(p->type!=V4L2_TUNER_SDR&&p->type!=V4L2_TUNER_RF)return-EINVAL;
@@ -1746,7 +1764,11 @@ static int v4l_s_std(const struct v4l2_ioctl_ops *ops,{structvideo_device*vfd=video_devdata(file);v4l2_std_idid=*(v4l2_std_id*)arg,norm;+intret;+ret=v4l_enable_media_source(vfd);+if(ret)+returnret;norm=id&vfd->tvnorms;if(vfd->tvnorms&&!norm)/* Check if std is supported */return-EINVAL;
@@ -1760,7 +1782,11 @@ static int v4l_querystd(const struct v4l2_ioctl_ops *ops,{structvideo_device*vfd=video_devdata(file);v4l2_std_id*p=arg;+intret;+ret=v4l_enable_media_source(vfd);+if(ret)+returnret;/**Ifnosignalisdetected,thenthedrivershouldreturn*V4L2_STD_UNKNOWN.Otherwiseitshouldreturntvnormswith
@@ -1779,7 +1805,11 @@ static int v4l_s_hw_freq_seek(const struct v4l2_ioctl_ops *ops,structvideo_device*vfd=video_devdata(file);structv4l2_hw_freq_seek*p=arg;enumv4l2_tuner_typetype;+intret;+ret=v4l_enable_media_source(vfd);+if(ret)+returnret;/* s_hw_freq_seek is not supported for SDR for now */if(vfd->vfl_type==VFL_TYPE_SDR)return-EINVAL;
Move the au8522_media_pads enum to au8522.h
from au8522_priv.h to export them. This will
allow au0828-core to use these defines instead
of hard-coding them them.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/dvb-frontends/au8522.h | 7 +++++++
drivers/media/dvb-frontends/au8522_priv.h | 8 --------
2 files changed, 7 insertions(+), 8 deletions(-)
Add new fields to struct media_device to add enable_source, and
disable_source handlers, and source_priv to stash driver private
data that is used to run these handlers. The enable_source handler
finds source entity for the passed in entity and checks if it is
available. When link is found, it activates it. Disable source
handler deactivates the link.
Bridge driver is expected to implement and set these handlers.
Signed-off-by: Shuah Khan <redacted>
---
include/media/media-device.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
Add new interfaces to register and unregister entity_notify
hook to media device. These interfaces allow drivers to add
hooks to take appropriate actions when new entities get added
to a shared media device. For example, au0828 bridge driver
registers an entity_notify hook to create links as needed
between media graph nodes.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/media-device.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
include/media/media-device.h | 25 ++++++++++++++++++
2 files changed, 86 insertions(+)
@@ -685,11 +694,59 @@ int __must_check __media_device_register(struct media_device *mdev,}EXPORT_SYMBOL_GPL(__media_device_register);+/**+*media_device_register_entity_notify-Registeramediaentitynotify+*callbackwithamediadevice.Whenanewentityisregistered,all+*theregisteredmedia_entity_notifycallbacksareinvoked.+*@mdev:Themediadevice+*@nptr:Themedia_entity_notify+*/+int__must_checkmedia_device_register_entity_notify(structmedia_device*mdev,+structmedia_entity_notify*nptr)+{+spin_lock(&mdev->lock);+list_add_tail(&nptr->list,&mdev->entity_notify);+spin_unlock(&mdev->lock);+return0;+}+EXPORT_SYMBOL_GPL(media_device_register_entity_notify);++/**+*__media_device_unregister_entity_notify-Unregisteramediaentitynotify+*callbackwithamediadevice.Whenanewentityisregistered,all+*theregisteredmedia_entity_notifycallbacksareinvoked.+*@mdev:Themediadevice+*@nptr:Themedia_entity_notify+*Non-lockingversion.Shouldbecalledwithmdev->lockheld.+*/+staticvoid__media_device_unregister_entity_notify(structmedia_device*mdev,+structmedia_entity_notify*nptr)+{+list_del(&nptr->list);+}++/**+*media_device_unregister_entity_notify-Unregisteramediaentitynotify+*callbackwithamediadevice.Whenanewentityisregistered,all+*theregisteredmedia_entity_notifycallbacksareinvoked.+*@mdev:Themediadevice+*@nptr:Themedia_entity_notify+*/+voidmedia_device_unregister_entity_notify(structmedia_device*mdev,+structmedia_entity_notify*nptr)+{+spin_lock(&mdev->lock);+__media_device_unregister_entity_notify(mdev,nptr);+spin_unlock(&mdev->lock);+}+EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);+voidmedia_device_unregister(structmedia_device*mdev){structmedia_entity*entity;structmedia_entity*next;structmedia_interface*intf,*tmp_intf;+structmedia_entity_notify*notify,*nextp;if(mdev==NULL)return;
@@ -706,6 +763,10 @@ void media_device_unregister(struct media_device *mdev)list_for_each_entry_safe(entity,next,&mdev->entities,graph_obj.list)__media_device_unregister_entity(entity);+/* Remove all entity_notify callbacks from the media device */+list_for_each_entry_safe(notify,nextp,&mdev->entity_notify,list)+__media_device_unregister_entity_notify(mdev,notify);+/* Remove all interfaces from the media device */list_for_each_entry_safe(intf,tmp_intf,&mdev->interfaces,graph_obj.list){
@@ -319,6 +325,9 @@ struct media_device {structlist_headpads;structlist_headlinks;+/* notify callback list invoked when a new entity is registered */+structlist_headentity_notify;+/* Protects the graph objects creation/removal */spinlock_tlock;/* Serializes graph operations. */
@@ -497,6 +506,11 @@ int __must_check media_device_register_entity(struct media_device *mdev,*/voidmedia_device_unregister_entity(structmedia_entity*entity);+int__must_checkmedia_device_register_entity_notify(structmedia_device*mdev,+structmedia_entity_notify*nptr);+voidmedia_device_unregister_entity_notify(structmedia_device*mdev,+structmedia_entity_notify*nptr);+/***media_device_get_devres()-getmediadeviceasdeviceresource*createsifonedoesn'texist
@@ -552,6 +566,17 @@ static inline int media_device_register_entity(struct media_device *mdev,staticinlinevoidmedia_device_unregister_entity(structmedia_entity*entity){}+staticinlineintmedia_device_register_entity_notify(+structmedia_device*mdev,+structmedia_entity_notify*nptr)+{+return0;+}+staticinlinevoidmedia_device_unregister_entity_notify(+structmedia_device*mdev,+structmedia_entity_notify*nptr)+{+}staticinlinestructmedia_device*media_device_get_devres(structdevice*dev){returnNULL;
From: kbuild test robot <hidden> Date: 2016-02-04 06:30:24
Hi Shuah,
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on next-20160203]
[cannot apply to v4.5-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Shuah-Khan/Sharing-media-resources-across-ALSA-and-au0828-drivers/20160204-121414
base: git://linuxtv.org/media_tree.git master
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
include/linux/init.h:1: warning: no structured comments found
kernel/sys.c:1: warning: no structured comments found
drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
drivers/dma-buf/reservation.c:1: warning: no structured comments found
include/linux/reservation.h:1: warning: no structured comments found
quoted
include/media/v4l2-mc.h:114: warning: No description found for parameter 'vdev'
include/media/v4l2-mc.h:128: warning: No description found for parameter 'vdev'
include/media/media-device.h:357: warning: No description found for parameter 'entity_notify'
include/media/media-device.h:357: warning: No description found for parameter 'source_priv'
include/media/media-device.h:357: warning: No description found for parameter 'enable_source'
include/media/media-device.h:357: warning: No description found for parameter 'disable_source'
include/media/media-device.h:357: warning: No description found for parameter 'entity_notify'
include/media/media-device.h:357: warning: No description found for parameter 'source_priv'
include/media/media-device.h:357: warning: No description found for parameter 'enable_source'
include/media/media-device.h:357: warning: No description found for parameter 'disable_source'
include/media/media-entity.h:840: warning: No description found for parameter 'entity'
include/media/media-entity.h:840: warning: No description found for parameter 'pipe'
include/media/media-entity.h:860: warning: No description found for parameter 'entity'
include/linux/spi/spi.h:540: warning: No description found for parameter 'max_transfer_size'
vim +/vdev +114 include/media/v4l2-mc.h
98 /**
99 * v4l_enable_media_source() - Hold media source for exclusive use
100 * if free
101 *
102 * @vdev - poniter to struct video_device
103 *
104 * This interface calls enable_source handler to determine if
105 * media source is free for use. The enable_source handler is
106 * responsible for checking is the media source is free and
107 * start a pipeline between the media source and the media
108 * entity associated with the video device. This interface
109 * should be called from v4l2-core and dvb-core interfaces
110 * that change the source configuration.
111 *
112 * Return: returns zero on success or a negative error code.
113 */
> 114 int v4l_enable_media_source(struct video_device *vdev);
115
116 /**
117 * v4l_disable_media_source() - Release media source
118 *
119 * @vdev - poniter to struct video_device
120 *
121 * This interface calls disable_source handler to release
122 * the media source. The disable_source handler stops the
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Em Wed, 03 Feb 2016 21:03:33 -0700
Shuah Khan [off-list ref] escreveu:
quoted hunk
Declare the interface types to be used on alsa for
the new G_TOPOLOGY ioctl.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/media-entity.c | 16 ++++++++++++++++
include/uapi/linux/media.h | 22 ++++++++++++++++++++++
2 files changed, 38 insertions(+)
We don't document the userspace API using kernel-doc, as it is too
poor for that. Also, we migrated the uAPI documentation from LaTex
(at DVB side) and from a separate DocBook document. Migrating those to
kernel-doc would need some rich documentation markup language, and
someone with lots of spare time.
Instead, we document them at a separate DocBook volume:
Documentation/DocBook/media_api.tmpl
The actual DocBook documents are at:
Documentation/DocBook/media/dvb - for the DVB side
Documentation/DocBook/media/v4l - for V4L2, RC and Media Controller
In the specific case of the Media Controller, the description of those
defines are at:
Documentation/DocBook/media/v4l/media-types.xml
Just edit it with some text editor and add the new fields there at the
right places.
Please test if the documentation is producing the right data, by using
the enclosed small script. The extra xmllint lines validate the syntax,
helping to identify hidden missing tags. The last line will produce a
single html file, instead of one html file per page (with is the default
for make htmldocs).
Regards,
Mauro
#!/bin/bash
LC_ALL=en_US.UTF-8
make cleanmediadocs
make DOCBOOKS=media_api.xml htmldocs 2>&1 | grep -v "element.*: validity error : ID .* already defined"
xmllint --noent --postvalid "$PWD/Documentation/DocBook/media_api.xml" >/tmp/x.xml 2>/dev/null
xmllint --noent --postvalid --noout /tmp/x.xml
xmlto html-nochunks -m ./Documentation/DocBook/stylesheet.xsl -o Documentation/DocBook/media Documentation/DocBook/media_api.xml >/dev/null 2>&1
Wrong way to document it. Please see my comments to patch 01/22.
+#define MEDIA_ENT_F_AUDIO_CAPTURE (MEDIA_ENT_F_BASE + 200)
+#define MEDIA_ENT_F_AUDIO_PLAYBACK (MEDIA_ENT_F_BASE + 201)
+#define MEDIA_ENT_F_AUDIO_MIXER (MEDIA_ENT_F_BASE + 202)
+
+/*
* Don't touch on those. The ranges MEDIA_ENT_F_OLD_BASE and
* MEDIA_ENT_F_OLD_SUBDEV_BASE are kept to keep backward compatibility
* with the legacy v1 API.The number range is out of range by purpose:
Em Wed, 03 Feb 2016 21:03:35 -0700
Shuah Khan [off-list ref] escreveu:
Add new interfaces to register and unregister entity_notify
hook to media device. These interfaces allow drivers to add
hooks to take appropriate actions when new entities get added
to a shared media device. For example, au0828 bridge driver
registers an entity_notify hook to create links as needed
between media graph nodes.
Shuah,
It seems you didn't address the documentation issues I pointed on the
last review. While not ideal, I'll accept, in this specific case,
although this requires an extra time for me (as I want to read the
documentation *before* actually reviewing the patches), I'm OK
if you send the documentation a the end of the series, but please
send it *together* with the patch series.
That affect not only me, but others that build documentation and
gets lots of errors if something is not documented at the right
place.
See below for some notes. I won't be commenting documentation on
the rest of this patch series.
@@ -685,11 +694,59 @@ int __must_check __media_device_register(struct media_device *mdev,}EXPORT_SYMBOL_GPL(__media_device_register);+/**+*media_device_register_entity_notify-Registeramediaentitynotify+*callbackwithamediadevice.Whenanewentityisregistered,all+*theregisteredmedia_entity_notifycallbacksareinvoked.+*@mdev:Themediadevice+*@nptr:Themedia_entity_notify+*/
Please split the comments into a short description and a detailed
description, and add blank lines to make it easier to be read by
mortals.
The short description appears at the index of the html page:
https://linuxtv.org/downloads/v4l-dvb-internals/device-drivers/ch06s04.html
So, we want it to be short ;)
The detailed description appears at the end:
https://linuxtv.org/downloads/v4l-dvb-internals/device-drivers/API-dvb-create-media-graph.html
Btw, in the above comment, you don't have actually a detailed description,
but, instead, a note. So, the best would be do to it as:
/**
* media_device_register_entity_notify() - Register a media entity notify
* callback with a media device.
*
* @mdev: The media device
* @nptr: The media_entity_notify
*
* NOTE: When a new entity is registered, all the registered
* media_entity_notify callbacks are invoked.
*/
The "NOTE: foo" will appear as:
https://linuxtv.org/downloads/v4l-dvb-internals/device-drivers/API-media-device-register-entity.html
P.S.: Don't forget to test if everything is OK with:
rm Documentation/DocBook/device-drivers.aux.xml Documentation/DocBook/device-drivers.xml
LC_ALL=en_US.UTF-8 make DOCBOOKS=device-drivers.xml htmldocs
The same note applied to the other DocBook tags you added.
Btw, add the documentation *only* at the .h.
quoted hunk
+int __must_check media_device_register_entity_notify(struct media_device *mdev,
+ struct media_entity_notify *nptr)
+{
+ spin_lock(&mdev->lock);
+ list_add_tail(&nptr->list, &mdev->entity_notify);
+ spin_unlock(&mdev->lock);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(media_device_register_entity_notify);
+
+/**
+ * __media_device_unregister_entity_notify - Unregister a media entity notify
+ * callback with a media device. When a new entity is registered, all
+ * the registered media_entity_notify callbacks are invoked.
+ * @mdev: The media device
+ * @nptr: The media_entity_notify
+ * Non-locking version. Should be called with mdev->lock held.
+ */
+static void __media_device_unregister_entity_notify(struct media_device *mdev,
+ struct media_entity_notify *nptr)
+{
+ list_del(&nptr->list);
+}
+
+/**
+ * media_device_unregister_entity_notify - Unregister a media entity notify
+ * callback with a media device. When a new entity is registered, all
+ * the registered media_entity_notify callbacks are invoked.
+ * @mdev: The media device
+ * @nptr: The media_entity_notify
+ */
+void media_device_unregister_entity_notify(struct media_device *mdev,
+ struct media_entity_notify *nptr)
+{
+ spin_lock(&mdev->lock);
+ __media_device_unregister_entity_notify(mdev, nptr);
+ spin_unlock(&mdev->lock);
+}
+EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);
+
void media_device_unregister(struct media_device *mdev)
{
struct media_entity *entity;
struct media_entity *next;
struct media_interface *intf, *tmp_intf;
+ struct media_entity_notify *notify, *nextp;
if (mdev == NULL)
return;
@@ -706,6 +763,10 @@ void media_device_unregister(struct media_device *mdev) list_for_each_entry_safe(entity, next, &mdev->entities, graph_obj.list) __media_device_unregister_entity(entity);+ /* Remove all entity_notify callbacks from the media device */+ list_for_each_entry_safe(notify, nextp, &mdev->entity_notify, list)+ __media_device_unregister_entity_notify(mdev, notify);+ /* Remove all interfaces from the media device */ list_for_each_entry_safe(intf, tmp_intf, &mdev->interfaces, graph_obj.list) {
@@ -319,6 +325,9 @@ struct media_device { struct list_head pads; struct list_head links;+ /* notify callback list invoked when a new entity is registered */+ struct list_head entity_notify;+
Documentation is missing. You should have noticed if you tried to
generate the docbook, as it would produce an error like:
include/media/media-device.h:357: warning: No description found for parameter 'entity_notify'
as reported by kbuild test robot.
Em Wed, 03 Feb 2016 21:03:38 -0700
Shuah Khan [off-list ref] escreveu:
Add non-locking __media_entity_pipeline_start/stop() interfaces
to be called from code paths that hold the graph_mutex. For this
change, the media_entity_pipeline_start() routine is renamed to
__media_entity_pipeline_start() minus the graph_mutex lock and
unlock. media_entity_pipeline_start() now calls the non-locking
__media_entity_pipeline_start() holding the graph_lock. The stop
interface, media_entity_pipeline_stop() routine is renamed to
__media_entity_pipeline_stop() minus the graph_mutex lock and
unlock. media_entity_pipeline_stop() now calls the non-locking
__media_entity_pipeline_stop() holding the graph_lock.
Please write a better patch descriptions. The above could easily
be split into 3 or 4 paragraphs, with would make it easier to
read.
Also, too much details, written on a spaghetti style;)
It is obvious by anyone that looked at the patch that
"media_entity_pipeline_start() routine is renamed to
__media_entity_pipeline_start() minus the graph_mutex lock and
unlock."
I read exactly the same thing at the C code, *and* at the
subject of the e-mail. Patch subject/description should tell
what, why and how (but you should not repeat what can easily
be inferred from looking at the diff).
I would describe it as:
Currently, media pipeline graph traversal routines take
the graph_mutex lock.
While this works for the current code, we'll need
unlocked versions of them because
(some explanation *why* you need it, with is the main
thing that should be at the description).
So, add non-locking media_entity_pipeline_start/stop()
variations.
Em Wed, 03 Feb 2016 21:03:39 -0700
Shuah Khan [off-list ref] escreveu:
quoted hunk
Add a new interfaces to be used by v4l-core to invoke enable
source and disable_source handlers in the media_device. The
enable_source helper function invokes the enable_source handler
to find media source entity connected to the entity and check
is it is available or busy. If source is available, link is
activated and pipeline is started. The disable_source helper
function invokes the disable_source handler to deactivate and
stop the pipeline.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/v4l2-core/Makefile | 2 +-
drivers/media/v4l2-core/v4l2-mc.c | 60 +++++++++++++++++++++++++++++++++++++++
include/media/v4l2-dev.h | 1 +
include/media/v4l2-mc.h | 52 +++++++++++++++++++++++++++++++++
4 files changed, 114 insertions(+), 1 deletion(-)
create mode 100644 drivers/media/v4l2-core/v4l2-mc.c
The code itself is ok. However, the above code would work better if you
declare them as static inline at the header file (v4l2-mc.h). That would
allow the compiler to optimize them with the caller codr (where the
declared structs like mdev, fh are likely already stored on some registers),
and even removing the function completely if compiled without the media
controller.
Em Wed, 03 Feb 2016 21:03:43 -0700
Shuah Khan [off-list ref] escreveu:
Change dvb frontend to check if tuner is free when
device opened in RW mode. Call to enable_source
handler either returns with an active pipeline to
tuner or error if tuner is busy. Tuner is released
when frontend is released calling the disable_source
handler.
This patch seems too early in the series, as I'm not seeing any patch
providing a replacement for the removed code yet.
@@ -596,104 +591,12 @@ static void dvb_frontend_wakeup(struct dvb_frontend *fe)wake_up_interruptible(&fepriv->wait_queue);}-/**-*dvb_enable_media_tuner()-triestoenabletheDVBtuner-*-*@fe:structdvb_frontendpointer-*-*Thisfunctionensuresthatjustonemediatunerisenabledforagiven-*frontend.Ithastwodifferentbehaviors:-*-Fortrivialdeviceswithjustonetuner:-*itjustenablestheexistingtuner->felink-*-Fordeviceswithmorethanonetuner:-*Itisuptothedrivertoimplementthelogicthatwillenableonetuner-*anddisabletheotherones.However,ifmorethanonetunerisenabledfor-*thesamefrontend,itwillprintanerrormessageandreturn-EINVAL.-*-*Atreturn,itwillreturntheerrorcodereturnedbymedia_entity_setup_link,-*or0ifeverythingisOK,ifnotunerislinkedtothefrontendorifthe-*mdevisNULL.-*/-#ifdef CONFIG_MEDIA_CONTROLLER_DVB-staticintdvb_enable_media_tuner(structdvb_frontend*fe)-{-structdvb_frontend_private*fepriv=fe->frontend_priv;-structdvb_adapter*adapter=fe->dvb;-structmedia_device*mdev=adapter->mdev;-structmedia_entity*entity,*source;-structmedia_link*link,*found_link=NULL;-intret,n_links=0,active_links=0;--fepriv->pipe_start_entity=NULL;--if(!mdev)-return0;--entity=fepriv->dvbdev->entity;-fepriv->pipe_start_entity=entity;--list_for_each_entry(link,&entity->links,list){-if(link->sink->entity==entity){-found_link=link;-n_links++;-if(link->flags&MEDIA_LNK_FL_ENABLED)-active_links++;-}-}--if(!n_links||active_links==1||!found_link)-return0;--/*-*Ifafrontendhasmorethanonetunerlinked,itisuptothedriver-*toselectwithonewillbetheactiveone,asthefrontendcorecan't-*guess.Ifthedriverdoesn'tdothat,itisabug.-*/-if(n_links>1&&active_links!=1){-dev_err(fe->dvb->device,-"WARNING: there are %d active links among %d tuners. This is a driver's bug!\n",-active_links,n_links);-return-EINVAL;-}--source=found_link->source->entity;-fepriv->pipe_start_entity=source;-list_for_each_entry(link,&source->links,list){-structmedia_entity*sink;-intflags=0;--sink=link->sink->entity;-if(sink==entity)-flags=MEDIA_LNK_FL_ENABLED;--ret=media_entity_setup_link(link,flags);-if(ret){-dev_err(fe->dvb->device,-"Couldn't change link %s->%s to %s. Error %d\n",-source->name,sink->name,-flags?"enabled":"disabled",-ret);-returnret;-}else-dev_dbg(fe->dvb->device,-"link %s->%s was %s\n",-source->name,sink->name,-flags?"ENABLED":"disabled");-}-return0;-}-#endif-staticintdvb_frontend_thread(void*data){structdvb_frontend*fe=data;structdvb_frontend_private*fepriv=fe->frontend_priv;enumfe_statuss;enumdvbfe_algoalgo;-#ifdef CONFIG_MEDIA_CONTROLLER_DVB-intret;-#endif-boolre_tune=false;boolsemheld=false;
@@ -706,20 +609,6 @@ static int dvb_frontend_thread(void *data)fepriv->wakeup=0;fepriv->reinitialise=0;-#ifdef CONFIG_MEDIA_CONTROLLER_DVB-ret=dvb_enable_media_tuner(fe);-if(ret){-/* FIXME: return an error if it fails */-dev_info(fe->dvb->device,-"proceeding with FE task\n");-}elseif(fepriv->pipe_start_entity){-ret=media_entity_pipeline_start(fepriv->pipe_start_entity,-&fepriv->pipe);-if(ret)-returnret;-}-#endif-dvb_frontend_init(fe);set_freezable();
Em Wed, 03 Feb 2016 21:03:44 -0700
Shuah Khan [off-list ref] escreveu:
Remove au0828_enable_analog_tuner() as it is
no longer needed because v4l2-core implements
common interfaces to check for media source
availability.
I didn't see such code at v4l2-core yet. Missing patch?
quoted hunk
In addition, queue_setup() no longer needs the
tuner availability check since v4l2-core does it.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/usb/au0828/au0828-video.c | 61 ---------------------------------
1 file changed, 61 deletions(-)
Em Wed, 03 Feb 2016 21:03:45 -0700
Shuah Khan [off-list ref] escreveu:
quoted hunk
Change au0828 to check if tuner is free or not
before changing tuner configuration.
vidioc_g_tuner(), and au0828_v4l2_close() now call
v4l-core interface v4l_enable_media_source() before
changing tuner configuration.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/usb/au0828/au0828-video.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
@@ -1006,8 +1006,12 @@ static int au0828_v4l2_close(struct file *filp)gotoend;if(dev->users==1){-/* Save some power by putting tuner to sleep */-v4l2_device_call_all(&dev->v4l2_dev,0,core,s_power,0);+/* Save some power by putting tuner to sleep, if it is free */+/* What happens when radio is using tuner?? */+ret=v4l_enable_media_source(vdev);+if(ret==0)+v4l2_device_call_all(&dev->v4l2_dev,0,core,+s_power,0);
Actually, for most USB devices (and this is the case of au0828), the
tuner can safely be disabled here if ALSA is not streaming, as, except
for some very old USB tuner models (like em28xx-based WinTV USB2) have
a separate audio output jack.
Btw, those devices that have a separate audio output jack have
analog tuners, like Philips FM1236. Those devices are always on, so
the s_power callback are silently ignored.
So, the best logic here would be to disable the tuner if:
- ALSA is not streaming;
- the last V4L2 file handler is closed.
Additionally, media source can be disabled if the above conditions
are met and if the device doesn't have a separate audio out plug
(or doesn't use a silicon tuner like xc2028/3028/4000/5000).
Of course, as ALSA may stream anytime, a callback would be needed to
enable the media source and power on the tuner, for radio to work.
I understand that the above would require some other non-trivial
changes, with would be best addressed on a separate patch series.
So, for now, I would add a FIXME there explaining the above.
quoted hunk
dev->std_set_in_tuner_core = 0;
/* When close the device, set the usb intf0 into alt0 to free
@@ -1408,10 +1412,16 @@ static int vidioc_s_audio(struct file *file, void *priv, const struct v4l2_audio static int vidioc_g_tuner(struct file *file, void *priv, struct v4l2_tuner *t) { struct au0828_dev *dev = video_drvdata(file);+ struct video_device *vfd = video_devdata(file);+ int ret; if (t->index != 0) return -EINVAL;+ ret = v4l_enable_media_source(vfd);+ if (ret)+ return ret;+ dprintk(1, "%s called std_set %d dev_state %d\n", __func__, dev->std_set_in_tuner_core, dev->dev_state);
Em Wed, 03 Feb 2016 21:03:46 -0700
Shuah Khan [off-list ref] escreveu:
quoted hunk
Change au0828 to use Managed Media Controller API to
share media device and coordinate creating/deleting
the shared media device with the snd-usb-audio driver.
The shared media device is created as device resource
of the parent usb device of the two drivers.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/usb/au0828/au0828-core.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
Why did you change that? On some boards, udev->product doesn't reflect
the brand name, but have just some random generic data.
Also, as the other logs associated with the device uses dev->board.name,
we want the media controller to use the same name here. Ok, if this
is null, we could use udev->product as a replacement.
Em Thu, 4 Feb 2016 08:05:00 -0200
Mauro Carvalho Chehab [off-list ref] escreveu:
Em Wed, 03 Feb 2016 21:03:46 -0700
Shuah Khan [off-list ref] escreveu:
quoted
Change au0828 to use Managed Media Controller API to
share media device and coordinate creating/deleting
the shared media device with the snd-usb-audio driver.
The shared media device is created as device resource
of the parent usb device of the two drivers.
Signed-off-by: Shuah Khan <redacted>
---
drivers/media/usb/au0828/au0828-core.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
Why did you change that? On some boards, udev->product doesn't reflect
the brand name, but have just some random generic data.
Also, as the other logs associated with the device uses dev->board.name,
we want the media controller to use the same name here. Ok, if this
is null, we could use udev->product as a replacement.
Ah, looking at patch 15/22, I understood why you wanted the above
change: if the device is registered first by ALSA, it won't have a
dev->board.name.
It looks OK to change from dev->board.name to udev->product then,
but please document the reason for it at the patch description.
Regards,
Mauro
Em Wed, 03 Feb 2016 21:03:52 -0700
Shuah Khan [off-list ref] escreveu:
quoted hunk
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 <redacted>
---
drivers/media/usb/au0828/au0828-core.c | 149 +++++++++++++++++++++++++++++++++
drivers/media/usb/au0828/au0828.h | 3 +
2 files changed, 152 insertions(+)
@@ -282,6 +282,7 @@ static int au0828_create_media_graph(struct au0828_dev *dev)return-EINVAL;if(tuner){+dev->tuner=tuner;/* create tuner to decoder link in deactivated state */ret=media_create_pad_link(tuner,TUNER_PAD_OUTPUT,decoder,0,0);
@@ -373,6 +374,150 @@ void au0828_media_graph_notify(struct media_entity *new, void *notify_data)#endif}+staticintau0828_enable_source(structmedia_entity*entity,+structmedia_pipeline*pipe)+{+#ifdef CONFIG_MEDIA_CONTROLLER+structmedia_entity*source;+structmedia_entity*sink;+structmedia_link*link,*found_link=NULL;+intret=0;+structmedia_device*mdev=entity->graph_obj.mdev;+structau0828_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;+gotoend;+}
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?
Em Wed, 03 Feb 2016 21:03:53 -0700
Shuah Khan [off-list ref] escreveu:
quoted hunk
Change ALSA driver to use Media Controller API to
share media resources with DVB and V4L2 drivers
on a AU0828 media device. Media Controller specific
initialization is done after sound card is registered.
ALSA creates Media interface and entity function graph
nodes for Control, Mixer, PCM Playback, and PCM Capture
devices.
snd_usb_hw_params() will call Media Controller enable
source handler interface to request the media resource.
If resource request is granted, it will release it from
snd_usb_hw_free(). If resource is busy, -EBUSY is returned.
Media specific cleanup is done in usb_audio_disconnect().
Signed-off-by: Shuah Khan <redacted>
---
sound/usb/Kconfig | 4 +
sound/usb/Makefile | 2 +
sound/usb/card.c | 14 +++
sound/usb/card.h | 1 +
sound/usb/media.c | 319 +++++++++++++++++++++++++++++++++++++++++++++++
sound/usb/media.h | 72 +++++++++++
sound/usb/mixer.h | 1 +
sound/usb/pcm.c | 28 ++++-
sound/usb/quirks-table.h | 1 +
sound/usb/stream.c | 2 +
sound/usb/usbaudio.h | 3 +
11 files changed, 442 insertions(+), 5 deletions(-)
create mode 100644 sound/usb/media.c
create mode 100644 sound/usb/media.h
@@ -156,6 +156,7 @@ struct snd_usb_substream {}dsd_dop;booltrigger_tstamp_pending_update;/* trigger timestamp being updated from initial estimate */+void*media_ctl;};
This is Takashi's call, but I would prefer to avoid using a void *
here. Fortunately, GCC is smart enough to handle struct pointers
even if the header doesn't have a full declaration of the struct.
So, I would change the above to:
struct media_device;
struct snd_usb_substream {
...
struct media_device *media_ctl;
};
This way, we ensure strong typecast checks when compiling the code under
sound/usb/media.c, while not needing to include media_device.h header here.
@@ -0,0 +1,319 @@+/*+*media.c-MediaControllerspecificALSAdrivercode+*+*Copyright(c)2015ShuahKhan<shuahkh@osg.samsung.com>+*Copyright(c)2015SamsungElectronicsCo.,Ltd.+*+*ThisfileisreleasedundertheGPLv2.+*/++/*+*ThisfileaddsMediaControllersupporttoALSAdriver+*tousetheMediaControllerAPItosharetunerwithDVB+*andV4L2driversthatcontrolmediadevice.Mediadevice+*iscreatedbasedonexistingquirksframework.Usingthis+*approach,themediacontrollerAPIusagecanbeaddedfor+*aspecificdevice.+*/++#include<linux/init.h>+#include<linux/list.h>+#include<linux/mutex.h>+#include<linux/slab.h>+#include<linux/usb.h>++#include<sound/pcm.h>+#include<sound/core.h>++#include"usbaudio.h"+#include"card.h"+#include"mixer.h"+#include"media.h"++staticintmedia_enable_source(structmedia_ctl*mctl)+{+if(mctl&&mctl->media_dev->enable_source)+returnmctl->media_dev->enable_source(&mctl->media_entity,+&mctl->media_pipe);+return0;+}++staticvoidmedia_disable_source(structmedia_ctl*mctl)+{+if(mctl&&mctl->media_dev->disable_source)+mctl->media_dev->disable_source(&mctl->media_entity);+}++intmedia_stream_init(structsnd_usb_substream*subs,structsnd_pcm*pcm,+intstream)+{+structmedia_device*mdev;+structmedia_ctl*mctl;+structdevice*pcm_dev=&pcm->streams[stream].dev;+u32intf_type;+intret=0;+u16mixer_pad;+structmedia_entity*entity;++mdev=subs->stream->chip->media_dev;+if(!mdev)+return-ENODEV;++if(subs->media_ctl)+return0;++/* allocate media_ctl */+mctl=kzalloc(sizeof(*mctl),GFP_KERNEL);+if(!mctl)+return-ENOMEM;++mctl->media_dev=mdev;+if(stream==SNDRV_PCM_STREAM_PLAYBACK){+intf_type=MEDIA_INTF_T_ALSA_PCM_PLAYBACK;+mctl->media_entity.function=MEDIA_ENT_F_AUDIO_PLAYBACK;+mctl->media_pad.flags=MEDIA_PAD_FL_SOURCE;+mixer_pad=1;+}else{+intf_type=MEDIA_INTF_T_ALSA_PCM_CAPTURE;+mctl->media_entity.function=MEDIA_ENT_F_AUDIO_CAPTURE;+mctl->media_pad.flags=MEDIA_PAD_FL_SINK;+mixer_pad=2;+}+mctl->media_entity.name=pcm->name;+media_entity_pads_init(&mctl->media_entity,1,&mctl->media_pad);+ret=media_device_register_entity(mctl->media_dev,+&mctl->media_entity);+if(ret)+gotoerr1;++mctl->intf_devnode=media_devnode_create(mdev,intf_type,0,+MAJOR(pcm_dev->devt),+MINOR(pcm_dev->devt));+if(!mctl->intf_devnode){+ret=-ENOMEM;+gotoerr2;+}+mctl->intf_link=media_create_intf_link(&mctl->media_entity,+&mctl->intf_devnode->intf,+MEDIA_LNK_FL_ENABLED);+if(!mctl->intf_link){+ret=-ENOMEM;+gotoerr3;+}++/* create link between mixer and audio */+media_device_for_each_entity(entity,mdev){+switch(entity->function){+caseMEDIA_ENT_F_AUDIO_MIXER:+ret=media_create_pad_link(entity,mixer_pad,+&mctl->media_entity,0,+MEDIA_LNK_FL_ENABLED);+if(ret)+gotoerr4;+break;+}+}++subs->media_ctl=mctl;+return0;++err4:+media_remove_intf_link(mctl->intf_link);+err3:+media_devnode_remove(mctl->intf_devnode);+err2:+media_device_unregister_entity(&mctl->media_entity);+err1:+kfree(mctl);+returnret;+}++voidmedia_stream_delete(structsnd_usb_substream*subs)+{+structmedia_ctl*mctl=subs->media_ctl;++if(mctl&&mctl->media_dev){+structmedia_device*mdev;++mdev=subs->stream->chip->media_dev;+if(mdev&&media_devnode_is_registered(&mdev->devnode)){+media_devnode_remove(mctl->intf_devnode);+media_device_unregister_entity(&mctl->media_entity);+media_entity_cleanup(&mctl->media_entity);+}+kfree(mctl);+subs->media_ctl=NULL;+}+}++intmedia_start_pipeline(structsnd_usb_substream*subs)+{+structmedia_ctl*mctl=subs->media_ctl;++if(mctl)+returnmedia_enable_source(mctl);+return0;+}++voidmedia_stop_pipeline(structsnd_usb_substream*subs)+{+structmedia_ctl*mctl=subs->media_ctl;++if(mctl)+media_disable_source(mctl);+}++intmedia_mixer_init(structsnd_usb_audio*chip)+{+structdevice*ctl_dev=&chip->card->ctl_dev;+structmedia_intf_devnode*ctl_intf;+structusb_mixer_interface*mixer;+structmedia_device*mdev=chip->media_dev;+structmedia_mixer_ctl*mctl;+u32intf_type=MEDIA_INTF_T_ALSA_CONTROL;+intret;++if(!mdev)+return-ENODEV;++ctl_intf=chip->ctl_intf_media_devnode;+if(!ctl_intf){+ctl_intf=(void*)media_devnode_create(mdev,+intf_type,0,+MAJOR(ctl_dev->devt),+MINOR(ctl_dev->devt));+if(!ctl_intf)+return-ENOMEM;+chip->ctl_intf_media_devnode=ctl_intf;+}++list_for_each_entry(mixer,&chip->mixer_list,list){++if(mixer->media_mixer_ctl)+continue;++/* allocate media_mixer_ctl */+mctl=kzalloc(sizeof(*mctl),GFP_KERNEL);+if(!mctl)+return-ENOMEM;++mctl->media_dev=mdev;+mctl->media_entity.function=MEDIA_ENT_F_AUDIO_MIXER;+mctl->media_entity.name=chip->card->mixername;+mctl->media_pad[0].flags=MEDIA_PAD_FL_SINK;+mctl->media_pad[1].flags=MEDIA_PAD_FL_SOURCE;+mctl->media_pad[2].flags=MEDIA_PAD_FL_SOURCE;+media_entity_pads_init(&mctl->media_entity,MEDIA_MIXER_PAD_MAX,+mctl->media_pad);+ret=media_device_register_entity(mctl->media_dev,+&mctl->media_entity);+if(ret){+kfree(mctl);+returnret;+}++mctl->intf_link=media_create_intf_link(&mctl->media_entity,+&ctl_intf->intf,+MEDIA_LNK_FL_ENABLED);+if(!mctl->intf_link){+media_device_unregister_entity(&mctl->media_entity);+media_entity_cleanup(&mctl->media_entity);+kfree(mctl);+return-ENOMEM;+}+mctl->intf_devnode=ctl_intf;+mixer->media_mixer_ctl=mctl;+}+return0;+}++staticvoidmedia_mixer_delete(structsnd_usb_audio*chip)+{+structusb_mixer_interface*mixer;+structmedia_device*mdev=chip->media_dev;++if(!mdev)+return;++list_for_each_entry(mixer,&chip->mixer_list,list){+structmedia_mixer_ctl*mctl;++mctl=mixer->media_mixer_ctl;+if(!mixer->media_mixer_ctl)+continue;++if(media_devnode_is_registered(&mdev->devnode)){+media_device_unregister_entity(&mctl->media_entity);+media_entity_cleanup(&mctl->media_entity);+}+kfree(mctl);+mixer->media_mixer_ctl=NULL;+}+if(media_devnode_is_registered(&mdev->devnode))+media_devnode_remove(chip->ctl_intf_media_devnode);+chip->ctl_intf_media_devnode=NULL;+}++intmedia_device_create(structsnd_usb_audio*chip,+structusb_interface*iface)+{+structmedia_device*mdev;+structusb_device*usbdev=interface_to_usbdev(iface);+intret;++mdev=media_device_get_devres(&usbdev->dev);+if(!mdev)+return-ENOMEM;+if(!mdev->dev){+/* register media device */+mdev->dev=&usbdev->dev;+if(usbdev->product)+strlcpy(mdev->model,usbdev->product,+sizeof(mdev->model));+if(usbdev->serial)+strlcpy(mdev->serial,usbdev->serial,+sizeof(mdev->serial));+strcpy(mdev->bus_info,usbdev->devpath);+mdev->hw_revision=le16_to_cpu(usbdev->descriptor.bcdDevice);+media_device_init(mdev);+}+if(!media_devnode_is_registered(&mdev->devnode)){+ret=media_device_register(mdev);+if(ret){+dev_err(&usbdev->dev,+"Couldn't register media device. Error: %d\n",+ret);+returnret;+}+}++/* save media device - avoid lookups */+chip->media_dev=mdev;++/* Create media entities for mixer and control dev */+ret=media_mixer_init(chip);+if(ret){+dev_err(&usbdev->dev,+"Couldn't create media mixer entities. Error: %d\n",+ret);++/* clear saved media_dev */+chip->media_dev=NULL;++returnret;+}+return0;+}++voidmedia_device_delete(structsnd_usb_audio*chip)+{+structmedia_device*mdev=chip->media_dev;++media_mixer_delete(chip);++if(mdev){+if(media_devnode_is_registered(&mdev->devnode))+media_device_unregister(mdev);+chip->media_dev=NULL;+}+}
Em Wed, 03 Feb 2016 21:03:54 -0700
Shuah Khan [off-list ref] escreveu:
media_device_unregister() checks if the media device
is registered or not as the first step. However, the
MEDIA_FLAG_REGISTERED bit doesn't get cleared until
the end leaving a large window for two drivers to
attempt media device unregister.
The above leads to general protection faults when
device is removed.
Fix the problem with two phase media device unregister.
Add a new interface media_devnode_start_unregister()
to clear the MEDIA_FLAG_REGISTERED bit. Change
media_device_unregister() call this interface to mark
the start of unregister. This will ensure that media
device unregister is done only once.
Hmm... it sounds simpler to use a kref at device register
and when other drivers need instantiate media_dev. We can
then do a kref_put() at unregister, and run the actual
unregister code and media_ctl kfree only when kref is
decremented on all drivers.
Regards,
Mauro
@@ -747,17 +747,17 @@ void media_device_unregister(struct media_device *mdev)structmedia_entity*next;structmedia_interface*intf,*tmp_intf;structmedia_entity_notify*notify,*nextp;+intret;if(mdev==NULL)return;-spin_lock(&mdev->lock);--/* Check if mdev was ever registered at all */-if(!media_devnode_is_registered(&mdev->devnode)){-spin_unlock(&mdev->lock);+/* Start unregister - continue if necessary */+ret=media_devnode_start_unregister(&mdev->devnode);+if(ret)return;-}++spin_lock(&mdev->lock);/* Remove all entities from the media device */list_for_each_entry_safe(entity,next,&mdev->entities,graph_obj.list)
@@ -272,15 +272,20 @@ error:returnret;}-voidmedia_devnode_unregister(structmedia_devnode*mdev)+int__must_checkmedia_devnode_start_unregister(structmedia_devnode*mdev){-/* Check if mdev was ever registered at all */-if(!media_devnode_is_registered(mdev))-return;-mutex_lock(&media_devnode_lock);+if(!media_devnode_is_registered(mdev)){+mutex_unlock(&media_devnode_lock);+return-EINVAL;+}clear_bit(MEDIA_FLAG_REGISTERED,&mdev->flags);mutex_unlock(&media_devnode_lock);+return0;+}++voidmedia_devnode_unregister(structmedia_devnode*mdev)+{device_unregister(&mdev->dev);}
@@ -156,6 +156,7 @@ struct snd_usb_substream {}dsd_dop;booltrigger_tstamp_pending_update;/* trigger timestamp being updated from initial estimate */+void*media_ctl;};
This is Takashi's call, but I would prefer to avoid using a void *
here. Fortunately, GCC is smart enough to handle struct pointers
even if the header doesn't have a full declaration of the struct.
So, I would change the above to:
struct media_device;
struct snd_usb_substream {
...
struct media_device *media_ctl;
};
This way, we ensure strong typecast checks when compiling the code under
sound/usb/media.c, while not needing to include media_device.h header here.
Agreed, this is no big merit to make it a void pointer.
Takashi
Em Wed, 03 Feb 2016 21:03:32 -0700
Shuah Khan [off-list ref] escreveu:
This patch series updates ALSA driver, and au0828 core
driver to use Managed Media controller API and Media
Controller API to share media resource (tuner).
Please see my comments to patch 20. I guess this series break support
for the digital-only boards below (where .has_analog is not set):
[AU0828_BOARD_HAUPPAUGE_HVR950Q_MXL] = {
.name = "Hauppauge HVR950Q rev xxF8",
.tuner_type = TUNER_XC5000,
.tuner_addr = 0x61,
.i2c_clk_divider = AU0828_I2C_CLK_250KHZ,
},
[AU0828_BOARD_DVICO_FUSIONHDTV7] = {
.name = "DViCO FusionHDTV USB",
.tuner_type = TUNER_XC5000,
.tuner_addr = 0x61,
.i2c_clk_divider = AU0828_I2C_CLK_250KHZ,
},
[AU0828_BOARD_HAUPPAUGE_WOODBURY] = {
.name = "Hauppauge Woodbury",
.tuner_type = TUNER_NXP_TDA18271,
.tuner_addr = 0x60,
.i2c_clk_divider = AU0828_I2C_CLK_250KHZ,
},
None of this boards have V4L2. So, only the DVB part of the driver
should be enabled. Well, the DVB core won't be creating a tuner
subdev. So, calling au0828_enable_source() (and disable source) would
return -ENODEV, causing the driver to fail.
This Patch v2 series is based on linux_media master.
This work addresses Mauro and Takashi's comments.
It is in a way better shape, but there are still several things I
noticed :)
Please see my per-patch review.
Composite or S-Video connector is the input case
is still under discussion and isn't addressed in
this series.
You need at least address pure DVB boards at patch 20 and
add there a FIXME for the composite/svideo connector pending
issue.
Patches 3 and 4 need documentation updates and will
be sent later this week.
Please send documentation at the same series. Nobody wants to see the
documentation warnings when checking at the main media development
branch or at -next. Also, it helps to better understand the changes.
Regards,
Mauro
Shuah Khan (22):
uapi/media.h: Declare interface types for ALSA
media: Add ALSA Media Controller function entities
media: Media Controller register/unregister entity_notify API
media: Media Controller enable/disable source handler API
media: Media Controller export non locking __media_entity_setup_link()
media: Media Controller non-locking
__media_entity_pipeline_start/stop()
media: v4l-core add enable/disable source common interfaces
media: Move au8522_media_pads enum to au8522.h from au8522_priv.h
media: au8522 change to create MC pad for ALSA Audio Out
media: Change v4l-core to check if source is free
media: dvb-frontend invoke enable/disable_source handlers
media: au0828 video remove au0828_enable_analog_tuner()
media: au0828 video change to use v4l_enable_media_source()
media: au0828 change to use Managed Media Controller API
media: au0828 handle media_init and media_register window
media: au0828 create tuner to decoder link in disabled state
media: au0828 disable tuner to demod link
media: au0828 Use au8522_media_pads enum for pad defines
media: au0828-core register entity_notify hook
media: au0828 add enable, disable source handlers
sound/usb: Use Media Controller API to share media resources
media: Ensure media device unregister is done only once
drivers/media/dvb-core/dvb_frontend.c | 139 ++----------
drivers/media/dvb-core/dvb_frontend.h | 3 +
drivers/media/dvb-frontends/au8522.h | 8 +
drivers/media/dvb-frontends/au8522_decoder.c | 1 +
drivers/media/dvb-frontends/au8522_priv.h | 8 -
drivers/media/media-device.c | 73 +++++-
drivers/media/media-devnode.c | 15 +-
drivers/media/media-entity.c | 51 ++++-
drivers/media/usb/au0828/au0828-core.c | 272 +++++++++++++++++++++--
drivers/media/usb/au0828/au0828-video.c | 75 +------
drivers/media/usb/au0828/au0828.h | 4 +
drivers/media/v4l2-core/Makefile | 2 +-
drivers/media/v4l2-core/v4l2-fh.c | 2 +
drivers/media/v4l2-core/v4l2-ioctl.c | 30 +++
drivers/media/v4l2-core/v4l2-mc.c | 60 +++++
drivers/media/v4l2-core/videobuf2-core.c | 4 +
include/media/media-device.h | 44 ++++
include/media/media-devnode.h | 17 ++
include/media/media-entity.h | 12 +
include/media/v4l2-dev.h | 1 +
include/media/v4l2-mc.h | 52 +++++
include/uapi/linux/media.h | 33 +++
sound/usb/Kconfig | 4 +
sound/usb/Makefile | 2 +
sound/usb/card.c | 14 ++
sound/usb/card.h | 1 +
sound/usb/media.c | 319 +++++++++++++++++++++++++++
sound/usb/media.h | 72 ++++++
sound/usb/mixer.h | 1 +
sound/usb/pcm.c | 28 ++-
sound/usb/quirks-table.h | 1 +
sound/usb/stream.c | 2 +
sound/usb/usbaudio.h | 3 +
33 files changed, 1117 insertions(+), 236 deletions(-)
create mode 100644 drivers/media/v4l2-core/v4l2-mc.c
create mode 100644 sound/usb/media.c
create mode 100644 sound/usb/media.h
From: kbuild test robot <hidden> Date: 2016-02-07 11:26:35
Hi Shuah,
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on next-20160205]
[cannot apply to v4.5-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Shuah-Khan/Sharing-media-resources-across-ALSA-and-au0828-drivers/20160204-121414
base: git://linuxtv.org/media_tree.git master
config: i386-randconfig-b0-02071259 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
sound/built-in.o: In function `media_stream_init':
quoted
(.text+0x3a043): undefined reference to `media_entity_pads_init'
sound/built-in.o: In function `media_stream_init':
quoted
(.text+0x3a04d): undefined reference to `media_device_register_entity'
sound/built-in.o: In function `media_stream_init':
quoted
(.text+0x3a080): undefined reference to `media_devnode_create'
sound/built-in.o: In function `media_stream_init':
quoted
(.text+0x3a09c): undefined reference to `media_create_intf_link'
sound/built-in.o: In function `media_stream_init':
quoted
(.text+0x3a0f4): undefined reference to `media_create_pad_link'
sound/built-in.o: In function `media_stream_init':
quoted
(.text+0x3a105): undefined reference to `media_remove_intf_link'
sound/built-in.o: In function `media_stream_init':
quoted
(.text+0x3a113): undefined reference to `media_devnode_remove'
sound/built-in.o: In function `media_stream_init':
quoted
(.text+0x3a121): undefined reference to `media_device_unregister_entity'
sound/built-in.o: In function `media_stream_delete':
(.text+0x3a1f4): undefined reference to `media_devnode_remove'
sound/built-in.o: In function `media_stream_delete':
(.text+0x3a1fc): undefined reference to `media_device_unregister_entity'
sound/built-in.o: In function `media_mixer_init':
(.text+0x3a347): undefined reference to `media_entity_pads_init'
sound/built-in.o: In function `media_mixer_init':
(.text+0x3a351): undefined reference to `media_device_register_entity'
sound/built-in.o: In function `media_mixer_init':
(.text+0x3a369): undefined reference to `media_create_intf_link'
sound/built-in.o: In function `media_mixer_init':
(.text+0x3a39e): undefined reference to `media_device_unregister_entity'
sound/built-in.o: In function `media_mixer_init':
(.text+0x3a3da): undefined reference to `media_devnode_create'
sound/built-in.o: In function `media_device_create':
quoted
(.text+0x3a424): undefined reference to `media_device_get_devres'
sound/built-in.o: In function `media_device_create':
quoted
(.text+0x3a475): undefined reference to `__media_device_register'
sound/built-in.o: In function `media_device_create':
quoted
(.text+0x3a526): undefined reference to `media_device_init'
sound/built-in.o: In function `media_device_delete':
(.text+0x3a5b1): undefined reference to `media_device_unregister_entity'
sound/built-in.o: In function `media_device_delete':
(.text+0x3a5f7): undefined reference to `media_devnode_remove'
sound/built-in.o: In function `media_device_delete':
quoted
(.text+0x3a603): undefined reference to `media_device_unregister'