[alsa-devel] [PATCH MC Next Gen v3 0/6] Add mixer and control interface media entities
This patch v3 series adds the following:
A fix to media_stream_delete() to remove intf devnode.
Add support for creating MEDIA_ENT_F_AUDIO_MIXER entity for each mixer and a MEDIA_INTF_T_ALSA_CONTROL control interface entity that links to mixer entities. MEDIA_INTF_T_ALSA_CONTROL entity corresponds to the control device for the card.
Change au0828_create_media_graph() to create pad link between MEDIA_ENT_F_AUDIO_MIXER entity and decoder's AU8522_PAD_AUDIO_OUT. With mixer entity now linked to decoder, change to link MEDIA_ENT_F_AUDIO_CAPTURE to mixer's source pad
In addition, this patch series consists of fixes to compile warnings found by kbuild bot, and an additional fix in media stream cleanup path.
Changes since v2:
1. Addressed Takashi's comments on the following ALSA patch: sound/usb: Create media mixer function and control interface entities 2. Include patches to fix compile warnings found by kbuild bot, and an additional fix in media stream cleanup path. 3. No changes to patches 1 and 3. 4. Changed patches: 2 5. New patches: 4,5, and 6
Changes since v1: 1. Included the fix to media_stream_delete() which was by mistake not included in the patch v1. 2. Fixed mixer to decoder link and mixer to capture node links based on Mauro Chehab's review comments on the media graph.
This patch series is dependent on an earlier patch series-es:
Patch series v2: https://www.mail-archive.com/linux-media@vger.kernel.org/msg93084.html
Patch series v1: Update ALSA, and au0828 drivers to use Managed Media Controller API: https://www.mail-archive.com/linux-media@vger.kernel.org/msg92752.html
Please find the media graphs for ALSA work with mixer and control interface nodes: (these are generated using mc_next_gen_test tool.
https://drive.google.com/folderview?id=0B0NIL0BQg-AlLWE3SzAxazBJWm8&usp=...
v4l2-utils patch adds mixer node support mc_next_gen_test tool: https://www.mail-archive.com/linux-media@vger.kernel.org/msg93086.html
Shuah Khan (6): sound/usb: Fix media_stream_delete() to remove intf devnode sound/usb: Create media mixer function and control interface entities media: au0828 create link between ALSA Mixer and decoder sound/usb: Fix media_stream_init() and media_stream_delete() error paths media: Fix compile warning when CONFIG_MEDIA_CONTROLLER_DVB is disabled media: au0828-core fix ignoring return value errors
drivers/media/dvb-core/dvb_frontend.c | 2 +- drivers/media/usb/au0828/au0828-core.c | 38 +++++++++--- drivers/media/usb/au0828/au0828.h | 1 + sound/usb/card.c | 5 ++ sound/usb/media.c | 102 ++++++++++++++++++++++++++++++++- sound/usb/media.h | 20 +++++++ sound/usb/mixer.h | 1 + sound/usb/usbaudio.h | 1 + 8 files changed, 158 insertions(+), 12 deletions(-)
media_stream_delete() doesn't remove intf_devnode. Fix it to call media_devnode_remove() to remove the intf_devnode.
Signed-off-by: Shuah Khan shuahkh@osg.samsung.com --- sound/usb/media.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/usb/media.c b/sound/usb/media.c index 9b455ad..0cbfee6 100644 --- a/sound/usb/media.c +++ b/sound/usb/media.c @@ -173,6 +173,7 @@ void media_stream_delete(struct snd_usb_substream *subs) mdev = media_device_find_devres(&subs->dev->dev); if (mdev) { media_entity_remove_links(&mctl->media_entity); + media_devnode_remove(mctl->intf_devnode); media_device_unregister_entity(&mctl->media_entity); media_entity_cleanup(&mctl->media_entity); }
Add support for creating MEDIA_ENT_F_AUDIO_MIXER entity for each mixer and a MEDIA_INTF_T_ALSA_CONTROL control interface entity that links to mixer entities. MEDIA_INTF_T_ALSA_CONTROL entity corresponds to the control device for the card.
Signed-off-by: Shuah Khan shuahkh@osg.samsung.com --- sound/usb/card.c | 5 +++ sound/usb/media.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++ sound/usb/media.h | 20 ++++++++++++ sound/usb/mixer.h | 1 + sound/usb/usbaudio.h | 1 + 5 files changed, 119 insertions(+)
diff --git a/sound/usb/card.c b/sound/usb/card.c index 469d2bf..d004cb4 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -560,6 +560,9 @@ static int usb_audio_probe(struct usb_interface *intf, if (err < 0) goto __error;
+ /* Create media entities for mixer and control dev */ + media_mixer_init(chip); + usb_chip[chip->index] = chip; chip->num_interfaces++; chip->probing = 0; @@ -616,6 +619,8 @@ static void usb_audio_disconnect(struct usb_interface *intf) list_for_each(p, &chip->midi_list) { snd_usbmidi_disconnect(p); } + /* delete mixer media resources */ + media_mixer_delete(chip); /* release mixer resources */ list_for_each_entry(mixer, &chip->mixer_list, list) { snd_usb_mixer_disconnect(mixer); diff --git a/sound/usb/media.c b/sound/usb/media.c index 0cbfee6..6df346a 100644 --- a/sound/usb/media.c +++ b/sound/usb/media.c @@ -199,4 +199,96 @@ void media_stop_pipeline(struct snd_usb_substream *subs) if (mctl) media_disable_source(mctl); } + +int media_mixer_init(struct snd_usb_audio *chip) +{ + struct device *ctl_dev = &chip->card->ctl_dev; + struct media_intf_devnode *ctl_intf; + struct usb_mixer_interface *mixer; + struct media_device *mdev; + struct media_mixer_ctl *mctl; + u32 intf_type = MEDIA_INTF_T_ALSA_CONTROL; + int ret; + + mdev = media_device_find_devres(&chip->dev->dev); + 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_ctl */ + mctl = kzalloc(sizeof(struct media_ctl), 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_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); + return ret; + } + + mctl->intf_link = media_create_intf_link(&mctl->media_entity, + &ctl_intf->intf, + MEDIA_LNK_FL_ENABLED); + if (!mctl->intf_link) { + media_entity_remove_links(&mctl->media_entity); + 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 = (void *) mctl; + } + return 0; +} + +void media_mixer_delete(struct snd_usb_audio *chip) +{ + struct usb_mixer_interface *mixer; + struct media_device *mdev; + + mdev = media_device_find_devres(&chip->dev->dev); + if (!mdev) + return; + + list_for_each_entry(mixer, &chip->mixer_list, list) { + struct media_mixer_ctl *mctl; + + mctl = (struct media_mixer_ctl *) mixer->media_mixer_ctl; + if (!mixer->media_mixer_ctl) + continue; + + media_entity_remove_links(&mctl->media_entity); + media_device_unregister_entity(&mctl->media_entity); + media_entity_cleanup(&mctl->media_entity); + kfree(mctl); + mixer->media_mixer_ctl = NULL; + } + media_devnode_remove(chip->ctl_intf_media_devnode); +} + #endif diff --git a/sound/usb/media.h b/sound/usb/media.h index cdcfb80..8268e52 100644 --- a/sound/usb/media.h +++ b/sound/usb/media.h @@ -20,6 +20,7 @@ #ifdef USE_MEDIA_CONTROLLER #include <media/media-device.h> #include <media/media-entity.h> +#include <sound/asound.h>
struct media_ctl { struct media_device *media_dev; @@ -30,6 +31,21 @@ struct media_ctl { struct media_pipeline media_pipe; };
+/* One source pad each for SNDRV_PCM_STREAM_CAPTURE and + * SNDRV_PCM_STREAM_PLAYBACK. One for sink pad to link + * to AUDIO Source +*/ +#define MEDIA_MIXER_PAD_MAX (SNDRV_PCM_STREAM_LAST + 2) + +struct media_mixer_ctl { + struct media_device *media_dev; + struct media_entity media_entity; + struct media_intf_devnode *intf_devnode; + struct media_link *intf_link; + struct media_pad media_pad[MEDIA_MIXER_PAD_MAX]; + struct media_pipeline media_pipe; +}; + int media_device_init(struct snd_usb_audio *chip, struct usb_interface *iface); void media_device_delete(struct usb_interface *iface); int media_stream_init(struct snd_usb_substream *subs, struct snd_pcm *pcm, @@ -37,6 +53,8 @@ int media_stream_init(struct snd_usb_substream *subs, struct snd_pcm *pcm, void media_stream_delete(struct snd_usb_substream *subs); int media_start_pipeline(struct snd_usb_substream *subs); void media_stop_pipeline(struct snd_usb_substream *subs); +int media_mixer_init(struct snd_usb_audio *chip); +void media_mixer_delete(struct snd_usb_audio *chip); #else static inline int media_device_init(struct snd_usb_audio *chip, struct usb_interface *iface) @@ -49,5 +67,7 @@ static inline void media_stream_delete(struct snd_usb_substream *subs) { } static inline int media_start_pipeline(struct snd_usb_substream *subs) { return 0; } static inline void media_stop_pipeline(struct snd_usb_substream *subs) { } +static int media_mixer_init(struct snd_usb_audio *chip) { return 0; } +static void media_mixer_delete(struct snd_usb_audio *chip) { } #endif #endif /* __MEDIA_H */ diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h index d3268f0..9d62a9e 100644 --- a/sound/usb/mixer.h +++ b/sound/usb/mixer.h @@ -22,6 +22,7 @@ struct usb_mixer_interface { struct urb *rc_urb; struct usb_ctrlrequest *rc_setup_packet; u8 rc_buffer[6]; + void *media_mixer_ctl; };
#define MAX_CHANNELS 16 /* max logical channels */ diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index c2dbf1d..b2be7c3 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -59,6 +59,7 @@ struct snd_usb_audio { bool autoclock; /* from the 'autoclock' module param */
struct usb_host_interface *ctrl_intf; /* the audio control interface */ + void *ctl_intf_media_devnode; };
#define usb_audio_err(chip, fmt, args...) \
Change au0828_create_media_graph() to create pad link between MEDIA_ENT_F_AUDIO_MIXER entity and decoder's AU8522_PAD_AUDIO_OUT. With mixer entity now linked to decoder, change to link MEDIA_ENT_F_AUDIO_CAPTURE to mixer's source pad.
Signed-off-by: Shuah Khan shuahkh@osg.samsung.com --- drivers/media/usb/au0828/au0828-core.c | 17 ++++++++++++++--- drivers/media/usb/au0828/au0828.h | 1 + 2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c index 7af5d0d..3ef6fee 100644 --- a/drivers/media/usb/au0828/au0828-core.c +++ b/drivers/media/usb/au0828/au0828-core.c @@ -225,6 +225,7 @@ void au0828_create_media_graph(struct media_entity *new, void *notify_data) struct media_entity *entity; struct media_entity *tuner = NULL, *decoder = NULL; struct media_entity *audio_capture = NULL; + struct media_entity *mixer = NULL; int i, ret;
if (!mdev) @@ -245,6 +246,9 @@ void au0828_create_media_graph(struct media_entity *new, void *notify_data) case MEDIA_ENT_F_AUDIO_CAPTURE: audio_capture = entity; break; + case MEDIA_ENT_F_AUDIO_MIXER: + mixer = entity; + break; } }
@@ -309,13 +313,20 @@ void au0828_create_media_graph(struct media_entity *new, void *notify_data) } }
- if (audio_capture && !dev->audio_capture_linked) { - ret = media_create_pad_link(decoder, AU8522_PAD_AUDIO_OUT, - audio_capture, 0, + if (mixer && audio_capture && !dev->audio_capture_linked) { + ret = media_create_pad_link(mixer, 1, audio_capture, 0, MEDIA_LNK_FL_ENABLED); if (ret == 0) dev->audio_capture_linked = 1; } + + if (mixer && !dev->mixer_linked) { + ret = media_create_pad_link(decoder, AU8522_PAD_AUDIO_OUT, + mixer, 0, + MEDIA_LNK_FL_ENABLED); + if (ret == 0) + dev->mixer_linked = 1; + } #endif }
diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h index 2f4d597..6dd81b2 100644 --- a/drivers/media/usb/au0828/au0828.h +++ b/drivers/media/usb/au0828/au0828.h @@ -288,6 +288,7 @@ struct au0828_dev { bool vdev_linked; bool vbi_linked; bool audio_capture_linked; + bool mixer_linked; struct media_link *active_link; struct media_entity *active_link_owner; #endif
Fix media_stream_init() to free memory in error paths. Fix media_stream_delete() to remove unnecessary set media_dev pointer to null before free mctl.
Signed-off-by: Shuah Khan shuahkh@osg.samsung.com --- sound/usb/media.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/sound/usb/media.c b/sound/usb/media.c index 6df346a..bebe27b 100644 --- a/sound/usb/media.c +++ b/sound/usb/media.c @@ -127,7 +127,6 @@ int media_stream_init(struct snd_usb_substream *subs, struct snd_pcm *pcm, if (!mctl) return -ENOMEM;
- subs->media_ctl = (void *) mctl; mctl->media_dev = mdev; if (stream == SNDRV_PCM_STREAM_PLAYBACK) { intf_type = MEDIA_INTF_T_ALSA_PCM_PLAYBACK; @@ -143,13 +142,16 @@ int media_stream_init(struct snd_usb_substream *subs, struct snd_pcm *pcm, media_entity_init(&mctl->media_entity, 1, &mctl->media_pad); ret = media_device_register_entity(mctl->media_dev, &mctl->media_entity); - if (ret) + if (ret) { + kfree(mctl); return ret; + } mctl->intf_devnode = media_devnode_create(mdev, intf_type, 0, MAJOR(pcm_dev->devt), MINOR(pcm_dev->devt)); if (!mctl->intf_devnode) { media_device_unregister_entity(&mctl->media_entity); + kfree(mctl); return -ENOMEM; } mctl->intf_link = media_create_intf_link(&mctl->media_entity, @@ -158,8 +160,10 @@ int media_stream_init(struct snd_usb_substream *subs, struct snd_pcm *pcm, if (!mctl->intf_link) { media_devnode_remove(mctl->intf_devnode); media_device_unregister_entity(&mctl->media_entity); + kfree(mctl); return -ENOMEM; } + subs->media_ctl = (void *) mctl; return 0; }
@@ -177,7 +181,6 @@ void media_stream_delete(struct snd_usb_substream *subs) media_device_unregister_entity(&mctl->media_entity); media_entity_cleanup(&mctl->media_entity); } - mctl->media_dev = NULL; kfree(mctl); subs->media_ctl = NULL; }
Fix the following compile warning when CONFIG_MEDIA_CONTROLLER_DVB is disabled:
drivers/media/dvb-core/dvb_frontend.c: In function 'dvb_frontend_open': drivers/media/dvb-core/dvb_frontend.c:2526:1: warning: label 'err2' defined but not used [-Wunused-label] err2:
Signed-off-by: Shuah Khan shuahkh@osg.samsung.com --- drivers/media/dvb-core/dvb_frontend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c index 6a759f5..9e7ee83 100644 --- a/drivers/media/dvb-core/dvb_frontend.c +++ b/drivers/media/dvb-core/dvb_frontend.c @@ -2522,8 +2522,8 @@ err3: #ifdef CONFIG_MEDIA_CONTROLLER_DVB if (fe->dvb->mdev && fe->dvb->mdev->disable_source) fe->dvb->mdev->disable_source(dvbdev->entity); -#endif err2: +#endif dvb_generic_release(inode, file); err1: if (dvbdev->users == -1 && fe->ops.ts_bus_ctrl)
Fix the ignoring return value errors found during kbuild bot compile test.
drivers/media/usb/au0828/au0828-core.c:298:5: warning: ignoring return value of 'media_create_pad_link', declared with attribute warn_unused_result [-Wunused-result]
drivers/media/usb/au0828/au0828-core.c:356:2: warning: ignoring return value of 'media_device_register_entity_notify', declared with attribute warn_unused_result [-Wunused-result]
Signed-off-by: Shuah Khan shuahkh@osg.samsung.com --- drivers/media/usb/au0828/au0828-core.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c index 3ef6fee..5b595a6 100644 --- a/drivers/media/usb/au0828/au0828-core.c +++ b/drivers/media/usb/au0828/au0828-core.c @@ -299,15 +299,20 @@ void au0828_create_media_graph(struct media_entity *new, void *notify_data) if (!tuner) break;
- media_create_pad_link(ent, 0, tuner, - TUNER_PAD_RF_INPUT, - MEDIA_LNK_FL_ENABLED); + ret = media_create_pad_link(ent, 0, tuner, + TUNER_PAD_RF_INPUT, + MEDIA_LNK_FL_ENABLED); + if (ret) + pr_err("Create pad link err %d\n", ret); break; case AU0828_VMUX_COMPOSITE: case AU0828_VMUX_SVIDEO: default: /* AU0828_VMUX_DEBUG */ /* FIXME: fix the decoder PAD */ - media_create_pad_link(ent, 0, decoder, 0, 0); + ret = media_create_pad_link(ent, 0, decoder, + 0, 0); + if (ret) + pr_err("Create pad link err %d\n", ret); break; } } @@ -504,7 +509,13 @@ static void au0828_media_device_register(struct au0828_dev *dev, /* register entity_notify callback */ dev->entity_notify.notify_data = (void *) dev; dev->entity_notify.notify = au0828_create_media_graph; - media_device_register_entity_notify(mdev, &dev->entity_notify); + ret = media_device_register_entity_notify(mdev, &dev->entity_notify); + if (ret) { + dev_err(&udev->dev, + "Couldn't register entity_notify handler. Error: %d\n", + ret); + return; + }
/* set enable_source */ mdev->source_priv = (void *) dev;
participants (1)
-
Shuah Khan