mbox series

[v9,00/15] Add audio support in v4l2 framework

Message ID 1699595289-25773-1-git-send-email-shengjiu.wang@nxp.com
Headers show
Series Add audio support in v4l2 framework | expand

Message

Shengjiu Wang Nov. 10, 2023, 5:47 a.m. UTC
Audio signal processing also has the requirement for memory to
memory similar as Video.

This asrc memory to memory (memory ->asrc->memory) case is a non
real time use case.

User fills the input buffer to the asrc module, after conversion, then asrc
sends back the output buffer to user. So it is not a traditional ALSA playback
and capture case.

It is a specific use case,  there is no reference in current kernel.
v4l2 memory to memory is the closed implementation,  v4l2 current
support video, image, radio, tuner, touch devices, so it is not
complicated to add support for this specific audio case.

Because we had implemented the "memory -> asrc ->i2s device-> codec"
use case in ALSA.  Now the "memory->asrc->memory" needs
to reuse the code in asrc driver, so the first 3 patches is for refining
the code to make it can be shared by the "memory->asrc->memory"
driver.

The main change is in the v4l2 side, A /dev/vl4-audioX will be created,
user applications only use the ioctl of v4l2 framework.

Other change is to add memory to memory support for two kinds of i.MX ASRC
module.

changes in v9:
- add MEDIA_ENT_F_PROC_AUDIO_RESAMPLER.
- add MEDIA_INTF_T_V4L_AUDIO
- add media controller support
- refine the vim2m-audio to support 8k<->16k conversion.

changes in v8:
- refine V4L2_CAP_AUDIO_M2M to be 0x00000008
- update doc for FIXED_POINT
- address comments for imx-asrc

changes in v7:
- add acked-by from Mark
- separate commit for fixed point, m2m audio class, audio rate controls
- use INTEGER_MENU for rate,  FIXED_POINT for rate offset
- remove used fmts
- address other comments for Hans

changes in v6:
- use m2m_prepare/m2m_unprepare/m2m_start/m2m_stop to replace
  m2m_start_part_one/m2m_stop_part_one, m2m_start_part_two/m2m_stop_part_two.
- change V4L2_CTRL_TYPE_ASRC_RATE to V4L2_CTRL_TYPE_FIXED_POINT
- fix warning by kernel test rebot
- remove some unused format V4L2_AUDIO_FMT_XX
- Get SNDRV_PCM_FORMAT from V4L2_AUDIO_FMT in driver.
- rename audm2m to viaudm2m.

changes in v5:
- remove V4L2_AUDIO_FMT_LPCM
- define audio pixel format like V4L2_AUDIO_FMT_S8...
- remove rate and format in struct v4l2_audio_format.
- Add V4L2_CID_ASRC_SOURCE_RATE and V4L2_CID_ASRC_DEST_RATE controls
- updata document accordingly.

changes in v4:
- update document style
- separate V4L2_AUDIO_FMT_LPCM and V4L2_CAP_AUDIO_M2M in separate commit

changes in v3:
- Modify documents for adding audio m2m support
- Add audio virtual m2m driver
- Defined V4L2_AUDIO_FMT_LPCM format type for audio.
- Defined V4L2_CAP_AUDIO_M2M capability type for audio m2m case.
- with modification in v4l-utils, pass v4l2-compliance test.

changes in v2:
- decouple the implementation in v4l2 and ALSA
- implement the memory to memory driver as a platfrom driver
  and move it to driver/media
- move fsl_asrc_common.h to include/sound folder

Shengjiu Wang (15):
  ASoC: fsl_asrc: define functions for memory to memory usage
  ASoC: fsl_easrc: define functions for memory to memory usage
  ASoC: fsl_asrc: move fsl_asrc_common.h to include/sound
  ASoC: fsl_asrc: register m2m platform device
  ASoC: fsl_easrc: register m2m platform device
  media: uapi: Add V4L2_CAP_AUDIO_M2M capability flag
  media: v4l2: Add audio capture and output support
  media: uapi: Define audio sample format fourcc type
  media: uapi: Add V4L2_CTRL_CLASS_M2M_AUDIO
  media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
  media: uapi: Add audio rate controls support
  media: uapi: Declare interface types for Audio
  media: uapi: Add an entity type for audio resampler
  media: imx-asrc: Add memory to memory driver
  media: vim2m-audio: add virtual driver for audio memory to memory

 .../media/mediactl/media-types.rst            |    9 +
 .../userspace-api/media/v4l/buffer.rst        |    6 +
 .../userspace-api/media/v4l/common.rst        |    1 +
 .../media/v4l/dev-audio-mem2mem.rst           |   71 +
 .../userspace-api/media/v4l/devices.rst       |    1 +
 .../media/v4l/ext-ctrls-audio-m2m.rst         |   41 +
 .../userspace-api/media/v4l/pixfmt-audio.rst  |   87 ++
 .../userspace-api/media/v4l/pixfmt.rst        |    1 +
 .../media/v4l/vidioc-enum-fmt.rst             |    2 +
 .../media/v4l/vidioc-g-ext-ctrls.rst          |   17 +-
 .../userspace-api/media/v4l/vidioc-g-fmt.rst  |    4 +
 .../media/v4l/vidioc-querycap.rst             |    3 +
 .../media/v4l/vidioc-queryctrl.rst            |    9 +-
 .../media/videodev2.h.rst.exceptions          |    4 +
 .../media/common/videobuf2/videobuf2-v4l2.c   |    4 +
 drivers/media/platform/nxp/Kconfig            |   14 +
 drivers/media/platform/nxp/Makefile           |    1 +
 drivers/media/platform/nxp/imx-asrc.c         | 1235 +++++++++++++++++
 drivers/media/test-drivers/Kconfig            |   11 +
 drivers/media/test-drivers/Makefile           |    1 +
 drivers/media/test-drivers/vim2m-audio.c      |  799 +++++++++++
 drivers/media/v4l2-core/v4l2-ctrls-api.c      |    5 +-
 drivers/media/v4l2-core/v4l2-ctrls-core.c     |    2 +
 drivers/media/v4l2-core/v4l2-ctrls-defs.c     |   16 +
 drivers/media/v4l2-core/v4l2-dev.c            |   21 +
 drivers/media/v4l2-core/v4l2-ioctl.c          |   66 +
 drivers/media/v4l2-core/v4l2-mem2mem.c        |   13 +-
 include/media/v4l2-dev.h                      |    2 +
 include/media/v4l2-ioctl.h                    |   34 +
 .../fsl => include/sound}/fsl_asrc_common.h   |   60 +
 include/uapi/linux/media.h                    |    2 +
 include/uapi/linux/v4l2-controls.h            |    9 +
 include/uapi/linux/videodev2.h                |   42 +
 sound/soc/fsl/fsl_asrc.c                      |  144 ++
 sound/soc/fsl/fsl_asrc.h                      |    4 +-
 sound/soc/fsl/fsl_asrc_dma.c                  |    2 +-
 sound/soc/fsl/fsl_easrc.c                     |  233 ++++
 sound/soc/fsl/fsl_easrc.h                     |    6 +-
 38 files changed, 2967 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst
 create mode 100644 Documentation/userspace-api/media/v4l/ext-ctrls-audio-m2m.rst
 create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-audio.rst
 create mode 100644 drivers/media/platform/nxp/imx-asrc.c
 create mode 100644 drivers/media/test-drivers/vim2m-audio.c
 rename {sound/soc/fsl => include/sound}/fsl_asrc_common.h (60%)

Comments

Krzysztof Kozlowski Nov. 11, 2023, 8:16 a.m. UTC | #1
On 10/11/2023 06:48, Shengjiu Wang wrote:
> +static int asrc_m2m_probe(struct platform_device *pdev)
> +{
> +	struct fsl_asrc_m2m_pdata *data = pdev->dev.platform_data;
> +	struct device *dev = &pdev->dev;
> +	struct asrc_m2m *m2m;
> +	int ret;
> +
> +	m2m = devm_kzalloc(dev, sizeof(struct asrc_m2m), GFP_KERNEL);

sizeof(*)

> +	if (!m2m)
> +		return -ENOMEM;
> +
> +	m2m->pdata = *data;
> +	m2m->pdev = pdev;
> +
> +	ret = v4l2_device_register(dev, &m2m->v4l2_dev);
> +	if (ret) {
> +		dev_err(dev, "failed to register v4l2 device\n");
> +		goto err_register;
> +	}
> +
> +	m2m->m2m_dev = v4l2_m2m_init(&asrc_m2m_ops);
> +	if (IS_ERR(m2m->m2m_dev)) {
> +		dev_err(dev, "failed to register v4l2 device\n");

Why aren't you using dev_err_probe() at all?

> +		ret = PTR_ERR(m2m->m2m_dev);
> +		goto err_m2m;
> +	}
> +
> +	m2m->dec_vdev = video_device_alloc();
> +	if (!m2m->dec_vdev) {
> +		dev_err(dev, "failed to register v4l2 device\n");

Why do you print errors on ENOMEM?

Did you run coccinelle?

> +		ret = -ENOMEM;
> +		goto err_vdev_alloc;
> +	}
> +
> +	mutex_init(&m2m->mlock);
> +
> +	m2m->dec_vdev->fops = &asrc_m2m_fops;
> +	m2m->dec_vdev->ioctl_ops = &asrc_m2m_ioctl_ops;
> +	m2m->dec_vdev->minor = -1;
> +	m2m->dec_vdev->release = video_device_release;
> +	m2m->dec_vdev->lock = &m2m->mlock; /* lock for ioctl serialization */
> +	m2m->dec_vdev->v4l2_dev = &m2m->v4l2_dev;
> +	m2m->dec_vdev->vfl_dir = VFL_DIR_M2M;
> +	m2m->dec_vdev->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_AUDIO_M2M;
> +
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +	m2m->mdev.dev = &pdev->dev;
> +	strscpy(m2m->mdev.model, M2M_DRV_NAME, sizeof(m2m->mdev.model));
> +	snprintf(m2m->mdev.bus_info, sizeof(m2m->mdev.bus_info),
> +		 "platform:%s", M2M_DRV_NAME);
> +	media_device_init(&m2m->mdev);
> +	m2m->mdev.ops = &asrc_m2m_media_ops;
> +	m2m->v4l2_dev.mdev = &m2m->mdev;
> +#endif
> +
> +	ret = video_register_device(m2m->dec_vdev, VFL_TYPE_AUDIO, -1);
> +	if (ret) {
> +		dev_err(dev, "failed to register video device\n");
> +		goto err_vdev_register;
> +	}
> +
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +	ret = v4l2_m2m_register_media_controller(m2m->m2m_dev, m2m->dec_vdev,
> +						 MEDIA_ENT_F_PROC_AUDIO_RESAMPLER);
> +	if (ret) {
> +		dev_err(dev, "Failed to init mem2mem media controller\n");
> +		goto error_v4l2;
> +	}
> +
> +	ret = media_device_register(&m2m->mdev);
> +	if (ret) {
> +		dev_err(dev, "Failed to register mem2mem media device\n");
> +		goto error_m2m_mc;
> +	}
> +#endif
> +
> +	video_set_drvdata(m2m->dec_vdev, m2m);
> +	platform_set_drvdata(pdev, m2m);
> +	pm_runtime_enable(&pdev->dev);
> +
> +	return 0;
> +
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +error_m2m_mc:
> +	v4l2_m2m_unregister_media_controller(m2m->m2m_dev);
> +#endif
> +error_v4l2:
> +	video_unregister_device(m2m->dec_vdev);
> +err_vdev_register:
> +	video_device_release(m2m->dec_vdev);
> +err_vdev_alloc:
> +	v4l2_m2m_release(m2m->m2m_dev);
> +err_m2m:
> +	v4l2_device_unregister(&m2m->v4l2_dev);
> +err_register:
> +	return ret;
> +}
> +
> +static void asrc_m2m_remove(struct platform_device *pdev)
> +{
> +	struct asrc_m2m *m2m = platform_get_drvdata(pdev);
> +
> +	pm_runtime_disable(&pdev->dev);
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +	media_device_unregister(&m2m->mdev);
> +	v4l2_m2m_unregister_media_controller(m2m->m2m_dev);
> +#endif
> +	video_unregister_device(m2m->dec_vdev);
> +	video_device_release(m2m->dec_vdev);
> +	v4l2_m2m_release(m2m->m2m_dev);
> +	v4l2_device_unregister(&m2m->v4l2_dev);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +/* suspend callback for m2m */
> +static int asrc_m2m_suspend(struct device *dev)
> +{
> +	struct asrc_m2m *m2m = dev_get_drvdata(dev);
> +	struct fsl_asrc *asrc = m2m->pdata.asrc;
> +	struct fsl_asrc_pair *pair;
> +	unsigned long lock_flags;
> +	int i;
> +
> +	for (i = 0; i < PAIR_CTX_NUM; i++) {
> +		spin_lock_irqsave(&asrc->lock, lock_flags);
> +		pair = asrc->pair[i];
> +		if (!pair || !pair->req_pair) {
> +			spin_unlock_irqrestore(&asrc->lock, lock_flags);
> +			continue;
> +		}
> +		if (!completion_done(&pair->complete[V4L_OUT])) {
> +			if (pair->dma_chan[V4L_OUT])
> +				dmaengine_terminate_all(pair->dma_chan[V4L_OUT]);
> +			asrc_input_dma_callback((void *)pair);
> +		}
> +		if (!completion_done(&pair->complete[V4L_CAP])) {
> +			if (pair->dma_chan[V4L_CAP])
> +				dmaengine_terminate_all(pair->dma_chan[V4L_CAP]);
> +			asrc_output_dma_callback((void *)pair);
> +		}
> +
> +		if (asrc->m2m_pair_suspend)
> +			asrc->m2m_pair_suspend(pair);
> +
> +		spin_unlock_irqrestore(&asrc->lock, lock_flags);
> +	}
> +
> +	return 0;
> +}
> +
> +static int asrc_m2m_resume(struct device *dev)
> +{
> +	struct asrc_m2m *m2m = dev_get_drvdata(dev);
> +	struct fsl_asrc *asrc = m2m->pdata.asrc;
> +	struct fsl_asrc_pair *pair;
> +	unsigned long lock_flags;
> +	int i;
> +
> +	for (i = 0; i < PAIR_CTX_NUM; i++) {
> +		spin_lock_irqsave(&asrc->lock, lock_flags);
> +		pair = asrc->pair[i];
> +		if (!pair || !pair->req_pair) {
> +			spin_unlock_irqrestore(&asrc->lock, lock_flags);
> +			continue;
> +		}
> +		if (asrc->m2m_pair_resume)
> +			asrc->m2m_pair_resume(pair);
> +
> +		spin_unlock_irqrestore(&asrc->lock, lock_flags);
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops asrc_m2m_pm_ops = {
> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(asrc_m2m_suspend,
> +				      asrc_m2m_resume)
> +};
> +
> +static struct platform_driver asrc_m2m_driver = {
> +	.probe  = asrc_m2m_probe,
> +	.remove_new = asrc_m2m_remove,
> +	.driver = {
> +		.name = M2M_DRV_NAME,
> +		.pm = &asrc_m2m_pm_ops,
> +	},
> +};
> +module_platform_driver(asrc_m2m_driver);
> +
> +MODULE_DESCRIPTION("Freescale ASRC M2M driver");
> +MODULE_ALIAS("platform:" M2M_DRV_NAME);

You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong (e.g. misses either
entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
for incomplete ID table.


> +MODULE_LICENSE("GPL");

Best regards,
Krzysztof
Krzysztof Kozlowski Nov. 16, 2023, 11:15 a.m. UTC | #2
On 16/11/2023 09:32, Shengjiu Wang wrote:

>>> +MODULE_DESCRIPTION("Freescale ASRC M2M driver");
>>> +MODULE_ALIAS("platform:" M2M_DRV_NAME);
>>
>> You should not need MODULE_ALIAS() in normal cases. If you need it,
>> usually it means your device ID table is wrong (e.g. misses either
>> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
>> for incomplete ID table.
> 
> 
> This driver don't have MODULE_DEVICE_TABLE.  it is only registered
> by platform_device_register_data().

Which is the problem. I thought I made myself clear but if it is not the
case: drop MODULE_ALIAS.

Best regards,
Krzysztof