All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
Cc: <alsa-devel@alsa-project.org>, <linux-kernel@vger.kernel.org>,
	<perex@perex.cz>, <tiwai@suse.com>,
	<pierre-louis.bossart@linux.intel.com>, <broonie@kernel.org>,
	<joe@perches.com>, <lgirdwood@gmail.com>, <lars@metafoo.de>,
	<kuninori.morimoto.gx@renesas.com>, <nicolas.ferre@microchip.com>,
	<Cristian.Birsan@microchip.com>
Subject: Re: [RFC PATCH 0/6] soc-pcm: Add separate snd_pcm_runtime for BEs
Date: Wed, 19 May 2021 16:15:08 +0200	[thread overview]
Message-ID: <s5him3eizdf.wl-tiwai@suse.de> (raw)
In-Reply-To: <20210519104842.977895-1-codrin.ciubotariu@microchip.com>

On Wed, 19 May 2021 12:48:36 +0200,
Codrin Ciubotariu wrote:
> 
> This patchset adds a different snd_pcm_runtime in the BE's substream,
> replacing the FE's snd_pcm_runtime. With a different structure, the BE
> HW capabilities and constraints will no longer merge with the FE ones.
> This allows for error detection if the be_hw_params_fixup() applies HW
> parameters not supported by the BE DAIs. Also, it calculates values
> needed for mem-to-dev/dev-to-mem DMA transfers, such as buffer size and
> period size, if needed.
> 
> The first 4 patches are preparatory patches, that just group and export
> functions used to allocate and initialize the snd_pcm_runtime. Also, the
> functions that set and apply the HW constraints are exported.
> The 5th patch does (almost) everything need to create the new snd_pcm_runtime
> for BEs, which includes allocation, initializing the HW capabilities,
> HW constraints and HW parameters. The BE HW parameters are no longer
> copied from the FE. They are recalculated, based on HW capabilities,
> constraints and the be_hw_params_fixup() callback.
> The 6th and last patch basically adds support for the PCM generic
> dmaengine to be used as a platform driver for BE DAI links. It allocates
> a buffer, needed by the DMA transfers that do not support dev-to-dev
> transfers between FE and BE DAIs.
> 
> This is a superset of
> https://mailman.alsa-project.org/pipermail/alsa-devel/2021-March/182630.html
> which only handles the BE HW constraints. This patchset aims to be more
> complete, defining a a snd_pcm_runtime between each FE and BE and can
> be used between any DAI link connection. I am sure I am not handling all
> the needed members of snd_pcm_runtime (such as handling
> struct snd_pcm_mmap_status *status), but I would like to have your
> feedback regarding this idea.

I'm also concerned about the handling of other fields in runtime
object, maybe allocating a complete runtime object for each BE is an
overkill and fragile.  Could it be rather only hw_constraints to be
unique for each BE, instead?

Also, the last patch allows only IRAM type, which sounds already
doubtful.  The dmaengine code should be generic.

Last but not least, one minor nitpick: please use EXPORT_SYMBOL_GPL()
for the newly introduced symbols.


thanks,

Takashi

WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai@suse.de>
To: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
Cc: alsa-devel@alsa-project.org, lars@metafoo.de,
	kuninori.morimoto.gx@renesas.com, lgirdwood@gmail.com,
	linux-kernel@vger.kernel.org, nicolas.ferre@microchip.com,
	pierre-louis.bossart@linux.intel.com, tiwai@suse.com,
	broonie@kernel.org, joe@perches.com,
	Cristian.Birsan@microchip.com
Subject: Re: [RFC PATCH 0/6] soc-pcm: Add separate snd_pcm_runtime for BEs
Date: Wed, 19 May 2021 16:15:08 +0200	[thread overview]
Message-ID: <s5him3eizdf.wl-tiwai@suse.de> (raw)
In-Reply-To: <20210519104842.977895-1-codrin.ciubotariu@microchip.com>

On Wed, 19 May 2021 12:48:36 +0200,
Codrin Ciubotariu wrote:
> 
> This patchset adds a different snd_pcm_runtime in the BE's substream,
> replacing the FE's snd_pcm_runtime. With a different structure, the BE
> HW capabilities and constraints will no longer merge with the FE ones.
> This allows for error detection if the be_hw_params_fixup() applies HW
> parameters not supported by the BE DAIs. Also, it calculates values
> needed for mem-to-dev/dev-to-mem DMA transfers, such as buffer size and
> period size, if needed.
> 
> The first 4 patches are preparatory patches, that just group and export
> functions used to allocate and initialize the snd_pcm_runtime. Also, the
> functions that set and apply the HW constraints are exported.
> The 5th patch does (almost) everything need to create the new snd_pcm_runtime
> for BEs, which includes allocation, initializing the HW capabilities,
> HW constraints and HW parameters. The BE HW parameters are no longer
> copied from the FE. They are recalculated, based on HW capabilities,
> constraints and the be_hw_params_fixup() callback.
> The 6th and last patch basically adds support for the PCM generic
> dmaengine to be used as a platform driver for BE DAI links. It allocates
> a buffer, needed by the DMA transfers that do not support dev-to-dev
> transfers between FE and BE DAIs.
> 
> This is a superset of
> https://mailman.alsa-project.org/pipermail/alsa-devel/2021-March/182630.html
> which only handles the BE HW constraints. This patchset aims to be more
> complete, defining a a snd_pcm_runtime between each FE and BE and can
> be used between any DAI link connection. I am sure I am not handling all
> the needed members of snd_pcm_runtime (such as handling
> struct snd_pcm_mmap_status *status), but I would like to have your
> feedback regarding this idea.

I'm also concerned about the handling of other fields in runtime
object, maybe allocating a complete runtime object for each BE is an
overkill and fragile.  Could it be rather only hw_constraints to be
unique for each BE, instead?

Also, the last patch allows only IRAM type, which sounds already
doubtful.  The dmaengine code should be generic.

Last but not least, one minor nitpick: please use EXPORT_SYMBOL_GPL()
for the newly introduced symbols.


thanks,

Takashi

  parent reply	other threads:[~2021-05-19 14:16 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-19 10:48 [RFC PATCH 0/6] soc-pcm: Add separate snd_pcm_runtime for BEs Codrin Ciubotariu
2021-05-19 10:48 ` Codrin Ciubotariu
2021-05-19 10:48 ` [RFC PATCH 1/6] ALSA: core: pcm: Create helpers to allocate/free struct snd_pcm_runtime Codrin Ciubotariu
2021-05-19 10:48   ` Codrin Ciubotariu
2021-05-19 10:48 ` [RFC PATCH 2/6] ALSA: pcm: Export constraints initialization functions Codrin Ciubotariu
2021-05-19 10:48   ` Codrin Ciubotariu
2021-05-19 10:48 ` [RFC PATCH 3/6] ALSA: pcm: Check for substream->ops before substream->ops->mmap Codrin Ciubotariu
2021-05-19 10:48   ` Codrin Ciubotariu
2021-05-19 10:48 ` [RFC PATCH 4/6] ALSA: pcm: Create function for snd_pcm_runtime initialization Codrin Ciubotariu
2021-05-19 10:48   ` Codrin Ciubotariu
2021-05-19 10:48 ` [RFC PATCH 5/6] ASoC: soc-pcm: Create new snd_pcm_runtime for BE DAIs Codrin Ciubotariu
2021-05-19 10:48   ` Codrin Ciubotariu
2021-05-19 10:48 ` [RFC PATCH 6/6] ASoC: dmaengine: Allocate buffer if substream is unmanaged Codrin Ciubotariu
2021-05-19 10:48   ` Codrin Ciubotariu
2021-05-19 14:15 ` Takashi Iwai [this message]
2021-05-19 14:15   ` [RFC PATCH 0/6] soc-pcm: Add separate snd_pcm_runtime for BEs Takashi Iwai
2021-05-19 15:08   ` Codrin.Ciubotariu
2021-05-19 15:08     ` Codrin.Ciubotariu
2021-05-19 15:41     ` Takashi Iwai
2021-05-19 15:41       ` Takashi Iwai
2021-05-20 13:59       ` Codrin.Ciubotariu
2021-05-20 13:59         ` Codrin.Ciubotariu
2021-05-21 14:26         ` Takashi Iwai
2021-05-21 14:26           ` Takashi Iwai
2021-05-24 19:33           ` Codrin.Ciubotariu
2021-05-24 19:33             ` Codrin.Ciubotariu

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=s5him3eizdf.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=Cristian.Birsan@microchip.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=codrin.ciubotariu@microchip.com \
    --cc=joe@perches.com \
    --cc=kuninori.morimoto.gx@renesas.com \
    --cc=lars@metafoo.de \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nicolas.ferre@microchip.com \
    --cc=perex@perex.cz \
    --cc=pierre-louis.bossart@linux.intel.com \
    --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.