On 11. 09. 23 17:45, Cezary Rojewski wrote:
On 2023-09-11 10:43 AM, Cezary Rojewski wrote:
On 2023-09-11 9:35 AM, Jaroslav Kysela wrote:
On 08. 09. 23 16:36, Cezary Rojewski wrote:
...
After reading all of this again, I'm fine with rewording MSBITS_32 to
MSBITS_MAX.
As I do not see any other points to address here and review of v1 has no
points to address either, I'll send v2 with this single change. If I'd
missed anything, let me know.
The subformat bitmask should be also refined/updated depending on the
selected format.
https://lore.kernel.org/alsa-devel/f97bbbd5-1397-f5d3-5ccf-420ec813deac@xxxxxxxx/
It requires new code in pcm_lib.c and ASoC PCM core code.
Could you help me understand what new code is needed? The
get_subformat() example raised more questions than answers. The patchset
defines snd_pcm_subformat_width(), perhaps you meant that I should
update that function by adding paramter 'format' to its parameters list
and handle it accordingly?
Any guidance would be much appreciated.
What I come up with is a hw_rule for subformat that I add in
snd_pcm_hw_constraints_init(). That piece, plus both STD and MSBITS_MAX
ORed into hw->subformats in snd_pcm_hw_constraints_complete() make
things spin.
static int snd_pcm_hw_rule_subformat(struct snd_pcm_hw_params *params,
struct snd_pcm_hw_rule *rule)
{
struct snd_mask *subformat_mask = hw_param_mask(params,
SNDRV_PCM_HW_PARAM_SUBFORMAT);
struct snd_mask *format_mask = hw_param_mask(params,
SNDRV_PCM_HW_PARAM_FORMAT);
snd_pcm_format_t f;
struct snd_mask m;
int width;
snd_mask_none(&m);
snd_mask_set(&m, SNDRV_PCM_SUBFORMAT_STD);
snd_mask_set(&m, SNDRV_PCM_SUBFORMAT_MSBITS_MAX);
pcm_for_each_format(f) {
if (!snd_mask_test_format(format_mask, f))
continue;
width = snd_pcm_format_width(f);
switch (width) {
case 32:
snd_mask_set(&m, SNDRV_PCM_SUBFORMAT_MSBITS_20);
snd_mask_set(&m, SNDRV_PCM_SUBFORMAT_MSBITS_24);
break;
default:
break;
}
}
return snd_mask_refine(subformat_mask, &m);
}
However, this means snd_hdac_query_supported_pcm() becomes confusing as
you need to MSBITS_MAX regardless of what the codec supports.
After spending additional few hours on this, I'd say I preferred how
things look with MSBITS_32 instead. STD and MSBITS_MAX existing
simultaneously is confusing too.
This is not a correct implementation. Many things are missing including the
proper subformats filter. I sent my own version here for discussion:
https://lore.kernel.org/alsa-devel/20230912162526.7138-1-perex@xxxxxxxx/
Jaroslav
--
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
