Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1344632 > unrolled thread

Re: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources

Started byTakashi Iwai <tiwai@suse.de>
First post2016-02-26 21:00 +0100
Last post2016-02-29 07:10 +0100
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources Takashi Iwai <tiwai@suse.de> - 2016-02-26 21:00 +0100
    Re: [PATCH v3 22/22] sound/usb: Use Media Controller API to share  media resources Shuah Khan <shuahkh@osg.samsung.com> - 2016-02-26 21:10 +0100
      Re: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources Takashi Iwai <tiwai@suse.de> - 2016-02-26 22:00 +0100
        Re: [PATCH v3 22/22] sound/usb: Use Media Controller API to share  media resources Shuah Khan <shuahkh@osg.samsung.com> - 2016-02-27 04:00 +0100
          Re: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources Takashi Iwai <tiwai@suse.de> - 2016-02-27 08:50 +0100
            Re: [PATCH v3 22/22] sound/usb: Use Media Controller API to share  media resources Mauro Carvalho Chehab <mchehab@osg.samsung.com> - 2016-02-27 13:50 +0100
            Re: [PATCH v3 22/22] sound/usb: Use Media Controller API to share  media resources Shuah Khan <shuahkh@osg.samsung.com> - 2016-02-29 07:10 +0100

#1344632 — Re: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources

FromTakashi Iwai <tiwai@suse.de>
Date2016-02-26 21:00 +0100
SubjectRe: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources
Message-ID<r6wIq-8ao-17@gated-at.bofh.it>
On Fri, 12 Feb 2016 00:41:38 +0100,
Shuah Khan wrote:
> 
> 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 <shuahkh@osg.samsung.com>
> ---
>  sound/usb/Kconfig        |   4 +
>  sound/usb/Makefile       |   2 +
>  sound/usb/card.c         |  14 +++
>  sound/usb/card.h         |   3 +
>  sound/usb/media.c        | 318 +++++++++++++++++++++++++++++++++++++++++++++++
>  sound/usb/media.h        |  72 +++++++++++
>  sound/usb/mixer.h        |   3 +
>  sound/usb/pcm.c          |  28 ++++-
>  sound/usb/quirks-table.h |   1 +
>  sound/usb/stream.c       |   2 +
>  sound/usb/usbaudio.h     |   6 +
>  11 files changed, 448 insertions(+), 5 deletions(-)
>  create mode 100644 sound/usb/media.c
>  create mode 100644 sound/usb/media.h
> 
> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
> index a452ad7..ba117f5 100644
> --- a/sound/usb/Kconfig
> +++ b/sound/usb/Kconfig
> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
>  	select SND_RAWMIDI
>  	select SND_PCM
>  	select BITREVERSE
> +	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && MEDIA_SUPPORT

Looking at the media Kconfig again, this would be broken if
MEDIA_SUPPORT=m and SND_USB_AUDIO=y.  The ugly workaround is something
like:
	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER \
		if MEDIA_CONTROLLER && (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND)

Other than that, it looks more or less OK to me.
The way how media_stream_init() gets called is a bit worrisome, but it
should work practically.  Another concern is about the disconnection.
Can all function calls in media_device_delete() be safe even if it's
called while the application still opens the MC device?

In anyway, such a thing can be improved later once after we get this
merged, too.  So, feel fee to take my ack.

 Acked-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

[toc] | [next] | [standalone]

#1344638 — Re: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources

FromShuah Khan <shuahkh@osg.samsung.com>
Date2016-02-26 21:10 +0100
SubjectRe: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources
Message-ID<r6wS6-8v1-11@gated-at.bofh.it>
In reply to#1344632
On 02/26/2016 12:55 PM, Takashi Iwai wrote:
> On Fri, 12 Feb 2016 00:41:38 +0100,
> Shuah Khan wrote:
>>
>> 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 <shuahkh@osg.samsung.com>
>> ---
>>  sound/usb/Kconfig        |   4 +
>>  sound/usb/Makefile       |   2 +
>>  sound/usb/card.c         |  14 +++
>>  sound/usb/card.h         |   3 +
>>  sound/usb/media.c        | 318 +++++++++++++++++++++++++++++++++++++++++++++++
>>  sound/usb/media.h        |  72 +++++++++++
>>  sound/usb/mixer.h        |   3 +
>>  sound/usb/pcm.c          |  28 ++++-
>>  sound/usb/quirks-table.h |   1 +
>>  sound/usb/stream.c       |   2 +
>>  sound/usb/usbaudio.h     |   6 +
>>  11 files changed, 448 insertions(+), 5 deletions(-)
>>  create mode 100644 sound/usb/media.c
>>  create mode 100644 sound/usb/media.h
>>
>> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
>> index a452ad7..ba117f5 100644
>> --- a/sound/usb/Kconfig
>> +++ b/sound/usb/Kconfig
>> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
>>  	select SND_RAWMIDI
>>  	select SND_PCM
>>  	select BITREVERSE
>> +	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && MEDIA_SUPPORT
> 
> Looking at the media Kconfig again, this would be broken if
> MEDIA_SUPPORT=m and SND_USB_AUDIO=y.  The ugly workaround is something
> like:
> 	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER \
> 		if MEDIA_CONTROLLER && (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND)

My current config is MEDIA_SUPPORT=m and SND_USB_AUDIO=y
It is working and I didn't see any issues so far. Maybe
the current change is good. I am hoping kbuild-bot can
find issues (if any) once it gets merged.

> 
> Other than that, it looks more or less OK to me.
> The way how media_stream_init() gets called is a bit worrisome, but it
> should work practically.  Another concern is about the disconnection.
> Can all function calls in media_device_delete() be safe even if it's
> called while the application still opens the MC device?

Right. I have been looking into device removal path when
ioctls are active and I can resolve any issues that might
surface while an audio app is active when device is removed.

> 
> In anyway, such a thing can be improved later once after we get this
> merged, too.  So, feel fee to take my ack.
> 
>  Acked-by: Takashi Iwai <tiwai@suse.de>

thanks for the Ack
-- Shuah

> 
> 
> thanks,
> 
> Takashi
> 


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

[toc] | [prev] | [next] | [standalone]

#1344676

FromTakashi Iwai <tiwai@suse.de>
Date2016-02-26 22:00 +0100
Message-ID<r6xEu-mo-13@gated-at.bofh.it>
In reply to#1344638
On Fri, 26 Feb 2016 21:08:43 +0100,
Shuah Khan wrote:
> 
> On 02/26/2016 12:55 PM, Takashi Iwai wrote:
> > On Fri, 12 Feb 2016 00:41:38 +0100,
> > Shuah Khan wrote:
> >>
> >> 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 <shuahkh@osg.samsung.com>
> >> ---
> >>  sound/usb/Kconfig        |   4 +
> >>  sound/usb/Makefile       |   2 +
> >>  sound/usb/card.c         |  14 +++
> >>  sound/usb/card.h         |   3 +
> >>  sound/usb/media.c        | 318 +++++++++++++++++++++++++++++++++++++++++++++++
> >>  sound/usb/media.h        |  72 +++++++++++
> >>  sound/usb/mixer.h        |   3 +
> >>  sound/usb/pcm.c          |  28 ++++-
> >>  sound/usb/quirks-table.h |   1 +
> >>  sound/usb/stream.c       |   2 +
> >>  sound/usb/usbaudio.h     |   6 +
> >>  11 files changed, 448 insertions(+), 5 deletions(-)
> >>  create mode 100644 sound/usb/media.c
> >>  create mode 100644 sound/usb/media.h
> >>
> >> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
> >> index a452ad7..ba117f5 100644
> >> --- a/sound/usb/Kconfig
> >> +++ b/sound/usb/Kconfig
> >> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
> >>  	select SND_RAWMIDI
> >>  	select SND_PCM
> >>  	select BITREVERSE
> >> +	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && MEDIA_SUPPORT
> > 
> > Looking at the media Kconfig again, this would be broken if
> > MEDIA_SUPPORT=m and SND_USB_AUDIO=y.  The ugly workaround is something
> > like:
> > 	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER \
> > 		if MEDIA_CONTROLLER && (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND)
> 
> My current config is MEDIA_SUPPORT=m and SND_USB_AUDIO=y
> It is working and I didn't see any issues so far.

Hmm, how does it be?  In drivers/media/Makefile:

ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
  obj-$(CONFIG_MEDIA_SUPPORT) += media.o
endif

So it's a module.  Meanwhile you have reference from usb-audio driver
that is built-in kernel.  How is the symbol resolved?

> Maybe
> the current change is good. I am hoping kbuild-bot can
> find issues (if any) once it gets merged.
> 
> > 
> > Other than that, it looks more or less OK to me.
> > The way how media_stream_init() gets called is a bit worrisome, but it
> > should work practically.  Another concern is about the disconnection.
> > Can all function calls in media_device_delete() be safe even if it's
> > called while the application still opens the MC device?
> 
> Right. I have been looking into device removal path when
> ioctls are active and I can resolve any issues that might
> surface while an audio app is active when device is removed.

So, it's 100% safe to call all these media_*() functions while the
device is being accessed before closing?


Takashi

[toc] | [prev] | [next] | [standalone]

#1344895 — Re: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources

FromShuah Khan <shuahkh@osg.samsung.com>
Date2016-02-27 04:00 +0100
SubjectRe: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources
Message-ID<r6DgR-4yu-1@gated-at.bofh.it>
In reply to#1344676
On 02/26/2016 01:50 PM, Takashi Iwai wrote:
> On Fri, 26 Feb 2016 21:08:43 +0100,
> Shuah Khan wrote:
>>
>> On 02/26/2016 12:55 PM, Takashi Iwai wrote:
>>> On Fri, 12 Feb 2016 00:41:38 +0100,
>>> Shuah Khan wrote:
>>>>
>>>> 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 <shuahkh@osg.samsung.com>
>>>> ---
>>>>  sound/usb/Kconfig        |   4 +
>>>>  sound/usb/Makefile       |   2 +
>>>>  sound/usb/card.c         |  14 +++
>>>>  sound/usb/card.h         |   3 +
>>>>  sound/usb/media.c        | 318 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>  sound/usb/media.h        |  72 +++++++++++
>>>>  sound/usb/mixer.h        |   3 +
>>>>  sound/usb/pcm.c          |  28 ++++-
>>>>  sound/usb/quirks-table.h |   1 +
>>>>  sound/usb/stream.c       |   2 +
>>>>  sound/usb/usbaudio.h     |   6 +
>>>>  11 files changed, 448 insertions(+), 5 deletions(-)
>>>>  create mode 100644 sound/usb/media.c
>>>>  create mode 100644 sound/usb/media.h
>>>>
>>>> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
>>>> index a452ad7..ba117f5 100644
>>>> --- a/sound/usb/Kconfig
>>>> +++ b/sound/usb/Kconfig
>>>> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
>>>>  	select SND_RAWMIDI
>>>>  	select SND_PCM
>>>>  	select BITREVERSE
>>>> +	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && MEDIA_SUPPORT
>>>
>>> Looking at the media Kconfig again, this would be broken if
>>> MEDIA_SUPPORT=m and SND_USB_AUDIO=y.  The ugly workaround is something
>>> like:
>>> 	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER \
>>> 		if MEDIA_CONTROLLER && (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND)
>>
>> My current config is MEDIA_SUPPORT=m and SND_USB_AUDIO=y
>> It is working and I didn't see any issues so far.
> 
> Hmm, how does it be?  In drivers/media/Makefile:
> 
> ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
>   obj-$(CONFIG_MEDIA_SUPPORT) += media.o
> endif
> 
> So it's a module.  Meanwhile you have reference from usb-audio driver
> that is built-in kernel.  How is the symbol resolved?

Sorry my mistake. I misspoke. My config had:
CONFIG_MEDIA_SUPPORT=m
CONFIG_MEDIA_CONTROLLER=y
CONFIG_SND_USB_AUDIO=m

The following doesn't work as you pointed out.

CONFIG_MEDIA_SUPPORT=m
CONFIG_MEDIA_CONTROLLER=y
CONFIG_SND_USB_AUDIO=y

okay here is what will work for all of the possible
combinations of CONFIG_MEDIA_SUPPORT and CONFIG_SND_USB_AUDIO

select SND_USB_AUDIO_USE_MEDIA_CONTROLLER \
       if MEDIA_CONTROLLER && ((MEDIA_SUPPORT=y) || (MEDIA_SUPPORT=m && SND_USB_AUDIO=m))

The above will cover the cases when

1. CONFIG_MEDIA_SUPPORT and CONFIG_SND_USB_AUDIO are
   both modules
   CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER is selected

2. CONFIG_MEDIA_SUPPORT=y and CONFIG_SND_USB_AUDIO=m
   CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER is selected

3. CONFIG_MEDIA_SUPPORT=y and CONFIG_SND_USB_AUDIO=y
   CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER is selected

4. CONFIG_MEDIA_SUPPORT=m and CONFIG_SND_USB_AUDIO=y
   This is when we don't want
   CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER selected

I verified all of the above combinations to make sure
the logic works.

If you think of a better way to do this please let me
know. I will go ahead and send patch v4 with the above
change and you can decide if that is acceptable.

>>>
>>> Other than that, it looks more or less OK to me.
>>> The way how media_stream_init() gets called is a bit worrisome, but it
>>> should work practically.  Another concern is about the disconnection.
>>> Can all function calls in media_device_delete() be safe even if it's
>>> called while the application still opens the MC device?
>>
>> Right. I have been looking into device removal path when
>> ioctls are active and I can resolve any issues that might
>> surface while an audio app is active when device is removed.
> 
> So, it's 100% safe to call all these media_*() functions while the
> device is being accessed before closing?
> 

There is a known problem with device removal when
media device file is open and ioctl is in progress.
This isn't specific to this patch series, a general
problem that is related to media device removal in
general.

I am working on a fix for this problem. As you said,
earlier, I can work on fixing issues after the merge.

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

[toc] | [prev] | [next] | [standalone]

#1344937

FromTakashi Iwai <tiwai@suse.de>
Date2016-02-27 08:50 +0100
Message-ID<r6HNv-88y-3@gated-at.bofh.it>
In reply to#1344895
On Sat, 27 Feb 2016 03:55:39 +0100,
Shuah Khan wrote:
> 
> On 02/26/2016 01:50 PM, Takashi Iwai wrote:
> > On Fri, 26 Feb 2016 21:08:43 +0100,
> > Shuah Khan wrote:
> >>
> >> On 02/26/2016 12:55 PM, Takashi Iwai wrote:
> >>> On Fri, 12 Feb 2016 00:41:38 +0100,
> >>> Shuah Khan wrote:
> >>>>
> >>>> 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 <shuahkh@osg.samsung.com>
> >>>> ---
> >>>>  sound/usb/Kconfig        |   4 +
> >>>>  sound/usb/Makefile       |   2 +
> >>>>  sound/usb/card.c         |  14 +++
> >>>>  sound/usb/card.h         |   3 +
> >>>>  sound/usb/media.c        | 318 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>>  sound/usb/media.h        |  72 +++++++++++
> >>>>  sound/usb/mixer.h        |   3 +
> >>>>  sound/usb/pcm.c          |  28 ++++-
> >>>>  sound/usb/quirks-table.h |   1 +
> >>>>  sound/usb/stream.c       |   2 +
> >>>>  sound/usb/usbaudio.h     |   6 +
> >>>>  11 files changed, 448 insertions(+), 5 deletions(-)
> >>>>  create mode 100644 sound/usb/media.c
> >>>>  create mode 100644 sound/usb/media.h
> >>>>
> >>>> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
> >>>> index a452ad7..ba117f5 100644
> >>>> --- a/sound/usb/Kconfig
> >>>> +++ b/sound/usb/Kconfig
> >>>> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
> >>>>  	select SND_RAWMIDI
> >>>>  	select SND_PCM
> >>>>  	select BITREVERSE
> >>>> +	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && MEDIA_SUPPORT
> >>>
> >>> Looking at the media Kconfig again, this would be broken if
> >>> MEDIA_SUPPORT=m and SND_USB_AUDIO=y.  The ugly workaround is something
> >>> like:
> >>> 	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER \
> >>> 		if MEDIA_CONTROLLER && (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND)
> >>
> >> My current config is MEDIA_SUPPORT=m and SND_USB_AUDIO=y
> >> It is working and I didn't see any issues so far.
> > 
> > Hmm, how does it be?  In drivers/media/Makefile:
> > 
> > ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
> >   obj-$(CONFIG_MEDIA_SUPPORT) += media.o
> > endif
> > 
> > So it's a module.  Meanwhile you have reference from usb-audio driver
> > that is built-in kernel.  How is the symbol resolved?
> 
> Sorry my mistake. I misspoke. My config had:
> CONFIG_MEDIA_SUPPORT=m
> CONFIG_MEDIA_CONTROLLER=y
> CONFIG_SND_USB_AUDIO=m
> 
> The following doesn't work as you pointed out.
> 
> CONFIG_MEDIA_SUPPORT=m
> CONFIG_MEDIA_CONTROLLER=y
> CONFIG_SND_USB_AUDIO=y
> 
> okay here is what will work for all of the possible
> combinations of CONFIG_MEDIA_SUPPORT and CONFIG_SND_USB_AUDIO
> 
> select SND_USB_AUDIO_USE_MEDIA_CONTROLLER \
>        if MEDIA_CONTROLLER && ((MEDIA_SUPPORT=y) || (MEDIA_SUPPORT=m && SND_USB_AUDIO=m))
> 
> The above will cover the cases when
> 
> 1. CONFIG_MEDIA_SUPPORT and CONFIG_SND_USB_AUDIO are
>    both modules
>    CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER is selected
> 
> 2. CONFIG_MEDIA_SUPPORT=y and CONFIG_SND_USB_AUDIO=m
>    CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER is selected
> 
> 3. CONFIG_MEDIA_SUPPORT=y and CONFIG_SND_USB_AUDIO=y
>    CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER is selected
> 
> 4. CONFIG_MEDIA_SUPPORT=m and CONFIG_SND_USB_AUDIO=y
>    This is when we don't want
>    CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER selected
> 
> I verified all of the above combinations to make sure
> the logic works.
> 
> If you think of a better way to do this please let me
> know. I will go ahead and send patch v4 with the above
> change and you can decide if that is acceptable.

I'm not 100% sure whether CONFIG_SND_USB_AUDIO=m can be put there as
conditional inside CONFIG_SND_USB_AUDIO definition.  Maybe a safer
form would be like:

config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
	bool
	default y
	depends on SND_USB_AUDIO
	depends on MEDIA_CONTROLLER
	depends on (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND_USB_AUDIO)

and drop select from SND_USB_AUDIO.


> >>> Other than that, it looks more or less OK to me.
> >>> The way how media_stream_init() gets called is a bit worrisome, but it
> >>> should work practically.  Another concern is about the disconnection.
> >>> Can all function calls in media_device_delete() be safe even if it's
> >>> called while the application still opens the MC device?
> >>
> >> Right. I have been looking into device removal path when
> >> ioctls are active and I can resolve any issues that might
> >> surface while an audio app is active when device is removed.
> > 
> > So, it's 100% safe to call all these media_*() functions while the
> > device is being accessed before closing?
> > 
> 
> There is a known problem with device removal when
> media device file is open and ioctl is in progress.
> This isn't specific to this patch series, a general
> problem that is related to media device removal in
> general.
> 
> I am working on a fix for this problem. As you said,
> earlier, I can work on fixing issues after the merge.

OK, understood.


Takashi

[toc] | [prev] | [next] | [standalone]

#1345012 — Re: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources

FromMauro Carvalho Chehab <mchehab@osg.samsung.com>
Date2016-02-27 13:50 +0100
SubjectRe: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources
Message-ID<r6MtQ-30v-3@gated-at.bofh.it>
In reply to#1344937
Em Sat, 27 Feb 2016 08:48:05 +0100
Takashi Iwai <tiwai@suse.de> escreveu:

> On Sat, 27 Feb 2016 03:55:39 +0100,
> Shuah Khan wrote:
> > 
> > On 02/26/2016 01:50 PM, Takashi Iwai wrote:  
> > > On Fri, 26 Feb 2016 21:08:43 +0100,
> > > Shuah Khan wrote:  
> > >>
> > >> On 02/26/2016 12:55 PM, Takashi Iwai wrote:  
> > >>> On Fri, 12 Feb 2016 00:41:38 +0100,
> > >>> Shuah Khan wrote:  
> > >>>>
> > >>>> 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 <shuahkh@osg.samsung.com>
> > >>>> ---
> > >>>>  sound/usb/Kconfig        |   4 +
> > >>>>  sound/usb/Makefile       |   2 +
> > >>>>  sound/usb/card.c         |  14 +++
> > >>>>  sound/usb/card.h         |   3 +
> > >>>>  sound/usb/media.c        | 318 +++++++++++++++++++++++++++++++++++++++++++++++
> > >>>>  sound/usb/media.h        |  72 +++++++++++
> > >>>>  sound/usb/mixer.h        |   3 +
> > >>>>  sound/usb/pcm.c          |  28 ++++-
> > >>>>  sound/usb/quirks-table.h |   1 +
> > >>>>  sound/usb/stream.c       |   2 +
> > >>>>  sound/usb/usbaudio.h     |   6 +
> > >>>>  11 files changed, 448 insertions(+), 5 deletions(-)
> > >>>>  create mode 100644 sound/usb/media.c
> > >>>>  create mode 100644 sound/usb/media.h
> > >>>>
> > >>>> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
> > >>>> index a452ad7..ba117f5 100644
> > >>>> --- a/sound/usb/Kconfig
> > >>>> +++ b/sound/usb/Kconfig
> > >>>> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
> > >>>>  	select SND_RAWMIDI
> > >>>>  	select SND_PCM
> > >>>>  	select BITREVERSE
> > >>>> +	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && MEDIA_SUPPORT  
> > >>>
> > >>> Looking at the media Kconfig again, this would be broken if
> > >>> MEDIA_SUPPORT=m and SND_USB_AUDIO=y.  The ugly workaround is something
> > >>> like:
> > >>> 	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER \
> > >>> 		if MEDIA_CONTROLLER && (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND)  
> > >>
> > >> My current config is MEDIA_SUPPORT=m and SND_USB_AUDIO=y
> > >> It is working and I didn't see any issues so far.  
> > > 
> > > Hmm, how does it be?  In drivers/media/Makefile:
> > > 
> > > ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
> > >   obj-$(CONFIG_MEDIA_SUPPORT) += media.o
> > > endif
> > > 
> > > So it's a module.  Meanwhile you have reference from usb-audio driver
> > > that is built-in kernel.  How is the symbol resolved?  
> > 
> > Sorry my mistake. I misspoke. My config had:
> > CONFIG_MEDIA_SUPPORT=m
> > CONFIG_MEDIA_CONTROLLER=y
> > CONFIG_SND_USB_AUDIO=m
> > 
> > The following doesn't work as you pointed out.
> > 
> > CONFIG_MEDIA_SUPPORT=m
> > CONFIG_MEDIA_CONTROLLER=y
> > CONFIG_SND_USB_AUDIO=y
> > 
> > okay here is what will work for all of the possible
> > combinations of CONFIG_MEDIA_SUPPORT and CONFIG_SND_USB_AUDIO
> > 
> > select SND_USB_AUDIO_USE_MEDIA_CONTROLLER \
> >        if MEDIA_CONTROLLER && ((MEDIA_SUPPORT=y) || (MEDIA_SUPPORT=m && SND_USB_AUDIO=m))
> > 
> > The above will cover the cases when
> > 
> > 1. CONFIG_MEDIA_SUPPORT and CONFIG_SND_USB_AUDIO are
> >    both modules
> >    CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER is selected
> > 
> > 2. CONFIG_MEDIA_SUPPORT=y and CONFIG_SND_USB_AUDIO=m
> >    CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER is selected
> > 
> > 3. CONFIG_MEDIA_SUPPORT=y and CONFIG_SND_USB_AUDIO=y
> >    CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER is selected
> > 
> > 4. CONFIG_MEDIA_SUPPORT=m and CONFIG_SND_USB_AUDIO=y
> >    This is when we don't want
> >    CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER selected
> > 
> > I verified all of the above combinations to make sure
> > the logic works.
> > 
> > If you think of a better way to do this please let me
> > know. I will go ahead and send patch v4 with the above
> > change and you can decide if that is acceptable.  
> 
> I'm not 100% sure whether CONFIG_SND_USB_AUDIO=m can be put there as
> conditional inside CONFIG_SND_USB_AUDIO definition.  Maybe a safer
> form would be like:
> 
> config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
> 	bool
> 	default y
> 	depends on SND_USB_AUDIO
> 	depends on MEDIA_CONTROLLER
> 	depends on (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND_USB_AUDIO)
> 
> and drop select from SND_USB_AUDIO.
> 
> 
> > >>> Other than that, it looks more or less OK to me.
> > >>> The way how media_stream_init() gets called is a bit worrisome, but it
> > >>> should work practically.  Another concern is about the disconnection.
> > >>> Can all function calls in media_device_delete() be safe even if it's
> > >>> called while the application still opens the MC device?  
> > >>
> > >> Right. I have been looking into device removal path when
> > >> ioctls are active and I can resolve any issues that might
> > >> surface while an audio app is active when device is removed.  
> > > 
> > > So, it's 100% safe to call all these media_*() functions while the
> > > device is being accessed before closing?
> > >   
> > 
> > There is a known problem with device removal when
> > media device file is open and ioctl is in progress.
> > This isn't specific to this patch series, a general
> > problem that is related to media device removal in
> > general.
> > 
> > I am working on a fix for this problem. As you said,
> > earlier, I can work on fixing issues after the merge.  
> 
> OK, understood.
> 
> 
> Takashi


As there are still some issues here, I decided to apply all but
this one, in order to avoid having compilation issues with random
configs.

Shuah,

As there were several conflicts when I applied the initial 21 patches
from this series, please apply this one on the top of the latest
tree. 

As this patch touches only at sound, it is independent, so
you won't need to rebase it. However, I might have solved some
conflicts badly, a final test wouldn't hurt ;)

Regards,
Mauro

[toc] | [prev] | [next] | [standalone]

#1345512 — Re: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources

FromShuah Khan <shuahkh@osg.samsung.com>
Date2016-02-29 07:10 +0100
SubjectRe: [PATCH v3 22/22] sound/usb: Use Media Controller API to share media resources
Message-ID<r7pbQ-6Z8-1@gated-at.bofh.it>
In reply to#1344937
On 02/27/2016 12:48 AM, Takashi Iwai wrote:
> On Sat, 27 Feb 2016 03:55:39 +0100,
> Shuah Khan wrote:
>>
>> On 02/26/2016 01:50 PM, Takashi Iwai wrote:
>>> On Fri, 26 Feb 2016 21:08:43 +0100,
>>> Shuah Khan wrote:
>>>>
>>>> On 02/26/2016 12:55 PM, Takashi Iwai wrote:
>>>>> On Fri, 12 Feb 2016 00:41:38 +0100,
>>>>> Shuah Khan wrote:
>>>>>>
>>>>>> 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 <shuahkh@osg.samsung.com>
>>>>>> ---
>>>>>>  sound/usb/Kconfig        |   4 +
>>>>>>  sound/usb/Makefile       |   2 +
>>>>>>  sound/usb/card.c         |  14 +++
>>>>>>  sound/usb/card.h         |   3 +
>>>>>>  sound/usb/media.c        | 318 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>  sound/usb/media.h        |  72 +++++++++++
>>>>>>  sound/usb/mixer.h        |   3 +
>>>>>>  sound/usb/pcm.c          |  28 ++++-
>>>>>>  sound/usb/quirks-table.h |   1 +
>>>>>>  sound/usb/stream.c       |   2 +
>>>>>>  sound/usb/usbaudio.h     |   6 +
>>>>>>  11 files changed, 448 insertions(+), 5 deletions(-)
>>>>>>  create mode 100644 sound/usb/media.c
>>>>>>  create mode 100644 sound/usb/media.h
>>>>>>
>>>>>> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
>>>>>> index a452ad7..ba117f5 100644
>>>>>> --- a/sound/usb/Kconfig
>>>>>> +++ b/sound/usb/Kconfig
>>>>>> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
>>>>>>  	select SND_RAWMIDI
>>>>>>  	select SND_PCM
>>>>>>  	select BITREVERSE
>>>>>> +	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && MEDIA_SUPPORT
>>>>>
>>>>> Looking at the media Kconfig again, this would be broken if
>>>>> MEDIA_SUPPORT=m and SND_USB_AUDIO=y.  The ugly workaround is something
>>>>> like:
>>>>> 	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER \
>>>>> 		if MEDIA_CONTROLLER && (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND)
>>>>
>>>> My current config is MEDIA_SUPPORT=m and SND_USB_AUDIO=y
>>>> It is working and I didn't see any issues so far.
>>>
>>> Hmm, how does it be?  In drivers/media/Makefile:
>>>
>>> ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
>>>   obj-$(CONFIG_MEDIA_SUPPORT) += media.o
>>> endif
>>>
>>> So it's a module.  Meanwhile you have reference from usb-audio driver
>>> that is built-in kernel.  How is the symbol resolved?
>>
>> Sorry my mistake. I misspoke. My config had:
>> CONFIG_MEDIA_SUPPORT=m
>> CONFIG_MEDIA_CONTROLLER=y
>> CONFIG_SND_USB_AUDIO=m
>>
>> The following doesn't work as you pointed out.
>>
>> CONFIG_MEDIA_SUPPORT=m
>> CONFIG_MEDIA_CONTROLLER=y
>> CONFIG_SND_USB_AUDIO=y
>>
>> okay here is what will work for all of the possible
>> combinations of CONFIG_MEDIA_SUPPORT and CONFIG_SND_USB_AUDIO
>>
>> select SND_USB_AUDIO_USE_MEDIA_CONTROLLER \
>>        if MEDIA_CONTROLLER && ((MEDIA_SUPPORT=y) || (MEDIA_SUPPORT=m && SND_USB_AUDIO=m))
>>
>> The above will cover the cases when
>>
>> 1. CONFIG_MEDIA_SUPPORT and CONFIG_SND_USB_AUDIO are
>>    both modules
>>    CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER is selected
>>
>> 2. CONFIG_MEDIA_SUPPORT=y and CONFIG_SND_USB_AUDIO=m
>>    CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER is selected
>>
>> 3. CONFIG_MEDIA_SUPPORT=y and CONFIG_SND_USB_AUDIO=y
>>    CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER is selected
>>
>> 4. CONFIG_MEDIA_SUPPORT=m and CONFIG_SND_USB_AUDIO=y
>>    This is when we don't want
>>    CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER selected
>>
>> I verified all of the above combinations to make sure
>> the logic works.
>>
>> If you think of a better way to do this please let me
>> know. I will go ahead and send patch v4 with the above
>> change and you can decide if that is acceptable.
> 
> I'm not 100% sure whether CONFIG_SND_USB_AUDIO=m can be put there as
> conditional inside CONFIG_SND_USB_AUDIO definition.  Maybe a safer
> form would be like:
> 
> config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
> 	bool
> 	default y
> 	depends on SND_USB_AUDIO
> 	depends on MEDIA_CONTROLLER
> 	depends on (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND_USB_AUDIO)
> 
> and drop select from SND_USB_AUDIO.
> 
> 
>>>>> Other than that, it looks more or less OK to me.
>>>>> The way how media_stream_init() gets called is a bit worrisome, but it
>>>>> should work practically.  Another concern is about the disconnection.
>>>>> Can all function calls in media_device_delete() be safe even if it's
>>>>> called while the application still opens the MC device?
>>>>
>>>> Right. I have been looking into device removal path when
>>>> ioctls are active and I can resolve any issues that might
>>>> surface while an audio app is active when device is removed.
>>>
>>> So, it's 100% safe to call all these media_*() functions while the
>>> device is being accessed before closing?
>>>
>>
>> There is a known problem with device removal when
>> media device file is open and ioctl is in progress.
>> This isn't specific to this patch series, a general
>> problem that is related to media device removal in
>> general.
>>
>> I am working on a fix for this problem. As you said,
>> earlier, I can work on fixing issues after the merge.
> 
> OK, understood.
> 
> 

Hi Takashi,

I sent patch v4 that works for all combinations
of MEDIA_SUPPORT and SND_USB_AUDIO.

Please review and Ack if you are okay with this
version. Thanks for your help with the Kconfig
file changes.

-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web