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@pere...
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.
Czarek