| Message ID | 20190905053302.9262-2-pawel.harlozinski@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,1/3] ALSA:hda: Simplify and clear calculating SDxFMT | expand |
On Thu, 05 Sep 2019 07:33:00 +0200, Pawel Harlozinski wrote: > > Adds SNDRV_PCM_RATE_24000 at the bottom to keep backward compability > with alsa library. > > Signed-off-by: Pawel Harlozinski <pawel.harlozinski@linux.intel.com> No. Such a fancy rate has to be handled inside the driver locally instead of adding to the common rate. Takashi > --- > include/sound/pcm.h | 1 + > sound/core/pcm_native.c | 4 ++-- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/include/sound/pcm.h b/include/sound/pcm.h > index bbe6eb1ff5d2..09d0a2a2dce8 100644 > --- a/include/sound/pcm.h > +++ b/include/sound/pcm.h > @@ -119,6 +119,7 @@ struct snd_pcm_ops { > #define SNDRV_PCM_RATE_192000 (1<<12) /* 192000Hz */ > #define SNDRV_PCM_RATE_352800 (1<<13) /* 352800Hz */ > #define SNDRV_PCM_RATE_384000 (1<<14) /* 384000Hz */ > +#define SNDRV_PCM_RATE_24000 (1<<15) /* 24000Hz */ > > #define SNDRV_PCM_RATE_CONTINUOUS (1<<30) /* continuous range */ > #define SNDRV_PCM_RATE_KNOT (1<<31) /* supports more non-continuos rates */ > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c > index 11e653c8aa0e..f52f28e3edb1 100644 > --- a/sound/core/pcm_native.c > +++ b/sound/core/pcm_native.c > @@ -2164,13 +2164,13 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params, > return snd_interval_refine(hw_param_interval(params, rule->var), &t); > } > > -#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12 > +#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12 || SNDRV_PCM_RATE_24000 != 1 << 15 > #error "Change this table" > #endif > > static const unsigned int rates[] = { > 5512, 8000, 11025, 16000, 22050, 32000, 44100, > - 48000, 64000, 88200, 96000, 176400, 192000, 352800, 384000 > + 48000, 64000, 88200, 96000, 176400, 192000, 352800, 384000, 24000 > }; > > const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = { > -- > 2.17.1 >
On 9/5/19 12:48 AM, Takashi Iwai wrote: > On Thu, 05 Sep 2019 07:33:00 +0200, > Pawel Harlozinski wrote: >> >> Adds SNDRV_PCM_RATE_24000 at the bottom to keep backward compability >> with alsa library. >> >> Signed-off-by: Pawel Harlozinski <pawel.harlozinski@linux.intel.com> > > No. Such a fancy rate has to be handled inside the driver locally > instead of adding to the common rate. It's not that crazy, this is supported in the HDaudio spec: Sample Base Rate Divisor (DIV): 000 = Divide by 1 (48 kHz, 44.1 kHz) 001 = Divide by 2 (24 kHz, 22.05 kHz) I am not sure why 22.05 made the cut and not 24 kHz, they are both derived from common clocks with the same divider... Same for 11.025 and 12... > > > Takashi > >> --- >> include/sound/pcm.h | 1 + >> sound/core/pcm_native.c | 4 ++-- >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/include/sound/pcm.h b/include/sound/pcm.h >> index bbe6eb1ff5d2..09d0a2a2dce8 100644 >> --- a/include/sound/pcm.h >> +++ b/include/sound/pcm.h >> @@ -119,6 +119,7 @@ struct snd_pcm_ops { >> #define SNDRV_PCM_RATE_192000 (1<<12) /* 192000Hz */ >> #define SNDRV_PCM_RATE_352800 (1<<13) /* 352800Hz */ >> #define SNDRV_PCM_RATE_384000 (1<<14) /* 384000Hz */ >> +#define SNDRV_PCM_RATE_24000 (1<<15) /* 24000Hz */ >> >> #define SNDRV_PCM_RATE_CONTINUOUS (1<<30) /* continuous range */ >> #define SNDRV_PCM_RATE_KNOT (1<<31) /* supports more non-continuos rates */ >> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c >> index 11e653c8aa0e..f52f28e3edb1 100644 >> --- a/sound/core/pcm_native.c >> +++ b/sound/core/pcm_native.c >> @@ -2164,13 +2164,13 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params, >> return snd_interval_refine(hw_param_interval(params, rule->var), &t); >> } >> >> -#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12 >> +#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12 || SNDRV_PCM_RATE_24000 != 1 << 15 >> #error "Change this table" >> #endif >> >> static const unsigned int rates[] = { >> 5512, 8000, 11025, 16000, 22050, 32000, 44100, >> - 48000, 64000, 88200, 96000, 176400, 192000, 352800, 384000 >> + 48000, 64000, 88200, 96000, 176400, 192000, 352800, 384000, 24000 >> }; >> >> const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = { >> -- >> 2.17.1 >>
On Thu, 05 Sep 2019 15:11:43 +0200, Pierre-Louis Bossart wrote: > > On 9/5/19 12:48 AM, Takashi Iwai wrote: > > On Thu, 05 Sep 2019 07:33:00 +0200, > > Pawel Harlozinski wrote: > >> > >> Adds SNDRV_PCM_RATE_24000 at the bottom to keep backward compability > >> with alsa library. > >> > >> Signed-off-by: Pawel Harlozinski <pawel.harlozinski@linux.intel.com> > > > > No. Such a fancy rate has to be handled inside the driver locally > > instead of adding to the common rate. > > It's not that crazy, this is supported in the HDaudio spec: > > Sample Base Rate Divisor (DIV): > 000 = Divide by 1 (48 kHz, 44.1 kHz) > 001 = Divide by 2 (24 kHz, 22.05 kHz) > > I am not sure why 22.05 made the cut and not 24 kHz, they are both > derived from common clocks with the same divider... Same for 11.025 > and 12... I'm not against supporting it. It'd be fine if it were the changes that are applied only to HD-audio driver side. What I'm against is to change the ALSA PCM core. It's not necessarily done there at all. thanks, Takashi
>>>> Adds SNDRV_PCM_RATE_24000 at the bottom to keep backward compability >>>> with alsa library. >>>> >>>> Signed-off-by: Pawel Harlozinski <pawel.harlozinski@linux.intel.com> >>> >>> No. Such a fancy rate has to be handled inside the driver locally >>> instead of adding to the common rate. >> >> It's not that crazy, this is supported in the HDaudio spec: >> >> Sample Base Rate Divisor (DIV): >> 000 = Divide by 1 (48 kHz, 44.1 kHz) >> 001 = Divide by 2 (24 kHz, 22.05 kHz) >> >> I am not sure why 22.05 made the cut and not 24 kHz, they are both >> derived from common clocks with the same divider... Same for 11.025 >> and 12... > > I'm not against supporting it. It'd be fine if it were the changes > that are applied only to HD-audio driver side. What I'm against is to > change the ALSA PCM core. It's not necessarily done there at all. Humm, out of curiosity what is the issue here? Would this addition break anything? I don't personally care too much but I've never quite understood why the ALSA core only defined a subset of 'common' rates.
On Thu, 05 Sep 2019 16:00:51 +0200, Pierre-Louis Bossart wrote: > > > >>>> Adds SNDRV_PCM_RATE_24000 at the bottom to keep backward compability > >>>> with alsa library. > >>>> > >>>> Signed-off-by: Pawel Harlozinski <pawel.harlozinski@linux.intel.com> > >>> > >>> No. Such a fancy rate has to be handled inside the driver locally > >>> instead of adding to the common rate. > >> > >> It's not that crazy, this is supported in the HDaudio spec: > >> > >> Sample Base Rate Divisor (DIV): > >> 000 = Divide by 1 (48 kHz, 44.1 kHz) > >> 001 = Divide by 2 (24 kHz, 22.05 kHz) > >> > >> I am not sure why 22.05 made the cut and not 24 kHz, they are both > >> derived from common clocks with the same divider... Same for 11.025 > >> and 12... > > > > I'm not against supporting it. It'd be fine if it were the changes > > that are applied only to HD-audio driver side. What I'm against is to > > change the ALSA PCM core. It's not necessarily done there at all. > > Humm, out of curiosity what is the issue here? Would this addition > break anything? I don't personally care too much but I've never quite > understood why the ALSA core only defined a subset of 'common' rates. It's simply a policy that we don't add a thing just because one driver wants for some reason possibly no one would actually use. In general such a core stuff is changed only when it has to be and inevitably necessary, and/or it'd be benefit for all the rest and majority users. thanks, Takashi
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index bbe6eb1ff5d2..09d0a2a2dce8 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -119,6 +119,7 @@ struct snd_pcm_ops { #define SNDRV_PCM_RATE_192000 (1<<12) /* 192000Hz */ #define SNDRV_PCM_RATE_352800 (1<<13) /* 352800Hz */ #define SNDRV_PCM_RATE_384000 (1<<14) /* 384000Hz */ +#define SNDRV_PCM_RATE_24000 (1<<15) /* 24000Hz */ #define SNDRV_PCM_RATE_CONTINUOUS (1<<30) /* continuous range */ #define SNDRV_PCM_RATE_KNOT (1<<31) /* supports more non-continuos rates */ diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 11e653c8aa0e..f52f28e3edb1 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2164,13 +2164,13 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params, return snd_interval_refine(hw_param_interval(params, rule->var), &t); } -#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12 +#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12 || SNDRV_PCM_RATE_24000 != 1 << 15 #error "Change this table" #endif static const unsigned int rates[] = { 5512, 8000, 11025, 16000, 22050, 32000, 44100, - 48000, 64000, 88200, 96000, 176400, 192000, 352800, 384000 + 48000, 64000, 88200, 96000, 176400, 192000, 352800, 384000, 24000 }; const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {
Adds SNDRV_PCM_RATE_24000 at the bottom to keep backward compability with alsa library. Signed-off-by: Pawel Harlozinski <pawel.harlozinski@linux.intel.com> --- include/sound/pcm.h | 1 + sound/core/pcm_native.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)