From: Mark Brown <broonie@kernel.org>
To: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc: Linux-ALSA <alsa-devel@alsa-project.org>
Subject: Re: [PATCH v2 2/7] ASoC: soc-core: add snd_soc_runtime_get_dai_fmt()
Date: Wed, 26 May 2021 00:20:25 +0100 [thread overview]
Message-ID: <YK2GOSIgiz2EmpUe@sirena.org.uk> (raw)
In-Reply-To: <87y2ckaifm.wl-kuninori.morimoto.gx@renesas.com>
[-- Attachment #1: Type: text/plain, Size: 3067 bytes --]
On Wed, May 12, 2021 at 09:45:33AM +0900, Kuninori Morimoto wrote:
Hi Morimoto-san,
First up sorry it's taken me a while to respond to this - I
wanted to get it clear in my head what I thought the best
approach is here. I think your code here is good with a couple
of documentation tweaks:
> For example, some driver want to select format "automatically",
> but want to select other fields "manually", because of complex limitation.
> Or other example, in case of both CPU and Codec are possible to be
> clock provider, but the quality was different.
> In these case, user need/want to *manually* select each fields
> from Sound Card driver.
>
> It uses Sound Card specified fields preferentially, and try to select
> non-specific fields from CPU and Codec driver settings if driver had
> callbacks.
> In other words, we can select all dai_fmt via Sound Card driver
> same as before.
I mentioned last time around the idea of having first and second
preferences for the DAIs, for things like "this will work but
only if you get the configuration exactly right". These patches
don't support that, they just treat everything the DAI reports as
being equally valid. I've actually come round to thinking that
might be OK for now but...
> +/**
> + * snd_soc_dai_get_fmt - get enable DAI hardware audio format.
s/get enable DAI hardware/get supported/
> + * @dai: DAI
> + * @fmt: SND_SOC_POSSIBLE_DAIFMT_* format value.
> + *
> + * Configures the DAI hardware format and clocking.
...I think here we should say something like
This should return only formats implemented with high
quality by the DAI so that the core can configure a
format which will work well with other devices. For
example devices which don't support both edges of the
LRCLK signal in I2S style formats should only list DSP
modes. This will mean that sometimes fewer formats
are reported here than are supported by set_fmt().
so that instead of worrying about formats that aren't quite
supported properly we just tell drivers not to list them for now
so if the system is relying on the framework to pick the DAI
format then we know it's one that's robust, we're not going to
choose one that the hardware in one of the devices isn't very
good at. Does that make sense to you?
If we did do first and second choices we'd get an algorithm like:
1. Try to match both DAI's 1st choice, if match use that.
2. Pick one DAI A, try it's 2nd choice with the other DAI B's 1st
choice.
3. Swap and try 1st choice of A and 2nd choice of B.
4. Try 2nd choices of both DAIs.
but I think that might get too complicated and we'd end up stuck
if we ever wanted to change the algorithm since DTs that don't
specify a format may break if a different format is picked. If
we go with ranking every format individually it gets even more
complicated for both driver authors and the core.
I think with the documentation tweak above your idea is a better
one for now, we can always go back later with more experience.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2021-05-25 23:20 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-12 0:45 [PATCH v2 0/7] ASoC: adds new .get_fmt support Kuninori Morimoto
2021-05-12 0:45 ` [PATCH v2 1/7] ASoC: soc-core: move snd_soc_runtime_set_dai_fmt() to upside Kuninori Morimoto
2021-05-12 0:45 ` [PATCH v2 2/7] ASoC: soc-core: add snd_soc_runtime_get_dai_fmt() Kuninori Morimoto
2021-05-25 23:20 ` Mark Brown [this message]
2021-05-26 2:46 ` Kuninori Morimoto
2021-05-26 22:54 ` Kuninori Morimoto
2021-05-26 23:19 ` Mark Brown
2021-05-12 0:45 ` [PATCH v2 3/7] ASoC: ak4613: add .get_fmt support Kuninori Morimoto
2021-05-12 0:45 ` [PATCH v2 4/7] ASoC: pcm3168a: " Kuninori Morimoto
2021-05-12 0:45 ` [PATCH v2 5/7] ASoC: rsnd: " Kuninori Morimoto
2021-05-12 0:45 ` [PATCH v2 6/7] ASoC: fsi: " Kuninori Morimoto
2021-05-12 0:45 ` [PATCH v2 7/7] ASoC: hdmi-codec: " Kuninori Morimoto
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=YK2GOSIgiz2EmpUe@sirena.org.uk \
--to=broonie@kernel.org \
--cc=alsa-devel@alsa-project.org \
--cc=kuninori.morimoto.gx@renesas.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.