All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sakari Ailus <sakari.ailus@iki.fi>
To: Shuah Khan <shuah@kernel.org>
Cc: mchehab@kernel.org, perex@perex.cz, tiwai@suse.com,
	hverkuil@xs4all.nl, linux-media@vger.kernel.org,
	linux-kernel@vger.kernel.org, alsa-devel@alsa-project.org
Subject: Re: [PATCH v10 0/4] Media Device Allocator API
Date: Fri, 25 Jan 2019 17:28:36 +0200	[thread overview]
Message-ID: <20190125152836.njwepezj2xf4xp22@valkosipuli.retiisi.org.uk> (raw)
In-Reply-To: <cover.1548360791.git.shuah@kernel.org>

Hi Shuah,

On Thu, Jan 24, 2019 at 01:32:37PM -0700, Shuah Khan wrote:
> Media Device Allocator API to allows multiple drivers share a media device.
> This API solves a very common use-case for media devices where one physical
> device (an USB stick) provides both audio and video. When such media device
> exposes a standard USB Audio class, a proprietary Video class, two or more
> independent drivers will share a single physical USB bridge. In such cases,
> it is necessary to coordinate access to the shared resource.
> 
> Using this API, drivers can allocate a media device with the shared struct
> device as the key. Once the media device is allocated by a driver, other
> drivers can get a reference to it. The media device is released when all
> the references are released.

Thanks for the update. I have to apologise I haven't ended up reviewing the
set for some time. After taking a look at the current version, I'm happy to
see that a number of issues recognised during earlier review rounds have
been addressed.

Would you happen to have a media graph (media-ctl --print-dot and media-ctl
-p) from the device? That'd help understanding the device a bit better for
those who are not familiar with it.

> 
> - This patch series is tested on 5.0-rc3 and addresses comments on
>   v9 series from Hans Verkuil.
> - v9 was tested on 4.20-rc6.
> - Tested sharing resources with kaffeine, vlc, xawtv, tvtime, and
>   arecord. When analog is streaming, digital and audio user-space
>   applications detect that the tuner is busy and exit. When digital
>   is streaming, analog and audio applications detect that the tuner is
>   busy and exit. When arecord is owns the tuner, digital and analog
>   detect that the tuner is busy and exit.
> - Tested media device allocator API with bind/unbind testing on
>   snd-usb-audio and au0828 drivers to make sure /dev/mediaX is released
>   only when the last driver is unbound.
> - Addressed review comments from Hans on the RFC v8 (rebased on 4.19)
> - Updated change log to describe the use-case more clearly.
> - No changes to 0001,0002 code since the v7 referenced below.
> - 0003 is a new patch to enable ALSA defines that have been
>   disabled for kernel between 4.9 and 4.19.
> - Minor merge conflict resolution in 0004.
> - Added SPDX to new files.
> 
> Changes since v9:
> - Patch 1: Fix mutex assert warning from find_module() calls. This
>   code was written before the change to find_module() that requires
>   callers to hold module_mutex. I missed this during my testing on
>   4.20-rc6. Hans Verkuil reported the problem.
> - Patch 4: sound/usb: Initializes all the entities it can before
>   registering the device based on comments from Hans Verkuil
> - Carried Reviewed-by tag from Takashi Iwai for the sound from v9.
> - No changes to Patches 2 and 3.
> 
> References:
> https://lkml.org/lkml/2018/11/2/169
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg105854.html
> 
> Shuah Khan (4):
>   media: Media Device Allocator API
>   media: change au0828 to use Media Device Allocator API
>   media: media.h: Enable ALSA MEDIA_INTF_T* interface types
>   sound/usb: Use Media Controller API to share media resources
> 
>  Documentation/media/kapi/mc-core.rst   |  41 ++++
>  drivers/media/Makefile                 |   4 +
>  drivers/media/media-dev-allocator.c    | 144 +++++++++++
>  drivers/media/usb/au0828/au0828-core.c |  12 +-
>  drivers/media/usb/au0828/au0828.h      |   1 +
>  include/media/media-dev-allocator.h    |  53 ++++
>  include/uapi/linux/media.h             |  25 +-
>  sound/usb/Kconfig                      |   4 +
>  sound/usb/Makefile                     |   2 +
>  sound/usb/card.c                       |  14 ++
>  sound/usb/card.h                       |   3 +
>  sound/usb/media.c                      | 327 +++++++++++++++++++++++++
>  sound/usb/media.h                      |  74 ++++++
>  sound/usb/mixer.h                      |   3 +
>  sound/usb/pcm.c                        |  29 ++-
>  sound/usb/quirks-table.h               |   1 +
>  sound/usb/stream.c                     |   2 +
>  sound/usb/usbaudio.h                   |   6 +
>  18 files changed, 723 insertions(+), 22 deletions(-)
>  create mode 100644 drivers/media/media-dev-allocator.c
>  create mode 100644 include/media/media-dev-allocator.h
>  create mode 100644 sound/usb/media.c
>  create mode 100644 sound/usb/media.h
> 

-- 
Kind regards,

Sakari Ailus

WARNING: multiple messages have this Message-ID (diff)
From: Sakari Ailus <sakari.ailus@iki.fi>
To: Shuah Khan <shuah@kernel.org>
Cc: alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
	tiwai@suse.com, hverkuil@xs4all.nl, mchehab@kernel.org,
	linux-media@vger.kernel.org
Subject: Re: [PATCH v10 0/4] Media Device Allocator API
Date: Fri, 25 Jan 2019 17:28:36 +0200	[thread overview]
Message-ID: <20190125152836.njwepezj2xf4xp22@valkosipuli.retiisi.org.uk> (raw)
In-Reply-To: <cover.1548360791.git.shuah@kernel.org>

Hi Shuah,

On Thu, Jan 24, 2019 at 01:32:37PM -0700, Shuah Khan wrote:
> Media Device Allocator API to allows multiple drivers share a media device.
> This API solves a very common use-case for media devices where one physical
> device (an USB stick) provides both audio and video. When such media device
> exposes a standard USB Audio class, a proprietary Video class, two or more
> independent drivers will share a single physical USB bridge. In such cases,
> it is necessary to coordinate access to the shared resource.
> 
> Using this API, drivers can allocate a media device with the shared struct
> device as the key. Once the media device is allocated by a driver, other
> drivers can get a reference to it. The media device is released when all
> the references are released.

Thanks for the update. I have to apologise I haven't ended up reviewing the
set for some time. After taking a look at the current version, I'm happy to
see that a number of issues recognised during earlier review rounds have
been addressed.

Would you happen to have a media graph (media-ctl --print-dot and media-ctl
-p) from the device? That'd help understanding the device a bit better for
those who are not familiar with it.

> 
> - This patch series is tested on 5.0-rc3 and addresses comments on
>   v9 series from Hans Verkuil.
> - v9 was tested on 4.20-rc6.
> - Tested sharing resources with kaffeine, vlc, xawtv, tvtime, and
>   arecord. When analog is streaming, digital and audio user-space
>   applications detect that the tuner is busy and exit. When digital
>   is streaming, analog and audio applications detect that the tuner is
>   busy and exit. When arecord is owns the tuner, digital and analog
>   detect that the tuner is busy and exit.
> - Tested media device allocator API with bind/unbind testing on
>   snd-usb-audio and au0828 drivers to make sure /dev/mediaX is released
>   only when the last driver is unbound.
> - Addressed review comments from Hans on the RFC v8 (rebased on 4.19)
> - Updated change log to describe the use-case more clearly.
> - No changes to 0001,0002 code since the v7 referenced below.
> - 0003 is a new patch to enable ALSA defines that have been
>   disabled for kernel between 4.9 and 4.19.
> - Minor merge conflict resolution in 0004.
> - Added SPDX to new files.
> 
> Changes since v9:
> - Patch 1: Fix mutex assert warning from find_module() calls. This
>   code was written before the change to find_module() that requires
>   callers to hold module_mutex. I missed this during my testing on
>   4.20-rc6. Hans Verkuil reported the problem.
> - Patch 4: sound/usb: Initializes all the entities it can before
>   registering the device based on comments from Hans Verkuil
> - Carried Reviewed-by tag from Takashi Iwai for the sound from v9.
> - No changes to Patches 2 and 3.
> 
> References:
> https://lkml.org/lkml/2018/11/2/169
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg105854.html
> 
> Shuah Khan (4):
>   media: Media Device Allocator API
>   media: change au0828 to use Media Device Allocator API
>   media: media.h: Enable ALSA MEDIA_INTF_T* interface types
>   sound/usb: Use Media Controller API to share media resources
> 
>  Documentation/media/kapi/mc-core.rst   |  41 ++++
>  drivers/media/Makefile                 |   4 +
>  drivers/media/media-dev-allocator.c    | 144 +++++++++++
>  drivers/media/usb/au0828/au0828-core.c |  12 +-
>  drivers/media/usb/au0828/au0828.h      |   1 +
>  include/media/media-dev-allocator.h    |  53 ++++
>  include/uapi/linux/media.h             |  25 +-
>  sound/usb/Kconfig                      |   4 +
>  sound/usb/Makefile                     |   2 +
>  sound/usb/card.c                       |  14 ++
>  sound/usb/card.h                       |   3 +
>  sound/usb/media.c                      | 327 +++++++++++++++++++++++++
>  sound/usb/media.h                      |  74 ++++++
>  sound/usb/mixer.h                      |   3 +
>  sound/usb/pcm.c                        |  29 ++-
>  sound/usb/quirks-table.h               |   1 +
>  sound/usb/stream.c                     |   2 +
>  sound/usb/usbaudio.h                   |   6 +
>  18 files changed, 723 insertions(+), 22 deletions(-)
>  create mode 100644 drivers/media/media-dev-allocator.c
>  create mode 100644 include/media/media-dev-allocator.h
>  create mode 100644 sound/usb/media.c
>  create mode 100644 sound/usb/media.h
> 

-- 
Kind regards,

Sakari Ailus

  parent reply	other threads:[~2019-01-25 15:29 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-24 20:32 [PATCH v10 0/4] Media Device Allocator API Shuah Khan
2019-01-24 20:32 ` [PATCH v10 1/4] media: " Shuah Khan
2019-01-25 15:38   ` Sakari Ailus
2019-01-26  0:27     ` shuah
2019-01-26  0:27       ` shuah
2019-01-24 20:32 ` [PATCH v10 2/4] media: change au0828 to use " Shuah Khan
2019-01-24 20:32 ` [PATCH v10 3/4] media: media.h: Enable ALSA MEDIA_INTF_T* interface types Shuah Khan
2019-01-24 20:32 ` [PATCH v10 4/4] sound/usb: Use Media Controller API to share media resources Shuah Khan
2019-01-25 15:28 ` Sakari Ailus [this message]
2019-01-25 15:28   ` [PATCH v10 0/4] Media Device Allocator API Sakari Ailus
2019-01-26  0:19   ` shuah
2019-01-26  0:19     ` shuah
2019-01-28 12:03 ` Hans Verkuil
2019-01-28 23:48   ` shuah
2019-01-29  9:43     ` Hans Verkuil
2019-01-30  1:50       ` shuah
2019-01-30  1:50         ` shuah
2019-01-30  7:42         ` Hans Verkuil
2019-01-30  7:42           ` Hans Verkuil
2019-02-01  0:46           ` shuah
2019-02-01  9:21             ` Hans Verkuil
2019-02-05 18:10               ` shuah
2019-02-06  7:36                 ` Hans Verkuil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190125152836.njwepezj2xf4xp22@valkosipuli.retiisi.org.uk \
    --to=sakari.ailus@iki.fi \
    --cc=alsa-devel@alsa-project.org \
    --cc=hverkuil@xs4all.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=perex@perex.cz \
    --cc=shuah@kernel.org \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.