On 13. 09. 23 9:44, Cezary Rojewski wrote:
On 2023-09-12 6:30 PM, Jaroslav Kysela wrote:
On 11. 09. 23 17:45, Cezary Rojewski wrote:
On 2023-09-11 10:43 AM, Cezary Rojewski wrote:
...
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@perex.cz/
I do appreciate the input but I expected that, through guidance, this patch gets updated. Sending a separate patch, not connected to this patchset - not even a single reference within the commit message body - is not nice.
The basic API extension is self-contained and I marked it as RFC. The connection was added to this thread.
I'd rather send v2 with your patch incorporated as a part of the patchset.
No problem. Please, add these cosmetic changes to my patch:
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index f414f8fd217b..cb376e428f59 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -1412,9 +1412,10 @@ static int snd_pcm_hw_rule_subformats(struct snd_pcm_hw_params *params, struct snd_mask m; struct snd_mask *fmask = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); struct snd_mask *mask = hw_param_mask(params, SNDRV_PCM_HW_PARAM_SUBFORMAT); + bool found; + snd_mask_none(&m); snd_mask_set(&m, (__force unsigned)SNDRV_PCM_SUBFORMAT_STD); - bool found; pcm_for_each_format(k) { if (!snd_mask_test(fmask, k)) continue; @@ -1437,7 +1438,6 @@ static int snd_pcm_hw_rule_subformats(struct snd_pcm_hw_params *params, * @runtime: PCM runtime instance * @cond: condition bits * @subformats: array with struct snd_pcm_subformat elements - * @nmemd: size of array with struct snd_pcm_subformat elements * * This constraint will set relation between format and subformats. * The STD and MAX subformats are handled automatically. If the driver
Jaroslav