Skip to content

ALSA/ASoC: hda: Address format selection limitations and ambiguity #4539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
crojewsk-intel wants to merge 17 commits into from
Closed

ALSA/ASoC: hda: Address format selection limitations and ambiguity #4539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
33c4d5d
ALSA: pcm: Introduce MSBITS subformat interface
crojewsk-intel Aug 4, 2023
fcd069e
ALSA: pcm: Honor subformat when configuring substream
crojewsk-intel Aug 4, 2023
c3e1ccf
ALSA: hda: Honor subformat when querying PCMs
crojewsk-intel Aug 4, 2023
1f8b4a0
ASoC: pcm: Honor subformat when configuring runtime
crojewsk-intel Aug 10, 2023
f92e880
ALSA: hda: Upgrade stream-format infrastructure
crojewsk-intel Aug 10, 2023
ce49474
ALSA: hda: Switch to new stream-format interface
crojewsk-intel Aug 10, 2023
9259b37
ALSA: hda/hdmi: Switch to new stream-format interface
crojewsk-intel Aug 10, 2023
20c2126
ALSA: hda/ca0132: Switch to new stream-format interface
crojewsk-intel Aug 10, 2023
9f1bc04
ASoC: codecs: hda: Switch to new stream-format interface
crojewsk-intel Jul 26, 2023
5736c7e
ASoC: codecs: hdac_hda: Switch to new stream-format interface
crojewsk-intel Aug 10, 2023
a9bb7c8
ASoC: codecs: hdac_hdmi: Switch to new stream-format interface
crojewsk-intel Aug 10, 2023
015312b
ASoC: Intel Skylake: Switch to new stream-format interface
crojewsk-intel Aug 10, 2023
47ab5ec
ASoC: SOF: Intel: Switch to new stream-format interface
crojewsk-intel Aug 10, 2023
177d6b6
ASoC: Intel: avs: Switch to new stream-format interface
crojewsk-intel Jul 26, 2023
eb37244
ALSA: hda: Drop snd_hdac_calc_stream_format()
crojewsk-intel Aug 11, 2023
f623575
ASoC: Intel: avs: Kill S24_LE in HDAudio streaming
crojewsk-intel Aug 4, 2023
03d751b
ASoC: Intel: avs: Unhardcode HDAudio BE DAI drivers description
crojewsk-intel Jul 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions include/sound/hda_codec.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ struct hda_pcm_stream {
hda_nid_t nid; /* default NID to query rates/formats/bps, or set up */
u32 rates; /* supported rates */
u64 formats; /* supported formats (SNDRV_PCM_FMTBIT_) */
u64 subformats; /* supported subformats (SNDRV_PCM_SUBFMTBIT_) */
unsigned int maxbps; /* supported max. bit per sample */
const struct snd_pcm_chmap_elem *chmap; /* chmap to override */
struct hda_pcm_ops ops;
Expand Down Expand Up @@ -448,8 +449,8 @@ void __snd_hda_codec_cleanup_stream(struct hda_codec *codec, hda_nid_t nid,
#define snd_hda_codec_cleanup_stream(codec, nid) \
__snd_hda_codec_cleanup_stream(codec, nid, 0)

#define snd_hda_query_supported_pcm(codec, nid, ratesp, fmtsp, bpsp) \
snd_hdac_query_supported_pcm(&(codec)->core, nid, ratesp, fmtsp, bpsp)
#define snd_hda_query_supported_pcm(codec, nid, ratesp, fmtsp, subfmtp, bpsp) \
snd_hdac_query_supported_pcm(&(codec)->core, nid, ratesp, fmtsp, subfmtp, bpsp)
#define snd_hda_is_supported_format(codec, nid, fmt) \
snd_hdac_is_supported_format(&(codec)->core, nid, fmt)

Expand Down
12 changes: 6 additions & 6 deletions include/sound/hdaudio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,13 @@ int snd_hdac_get_connections(struct hdac_device *codec, hda_nid_t nid,
hda_nid_t *conn_list, int max_conns);
int snd_hdac_get_sub_nodes(struct hdac_device *codec, hda_nid_t nid,
hda_nid_t *start_id);
unsigned int snd_hdac_calc_stream_format(unsigned int rate,
unsigned int channels,
snd_pcm_format_t format,
unsigned int maxbps,
unsigned short spdif_ctls);
unsigned int snd_hdac_stream_format_bps(snd_pcm_format_t format, snd_pcm_subformat_t subformat,
unsigned int maxbps);
unsigned int snd_hdac_stream_format(unsigned int channels, unsigned int bps, unsigned int rate);
unsigned int snd_hdac_spdif_stream_format(unsigned int channels, unsigned int bps,
unsigned int rate, unsigned short spdif_ctls);
int snd_hdac_query_supported_pcm(struct hdac_device *codec, hda_nid_t nid,
u32 *ratesp, u64 *formatsp, unsigned int *bpsp);
u32 *ratesp, u64 *formatsp, u64 *subformatsp, unsigned int *bpsp);
bool snd_hdac_is_supported_format(struct hdac_device *codec, hda_nid_t nid,
unsigned int format);

Expand Down
8 changes: 8 additions & 0 deletions include/sound/pcm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
struct snd_pcm_hardware {
unsigned int info; /* SNDRV_PCM_INFO_* */
u64 formats; /* SNDRV_PCM_FMTBIT_* */
u64 subformats; /* SNDRV_PCM_SUBFMTBIT_* */
unsigned int rates; /* SNDRV_PCM_RATE_* */
unsigned int rate_min; /* min rate */
unsigned int rate_max; /* max rate */
Expand Down Expand Up @@ -219,6 +220,12 @@ struct snd_pcm_ops {
#define SNDRV_PCM_FMTBIT_U20 SNDRV_PCM_FMTBIT_U20_BE
#endif

#define _SNDRV_PCM_SUBFMTBIT(fmt) BIT((__force int)SNDRV_PCM_SUBFORMAT_##fmt)
#define SNDRV_PCM_SUBFMTBIT_STD _SNDRV_PCM_SUBFMTBIT(STD)
#define SNDRV_PCM_SUBFMTBIT_MSBITS_20 _SNDRV_PCM_SUBFMTBIT(MSBITS_20)
#define SNDRV_PCM_SUBFMTBIT_MSBITS_24 _SNDRV_PCM_SUBFMTBIT(MSBITS_24)
#define SNDRV_PCM_SUBFMTBIT_MSBITS_32 _SNDRV_PCM_SUBFMTBIT(MSBITS_32)

struct snd_pcm_file {
struct snd_pcm_substream *substream;
int no_compat_mmap;
Expand Down Expand Up @@ -1130,6 +1137,7 @@ int snd_pcm_format_physical_width(snd_pcm_format_t format); /* in bits */
ssize_t snd_pcm_format_size(snd_pcm_format_t format, size_t samples);
const unsigned char *snd_pcm_format_silence_64(snd_pcm_format_t format);
int snd_pcm_format_set_silence(snd_pcm_format_t format, void *buf, unsigned int frames);
int snd_pcm_subformat_width(snd_pcm_subformat_t subformat);

void snd_pcm_set_ops(struct snd_pcm * pcm, int direction,
const struct snd_pcm_ops *ops);
Expand Down
2 changes: 2 additions & 0 deletions include/sound/pcm_params.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,8 @@ static inline int params_physical_width(const struct snd_pcm_hw_params *p)
return snd_pcm_format_physical_width(params_format(p));
}

int snd_pcm_hw_params_bps(const struct snd_pcm_hw_params *p);

static inline void
params_set_format(struct snd_pcm_hw_params *p, snd_pcm_format_t fmt)
{
Expand Down
1 change: 1 addition & 0 deletions include/sound/soc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,7 @@ enum snd_soc_trigger_order {
struct snd_soc_pcm_stream {
const char *stream_name;
u64 formats; /* SNDRV_PCM_FMTBIT_* */
u64 subformats; /* SNDRV_PCM_SUBFMTBIT_* */
unsigned int rates; /* SNDRV_PCM_RATE_* */
unsigned int rate_min; /* min rate */
unsigned int rate_max; /* max rate */
Expand Down
5 changes: 4 additions & 1 deletion include/uapi/sound/asound.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,10 @@ typedef int __bitwise snd_pcm_format_t;

typedef int __bitwise snd_pcm_subformat_t;
#define SNDRV_PCM_SUBFORMAT_STD ((__force snd_pcm_subformat_t) 0)
#define SNDRV_PCM_SUBFORMAT_LAST SNDRV_PCM_SUBFORMAT_STD
#define SNDRV_PCM_SUBFORMAT_MSBITS_20 ((__force snd_pcm_subformat_t) 1)
#define SNDRV_PCM_SUBFORMAT_MSBITS_24 ((__force snd_pcm_subformat_t) 2)
#define SNDRV_PCM_SUBFORMAT_MSBITS_32 ((__force snd_pcm_subformat_t) 3)
#define SNDRV_PCM_SUBFORMAT_LAST SNDRV_PCM_SUBFORMAT_MSBITS_32

#define SNDRV_PCM_INFO_MMAP 0x00000001 /* hardware supports mmap */
#define SNDRV_PCM_INFO_MMAP_VALID 0x00000002 /* period data are valid during transfer */
Expand Down
3 changes: 3 additions & 0 deletions sound/core/pcm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,9 @@ static const char * const snd_pcm_access_names[] = {

static const char * const snd_pcm_subformat_names[] = {
SUBFORMAT(STD),
SUBFORMAT(MSBITS_20),
SUBFORMAT(MSBITS_24),
SUBFORMAT(MSBITS_32),
};

static const char * const snd_pcm_tstamp_mode_names[] = {
Expand Down
30 changes: 30 additions & 0 deletions sound/core/pcm_lib.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1706,6 +1706,36 @@ int snd_pcm_hw_param_last(struct snd_pcm_substream *pcm,
}
EXPORT_SYMBOL(snd_pcm_hw_param_last);

/**
* snd_pcm_hw_params_bps - Get the number of bits per the sample.
* @p: hardware parameters
*
* Return: The number of bits per sample based on the format,
* subformat and msbits the specified hw params has.
*/
int snd_pcm_hw_params_bps(const struct snd_pcm_hw_params *p)

ghost Aug 29, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to update the params_width() so all drivers will get to support the subformat out of box?

{
snd_pcm_subformat_t subformat = params_subformat(p);
snd_pcm_format_t format = params_format(p);
int width;

switch (format) {
case SNDRV_PCM_FORMAT_S32_LE:
case SNDRV_PCM_FORMAT_U32_LE:
case SNDRV_PCM_FORMAT_S32_BE:
case SNDRV_PCM_FORMAT_U32_BE:
width = snd_pcm_subformat_width(subformat);
if (width)
return width;
if (p->msbits)
return (int)p->msbits;
fallthrough;
default:
return snd_pcm_format_width(format);
}
}
EXPORT_SYMBOL(snd_pcm_hw_params_bps);

static int snd_pcm_lib_ioctl_reset(struct snd_pcm_substream *substream,
void *arg)
{
Expand Down
23 changes: 23 additions & 0 deletions sound/core/pcm_misc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,29 @@ int snd_pcm_format_set_silence(snd_pcm_format_t format, void *data, unsigned int
}
EXPORT_SYMBOL(snd_pcm_format_set_silence);

/**
* snd_pcm_subformat_width - return the bit-width of the subformat
* @subformat: the subformat to check
*
* Return: The bit-width of the subformat, or a negative error code
* otherwise.
*/
int snd_pcm_subformat_width(snd_pcm_subformat_t subformat)
{
switch (subformat) {
case SNDRV_PCM_SUBFORMAT_MSBITS_20:
return 20;
case SNDRV_PCM_SUBFORMAT_MSBITS_24:
return 24;
case SNDRV_PCM_SUBFORMAT_MSBITS_32:
return 32;
case SNDRV_PCM_SUBFORMAT_STD:
default:
return 0;
}
}
EXPORT_SYMBOL(snd_pcm_subformat_width);

/**
* snd_pcm_hw_limit_rates - determine rate_min/rate_max fields
* @hw: the pcm hw instance
Expand Down
5 changes: 3 additions & 2 deletions sound/core/pcm_native.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2614,6 +2614,8 @@ static int snd_pcm_hw_constraints_complete(struct snd_pcm_substream *substream)
int err;
unsigned int mask = 0;

/* All PCMs support at least the default STD subformat. */
hw->subformats |= PARAM_MASK_BIT(SNDRV_PCM_SUBFORMAT_STD);

ghost Aug 25, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no definition of SNDRV_PCM_SUBFORMAT_STD that I can find, but it looks to me that it is there to not fail when setting the mask?
SNDRV_PCM_SUBFORMAT_STD does not cover any supported subformat, it is just BIT(0) and to keep snd_pcm_hw_constraint_mask() not to fail few lines down?

ghost Aug 25, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

STD is the default and expected to be always supported by any device. So, when adding new flags I had to OR it here to avoid breaking audio for most of the world.

ghost Aug 29, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the upstream discussion, the SNDRV_PCM_SUBFORMAT_STD tells that the whole physical width is used?
But how that maps with S24_LE for example?

I would expect that drivers would set explicit subformats if they care and they might say that I'm not supporting STD and here you override that statement.

if (!hw->subformats)
	hw->subformats = PARAM_MASK_BIT(SNDRV_PCM_SUBFORMAT_STD);

might be what should be done here?

if (hw->info & SNDRV_PCM_INFO_INTERLEAVED)
mask |= PARAM_MASK_BIT(SNDRV_PCM_ACCESS_RW_INTERLEAVED);
if (hw->info & SNDRV_PCM_INFO_NONINTERLEAVED)
Expand All @@ -2634,8 +2636,7 @@ static int snd_pcm_hw_constraints_complete(struct snd_pcm_substream *substream)
if (err < 0)
return err;

err = snd_pcm_hw_constraint_mask(runtime, SNDRV_PCM_HW_PARAM_SUBFORMAT,
PARAM_MASK_BIT(SNDRV_PCM_SUBFORMAT_STD));
err = snd_pcm_hw_constraint_mask(runtime, SNDRV_PCM_HW_PARAM_SUBFORMAT, hw->subformats);

ghost Aug 25, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

snd_pcm_hw_constraint_mask64() ?

ghost Aug 25, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack.

if (err < 0)
return err;

Expand Down
Loading