On Fri, 17 Nov 2023 13:05:54 +0100, Cezary Rojewski wrote: > > Patchset aims to address format selection restrictions present currently > in the HDAudio library. Formats which we are concerned about are 20 and > 24 valid bits per sample within 32 bit depth container. One may identify > them as S20_LE and S24_LE except that those, according to comments found > in include/uapi/sound/asound.h, are for LSB-aligned scenarios. HDAudio > streams expect MSB-aligned data, no matter if we are speaking of HOST > (SDxFMT) or LINK (PPLCxFMT) side - chapter 4.5.1 of the public HDAudio > specification. In short, S20_LE and S24_LE are invalid options. > > Right now, given the implementation of snd_hdac_query_supported_pcm() > within sound/hda/hdac_device.c, even if a codec responds with: "I > support all possible formats specified within HDAudio specification", > there will be no option to open a 20/32 or 24/32 stream. The kernel will > force the stream to be opened using the highest available bit depth. > > After discussing subject initially with Jaroslav and Takashi, suggestion > was made to utilize 'subformat' option to address the problem. The > eye-opening discussion begun much earlier though, in 2019 [1]. > > Paired with PRs for alsa-utils [2] and alsa-lib [3]. > > > Flow of changes: > > The very first patch adds MSBITS subformat options to allow for granular > 20/32, 24/32 and 32/32 format selection. The next two make sure > subformat is actually honored during runtime. Most of that code is based > on format-related API. > > Follow up is upgrade to the hda stream-format interface - several > functions are added to make the granular format selection simple in the > HDAudio world. Core of the implementation is based on the existing > snd_hdac_calc_stream_format(). The next ten patches are straightforward > switch from one interface to another with cleanup of now-unsed function > as a finishing touch. > > Last but not least - the avs-driver, on which the problem analyzed and > debugged, is updated to no longer acknowledge S24_LE as a valid format > option. > > Results with skylake-driver and snd_hda_intel show status quo on our > RVPs. PR filed on SOF github shows promising results too [4]. > > > Changes in v5: > - reworded 'bps' to 'bits' in all occurrences. > - fixed an issue of MSBITS_MAX not being reported for S32_LE as reported > by Jaroslav > - snd_pcm_subformat_width() has been inlined into its only user: > snd_pcm_hw_params_bits() as suggested by Jaroslav. > - updated commit messege for patch 01 as it was out of date given the > recent updates. > > Changes in v4: > - fixed compilation issues in sof-driver, patch 12/16, reported by ikp > - fixed sparse warnings in patch 01/16, reported by ikp > - updated commit message for patch 03: "ASoC pcm: Honor subformat when > configuring runtime", as snd_pcm_hw_copy() is gone since revision v3. > > Changes in v3: > - merged the first two patches as suggested by Jaroslav > - re-authored patch 01 to Jaroslav, added my Co-developed-by. > - added Jaroslav' Co-developed-by to patch 02. > - 'subformats' field now S32_LE-specific. Given the fact that it is the > only format currently requiring subformat-intervention, functionality > is narrowed to reduce amount of memory allocations and cleanup. > Suggested by Jaroslav. > - note to the above: the hdaudio part converted 1:1 as requested, patch > 02/16 > - note #2: alsa part converted to S32_LE-specific yet without addition > of the chicken bit. Instead, struct snd_pcm_hardware is updated with > u32 subformat mask to do the job. > - ALSA-core additions in form of snd_pcm_subformat_width() and > snd_pcm_hw_params_bps() relocated from 01/16 to the user, patch 05. > > Changes in v2: > - patch 01/17, introduced struct snd_pcm_subformat which task is to > represent subformat-mask on per format basis. Expectation is that > manipulated arrays of subformats always end with a sentinel entry > - patch 01/17, added snd_pcm_hw_copy() as the copying snd_pcm_hardware > becomes non-trivial > - patch 02/17, added hw_rule that produces final subformat mask > based on provided formats as suggested by Jaroslav > - patch 04/17, soc_pcm_hw_update_subformat() refactored as the subformat > intersection becomes non-trivial > - relevant functions releasing resources occupied by hda_pcm and > snd_pcm_runtime updated to also kfree() subformats > - except for 16/17, no changes to patches past 04/17, retaining acks for > these > > Changes in v1: > - fixed UBSAN due to missing snd_pcm_subformat_names[] entries for new > subformats > - as HDMI stream capabilities are assigned on PCM open, patch 16/17 has > been updated to ignore such codecs for now. A separate patchset will > take care of this case > - params_bps() reworded to snd_pcm_hw_params_bps() > - fixed compilation issues in sof-driver, patch 13/17 > > > [1]: https://lore.kernel.org/alsa-devel/20190905053302.9262-1-pawel.harlozinski@xxxxxxxxxxxxxxx/ > [2]: https://github.com/alsa-project/alsa-utils/pull/228 > [3]: https://github.com/alsa-project/alsa-lib/pull/342 > [4]: https://github.com/thesofproject/linux/pull/4539 > > Cezary Rojewski (15): > ALSA: hda: Honor subformat when querying PCMs > ASoC: pcm: Honor subformat when configuring runtime > ALSA: hda: Upgrade stream-format infrastructure > ALSA: hda: Switch to new stream-format interface > ALSA: hda/hdmi: Switch to new stream-format interface > ALSA: hda/ca0132: Switch to new stream-format interface > ASoC: codecs: hda: Switch to new stream-format interface > ASoC: codecs: hdac_hda: Switch to new stream-format interface > ASoC: codecs: hdac_hdmi: Switch to new stream-format interface > ASoC: Intel Skylake: Switch to new stream-format interface > ASoC: SOF: Intel: Switch to new stream-format interface > ASoC: Intel: avs: Switch to new stream-format interface > ALSA: hda: Drop snd_hdac_calc_stream_format() > ASoC: Intel: avs: Kill S24_LE format > ASoC: Intel: avs: Unhardcode HDAudio BE DAI drivers description > > Jaroslav Kysela (1): > ALSA: pcm: Introduce MSBITS subformat interface Now the patch 1 became the one from Jaroslav, it's a good move. Jaroslav, could you take a look at the rest patches? I'm going to merge this in this week unless any objection comes up. thanks, Takashi
